refactor(encryption): extract standalone encrypter/decrypter #1945
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive encryption and decryption system in the runtime package. The changes include creating two new classes, Changes
Sequence DiagramsequenceDiagram
participant Client
participant Encrypter
participant Decrypter
participant CryptoAPI
Client->>Encrypter: encrypt(data)
Encrypter->>CryptoAPI: loadKey()
Encrypter->>CryptoAPI: getKeyDigest()
Encrypter->>CryptoAPI: encrypt data
Encrypter-->>Client: encrypted data
Client->>Decrypter: decrypt(data)
Decrypter->>CryptoAPI: loadKey()
Decrypter->>CryptoAPI: getKeyDigest()
Decrypter->>CryptoAPI: decrypt data
Decrypter-->>Client: decrypted data
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: 2
🧹 Nitpick comments (2)
packages/runtime/src/encryption/index.ts (2)
20-26: Optimize key initialization to avoid redundant operationsIn the
encryptmethod,this.keyandthis.keyDigestare initialized when they are undefined. Ifencryptis called concurrently before these properties are set, it could result in multiple initializations, potentially impacting performance. Consider initializingthis.keyandthis.keyDigestin the constructor or implementing a mechanism (such as a promise-based initialization guard) to ensure they are loaded only once.
54-61: Prevent multiple initializations of decryption keysIn the
decryptmethod,this.keysis populated when it is empty. Ifdecryptis called concurrently beforethis.keysis initialized, it may lead to redundant initializations. To improve efficiency, consider initializingthis.keysin the constructor or using a single initialization promise to ensure the keys are loaded only once.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/runtime/package.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
packages/runtime/src/encryption/index.ts(1 hunks)packages/runtime/src/encryption/utils.ts(1 hunks)packages/runtime/src/enhancements/node/encryption.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
🔇 Additional comments (1)
packages/runtime/src/encryption/utils.ts (1)
1-96: LGTM!The encryption utilities are well-implemented, adhering to best practices with appropriate error handling and input validation.
| private encrypter: Encrypter | undefined; | ||
| private decrypter: Decrypter | undefined; |
There was a problem hiding this comment.
Ensure 'this.encrypter' is defined before usage to prevent runtime errors
In cases where custom encryption is used, this.encrypter remains undefined, but in the encrypt method, this.encrypter!.encrypt(data) is called without checking if this.encrypter is defined. This could lead to a runtime error if this.encrypter is undefined. Consider adding a check to ensure this.encrypter is defined before using it or refactor the code to guarantee that this.encrypter is always initialized when not using custom encryption.
Apply this diff to check if this.encrypter is defined:
+ if (!this.encrypter) {
+ throw new Error('Encrypter is not initialized');
+ }
return this.encrypter.encrypt(data);Also applies to: 82-82
| private readonly ENCRYPTER_VERSION = 1; | ||
| private readonly KEY_DIGEST_BYTES = 8; | ||
| private encrypter: Encrypter | undefined; | ||
| private decrypter: Decrypter | undefined; |
There was a problem hiding this comment.
Ensure 'this.decrypter' is defined before usage to prevent runtime errors
Similarly, when custom encryption is employed, this.decrypter remains undefined. In the decrypt method, calling this.decrypter!.decrypt(data) without verifying that this.decrypter is defined may result in a runtime error. Please ensure this.decrypter is defined before usage or modify the code to handle scenarios where custom encryption is used.
Apply this diff to check if this.decrypter is defined:
+ if (!this.decrypter) {
+ throw new Error('Decrypter is not initialized');
+ }
return this.decrypter.decrypt(data);Also applies to: 90-90
No description provided.