feat(encryption): add encryption plugin to close feature gap#665
feat(encryption): add encryption plugin to close feature gap#665genu wants to merge 1 commit intozenstackhq:devfrom
Conversation
📝 WalkthroughWalkthroughA new encryption plugin for ZenStack ORM is introduced, featuring AES-256-GCM encryption with key rotation support. The plugin transparently encrypts and decrypts fields marked with the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@packages/plugins/encryption/src/plugin.ts`:
- Around line 232-241: The current catch in the encrypted-field handling block
(isEncryptedField, decryptValue, data[fieldName], modelName, fieldName) leaves
the original ciphertext in data on decryption errors; change the catch to avoid
returning ciphertext by assigning a safe sentinel (e.g., data[fieldName] = null
or undefined) and logging the error details (include modelName/fieldName and the
caught error) instead of only console.warn; alternatively, if callers require
plaintext, rethrow the error from the catch so upstream can handle it—pick one
strategy and apply it consistently in this block.
- Around line 274-275: Replace the JSON.parse(JSON.stringify(args)) deep-clone
(which corrupts Date/BigInt/undefined/etc.) with a proper structured clone for
processedArgs before calling proceed: use structuredClone(args) when available,
and fall back to v8.serialize/deserialize (require('v8').serialize/deserialize)
for older Node versions; ensure you still preserve the existing behavior when
args is undefined and keep the variable name processedArgs and its use in the
call to proceed unchanged.
- Around line 264-266: The code in onQuery is using an untyped cast to access
the schema; replace "(client as any).schema" with the typed property
"client.$schema" and assign that to schema (keeps SchemaDef type). Update the
destructuring in onQuery (ctx) to use client.$schema when creating the schema
variable so the code uses the ClientContract's typed schema consistently with
other plugins like policy.
In `@packages/plugins/encryption/src/utils.ts`:
- Around line 56-64: The return uses String.fromCharCode(...cipherBytes) which
will crash for large inputs; in the _encrypt function replace the spread call
with a safe conversion that builds the binary string in chunks (e.g., loop over
cipherBytes in slices of ~32k bytes and call String.fromCharCode on each slice
then join) before btoa; reference the symbols _encrypt, cipherBytes and the
return expression so you update that line to concatenate chunked
String.fromCharCode results (do not change _decrypt since its Uint8Array.from
usage is already safe).
In `@tests/e2e/orm/client-api/encrypted.test.ts`:
- Around line 264-272: The test creates a wrapped client via
client.$use(newPlugin) assigned to client2 but only calls client.$disconnect(),
which can leak resources held by the wrapper; update the teardown to also
disconnect client2 (call client2.$disconnect() after the test actions or in the
same cleanup block) so both client and client2 are properly closed, ensuring any
resources held by the $use wrapper are released.
- Around line 125-135: The test named "handles null values gracefully" actually
creates a non-optional encrypted field (secretToken) with an empty string via
client.user.create and asserts ''. Rename the test to something like "handles
empty-string encrypted values" OR add a new test that uses an optional encrypted
field (e.g., optionalSecretToken or a nullable User model field) and create a
user with that field set to null, then assert the returned value is null; update
the test title and assertions accordingly and reference the existing
client.user.create usage and secretToken field in your changes.
- Around line 90-103: The test "encrypts and decrypts a single field on
findMany" is order-dependent because client.user.findMany() lacks an orderBy;
update the call to include a deterministic ordering (e.g., orderBy: { email:
'asc' }) or sort the returned users before asserting so users[0].secretToken and
users[1].secretToken are stable; modify the client.user.findMany invocation in
that test to include the orderBy clause (or apply a stable sort) to eliminate
flakiness.
🧹 Nitpick comments (6)
packages/plugins/encryption/src/utils.ts (1)
83-84: Parsed version (v) is unused — consider validating it.The version field is extracted from the encryption metadata but never checked. If a future version changes the format, decryption would silently attempt the wrong logic. Consider adding a version check:
- const { a: algorithm, k: keyDigest } = encryptionMetaSchema.parse(metaObj); + const { v: version, a: algorithm, k: keyDigest } = encryptionMetaSchema.parse(metaObj); + if (version !== ENCRYPTER_VERSION) { + throw new Error(`Unsupported encryption version: ${version}`); + }packages/plugins/encryption/src/encrypter.ts (1)
19-29: Minor: concurrent calls can trigger duplicate key loading.If multiple
encrypt()calls are in-flight before the first one resolves, each will independently callloadKey/getKeyDigestsincethis.keyis stillundefined. The race is benign (results are equivalent), but you could avoid the redundant work by caching the initialization promise:♻️ Optional lazy-init pattern
export class Encrypter { - private key: CryptoKey | undefined; - private keyDigest: string | undefined; + private initPromise: Promise<void> | undefined; + private key!: CryptoKey; + private keyDigest!: string; constructor(private readonly encryptionKey: Uint8Array) { if (encryptionKey.length !== ENCRYPTION_KEY_BYTES) { throw new Error(`Encryption key must be ${ENCRYPTION_KEY_BYTES} bytes`); } } + private init(): Promise<void> { + if (!this.initPromise) { + this.initPromise = (async () => { + this.key = await loadKey(this.encryptionKey, ['encrypt']); + this.keyDigest = await getKeyDigest(this.encryptionKey); + })(); + } + return this.initPromise; + } + async encrypt(data: string): Promise<string> { - if (!this.key) { - this.key = await loadKey(this.encryptionKey, ['encrypt']); - } - - if (!this.keyDigest) { - this.keyDigest = await getKeyDigest(this.encryptionKey); - } - + await this.init(); return _encrypt(data, this.key, this.keyDigest); } }packages/plugins/encryption/src/decrypter.ts (1)
24-37: Lazy init has a benign race under concurrent calls.If multiple
decrypt()calls arrive before keys are loaded, each will trigger a redundantPromise.allsince thethis.keys.length === 0check has no concurrency guard. The result is idempotent so this is safe, but you could cache the initialization promise to avoid redundant key imports.♻️ Optional: deduplicate concurrent init
export class Decrypter { private keys: Array<{ key: CryptoKey; digest: string }> = []; + private initPromise: Promise<void> | undefined; ... async decrypt(data: string): Promise<string> { - if (this.keys.length === 0) { - this.keys = await Promise.all( - this.decryptionKeys.map(async (key) => ({ - key: await loadKey(key, ['decrypt']), - digest: await getKeyDigest(key), - })), - ); - } + if (this.keys.length === 0) { + this.initPromise ??= (async () => { + this.keys = await Promise.all( + this.decryptionKeys.map(async (key) => ({ + key: await loadKey(key, ['decrypt']), + digest: await getKeyDigest(key), + })), + ); + })(); + await this.initPromise; + } return _decrypt(data, async (digest) =>packages/plugins/encryption/src/plugin.ts (2)
69-71: Empty strings bypass encryption and are stored as plaintext.The
value === ''check on line 70 skips encryption for empty strings. While this avoids encrypting vacuous data, it also means empty strings are distinguishable from non-empty values in the database — an observer can tell which rows have empty encrypted fields. If this is intentional, a brief comment clarifying the rationale would help future readers.
61-89: Significant code duplication in nested write traversal — consider consolidating.
encryptNestedWritesmanually handles six nested operation types (create,createMany,update,updateMany,upsert,connectOrCreate), each with nearly identical array-vs-object branching. A table-driven approach mapping operation names to their data extraction logic would reduce duplication and make it easier to add new operation types.♻️ Sketch of a table-driven approach
const nestedOps: Array<{ key: string; extractData: (v: Record<string, unknown>) => Record<string, unknown>[]; }> = [ { key: 'create', extractData: (v) => [v] }, { key: 'createMany', extractData: (v) => Array.isArray(v.data) ? v.data as Record<string, unknown>[] : [] }, { key: 'update', extractData: (v) => v.data ? [v.data as Record<string, unknown>] : [v] }, { key: 'upsert', extractData: (v) => [v.create, v.update].filter(Boolean) as Record<string, unknown>[] }, { key: 'connectOrCreate', extractData: (v) => v.create ? [v.create as Record<string, unknown>] : [] }, // updateMany similar to update... ]; for (const { key, extractData } of nestedOps) { const raw = data[key]; if (!raw) continue; const items = Array.isArray(raw) ? raw : [raw]; for (const item of items) { for (const d of extractData(item as Record<string, unknown>)) { await encryptWriteData(schema, modelName, d); } } }Also applies to: 94-211
tests/e2e/orm/client-api/encrypted.test.ts (1)
6-28: Remove the duplicatetests/e2e/orm/schemas/encrypted/schema.zmodelfile or refactor the test to use it.The inline schema in this test is identical to the dedicated
.zmodelfile, but the.zmodelfile is not referenced anywhere in the codebase. This creates dead code and risk of divergence. Either import/read the schema from the.zmodelfile or delete the unused file.
| // Handle encrypted string fields | ||
| if (isEncryptedField(field) && typeof value === 'string') { | ||
| try { | ||
| data[fieldName] = await decryptValue(modelName, field, value); | ||
| } catch (error) { | ||
| // If decryption fails, log warning and keep original value | ||
| console.warn(`Failed to decrypt field ${modelName}.${fieldName}:`, error); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Decryption failure silently returns ciphertext to the application.
When decryption fails, the console.warn fires but the original encrypted (ciphertext) value is returned to the caller. This could leak ciphertext into the UI or downstream logic where plaintext is expected, potentially causing confusing bugs. Consider whether throwing or returning a sentinel value (e.g., null or undefined) would be safer, depending on your threat model.
🤖 Prompt for AI Agents
In `@packages/plugins/encryption/src/plugin.ts` around lines 232 - 241, The
current catch in the encrypted-field handling block (isEncryptedField,
decryptValue, data[fieldName], modelName, fieldName) leaves the original
ciphertext in data on decryption errors; change the catch to avoid returning
ciphertext by assigning a safe sentinel (e.g., data[fieldName] = null or
undefined) and logging the error details (include modelName/fieldName and the
caught error) instead of only console.warn; alternatively, if callers require
plaintext, rethrow the error from the catch so upstream can handle it—pick one
strategy and apply it consistently in this block.
| onQuery: async (ctx) => { | ||
| const { model, operation, args, proceed, client } = ctx; | ||
| const schema = (client as any).schema as SchemaDef; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check how other plugins access the schema from the client context
rg -n --type=ts 'client.*schema\b' -g '!node_modules' -g '!dist' -C3Repository: zenstackhq/zenstack-v3
Length of output: 50379
🏁 Script executed:
# Look for the type definition of the client context and definePlugin
fd -e ts -e tsx | xargs grep -l "definePlugin\|SchemaDef" | head -20Repository: zenstackhq/zenstack-v3
Length of output: 1170
🏁 Script executed:
# Check the plugin context/client type definitions
rg -n --type=ts 'interface.*Client|type.*Client|class.*Client' -g '!node_modules' -g '!dist' | head -30Repository: zenstackhq/zenstack-v3
Length of output: 3907
🏁 Script executed:
# Look at the specific file to understand the context better
head -300 packages/plugins/encryption/src/plugin.ts | tail -100Repository: zenstackhq/zenstack-v3
Length of output: 3946
🏁 Script executed:
# Look for ClientContract interface and schema property definitions
rg -n "interface ClientContract|class ClientContract" --type=ts -g '!node_modules' -A 20 | head -80Repository: zenstackhq/zenstack-v3
Length of output: 48
🏁 Script executed:
# Search for the plugin hook context type definition
rg -n "onQuery.*ctx|QueryHookContext|OnQueryContext" --type=ts -g '!node_modules' -B 2 -A 10 | head -100Repository: zenstackhq/zenstack-v3
Length of output: 7734
🏁 Script executed:
# Check how definePlugin is used and what context it provides
rg -n "definePlugin" --type=ts -g '!node_modules' -B 3 -A 15 | head -120Repository: zenstackhq/zenstack-v3
Length of output: 8705
🏁 Script executed:
# Look at the exact client parameter type in plugin context
cat packages/plugins/encryption/src/plugin.ts | head -50Repository: zenstackhq/zenstack-v3
Length of output: 1888
🏁 Script executed:
# Get the full OnQueryHookContext type definition
sed -n '139,170p' packages/orm/src/client/plugin.tsRepository: zenstackhq/zenstack-v3
Length of output: 785
🏁 Script executed:
# Look for the client property in OnQueryHookContext
rg -n "client:" packages/orm/src/client/plugin.ts -A 5 -B 2Repository: zenstackhq/zenstack-v3
Length of output: 1350
🏁 Script executed:
# Check if there's a public $schema property on the client or if only internal schema exists
rg -n "\$schema|\.schema" packages/orm/src/client/contract.ts -B 2 -A 2 | head -80Repository: zenstackhq/zenstack-v3
Length of output: 48
🏁 Script executed:
# Look for ClientContract interface definition
rg -n "interface ClientContract|type ClientContract" packages/orm/src/client/contract.ts -A 30 | head -100Repository: zenstackhq/zenstack-v3
Length of output: 1192
🏁 Script executed:
# Check what properties ClientContract exposes
rg -n "schema|\\$schema" packages/orm/src/client/contract.ts | head -40Repository: zenstackhq/zenstack-v3
Length of output: 180
🏁 Script executed:
# Look at the actual client implementation to see what schema property is exposed
sed -n '1,100p' packages/orm/src/client/contract.tsRepository: zenstackhq/zenstack-v3
Length of output: 3002
🏁 Script executed:
# Confirm there are no other schema-related properties on ClientContract besides $schema
sed -n '95,200p' packages/orm/src/client/contract.ts | grep -E "schema|Schema"Repository: zenstackhq/zenstack-v3
Length of output: 1219
🏁 Script executed:
# Check if the client object passed to plugin context would have $schema accessible
rg -n "client\.\\\$schema" packages/orm/src/client/client-impl.ts -B 2 -A 2 | head -40Repository: zenstackhq/zenstack-v3
Length of output: 1223
🏁 Script executed:
# Look for examples in the plugin test to see how they access schema
rg -n "client\." tests/e2e/orm/plugin-infra/on-query-hooks.test.ts | head -20Repository: zenstackhq/zenstack-v3
Length of output: 958
🏁 Script executed:
# Check if OnQueryHookContext exposes schema directly or if it needs to come from client
sed -n '139,170p' packages/orm/src/client/plugin.tsRepository: zenstackhq/zenstack-v3
Length of output: 785
🏁 Script executed:
# Look for other plugins to see if they have the same pattern
find packages/plugins -name "*.ts" -type f | xargs grep -l "onQuery" | head -5Repository: zenstackhq/zenstack-v3
Length of output: 207
🏁 Script executed:
# Check if there are other plugins using similar pattern
rg -n "client.*schema" packages/plugins --type=ts | grep -v node_modulesRepository: zenstackhq/zenstack-v3
Length of output: 4634
Use client.$schema instead of (client as any).schema for typed access.
The ClientContract interface provides a typed $schema property. Replace (client as any).schema with client.$schema to maintain type safety and consistency with other plugins in the codebase (see policy plugin).
🤖 Prompt for AI Agents
In `@packages/plugins/encryption/src/plugin.ts` around lines 264 - 266, The code
in onQuery is using an untyped cast to access the schema; replace "(client as
any).schema" with the typed property "client.$schema" and assign that to schema
(keeps SchemaDef type). Update the destructuring in onQuery (ctx) to use
client.$schema when creating the schema variable so the code uses the
ClientContract's typed schema consistently with other plugins like policy.
| // Clone args to avoid mutating original | ||
| const processedArgs = args ? JSON.parse(JSON.stringify(args)) : undefined; |
There was a problem hiding this comment.
JSON.parse(JSON.stringify(args)) corrupts non-JSON-safe values.
This deep-clone approach converts Date objects to strings, drops undefined values, and throws on BigInt. Since the entire args object is cloned — including where clauses, orderBy, etc. — and the clone is passed to proceed, any non-string date filters or special values in the args will be silently mangled before reaching the ORM.
Consider using structuredClone(args) (supported in Node ≥17) which correctly handles Date, RegExp, ArrayBuffer, Map, Set, etc.
🔧 Proposed fix
- const processedArgs = args ? JSON.parse(JSON.stringify(args)) : undefined;
+ const processedArgs = args ? structuredClone(args) : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Clone args to avoid mutating original | |
| const processedArgs = args ? JSON.parse(JSON.stringify(args)) : undefined; | |
| // Clone args to avoid mutating original | |
| const processedArgs = args ? structuredClone(args) : undefined; |
🤖 Prompt for AI Agents
In `@packages/plugins/encryption/src/plugin.ts` around lines 274 - 275, Replace
the JSON.parse(JSON.stringify(args)) deep-clone (which corrupts
Date/BigInt/undefined/etc.) with a proper structured clone for processedArgs
before calling proceed: use structuredClone(args) when available, and fall back
to v8.serialize/deserialize (require('v8').serialize/deserialize) for older Node
versions; ensure you still preserve the existing behavior when args is undefined
and keep the variable name processedArgs and its use in the call to proceed
unchanged.
|
|
||
| // combine IV and encrypted data into a single array of bytes | ||
| const cipherBytes = [...iv, ...new Uint8Array(encrypted)]; | ||
|
|
||
| // encryption metadata | ||
| const meta = { v: ENCRYPTER_VERSION, a: ALGORITHM, k: keyDigest }; | ||
|
|
||
| // convert concatenated result to base64 string | ||
| return `${btoa(JSON.stringify(meta))}.${btoa(String.fromCharCode(...cipherBytes))}`; |
There was a problem hiding this comment.
String.fromCharCode(...cipherBytes) will blow the call stack for large inputs.
Spreading a large array into String.fromCharCode passes each element as a function argument. Most JS engines limit argument count to ~65K–500K, so encrypting a string longer than roughly 60 KB will throw a RangeError: Maximum call stack size exceeded. The same pattern appears in _decrypt at Line 93.
Consider chunking or using a different approach:
♻️ Suggested fix
- // combine IV and encrypted data into a single array of bytes
- const cipherBytes = [...iv, ...new Uint8Array(encrypted)];
-
- // encryption metadata
- const meta = { v: ENCRYPTER_VERSION, a: ALGORITHM, k: keyDigest };
-
- // convert concatenated result to base64 string
- return `${btoa(JSON.stringify(meta))}.${btoa(String.fromCharCode(...cipherBytes))}`;
+ // combine IV and encrypted data into a single array of bytes
+ const cipherBytes = new Uint8Array(iv.length + encrypted.byteLength);
+ cipherBytes.set(iv, 0);
+ cipherBytes.set(new Uint8Array(encrypted), iv.length);
+
+ // encryption metadata
+ const meta = { v: ENCRYPTER_VERSION, a: ALGORITHM, k: keyDigest };
+
+ // convert concatenated result to base64 string
+ const binaryString = Array.from(cipherBytes, (byte) => String.fromCharCode(byte)).join('');
+ return `${btoa(JSON.stringify(meta))}.${btoa(binaryString)}`;Apply the same fix in _decrypt (Line 93):
- const bytes = Uint8Array.from(atob(cipherText), (c) => c.charCodeAt(0));The _decrypt side is already safe since Uint8Array.from with a mapping function doesn't spread arguments — no change needed there.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // combine IV and encrypted data into a single array of bytes | |
| const cipherBytes = [...iv, ...new Uint8Array(encrypted)]; | |
| // encryption metadata | |
| const meta = { v: ENCRYPTER_VERSION, a: ALGORITHM, k: keyDigest }; | |
| // convert concatenated result to base64 string | |
| return `${btoa(JSON.stringify(meta))}.${btoa(String.fromCharCode(...cipherBytes))}`; | |
| // combine IV and encrypted data into a single array of bytes | |
| const cipherBytes = new Uint8Array(iv.length + encrypted.byteLength); | |
| cipherBytes.set(iv, 0); | |
| cipherBytes.set(new Uint8Array(encrypted), iv.length); | |
| // encryption metadata | |
| const meta = { v: ENCRYPTER_VERSION, a: ALGORITHM, k: keyDigest }; | |
| // convert concatenated result to base64 string | |
| const binaryString = Array.from(cipherBytes, (byte) => String.fromCharCode(byte)).join(''); | |
| return `${btoa(JSON.stringify(meta))}.${btoa(binaryString)}`; |
🤖 Prompt for AI Agents
In `@packages/plugins/encryption/src/utils.ts` around lines 56 - 64, The return
uses String.fromCharCode(...cipherBytes) which will crash for large inputs; in
the _encrypt function replace the spread call with a safe conversion that builds
the binary string in chunks (e.g., loop over cipherBytes in slices of ~32k bytes
and call String.fromCharCode on each slice then join) before btoa; reference the
symbols _encrypt, cipherBytes and the return expression so you update that line
to concatenate chunked String.fromCharCode results (do not change _decrypt since
its Uint8Array.from usage is already safe).
| it('encrypts and decrypts a single field on findMany', async () => { | ||
| await client.user.create({ | ||
| data: { email: 'test1@test.com', secretToken: 'secret-1' }, | ||
| }); | ||
| await client.user.create({ | ||
| data: { email: 'test2@test.com', secretToken: 'secret-2' }, | ||
| }); | ||
|
|
||
| const users = await client.user.findMany(); | ||
|
|
||
| expect(users).toHaveLength(2); | ||
| expect(users[0].secretToken).toBe('secret-1'); | ||
| expect(users[1].secretToken).toBe('secret-2'); | ||
| }); |
There was a problem hiding this comment.
findMany without orderBy makes assertions order-dependent.
Lines 101-102 assert on users[0] and users[1] by expected order, but findMany() on line 98 has no orderBy. While SQLite may return rows in insertion order, this is not guaranteed and can cause flaky tests. Compare with the createMany test on line 222 which correctly uses orderBy: { email: 'asc' }.
🔧 Proposed fix
- const users = await client.user.findMany();
+ const users = await client.user.findMany({
+ orderBy: { email: 'asc' },
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('encrypts and decrypts a single field on findMany', async () => { | |
| await client.user.create({ | |
| data: { email: 'test1@test.com', secretToken: 'secret-1' }, | |
| }); | |
| await client.user.create({ | |
| data: { email: 'test2@test.com', secretToken: 'secret-2' }, | |
| }); | |
| const users = await client.user.findMany(); | |
| expect(users).toHaveLength(2); | |
| expect(users[0].secretToken).toBe('secret-1'); | |
| expect(users[1].secretToken).toBe('secret-2'); | |
| }); | |
| it('encrypts and decrypts a single field on findMany', async () => { | |
| await client.user.create({ | |
| data: { email: 'test1@test.com', secretToken: 'secret-1' }, | |
| }); | |
| await client.user.create({ | |
| data: { email: 'test2@test.com', secretToken: 'secret-2' }, | |
| }); | |
| const users = await client.user.findMany({ | |
| orderBy: { email: 'asc' }, | |
| }); | |
| expect(users).toHaveLength(2); | |
| expect(users[0].secretToken).toBe('secret-1'); | |
| expect(users[1].secretToken).toBe('secret-2'); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/e2e/orm/client-api/encrypted.test.ts` around lines 90 - 103, The test
"encrypts and decrypts a single field on findMany" is order-dependent because
client.user.findMany() lacks an orderBy; update the call to include a
deterministic ordering (e.g., orderBy: { email: 'asc' }) or sort the returned
users before asserting so users[0].secretToken and users[1].secretToken are
stable; modify the client.user.findMany invocation in that test to include the
orderBy clause (or apply a stable sort) to eliminate flakiness.
| it('handles null values gracefully', async () => { | ||
| // Create user with non-nullable encrypted field using a value | ||
| const user = await client.user.create({ | ||
| data: { | ||
| email: 'test@test.com', | ||
| secretToken: '', | ||
| }, | ||
| }); | ||
|
|
||
| expect(user.secretToken).toBe(''); | ||
| }); |
There was a problem hiding this comment.
Test name says "null" but actually tests empty string.
The test is titled "handles null values gracefully" but the secretToken field is non-optional (String, not String?), so it can never be null. The test actually verifies empty-string handling. Consider renaming or adding a test with an optional @encrypted field to cover actual null values.
🔧 Suggested rename
- it('handles null values gracefully', async () => {
+ it('handles empty string values gracefully', async () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('handles null values gracefully', async () => { | |
| // Create user with non-nullable encrypted field using a value | |
| const user = await client.user.create({ | |
| data: { | |
| email: 'test@test.com', | |
| secretToken: '', | |
| }, | |
| }); | |
| expect(user.secretToken).toBe(''); | |
| }); | |
| it('handles empty string values gracefully', async () => { | |
| // Create user with non-nullable encrypted field using a value | |
| const user = await client.user.create({ | |
| data: { | |
| email: 'test@test.com', | |
| secretToken: '', | |
| }, | |
| }); | |
| expect(user.secretToken).toBe(''); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/e2e/orm/client-api/encrypted.test.ts` around lines 125 - 135, The test
named "handles null values gracefully" actually creates a non-optional encrypted
field (secretToken) with an empty string via client.user.create and asserts ''.
Rename the test to something like "handles empty-string encrypted values" OR add
a new test that uses an optional encrypted field (e.g., optionalSecretToken or a
nullable User model field) and create a user with that field set to null, then
assert the returned value is null; update the test title and assertions
accordingly and reference the existing client.user.create usage and secretToken
field in your changes.
| // Use the same client but with new plugin to verify key rotation | ||
| const client2 = client.$use(newPlugin); | ||
|
|
||
| // Should still be able to read the old data (decrypted with old key) | ||
| const found = await client2.user.findFirst(); | ||
| expect(found?.secretToken).toBe('my-secret'); | ||
|
|
||
| await client.$disconnect(); | ||
| }); |
There was a problem hiding this comment.
Key rotation test: client2 is not disconnected.
Line 271 disconnects only client, but client2 (created via client.$use(newPlugin) on line 265) is never disconnected. If $use creates a wrapper that holds its own resources, this could leak.
🔧 Proposed fix
+ await client2.$disconnect();
await client.$disconnect();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Use the same client but with new plugin to verify key rotation | |
| const client2 = client.$use(newPlugin); | |
| // Should still be able to read the old data (decrypted with old key) | |
| const found = await client2.user.findFirst(); | |
| expect(found?.secretToken).toBe('my-secret'); | |
| await client.$disconnect(); | |
| }); | |
| // Use the same client but with new plugin to verify key rotation | |
| const client2 = client.$use(newPlugin); | |
| // Should still be able to read the old data (decrypted with old key) | |
| const found = await client2.findFirst(); | |
| expect(found?.secretToken).toBe('my-secret'); | |
| await client2.$disconnect(); | |
| await client.$disconnect(); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/e2e/orm/client-api/encrypted.test.ts` around lines 264 - 272, The test
creates a wrapped client via client.$use(newPlugin) assigned to client2 but only
calls client.$disconnect(), which can leak resources held by the wrapper; update
the teardown to also disconnect client2 (call client2.$disconnect() after the
test actions or in the same cleanup block) so both client and client2 are
properly closed, ensuring any resources held by the $use wrapper are released.
1 participant
Summary by CodeRabbit
New Features
@encryptedattribute for String fields.Tests