fix(encryption): decrypt fields in nested read results #1934
Conversation
📝 WalkthroughWalkthroughThe pull request introduces enhancements to the encryption handling in the runtime, specifically in the Changes
Sequence DiagramsequenceDiagram
participant Client
participant EncryptedHandler
participant DataModel
Client->>EncryptedHandler: Attempt to decrypt/encrypt data
EncryptedHandler->>EncryptedHandler: Check for null/empty values
alt Valid data
EncryptedHandler->>DataModel: Process nested structures
alt Nested array
EncryptedHandler->>EncryptedHandler: Recursively decrypt/encrypt items
end
DataModel-->>EncryptedHandler: Return processed data
else Invalid data
EncryptedHandler-->>Client: Skip processing, return original value
end
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/runtime/src/enhancements/node/encryption.ts (3)
144-144: Confirm error-handling around this field resolution.
IfresolveFieldthrows or returns an unexpected result for certain models, you should consider a try-catch or additional checks to avoid silently skipping fields.
149-156: Consider performance impact of recursively decrypting large arrays.
Recursively decrypting deep or large arrays might be expensive. If performance is a concern, you might consider batching or parallelizing these decrypt operations, if the underlying structure allows it.
157-163: Log the field name on decryption failure for better debugging.
When decryption fails, the warning logs the error but omits which field is causing the issue. Consider improving the log message to include the model and field name for more straightforward debugging.- this.logger.warn(`Decryption failed, keeping original value: ${error}`); + this.logger.warn(`Decryption failed for ${realModel}.${field}: ${error} -- keeping original value`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime/src/enhancements/node/encryption.ts(1 hunks)tests/integration/tests/enhancements/with-encrypted/with-encrypted.test.ts(1 hunks)
🔇 Additional comments (3)
packages/runtime/src/enhancements/node/encryption.ts (1)
141-142: Validate the falsy check to include empty strings as intended.
Your comment says "Don't decrypt null, undefined or empty string," but the actual logic only checks if (!entityData[field]), which handles null/undefined but also interprets empty strings ('') as falsy. Ensure that this behavior is intentional, given that '' is a valid string content.
tests/integration/tests/enhancements/with-encrypted/with-encrypted.test.ts (2)
63-77: Ensure that empty or null titles are tested.
The model Post includes an @encrypted title, but the test does not cover the scenario where title might be empty or null. Adding negative or edge-case tests ensures that your new decryption checks (lines 141-142 in encryption.ts) are fully validated.
78-107: Well-structured test verifying nested decryption logic.
Creating a parent record with an embedded post, then verifying that raw queries do not reveal the plaintext, effectively covers the new nested decryption feature. Great job!
No description provided.