Merge pull request #618 from cipherstash/feat/eql-v3-type-robustness

refactor(stack): restore erased EQL v3 envelope + Result types

Author: coderdan

.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<unknown[]>` 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<T, EncryptionError>`. 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).

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<Encrypted, …>`, 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<Result<Encrypted, EncryptionError>>['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<Encrypted>`; 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<Encrypted>` client contract.
+    createEncryptionOperatorsV3(erased)
+  })
 })

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/)
+  })
 })

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<Encrypted>
   bulkEncrypt?(
     plaintexts: never,
     opts: { table: AnyV3Table; column: AnyEncryptedV3Column },
-  ): unknown
+  ): ChainableOperation<BulkEncryptedData>
 }
 
 /**
@@ -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<T> = {
+  audit(config: AuditConfig): AuditableOperation<T>
+  then: PromiseLike<Result<T, EncryptionError>>['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<T> = {
+  withLockContext(lockContext: LockContext): AuditableOperation<T>
+  audit(config: AuditConfig): AuditableOperation<T>
+  then: PromiseLike<Result<T, EncryptionError>>['then']
 }
 
 async function mapWithConcurrency<T, R>(
@@ -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<T>(
+    op: ChainableOperation<T>,
     opts?: EncryptionOperatorCallOpts,
-  ): ChainableOperation {
+  ): AuditableOperation<T> {
     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)}`)

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'
@@ -443,7 +445,7 @@ export class EncryptedQueryBuilderV3Impl<
    */
   protected override async encryptCollectedTerms(
     terms: ScalarQueryTerm[],
-  ): Promise<unknown[]> {
+  ): Promise<EncryptedQueryResult[]> {
     const groups = new Map<
       V3ColumnLike,
       { indices: number[]; values: ScalarQueryTerm['value'][] }
@@ -459,7 +461,11 @@ export class EncryptedQueryBuilderV3Impl<
     const bulkEncrypt = this.encryptionClient.bulkEncrypt?.bind(
       this.encryptionClient,
     )
-    const results = new Array<unknown>(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_<name>` twins, so v3 sends full envelopes where
+    // v2 sends `encryptQuery` composite literals; both are `EncryptedQueryResult`.
+    const results = new Array<EncryptedQueryResult>(terms.length)
 
     await Promise.all(
       Array.from(groups, async ([column, { indices, values }]) => {
@@ -481,7 +487,7 @@ export class EncryptedQueryBuilderV3Impl<
     bulkEncrypt: NonNullable<EncryptionClient['bulkEncrypt']>,
     column: V3ColumnLike,
     values: ScalarQueryTerm['value'][],
-  ): Promise<unknown[]> {
+  ): Promise<Array<Encrypted | null>> {
     const baseOp = bulkEncrypt(
       values.map((plaintext) => ({ plaintext })) as never,
       { column, table: this.v3Table } as never,
@@ -497,21 +503,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<unknown[]> {
+  ): Promise<Encrypted[]> {
     return Promise.all(
       values.map(async (value) => {
         const baseOp = this.encryptionClient.encrypt(value, {

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<unknown[]> {
+  ): Promise<EncryptedQueryResult[]> {
     // 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[]
 }