feat(prisma-next): EQL v3 (domain-based) String support#515
Open
tobyhede wants to merge 21 commits into
Open
Conversation
|
|
Warning Review limit reached
More reviews will be available in 11 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (86)
📝 WalkthroughWalkthroughThis PR adds EQL v3 string support to prisma-next and drizzle, ships baseline install assets, wires new operator and middleware behavior, updates the Prisma example with a ChangesEQL v3 string support
Sequence Diagram(s)sequenceDiagram
participant App
participant QueryOps
participant bulkEncryptV3Middleware
participant CipherstashSdk
participant Postgres
App->>QueryOps: build v3 predicate
QueryOps->>bulkEncryptV3Middleware: stamped v3 envelope params
bulkEncryptV3Middleware->>CipherstashSdk: bulkEncryptQuery(routingKey, queryType, values)
CipherstashSdk-->>bulkEncryptV3Middleware: encrypted query terms
bulkEncryptV3Middleware->>Postgres: execute SQL with v3 jsonb wire params
Postgres-->>App: matching rows
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…zed descriptor Parameterize the shared cell-codec factory with optional wire/metadata/middleware options (v2 defaults unchanged), add createCipherstashStringV3Codec, and register the v3 descriptor. Update the descriptor-count pins to include the 7th (v3) codec.
…agging Operator factories now compute the lowering template inside impl via dialectForCodecId(selfCodec.codecId); cipherstashEq/Ilike convert to the shared cipherstash:string trait (attaching to both string codecs); add V3_ENVELOPE_COERCERS, queryTypeForIndex, the index/operator mismatch guard, and setHandleQueryType on the envelope handle. v3 param cast is plain jsonb (meta.nativeType), not the column domain, so search terms don't fail the full-payload domain CHECK.
…n hook migration.json/end-contract.* are regenerated by the framework contract-emit CLI during e2e setup (Task 15); ops.json is generated here from the operations getter.
Register the v3 codec hooks + v3 baseline migration in the control descriptor, add bulkEncryptV3Middleware to cipherstashFromStack, re-export the v3 surface from runtime/middleware, and advance the head ref to the union of baseline invariants.
Adds the UserV3 model (validated end-to-end: contract emit + migration plan emit the per-index eql_v3.text_eq/text_match/text_ord domains via expandNativeType), the v3 seed/oracle helpers, the gated e2e matrix, and the user_v3 truncate. The e2e run requires live Postgres + ZeroKMS credentials (gated on DATABASE_URL).
… guard, fix docs/test labels) - Move V3_INDEX_VALUES + isV3Index to v3/domain-map.ts (single source of truth; was duplicated in derive-schemas.ts and codec-hooks-v3.ts). - Align vendor-script run comment with REFRESH_EQL_V3.md (node --experimental-strip-types). - Rename the mislabeled e2e 'NULL round-trips' test (UserV3 columns are NOT NULL).
- Remove dead v3Dialect.orderBy() (unreachable — cipherstashAsc/Desc are v2-only); document that v3 ORDER BY sort is not yet wired (README + e2e header). - Fix stale eqlOperator docblock (now trait-dispatched + dialect-selected, not pinned to string@1 / eql_v2). - Reuse FLAG_DISPATCH in applyV3Index instead of a parallel if-chain. - Expand tests: full index/operator mismatch matrix, V3_ENVELOPE_COERCERS reject path, setHandleQueryType write-once-wins conflict, encryptQuery failure branch; tighten the sdk-adapter column assertion; add gt/lte e2e ord cases. - Align vendor-script + migration regen comments with the actual invocation.
tobyhede
force-pushed
the
eql-v3-prisma-next
branch
from
e244b8d to
5f75946
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/prisma-next/src/execution/cell-codec-factory.ts (1)
163-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle pre-encoded non-string values before calling
expose().Line 163 only bypasses when the value is a string. For v3, the wire is plain
jsonb(object), so Line 166 can callexpose()on a non-envelope and crash with aTypeError.💡 Proposed fix
- if (typeof value === 'string') { - return value; - } - const handle = value.expose(); + const rawValue = value as unknown; + if ( + rawValue === null || + rawValue === undefined || + typeof rawValue !== 'object' || + !('expose' in rawValue) || + typeof (rawValue as { expose?: unknown }).expose !== 'function' + ) { + // Already encoded/replaced by middleware (v2 string or v3 jsonb). + return rawValue; + } + const handle = (rawValue as E).expose();Also applies to: 198-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/prisma-next/src/execution/cell-codec-factory.ts` around lines 163 - 167, The code at line 166 calls expose() on non-string values without checking if they are pre-encoded plain objects (jsonb), which can cause a TypeError crash. Before calling expose() on the value, add a check to determine if the value is already a pre-encoded non-envelope object, and if so, handle it directly without calling expose(). This same fix needs to be applied to the similar code pattern around line 198 as well.examples/prisma/test/e2e/global-setup.ts (1)
101-109:⚠️ Potential issue | 🟠 MajorThe migration graph has separate, unconnected lineages preventing invariant satisfaction.
The cipherstash bundle migrations (
20260601T0000_install_eql_bundleand20260601T0100_install_eql_v3_bundle) are independent roots (from: null). The app migrations are also rooted separately, with the newer migration (20260616T2336_migration) providing no invariants (providedInvariants: []) and declaring no explicit dependencies on the bundle migrations. Whenprisma-next migration applyruns on the app migrations, the planner cannot resolve a path through the bundle migrations to satisfy the required invariantscipherstash:install-eql-bundle-v1andcipherstash:install-eql-v3-bundle-v1, resulting in theMIGRATION.NO_INVARIANT_PATHerror. Explicitly connect the app migration lineage to depend on the bundle migrations to establish a valid invariant path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/prisma/test/e2e/global-setup.ts` around lines 101 - 109, The migration graph has disconnected lineages where the app migration (20260616T2336_migration) does not depend on the bundle migrations (20260601T0000_install_eql_bundle and 20260601T0100_install_eql_v3_bundle), causing the invariant path resolution to fail when prisma-next migration apply executes. Update the app migration file to explicitly declare dependencies on the bundle migrations by adding the appropriate from field references to connect the lineages and establish a valid path for satisfying the required cipherstash invariants (cipherstash:install-eql-bundle-v1 and cipherstash:install-eql-v3-bundle-v1).Source: Pipeline failures
🧹 Nitpick comments (3)
packages/drizzle/src/pg/v3/eql-v3-type.ts (1)
36-38: 💤 Low valueMinor type annotation mismatch with
v3FromDriversignature.The
fromDrivercallback is typed as(value: string | null): TData, butv3FromDriveracceptsstring | object | null | undefinedbecause the Postgres driver may return pre-parsed JSONB objects. This works correctly at runtime sincev3FromDriverhandles all cases, but the type annotation is narrower than actual behavior.Consider widening the type to match the actual function signature:
Suggested fix
- fromDriver(value: string | null): TData { - return v3FromDriver<TData>(value) + fromDriver(value: string | object | null): TData { + return v3FromDriver<TData>(value) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/drizzle/src/pg/v3/eql-v3-type.ts` around lines 36 - 38, The `fromDriver` method callback has a type annotation that is too narrow for the `value` parameter. Update the type annotation of the `value` parameter in the `fromDriver` callback to match the actual signature of `v3FromDriver`, which accepts `string | object | null | undefined`. Change the current type from `string | null` to `string | object | null | undefined` to accurately reflect that the Postgres driver may return pre-parsed JSONB objects in addition to strings.packages/prisma-next/src/v3/domain-map.ts (1)
52-59: ⚡ Quick winConsider enhancing the error message for unsupported indices.
The error at line 57 states that an index is unsupported but doesn't indicate which indices are valid. For better developer experience, consider listing the valid options.
💡 Proposed enhancement
const domain = scalar.byIndex[index] - if (!domain) throw new Error(`Unsupported v3 index "${index}" for dataType "${dataType}".`) + if (!domain) { + const validIndices = V3_INDEX_VALUES.join('", "') + throw new Error(`Unsupported v3 index "${index}" for dataType "${dataType}". Valid indices: "${validIndices}".`) + } return domain🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/prisma-next/src/v3/domain-map.ts` around lines 52 - 59, The error message in the eqlV3Domain function when checking if an index is unsupported (the if (!domain) condition at line 57) does not provide information about which indices are actually valid for the given dataType. Extract the valid index keys from scalar.byIndex and include them in the error message to help developers understand what valid options are available when they encounter this error.packages/prisma-next/src/execution/sdk.ts (1)
72-76: ⚡ Quick winNarrow
queryTypeto the supported literals.
queryType: stringallows invalid values across the SDK boundary and shifts failures to runtime. Prefer a literal union matching the documented query types.Suggested diff
+export type CipherstashQueryType = 'equality' | 'freeTextSearch' | 'orderAndRange'; + export interface CipherstashBulkEncryptQueryArgs { readonly routingKey: CipherstashRoutingKey; - readonly queryType: string; + readonly queryType: CipherstashQueryType; readonly values: ReadonlyArray<unknown>; readonly signal?: AbortSignal; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/prisma-next/src/execution/sdk.ts` around lines 72 - 76, The queryType field in the CipherstashBulkEncryptQueryArgs interface is typed as a generic string, which allows invalid values and causes failures at runtime. Replace the queryType: string field with a literal union type that matches the documented and supported query types. Identify all valid query type values used across the SDK and create a union type like queryType: 'type1' | 'type2' | 'type3' to enforce type safety at compile time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/drizzle/__tests__/v3/extraction.test.ts`:
- Around line 41-56: Move the assertion that validates `sharedMap` is an
instance of Map to occur before the `sharedMap.delete('v3_shared_t_eq')` call.
Currently the assertion using `expect(sharedMap).toBeInstanceOf(Map)` happens
after the delete operation, which means if sharedMap is not properly initialized
as a Map, the delete call will fail with a confusing TypeError instead of
showing the intended assertion failure. Reorder the code so the Map type
assertion validates the sharedMap variable immediately after it is cast and
retrieved from globalThis, before any mutations are performed on it.
In `@packages/prisma-next/src/execution/operators.ts`:
- Around line 149-165: The setHandleQueryType function call is directly mutating
the envelope object that belongs to the caller, which can cause issues if the
same envelope is reused in later INSERT/UPDATE operations where the stale
queryType marker would incorrectly route data through encryptQuery instead of
bulkEncrypt. Instead of mutating the caller's envelope directly, either clone
the envelope before calling setHandleQueryType on the clone, or scope the
queryType metadata to the current operation context as ParamRef-local metadata
rather than stamping it on the shared envelope object. This ensures the
queryType marker only applies to the current search operation and does not leak
to subsequent storage operations.
In `@packages/prisma-next/src/middleware/bulk-encrypt-v3.ts`:
- Around line 39-44: The bulkEncryptV3Middleware function currently uses
markBulkEncryptMiddlewareRegistered which shares a registration flag with v2
middleware, allowing v2 checks to pass even when only v3 is registered. Replace
the shared registration tracking with version-specific registration functions:
create separate registration and checking mechanisms for v3 (and similarly for
v2), then update bulkEncryptV3Middleware to call a v3-specific marking function
instead of the shared markBulkEncryptMiddlewareRegistered. Finally, update the
encode sentinels in both v2 and v3 codecs to check their respective
version-specific registration flags instead of the shared flag.
---
Outside diff comments:
In `@examples/prisma/test/e2e/global-setup.ts`:
- Around line 101-109: The migration graph has disconnected lineages where the
app migration (20260616T2336_migration) does not depend on the bundle migrations
(20260601T0000_install_eql_bundle and 20260601T0100_install_eql_v3_bundle),
causing the invariant path resolution to fail when prisma-next migration apply
executes. Update the app migration file to explicitly declare dependencies on
the bundle migrations by adding the appropriate from field references to connect
the lineages and establish a valid path for satisfying the required cipherstash
invariants (cipherstash:install-eql-bundle-v1 and
cipherstash:install-eql-v3-bundle-v1).
In `@packages/prisma-next/src/execution/cell-codec-factory.ts`:
- Around line 163-167: The code at line 166 calls expose() on non-string values
without checking if they are pre-encoded plain objects (jsonb), which can cause
a TypeError crash. Before calling expose() on the value, add a check to
determine if the value is already a pre-encoded non-envelope object, and if so,
handle it directly without calling expose(). This same fix needs to be applied
to the similar code pattern around line 198 as well.
---
Nitpick comments:
In `@packages/drizzle/src/pg/v3/eql-v3-type.ts`:
- Around line 36-38: The `fromDriver` method callback has a type annotation that
is too narrow for the `value` parameter. Update the type annotation of the
`value` parameter in the `fromDriver` callback to match the actual signature of
`v3FromDriver`, which accepts `string | object | null | undefined`. Change the
current type from `string | null` to `string | object | null | undefined` to
accurately reflect that the Postgres driver may return pre-parsed JSONB objects
in addition to strings.
In `@packages/prisma-next/src/execution/sdk.ts`:
- Around line 72-76: The queryType field in the CipherstashBulkEncryptQueryArgs
interface is typed as a generic string, which allows invalid values and causes
failures at runtime. Replace the queryType: string field with a literal union
type that matches the documented and supported query types. Identify all valid
query type values used across the SDK and create a union type like queryType:
'type1' | 'type2' | 'type3' to enforce type safety at compile time.
In `@packages/prisma-next/src/v3/domain-map.ts`:
- Around line 52-59: The error message in the eqlV3Domain function when checking
if an index is unsupported (the if (!domain) condition at line 57) does not
provide information about which indices are actually valid for the given
dataType. Extract the valid index keys from scalar.byIndex and include them in
the error message to help developers understand what valid options are available
when they encounter this error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4266b66d-951c-4a12-a5e0-70241b420ad5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (111)
examples/prisma/migrations/app/20260616T2336_migration/end-contract.d.tsexamples/prisma/migrations/app/20260616T2336_migration/end-contract.jsonexamples/prisma/migrations/app/20260616T2336_migration/migration.jsonexamples/prisma/migrations/app/20260616T2336_migration/migration.tsexamples/prisma/migrations/app/20260616T2336_migration/ops.jsonexamples/prisma/migrations/app/20260616T2336_migration/start-contract.d.tsexamples/prisma/migrations/app/20260616T2336_migration/start-contract.jsonexamples/prisma/migrations/cipherstash/20260601T0100_install_eql_v3_bundle/contract.jsonexamples/prisma/migrations/cipherstash/20260601T0100_install_eql_v3_bundle/migration.jsonexamples/prisma/migrations/cipherstash/20260601T0100_install_eql_v3_bundle/ops.jsonexamples/prisma/migrations/cipherstash/refs/head.jsonexamples/prisma/prisma/schema.prismaexamples/prisma/src/prisma/contract.d.tsexamples/prisma/src/prisma/contract.jsonexamples/prisma/test/e2e/global-setup.tsexamples/prisma/test/e2e/harness.tsexamples/prisma/test/e2e/helpers/eql-v3-seed.tsexamples/prisma/test/e2e/str-v3.e2e.test.tspackages/drizzle/.gitattributespackages/drizzle/__tests__/eql-v3.test.tspackages/drizzle/__tests__/fixtures/cipherstash-encrypt-v3.sqlpackages/drizzle/__tests__/fixtures/eql-v3-seed-data.tspackages/drizzle/__tests__/test-utils.tspackages/drizzle/__tests__/v3/codec.test.tspackages/drizzle/__tests__/v3/detection.test.tspackages/drizzle/__tests__/v3/dialect.test.tspackages/drizzle/__tests__/v3/domain-map.test.tspackages/drizzle/__tests__/v3/eql-v3-type.test.tspackages/drizzle/__tests__/v3/exports.test.tspackages/drizzle/__tests__/v3/extraction.test.tspackages/drizzle/__tests__/v3/helpers/install-v3.tspackages/drizzle/__tests__/v3/operators-v3.test.tspackages/drizzle/__tests__/v3/provisioning.test.tspackages/drizzle/__tests__/v3/roundtrip-eq.test.tspackages/drizzle/package.jsonpackages/drizzle/scripts/refresh-eql-v3-sql.mdpackages/drizzle/src/pg/index.tspackages/drizzle/src/pg/operators.tspackages/drizzle/src/pg/sql-dialect.tspackages/drizzle/src/pg/v3/codec.tspackages/drizzle/src/pg/v3/domain-map.tspackages/drizzle/src/pg/v3/eql-v3-type.tspackages/drizzle/src/pg/v3/index.tspackages/drizzle/tsup.config.tspackages/prisma-next/.gitattributespackages/prisma-next/DEVELOPING.mdpackages/prisma-next/README.mdpackages/prisma-next/__tests__/fixtures/cipherstash-encrypt-v3.sqlpackages/prisma-next/migrations/20260601T0100_install_eql_v3_bundle/migration.jsonpackages/prisma-next/migrations/20260601T0100_install_eql_v3_bundle/migration.tspackages/prisma-next/migrations/20260601T0100_install_eql_v3_bundle/ops.jsonpackages/prisma-next/migrations/refs/head.jsonpackages/prisma-next/package.jsonpackages/prisma-next/scripts/REFRESH_EQL_V3.mdpackages/prisma-next/scripts/vendor-eql-v3-install.tspackages/prisma-next/src/contract-authoring.tspackages/prisma-next/src/execution/cell-codec-factory.tspackages/prisma-next/src/execution/codec-runtime.tspackages/prisma-next/src/execution/codec-v3.tspackages/prisma-next/src/execution/dialect.tspackages/prisma-next/src/execution/envelope-base.tspackages/prisma-next/src/execution/envelope-string.tspackages/prisma-next/src/execution/operators.tspackages/prisma-next/src/execution/parameterized.tspackages/prisma-next/src/execution/sdk.tspackages/prisma-next/src/exports/column-types.tspackages/prisma-next/src/exports/control.tspackages/prisma-next/src/exports/middleware.tspackages/prisma-next/src/exports/runtime.tspackages/prisma-next/src/extension-metadata/constants.tspackages/prisma-next/src/middleware/bulk-encrypt-v3.tspackages/prisma-next/src/middleware/bulk-encrypt.tspackages/prisma-next/src/migration/cipherstash-codec.tspackages/prisma-next/src/migration/codec-hooks-v3.tspackages/prisma-next/src/migration/eql-v3-bundle.tspackages/prisma-next/src/migration/eql-v3-install.generated.tspackages/prisma-next/src/stack/derive-schemas.tspackages/prisma-next/src/stack/from-stack.tspackages/prisma-next/src/stack/sdk-adapter.tspackages/prisma-next/src/types/operation-types.tspackages/prisma-next/src/v3/domain-map.tspackages/prisma-next/src/v3/wire-codec.tspackages/prisma-next/test/codec-runtime.test.tspackages/prisma-next/test/descriptor.test.tspackages/prisma-next/test/operation-types.types.test-d.tspackages/prisma-next/test/operator-lowering.test.tspackages/prisma-next/test/runtime-descriptor.test.tspackages/prisma-next/test/v3/authoring-v3.test.tspackages/prisma-next/test/v3/bulk-encrypt-v3.test.tspackages/prisma-next/test/v3/bundle.test.tspackages/prisma-next/test/v3/codec-hooks-v3.test.tspackages/prisma-next/test/v3/codec-v3.test.tspackages/prisma-next/test/v3/column-types.test.tspackages/prisma-next/test/v3/constants.test.tspackages/prisma-next/test/v3/control-v3.test.tspackages/prisma-next/test/v3/decrypt-all-v3.test.tspackages/prisma-next/test/v3/derive-schemas-v3.test.tspackages/prisma-next/test/v3/dialect.test.tspackages/prisma-next/test/v3/domain-map.test.tspackages/prisma-next/test/v3/edge-cases-v3.test.tspackages/prisma-next/test/v3/exports.test.tspackages/prisma-next/test/v3/from-stack-v3.test.tspackages/prisma-next/test/v3/helpers/fake-sdk.tspackages/prisma-next/test/v3/migration-v3.test.tspackages/prisma-next/test/v3/operators-v2-unchanged.test.tspackages/prisma-next/test/v3/operators-v3.test.tspackages/prisma-next/test/v3/parameterized.test.tspackages/prisma-next/test/v3/sdk-adapter-query.test.tspackages/prisma-next/test/v3/trait-parity.test.tspackages/prisma-next/test/v3/wire-codec.test.tspackages/prisma-next/vitest.config.ts
Comment on lines
+41
to
+56
| const mapKey = Symbol.for('@cipherstash/drizzle/pg:columnConfigMap') | ||
| const sharedMap = (globalThis as Record<symbol, unknown>)[mapKey] as Map< | ||
| string, | ||
| { name: string } | ||
| > | ||
| // Use a column name unique to this test so the global, name-keyed registry | ||
| // can't yield a false positive from another test's prior registration. | ||
| sharedMap.delete('v3_shared_t_eq') | ||
| const table = pgTable('v3_shared', { | ||
| v3_shared_t_eq: eqlV3Type<string>('v3_shared_t_eq', { | ||
| dataType: 'text', | ||
| index: 'equality', | ||
| }), | ||
| }) | ||
| expect(sharedMap).toBeInstanceOf(Map) | ||
| expect(sharedMap.get('v3_shared_t_eq')?.name).toBe('v3_shared_t_eq') |
There was a problem hiding this comment.
Assert sharedMap before mutating it.
At Line 48, sharedMap.delete(...) runs before confirming sharedMap is a Map. If initialization breaks, this throws a TypeError and hides the intended assertion failure.
Suggested fix
const mapKey = Symbol.for('`@cipherstash/drizzle/pg`:columnConfigMap')
const sharedMap = (globalThis as Record<symbol, unknown>)[mapKey] as Map<
string,
{ name: string }
>
+ expect(sharedMap).toBeInstanceOf(Map)
// Use a column name unique to this test so the global, name-keyed registry
// can't yield a false positive from another test's prior registration.
sharedMap.delete('v3_shared_t_eq')
@@
- expect(sharedMap).toBeInstanceOf(Map)
expect(sharedMap.get('v3_shared_t_eq')?.name).toBe('v3_shared_t_eq')📝 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.
Suggested change
| const mapKey = Symbol.for('@cipherstash/drizzle/pg:columnConfigMap') | |
| const sharedMap = (globalThis as Record<symbol, unknown>)[mapKey] as Map< | |
| string, | |
| { name: string } | |
| > | |
| // Use a column name unique to this test so the global, name-keyed registry | |
| // can't yield a false positive from another test's prior registration. | |
| sharedMap.delete('v3_shared_t_eq') | |
| const table = pgTable('v3_shared', { | |
| v3_shared_t_eq: eqlV3Type<string>('v3_shared_t_eq', { | |
| dataType: 'text', | |
| index: 'equality', | |
| }), | |
| }) | |
| expect(sharedMap).toBeInstanceOf(Map) | |
| expect(sharedMap.get('v3_shared_t_eq')?.name).toBe('v3_shared_t_eq') | |
| const mapKey = Symbol.for('`@cipherstash/drizzle/pg`:columnConfigMap') | |
| const sharedMap = (globalThis as Record<symbol, unknown>)[mapKey] as Map< | |
| string, | |
| { name: string } | |
| > | |
| expect(sharedMap).toBeInstanceOf(Map) | |
| // Use a column name unique to this test so the global, name-keyed registry | |
| // can't yield a false positive from another test's prior registration. | |
| sharedMap.delete('v3_shared_t_eq') | |
| const table = pgTable('v3_shared', { | |
| v3_shared_t_eq: eqlV3Type<string>('v3_shared_t_eq', { | |
| dataType: 'text', | |
| index: 'equality', | |
| }), | |
| }) | |
| expect(sharedMap.get('v3_shared_t_eq')?.name).toBe('v3_shared_t_eq') |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/drizzle/__tests__/v3/extraction.test.ts` around lines 41 - 56, Move
the assertion that validates `sharedMap` is an instance of Map to occur before
the `sharedMap.delete('v3_shared_t_eq')` call. Currently the assertion using
`expect(sharedMap).toBeInstanceOf(Map)` happens after the delete operation,
which means if sharedMap is not properly initialized as a Map, the delete call
will fail with a confusing TypeError instead of showing the intended assertion
failure. Reorder the code so the Map type assertion validates the sharedMap
variable immediately after it is cast and retrieved from globalThis, before any
mutations are performed on it.
Comment on lines
+39
to
+44
| export function bulkEncryptV3Middleware(sdk: CipherstashSdk): SqlMiddleware { | ||
| // Same sdk-keyed WeakSet as v2 (idempotent if both middlewares share an sdk); | ||
| // the v3 codec's encode sentinel consults it to distinguish "middleware will | ||
| // fill ciphertext" from a misconfig. | ||
| markBulkEncryptMiddlewareRegistered(sdk); | ||
| return { |
There was a problem hiding this comment.
Split middleware-registration tracking by codec family.
bulkEncryptV3Middleware marks the same sdk-level registration flag used by v2. If a consumer wires only v3 middleware, v2 encode-path checks can still pass and leave v2 envelopes unresolved until a later runtime failure. Registration should be version/family-specific (or codec-set specific) so each codec validates presence of its own middleware.
Suggested direction
- markBulkEncryptMiddlewareRegistered(sdk);
+ markBulkEncryptMiddlewareRegistered(sdk, 'v3');And similarly in v2 middleware:
- markBulkEncryptMiddlewareRegistered(sdk);
+ markBulkEncryptMiddlewareRegistered(sdk, 'v2');Then have encode sentinels check the matching family.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/prisma-next/src/middleware/bulk-encrypt-v3.ts` around lines 39 - 44,
The bulkEncryptV3Middleware function currently uses
markBulkEncryptMiddlewareRegistered which shares a registration flag with v2
middleware, allowing v2 checks to pass even when only v3 is registered. Replace
the shared registration tracking with version-specific registration functions:
create separate registration and checking mechanisms for v3 (and similarly for
v2), then update bulkEncryptV3Middleware to call a v3-specific marking function
instead of the shared markBulkEncryptMiddlewareRegistered. Finally, update the
encode sentinels in both v2 and v3 codecs to check their respective
version-specific registration flags instead of the shared flag.
…ttributes The ~0.5 MB EQL v3 SQL bundle is committed in four forms (fixture, generated .ts, package ops.json, example ops.json) plus generated contract/migration snapshots — ~95% of the diff. Mark them linguist-vendored/linguist-generated so GitHub collapses them and surfaces the ~2.2k lines of authored source/tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds EQL v3 (domain-based, plain-jsonb, extracted-index-term) encryption support for the
String/textscalar to@cipherstash/prisma-next, coexisting with the existing EQL v2 implementation behind a SQL-dialect version seam. v3 is additive — v2 emission and behaviour are byte-for-byte unchanged.What's new
cipherstash.EncryptedStringV3({ index })column type (PSL + TSencryptedStringV3), one index capability per column → one Postgres domain (eql_v3.text_eq/text_match/text_ord).v2Dialect/v3Dialect,dialectForCodecId): operator factories compute the lowering template insideimplfrom the column's codec id — extracted-index-term SQL for v3 (eq_term = hmac_256(…),ord_term <> ore_block_…,match_term @> bloom_filter).cipherstash/string-v3@1) reusing the sharedmakeCipherstashCellCodecfactory + theEncryptedStringenvelope; param cast is plainjsonb(not the domain) so search terms don't fail the full-payload domain CHECK.bulkEncryptV3Middlewareroutes WHERE needles to the SDK's newbulkEncryptQuery(search terms viaencryptQuery) and stored values tobulkEncrypt, keyed by aqueryTypemarker stamped by the v3 operators.expandNativeType; v3 baseline migration installs the vendoredeql_v3bundle alongside v2.cipherstashFromStackwiring;deriveStackSchemasv3 branch; README + DEVELOPING docs.Design notes
cipherstashGton an equality column) is rejected at query-build time by a runtime guard (milestone-1 trade-off; per-index codec ids could restore compile-time gating later).ORDER BYsort (cipherstashAsc/Desc) is not yet wired for v3 (helpers are v2-only) — documented.eql_v3SQL bundle is vendored byte-for-byte from upstreamcipherstash/encrypt-query-language(same artifactpackages/drizzlevendors), pinned by a round-trip test.Test Plan
pnpm vitest runinpackages/prisma-next— 518 tests, 60 files, no type errors (24 v3 unit/property/integration files: domain-map, wire-codec, dialect, constants/traits, parameterized, codec, column-types + PSL authoring, operators (v3 SQL + full mismatch matrix + queryType tagging), v2-unchanged regression, sdk-adapter, middleware split, derive-schemas, decrypt read-path, migration + expandNativeType hook, runtime/control/from-stack wiring, edge cases, trait parity).pnpm typecheckclean (package + example).prisma-next contract emit+migration planagainst the exampleUserV3model emit exactlyCREATE TABLE user_v3 (email eql_v3.text_eq, bio eql_v3.text_match, name eql_v3.text_ord, …); example typechecks withdb.orm.UserV3.where(u => u.email.cipherstashEq(...)).examples/prisma/test/e2e/str-v3.e2e.test.ts) — authored + gated onDATABASE_URL; not yet run against live Postgres + ZeroKMS (requiresCS_*credentials).Notes for reviewers
pnpm lintis currently broken repo-wide by a pre-existing biome config (schema 1.8.3) vs pinned biome (^2.4.15) mismatch — unrelated to this PR.Summary by CodeRabbit
New Features
EncryptedStringV3column type with single-capability index selection (equality, freeTextSearch, orderAndRange)UserV3model demonstrates v3 column usageTests