From c4787c0cce9432702417c3de25395b61f7fc572b Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sat, 11 Jul 2026 04:11:30 +1000 Subject: [PATCH 1/3] refactor(stack): restore the erased EQL v3 envelope and Result types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refactor the integration harness (#616) was built to make safe. Both v3 adapters typed their operand-encryption paths as `unknown` and dropped the `Result` wrapper, so the payload type and the failure channel were invisible. - eql/v3/drizzle/operators.ts: `encrypt`/`bulkEncrypt` were typed to return `unknown`; the operation `Result` collapsed to `{ data?: unknown; failure? }`; the bulk response was cast `as Array<{ data: unknown }>`. Now the client surface returns a typed `ChainableOperation` / `ChainableOperation` whose `then` resolves `Result`, with the casts gone. - supabase/query-builder-v3.ts (and the base query-builder.ts): `encryptCollectedTerms` / `bulkEncryptGroup` / `encryptGroupPerTerm` returned `Promise`. They now return `EncryptedQueryResult[]` / `Array` / `Encrypted[]`. Types come from protect-ffi via stack's existing re-exports (`Encrypted`, `BulkEncryptedData`, `EncryptedQueryResult`) — NOT from @cipherstash/eql, which is only a devDependency; importing it into src would emit unresolvable .d.ts references. See the PR thread for why per-column precision isn't threadable through the dynamic operator API and the domain union is the right granularity. Un-erasing surfaced two things the `unknown` hid: - The Supabase base/v3 divergence: the v2 path yields `encryptQuery` composite literals, the v3 path yields `JSON.stringify`'d envelope strings. Both are `EncryptedQueryResult` (its `string` arm), now stated in the type. - A latent bug in `ChainableOperation`: it required `withLockContext` to return something re-lockable, but the real `EncryptOperationWithLockContext` isn't. Now two-tier (`ChainableOperation` -> `AuditableOperation`), which is what lets the real client satisfy the surface with no cast — the M1 regression the .test-d.ts guards. Strengthened that test: a `{ encrypt }` returning `unknown` no longer typechecks. Type-only: no runtime logic changed (same JSON bytes, same length check). Src typechecks with 0 errors; type tests pass; 703 adapter unit tests + 1561 total unit tests pass. Real-crypto integration is covered by #616's suites on CI — local re-run was blocked by an OrbStack daemon outage, not by this change. --- .changeset/eql-v3-adapter-type-robustness.md | 28 +++++++++ .../__tests__/drizzle-v3/operators.test-d.ts | 17 +++++- .../stack/src/eql/v3/drizzle/operators.ts | 61 +++++++++++++------ .../stack/src/supabase/query-builder-v3.ts | 23 ++++--- packages/stack/src/supabase/query-builder.ts | 3 +- 5 files changed, 105 insertions(+), 27 deletions(-) create mode 100644 .changeset/eql-v3-adapter-type-robustness.md diff --git a/.changeset/eql-v3-adapter-type-robustness.md b/.changeset/eql-v3-adapter-type-robustness.md new file mode 100644 index 000000000..5ce15a426 --- /dev/null +++ b/.changeset/eql-v3-adapter-type-robustness.md @@ -0,0 +1,28 @@ +--- +'@cipherstash/stack': patch +--- + +Restore the EQL v3 envelope and `Result` types the adapters were erasing. + +Both v3 adapters typed their operand-encryption paths as `unknown` and dropped +the `Result` wrapper, so the query-type encoding and the failure channel were +invisible to the type system: + +- `eql/v3/drizzle/operators.ts` typed the client's `encrypt`/`bulkEncrypt` as + returning `unknown`, collapsed the operation's `Result` to + `{ data?: unknown; failure?: { message } }`, and cast the bulk response to + `Array<{ data: unknown }>`. +- `supabase/query-builder-v3.ts` returned `Promise` from + `encryptCollectedTerms`, `bulkEncryptGroup` and `encryptGroupPerTerm`, and the + base `query-builder.ts` did the same. + +These now carry the SDK's real types — `Encrypted` (the storage envelope union, +which includes every v3 per-domain payload), `BulkEncryptedData`, and +`EncryptedQueryResult` — threaded through a properly-typed operation surface that +resolves `Result`. The Supabase divergence the erasure hid is +now explicit: the v2 path yields `encryptQuery` composite literals and the v3 +path yields `JSON.stringify`'d envelope strings, and both are `EncryptedQueryResult`. + +No runtime change. The un-erasure has teeth: a `{ encrypt }` double returning +`unknown` no longer satisfies `createEncryptionOperatorsV3`, pinned in +`operators.test-d.ts`. diff --git a/packages/stack/__tests__/drizzle-v3/operators.test-d.ts b/packages/stack/__tests__/drizzle-v3/operators.test-d.ts index 564c6615d..80e2c879c 100644 --- a/packages/stack/__tests__/drizzle-v3/operators.test-d.ts +++ b/packages/stack/__tests__/drizzle-v3/operators.test-d.ts @@ -1,7 +1,12 @@ +import type { Result } from '@byteslice/result' import { describe, expectTypeOf, it } from 'vitest' import type { EncryptionClient } from '@/encryption' +import type { AuditConfig } from '@/encryption/operations/base-operation' import type { EncryptionV3 } from '@/encryption/v3' import { createEncryptionOperatorsV3 } from '@/eql/v3/drizzle' +import type { EncryptionError } from '@/errors' +import type { LockContext } from '@/identity' +import type { Encrypted } from '@/types' /** * Static regression guard for M1: `createEncryptionOperatorsV3` must accept the @@ -27,8 +32,18 @@ describe('createEncryptionOperatorsV3 - client parameter (M1)', () => { }) it('accepts a minimal structural { encrypt } double', () => { + // The double now models what the real client returns: an operation whose + // `then` resolves a `Result`, not `unknown`. That is the point + // of the un-erasure — a `{ encrypt }` returning `unknown` no longer + // typechecks, because the factory reads `result.data` as an `Encrypted` + // envelope rather than casting it. + type Op = { + withLockContext(lc: LockContext): Op + audit(cfg: AuditConfig): Op + then: PromiseLike>['then'] + } const double = { - encrypt: (_plaintext: never, _opts: never) => ({}) as unknown, + encrypt: (_plaintext: never, _opts: never) => ({}) as Op, } expectTypeOf(createEncryptionOperatorsV3).toBeCallableWith(double) }) diff --git a/packages/stack/src/eql/v3/drizzle/operators.ts b/packages/stack/src/eql/v3/drizzle/operators.ts index 251f4e4b3..bfdba90a3 100644 --- a/packages/stack/src/eql/v3/drizzle/operators.ts +++ b/packages/stack/src/eql/v3/drizzle/operators.ts @@ -1,3 +1,4 @@ +import type { Result } from '@byteslice/result' import { and, asc, @@ -17,9 +18,11 @@ import { import type { PgTable } from 'drizzle-orm/pg-core' import type { AuditConfig } from '@/encryption/operations/base-operation' import type { AnyEncryptedV3Column, AnyV3Table } from '@/eql/v3' +import type { EncryptionError } from '@/errors' import type { LockContext } from '@/identity' import type { ColumnSchema } from '@/schema' import { matchNeedleError } from '@/schema/match-defaults' +import type { BulkEncryptedData, Encrypted } from '@/types' import { getEqlV3Column } from './column.js' import { extractEncryptionSchemaV3, @@ -48,11 +51,11 @@ type OperandEncryptionClient = { encrypt( plaintext: never, opts: { table: AnyV3Table; column: AnyEncryptedV3Column }, - ): unknown + ): ChainableOperation bulkEncrypt?( plaintexts: never, opts: { table: AnyV3Table; column: AnyEncryptedV3Column }, - ): unknown + ): ChainableOperation } /** @@ -91,13 +94,33 @@ export type EncryptionOperatorCallOpts = { audit?: AuditConfig } -type ChainableOperation = { - withLockContext(lockContext: LockContext): ChainableOperation - audit(config: AuditConfig): ChainableOperation - then: PromiseLike<{ - data?: unknown - failure?: { message: string } - }>['then'] +/** + * An SDK encryption operation after its lock context has been applied: still + * auditable and awaitable, but not re-lockable. `withLockContext` returns this, + * not the full {@link ChainableOperation}, mirroring the real + * `EncryptOperationWithLockContext`, which drops `withLockContext` (you cannot + * lock-context twice). Modelling that is what lets the real client type satisfy + * the structural surface with no cast. + */ +type AuditableOperation = { + audit(config: AuditConfig): AuditableOperation + then: PromiseLike>['then'] +} + +/** + * The subset of an SDK encryption operation this factory drives: the fluent + * `withLockContext`/`audit` chain, and a `then` that resolves the operation's + * `Result`. Generic over the resolved payload `T` so `encrypt` carries an + * `Encrypted` envelope and `bulkEncrypt` a `BulkEncryptedData`, rather than the + * `unknown` this erased to before. + * + * Structural, not the concrete `EncryptOperation` class, because the client is + * passed in and the factory must accept any implementation with this surface. + */ +type ChainableOperation = { + withLockContext(lockContext: LockContext): AuditableOperation + audit(config: AuditConfig): AuditableOperation + then: PromiseLike>['then'] } async function mapWithConcurrency( @@ -225,10 +248,10 @@ export function createEncryptionOperatorsV3( const ORDERING_INDEXES = ['ore', 'ope'] as const const MATCH_INDEXES = ['match'] as const - function applyOperationOptions( - op: ChainableOperation, + function applyOperationOptions( + op: ChainableOperation, opts?: EncryptionOperatorCallOpts, - ): ChainableOperation { + ): AuditableOperation { const lockContext = opts?.lockContext ?? defaults.lockContext const audit = opts?.audit ?? defaults.audit const withLock = lockContext ? op.withLockContext(lockContext) : op @@ -298,12 +321,13 @@ export function createEncryptionOperatorsV3( client.encrypt(value as never, { table: ctx.table, column: ctx.builder, - }) as unknown as ChainableOperation, + }), opts, ) if (result.failure) { throw operandFailure(ctx, operator, result.failure.message) } + // `result.data` is now `Encrypted` — the storage envelope — not `unknown`. return sql`${JSON.stringify(result.data)}` } @@ -336,19 +360,22 @@ export function createEncryptionOperatorsV3( bulkEncrypt(values.map((plaintext) => ({ plaintext })) as never, { table: ctx.table, column: ctx.builder, - }) as unknown as ChainableOperation, + }), opts, ) if (result.failure) { throw operandFailure(ctx, operator, result.failure.message) } - const encrypted = result.data as Array<{ data: unknown }> | undefined - if (!Array.isArray(encrypted) || encrypted.length !== values.length) { + // `result.data` is `BulkEncryptedData` — `{ id?, data: Encrypted | null }[]` + // — not `unknown`. The length check stays: a position-stable contract + // violation must not silently truncate the predicate. + const encrypted = result.data + if (encrypted.length !== values.length) { throw operandFailure( ctx, operator, - `bulk encryption returned ${Array.isArray(encrypted) ? encrypted.length : 0} terms for ${values.length} values.`, + `bulk encryption returned ${encrypted.length} terms for ${values.length} values.`, ) } return encrypted.map((term) => sql`${JSON.stringify(term.data)}`) diff --git a/packages/stack/src/supabase/query-builder-v3.ts b/packages/stack/src/supabase/query-builder-v3.ts index 51bd7d230..0bda15026 100644 --- a/packages/stack/src/supabase/query-builder-v3.ts +++ b/packages/stack/src/supabase/query-builder-v3.ts @@ -8,6 +8,8 @@ import type { } from '@/schema' import type { BuildableQueryColumn, + Encrypted, + EncryptedQueryResult, QueryTypeName, ScalarQueryTerm, } from '@/types' @@ -448,7 +450,7 @@ export class EncryptedQueryBuilderV3Impl< */ protected override async encryptCollectedTerms( terms: ScalarQueryTerm[], - ): Promise { + ): Promise { const groups = new Map< V3ColumnLike, { indices: number[]; values: ScalarQueryTerm['value'][] } @@ -464,7 +466,11 @@ export class EncryptedQueryBuilderV3Impl< const bulkEncrypt = this.encryptionClient.bulkEncrypt?.bind( this.encryptionClient, ) - const results = new Array(terms.length) + // Each term becomes the `JSON.stringify`'d storage envelope — a `string`, + // which is one arm of `EncryptedQueryResult`. PostgREST cannot cast a filter + // value to the `eql_v3.query_` twins, so v3 sends full envelopes where + // v2 sends `encryptQuery` composite literals; both are `EncryptedQueryResult`. + const results = new Array(terms.length) await Promise.all( Array.from(groups, async ([column, { indices, values }]) => { @@ -486,7 +492,7 @@ export class EncryptedQueryBuilderV3Impl< bulkEncrypt: NonNullable, column: V3ColumnLike, values: ScalarQueryTerm['value'][], - ): Promise { + ): Promise> { const baseOp = bulkEncrypt( values.map((plaintext) => ({ plaintext })) as never, { column, table: this.v3Table } as never, @@ -502,11 +508,12 @@ export class EncryptedQueryBuilderV3Impl< // `bulkEncrypt` is position-stable, so a length mismatch means the contract // was violated. Truncating instead would silently widen an `in` predicate - // (or narrow a `not.in`) to whatever came back. - const encrypted = result.data as Array<{ data: unknown }> | undefined - if (!Array.isArray(encrypted) || encrypted.length !== values.length) { + // (or narrow a `not.in`) to whatever came back. `result.data` is now + // `BulkEncryptedData` — `{ id?, data: Encrypted | null }[]` — not `unknown`. + const encrypted = result.data + if (encrypted.length !== values.length) { this.encryptionFailure( - `bulk encryption returned ${Array.isArray(encrypted) ? encrypted.length : 0} terms for ${values.length} values on column "${column.getName()}".`, + `bulk encryption returned ${encrypted.length} terms for ${values.length} values on column "${column.getName()}".`, ) } return encrypted.map((term) => term.data) @@ -516,7 +523,7 @@ export class EncryptedQueryBuilderV3Impl< private async encryptGroupPerTerm( column: V3ColumnLike, values: ScalarQueryTerm['value'][], - ): Promise { + ): Promise { return Promise.all( values.map(async (value) => { const baseOp = this.encryptionClient.encrypt(value, { diff --git a/packages/stack/src/supabase/query-builder.ts b/packages/stack/src/supabase/query-builder.ts index 1defbf8b7..c21d86def 100644 --- a/packages/stack/src/supabase/query-builder.ts +++ b/packages/stack/src/supabase/query-builder.ts @@ -10,6 +10,7 @@ import type { EncryptedTable, EncryptedTableColumn } from '@/schema' import { EncryptedColumn } from '@/schema' import type { BuildableQueryColumn, + EncryptedQueryResult, QueryTypeName, ScalarQueryTerm, } from '@/types' @@ -741,7 +742,7 @@ export class EncryptedQueryBuilderImpl< */ protected async encryptCollectedTerms( terms: ScalarQueryTerm[], - ): Promise { + ): Promise { // Batch encrypt all terms in one call const baseOp = this.encryptionClient.encryptQuery(terms) const op = this.lockContext From a068a4da5007d5e316373d569a37b437ad282502 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sun, 12 Jul 2026 21:19:06 +1000 Subject: [PATCH 2/3] fix(stack): address code-review findings on the v3 type restoration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - operators.test-d.ts: add the negative assertion the changeset claimed but never made — a `{ encrypt }` double returning `unknown` must be REJECTED by createEncryptionOperatorsV3. The prior positive `toBeCallableWith(double)` could not catch a re-erasure (a correct value is assignable to `unknown`), so the "pinned" guarantee was toothless. The `@ts-expect-error` now goes unused and fails the moment the client contract regresses to `unknown`. - query-builder.ts: type `EncryptedFilterState.encryptedValues` as `EncryptedQueryResult[]`, not `unknown[]`. `encryptCollectedTerms` was restored to return that type but the value widened straight back to `unknown` at this field, so the use site (`encryptedValues[i]`) still saw `unknown` — the restoration never reached the consumer for the base builder. Now it does. - query-builder-v3.ts: guard the `null` arm in `bulkEncryptGroup`. The restored `Encrypted | null` type makes null reachable; an unguarded null envelope would be `JSON.stringify`'d to the literal `"null"` and sent as a filter operand (matching the wrong rows). Treat it as a contract violation. - changeset: `patch` → `minor`. Tightening the public `createEncryptionOperatorsV3` client contract is a compile-time breaking change for loosely-typed downstream clients; corrected the "no runtime change" line to note the new null guard. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w --- .changeset/eql-v3-adapter-type-robustness.md | 18 ++++++++++++++---- .../__tests__/drizzle-v3/operators.test-d.ts | 15 +++++++++++++++ .../stack/src/supabase/query-builder-v3.ts | 14 +++++++++++++- packages/stack/src/supabase/query-builder.ts | 6 +++++- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/.changeset/eql-v3-adapter-type-robustness.md b/.changeset/eql-v3-adapter-type-robustness.md index 5ce15a426..f7cddd986 100644 --- a/.changeset/eql-v3-adapter-type-robustness.md +++ b/.changeset/eql-v3-adapter-type-robustness.md @@ -1,5 +1,5 @@ --- -'@cipherstash/stack': patch +'@cipherstash/stack': minor --- Restore the EQL v3 envelope and `Result` types the adapters were erasing. @@ -23,6 +23,16 @@ resolves `Result`. The Supabase divergence the erasure hid i now explicit: the v2 path yields `encryptQuery` composite literals and the v3 path yields `JSON.stringify`'d envelope strings, and both are `EncryptedQueryResult`. -No runtime change. The un-erasure has teeth: a `{ encrypt }` double returning -`unknown` no longer satisfies `createEncryptionOperatorsV3`, pinned in -`operators.test-d.ts`. +Bumped `minor`, not `patch`: `createEncryptionOperatorsV3` is a public export +(`@cipherstash/stack/eql/v3/drizzle`), and tightening its client contract from +`unknown` to a typed operation surface is a compile-time breaking change — a +downstream consumer passing a loosely-typed (`unknown`-returning) client double +will now fail `tsc`. That tightening has teeth: `operators.test-d.ts` pins it +with a negative type-test asserting an `unknown`-returning `{ encrypt }` double +is rejected (a positive "correctly-typed double is accepted" assertion cannot +catch a re-erasure, since a correct value is assignable to `unknown`). + +Behaviour is otherwise unchanged, with one addition: the Supabase v3 bulk path +now rejects a `null` envelope returned by `bulkEncrypt` (the restored +`Encrypted | null` type makes that arm reachable, and a `null` would otherwise +be `JSON.stringify`'d to the literal `"null"` and sent as a filter operand). diff --git a/packages/stack/__tests__/drizzle-v3/operators.test-d.ts b/packages/stack/__tests__/drizzle-v3/operators.test-d.ts index 80e2c879c..bb6cd1861 100644 --- a/packages/stack/__tests__/drizzle-v3/operators.test-d.ts +++ b/packages/stack/__tests__/drizzle-v3/operators.test-d.ts @@ -47,4 +47,19 @@ describe('createEncryptionOperatorsV3 - client parameter (M1)', () => { } expectTypeOf(createEncryptionOperatorsV3).toBeCallableWith(double) }) + + it('rejects a { encrypt } double that returns `unknown` (the erasure regression)', () => { + // The complement of the test above, and the one with teeth: `toBeCallableWith` + // a correctly-typed double keeps passing even if the client contract + // regressed to `unknown` (a correct value is assignable to `unknown`), so it + // cannot catch re-erasure. This can. `encrypt` returning `unknown` must NOT + // satisfy the factory, whose contract is `ChainableOperation`; if + // the erasure ever comes back, the `@ts-expect-error` goes unused and fails. + const erased = { + encrypt: (_plaintext: never, _opts: never): unknown => ({}), + } + // @ts-expect-error — `encrypt` returning `unknown` does not satisfy the + // factory's `ChainableOperation` client contract. + createEncryptionOperatorsV3(erased) + }) }) diff --git a/packages/stack/src/supabase/query-builder-v3.ts b/packages/stack/src/supabase/query-builder-v3.ts index 0bda15026..85a98f40f 100644 --- a/packages/stack/src/supabase/query-builder-v3.ts +++ b/packages/stack/src/supabase/query-builder-v3.ts @@ -516,7 +516,19 @@ export class EncryptedQueryBuilderV3Impl< `bulk encryption returned ${encrypted.length} terms for ${values.length} values on column "${column.getName()}".`, ) } - return encrypted.map((term) => term.data) + return encrypted.map((term, i) => { + // `BulkEncryptedData` types the element as `Encrypted | null`. A `null` + // envelope here would be `JSON.stringify`'d to the literal string `"null"` + // and sent as the filter operand — silently matching whatever `"null"` + // encodes to rather than failing. A query term should never encrypt to a + // null envelope, so treat it as a contract violation, not a value. + if (term.data === null) { + this.encryptionFailure( + `bulk encryption returned a null envelope at position ${i} for column "${column.getName()}".`, + ) + } + return term.data + }) } /** Fallback for a client that predates `bulkEncrypt`. */ diff --git a/packages/stack/src/supabase/query-builder.ts b/packages/stack/src/supabase/query-builder.ts index c21d86def..890cfc483 100644 --- a/packages/stack/src/supabase/query-builder.ts +++ b/packages/stack/src/supabase/query-builder.ts @@ -1533,7 +1533,11 @@ type TermMapping = } type EncryptedFilterState = { - encryptedValues: unknown[] + // `EncryptedQueryResult[]`, not `unknown[]` — `encryptCollectedTerms` returns + // that type, and typing the field to match is what lets the restored envelope + // type reach the use site (`encryptedValues[i]`) instead of widening back to + // `unknown` at this boundary. + encryptedValues: EncryptedQueryResult[] termMap: TermMapping[] } From 311dfc651c993fcd1c8196c18c3ed427c734618a Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Sun, 12 Jul 2026 21:21:36 +1000 Subject: [PATCH 3/3] test(stack): pin the v3 bulk null-envelope guard Drives `bulkEncrypt` to return a length-matched response with a null envelope at position 0 and asserts the query fails 500 with "null envelope at position 0" rather than sending the literal "null" as the `in` operand. Sibling to the existing length-mismatch guard test. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w --- .../__tests__/supabase-v3-builder.test.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/stack/__tests__/supabase-v3-builder.test.ts b/packages/stack/__tests__/supabase-v3-builder.test.ts index c879f5b2e..b2e8cc3ab 100644 --- a/packages/stack/__tests__/supabase-v3-builder.test.ts +++ b/packages/stack/__tests__/supabase-v3-builder.test.ts @@ -1901,4 +1901,29 @@ describe('v3 in-list term encryption batches by column', () => { expect(status).toBe(500) expect(error?.message).toMatch(/1 term(s)? for 2 value(s)?/) }) + + it('rejects a bulk response with a null envelope rather than sending "null"', async () => { + const supabase = createMockSupabase() + const encryption = createMockEncryptionClient() + // A length-matched response, but position 0 is a null envelope. Without the + // guard, `JSON.stringify(null)` would send the literal `"null"` as the `in` + // operand — matching whatever `"null"` encodes to instead of failing. + ;( + encryption as unknown as { bulkEncrypt: (...a: unknown[]) => unknown } + ).bulkEncrypt = () => + operation([{ data: null }, { data: fakeEnvelope('grace', 'nickname') }]) + + const { error, status } = await new EncryptedQueryBuilderV3Impl( + 'users', + users, + encryption, + supabase.client, + USERS_ALL_COLUMNS, + ) + .select('id') + .in('nickname', ['ada', 'grace']) + + expect(status).toBe(500) + expect(error?.message).toMatch(/null envelope at position 0/) + }) })