CEX-65 new encryption#282
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: lokalise/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request introduces a new Sequence DiagramssequenceDiagram
actor User
participant App as Application
participant Parser as parseEnvelopeEncryptorConfig
participant ConfigScope
participant Encryptor as EnvelopeEncryptor
participant Crypto as AES-256-GCM
User->>App: Initialize application
App->>Parser: Parse encryption config
Parser->>ConfigScope: Read ENCRYPTION_KEYS,<br/>ENCRYPTION_ACTIVE_KEY_ID,<br/>ENCRYPTION_HASH_PEPPER
ConfigScope-->>Parser: Config values
Parser->>Parser: Validate & decode<br/>base64 keys
Parser-->>App: EnvelopeEncryptorConfig
App->>Encryptor: new EnvelopeEncryptor(config)
Encryptor->>Encryptor: Validate config at<br/>construction
Encryptor-->>App: Ready instance
Note over App,Crypto: Encryption flow
User->>App: encrypt(plaintext)
App->>Encryptor: encrypt(plaintext)
Encryptor->>Crypto: Generate random IV
Crypto-->>Encryptor: IV
Encryptor->>Crypto: AES-256-GCM encrypt<br/>with active key
Crypto-->>Encryptor: ciphertext + authTag
Encryptor->>Encryptor: Format envelope:<br/>prefix+keyId:base64url(iv||authTag||ciphertext)
Encryptor-->>App: envelope
App-->>User: encrypted result
Note over App,Crypto: Decryption flow
User->>App: decrypt(envelope)
App->>Encryptor: decrypt(envelope)
Encryptor->>Encryptor: Check envelope prefix
alt Has prefix
Encryptor->>Encryptor: Parse keyId & extract<br/>iv||authTag||ciphertext
Encryptor->>Encryptor: Lookup key by keyId
alt Key found
Encryptor->>Crypto: AES-256-GCM decrypt<br/>with selected key
Crypto-->>Encryptor: plaintext (or auth fail)
Encryptor-->>App: plaintext
else Key not found
Encryptor-->>App: EncryptionKeyNotConfiguredError
end
else No prefix
Encryptor-->>App: Plaintext passthrough
end
App-->>User: decrypted result
sequenceDiagram
actor User
participant Encryptor as EnvelopeEncryptor
participant HMAC as HMAC-SHA256
participant User2 as Lookup Hash
User->>Encryptor: hash(plaintext)
Encryptor->>HMAC: HMAC-SHA256(pepper, plaintext)
HMAC-->>Encryptor: hex digest (64 chars)
Encryptor-->>User2: Deterministic hash
Note over Encryptor,User2: Hash is independent<br/>of encryption key rotation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/utils/envelopeEncryptor/EnvelopeEncryptor.ts`:
- Around line 181-183: encryptJson currently calls JSON.stringify(value) and
passes the result directly to this.encrypt, which leads to a raw crypto error
when JSON.stringify returns undefined for non-serializable top-level values
(e.g., undefined, functions, symbols). Update encryptJson (and keep reference to
this.encrypt) to capture the result of JSON.stringify, check if it returned
undefined, and if so throw a clear, descriptive application error (e.g., "Value
is not JSON-serializable") before calling this.encrypt; otherwise pass the
stringified value into this.encrypt as before.
- Around line 105-107: The constructor of EnvelopeEncryptor stores caller-owned
references (config.keys, config.hashPepper) which allows external mutation; fix
by cloning the key material on assignment: create a new Map for this.keys and
for each entry copy the key id and clone the Buffer value (e.g.,
Buffer.from(value)) before inserting, assign this.activeKeyId from
config.activeKeyId (string copy) and clone this.hashPepper with
Buffer.from(config.hashPepper) so the class holds independent buffers; update
the constructor assignment logic in EnvelopeEncryptor to use these clones.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: lokalise/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a7212db-a525-413f-94bc-fe68326ea6c5
📒 Files selected for processing (7)
src/index.tssrc/utils/encryptionUtility.tssrc/utils/envelopeEncryptor/EnvelopeEncryptor.spec.tssrc/utils/envelopeEncryptor/EnvelopeEncryptor.tssrc/utils/envelopeEncryptor/envelopeEncryptorErrors.tssrc/utils/envelopeEncryptor/parseEnvelopeEncryptorConfig.spec.tssrc/utils/envelopeEncryptor/parseEnvelopeEncryptorConfig.ts
Changes
Please describe
Checklist
major,minor,patchorskip-release