-
-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(encryption): rename encrypted to encryption #1928
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a significant refactoring of the encryption functionality in the runtime package. The changes involve removing the old Changes
Sequence DiagramsequenceDiagram
participant Client
participant Prisma
participant EncryptedHandler
participant Encryption
Client->>Prisma: Apply withEncrypted
Prisma->>EncryptedHandler: Create proxy
EncryptedHandler->>Encryption: Validate options
Client->>Prisma: Write data
Prisma->>EncryptedHandler: Preprocess write payload
EncryptedHandler->>Encryption: Encrypt fields
Prisma->>Client: Save encrypted data
Client->>Prisma: Read data
Prisma->>EncryptedHandler: Process result
EncryptedHandler->>Encryption: Decrypt fields
EncryptedHandler->>Client: Return decrypted data
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/runtime/src/enhancements/edge/encryption.ts (3)
1-1
: Remove or clarify unused reference on line 1
It appears that../node/encryption.ts
is left at the top of this file without being imported, which can cause confusion. If this file reference is no longer needed, consider removing it or converting it to a comment to clarify its purpose.-../node/encryption.ts +// ../node/encryption.ts // Clarify or remove if unused
Line range hint
21-32
: Ensure robust validation of encryption options
Within the constructor, the checks for encryption options are good. However, storingoptions.encryption
in a local member variable can improve readability and maintainability (instead of accessingthis.options.encryption!
repeatedly).constructor(prisma: DbClientContract, model: string, options: InternalEnhancementOptions) { super(prisma, model, options); + const encryptionConfig = options.encryption; if (!options.encryption) { throw this.queryUtils.unknownError('Encryption options must be provided'); } if (this.isCustomEncryption(options.encryption!)) { if (!options.encryption.encrypt || !options.encryption.decrypt) { throw this.queryUtils.unknownError('Custom encryption must provide encrypt and decrypt functions'); } } else { if (!options.encryption.encryptionKey) { throw this.queryUtils.unknownError('Encryption key must be provided'); } if (options.encryption.encryptionKey.length !== 32) { throw this.queryUtils.unknownError('Encryption key must be 32 bytes'); } } }
Line range hint
55-78
: Cache imported CryptoKey to improve performance
Repeatedly callingcrypto.subtle.importKey
insideencrypt
can be expensive if this method is invoked frequently. Consider caching the imported CryptoKey in the constructor or a lazy-loaded property.private cachedKey: CryptoKey | undefined; private async encrypt(field: FieldInfo, data: string): Promise<string> { const encryption = this.options.encryption!; if (this.isCustomEncryption(encryption)) { return encryption.encrypt(this.model, field, data); } - const key = await this.getKey(encryption.encryptionKey); + const key = this.cachedKey ?? await this.getKey(encryption.encryptionKey); + this.cachedKey = key; const iv = crypto.getRandomValues(new Uint8Array(12)); ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime/src/enhancements/edge/encrypted.ts
(0 hunks)packages/runtime/src/enhancements/edge/encryption.ts
(1 hunks)packages/runtime/src/enhancements/node/create-enhancement.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/runtime/src/enhancements/edge/encrypted.ts
🔇 Additional comments (3)
packages/runtime/src/enhancements/edge/encryption.ts (2)
Line range hint 123-147
: Confirm consistent encryption across all field types
Within preprocessWritePayload
, only string fields are encrypted. If a future use case expands encryption to other data types (e.g., JSON or Buffer fields), this logic might need adjustment.
Consider searching the codebase for all @encrypted
usage to ensure only string fields are annotated:
✅ Verification successful
Based on the search results, I can provide the final response:
@encrypted
attribute is correctly restricted to String fields only
The codebase shows consistent handling of encrypted fields:
- The
@encrypted
attribute is explicitly restricted to String fields in the schema definition (stdlib.zmodel
):@@@targetField([StringField])
- The encryption implementation in
packages/runtime/src/enhancements/node/encryption.ts
validates field type:if (encAttr && field.type === 'String')
- Test cases confirm this restriction:
- All valid test cases use String fields with
@encrypted
- There's a test verifying that using
@encrypted
on a Bytes field fails with "attribute '@encrypted' cannot be used on this type of field
"
- All valid test cases use String fields with
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for @encrypted usage throughout the project
rg -A 4 '@encrypted'
Length of output: 9659
Line range hint 80-101
: Validate ciphertext length prior to decryption
The decryption method expects at least 12 bytes for the IV. Consider validating the decrypted byte array length before slicing to avoid runtime errors on malformed or truncated data.
const bytes = Uint8Array.from(atob(data), (c) => c.charCodeAt(0));
-const decrypted = await crypto.subtle.decrypt(
- {
- name: 'AES-GCM',
- iv: bytes.slice(0, 12),
- },
- key,
- bytes.slice(12)
-);
+if (bytes.length < 12) {
+ throw this.queryUtils.unknownError('Ciphertext too short to contain IV');
+}
+const decrypted = await crypto.subtle.decrypt(
+ {
+ name: 'AES-GCM',
+ iv: bytes.slice(0, 12),
+ },
+ key,
+ bytes.slice(12)
+);
packages/runtime/src/enhancements/node/create-enhancement.ts (1)
13-13
: Encapsulate encryption import usage
The addition of withEncrypted
is a logical replacement for the removed encrypted.ts
. Confirm that all references to the old file are removed and that no stale imports remain.
✅ Verification successful
No stale imports or references to old encrypted.ts
found
Based on the comprehensive search results, there are no references to the old encrypted.ts
file in the codebase. The encryption-related code is properly organized with:
- The new
withEncrypted
import from './encryption' - Consistent usage of encryption-related functionality across the codebase
- No stale or conflicting imports detected
The migration from the old encrypted.ts
to the new structure appears to be complete and clean.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any reference to the old `encrypted.ts` file
rg 'encrypted.ts'
Length of output: 17
Script:
#!/bin/bash
# Search for any references to 'encrypted' in the codebase
rg -i 'encrypted' --type ts
Length of output: 7699
Script:
#!/bin/bash
# Search for any imports related to encryption
ast-grep --pattern 'import { $_ } from $path' | grep -i 'encrypt'
Length of output: 65
Script:
#!/bin/bash
# Find all TypeScript files that might contain encryption-related code
fd -e ts -e tsx | xargs grep -l -i 'encrypt'
Length of output: 501
No description provided.