From 0ebf57e29f046b6e292299da280e625fd3d0b3fe Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 20:33:42 +1000 Subject: [PATCH 1/6] fix(stack): close v3 drizzle fail-open paths, fix false-negative tests An audit of the EQL v3 Drizzle adapter found the index/query-type wiring correct, but several tests unable to detect the bug they guard. Three stayed green under the exact regression they existed to catch, and two source paths failed open. Source: - `contains` now rejects a needle shorter than the tokenizer's `token_length`. A short needle builds an EMPTY bloom filter and `stored_bf @> '{}'` holds for every row. Measured live: needles "ad", "a" and "x" each returned 3/3 seeded rows -- "x" occurs in none of them. The floor lives in `schema/match-defaults` so v2 and v3 share it; they build byte-identical blooms. - `v3FromDriver` throws the new `EqlV3CodecError` rather than surfacing a raw SyntaxError on malformed JSON or passing a wrong-shape payload through: `v3FromDriver('5')` returned `5` typed as `Encrypted`. The shape guard accepts SteVec documents, whose ciphertext is at `sv[0].c` with no top-level `c`. - Removed the unreachable `sql`false`` fallback in `inArray`/`notInArray`; the empty-list guard throws first. Tests (each verified RED against the bug it guards, then reverted): - `and` was indistinguishable from `or`: both predicates were true for ROW_B alone, so swapping the operator still passed. Now paired over disjoint rows and asserted from both directions. - bigint filters ran against a one-row table, where `gt` returning every row passed. Seeded a negative row they must exclude. - `between` was only ever called with identical bounds against a constant encrypt stub, so a min/max transposition in `range` was invisible. Added an echoing stub that pins operand order. - `contains` had no needle that must NOT match, and none longer than `token_length`. The unit test searched `TEXT_S[0]` -- the empty string -- i.e. it asserted the fail-open. Now drives four needles through a substring oracle, including a negative one. - Seeded a third matrix row. With two, every predicate could only return [A], [B], [A,B] or [] -- ordering ties were untestable and `eq` over-matching undetectable. Row `i` takes `samples[min(i, len-1)]`, the scheme `v3-matrix/matrix-live-pg` already uses. Lock context: The suite supplied the same context on seed and query in every test, so a regression dropping it from the index term would drop it identically on both sides and stay green. Added the negatives obtainable with one JWT: an identity-bound row must not match an `eq` issued with no lock context, and must not decrypt without it. A cross-identity proof needs a second JWT with a different `sub` and remains a follow-up. `lock-context.test.ts` wrapped `decryptModel` in try/catch, but it reports denial as a `Result` and never throws -- the test ran zero assertions and would also have passed had decryption wrongly succeeded. Smaller pins: sql-dialect asserts operands BIND rather than interpolate; schema-extraction uses toStrictEqual; the barrel is pinned exhaustively; codec fixtures are realistic envelopes; the column's customType mappers are exercised. Not included, per plan: the SQL `bloom_filter` empty-array change (contradicts a documented semantic; needs sign-off) and the Supabase needle guard (lives on another branch). --- .changeset/eql-v3-drizzle-fail-open-guards.md | 25 ++ .../stack/__tests__/drizzle-v3/codec.test.ts | 95 +++++- .../stack/__tests__/drizzle-v3/column.test.ts | 41 +++ .../__tests__/drizzle-v3/exports.test.ts | 29 ++ .../drizzle-v3/operators-live-pg.test.ts | 283 ++++++++++++------ .../operators-lock-context-live-pg.test.ts | 80 ++++- .../__tests__/drizzle-v3/operators.test.ts | 66 +++- .../drizzle-v3/schema-extraction.test.ts | 12 +- .../__tests__/drizzle-v3/sql-dialect.test.ts | 42 +++ packages/stack/__tests__/lock-context.test.ts | 20 +- packages/stack/src/eql/v3/drizzle/codec.ts | 54 +++- packages/stack/src/eql/v3/drizzle/index.ts | 2 +- .../stack/src/eql/v3/drizzle/operators.ts | 29 +- packages/stack/src/schema/match-defaults.ts | 39 +++ 14 files changed, 704 insertions(+), 113 deletions(-) create mode 100644 .changeset/eql-v3-drizzle-fail-open-guards.md diff --git a/.changeset/eql-v3-drizzle-fail-open-guards.md b/.changeset/eql-v3-drizzle-fail-open-guards.md new file mode 100644 index 000000000..7774e7e3b --- /dev/null +++ b/.changeset/eql-v3-drizzle-fail-open-guards.md @@ -0,0 +1,25 @@ +--- +'@cipherstash/stack': minor +--- + +Close two fail-open paths in the EQL v3 Drizzle adapter. + +`ops.contains()` now throws `EncryptionOperatorError` when the search term is +shorter than the match index tokenizer's `token_length` (3 by default). Such a +term produces no ngrams, so its bloom filter is empty — and `stored_bf @> '{}'` +is true for every row. Previously a user searching `"ad"` silently received the +entire table; measured live, the needles `"ad"`, `"a"` and `"x"` each returned +every seeded row, including one in which `"x"` did not appear. The length floor +is shared with the v2 adapter via `matchNeedleError` in `schema/match-defaults`, +since both build byte-identical bloom filters. + +`v3FromDriver()` now throws the new `EqlV3CodecError` on a payload that is not +an EQL envelope, instead of surfacing a raw `SyntaxError` for malformed JSON and +passing a bare scalar through unchecked — `v3FromDriver('5')` previously returned +`5` typed as `Encrypted`, which then reached `decrypt` as garbage. The guard +accepts both scalar envelopes (ciphertext at `c`) and SteVec documents +(ciphertext at `sv[0].c`). `EqlV3CodecError` is exported from +`@cipherstash/stack/eql/v3/drizzle` so callers can catch it. + +Also removes an unreachable branch in `inArray`/`notInArray`, whose empty-list +guard already throws before it. diff --git a/packages/stack/__tests__/drizzle-v3/codec.test.ts b/packages/stack/__tests__/drizzle-v3/codec.test.ts index 698b34821..3deb58d50 100644 --- a/packages/stack/__tests__/drizzle-v3/codec.test.ts +++ b/packages/stack/__tests__/drizzle-v3/codec.test.ts @@ -1,9 +1,32 @@ import { describe, expect, it } from 'vitest' -import { v3FromDriver, v3ToDriver } from '@/eql/v3/drizzle/codec' +import { + EqlV3CodecError, + v3FromDriver, + v3ToDriver, +} from '@/eql/v3/drizzle/codec' + +// A realistic `public.text_eq` envelope: schema version, table/column +// identifier, mp_base85 ciphertext, HMAC term. The trivial `{v:1,c:'ct'}` +// fixture this replaced could not distinguish an envelope from a bare object. +const ENVELOPE = { + v: 3, + i: { t: 'users', c: 'email' }, + c: 'mBbKmbNmNhX@Vv0N%1jQvA', + hm: '3a1f9c0e5b7d2a48e6c1f0b93d7a2e58c4b6f19d0e3a7c25b8f14d69a0c3e7b2', +} as const + +// SteVec documents carry the root ciphertext at `sv[0].c` and have no top-level +// `c` — the envelope guard must accept them. +const STE_VEC_ENVELOPE = { + v: 3, + k: 'sv', + i: { t: 'users', c: 'profile' }, + sv: [{ c: 'mBbKciphertext', s: 'selector', b: 'term' }], +} as const describe('v3 codec', () => { - it('serialises an object to a jsonb string', () => { - expect(v3ToDriver({ v: 1, c: 'ct' })).toBe('{"v":1,"c":"ct"}') + it('serialises an envelope to a jsonb string', () => { + expect(v3ToDriver(ENVELOPE as never)).toBe(JSON.stringify(ENVELOPE)) }) it('maps null/undefined to SQL NULL (JS null), never the JSON null literal', () => { @@ -11,13 +34,29 @@ describe('v3 codec', () => { expect(v3ToDriver(undefined)).toBeNull() }) - it('parses a jsonb string back to an object', () => { - expect(v3FromDriver('{"v":1,"c":"ct"}')).toEqual({ v: 1, c: 'ct' }) + it('round-trips a realistic envelope through both directions', () => { + const serialised = v3ToDriver(ENVELOPE as never) + expect(v3FromDriver(serialised)).toEqual(ENVELOPE) + }) + + it('round-trips unicode and a long ciphertext', () => { + const envelope = { + ...ENVELOPE, + i: { t: 'users', c: 'café_☕' }, + c: 'z'.repeat(8192), + } + expect(v3FromDriver(v3ToDriver(envelope as never))).toEqual(envelope) + }) + + it('passes an already-parsed envelope through unchanged', () => { + expect(v3FromDriver(ENVELOPE as never)).toBe(ENVELOPE) }) - it('passes an already-parsed object through unchanged', () => { - const obj = { v: 1 } - expect(v3FromDriver(obj)).toBe(obj) + it('accepts a SteVec document, whose ciphertext lives at sv[0].c', () => { + expect(v3FromDriver(STE_VEC_ENVELOPE as never)).toBe(STE_VEC_ENVELOPE) + expect(v3FromDriver(JSON.stringify(STE_VEC_ENVELOPE))).toEqual( + STE_VEC_ENVELOPE, + ) }) it('normalises null/undefined to SQL NULL (JS null) on read', () => { @@ -26,6 +65,44 @@ describe('v3 codec', () => { }) it('does not throw on a stray bigint in the envelope (defensive)', () => { - expect(v3ToDriver({ v: 1n } as never)).toBe('{"v":"1"}') + expect(v3ToDriver({ ...ENVELOPE, v: 3n } as never)).toContain('"v":"3"') + }) + + // Before these, a malformed string surfaced a raw SyntaxError and a + // wrong-shape payload was returned as-is — `v3FromDriver('5')` yielded `5` + // typed as an envelope, which then reached `decrypt` as garbage. + describe('malformed and non-envelope payloads', () => { + it.each([ + '{bad', + '', + '{"v":3,', + ])('throws EqlV3CodecError on unparseable jsonb %j', (raw) => { + expect(() => v3FromDriver(raw)).toThrow(EqlV3CodecError) + expect(() => v3FromDriver(raw)).toThrow(/Failed to parse/) + }) + + it.each([ + '5', + '"a string"', + 'true', + '[]', + '[{"v":3,"c":"ct"}]', + ])('throws on valid JSON %j that is not an envelope', (raw) => { + expect(() => v3FromDriver(raw)).toThrow(EqlV3CodecError) + }) + + it('throws on an object missing the schema version', () => { + expect(() => v3FromDriver({ c: 'ct' })).toThrow(/missing "v"/) + }) + + it('throws on an object carrying no ciphertext', () => { + expect(() => v3FromDriver({ v: 3, i: { t: 'u', c: 'e' } })).toThrow( + /missing a ciphertext/, + ) + }) + + it('throws on an array', () => { + expect(() => v3FromDriver([ENVELOPE] as never)).toThrow(/got an array/) + }) }) }) diff --git a/packages/stack/__tests__/drizzle-v3/column.test.ts b/packages/stack/__tests__/drizzle-v3/column.test.ts index 465938c03..d1819cead 100644 --- a/packages/stack/__tests__/drizzle-v3/column.test.ts +++ b/packages/stack/__tests__/drizzle-v3/column.test.ts @@ -133,4 +133,45 @@ describe('makeEqlV3Column', () => { expect(pgColumn?.getSQLType()).toBe(eqlType) expect(getEqlV3Column(columnName, pgColumn)?.getEqlType()).toBe(eqlType) }) + + // The `toDriver`/`fromDriver` closures wired into `customType` (column.ts) are + // otherwise only tested via the standalone codec functions, so the wiring + // itself — including the SQL-NULL safety net — was never exercised. + describe('codec wiring on the built column', () => { + type Mapper = { + mapToDriverValue(value: unknown): unknown + mapFromDriverValue(value: unknown): unknown + } + const mapperFor = (name: string): Mapper => { + const table = pgTable('users', { + [name]: makeEqlV3Column(v3Types.TextEq(name)), + } as never) + return (table as unknown as Record)[name] + } + + const ENVELOPE = { + v: 3, + i: { t: 'users', c: 'nickname' }, + c: 'mBbKciphertext', + hm: 'hmac', + } + + it('serialises an envelope on write and parses it back on read', () => { + const column = mapperFor('nickname') + const driverValue = column.mapToDriverValue(ENVELOPE) + + expect(driverValue).toBe(JSON.stringify(ENVELOPE)) + expect(column.mapFromDriverValue(driverValue)).toEqual(ENVELOPE) + }) + + it('maps a null envelope to SQL NULL on write', () => { + expect(mapperFor('nickname').mapToDriverValue(null)).toBeNull() + }) + + it('rejects a non-envelope driver value rather than passing it through', () => { + expect(() => mapperFor('nickname').mapFromDriverValue('5')).toThrow( + /EQL encrypted envelope/, + ) + }) + }) }) diff --git a/packages/stack/__tests__/drizzle-v3/exports.test.ts b/packages/stack/__tests__/drizzle-v3/exports.test.ts index 541c06fa8..a192be375 100644 --- a/packages/stack/__tests__/drizzle-v3/exports.test.ts +++ b/packages/stack/__tests__/drizzle-v3/exports.test.ts @@ -1,12 +1,41 @@ import { describe, expect, it } from 'vitest' import * as barrel from '@/eql/v3/drizzle' +// Exhaustive, so ADDING an export fails here too. The previous version asserted +// only four names, leaving the codec/column re-exports unpinned and letting a +// new export slip into the public surface unnoticed. +const PUBLIC_SURFACE = [ + 'EncryptionOperatorError', + 'EqlV3CodecError', + 'createEncryptionOperatorsV3', + 'extractEncryptionSchemaV3', + 'getEqlV3Column', + 'isEqlV3Column', + 'makeEqlV3Column', + 'types', + 'v3FromDriver', + 'v3ToDriver', +] as const + describe('v3 drizzle barrel', () => { + it('exports exactly the public surface', () => { + expect(Object.keys(barrel).sort()).toEqual([...PUBLIC_SURFACE]) + }) + it('exports the public surface', () => { expect(typeof barrel.createEncryptionOperatorsV3).toBe('function') expect(typeof barrel.extractEncryptionSchemaV3).toBe('function') expect(typeof barrel.EncryptionOperatorError).toBe('function') + expect(typeof barrel.EqlV3CodecError).toBe('function') expect(typeof barrel.types.TextSearch).toBe('function') expect(barrel.types.IntegerOrd('age')).toBeDefined() }) + + it('re-exports the codec and column helpers as callables', () => { + expect(typeof barrel.v3ToDriver).toBe('function') + expect(typeof barrel.v3FromDriver).toBe('function') + expect(typeof barrel.getEqlV3Column).toBe('function') + expect(typeof barrel.isEqlV3Column).toBe('function') + expect(typeof barrel.makeEqlV3Column).toBe('function') + }) }) diff --git a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts index 9819d955b..0972af1c8 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts @@ -38,6 +38,14 @@ const ACCOUNT_TABLE_NAME = 'protect_ci_v3_drizzle_matrix_accounts' const RUN = `run-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` const ROW_A = 'row-a' const ROW_B = 'row-b' +// A third row. With only two rows every predicate can return just [A], [B], +// [A,B] or [] — so an `eq` that over-matches a near-miss value is undetectable, +// and ordering can never carry a tie. Row `i` takes `samples[min(i, len-1)]` +// per domain (the scheme `v3-matrix/matrix-live-pg.test.ts` already uses), so +// domains with a third distinct sample get a near-miss row, and domains with +// only two (date, timestamp, boolean) get a deliberate ORDER BY tie with ROW_B. +const ROW_C = 'row-c' +const ROWS = [ROW_A, ROW_B, ROW_C] as const const matrixEntries = typedEntries(V3_MATRIX) const matrixColumns = Object.fromEntries( @@ -82,12 +90,16 @@ const bigintTable = pgTable(BIGINT_TABLE_NAME, { // Number.MAX_SAFE_INTEGER losslessly through the encrypted column. const BIGINT_BALANCE = 9223372036854775807n const BIGINT_LEDGER = -9223372036854775808n +// A second row the filters must exclude: a negative balance (so `gt(0n)` has +// something to reject) and a small positive ledger. +const BIGINT_B_BALANCE = -5n +const BIGINT_B_LEDGER = 100n const schema = extractEncryptionSchemaV3(matrixTable) const bigintSchema = extractEncryptionSchemaV3(bigintTable) type PlainValue = string | number | bigint | boolean | Date -type RowKey = typeof ROW_A | typeof ROW_B +type RowKey = (typeof ROWS)[number] type MatrixPlainRow = Record type SelectRow = { rowKey: string } type Db = ReturnType @@ -130,7 +142,7 @@ const scoped = (cond: SQL | undefined): SQL | undefined => cond ? and(drizzleEq(matrixTable.testRunId, RUN), cond) : cond const plainValue = (spec: DomainSpec, rowKey: RowKey): PlainValue => - spec.samples[rowKey === ROW_A ? 0 : 1] + spec.samples[Math.min(ROWS.indexOf(rowKey), spec.samples.length - 1)] function comparePlain(left: PlainValue, right: PlainValue): number { if (left instanceof Date && right instanceof Date) { @@ -162,15 +174,21 @@ function expectedKeysFor( spec: DomainSpec, predicate: (value: PlainValue) => boolean, ): RowKey[] { - return ([ROW_A, ROW_B] as const).filter((rowKey) => - predicate(plainValue(spec, rowKey)), - ) + return ROWS.filter((rowKey) => predicate(plainValue(spec, rowKey))) } +/** + * Oracle for the encrypted ORDER BY. Domains with only two samples give ROW_B + * and ROW_C equal values, so the comparison alone does not determine the row + * order — ties break on `rowKey` ascending, which the query mirrors with a + * secondary `ORDER BY row_key`. Without that both sides would be arbitrary and + * the test would flake rather than prove ordering. + */ function sortedKeysFor(spec: DomainSpec, direction: 'asc' | 'desc'): RowKey[] { - return ([ROW_A, ROW_B] as const).sort((left, right) => { + return [...ROWS].sort((left, right) => { const cmp = comparePlain(plainValue(spec, left), plainValue(spec, right)) - return direction === 'asc' ? cmp : -cmp + if (cmp !== 0) return direction === 'asc' ? cmp : -cmp + return left < right ? -1 : left > right ? 1 : 0 }) } @@ -190,11 +208,11 @@ function unwrap(result: { data?: T; failure?: { message: string } }): T { } function encryptedInsertRows(): MatrixPlainRow[] { - return ([ROW_A, ROW_B] as const).map((rowKey) => { + return ROWS.map((rowKey) => { const row: MatrixPlainRow = { rowKey, testRunId: RUN, - nullableTextEq: rowKey === ROW_A ? null : 'nullable-present', + nullableTextEq: rowKey === ROW_A ? null : `nullable-${rowKey}`, } for (const [eqlType, spec] of matrixEntries) { row[slug(eqlType)] = plainValue(spec, rowKey) @@ -253,6 +271,10 @@ beforeAll(async () => { // A3 end-to-end, cast-free: encrypt a native bigint model, insert the // resulting envelope rows (typed against the column's `Encrypted` data slot), // no `as never` anywhere. + // + // ROW_B exists so the filter proofs below have a row they must EXCLUDE. On a + // one-row table `gt(balance, 0n)` returning every row is indistinguishable + // from it returning the right row. const bigintRows = unwrap( await client.bulkEncryptModels( [ @@ -262,6 +284,12 @@ beforeAll(async () => { balance: BIGINT_BALANCE, ledger: BIGINT_LEDGER, }, + { + rowKey: ROW_B, + testRunId: RUN, + balance: BIGINT_B_BALANCE, + ledger: BIGINT_B_LEDGER, + }, ], bigintSchema, ), @@ -281,21 +309,29 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { it.each(equalityDomains)( '%s eq selects the exact row', async (eqlType, spec) => { + const bound = plainValue(spec, ROW_A) const rows = await selectRowKeys( - await ops.eq(matrixColumn(eqlType), plainValue(spec, ROW_A)), + await ops.eq(matrixColumn(eqlType), bound), + ) + // Oracle, not `[ROW_A]`: ROW_C carries a near-miss value for domains with + // a third sample, so an `eq` that over-matches now shows up here. + expect(rows).toEqual( + expectedKeysFor(spec, (value) => comparePlain(value, bound) === 0), ) - expect(rows).toEqual([ROW_A]) }, 30000, ) it.each(equalityDomains)( - '%s ne selects the complement row', + '%s ne selects the complement rows', async (eqlType, spec) => { + const bound = plainValue(spec, ROW_A) const rows = await selectRowKeys( - await ops.ne(matrixColumn(eqlType), plainValue(spec, ROW_A)), + await ops.ne(matrixColumn(eqlType), bound), + ) + expect(rows).toEqual( + expectedKeysFor(spec, (value) => comparePlain(value, bound) !== 0), ) - expect(rows).toEqual([ROW_B]) }, 30000, ) @@ -303,13 +339,15 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { it.each(equalityDomains)( '%s inArray selects all listed rows', async (eqlType, spec) => { + const listed = [plainValue(spec, ROW_A), plainValue(spec, ROW_B)] const rows = await selectRowKeys( - await ops.inArray(matrixColumn(eqlType), [ - plainValue(spec, ROW_A), - plainValue(spec, ROW_B), - ]), + await ops.inArray(matrixColumn(eqlType), listed), + ) + expect(rows).toEqual( + expectedKeysFor(spec, (value) => + listed.some((entry) => comparePlain(value, entry) === 0), + ), ) - expect(rows).toEqual([ROW_A, ROW_B]) }, 30000, ) @@ -317,10 +355,13 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { it.each(equalityDomains)( '%s notInArray excludes listed rows', async (eqlType, spec) => { + const bound = plainValue(spec, ROW_A) const rows = await selectRowKeys( - await ops.notInArray(matrixColumn(eqlType), [plainValue(spec, ROW_A)]), + await ops.notInArray(matrixColumn(eqlType), [bound]), + ) + expect(rows).toEqual( + expectedKeysFor(spec, (value) => comparePlain(value, bound) !== 0), ) - expect(rows).toEqual([ROW_B]) }, 30000, ) @@ -339,21 +380,29 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { 30000, ) + // The bounds span ROW_A and ROW_B. ROW_C carries a third sample that may sit + // outside that span (most domains) or tie with ROW_B (date, timestamp), so + // membership is computed rather than assumed — with only A and B seeded these + // were `[ROW_A, ROW_B]` and `[]`, which no longer holds. + const spanBounds = (spec: DomainSpec): [PlainValue, PlainValue] => { + const sorted = [plainValue(spec, ROW_A), plainValue(spec, ROW_B)].sort( + comparePlain, + ) + return [sorted[0], sorted[1]] + } + const withinSpan = (spec: DomainSpec) => (value: PlainValue) => { + const [min, max] = spanBounds(spec) + return comparePlain(value, min) >= 0 && comparePlain(value, max) <= 0 + } + it.each(orderDomains)( '%s between selects the inclusive range', async (eqlType, spec) => { - const sortedValues = [ - plainValue(spec, ROW_A), - plainValue(spec, ROW_B), - ].sort(comparePlain) + const [min, max] = spanBounds(spec) const rows = await selectRowKeys( - await ops.between( - matrixColumn(eqlType), - sortedValues[0], - sortedValues[1], - ), + await ops.between(matrixColumn(eqlType), min, max), ) - expect(rows).toEqual([ROW_A, ROW_B]) + expect(rows).toEqual(expectedKeysFor(spec, withinSpan(spec))) }, 30000, ) @@ -361,26 +410,21 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { it.each(orderDomains)( '%s notBetween excludes the inclusive range', async (eqlType, spec) => { - const sortedValues = [ - plainValue(spec, ROW_A), - plainValue(spec, ROW_B), - ].sort(comparePlain) + const [min, max] = spanBounds(spec) const rows = await selectRowKeys( - await ops.notBetween( - matrixColumn(eqlType), - sortedValues[0], - sortedValues[1], - ), + await ops.notBetween(matrixColumn(eqlType), min, max), + ) + expect(rows).toEqual( + expectedKeysFor(spec, (value) => !withinSpan(spec)(value)), ) - expect(rows).toEqual([]) }, 30000, ) - // The spanning cases above only ever prove INCLUSION (both rows are inside - // the range, so `between` -> [A,B] and `notBetween` -> []). These narrow - // cases use a single-point range at ROW_A's value to prove the operators - // also EXCLUDE: `between` must drop ROW_B, `notBetween` must keep it. Without + // The spanning cases above put ROW_A and ROW_B inside the range, so on most + // domains only ROW_C proves exclusion. These narrow cases use a single-point + // range at ROW_A's value to prove the operators EXCLUDE regardless of where + // ROW_C falls: `between` must drop ROW_B, `notBetween` must keep it. Without // these, a `between` that matched everything (or a `notBetween` no-op) passes. it.each(orderDomains)( '%s between at a single point excludes the out-of-range row', @@ -441,6 +485,9 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { 30000, ) + // The secondary `ORDER BY row_key` mirrors the oracle's tie-break. Domains + // with only two samples (date, timestamp) give ROW_B and ROW_C equal values, + // so without it the tied pair's order is arbitrary in Postgres. it.each(orderDomains)( '%s asc orders by encrypted order term', async (eqlType, spec) => { @@ -448,7 +495,10 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { .select({ rowKey: matrixTable.rowKey }) .from(matrixTable) .where(drizzleEq(matrixTable.testRunId, RUN)) - .orderBy(ops.asc(matrixColumn(eqlType)))) as SelectRow[] + .orderBy( + ops.asc(matrixColumn(eqlType)), + drizzleAsc(matrixTable.rowKey), + )) as SelectRow[] expect(rows.map((row) => row.rowKey)).toEqual(sortedKeysFor(spec, 'asc')) }, 30000, @@ -461,27 +511,52 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { .select({ rowKey: matrixTable.rowKey }) .from(matrixTable) .where(drizzleEq(matrixTable.testRunId, RUN)) - .orderBy(ops.desc(matrixColumn(eqlType)))) as SelectRow[] + .orderBy( + ops.desc(matrixColumn(eqlType)), + drizzleAsc(matrixTable.rowKey), + )) as SelectRow[] expect(rows.map((row) => row.rowKey)).toEqual(sortedKeysFor(spec, 'desc')) }, 30000, ) - it.each(matchDomains)( - '%s contains matches plaintext terms', - async (eqlType, spec) => { + // Needles are driven through the same substring oracle, so each domain gets + // the rows it should. `'ada'` is exactly `token_length`; `'lovelace'` and + // `'grace'` are longer (the suite previously had no `contains` proof above + // the token length at all); `'qqqzzz'` is present in no row, so a `contains` + // that matched everything would fail here rather than pass silently. + it.each( + matchDomains.flatMap(([eqlType, spec]) => + ['ada', 'lovelace', 'grace', 'qqqzzz'].map( + (needle) => [eqlType, spec, needle] as const, + ), + ), + )( + '%s contains %s matches exactly the rows holding it', + async (eqlType, spec, needle) => { const rows = await selectRowKeys( - await ops.contains(matrixColumn(eqlType), 'ada'), + await ops.contains(matrixColumn(eqlType), needle), ) expect(rows).toEqual( expectedKeysFor(spec, (value) => - String(value).toLowerCase().includes('ada'), + String(value).toLowerCase().includes(needle), ), ) }, 30000, ) + // A needle below the tokenizer's `token_length` builds an EMPTY bloom filter, + // and `stored_bf @> '{}'` holds for every row — so before the SDK guard this + // silently returned the whole table. + it.each( + matchDomains, + )('%s contains rejects a needle shorter than token_length', async (eqlType) => { + await expect( + ops.contains(matrixColumn(eqlType), 'ad'), + ).rejects.toBeInstanceOf(EncryptionOperatorError) + }, 30000) + it.each( noEqualityDomains, )('%s eq rejects unsupported equality', async (eqlType, spec) => { @@ -512,14 +587,26 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { ).rejects.toBeInstanceOf(EncryptionOperatorError) }) - it('and combines encrypted predicates', async () => { - const rows = await selectRowKeys( - await ops.and( - ops.eq(matrixColumn('public.text_eq'), 'ada@example.com'), - ops.lt(matrixColumn('public.integer_ord'), 0), - ), - ) - expect(rows).toEqual([ROW_B]) + // The two predicates must be satisfied by DISJOINT row sets, or `and` and + // `or` return the same thing and the test cannot tell them apart. The old + // pairing (`text_eq = 'ada@example.com'` AND `integer_ord < 0`) was true for + // ROW_B alone under both operators, so swapping `and` for `or` still passed. + // text_eq = 'ada@example.com' -> ROW_B only + // integer_ord >= 0 -> ROW_A (0) and ROW_C (2147483647), not ROW_B (-42) + const disjointPredicates = () => + [ + ops.eq(matrixColumn('public.text_eq'), 'ada@example.com'), + ops.gte(matrixColumn('public.integer_ord'), 0), + ] as const + + it('and requires both encrypted predicates, unlike or', async () => { + const rows = await selectRowKeys(await ops.and(...disjointPredicates())) + expect(rows).toEqual([]) + }, 30000) + + it('or requires either encrypted predicate, unlike and', async () => { + const rows = await selectRowKeys(await ops.or(...disjointPredicates())) + expect(rows).toEqual([ROW_A, ROW_B, ROW_C]) }, 30000) it('or combines encrypted predicates', async () => { @@ -536,7 +623,7 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { const rows = await selectRowKeys( ops.not(await ops.eq(matrixColumn('public.text_eq'), '')), ) - expect(rows).toEqual([ROW_B]) + expect(rows).toEqual([ROW_B, ROW_C]) }, 30000) it('isNull and isNotNull work on nullable encrypted columns', async () => { @@ -545,7 +632,7 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { ) expect( await selectRowKeys(ops.isNotNull(matrixTable.nullableTextEq)), - ).toEqual([ROW_B]) + ).toEqual([ROW_B, ROW_C]) }, 30000) it('exists and notExists work with correlated subqueries', async () => { @@ -571,9 +658,11 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { ) expect(await selectRowKeys(ops.exists(primaryAccount))).toEqual([ROW_A]) + // ROW_C has no account row at all, so it too has no 'missing' account. expect(await selectRowKeys(ops.notExists(missingAccount))).toEqual([ ROW_A, ROW_B, + ROW_C, ]) }, 30000) @@ -626,7 +715,8 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { ]), ) // '' -> ROW_A, 'ada@example.com' -> ROW_B; the three "nobody" terms match - // nothing, exercising the pool without changing the expected set. + // nothing, exercising the pool without changing the expected set. ROW_C + // ('Ada Lovelace') is listed by neither and must be excluded. expect(rows).toEqual([ROW_A, ROW_B]) }, 30000) @@ -640,8 +730,9 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { 'nobody-4@example.com', ]), ) - // Only '' is excluded (ROW_A); ROW_B ('ada@example.com') survives. - expect(rows).toEqual([ROW_B]) + // Only '' is excluded (ROW_A); ROW_B ('ada@example.com') and ROW_C + // ('Ada Lovelace') survive. + expect(rows).toEqual([ROW_B, ROW_C]) }, 30000) // A3 + bigint lock: a statically-typed bigint table round-trips a real i64 @@ -651,7 +742,12 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { const encrypted = await db .select({ balance: bigintTable.balance, ledger: bigintTable.ledger }) .from(bigintTable) - .where(drizzleEq(bigintTable.testRunId, RUN)) + .where( + and( + drizzleEq(bigintTable.testRunId, RUN), + drizzleEq(bigintTable.rowKey, ROW_A), + ), + ) expect(encrypted).toHaveLength(1) const decrypted = unwrap( @@ -661,27 +757,44 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { expect(decrypted.ledger).toBe(BIGINT_LEDGER) }, 30000) - it('filters a bigint column by encrypted equality and ordering', async () => { - const byLedger = (await db + // Each assertion below names a row that must be EXCLUDED. Against the former + // one-row table an operator matching everything passed all of these. + const bigintRowKeys = async (condition: SQL): Promise => { + const rows = (await db .select({ rowKey: bigintTable.rowKey }) .from(bigintTable) - .where( - and( - drizzleEq(bigintTable.testRunId, RUN), - await ops.eq(bigintTable.ledger, BIGINT_LEDGER), - ), - )) as SelectRow[] - expect(byLedger.map((row) => row.rowKey)).toEqual([ROW_A]) + .where(and(drizzleEq(bigintTable.testRunId, RUN), condition)) + .orderBy(drizzleAsc(bigintTable.rowKey))) as SelectRow[] + return rows.map((row) => row.rowKey) + } - const byBalance = (await db - .select({ rowKey: bigintTable.rowKey }) - .from(bigintTable) - .where( - and( - drizzleEq(bigintTable.testRunId, RUN), - await ops.gt(bigintTable.balance, 0n), - ), - )) as SelectRow[] - expect(byBalance.map((row) => row.rowKey)).toEqual([ROW_A]) + it('filters a bigint column by encrypted equality, excluding the other row', async () => { + expect( + await bigintRowKeys(await ops.eq(bigintTable.ledger, BIGINT_LEDGER)), + ).toEqual([ROW_A]) + expect( + await bigintRowKeys(await ops.eq(bigintTable.ledger, BIGINT_B_LEDGER)), + ).toEqual([ROW_B]) + // i64::MIN and 100n are both representable; a needle matching neither row + // must return nothing rather than everything. + expect(await bigintRowKeys(await ops.eq(bigintTable.ledger, 7n))).toEqual( + [], + ) + }, 30000) + + it('filters a bigint column by encrypted ordering across zero', async () => { + // ROW_B's balance is -5n, so `> 0n` must reject it. + expect(await bigintRowKeys(await ops.gt(bigintTable.balance, 0n))).toEqual([ + ROW_A, + ]) + expect(await bigintRowKeys(await ops.lt(bigintTable.balance, 0n))).toEqual([ + ROW_B, + ]) + // Range up to i64::MAX inclusive: ROW_A sits exactly on the upper bound. + expect( + await bigintRowKeys( + await ops.between(bigintTable.balance, 0n, BIGINT_BALANCE), + ), + ).toEqual([ROW_A]) }, 30000) }) diff --git a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts index 94487619a..e9b044065 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts @@ -17,9 +17,17 @@ * Gated twice: `describeLivePg` (needs `DATABASE_URL` + CS creds) and an inner * `USER_JWT` guard (soft-skip, matching the existing identity/lock-context * suites). Whether the searchable index terms are themselves identity-bound is - * decided inside `@cipherstash/protect-ffi`, not this repo — so we assert the - * SYMMETRIC behaviour (same lock context on seed + query matches and decrypts), - * not a cross-identity non-match. + * decided inside `@cipherstash/protect-ffi`, not this repo. + * + * We assert the symmetric behaviour (same lock context on seed + query matches + * and decrypts) AND the negative path — an identity-bound row must not match a + * query issued with no lock context, and must not decrypt without it. The + * symmetric tests alone are insufficient: they drop the context identically on + * both sides, so they stay green even if it were ignored entirely. + * + * A cross-identity non-match (sealed under A, queried under B) still needs a + * second JWT with a different `sub` — a lock context only names the claim while + * ZeroKMS resolves its value from the authenticating token. Tracked separately. */ import 'dotenv/config' import { OidcFederationStrategy } from '@cipherstash/auth' @@ -75,6 +83,23 @@ function unwrap(result: { data?: T; failure?: { message: string } }): T { return result.data as T } +/** + * True when a decrypt attempt was DENIED. `decrypt` reports denial as a + * `Result.failure` rather than throwing, so a bare `await` would resolve and a + * naive `expect(...).rejects` would never fire — denial has to be read off the + * result. A throw also counts as denial. + */ +async function decryptDenied( + attempt: () => PromiseLike<{ failure?: { message: string } }>, +): Promise { + try { + const result = await attempt() + return Boolean(result.failure) + } catch { + return true + } +} + /** Run-scoped SELECT of row keys under an already-encrypted SQL condition. */ async function selectRowKeys(condition: SQL): Promise { const rows = (await db @@ -178,4 +203,53 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { }) expect(await selectRowKeys(condition)).toEqual([ROW_B]) }, 30000) + + // NEGATIVE PATH. The three tests above all supply the SAME lock context on + // seed and on query, so they cannot distinguish "lock context applied" from + // "lock context silently ignored": a regression that dropped the context from + // the index term would drop it identically on both sides and still match. + // These assert that an identity-bound row is NOT reachable without its + // context — the property the suite exists to protect. + // + // A true CROSS-identity proof (sealed under A, queried under B) needs a + // second JWT with a different `sub`; the lock context only names the claim + // (`['sub']`) while ZeroKMS resolves its value from the authenticating token. + // No `USER_JWT_B` exists, so that remains a follow-up. + it('an identity-bound row does not match an eq issued with no lock context', async () => { + if (skipUnlessJwt()) return + const condition = await ops.eq(secretTable.secret, SECRET_A) + expect(await selectRowKeys(condition)).toEqual([]) + }, 30000) + + it('an identity-bound row does not decrypt without its lock context', async () => { + if (skipUnlessJwt()) return + const [row] = await sqlClient.unsafe>( + `SELECT secret::jsonb AS value FROM ${TABLE_NAME} + WHERE test_run_id = $1 AND row_key = $2`, + [RUN, ROW_A], + ) + + // `decrypt` reports denial as a `Result.failure` and does not throw, so a + // bare `await` here would silently pass. Require denial via either channel. + expect(await decryptDenied(() => client.decrypt(row.value as never))).toBe( + true, + ) + }, 30000) + + it('an identity-bound row does not decrypt under a different identity claim', async () => { + if (skipUnlessJwt()) return + const [row] = await sqlClient.unsafe>( + `SELECT secret::jsonb AS value FROM ${TABLE_NAME} + WHERE test_run_id = $1 AND row_key = $2`, + [RUN, ROW_A], + ) + + expect( + await decryptDenied(() => + client + .decrypt(row.value as never) + .withLockContext({ identityClaim: ['email'] } as never), + ), + ).toBe(true) + }, 30000) }) diff --git a/packages/stack/__tests__/drizzle-v3/operators.test.ts b/packages/stack/__tests__/drizzle-v3/operators.test.ts index 219ecba06..7452efcb4 100644 --- a/packages/stack/__tests__/drizzle-v3/operators.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators.test.ts @@ -99,6 +99,21 @@ const matrixColumn = (eqlType: string) => const sampleFor = (spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX]) => spec.samples[0] +// `contains` needles must reach the match tokenizer's `token_length` (3), so +// they cannot come from `sampleFor` — `TEXT_S[0]` is the empty string, which +// tokenizes to nothing and is rejected as unanswerable. +const needleFor = ( + spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX], +): string => { + const needle = spec.samples.find( + (sample) => typeof sample === 'string' && sample.length >= 3, + ) + if (typeof needle !== 'string') { + throw new Error('no searchable sample for a match domain') + } + return needle +} + const equalityDomains = matrixEntries.filter( ([, spec]) => spec.indexes.unique || spec.indexes.ore, ) @@ -260,6 +275,33 @@ describe('createEncryptionOperatorsV3 - comparison & range', () => { expect(q.params).toEqual([TERM_JSON, TERM_JSON]) }) + // Every other `between` case passes identical bounds against a constant + // encrypt stub, so the operand never reaches an assertion and a min/max + // transposition inside `range` is invisible. Echo the plaintext through the + // stub instead, and pin that `gte` binds `min` and `lte` binds `max`. + it('between binds min to gte and max to lte, in that order', async () => { + const { ops, render } = setup(async (value) => ({ + data: { p: value } as never, + })) + + const q = render(await ops.between(users.age, -128, 127)) + + expect(q.sql).toBe( + '(eql_v3.gte("users"."age", $1::jsonb) AND eql_v3.lte("users"."age", $2::jsonb))', + ) + expect(q.params).toEqual(['{"p":-128}', '{"p":127}']) + }) + + it('notBetween binds min to gte and max to lte, in that order', async () => { + const { ops, render } = setup(async (value) => ({ + data: { p: value } as never, + })) + + const q = render(await ops.notBetween(users.age, -128, 127)) + + expect(q.params).toEqual(['{"p":-128}', '{"p":127}']) + }) + it('not(between(...)) negates the whole range, not just its lower bound', async () => { const { ops, render } = setup() @@ -309,7 +351,7 @@ describe('createEncryptionOperatorsV3 - free-text match', () => { matchDomains, )('%s contains emits latest eql_v3.contains with a full-envelope operand', async (eqlType, spec) => { const { ops, encrypt, render } = setup() - const q = render(await ops.contains(matrixColumn(eqlType), sampleFor(spec))) + const q = render(await ops.contains(matrixColumn(eqlType), needleFor(spec))) expect(q.sql).toContain( `eql_v3.contains("matrix_users"."${slug(eqlType)}", $1::jsonb)`, @@ -318,6 +360,28 @@ describe('createEncryptionOperatorsV3 - free-text match', () => { expect(encrypt.mock.calls[0]?.[1]?.column.getName()).toBe(slug(eqlType)) }) + // A needle shorter than the tokenizer's `token_length` produces an empty + // bloom filter, and `stored_bf @> '{}'` is true for every row — so this must + // throw rather than silently return the whole table. + it.each( + matchDomains, + )('%s contains rejects a needle shorter than token_length before encrypting', async (eqlType) => { + const { ops, encrypt } = setup() + await expect(ops.contains(matrixColumn(eqlType), 'ad')).rejects.toThrow( + /at least 3 characters/, + ) + await expect(ops.contains(matrixColumn(eqlType), '')).rejects.toThrow( + EncryptionOperatorError, + ) + expect(encrypt).not.toHaveBeenCalled() + }) + + it('contains accepts a needle exactly at token_length', async () => { + const { ops, render } = setup() + const q = render(await ops.contains(users.email, 'ada')) + expect(q.sql).toContain('eql_v3.contains("users"."email", $1::jsonb)') + }) + it('negation is expressed through the passthrough Drizzle not operator', async () => { const { ops, render } = setup() const q = render(ops.not(await ops.contains(users.email, 'example.com'))) diff --git a/packages/stack/__tests__/drizzle-v3/schema-extraction.test.ts b/packages/stack/__tests__/drizzle-v3/schema-extraction.test.ts index 2b7927593..1d1e731d2 100644 --- a/packages/stack/__tests__/drizzle-v3/schema-extraction.test.ts +++ b/packages/stack/__tests__/drizzle-v3/schema-extraction.test.ts @@ -24,7 +24,7 @@ describe('extractEncryptionSchemaV3', () => { ) const authored = encryptedTable('users', authoredColumns as never) - expect(extracted.build()).toEqual(authored.build()) + expect(extracted.build()).toStrictEqual(authored.build()) }) it('rebuilds an equivalent eql/v3 encryptedTable from a drizzle table', () => { @@ -40,7 +40,7 @@ describe('extractEncryptionSchemaV3', () => { age: v3Types.IntegerOrd('age'), }) - expect(extracted.build()).toEqual(authored.build()) + expect(extracted.build()).toStrictEqual(authored.build()) }) it('keeps same-named columns on different tables bound to their own v3 domains', () => { @@ -54,12 +54,12 @@ describe('extractEncryptionSchemaV3', () => { const accountsSchema = extractEncryptionSchemaV3(accounts) const metricsSchema = extractEncryptionSchemaV3(metrics) - expect(accountsSchema.build()).toEqual( + expect(accountsSchema.build()).toStrictEqual( encryptedTable('accounts', { email: v3Types.TextEq('email'), }).build(), ) - expect(metricsSchema.build()).toEqual( + expect(metricsSchema.build()).toStrictEqual( encryptedTable('metrics', { email: v3Types.IntegerOrd('email'), }).build(), @@ -72,7 +72,7 @@ describe('extractEncryptionSchemaV3', () => { emailAddress: types.TextEq('email_address'), }) - expect(extractEncryptionSchemaV3(users).build()).toEqual( + expect(extractEncryptionSchemaV3(users).build()).toStrictEqual( encryptedTable('users', { createdOn: v3Types.Date('created_on'), emailAddress: v3Types.TextEq('email_address'), @@ -89,7 +89,7 @@ describe('extractEncryptionSchemaV3', () => { }, } - expect(extractEncryptionSchemaV3(table as never).build()).toEqual( + expect(extractEncryptionSchemaV3(table as never).build()).toStrictEqual( encryptedTable('users', { createdOn: v3Types.Date('created_on'), }).build(), diff --git a/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts b/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts index ba139daf0..caef1c3f9 100644 --- a/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts +++ b/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts @@ -5,6 +5,7 @@ import { EQL_V3_FN_SCHEMA, v3Dialect } from '@/eql/v3/drizzle/sql-dialect' const dialect = new PgDialect() const render = (s: ReturnType) => dialect.sqlToQuery(s).sql +const renderFull = (s: ReturnType) => dialect.sqlToQuery(s) const col = sql`"users"."x"` const enc = sql`${'{"v":"t"}'}` @@ -57,6 +58,47 @@ describe('v3Dialect', () => { expect(render(v3Dialect.orderBy(col))).toBe('eql_v3.ord_term("users"."x")') }) + // `render` above discards `.params`, so nothing here proved a value was BOUND + // rather than concatenated into the SQL text. The only `sql.raw` in the + // dialect interpolates constants (the schema + function name), so no + // injectable path exists today — these pin that. + describe('operand values are bound, never interpolated into SQL text', () => { + const hostile = '{"v":"\\" OR 1=1 --","x":"$1","y":"back\\\\slash"}' + + it.each([ + ['equality', () => v3Dialect.equality('eq', col, sql`${hostile}`)], + ['comparison', () => v3Dialect.comparison('gte', col, sql`${hostile}`)], + ['contains', () => v3Dialect.contains(col, sql`${hostile}`)], + ])('%s binds a hostile operand as $1', (_name, build) => { + const query = renderFull(build()) + + expect(query.params).toEqual([hostile]) + expect(query.sql).toContain('$1::jsonb') + // The raw value must appear nowhere in the SQL text. + expect(query.sql).not.toContain('OR 1=1') + expect(query.sql).not.toContain('back\\slash') + }) + + it('range binds both bounds positionally, min first', () => { + const query = renderFull( + v3Dialect.range(col, sql`${'{"b":"min"}'}`, sql`${'{"b":"max"}'}`), + ) + + expect(query.params).toEqual(['{"b":"min"}', '{"b":"max"}']) + expect(query.sql).toBe( + '(eql_v3.gte("users"."x", $1::jsonb) AND eql_v3.lte("users"."x", $2::jsonb))', + ) + }) + + it('binds a large ciphertext without truncating or inlining it', () => { + const big = `{"c":"${'z'.repeat(16384)}"}` + const query = renderFull(v3Dialect.equality('eq', col, sql`${big}`)) + + expect(query.params).toEqual([big]) + expect(query.sql).toBe('eql_v3.eq("users"."x", $1::jsonb)') + }) + }) + it('every helper schema-qualifies its function call', () => { const lo = sql`${'{"v":"lo"}'}` const hi = sql`${'{"v":"hi"}'}` diff --git a/packages/stack/__tests__/lock-context.test.ts b/packages/stack/__tests__/lock-context.test.ts index ff2136297..f27f55832 100644 --- a/packages/stack/__tests__/lock-context.test.ts +++ b/packages/stack/__tests__/lock-context.test.ts @@ -123,12 +123,26 @@ describe('identity-bound encryption via OidcFederationStrategy + lock context', } // Decrypting without the identity claim cannot reproduce the key tag. + // + // `decryptModel` REPORTS failure as a `Result` rather than throwing, so the + // previous `try/catch` here ran zero assertions on the happy path and would + // also have passed had the decryption wrongly SUCCEEDED. Accept denial via + // either channel, but require that it is denied. + let denied = false + let message = '' try { - await protectClient.decryptModel(encryptedModel.data) + const result = await protectClient.decryptModel(encryptedModel.data) + if (result.failure) { + denied = true + message = result.failure.message + } } catch (error) { - const e = error as Error - expect(e.message.startsWith('Failed to retrieve key')).toEqual(true) + denied = true + message = (error as Error).message } + + expect(denied).toBe(true) + expect(message).toMatch(/^Failed to retrieve key/) }, 30000) it('should bulk encrypt and decrypt models bound to the user identity', async () => { diff --git a/packages/stack/src/eql/v3/drizzle/codec.ts b/packages/stack/src/eql/v3/drizzle/codec.ts index 43973ef03..f53ed7e71 100644 --- a/packages/stack/src/eql/v3/drizzle/codec.ts +++ b/packages/stack/src/eql/v3/drizzle/codec.ts @@ -7,6 +7,42 @@ import type { Encrypted } from '@/types' +/** Thrown when a driver value cannot be read back as an EQL v3 envelope. */ +export class EqlV3CodecError extends Error { + constructor(message: string) { + super(message) + this.name = 'EqlV3CodecError' + } +} + +/** + * A stored EQL envelope always carries a schema version (`v`) and a ciphertext: + * at `c` on scalar payloads, or at `sv[0].c` on a SteVec document (`k: "sv"`), + * which has no top-level `c`. Checking `v` plus either carrier distinguishes an + * envelope from a bare scalar, an array, or an unrelated object — without + * paying a full structural validation on every decrypted row. + */ +function assertEnvelope(value: unknown, source: string): Encrypted { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new EqlV3CodecError( + `Expected an EQL encrypted envelope from ${source}, got ${Array.isArray(value) ? 'an array' : typeof value}. The column may not hold EQL data.`, + ) + } + const envelope = value as { v?: unknown; c?: unknown; sv?: unknown } + const missing = + envelope.v === undefined + ? '"v"' + : envelope.c === undefined && envelope.sv === undefined + ? 'a ciphertext ("c", or "sv" for a SteVec document)' + : undefined + if (missing) { + throw new EqlV3CodecError( + `Expected an EQL encrypted envelope from ${source}, but it is missing ${missing}. The column may not hold EQL data.`, + ) + } + return value as Encrypted +} + /** * `JSON.stringify` replacer that renders any stray `bigint` as its decimal * string instead of throwing `TypeError: Do not know how to serialize a @@ -30,7 +66,11 @@ export function v3ToDriver(value: Encrypted | null | undefined): string | null { /** Parse a driver jsonb value back into an encrypted envelope. `postgres` * hands back an already-parsed object for jsonb; a string is parsed. Null and - * undefined normalise to `null` (SQL NULL). */ + * undefined normalise to `null` (SQL NULL). + * + * Malformed and non-envelope payloads throw {@link EqlV3CodecError} rather than + * surfacing a raw `SyntaxError` or passing a bare scalar through as though it + * were an envelope — a wrong value here reaches `decrypt` as garbage. */ export function v3FromDriver( value: string | object | null | undefined, ): Encrypted | null { @@ -38,7 +78,15 @@ export function v3FromDriver( return null } if (typeof value === 'object') { - return value as Encrypted + return assertEnvelope(value, 'the driver') + } + let parsed: unknown + try { + parsed = JSON.parse(value) + } catch (cause) { + throw new EqlV3CodecError( + `Failed to parse an EQL v3 encrypted envelope from the driver: ${cause instanceof Error ? cause.message : String(cause)}`, + ) } - return JSON.parse(value) as Encrypted + return assertEnvelope(parsed, 'the driver') } diff --git a/packages/stack/src/eql/v3/drizzle/index.ts b/packages/stack/src/eql/v3/drizzle/index.ts index 0eaf98302..7c0426d4c 100644 --- a/packages/stack/src/eql/v3/drizzle/index.ts +++ b/packages/stack/src/eql/v3/drizzle/index.ts @@ -1,4 +1,4 @@ -export { v3FromDriver, v3ToDriver } from './codec.js' +export { EqlV3CodecError, v3FromDriver, v3ToDriver } from './codec.js' export { getEqlV3Column, isEqlV3Column, makeEqlV3Column } from './column.js' export { createEncryptionOperatorsV3, diff --git a/packages/stack/src/eql/v3/drizzle/operators.ts b/packages/stack/src/eql/v3/drizzle/operators.ts index e5f927997..366ae4e5a 100644 --- a/packages/stack/src/eql/v3/drizzle/operators.ts +++ b/packages/stack/src/eql/v3/drizzle/operators.ts @@ -19,6 +19,7 @@ import type { AuditConfig } from '@/encryption/operations/base-operation' import type { AnyEncryptedV3Column, AnyV3Table } from '@/eql/v3' import type { LockContext } from '@/identity' import type { ColumnSchema } from '@/schema' +import { matchNeedleError } from '@/schema/match-defaults' import { getEqlV3Column } from './column.js' import { extractEncryptionSchemaV3, @@ -248,6 +249,28 @@ export function createEncryptionOperatorsV3( } } + /** + * Reject a free-text needle the column's match index cannot answer. A needle + * shorter than the tokenizer's `token_length` yields an empty bloom filter, + * and `stored_bf @> '{}'` holds for every row — so without this the query + * silently returns the whole table. + */ + function requireAnswerableNeedle( + ctx: ColumnContext, + value: unknown, + operator: string, + ): void { + const match = ctx.indexes.match + if (!match) return + const reason = matchNeedleError(value, match) + if (reason) { + throw new EncryptionOperatorError( + `Operator "${operator}" cannot search column "${ctx.columnName}": ${reason}`, + { columnName: ctx.columnName, tableName: ctx.tableName, operator }, + ) + } + } + function operandFailure( ctx: ColumnContext, operator: string, @@ -383,6 +406,7 @@ export function createEncryptionOperatorsV3( ): Promise { const ctx = resolveContext(left, operator) requireIndex(ctx, MATCH_INDEXES, operator, 'free-text search') + requireAnswerableNeedle(ctx, right, operator) const enc = await encryptOperand(ctx, right, operator, opts) return v3Dialect.contains(colSql(left), enc) } @@ -409,8 +433,9 @@ export function createEncryptionOperatorsV3( const conditions = encrypted.map((enc) => v3Dialect.equality(op, colSql(left), enc), ) - const combined = negate ? and(...conditions) : or(...conditions) - return combined ?? (negate ? sql`true` : sql`false`) + // The empty-list guard above leaves `conditions` non-empty, so `and`/`or` + // never return undefined here. + return (negate ? and(...conditions) : or(...conditions)) as SQL } function orderTerm(column: SQLWrapper, operator: string): SQL { diff --git a/packages/stack/src/schema/match-defaults.ts b/packages/stack/src/schema/match-defaults.ts index e7518590e..c484160eb 100644 --- a/packages/stack/src/schema/match-defaults.ts +++ b/packages/stack/src/schema/match-defaults.ts @@ -74,3 +74,42 @@ export function resolveMatchOpts(opts?: MatchIndexOpts): BuiltMatchIndexOpts { include_original: opts?.include_original ?? defaults.include_original, }) } + +/** + * The shortest needle a match index can answer, or `undefined` when its + * tokenizer imposes no floor (`standard` splits on word boundaries, so any + * non-empty needle yields at least one token). + * + * Accepts the loose {@link MatchIndexOpts} because a `ColumnSchema`'s built + * `indexes.match` is typed from the zod schema, where `tokenizer` is optional. + * An absent tokenizer resolves to the same default the schema itself applies + * (`ngram`, `token_length: 3`) rather than skipping the floor — skipping would + * reintroduce the fail-open this guard exists to close. + */ +export function matchNeedleMinLength(opts: MatchIndexOpts): number | undefined { + const tokenizer = opts.tokenizer ?? defaultMatchOpts().tokenizer + return tokenizer.kind === 'ngram' ? tokenizer.token_length : undefined +} + +/** + * Why a needle cannot be answered by this match index, or `undefined` when it + * can. Callers throw their own error type with this as the reason. + * + * A needle shorter than the ngram tokenizer's `token_length` produces NO + * ngrams, so its bloom filter is empty — and `stored_bf @> '{}'` is true for + * every row ("contains nothing, contained by everything"). Such a query is + * unanswerable rather than merely unmatched, so it must fail loudly instead of + * silently returning the whole table. + * + * Shared by the v2 and v3 adapters: both build byte-identical bloom filters, so + * the floor is a property of the index config, not of the adapter version. + */ +export function matchNeedleError( + needle: unknown, + opts: MatchIndexOpts, +): string | undefined { + if (typeof needle !== 'string') return undefined + const min = matchNeedleMinLength(opts) + if (min === undefined || needle.length >= min) return undefined + return `free-text search needs at least ${min} characters (the index tokenizer's token_length), but the search term ${JSON.stringify(needle)} has ${needle.length}. A shorter term produces no tokens and would match every row.` +} From 67d840f301dab562f67f0b6abc7e697b4943a160 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 21:07:40 +1000 Subject: [PATCH 2/6] fix(stack): count codepoints in the free-text needle floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #607 found the needle guard it added was itself incomplete, and two claims it made were false. Three agents verified each finding against live Postgres before anything changed here. The guard counted UTF-16 code units; the ngram tokenizer counts CODEPOINTS. A needle of 1-2 astral-plane characters cleared the floor, produced no trigram, and so built an EMPTY bloom filter -- `stored_bf @> '{}'` matches every row. Measured live against three seeded rows: "👍👍" utf16=4 cp=2 -> 3/3 rows <- passed the guard "👍👍👍" utf16=6 cp=3 -> 0/3 rows "joh" utf16=3 cp=3 -> 1/3 rows Codepoints, not graphemes: NFD "e+combining-acute" twice is 4 codepoints but 2 grapheme clusters, and does build a non-empty filter (0/3 rows live). A grapheme floor would wrongly reject it; a byte floor would wrongly accept the emoji. The three measurements are consistent only with codepoint counting. Also rejects the empty needle under ANY tokenizer. `matchNeedleMinLength` returns undefined for the `standard` tokenizer, so the guard short-circuited and `contains(col, '')` tokenized to nothing and matched every row. Unreachable via the v3 `types` namespace today, which hardcodes ngram, but the schema permits it. Tests, written first and watched fail: - `__tests__/match-needle-guard.test.ts` pins the unit. The astral and empty-needle cases failed before the fix; the NFD case passed before and after, proving the fix does not over-correct into rejecting usable needles. Unicode is built from explicit escapes -- the precomposed NFC form is a different, 2-codepoint string, so a bare literal would silently test the opposite case depending on the file's on-disk normalisation. - The live `contains` rejection now asserts the guard's own message, not just `EncryptionOperatorError`. `operandFailure` throws the same class, and that is not hypothetical: `text_search` carries an `ore` index and cannot encrypt any non-ASCII operand ("Can only order strings that are pure ASCII"), so the astral case was passing there for the wrong reason. That constraint now has its own test, and the codepoint complement runs only on match-only columns. Two false claims corrected rather than papered over: - The changeset and the doc comment both said the floor was "shared with the v2 adapter". It is not -- `matchNeedleError` has exactly one caller, in v3. v2's `like`/`ilike` reduces to `eql_v2.bloom_filter(a) @> eql_v2.bloom_filter(b)` (cipherstash-encrypt.sql), native array containment, so v2 has the same fail-open and is NOT fixed here. Wiring it in needs its own measurement first: v2 needles carry SQL wildcards, so the floor may belong on the unwrapped term. - `decryptDenied` returned a bare boolean and treated any throw as denial, so a ZeroKMS outage read as "the identity boundary held". It now returns the message and the call sites assert /^Failed to retrieve key/, matching the assertion this PR already made in `lock-context.test.ts`. The `as SQL` cast in `inArrayOp` was reviewed and kept: drizzle returns undefined only for an empty filtered list, and the empty-list guard throws first. Restoring the old `?? sql`false`` fallback would be worse -- it would mask a `notInArray` that silently matches everything. --- .changeset/eql-v3-drizzle-fail-open-guards.md | 27 ++++-- .../drizzle-v3/operators-live-pg.test.ts | 72 +++++++++++++- .../operators-lock-context-live-pg.test.ts | 59 ++++++++---- .../__tests__/match-needle-guard.test.ts | 93 +++++++++++++++++++ packages/stack/src/schema/match-defaults.ts | 40 ++++++-- 5 files changed, 250 insertions(+), 41 deletions(-) create mode 100644 packages/stack/__tests__/match-needle-guard.test.ts diff --git a/.changeset/eql-v3-drizzle-fail-open-guards.md b/.changeset/eql-v3-drizzle-fail-open-guards.md index 7774e7e3b..1c94097fa 100644 --- a/.changeset/eql-v3-drizzle-fail-open-guards.md +++ b/.changeset/eql-v3-drizzle-fail-open-guards.md @@ -4,14 +4,20 @@ Close two fail-open paths in the EQL v3 Drizzle adapter. -`ops.contains()` now throws `EncryptionOperatorError` when the search term is -shorter than the match index tokenizer's `token_length` (3 by default). Such a -term produces no ngrams, so its bloom filter is empty — and `stored_bf @> '{}'` -is true for every row. Previously a user searching `"ad"` silently received the -entire table; measured live, the needles `"ad"`, `"a"` and `"x"` each returned -every seeded row, including one in which `"x"` did not appear. The length floor -is shared with the v2 adapter via `matchNeedleError` in `schema/match-defaults`, -since both build byte-identical bloom filters. +`ops.contains()` now throws `EncryptionOperatorError` for a search term that +tokenizes to nothing: the empty string, or a term shorter than the match index +tokenizer's `token_length` (3 by default). Such a term produces an empty bloom +filter, and `stored_bf @> '{}'` is true for every row — so a user searching +`"ad"` silently received the entire table. Measured live, the terms `"ad"`, +`"a"` and `"x"` each returned every seeded row, including one in which `"x"` +did not appear. + +The floor counts Unicode codepoints, matching the tokenizer. A UTF-16 length +check would wave through an astral-plane term — `"👍👍"` is 4 code units but +only 2 codepoints, yields no trigram, and matched every row. + +**Breaking for callers passing short terms:** `contains()` calls that previously +returned every row now throw. Terms of 3+ codepoints are unaffected. `v3FromDriver()` now throws the new `EqlV3CodecError` on a payload that is not an EQL envelope, instead of surfacing a raw `SyntaxError` for malformed JSON and @@ -23,3 +29,8 @@ accepts both scalar envelopes (ciphertext at `c`) and SteVec documents Also removes an unreachable branch in `inArray`/`notInArray`, whose empty-list guard already throws before it. + +Note: the v2 Drizzle adapter's `like`/`ilike` path builds the same bloom filters +and has the same short-term fail-open. It is **not** fixed here — v2 terms carry +SQL wildcards, so the floor must be measured against what its tokenizer actually +receives before the shared guard can be reused. Tracked separately. diff --git a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts index 0972af1c8..41bfa2bfb 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts @@ -549,12 +549,74 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { // A needle below the tokenizer's `token_length` builds an EMPTY bloom filter, // and `stored_bf @> '{}'` holds for every row — so before the SDK guard this // silently returned the whole table. + // + // `'👍👍'` is the regression case: 4 UTF-16 code units but only 2 codepoints, + // so a `needle.length` floor waved it through and it matched every row. The + // tokenizer counts codepoints. `''` tokenizes to nothing under any tokenizer. it.each( - matchDomains, - )('%s contains rejects a needle shorter than token_length', async (eqlType) => { - await expect( - ops.contains(matrixColumn(eqlType), 'ad'), - ).rejects.toBeInstanceOf(EncryptionOperatorError) + matchDomains.flatMap(([eqlType]) => + ['ad', '👍👍', ''].map((needle) => [eqlType, needle] as const), + ), + )( + '%s contains rejects the unanswerable needle %j before encrypting', + async (eqlType, needle) => { + const attempt = ops.contains(matrixColumn(eqlType), needle) + await expect(attempt).rejects.toBeInstanceOf(EncryptionOperatorError) + // Assert the GUARD rejected it, not the encryption layer. + // `operandFailure` also throws `EncryptionOperatorError`, so matching only + // the class would let an encryption error stand in for the guard and the + // test would pass for the wrong reason. + await expect(attempt).rejects.toThrow(/free-text search needs/) + }, + 30000, + ) + + // The complement: a needle that DOES reach the floor in codepoints must be + // ANSWERED, not over-rejected — otherwise the fix for the astral fail-open + // would just over-correct into rejecting usable needles. + // + // Restricted to match-ONLY columns: a column that also carries an `ore` index + // (`text_search`) cannot encrypt a non-ASCII operand at all — the ORE term + // raises "Can only order strings that are pure ASCII" — so a non-ASCII needle + // there fails inside encryption, long after the guard has passed it. That + // constraint is pinned by its own test below rather than silently conflated + // with the codepoint floor. + // + // Escapes, not literals: the precomposed NFC form of `ee-with-accents` is only + // 2 codepoints and IS rejected, so a bare literal would test the opposite case + // depending on the file's on-disk normalisation. + const NFD_EE = 'e\u0301e\u0301' // 4 codepoints, 2 grapheme clusters + const matchOnlyDomains = matchDomains.filter(([, spec]) => !spec.indexes.ore) + + it.each( + matchOnlyDomains.flatMap(([eqlType]) => + ['\u{1F44D}\u{1F44D}\u{1F44D}', NFD_EE].map( + (needle) => [eqlType, needle] as const, + ), + ), + )( + '%s contains answers the codepoint-sufficient needle %j with no match', + async (eqlType, needle) => { + const rows = await selectRowKeys( + await ops.contains(matrixColumn(eqlType), needle), + ) + expect(rows).toEqual([]) + }, + 30000, + ) + + // An `ore`-bearing match column rejects a non-ASCII needle inside ENCRYPTION, + // not in the needle guard. Pinned so the distinction stays visible: the guard + // is about tokenization, this is about the ORE term's ASCII-only ordering. + it.each( + matchDomains.filter(([, spec]) => spec.indexes.ore), + )('%s contains rejects a non-ASCII needle in the ORE term, not the guard', async (eqlType) => { + const attempt = ops.contains( + matrixColumn(eqlType), + '\u{1F44D}\u{1F44D}\u{1F44D}', + ) + await expect(attempt).rejects.toThrow(/pure ASCII/) + await expect(attempt).rejects.not.toThrow(/free-text search needs/) }, 30000) it.each( diff --git a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts index e9b044065..d5900d07e 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts @@ -84,22 +84,38 @@ function unwrap(result: { data?: T; failure?: { message: string } }): T { } /** - * True when a decrypt attempt was DENIED. `decrypt` reports denial as a - * `Result.failure` rather than throwing, so a bare `await` would resolve and a - * naive `expect(...).rejects` would never fire — denial has to be read off the - * result. A throw also counts as denial. + * The outcome of a decrypt attempt that is EXPECTED to be denied. `decrypt` + * reports denial as a `Result.failure` rather than throwing, so a bare `await` + * would resolve and a naive `expect(...).rejects` would never fire — denial has + * to be read off the result. A throw also counts as denial. + * + * Returns the message, not just a boolean, so callers can assert WHY it was + * denied. A bare boolean would let any infrastructure fault — a DNS failure, an + * expired CTS token, a ZeroKMS outage ("SendRequest: Failed to send request") — + * read as "the identity boundary held", and the security test would pass for + * the wrong reason. */ -async function decryptDenied( +async function decryptOutcome( attempt: () => PromiseLike<{ failure?: { message: string } }>, -): Promise { +): Promise<{ denied: boolean; message: string }> { try { const result = await attempt() - return Boolean(result.failure) - } catch { - return true + return { + denied: Boolean(result.failure), + message: result.failure?.message ?? '', + } + } catch (error) { + return { denied: true, message: (error as Error).message } } } +/** + * A genuine key-derivation denial. ZeroKMS cannot reproduce the key tag without + * the identity claim the row was sealed under, and says so. Asserted verbatim + * elsewhere in the suite (`lock-context.test.ts`, `protect-ops.test.ts`). + */ +const KEY_DENIAL = /^Failed to retrieve key/ + /** Run-scoped SELECT of row keys under an already-encrypted SQL condition. */ async function selectRowKeys(condition: SQL): Promise { const rows = (await db @@ -230,10 +246,14 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { ) // `decrypt` reports denial as a `Result.failure` and does not throw, so a - // bare `await` here would silently pass. Require denial via either channel. - expect(await decryptDenied(() => client.decrypt(row.value as never))).toBe( - true, + // bare `await` here would silently pass. Require denial via either channel, + // AND require it be a key-derivation denial rather than an outage. + const { denied, message } = await decryptOutcome(() => + client.decrypt(row.value as never), ) + + expect(denied).toBe(true) + expect(message).toMatch(KEY_DENIAL) }, 30000) it('an identity-bound row does not decrypt under a different identity claim', async () => { @@ -244,12 +264,13 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { [RUN, ROW_A], ) - expect( - await decryptDenied(() => - client - .decrypt(row.value as never) - .withLockContext({ identityClaim: ['email'] } as never), - ), - ).toBe(true) + const { denied, message } = await decryptOutcome(() => + client + .decrypt(row.value as never) + .withLockContext({ identityClaim: ['email'] } as never), + ) + + expect(denied).toBe(true) + expect(message).toMatch(KEY_DENIAL) }, 30000) }) diff --git a/packages/stack/__tests__/match-needle-guard.test.ts b/packages/stack/__tests__/match-needle-guard.test.ts new file mode 100644 index 000000000..d5ca8048c --- /dev/null +++ b/packages/stack/__tests__/match-needle-guard.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest' +import type { MatchIndexOpts } from '@/schema' +import { matchNeedleError, matchNeedleMinLength } from '@/schema/match-defaults' + +// The floor a free-text needle must clear before it yields any ngram. A needle +// below it tokenizes to nothing, so its bloom filter is EMPTY — and +// `stored_bf @> '{}'` is true for every row. Such a query is unanswerable, not +// merely unmatched, so the guard must reject it rather than silently return the +// whole table. +const ngram: MatchIndexOpts = { tokenizer: { kind: 'ngram', token_length: 3 } } + +describe('matchNeedleMinLength', () => { + it('is the ngram tokenizer token_length', () => { + expect(matchNeedleMinLength(ngram)).toBe(3) + expect( + matchNeedleMinLength({ tokenizer: { kind: 'ngram', token_length: 4 } }), + ).toBe(4) + }) + + it('defaults an absent tokenizer to the schema default rather than skipping the floor', () => { + expect(matchNeedleMinLength({})).toBe(3) + }) +}) + +describe('matchNeedleError', () => { + it('accepts a needle at or above the floor', () => { + expect(matchNeedleError('joh', ngram)).toBeUndefined() + expect(matchNeedleError('lovelace', ngram)).toBeUndefined() + }) + + it('rejects a needle below the floor, naming the floor and the term', () => { + expect(matchNeedleError('ad', ngram)).toMatch(/at least 3 characters/) + expect(matchNeedleError('ad', ngram)).toMatch(/"ad"/) + }) + + // The tokenizer counts Unicode CODEPOINTS. `'👍👍'` is 2 codepoints but 4 + // UTF-16 code units, so a `needle.length` check waves it through — and it + // then matches every row. Measured live: 3/3 rows returned. + it('rejects an astral-plane needle below the floor in CODEPOINTS, not UTF-16 units', () => { + expect('👍👍'.length).toBe(4) // pins why a naive length check passes it + expect([...'👍👍'].length).toBe(2) + + expect(matchNeedleError('👍👍', ngram)).toMatch(/at least 3 characters/) + }) + + it('reports the codepoint count, not the UTF-16 length, in the message', () => { + expect(matchNeedleError('👍👍', ngram)).toMatch(/has 2\b/) + }) + + it('accepts an astral-plane needle that reaches the floor in codepoints', () => { + expect(matchNeedleError('👍👍👍', ngram)).toBeUndefined() + }) + + // Combining acute accents. NFD 'e\u0301e\u0301' is 4 codepoints but only 2 + // grapheme clusters, and (measured live) builds a NON-empty filter — so the + // unit is codepoints, not graphemes. A grapheme floor would wrongly reject it. + // + // Built from explicit escapes: the PRECOMPOSED NFC form is a different string + // of only 2 codepoints, which the guard correctly rejects. A bare literal here + // would test whichever form the file happened to be normalised to on disk. + const NFD_EE = 'e\u0301e\u0301' + const NFC_EE = '\u00e9\u00e9' + + it('accepts a combining-accent needle that is 4 codepoints but only 2 graphemes', () => { + expect([...NFD_EE].length).toBe(4) + expect(matchNeedleError(NFD_EE, ngram)).toBeUndefined() + }) + + it('rejects the precomposed NFC form, which really is 2 codepoints', () => { + expect([...NFC_EE].length).toBe(2) + expect(matchNeedleError(NFC_EE, ngram)).toMatch(/at least 3 characters/) + }) + + it('rejects the empty needle whatever the tokenizer', () => { + expect(matchNeedleError('', ngram)).toBeTypeOf('string') + // A `standard` tokenizer imposes no ngram floor, but the empty string still + // yields zero tokens and so still matches every row. + expect( + matchNeedleError('', { tokenizer: { kind: 'standard' } }), + ).toBeTypeOf('string') + }) + + it('imposes no floor on a non-empty needle under a standard tokenizer', () => { + expect( + matchNeedleError('a', { tokenizer: { kind: 'standard' } }), + ).toBeUndefined() + }) + + it('ignores non-string operands, leaving them to the encryption layer', () => { + expect(matchNeedleError(42, ngram)).toBeUndefined() + expect(matchNeedleError(null, ngram)).toBeUndefined() + }) +}) diff --git a/packages/stack/src/schema/match-defaults.ts b/packages/stack/src/schema/match-defaults.ts index c484160eb..f2181c79b 100644 --- a/packages/stack/src/schema/match-defaults.ts +++ b/packages/stack/src/schema/match-defaults.ts @@ -95,21 +95,43 @@ export function matchNeedleMinLength(opts: MatchIndexOpts): number | undefined { * Why a needle cannot be answered by this match index, or `undefined` when it * can. Callers throw their own error type with this as the reason. * - * A needle shorter than the ngram tokenizer's `token_length` produces NO - * ngrams, so its bloom filter is empty — and `stored_bf @> '{}'` is true for - * every row ("contains nothing, contained by everything"). Such a query is - * unanswerable rather than merely unmatched, so it must fail loudly instead of - * silently returning the whole table. + * A needle that tokenizes to NOTHING has an empty bloom filter, and + * `stored_bf @> '{}'` is true for every row ("contains nothing, contained by + * everything"). Such a query is unanswerable rather than merely unmatched, so + * it must fail loudly instead of silently returning the whole table. Two ways + * to tokenize to nothing: * - * Shared by the v2 and v3 adapters: both build byte-identical bloom filters, so - * the floor is a property of the index config, not of the adapter version. + * - the empty needle, under ANY tokenizer; + * - a needle shorter than the ngram tokenizer's `token_length`. + * + * The ngram floor counts Unicode CODEPOINTS, because that is what the tokenizer + * counts. `needle.length` would count UTF-16 code units and wave through an + * astral-plane needle: `'👍👍'` is 4 code units but only 2 codepoints, yields no + * trigram, and (measured live) matched every row. Graphemes are the wrong unit + * in the other direction — NFD `'éé'` is 4 codepoints but 2 grapheme clusters, + * and does yield trigrams. + * + * Currently wired ONLY into the v3 Drizzle adapter (`eql/v3/drizzle/operators`). + * It lives here, beside the shared match defaults, because v2 builds the same + * bloom filters and needs the same floor — but v2's `like`/`ilike` path remains + * unguarded and still fails open. Do not reuse this for v2 without first + * measuring what its tokenizer actually receives: v2 needles carry SQL wildcards + * (`'%ada%'`), so the floor may have to apply to the unwrapped term rather than + * to the string the caller passed. */ export function matchNeedleError( needle: unknown, opts: MatchIndexOpts, ): string | undefined { if (typeof needle !== 'string') return undefined + + // Codepoints, not UTF-16 code units — see above. + const length = [...needle].length const min = matchNeedleMinLength(opts) - if (min === undefined || needle.length >= min) return undefined - return `free-text search needs at least ${min} characters (the index tokenizer's token_length), but the search term ${JSON.stringify(needle)} has ${needle.length}. A shorter term produces no tokens and would match every row.` + + if (length === 0) { + return `free-text search needs a non-empty search term; an empty term produces no tokens and would match every row.` + } + if (min === undefined || length >= min) return undefined + return `free-text search needs at least ${min} characters (the index tokenizer's token_length), but the search term ${JSON.stringify(needle)} has ${length}. A shorter term produces no tokens and would match every row.` } From 7a760414ddb421b16b4afa505d5f0d263fafabd1 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 22:00:58 +1000 Subject: [PATCH 3/6] fix(stack): preserve parse error cause in v3 drizzle codec v3FromDriver flattened the JSON.parse failure into a message string, discarding the SyntaxError and its stack. Widen EqlV3CodecError to accept ErrorOptions and forward { cause }, so a caller debugging a corrupt column keeps the original error. The package targets ES2022, so Error.cause is available; assertEnvelope's call sites have no upstream error and keep working since options is optional. Also fix a misleading it.each title in sql-dialect.test.ts: vitest consumed the `$1` as a tuple index and interpolated the builder function, rendering "binds a hostile operand as [Function]". Reword to name positional binding directly. --- .../stack/__tests__/drizzle-v3/codec.test.ts | 19 +++++++++++++++++++ .../__tests__/drizzle-v3/sql-dialect.test.ts | 4 +++- packages/stack/src/eql/v3/drizzle/codec.ts | 5 +++-- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/stack/__tests__/drizzle-v3/codec.test.ts b/packages/stack/__tests__/drizzle-v3/codec.test.ts index 3deb58d50..93093c27a 100644 --- a/packages/stack/__tests__/drizzle-v3/codec.test.ts +++ b/packages/stack/__tests__/drizzle-v3/codec.test.ts @@ -81,6 +81,25 @@ describe('v3 codec', () => { expect(() => v3FromDriver(raw)).toThrow(/Failed to parse/) }) + // The parse failure is the only codec path with an upstream error to + // preserve. Flattening it to a message string discards the SyntaxError and + // its stack, so a caller debugging a corrupt column loses where parsing + // actually broke. + it.each(['{bad', '', '{"v":3,'])( + 'preserves the underlying SyntaxError as `cause` for %j', + (raw) => { + let thrown: unknown + try { + v3FromDriver(raw) + } catch (error) { + thrown = error + } + + expect(thrown).toBeInstanceOf(EqlV3CodecError) + expect((thrown as EqlV3CodecError).cause).toBeInstanceOf(SyntaxError) + }, + ) + it.each([ '5', '"a string"', diff --git a/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts b/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts index caef1c3f9..142b79e58 100644 --- a/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts +++ b/packages/stack/__tests__/drizzle-v3/sql-dialect.test.ts @@ -69,7 +69,9 @@ describe('v3Dialect', () => { ['equality', () => v3Dialect.equality('eq', col, sql`${hostile}`)], ['comparison', () => v3Dialect.comparison('gte', col, sql`${hostile}`)], ['contains', () => v3Dialect.contains(col, sql`${hostile}`)], - ])('%s binds a hostile operand as $1', (_name, build) => { + // `$1` in a title is consumed by vitest as an index into the case tuple, + // so it cannot be used to name the SQL placeholder here. + ])('%s binds a hostile operand as a positional parameter', (_name, build) => { const query = renderFull(build()) expect(query.params).toEqual([hostile]) diff --git a/packages/stack/src/eql/v3/drizzle/codec.ts b/packages/stack/src/eql/v3/drizzle/codec.ts index f53ed7e71..c212d13e1 100644 --- a/packages/stack/src/eql/v3/drizzle/codec.ts +++ b/packages/stack/src/eql/v3/drizzle/codec.ts @@ -9,8 +9,8 @@ import type { Encrypted } from '@/types' /** Thrown when a driver value cannot be read back as an EQL v3 envelope. */ export class EqlV3CodecError extends Error { - constructor(message: string) { - super(message) + constructor(message: string, options?: ErrorOptions) { + super(message, options) this.name = 'EqlV3CodecError' } } @@ -86,6 +86,7 @@ export function v3FromDriver( } catch (cause) { throw new EqlV3CodecError( `Failed to parse an EQL v3 encrypted envelope from the driver: ${cause instanceof Error ? cause.message : String(cause)}`, + { cause }, ) } return assertEnvelope(parsed, 'the driver') From ecf273d3e7a8d56c570cb6b2cabe5fc6b4fb869c Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 22:58:05 +1000 Subject: [PATCH 4/6] fix(stack): reject empty SteVec, pin ops.and positive path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review findings on the v3 Drizzle adapter. assertEnvelope accepted `sv: []`, which satisfies `sv !== undefined` but has no `sv[0].c` — the guard's own premise. An empty, non-array, or null `sv` carries a ciphertext key but no ciphertext, and reached decrypt as garbage. Require a non-empty array. No encrypt path produces this, so it is hardening against a corrupt or hostile row rather than a live fail-open. needleFor selected samples with `sample.length >= 3`, counting UTF-16 code units where the production guard counts codepoints. An astral sample ('👍👍': 4 units, 2 codepoints) would be picked here and then rejected by the guard, blaming the wrong line. Latent only because every match-domain sample is currently ASCII. Extract it to v3-matrix/needle-for.ts and select with matchNeedleError itself, so the two cannot diverge; it now also honours a domain's own token_length. Pairing `and` over disjoint rows proved it is not `or`, but left every live `and` assertion expecting []. A constant-false `and` would satisfy that, and the `or` tests exercise a different operator. Restore the intersecting pair alongside it, so one live test proves `and` returns its row. The ['email'] lock-context test pinned /^Failed to retrieve key/. Naming a claim does not assert the token carries it: resolveLockContext is a passthrough and ZeroKMS resolves the value server-side, so a token with no `email` claim may be refused by authorization before key derivation is attempted. Accept either denial class, reject infrastructure faults explicitly. Kept loose because CI never runs this path (USER_JWT unset, #530). Collapse exports.test.ts from three tests to two: the two typeof sweeps were one sweep artificially split. The remaining sweep iterates Object.entries, so it cannot silently skip a name the key list forgot. The v2 like/ilike path has the same bloom-containment fail-open and remains unfixed here, as the changeset already notes. --- .changeset/eql-v3-drizzle-fail-open-guards.md | 6 +- .../stack/__tests__/drizzle-v3/codec.test.ts | 44 ++++++++++----- .../__tests__/drizzle-v3/exports.test.ts | 25 ++++----- .../drizzle-v3/operators-live-pg.test.ts | 16 ++++++ .../operators-lock-context-live-pg.test.ts | 24 +++++++- .../__tests__/drizzle-v3/operators.test.ts | 16 +----- .../__tests__/v3-matrix/needle-for.test.ts | 55 +++++++++++++++++++ .../stack/__tests__/v3-matrix/needle-for.ts | 26 +++++++++ packages/stack/src/eql/v3/drizzle/codec.ts | 11 +++- 9 files changed, 176 insertions(+), 47 deletions(-) create mode 100644 packages/stack/__tests__/v3-matrix/needle-for.test.ts create mode 100644 packages/stack/__tests__/v3-matrix/needle-for.ts diff --git a/.changeset/eql-v3-drizzle-fail-open-guards.md b/.changeset/eql-v3-drizzle-fail-open-guards.md index 1c94097fa..990508580 100644 --- a/.changeset/eql-v3-drizzle-fail-open-guards.md +++ b/.changeset/eql-v3-drizzle-fail-open-guards.md @@ -24,8 +24,10 @@ an EQL envelope, instead of surfacing a raw `SyntaxError` for malformed JSON and passing a bare scalar through unchecked — `v3FromDriver('5')` previously returned `5` typed as `Encrypted`, which then reached `decrypt` as garbage. The guard accepts both scalar envelopes (ciphertext at `c`) and SteVec documents -(ciphertext at `sv[0].c`). `EqlV3CodecError` is exported from -`@cipherstash/stack/eql/v3/drizzle` so callers can catch it. +(ciphertext at `sv[0].c`). A SteVec's `sv` must be a non-empty array: `sv[0]` is +the decryption root, so `sv: []` carries a ciphertext key but no ciphertext, and +is now rejected rather than passed to `decrypt`. `EqlV3CodecError` is exported +from `@cipherstash/stack/eql/v3/drizzle` so callers can catch it. Also removes an unreachable branch in `inArray`/`notInArray`, whose empty-list guard already throws before it. diff --git a/packages/stack/__tests__/drizzle-v3/codec.test.ts b/packages/stack/__tests__/drizzle-v3/codec.test.ts index 93093c27a..95c775f30 100644 --- a/packages/stack/__tests__/drizzle-v3/codec.test.ts +++ b/packages/stack/__tests__/drizzle-v3/codec.test.ts @@ -59,6 +59,21 @@ describe('v3 codec', () => { ) }) + // `sv: []` has no `sv[0]`, so the guard's premise — a ciphertext at the + // decryption root — does not hold. Presence of the key is not enough. + it.each([ + ['an empty SteVec', { v: 3, k: 'sv', sv: [] }], + ['a non-array SteVec', { v: 3, k: 'sv', sv: {} }], + ['a null SteVec with no c', { v: 3, k: 'sv', sv: null }], + ])('rejects %s', (_label, envelope) => { + expect(() => v3FromDriver(envelope as never)).toThrow(EqlV3CodecError) + expect(() => v3FromDriver(envelope as never)).toThrow(/ciphertext/) + }) + + it('still accepts a scalar envelope, which has c and no sv', () => { + expect(v3FromDriver(ENVELOPE as never)).toBe(ENVELOPE) + }) + it('normalises null/undefined to SQL NULL (JS null) on read', () => { expect(v3FromDriver(null)).toBeNull() expect(v3FromDriver(undefined)).toBeNull() @@ -85,20 +100,21 @@ describe('v3 codec', () => { // preserve. Flattening it to a message string discards the SyntaxError and // its stack, so a caller debugging a corrupt column loses where parsing // actually broke. - it.each(['{bad', '', '{"v":3,'])( - 'preserves the underlying SyntaxError as `cause` for %j', - (raw) => { - let thrown: unknown - try { - v3FromDriver(raw) - } catch (error) { - thrown = error - } - - expect(thrown).toBeInstanceOf(EqlV3CodecError) - expect((thrown as EqlV3CodecError).cause).toBeInstanceOf(SyntaxError) - }, - ) + it.each([ + '{bad', + '', + '{"v":3,', + ])('preserves the underlying SyntaxError as `cause` for %j', (raw) => { + let thrown: unknown + try { + v3FromDriver(raw) + } catch (error) { + thrown = error + } + + expect(thrown).toBeInstanceOf(EqlV3CodecError) + expect((thrown as EqlV3CodecError).cause).toBeInstanceOf(SyntaxError) + }) it.each([ '5', diff --git a/packages/stack/__tests__/drizzle-v3/exports.test.ts b/packages/stack/__tests__/drizzle-v3/exports.test.ts index a192be375..26fbb1ae9 100644 --- a/packages/stack/__tests__/drizzle-v3/exports.test.ts +++ b/packages/stack/__tests__/drizzle-v3/exports.test.ts @@ -22,20 +22,19 @@ describe('v3 drizzle barrel', () => { expect(Object.keys(barrel).sort()).toEqual([...PUBLIC_SURFACE]) }) - it('exports the public surface', () => { - expect(typeof barrel.createEncryptionOperatorsV3).toBe('function') - expect(typeof barrel.extractEncryptionSchemaV3).toBe('function') - expect(typeof barrel.EncryptionOperatorError).toBe('function') - expect(typeof barrel.EqlV3CodecError).toBe('function') + // The key check above sees names, not values: a re-export that resolves to + // `undefined` still lists its key. Sweep every export the barrel claims. + it('exports every name as a callable, and types as a namespace', () => { + const callables = Object.entries(barrel).filter( + ([name]) => name !== 'types', + ) + expect(callables).toHaveLength(PUBLIC_SURFACE.length - 1) + + for (const [name, value] of callables) { + expect(typeof value, name).toBe('function') + } + expect(typeof barrel.types.TextSearch).toBe('function') expect(barrel.types.IntegerOrd('age')).toBeDefined() }) - - it('re-exports the codec and column helpers as callables', () => { - expect(typeof barrel.v3ToDriver).toBe('function') - expect(typeof barrel.v3FromDriver).toBe('function') - expect(typeof barrel.getEqlV3Column).toBe('function') - expect(typeof barrel.isEqlV3Column).toBe('function') - expect(typeof barrel.makeEqlV3Column).toBe('function') - }) }) diff --git a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts index 41bfa2bfb..1371ba415 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts @@ -666,6 +666,22 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { expect(rows).toEqual([]) }, 30000) + // The disjoint pair above proves `and` is not `or`, but every `and` assertion + // over it expects []. A constant-false `and` would satisfy that too, and the + // `or` tests exercise a different operator. This pins the positive path: + // an intersecting pair that must actually return its row. + // text_eq = 'ada@example.com' -> ROW_B only + // integer_ord < 0 -> ROW_B (-42) only + it('and returns the rows satisfying both encrypted predicates', async () => { + const rows = await selectRowKeys( + await ops.and( + ops.eq(matrixColumn('public.text_eq'), 'ada@example.com'), + ops.lt(matrixColumn('public.integer_ord'), 0), + ), + ) + expect(rows).toEqual([ROW_B]) + }, 30000) + it('or requires either encrypted predicate, unlike and', async () => { const rows = await selectRowKeys(await ops.or(...disjointPredicates())) expect(rows).toEqual([ROW_A, ROW_B, ROW_C]) diff --git a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts index d5900d07e..4f722ebdc 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts @@ -116,6 +116,27 @@ async function decryptOutcome( */ const KEY_DENIAL = /^Failed to retrieve key/ +/** + * Denial under a claim the token may not carry at all. Naming a claim does not + * assert it exists: `resolveLockContext` is a passthrough, and ZeroKMS resolves + * the claim's value from the authenticating token. So decrypting under + * `['email']` a row sealed under `['sub']` either fails key derivation, or — if + * the token has no `email` claim — is refused by the authorization layer before + * key derivation is ever attempted. Both are denials; which one surfaces is a + * ZeroKMS server-side detail we do not pin. + * + * What must NOT pass is an infrastructure fault masquerading as a denial, so + * that is excluded separately. Kept loose rather than pinned to `KEY_DENIAL` + * because no CI run exercises this path — `USER_JWT` is unset in CI (#530) — + * and a wrong message-shape guess would only surface once that secret lands. + */ +const IDENTITY_DENIAL = + /failed to retrieve key|unauthoriz|unauthoris|forbidden|denied|not authorized|not authorised/i + +/** A transport/outage failure, which must never be mistaken for a denial. */ +const INFRA_FAULT = + /ECONNREFUSED|ECONNRESET|ETIMEDOUT|ENOTFOUND|EAI_AGAIN|socket hang up|timed? ?out|network error/i + /** Run-scoped SELECT of row keys under an already-encrypted SQL condition. */ async function selectRowKeys(condition: SQL): Promise { const rows = (await db @@ -271,6 +292,7 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { ) expect(denied).toBe(true) - expect(message).toMatch(KEY_DENIAL) + expect(message).toMatch(IDENTITY_DENIAL) + expect(message).not.toMatch(INFRA_FAULT) }, 30000) }) diff --git a/packages/stack/__tests__/drizzle-v3/operators.test.ts b/packages/stack/__tests__/drizzle-v3/operators.test.ts index 7452efcb4..2157adbad 100644 --- a/packages/stack/__tests__/drizzle-v3/operators.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators.test.ts @@ -23,6 +23,7 @@ import { typedEntries, V3_MATRIX, } from '../v3-matrix/catalog' +import { needleFor } from '../v3-matrix/needle-for' const TERM = { c: 'ct', v: 1 } const TERM_JSON = JSON.stringify(TERM) @@ -99,21 +100,6 @@ const matrixColumn = (eqlType: string) => const sampleFor = (spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX]) => spec.samples[0] -// `contains` needles must reach the match tokenizer's `token_length` (3), so -// they cannot come from `sampleFor` — `TEXT_S[0]` is the empty string, which -// tokenizes to nothing and is rejected as unanswerable. -const needleFor = ( - spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX], -): string => { - const needle = spec.samples.find( - (sample) => typeof sample === 'string' && sample.length >= 3, - ) - if (typeof needle !== 'string') { - throw new Error('no searchable sample for a match domain') - } - return needle -} - const equalityDomains = matrixEntries.filter( ([, spec]) => spec.indexes.unique || spec.indexes.ore, ) diff --git a/packages/stack/__tests__/v3-matrix/needle-for.test.ts b/packages/stack/__tests__/v3-matrix/needle-for.test.ts new file mode 100644 index 000000000..21b2a6e6f --- /dev/null +++ b/packages/stack/__tests__/v3-matrix/needle-for.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import { matchNeedleError } from '@/schema/match-defaults' +import { V3_MATRIX } from './catalog' +import { needleFor } from './needle-for' + +const MATCH_BLOCK = { tokenizer: { kind: 'ngram', token_length: 3 } } as const + +const specWith = (samples: readonly unknown[]) => + ({ samples, indexes: { match: MATCH_BLOCK } }) as never + +describe('needleFor', () => { + // The guard counts codepoints; a UTF-16 `.length` check would wave '👍👍' + // through (4 code units, 2 codepoints) and the `contains` test would then + // fail inside the production guard, pointing at the wrong line. + it('skips an astral sample that the production guard would reject', () => { + expect(needleFor(specWith(['👍👍', 'ada@example.com']))).toBe( + 'ada@example.com', + ) + }) + + it('skips the empty sample', () => { + expect(needleFor(specWith(['', 'Ada Lovelace']))).toBe('Ada Lovelace') + }) + + it('skips non-string samples', () => { + expect(needleFor(specWith([42, null, 'Ada']))).toBe('Ada') + }) + + it('throws when no sample can be searched', () => { + expect(() => needleFor(specWith(['', 'ab', '👍👍']))).toThrow( + /no searchable sample/, + ) + }) + + it('honours a larger token_length', () => { + const spec = { + samples: ['ada', 'adam'], + indexes: { match: { tokenizer: { kind: 'ngram', token_length: 4 } } }, + } as never + expect(needleFor(spec)).toBe('adam') + }) + + // The contract that matters: whatever it picks, the guard must accept. + it('never returns a needle the guard rejects, across every match domain', () => { + const matchDomains = Object.values(V3_MATRIX).filter( + (spec) => spec.indexes.match, + ) + expect(matchDomains.length).toBeGreaterThan(0) + + for (const spec of matchDomains) { + const needle = needleFor(spec) + expect(matchNeedleError(needle, spec.indexes.match ?? {})).toBeUndefined() + } + }) +}) diff --git a/packages/stack/__tests__/v3-matrix/needle-for.ts b/packages/stack/__tests__/v3-matrix/needle-for.ts new file mode 100644 index 000000000..cfde47563 --- /dev/null +++ b/packages/stack/__tests__/v3-matrix/needle-for.ts @@ -0,0 +1,26 @@ +import { matchNeedleError } from '@/schema/match-defaults' +import type { V3_MATRIX } from './catalog' + +type MatrixSpec = (typeof V3_MATRIX)[keyof typeof V3_MATRIX] + +/** + * Pick a sample from a match domain that can actually be used as a `contains` + * needle. `sampleFor` is no good here — `TEXT_S[0]` is the empty string, which + * tokenizes to nothing and is rejected as unanswerable. + * + * Selection runs the production predicate rather than a length check of our + * own. A `sample.length >= 3` check would count UTF-16 code units where the + * guard counts codepoints, so an astral sample ('👍👍': 4 units, 2 codepoints) + * would be picked here and then thrown out by the guard, blaming the wrong + * line. It would also ignore a domain that raises its own `token_length`. + */ +export const needleFor = (spec: MatrixSpec): string => { + const match = spec.indexes.match ?? {} + const needle = spec.samples.find( + (sample) => typeof sample === 'string' && !matchNeedleError(sample, match), + ) + if (typeof needle !== 'string') { + throw new Error('no searchable sample for a match domain') + } + return needle +} diff --git a/packages/stack/src/eql/v3/drizzle/codec.ts b/packages/stack/src/eql/v3/drizzle/codec.ts index c212d13e1..0db6f55b8 100644 --- a/packages/stack/src/eql/v3/drizzle/codec.ts +++ b/packages/stack/src/eql/v3/drizzle/codec.ts @@ -21,6 +21,10 @@ export class EqlV3CodecError extends Error { * which has no top-level `c`. Checking `v` plus either carrier distinguishes an * envelope from a bare scalar, an array, or an unrelated object — without * paying a full structural validation on every decrypted row. + * + * The `sv` carrier must be a non-empty array: `sv[0]` is the decryption root, + * so `sv: []` has a ciphertext key but no ciphertext, and would reach `decrypt` + * as garbage. */ function assertEnvelope(value: unknown, source: string): Encrypted { if (typeof value !== 'object' || value === null || Array.isArray(value)) { @@ -29,11 +33,14 @@ function assertEnvelope(value: unknown, source: string): Encrypted { ) } const envelope = value as { v?: unknown; c?: unknown; sv?: unknown } + // A SteVec carries its root ciphertext at `sv[0].c`, so an empty or non-array + // `sv` carries no ciphertext at all — presence of the key is not enough. + const hasSteVec = Array.isArray(envelope.sv) && envelope.sv.length > 0 const missing = envelope.v === undefined ? '"v"' - : envelope.c === undefined && envelope.sv === undefined - ? 'a ciphertext ("c", or "sv" for a SteVec document)' + : envelope.c === undefined && !hasSteVec + ? 'a ciphertext ("c", or a non-empty "sv" for a SteVec document)' : undefined if (missing) { throw new EqlV3CodecError( From 14d0e7809052f20224efbd54e2ae424ee5b0a7bd Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:21:38 +1000 Subject: [PATCH 5/6] test(stack): name the missing fixture in lock-context denial tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decryptOutcome counts a throw as denial, so an unseeded row makes `row.value` raise a TypeError that reads as "denied". The message assertions already reject that string, so the tests fail either way — but they fail blaming the denial regex rather than the fixture. Assert the row exists at all three `[row]` sites, not just the two decrypt-denial tests: the symmetric decrypt test has the same destructure, and leaving it unguarded is inconsistent. Raised by CodeRabbit on #607, against the older `decryptDenied` helper that returned a bare boolean. Under that version the false pass was real; the message assertions have since closed it. This is diagnostic clarity, not a correctness fix. --- .../drizzle-v3/operators-lock-context-live-pg.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts index 4f722ebdc..0e83533b0 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts @@ -226,6 +226,8 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { WHERE test_run_id = $1 AND row_key = $2`, [RUN, ROW_A], ) + expect(row).toBeDefined() + const decrypted = unwrap( await client.decrypt(row.value as never).withLockContext(IDENTITY_CLAIM), ) @@ -266,6 +268,12 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { [RUN, ROW_A], ) + // A missing fixture would make `row.value` throw a TypeError inside + // `decryptOutcome`, which counts a throw as denial. The message assertions + // below already reject that string, so the test fails either way — but it + // fails blaming the denial regex. Name the real fault here instead. + expect(row).toBeDefined() + // `decrypt` reports denial as a `Result.failure` and does not throw, so a // bare `await` here would silently pass. Require denial via either channel, // AND require it be a key-derivation denial rather than an outage. @@ -285,6 +293,8 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { [RUN, ROW_A], ) + expect(row).toBeDefined() + const { denied, message } = await decryptOutcome(() => client .decrypt(row.value as never) From 2bc48726648253ecbcead70418bb2dfc5de6ba82 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:34:31 +1000 Subject: [PATCH 6/6] test(stack): couple negative live assertions to internal controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An assertion of absence proves nothing unless something in the same test shows the row was reachable to begin with. A sibling `it` does not couple them: vitest runs on past a failure, so the control can go red while the negative test stays green in the same run. beforeAll already throws on a failed seed, so an unseeded fixture is not the live risk. The risk it cannot catch is an operator that silently matches nothing — a constant-false predicate, or encryption no-op'ing — which produces exactly the empty result each negative test asserts. - operators-live-pg: fold the intersecting `and` pair into the disjoint block, so [] cannot pass via a constant-false `and`. Add a present-needle control to the `contains` no-match block, whose only proof that contains can match lived in a sibling test. - operators-lock-context-live-pg: assert the same eq WITH the lock context finds ROW_A before asserting it is unreachable without one. - matrix-live-pg: two-sample domains (date, timestamp) have an empty strict interior, so `gt`/`lt` were unproven there. Add one-sided bounds, which are non-empty for every domain. - operators-null-live-pg: guard the [row] destructure so a missing fixture reads as a missing fixture, not a null-handling bug. Found by audit while triaging CodeRabbit's decrypt-denial comment on #607; same defect class, sites it did not reach. --- .../drizzle-v3/operators-live-pg.test.ts | 37 +++++++++++++------ .../operators-lock-context-live-pg.test.ts | 14 ++++++- .../drizzle-v3/operators-null-live-pg.test.ts | 3 ++ .../v3-matrix/matrix-live-pg.test.ts | 29 +++++++++++++++ 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts index 1371ba415..b1c119d91 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-live-pg.test.ts @@ -601,6 +601,15 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { await ops.contains(matrixColumn(eqlType), needle), ) expect(rows).toEqual([]) + + // The control. An over-rejecting guard would have thrown above, and a + // fail-open `contains` would have returned every row — but a + // constant-false `contains` also returns [], and nothing above catches + // it. Prove the same column still answers a needle that IS present. + const present = await selectRowKeys( + await ops.contains(matrixColumn(eqlType), 'ada'), + ) + expect(present.length).toBeGreaterThan(0) }, 30000, ) @@ -661,25 +670,29 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => { ops.gte(matrixColumn('public.integer_ord'), 0), ] as const - it('and requires both encrypted predicates, unlike or', async () => { - const rows = await selectRowKeys(await ops.and(...disjointPredicates())) - expect(rows).toEqual([]) - }, 30000) - - // The disjoint pair above proves `and` is not `or`, but every `and` assertion - // over it expects []. A constant-false `and` would satisfy that too, and the - // `or` tests exercise a different operator. This pins the positive path: - // an intersecting pair that must actually return its row. + // Two assertions, one block, deliberately. The disjoint pair proves `and` is + // not `or`: swapping the operator turns [] into [A,B,C]. But [] is also what + // a constant-false `and` returns, and `beforeAll` cannot catch that — it only + // catches a failed seed. The intersecting pair is the control that can: it + // must return its row, so it dies on a constant-false `and` and on an eq/lt + // term that silently matches nothing. Keeping it in a sibling `it` would not + // couple them, since vitest runs on past a failure and the sibling could go + // red while this stayed green. // text_eq = 'ada@example.com' -> ROW_B only + // integer_ord >= 0 -> ROW_A (0), ROW_C (2147483647), not ROW_B (-42) // integer_ord < 0 -> ROW_B (-42) only - it('and returns the rows satisfying both encrypted predicates', async () => { - const rows = await selectRowKeys( + it('and requires both encrypted predicates, unlike or', async () => { + expect(await selectRowKeys(await ops.and(...disjointPredicates()))).toEqual( + [], + ) + + const intersecting = await selectRowKeys( await ops.and( ops.eq(matrixColumn('public.text_eq'), 'ada@example.com'), ops.lt(matrixColumn('public.integer_ord'), 0), ), ) - expect(rows).toEqual([ROW_B]) + expect(intersecting).toEqual([ROW_B]) }, 30000) it('or requires either encrypted predicate, unlike and', async () => { diff --git a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts index 0e83533b0..392604aa2 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts @@ -256,8 +256,18 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => { // No `USER_JWT_B` exists, so that remains a follow-up. it('an identity-bound row does not match an eq issued with no lock context', async () => { if (skipUnlessJwt()) return - const condition = await ops.eq(secretTable.secret, SECRET_A) - expect(await selectRowKeys(condition)).toEqual([]) + + // The control, in this test rather than a sibling one. `[]` is what a held + // identity boundary and an unseeded fixture both look like, so the empty + // assertion below proves nothing on its own — it only means something + // against a demonstration that the row IS reachable with the context. + const bound = await ops.eq(secretTable.secret, SECRET_A, { + lockContext: IDENTITY_CLAIM as never, + }) + expect(await selectRowKeys(bound)).toEqual([ROW_A]) + + const unbound = await ops.eq(secretTable.secret, SECRET_A) + expect(await selectRowKeys(unbound)).toEqual([]) }, 30000) it('an identity-bound row does not decrypt without its lock context', async () => { diff --git a/packages/stack/__tests__/drizzle-v3/operators-null-live-pg.test.ts b/packages/stack/__tests__/drizzle-v3/operators-null-live-pg.test.ts index d224250b6..1ef2f908e 100644 --- a/packages/stack/__tests__/drizzle-v3/operators-null-live-pg.test.ts +++ b/packages/stack/__tests__/drizzle-v3/operators-null-live-pg.test.ts @@ -159,6 +159,9 @@ describeLivePg('v3 drizzle NULL persistence across tiers (live pg)', () => { WHERE test_run_id = $1 AND row_key = $2`, [RUN, ROW_A], ) + // Without this, a missing fixture makes `row.value` raise a TypeError and + // the failure reads as a null-handling bug rather than an absent row. + expect(row).toBeDefined() expect(row.value).toBeNull() }, 30000) diff --git a/packages/stack/__tests__/v3-matrix/matrix-live-pg.test.ts b/packages/stack/__tests__/v3-matrix/matrix-live-pg.test.ts index 1252b8378..6cf31ffbd 100644 --- a/packages/stack/__tests__/v3-matrix/matrix-live-pg.test.ts +++ b/packages/stack/__tests__/v3-matrix/matrix-live-pg.test.ts @@ -550,6 +550,35 @@ describeLivePg('v3 matrix live Postgres coverage (all 35 domains)', () => { [ORDER_RUN_ID, ids, lo, hi], ) expect(new Set(exclusive.map((r) => r.id))).toEqual(new Set(interiorIds)) + + // The controls. Domains with only two distinct samples (date, timestamp) + // have an empty interior, so the assertion above is satisfied by a + // constant-false `gt`/`lt` as well as by a correct one. One-sided strict + // bounds are non-empty for every domain: `gt(lo)` must return everything + // above the minimum, `lt(hi)` everything below the maximum. Both die on a + // constant-false operator, and on a constant-true one (which would drag in + // the excluded endpoint). + const aboveLo = await sql.unsafe( + `SELECT id FROM ${TABLE_NAME} + WHERE test_run_id = $1 + AND id = ANY($2) + AND eql_v3.gt("${col}", $3::jsonb)`, + [ORDER_RUN_ID, ids, lo], + ) + expect(new Set(aboveLo.map((r) => r.id))).toEqual( + new Set(sortedIdx.slice(1).map((i) => ids[i])), + ) + + const belowHi = await sql.unsafe( + `SELECT id FROM ${TABLE_NAME} + WHERE test_run_id = $1 + AND id = ANY($2) + AND eql_v3.lt("${col}", $3::jsonb)`, + [ORDER_RUN_ID, ids, hi], + ) + expect(new Set(belowHi.map((r) => r.id))).toEqual( + new Set(sortedIdx.slice(0, L - 1).map((i) => ids[i])), + ) }) it.each(