diff --git a/.changeset/eql-v3-adapter-type-robustness.md b/.changeset/eql-v3-adapter-type-robustness.md new file mode 100644 index 000000000..f7cddd986 --- /dev/null +++ b/.changeset/eql-v3-adapter-type-robustness.md @@ -0,0 +1,38 @@ +--- +'@cipherstash/stack': minor +--- + +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`. + +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 564c6615d..bb6cd1861 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,9 +32,34 @@ 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) }) + + 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/__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/) + }) }) 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..85a98f40f 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,21 +508,34 @@ 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) + 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`. */ 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..890cfc483 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 @@ -1532,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[] }