From 99f8b0aa48d610182bb553ab9fa5865ad9db5711 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 22:10:38 +1000 Subject: [PATCH 1/4] fix(stack): escape encrypted in-list operands; widen is/contains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #588. Two of these are broken features, not polish; both were invisible to the existing tests because `createMockSupabase` records the arguments handed to each builder method and never builds a URL. `.in()` on an encrypted column produced a request PostgREST rejects. Every encrypted operand is a serialized envelope, dense with `"` and `,`, and postgrest-js wraps a comma-bearing element as `"…"` without escaping the quotes already inside it: in.("{"v":1,"c":"…"}",…) ^ PostgREST ends the value here -> PGRST100 Encrypted lists now go through `filter(col, 'in', …)` with each element quoted and escaped, as the `.or()` path already did. This affects v2 as well as v3 — v2's `("a@b.com")` composite literal is itself quote-bearing and was equally broken — so three existing tests that asserted the old encoding are updated. `.not(col, 'in', […])` encrypted the whole list as a single ciphertext, so the filter silently matched nothing, and emitted an unparenthesized `not.in.a,b`. The not-collector now encrypts element-wise, mirroring the regular `in` and `or(… .in. …)` paths. A PostgREST list literal on an encrypted column throws rather than silently matching nothing. `is(col, null)` is widened to every row key. `is` is never encrypted and a NULL plaintext is stored as a SQL NULL, so on a storage-only column `IS NULL` is not merely legal but the only predicate available. `is(col, true)` stays a compile error on encrypted columns. `contains()` accepts native operands on plaintext columns, which fall through to PostgREST's native containment. Relatedly, `.or([{ op: 'contains' }])` now emits `cs` for plaintext conditions too — previously only encrypted ones were translated, so plaintext containment reached the wire as `.contains.` and failed to parse — and `splitOrString` tracks `{}` so an array/jsonb literal's commas no longer split the condition mid-value. `DOMAIN_REGISTRY` is derived from `types` rather than hand-listed, so the two cannot drift. The keys are an external contract (the information_schema query parameter), and because they are now derived from `getEqlType()`, no test that also derives them can detect a corrupted domain constant — the test file pins them against a hand-written literal list for exactly that reason. Tests: a `postgrest-wire` harness runs the real PostgrestClient against a capturing fetch, so operand serialization is asserted without a database. Adds biting coverage for the three null-prototype maps (`mergeDeclaredTables`, `v3Columns` pattern-filter lookup, mutation-model rebuild) and for the unreachable scalar-queryType backstop. --- .changeset/supabase-in-list-operands.md | 42 ++++ .../__tests__/eql-v3-domain-registry.test.ts | 98 ++++++--- .../stack/__tests__/helpers/postgrest-wire.ts | 68 ++++++ .../stack/__tests__/supabase-helpers.test.ts | 102 +++++++++ .../__tests__/supabase-schema-builder.test.ts | 26 +++ .../__tests__/supabase-v3-builder.test.ts | 208 +++++++++++++++++- .../__tests__/supabase-v3-pgrest-live.test.ts | 79 ++++++- .../stack/__tests__/supabase-v3-wire.test.ts | 131 +++++++++++ .../stack/__tests__/supabase-v3.test-d.ts | 61 ++++- packages/stack/src/eql/v3/domain-registry.ts | 88 +++----- packages/stack/src/supabase/helpers.ts | 105 ++++++++- .../stack/src/supabase/query-builder-v3.ts | 23 -- packages/stack/src/supabase/query-builder.ts | 127 +++++++---- packages/stack/src/supabase/types.ts | 59 ++++- 14 files changed, 1043 insertions(+), 174 deletions(-) create mode 100644 .changeset/supabase-in-list-operands.md create mode 100644 packages/stack/__tests__/helpers/postgrest-wire.ts create mode 100644 packages/stack/__tests__/supabase-v3-wire.test.ts diff --git a/.changeset/supabase-in-list-operands.md b/.changeset/supabase-in-list-operands.md new file mode 100644 index 000000000..2753b5ea8 --- /dev/null +++ b/.changeset/supabase-in-list-operands.md @@ -0,0 +1,42 @@ +--- +'@cipherstash/stack': minor +--- + +Fix encrypted `in`-list operands in the Supabase adapter, and widen the `is` / +`contains` type surfaces. + +**`in()` on an encrypted column produced a request PostgREST rejects.** Every +encrypted operand is a serialized envelope, dense with `"` and `,`. postgrest-js +wraps a comma-bearing element as `"…"` but never escapes the quotes already +inside it, so `.in('email', […])` emitted + +``` +in.("{"v":1,"c":"…"}",…) + ^ PostgREST ends the value here → PGRST100 +``` + +Encrypted lists are now emitted through `filter(col, 'in', …)` with each element +quoted and escaped, matching what the `.or()` path already did. This affects +**v2 as well as v3** — v2's `("a@b.com")` composite literal is itself +quote-bearing and was equally broken. + +**`not(col, 'in', […])` encrypted the whole list as a single ciphertext**, so +the filter silently matched nothing, and emitted an unparenthesized +`not.in.a,b`. Each element is now encrypted separately and the operand is +rendered as `not.in.(…)`. Passing a PostgREST list literal (`'(a,b)'`) for an +encrypted column now throws instead of silently matching nothing — pass an +array. + +**`is(col, null)` is now allowed on every column**, including storage-only +encrypted ones (`types.Boolean`, `types.Integer`, …). `is` is never encrypted +and a NULL plaintext is stored as a SQL NULL, so `IS NULL` is not merely legal +there but the only predicate those columns support. `is(col, true)` remains a +compile error on encrypted columns. + +**`contains()` accepts native operands on plaintext columns.** A plaintext +jsonb/array column falls through to PostgREST's native containment, so +`contains('tags', ['vip'])` and `contains('meta', { plan: 'pro' })` now +typecheck. Encrypted match columns still take a `string` token. Relatedly, +`.or([{ op: 'contains' }])` now emits PostgREST's `cs` operator for plaintext +columns too — previously only encrypted conditions were translated, so a +plaintext containment reached the wire as `.contains.` and failed to parse. diff --git a/packages/stack/__tests__/eql-v3-domain-registry.test.ts b/packages/stack/__tests__/eql-v3-domain-registry.test.ts index 8aee36350..ab9676f50 100644 --- a/packages/stack/__tests__/eql-v3-domain-registry.test.ts +++ b/packages/stack/__tests__/eql-v3-domain-registry.test.ts @@ -8,6 +8,61 @@ import { type V3ColumnFactory, } from '@/eql/v3/domain-registry' +/** + * The EXTERNAL CONTRACT: the SQL domain names this package ships as the + * `information_schema` query parameter (`introspect.ts`) and matches + * `domain_name` rows against. + * + * Hand-written ON PURPOSE. `DOMAIN_REGISTRY` is derived from + * `stripDomainSchema(factory(…).getEqlType())`, so any expectation *also* + * derived from `getEqlType()` only measures the source against itself: corrupt + * a domain constant in `columns.ts` and the registry silently re-keys, the SQL + * param ships the wrong name, real columns are misclassified as unmodelled — + * and a derived assertion still passes. This literal list is the only thing + * that fails. Do not compute it. + */ +const EXPECTED_DOMAIN_KEYS = [ + 'integer', + 'integer_eq', + 'integer_ord_ore', + 'integer_ord', + 'smallint', + 'smallint_eq', + 'smallint_ord_ore', + 'smallint_ord', + 'bigint', + 'bigint_eq', + 'bigint_ord_ore', + 'bigint_ord', + 'date', + 'date_eq', + 'date_ord_ore', + 'date_ord', + 'timestamp', + 'timestamp_eq', + 'timestamp_ord_ore', + 'timestamp_ord', + 'numeric', + 'numeric_eq', + 'numeric_ord_ore', + 'numeric_ord', + 'text', + 'text_eq', + 'text_match', + 'text_ord_ore', + 'text_ord', + 'text_search', + 'boolean', + 'real', + 'real_eq', + 'real_ord_ore', + 'real_ord', + 'double', + 'double_eq', + 'double_ord_ore', + 'double_ord', +] as const + describe('DOMAIN_REGISTRY', () => { it('strips the public. schema prefix', () => { expect(stripDomainSchema('public.text_search')).toBe('text_search') @@ -16,26 +71,25 @@ describe('DOMAIN_REGISTRY', () => { expect(stripDomainSchema('boolean')).toBe('boolean') }) - it('has an entry for every types factory, keyed by the unqualified domain', () => { - const factories = Object.values(types) as V3ColumnFactory[] - for (const factory of factories) { - const eqlType = factory('probe').getEqlType() - const key = stripDomainSchema(eqlType) - expect( - DOMAIN_REGISTRY[key], - `missing registry entry for ${key}`, - ).toBeDefined() - expect(DOMAIN_REGISTRY[key]('c').getEqlType()).toBe(eqlType) - } + it('keys are exactly the expected SQL domain names', () => { + expect(Object.keys(DOMAIN_REGISTRY).sort()).toEqual( + [...EXPECTED_DOMAIN_KEYS].sort(), + ) }) - it('has no registry entry that does not round-trip to its own key', () => { - for (const [key, factory] of Object.entries(DOMAIN_REGISTRY)) { - expect(stripDomainSchema(factory('c').getEqlType())).toBe(key) + it('maps each expected domain to a factory that builds that domain', () => { + for (const key of EXPECTED_DOMAIN_KEYS) { + const factory = factoryForDomain(key) + expect(factory, `missing registry entry for ${key}`).toBeDefined() + expect((factory as V3ColumnFactory)('c').getEqlType()).toBe( + `public.${key}`, + ) } }) - it('has exactly as many entries as there are types factories', () => { + // The derivation drops an entry rather than throwing only if two factories + // collide on one key; a short registry is that collision. + it('derives one entry per types factory, with no key collisions', () => { expect(Object.keys(DOMAIN_REGISTRY)).toHaveLength(Object.keys(types).length) }) @@ -44,18 +98,8 @@ describe('DOMAIN_REGISTRY', () => { expect(factoryForDomain('text_search')).toBe(DOMAIN_REGISTRY.text_search) }) - it('PROPERTY: round-trips for any registry key and rejects any non-key', () => { - const keys = Object.keys(DOMAIN_REGISTRY) - // Any known key builds a column whose stripped eqlType is that key. - fc.assert( - fc.property(fc.constantFrom(...keys), (key) => { - expect(stripDomainSchema(DOMAIN_REGISTRY[key]('c').getEqlType())).toBe( - key, - ) - }), - ) - // Any arbitrary string that is not a registry key resolves to undefined. - const keySet = new Set(keys) + it('PROPERTY: rejects any string that is not a registry key', () => { + const keySet = new Set(Object.keys(DOMAIN_REGISTRY)) fc.assert( fc.property(fc.string(), (s) => { fc.pre(!keySet.has(s)) diff --git a/packages/stack/__tests__/helpers/postgrest-wire.ts b/packages/stack/__tests__/helpers/postgrest-wire.ts new file mode 100644 index 000000000..d08088987 --- /dev/null +++ b/packages/stack/__tests__/helpers/postgrest-wire.ts @@ -0,0 +1,68 @@ +/** + * A test double that can see the WIRE FORMAT. + * + * `createMockSupabase` records the arguments handed to each builder method and + * stops there. That pins per-element *encryption* but is structurally blind to + * *encoding*: postgrest-js still has to serialize those arguments into a query + * string, and that is where an unescaped `"` inside an encrypted envelope turns + * `in.(…)` into a request PostgREST rejects. A mock that never builds a URL can + * never catch it. + * + * So this harness runs the REAL `PostgrestClient` with a `fetch` that captures + * the request URL and answers with canned rows. No database, no `DATABASE_URL` + * gate — but the operand is byte-for-byte what a live PostgREST would receive. + * + * Use it for anything whose correctness lives in the emitted query string + * (`in`, `not.in`, `or`); keep using `createMockSupabase` for everything else. + */ + +import { PostgrestClient } from '@supabase/postgrest-js' +import type { SupabaseQueryBuilder } from '@/supabase/types' + +export type WirePostgrest = { + /** Structurally a supabase client: `.from(table)` → a query builder. */ + client: { from(table: string): SupabaseQueryBuilder } + /** Every request URL issued, in order. */ + urls: string[] + /** The decoded operand for `column` on the last request, e.g. `in.("…","…")`. */ + operandFor(column: string): string +} + +export function createWirePostgrest(resultData: unknown = []): WirePostgrest { + const urls: string[] = [] + + const fetchImpl = (input: unknown): Promise => { + urls.push( + typeof input === 'string' + ? input + : String((input as { url: string }).url ?? input), + ) + return Promise.resolve( + new Response(JSON.stringify(resultData), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ) + } + + const client = new PostgrestClient('http://wire.test', { + fetch: fetchImpl as unknown as typeof fetch, + }) + + return { + client: client as unknown as WirePostgrest['client'], + urls, + operandFor(column: string): string { + const last = urls.at(-1) + if (last === undefined) throw new Error('no request was issued') + // `searchParams.get` percent-decodes; PostgREST sees exactly this. + const value = new URL(last).searchParams.get(column) + if (value === null) { + throw new Error( + `no filter emitted for column "${column}" in ${new URL(last).search}`, + ) + } + return value + }, + } +} diff --git a/packages/stack/__tests__/supabase-helpers.test.ts b/packages/stack/__tests__/supabase-helpers.test.ts index e7ce2604b..2782897e3 100644 --- a/packages/stack/__tests__/supabase-helpers.test.ts +++ b/packages/stack/__tests__/supabase-helpers.test.ts @@ -109,6 +109,78 @@ describe('rebuildOrString quoting', () => { }) }) +// --------------------------------------------------------------------------- +// `contains` is the ONLY FilterOp that is a supabase-js METHOD name rather than +// a PostgREST operator token. Every other member of the union (`eq`, `in`, +// `like`, `is`, …) spells the same in both. Left untranslated, `rebuildOrString` +// emits `tags.contains.vip`, which PostgREST rejects with PGRST100 +// ("unexpected \"c\" expecting \"not\" or operator"). +// +// Translating the operator alone is not enough: `cs` takes a CONTAINMENT +// literal, not the `(a,b)` list form arrays otherwise get, so `tags.cs.(vip)` +// fails with 22P02 ("malformed array literal"). Both halves are asserted here +// and executed against a real PostgREST in `supabase-v3-pgrest-live.test.ts`. +// --------------------------------------------------------------------------- + +describe('rebuildOrString containment', () => { + it("translates the `contains` FilterOp to PostgREST's `cs` token", () => { + expect(rebuildOrString([cond('tags', 'contains', 'vip')])).toBe( + 'tags.cs.vip', + ) + }) + + it('formats an array operand as an array literal, not an in-list', () => { + expect(rebuildOrString([cond('tags', 'contains', ['vip'])])).toBe( + 'tags.cs.{vip}', + ) + }) + + it('quotes a multi-element array literal, whose comma is reserved', () => { + expect(rebuildOrString([cond('tags', 'contains', ['vip', 'admin'])])).toBe( + 'tags.cs."{vip,admin}"', + ) + }) + + it('quotes an array element that itself contains a comma', () => { + // Inner array-literal quoting, then outer PostgREST quoting of the whole. + expect(rebuildOrString([cond('tags', 'contains', ['with,comma'])])).toBe( + 'tags.cs."{\\"with,comma\\"}"', + ) + }) + + it('formats an object operand as a jsonb literal', () => { + expect(rebuildOrString([cond('meta', 'contains', { a: 1 })])).toBe( + 'meta.cs."{\\"a\\":1}"', + ) + }) + + it('leaves an already-serialized encrypted envelope as a quoted scalar', () => { + // The v3 encrypted operand is `JSON.stringify(envelope)` — a string, not an + // array or object. It must keep taking the scalar quoting path. + expect(rebuildOrString([cond('email', 'contains', ENVELOPE)])).toBe( + `email.cs."{\\"v\\":1,\\"i\\":{\\"t\\":\\"users\\",\\"c\\":\\"email\\"},\\"c\\":\\"ct:abc\\"}"`, + ) + }) + + it('keeps the `cs` token a string-form caller already wrote', () => { + expect(rebuildOrString([cond('tags', 'cs', ['vip', 'admin'])])).toBe( + 'tags.cs."{vip,admin}"', + ) + }) + + it('negates containment as `not.cs`', () => { + expect(rebuildOrString([cond('tags', 'contains', ['vip'], true)])).toBe( + 'tags.not.cs.{vip}', + ) + }) + + it('still renders an `in` array as a parenthesized list', () => { + expect(rebuildOrString([cond('nickname', 'in', ['ada', 'grace'])])).toBe( + 'nickname.in.(ada,grace)', + ) + }) +}) + describe('parseOrString / rebuildOrString round-trip', () => { it('round-trips an encrypted JSON envelope operand', () => { const conditions = [ @@ -154,6 +226,36 @@ describe('parseOrString / rebuildOrString round-trip', () => { // that silently matched nothing. // --------------------------------------------------------------------------- +// A containment literal carries top-level commas inside its braces +// (`tags.cs.{vip,admin}`). PostgREST's own logic-tree parser tracks those +// braces; ours must too, or the condition is split mid-literal into +// `tags.cs.{vip` plus a fragment `admin}` that has no dot and is dropped +// entirely — a filter that silently matches the wrong rows. Only or-strings +// that also reference an encrypted column are rebuilt from the parse, so this +// corrupts precisely the mixed encrypted/plaintext case. +describe('parseOrString containment literals', () => { + it('does not split on a comma inside an array literal', () => { + expect(parseOrString('note.eq.hello,tags.cs.{vip,admin}')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'hello' }, + { column: 'tags', op: 'cs', negate: false, value: '{vip,admin}' }, + ]) + }) + + it('does not split on a comma inside a jsonb literal', () => { + expect(parseOrString('meta.cs.{"a":1,"b":2},note.eq.x')).toEqual([ + { column: 'meta', op: 'cs', negate: false, value: '{"a":1,"b":2}' }, + { column: 'note', op: 'eq', negate: false, value: 'x' }, + ]) + }) + + it('round-trips a plaintext containment literal through rebuild', () => { + const parsed = parseOrString('tags.cs.{vip,admin}') + expect(rebuildOrString(parsed as DbPendingOrCondition[])).toBe( + 'tags.cs."{vip,admin}"', + ) + }) +}) + describe('parseOrString negation', () => { it('lifts a not. prefix off the operator', () => { expect(parseOrString('nickname.not.eq.ada')).toEqual([ diff --git a/packages/stack/__tests__/supabase-schema-builder.test.ts b/packages/stack/__tests__/supabase-schema-builder.test.ts index f9d336b53..5195a313b 100644 --- a/packages/stack/__tests__/supabase-schema-builder.test.ts +++ b/packages/stack/__tests__/supabase-schema-builder.test.ts @@ -108,6 +108,32 @@ describe('synthesizeTables', () => { }) describe('mergeDeclaredTables', () => { + // The merge copies synthesized builders across by DB column name. Only the + // SYNTHESIZED side can carry `__proto__` — `encryptedTable()` rejects it as a + // declared key (`isReservedTableKey`) — so this is the one path that can + // reparent the merge target and drop the column from the encrypt config. + it('copies a synthesized __proto__ column as an own key', () => { + const protoIntrospection: IntrospectionResult = [ + { + tableName: 'users', + columns: [ + { columnName: '__proto__', domainName: 'text_search' }, + { columnName: 'email', domainName: 'text_search' }, + ], + }, + ] + const synth = synthesizeTables(protoIntrospection) + const declaredTable = encryptedTable('users', { + email: types.TextSearch('email'), + }) + + const merged = mergeDeclaredTables(synth, { users: declaredTable }) + const builders = merged.tables.get('users')!.columnBuilders + + expect(Object.hasOwn(builders, '__proto__')).toBe(true) + expect(Object.keys(builders).sort()).toEqual(['__proto__', 'email']) + }) + it('keeps the declared builder instance over the synthesized one', () => { const synth = synthesizeTables(introspection) const declaredTable = encryptedTable('users', { diff --git a/packages/stack/__tests__/supabase-v3-builder.test.ts b/packages/stack/__tests__/supabase-v3-builder.test.ts index dc784bace..68d5acc88 100644 --- a/packages/stack/__tests__/supabase-v3-builder.test.ts +++ b/packages/stack/__tests__/supabase-v3-builder.test.ts @@ -86,6 +86,38 @@ function orOperand(emitted: string, prefix: string): string { // --------------------------------------------------------------------------- describe('encryptedSupabaseV3 wire encoding', () => { + // The mutation model is rebuilt by assigning into a fresh object keyed by DB + // column name — `out[dbNameFor(key)] = value`. A DB column named `__proto__` + // (legal via a quoted identifier; reachable here as a rename target, since + // `isReservedTableKey` guards the JS property, not the DB name) would invoke + // the inherited `__proto__` SETTER on a plain object, creating no own key and + // silently sending an empty insert body. + it('keys a mutation on a DB column named __proto__ as an own key', async () => { + const supabase = createMockSupabase() + const secrets = encryptedTable('secrets', { + secret: types.TextSearch('__proto__'), + }) + const builder = new EncryptedQueryBuilderV3Impl( + 'secrets', + secrets, + createMockEncryptionClient(), + supabase.client, + null, + ) + + await builder.insert({ secret: 'x' }) + + const body = supabase.callsFor('insert')[0].args[0] as Record< + string, + unknown + > + expect(Object.hasOwn(body, '__proto__')).toBe(true) + expect(Object.keys(body)).toEqual(['__proto__']) + expect(isFakeEnvelope((body as { __proto__: unknown }).__proto__)).toBe( + true, + ) + }) + it('inserts the raw encrypted payload keyed by DB column name (no composite wrap)', async () => { const { es, supabase } = v3Instance() @@ -213,6 +245,21 @@ describe('encryptedSupabaseV3 wire encoding', () => { expect(supabase.callsFor('filter')).toHaveLength(0) }) + // `assertNotEncryptedPattern` reads `this.v3Columns[column]` with no own-key + // guard. On a plain object that INHERITS `Object.prototype.constructor` — + // truthy — so a plaintext column named `constructor` would be misclassified + // as encrypted and `like` would throw. The `select('*')`/`order()` sites are + // covered by the `a plaintext column named \`constructor\`` block below; this + // pins the pattern-filter site. + it('treats a plaintext column named `constructor` as plaintext', async () => { + const { es, supabase } = v3Instance() + + await es.from('users', users).select('id').like('constructor', '%x%') + + expect(supabase.callsFor('like')).toHaveLength(1) + expect(supabase.callsFor('like')[0].args[0]).toBe('constructor') + }) + it('maps not(contains) on encrypted columns to not(cs)', async () => { const { es, supabase } = v3Instance() @@ -226,6 +273,9 @@ describe('encryptedSupabaseV3 wire encoding', () => { expect(not.args[1]).toBe('cs') }) + // An encrypted in-list is emitted through `filter()` as a pre-formatted + // operand, NOT through postgrest-js's `in()`, which would leave the quotes + // inside each envelope unescaped. `supabase-v3-wire.test.ts` pins the bytes. it('encrypts each element of an in() filter', async () => { const { es, supabase } = v3Instance() @@ -234,12 +284,19 @@ describe('encryptedSupabaseV3 wire encoding', () => { .select('id, nickname') .in('nickname', ['ada', 'grace']) - const [inCall] = supabase.callsFor('in') + expect(supabase.callsFor('in')).toHaveLength(0) + const [inCall] = supabase.callsFor('filter') expect(inCall.args[0]).toBe('nickname') - const values = inCall.args[1] as string[] + expect(inCall.args[1]).toBe('in') + + const operand = inCall.args[2] as string + const values = operand + .slice(1, -1) + .split('","') + .map((e) => JSON.parse(e.replace(/^"|"$/g, '').replace(/\\(.)/g, '$1'))) expect(values).toHaveLength(2) - expect(JSON.parse(values[0]).pt).toBe('ada') - expect(JSON.parse(values[1]).pt).toBe('grace') + expect(values[0].pt).toBe('ada') + expect(values[1].pt).toBe('grace') }) it('maps match() keys to DB names and encrypts values', async () => { @@ -369,9 +426,10 @@ describe('encryptedSupabaseV3 wire encoding', () => { }) // `or()` had no v3 coverage at all. Any condition naming an encrypted column - // — under either its property or DB name — routes through - // `transformOrConditions`, which maps names; the verbatim branch is reached - // only when every condition names a plaintext column, which needs no mapping. + // — under either its property or DB name — routes through the parse → rebuild + // path, where `toDbSpace` has already mapped names; the verbatim branch is + // reached only when every condition names a plaintext column, which needs no + // mapping. it('maps property names to DB names in an or() string', async () => { const { es, supabase } = v3Instance() @@ -406,6 +464,23 @@ describe('encryptedSupabaseV3 wire encoding', () => { expect(emitted.endsWith(',note.eq.x')).toBe(true) }) + // An all-plaintext or() string is forwarded verbatim, so its containment + // literal is never parsed. Naming an encrypted column forces the rebuild + // path — and there the literal's own commas must not be mistaken for + // condition separators, or `note` is filtered on the truncated `{vip`. + it('preserves a plaintext containment literal when rebuilding a mixed or() string', async () => { + const { es, supabase } = v3Instance() + + await es + .from('users', users) + .select('id') + .or('email.eq.ada,note.cs.{vip,admin}') + + const emitted = supabase.callsFor('or')[0].args[0] as string + expect(emitted).toMatch(/^email\.eq\./) + expect(emitted.endsWith(',note.cs."{vip,admin}"')).toBe(true) + }) + it('keeps every filter array correlated in a combined query', async () => { const { es, supabase } = v3Instance() @@ -464,6 +539,16 @@ describe('encryptedSupabaseV3 wire encoding', () => { expect(supabase.callsFor('is')[0].args).toEqual(['created_at', null]) }) + // `IS NULL` is the only predicate a storage-only column supports, and the + // runtime has always forwarded it. The type surface now agrees (see + // supabase-v3.test-d.ts); this pins the runtime half. + it('forwards is(col, null) on a storage-only column', async () => { + const { es, supabase } = v3Instance() + await es.from('users', users).select('id').is('active', null) + expect(supabase.callsFor('is')[0].args).toEqual(['active', null]) + expect(supabase.callsFor('filter')).toHaveLength(0) + }) + it('does not encrypt not(col, is, null)', async () => { const { es, supabase } = v3Instance() await es.from('users', users).select('id').not('createdAt', 'is', null) @@ -718,6 +803,49 @@ describe('encryptedSupabaseV3 wire encoding', () => { /^email\.cs\."/, ) }) + + // The operator token depends on the OPERATOR, never on whether the operand + // was encrypted. `note` is a plaintext passthrough, so nothing encrypts and + // the rewrite used to be skipped, emitting `note.contains.plain` — which + // PostgREST rejects (PGRST100). Plaintext `contains` is advertised as native + // jsonb/array containment, so this is the path that must work. + it('rewrites a plaintext contains in a structured or() to cs', async () => { + const { es, supabase } = v3Instance() + + await es + .from('users', users) + .select('id') + .or([{ column: 'note', op: 'contains', value: 'plain' }]) + + expect(supabase.callsFor('or')[0].args[0]).toBe('note.cs.plain') + }) + + it('emits an array operand as a containment literal, not an in-list', async () => { + const { es, supabase } = v3Instance() + + await es + .from('users', users) + .select('id') + .or([{ column: 'note', op: 'contains', value: ['vip', 'admin'] }]) + + expect(supabase.callsFor('or')[0].args[0]).toBe('note.cs."{vip,admin}"') + }) + + it('rewrites contains alongside an encrypted condition in one or()', async () => { + const { es, supabase } = v3Instance() + + await es + .from('users', users) + .select('id') + .or([ + { column: 'email', op: 'eq', value: 'ada@example.com' }, + { column: 'note', op: 'contains', value: 'plain' }, + ]) + + expect(supabase.callsFor('or')[0].args[0] as string).toMatch( + /,note\.cs\.plain$/, + ) + }) }) describe('update / delete / single / maybeSingle', () => { @@ -1041,6 +1169,20 @@ describe('encryptedSupabase (v2) wire encoding is unchanged by the dialect seams expect(or.args[0]).toBe('id.eq.1,note.eq.x') }) + // `contains` is not a PostgREST operator in EITHER dialect — the structured + // or() path emitted `note.contains.x` on v2 too, since the base builder never + // translated the token. Fixed in `rebuildOrString`, so both dialects inherit it. + it('translates a structured or() contains to cs', async () => { + const { es, supabase } = v2Instance() + + await es + .from('users', usersV2) + .select('id') + .or([{ column: 'note', op: 'contains', value: 'x' }]) + + expect(supabase.callsFor('or')[0].args[0]).toBe('note.cs.x') + }) + // ------------------------------------------------------------------------- // Characterization tests for the paths `toDbSpace()` will rewrite. Each pins // the correlation between the term collector (`encryptFilterValues`) and the @@ -1074,14 +1216,23 @@ describe('encryptedSupabase (v2) wire encoding is unchanged by the dialect seams expect(plain.args).toEqual(['note', 'eq', 'plain']) }) + // The v2 composite literal `("a@b.com")` is itself quote-bearing, so it needs + // the same escaped operand as v3 — postgrest-js's `in()` would emit + // `in.("("a@b.com")")` and PostgREST would reject it. it('in() encrypts each element and leaves plaintext arrays alone', async () => { const { es, supabase } = v2Instance() await es.from('users', usersV2).select('id').in('email', ['a@b.com', 'c@d']) await es.from('users', usersV2).select('id').in('note', ['x', 'y']) - const [encrypted, plain] = supabase.callsFor('in') - expect(encrypted.args[1]).toEqual(['("a@b.com")', '("c@d")']) + const [encrypted] = supabase.callsFor('filter') + expect(encrypted.args).toEqual([ + 'email', + 'in', + '("(\\"a@b.com\\")","(\\"c@d\\")")', + ]) + + const [plain] = supabase.callsFor('in') expect(plain.args[1]).toEqual(['x', 'y']) }) @@ -1199,7 +1350,12 @@ describe('encryptedSupabase (v2) wire encoding is unchanged by the dialect seams .from('users', usersV2) .select('id') .in('email', ['a@b.com', null]) - expect(supabase.callsFor('in')[0].args[1]).toEqual(['("a@b.com")', null]) + // `null` stays a bare PostgREST `null`, never a ciphertext. + expect(supabase.callsFor('filter')[0].args).toEqual([ + 'email', + 'in', + '("(\\"a@b.com\\")",null)', + ]) }) it('treats is() as a predicate even with a non-null operand', async () => { @@ -1464,4 +1620,36 @@ describe('v3 raw filter() resolves the query type from the operator', () => { 'anything', ]) }) + + // `encryptCollectedTerms` rejects any queryType outside the three scalar EQL + // v3 kinds. No public call path can produce a fourth — `mapFilterOpToQueryType`, + // `queryTypeForRawOp` and `queryTypeForOrOp` are exhaustive — so this backstop + // is unreachable without breaking the internal contract, which is exactly what + // the subclass below does. Keep the guard: it is what a future producer + // gaining a fourth QueryTypeName would trip over. + it('rejects a query type outside the scalar EQL v3 kinds', async () => { + const supabase = createMockSupabase() + + class BogusQueryType extends EncryptedQueryBuilderV3Impl { + protected override queryTypeForRawOp(_operator: string) { + return 'searchableJson' as never + } + } + + const builder = new BogusQueryType( + 'users', + users, + createMockEncryptionClient(), + supabase.client, + USERS_ALL_COLUMNS, + ) + + const { error, status } = await builder + .select('id') + .filter('email', 'eq', 'a@b.com') + + expect(status).toBe(500) + expect(error?.message).toContain('query type "searchableJson"') + expect(error?.message).toContain('not supported on scalar EQL v3 columns') + }) }) diff --git a/packages/stack/__tests__/supabase-v3-pgrest-live.test.ts b/packages/stack/__tests__/supabase-v3-pgrest-live.test.ts index a6fc1c9f2..d1fe46ae7 100644 --- a/packages/stack/__tests__/supabase-v3-pgrest-live.test.ts +++ b/packages/stack/__tests__/supabase-v3-pgrest-live.test.ts @@ -193,6 +193,8 @@ const ALL_COLUMNS = [ 'created_at', 'active', 'note', + 'tags', + 'meta', ] // biome-ignore lint/suspicious/noExplicitAny: the suite addresses columns outside the declared row type @@ -225,7 +227,13 @@ beforeAll(async () => { amount public.integer_ord, created_at public.timestamp_ord, active public.boolean, - note TEXT + note TEXT, + -- Plaintext passthrough columns. contains() on these is PostgREST's + -- NATIVE containment (cs, i.e. the @> Postgres declares on array and + -- jsonb), not the bloom-filter operator the encrypted domains declare. + -- Only a real server can prove the adapter emits an operand each accepts. + tags TEXT[], + meta JSONB ) `) // The grants block covers eql_v3 objects, not application tables. @@ -252,6 +260,8 @@ describeLiveSupabasePgrest('supabase v3 adapter over real PostgREST', () => { createdAt: ADA_CREATED, active: true, note: 'plain', + tags: ['vip', 'admin'], + meta: { plan: 'pro', seats: 3 }, }) expect(error).toBeNull() @@ -412,6 +422,73 @@ describeLiveSupabasePgrest('supabase v3 adapter over real PostgREST', () => { expect(data).toEqual([]) }) + // --------------------------------------------------------------------------- + // Plaintext containment. `contains` is the one FilterOp that is a supabase-js + // METHOD name and not a PostgREST operator, so a structured `or()` used to + // emit `tags.contains.{…}` — PGRST100. Translating the token alone is not + // enough: `cs` takes a containment literal, so the `(a,b)` in-list form arrays + // otherwise get fails with 22P02. Both are executed here, against a real + // server, because a mock can only prove what string we emit. + // --------------------------------------------------------------------------- + + it('runs a native array containment through .contains() on a plaintext column', async () => { + const { data, error } = await from() + .select('row_key') + .contains('tags', ['vip']) + + expect(error).toBeNull() + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) + }) + + it('runs a native jsonb containment through .contains() on a plaintext column', async () => { + const { data, error } = await from() + .select('row_key') + .contains('meta', { plan: 'pro' }) + + expect(error).toBeNull() + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) + }) + + it('executes a structured or() contains on a plaintext array column', async () => { + const { data, error } = await from() + .select('row_key') + .or([{ column: 'tags', op: 'contains', value: ['vip', 'admin'] }]) + + expect(error).toBeNull() + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) + }) + + it('executes a structured or() contains on a plaintext jsonb column', async () => { + const { data, error } = await from() + .select('row_key') + .or([{ column: 'meta', op: 'contains', value: { plan: 'pro' } }]) + + expect(error).toBeNull() + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) + }) + + // A containment literal's braces hold top-level commas. Naming an encrypted + // column forces the parse → rebuild path, where those commas must not be read + // as condition separators — otherwise `tags` is filtered on a truncated + // `{vip` and the whole or-tree quietly returns the wrong rows. + it('preserves a plaintext containment literal in a mixed or() string', async () => { + const { data, error } = await from() + .select('row_key') + .or('nickname.eq.nobody,tags.cs.{vip,admin}') + + expect(error).toBeNull() + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) + }) + + it('matches nothing when the containment literal is not contained', async () => { + const { data, error } = await from() + .select('row_key') + .or([{ column: 'tags', op: 'contains', value: ['vip', 'absent'] }]) + + expect(error).toBeNull() + expect(data).toEqual([]) + }) + it('parses a negated encrypted scalar inside an or() condition', async () => { const { data, error } = await from() .select('row_key') diff --git a/packages/stack/__tests__/supabase-v3-wire.test.ts b/packages/stack/__tests__/supabase-v3-wire.test.ts new file mode 100644 index 000000000..5d33d3274 --- /dev/null +++ b/packages/stack/__tests__/supabase-v3-wire.test.ts @@ -0,0 +1,131 @@ +/** + * Wire-format tests: what PostgREST actually receives. + * + * These drive the real postgrest-js serializer (see `helpers/postgrest-wire`), + * because the encrypted operand is `JSON.stringify(envelope)` — dense with `"` + * and `,` — and postgrest-js's `in()`/`notIn()` wrap a comma-bearing element in + * `"…"` WITHOUT escaping the quotes already inside it. The mock-based suites + * assert the array handed to `.in()` and cannot see that. + */ + +import { describe, expect, it } from 'vitest' +import { encryptedTable, types } from '@/eql/v3' +import { EncryptedQueryBuilderV3Impl } from '@/supabase/query-builder-v3' +import { createWirePostgrest } from './helpers/postgrest-wire' +import { createMockEncryptionClient } from './helpers/supabase-mock' + +const users = encryptedTable('users', { + email: types.TextSearch('email'), + nickname: types.TextEq('nickname'), +}) + +const ALL_COLUMNS = ['id', 'email', 'nickname', 'note'] + +function wireInstance() { + const wire = createWirePostgrest([]) + const builder = () => + new EncryptedQueryBuilderV3Impl( + 'users', + users, + createMockEncryptionClient(), + wire.client, + ALL_COLUMNS, + ) + return { wire, builder } +} + +/** + * Split a PostgREST `(a,b)` list the way the server does: a `\` escapes the + * next character, and a top-level `,` only separates when not inside quotes. + * Returns each element with its quoting and escaping undone. + */ +function parseInList(operand: string, prefix: string): string[] { + expect(operand.startsWith(`${prefix}.(`)).toBe(true) + expect(operand.endsWith(')')).toBe(true) + const inner = operand.slice(`${prefix}.(`.length, -1) + + const out: string[] = [] + let cur = '' + let quoted = false + let escaped = false + for (const ch of inner) { + if (escaped) { + cur += ch + escaped = false + } else if (ch === '\\') { + escaped = true + } else if (ch === '"') { + quoted = !quoted + } else if (ch === ',' && !quoted) { + out.push(cur) + cur = '' + } else { + cur += ch + } + } + out.push(cur) + return out +} + +describe('encrypted in() emits a parseable PostgREST operand', () => { + it('escapes the quotes inside each encrypted element', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').in('nickname', ['ada', 'grace']) + + const operand = wire.operandFor('nickname') + // Unescaped, PostgREST terminates the value at the envelope's first `"`. + expect(operand).toContain('\\"') + + const elements = parseInList(operand, 'in') + expect(elements).toHaveLength(2) + const plaintexts = elements.map((e) => JSON.parse(e).pt) + expect(plaintexts).toEqual(['ada', 'grace']) + }) + + it('leaves a plaintext in() list alone', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').in('note', ['x', 'y']) + + expect(wire.operandFor('note')).toBe('in.(x,y)') + }) +}) + +describe('encrypted not(col, in, …) emits a parseable PostgREST operand', () => { + it('encrypts each element separately and escapes them', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').not('nickname', 'in', ['ada', 'grace']) + + const operand = wire.operandFor('nickname') + expect(operand.startsWith('not.in.(')).toBe(true) + + const elements = parseInList(operand, 'not.in') + expect(elements).toHaveLength(2) + const plaintexts = elements.map((e) => JSON.parse(e).pt) + // The whole array must never be encrypted as ONE ciphertext. + expect(plaintexts).toEqual(['ada', 'grace']) + }) + + it('leaves a plaintext not-in list alone', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').not('note', 'in', ['x', 'y']) + + expect(wire.operandFor('note')).toBe('not.in.(x,y)') + }) + + // A PostgREST list literal cannot be encrypted element-wise, and encrypting + // it whole silently matches nothing. Fail loudly instead. + it('rejects a PostgREST list literal on an encrypted column', async () => { + const { builder } = wireInstance() + + const { error, status } = await builder() + .select('id') + .not('nickname', 'in', '(ada,grace)') + + expect(status).toBe(500) + expect(error?.message).toMatch(/requires an array of values/) + }) +}) diff --git a/packages/stack/__tests__/supabase-v3.test-d.ts b/packages/stack/__tests__/supabase-v3.test-d.ts index 647743821..9b598a816 100644 --- a/packages/stack/__tests__/supabase-v3.test-d.ts +++ b/packages/stack/__tests__/supabase-v3.test-d.ts @@ -2,6 +2,7 @@ import { describe, expectTypeOf, it } from 'vitest' import { encryptedTable, type InferPlaintext, types } from '@/eql/v3' import { encryptedColumn, encryptedTable as v2EncryptedTable } from '@/schema' import { + type EncryptedQueryBuilderV3, type EncryptedSupabaseResponse, encryptedSupabase, encryptedSupabaseV3, @@ -21,6 +22,17 @@ const users = encryptedTable('users', { type UserRow = InferPlaintext +/** + * A declared table whose ROW also carries plaintext passthrough columns — + * `tags` (text[]) and `meta` (jsonb). `InferPlaintext` alone yields only the + * declared encrypted columns, so this is the shape that exercises the + * plaintext half of `V3FreeTextSearchableKeys`. + */ +declare const mixedBuilder: EncryptedQueryBuilderV3< + typeof users, + UserRow & { tags: string[]; meta: Record } +> + describe('encryptedSupabaseV3 typed surface (with schemas)', () => { it('rows carry each column its domain plaintext type', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { @@ -54,10 +66,23 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { const builder = supabase.from('users') // @ts-expect-error — active is public.boolean (storage only) builder.eq('active', true) - // @ts-expect-error — storage-only column is excluded from filter keys + // @ts-expect-error — you cannot IS TRUE-compare a ciphertext to a plaintext builder.is('active', true) }) + // `IS NULL` is forwarded unencrypted (a NULL plaintext is stored as a SQL + // NULL, not a ciphertext), and it is the ONLY predicate a storage-only column + // supports — so it must not be gated behind the filterable-key narrowing. + it('allows is(col, null) on every column, including storage-only ones', async () => { + const supabase = await encryptedSupabaseV3(supabaseClient, { + schemas: { users }, + }) + const builder = supabase.from('users') + builder.is('active', null) + builder.is('email', null) + builder.is('email', true) + }) + it('rejects order() on every encrypted column at the type level', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { schemas: { users }, @@ -100,6 +125,25 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { builder.contains('active', 'ada') }) + // `V3FreeTextSearchableKeys` deliberately admits plaintext row keys so that + // `contains()` reaches PostgREST's NATIVE jsonb/array containment — which the + // runtime already does, forwarding a non-encrypted operand straight to + // `q.contains`. A blanket `value: string` made that unreachable from + // TypeScript: the operand type must follow the column. + it('accepts native containment operands on a plaintext key', () => { + mixedBuilder.contains('tags', ['vip']) + mixedBuilder.contains('meta', { plan: 'pro' }) + mixedBuilder.contains('tags', 'vip') + }) + + it('still pins an encrypted text-search operand to string', () => { + mixedBuilder.contains('email', 'ada') + // @ts-expect-error — email is public.text_search: the match term is a string + mixedBuilder.contains('email', ['ada']) + // @ts-expect-error — bio is public.text_match: the match term is a string + mixedBuilder.contains('bio', { a: 1 }) + }) + it('does not expose like/ilike on the v3 builder, at any chain depth', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { schemas: { users }, @@ -192,6 +236,21 @@ describe('encryptedSupabaseV3 untyped surface (no schemas)', () => { builder.ilike('email', '%ada%') }) + // No `schemas` means no capability information, so nothing here can tell an + // encrypted match column from a plaintext jsonb one. The operand must accept + // both — the runtime decides which by looking the column up. + it('accepts native containment operands, having no capability info to narrow with', async () => { + const supabase = await encryptedSupabaseV3(supabaseClient) + const builder = supabase.from<{ + id: number + tags: string[] + meta: Record + }>('users') + builder.contains('tags', ['vip']) + builder.contains('meta', { plan: 'pro' }) + builder.contains('tags', 'vip') + }) + it('keeps like/ilike on the v2 builder', () => { const v2Users = v2EncryptedTable('users', { email: encryptedColumn('email').freeTextSearch(), diff --git a/packages/stack/src/eql/v3/domain-registry.ts b/packages/stack/src/eql/v3/domain-registry.ts index 01bc01ba4..b99a8fb67 100644 --- a/packages/stack/src/eql/v3/domain-registry.ts +++ b/packages/stack/src/eql/v3/domain-registry.ts @@ -5,11 +5,19 @@ import { types } from './types' export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column /** - * Unqualified Postgres `domain_name` → the existing `types` factory. Values are - * the `eql/v3/types.ts` factories (which pass the literal domain constants), - * NOT direct `new EncryptedXColumn(...)` calls — the constant carried by each - * factory is what keeps the domains nominally distinct. `TextSearch` has a - * different arity, so this is a value map, not a mechanical transform. + * Unqualified Postgres `domain_name` → the `eql/v3/types.ts` factory that + * builds that domain's column. + * + * DERIVED, not hand-listed. Every factory already carries its domain in the + * constant it passes to the column constructor, so the key is recoverable from + * the factory itself — and `types` and this registry cannot drift. Adding a + * domain to `types` is enough; forgetting an entry here is no longer possible. + * + * The keys are an external contract (they are the `information_schema` query + * parameter in `../../supabase/introspect.ts`). Because they are derived from + * `getEqlType()`, no test that also derives them can detect a corrupted domain + * constant — `eql-v3-domain-registry.test.ts` pins them against a hand-written + * literal list for exactly that reason. */ // NULL PROTOTYPE — load-bearing. A plain object literal inherits from // Object.prototype, so `DOMAIN_REGISTRY['constructor']` returns a *function* @@ -18,61 +26,21 @@ export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column // domain and "synthesized" from Object.prototype.constructor. `factoryForDomain` // additionally guards with `Object.hasOwn`; both are kept — belt and braces, // because a future refactor that drops the null prototype must not silently -// reopen the hole. -export const DOMAIN_REGISTRY: Record = Object.assign( - Object.create(null) as Record, - { - // integer - integer: types.Integer, - integer_eq: types.IntegerEq, - integer_ord_ore: types.IntegerOrdOre, - integer_ord: types.IntegerOrd, - // smallint - smallint: types.Smallint, - smallint_eq: types.SmallintEq, - smallint_ord_ore: types.SmallintOrdOre, - smallint_ord: types.SmallintOrd, - // bigint - bigint: types.Bigint, - bigint_eq: types.BigintEq, - bigint_ord_ore: types.BigintOrdOre, - bigint_ord: types.BigintOrd, - // date - date: types.Date, - date_eq: types.DateEq, - date_ord_ore: types.DateOrdOre, - date_ord: types.DateOrd, - // timestamp - timestamp: types.Timestamp, - timestamp_eq: types.TimestampEq, - timestamp_ord_ore: types.TimestampOrdOre, - timestamp_ord: types.TimestampOrd, - // numeric - numeric: types.Numeric, - numeric_eq: types.NumericEq, - numeric_ord_ore: types.NumericOrdOre, - numeric_ord: types.NumericOrd, - // text - text: types.Text, - text_eq: types.TextEq, - text_match: types.TextMatch, - text_ord_ore: types.TextOrdOre, - text_ord: types.TextOrd, - text_search: types.TextSearch, - // boolean - boolean: types.Boolean, - // real - real: types.Real, - real_eq: types.RealEq, - real_ord_ore: types.RealOrdOre, - real_ord: types.RealOrd, - // double - double: types.Double, - double_eq: types.DoubleEq, - double_ord_ore: types.DoubleOrdOre, - double_ord: types.DoubleOrd, - }, -) +// reopen the hole. It also makes the `Object.hasOwn` collision check below +// exact: on a plain object `'constructor' in registry` is spuriously true. +export const DOMAIN_REGISTRY: Record = (() => { + const registry = Object.create(null) as Record + for (const factory of Object.values(types) as V3ColumnFactory[]) { + // Probe name only: the constructors store their arguments and nothing else, + // so building one column per domain at module load is free of side effects. + const key = stripDomainSchema(factory('_probe').getEqlType()) + if (Object.hasOwn(registry, key)) { + throw new Error(`duplicate EQL v3 domain key: ${key}`) + } + registry[key] = factory + } + return registry +})() /** Strip a leading `public.` schema qualifier from a qualified `eqlType`. */ export function stripDomainSchema(eqlType: string): string { diff --git a/packages/stack/src/supabase/helpers.ts b/packages/stack/src/supabase/helpers.ts index 281f32851..5a700b5e3 100644 --- a/packages/stack/src/supabase/helpers.ts +++ b/packages/stack/src/supabase/helpers.ts @@ -260,18 +260,48 @@ export function parseOrString(orString: string): PendingOrCondition[] { return conditions } +/** + * PostgREST operator tokens whose operand is a CONTAINMENT literal — a + * Postgres array literal (`{vip,admin}`) or a jsonb literal (`{"a":1}`) — rather + * than a scalar or the `in`-list's `(a,b)`. + * + * `contains` is supabase-js's METHOD name for this operator; string-form `.or()` + * callers write PostgREST's `cs` directly. Both reach here. + */ +const CONTAINMENT_OPS = new Set(['contains', 'cs']) + +/** + * The PostgREST operator token for a {@link FilterOp}. + * + * `contains` is the only member of the union that is a supabase-js method name + * rather than a PostgREST operator: `eq`, `in`, `like`, `is` and the rest spell + * the same on both sides, but PostgREST's containment operator is `cs`, and + * `or=(tags.contains.vip)` is a PGRST100 parse error ("unexpected \"c\" + * expecting \"not\" or operator"). + * + * Applied unconditionally, NOT only to encrypted conditions. The token depends + * on the operator, never on whether the operand was encrypted — a plaintext + * jsonb/array column reached through `.or([{op: 'contains'}])` needs exactly the + * same translation, and gating it on encryption is what left plaintext + * containment broken while the encrypted path worked. + */ +function orOperatorToken(op: string): string { + return op === 'contains' ? 'cs' : op +} + /** * Rebuild an `.or()` string from structured conditions. */ export function rebuildOrString( conditions: DbPendingOrCondition[], ): DbFilterString { - // Callers must hand DB-space `c.column` values (see `transformOrConditions`). + // Callers must hand DB-space `c.column` values (see `toDbSpace`). return conditions .map((c) => { - const value = formatOrValue(c.value) - const op = c.negate ? `not.${c.op}` : c.op - return `${c.column}.${op}.${value}` + const op = orOperatorToken(c.op) + const value = formatOrValue(c.value, op) + const token = c.negate ? `not.${op}` : op + return `${c.column}.${token}.${value}` }) .join(',') as DbFilterString } @@ -301,10 +331,15 @@ function splitOrString(input: string): string[] { } else if (char === '"' && depth === 0) { inQuotes = !inQuotes current += char - } else if (char === '(' && !inQuotes) { + } else if ((char === '(' || char === '{') && !inQuotes) { + // `{` as well as `(`: a containment operand is an array (`{vip,admin}`) or + // a jsonb (`{"a":1,"b":2}`) literal, whose top-level commas are part of + // the value. PostgREST's own logic-tree parser tracks these braces; + // without them a condition splits mid-literal into `tags.cs.{vip` and a + // dotless fragment `admin}` that the loop below silently drops. depth++ current += char - } else if (char === ')' && !inQuotes) { + } else if ((char === ')' || char === '}') && !inQuotes) { depth-- current += char } else if (char === ',' && depth === 0 && !inQuotes) { @@ -370,7 +405,49 @@ function unescapeOrValue(str: string): string { return str.replace(/\\(.)/g, '$1') } -function formatOrValue(value: unknown): string { +/** + * Characters that force an ARRAY-literal element to be double-quoted. Wider + * than {@link POSTGREST_RESERVED} because the braces and whitespace that are + * harmless in a scalar operand are structural inside `{…}`. + */ +const ARRAY_ELEMENT_RESERVED = /[,"\\{}()\s]/ + +/** One element of a Postgres array literal. `NULL` is a keyword there, so a + * string that happens to spell it must be quoted to stay a string. */ +function arrayLiteralElement(value: unknown): string { + if (value === null || value === undefined) return 'NULL' + const str = String(value) + if (str === '' || ARRAY_ELEMENT_RESERVED.test(str) || /^null$/i.test(str)) { + return `"${escapeOrValue(str)}"` + } + return str +} + +/** + * The `cs` operand for a structured value: `{a,b}` for an array column, + * `{"a":1}` for a jsonb one. Returns null when `value` is a scalar, which takes + * the ordinary path — notably the v3 encrypted operand, already a + * `JSON.stringify`d envelope STRING, which must not be re-serialized. + */ +function containmentLiteral(value: unknown): string | null { + if (Array.isArray(value)) { + return `{${value.map((v) => arrayLiteralElement(v)).join(',')}}` + } + if (value !== null && typeof value === 'object' && !(value instanceof Date)) { + return JSON.stringify(value) + } + return null +} + +function formatOrValue(value: unknown, op?: string): string { + // A containment literal is a VALUE like any other once built: it goes on to + // the quoting below, because its comma would otherwise split the or-string at + // the top level. PostgREST accepts a quoted `"{vip,admin}"` inside `or=(…)`. + if (op !== undefined && CONTAINMENT_OPS.has(op)) { + const literal = containmentLiteral(value) + if (literal !== null) return formatOrValue(literal) + } + if (Array.isArray(value)) { return `(${value.map((v) => formatOrValue(v)).join(',')})` } @@ -389,3 +466,17 @@ function formatOrValue(value: unknown): string { return str } + +/** + * The operand for an `in`/`not.in` list: `(a,b)`, each element quoted and + * escaped exactly as the `or` path does. + * + * Required because postgrest-js's own `in()` wraps a comma-bearing element in + * `"…"` but never escapes the `"` already inside it — and every v3 encrypted + * operand is a `JSON.stringify`d envelope, so its quotes would terminate the + * value early. Emit this through `filter(col, 'in', …)` / `not(col, 'in', …)`, + * both of which forward the operand verbatim. + */ +export function formatInListOperand(values: readonly unknown[]): string { + return formatOrValue([...values]) +} diff --git a/packages/stack/src/supabase/query-builder-v3.ts b/packages/stack/src/supabase/query-builder-v3.ts index e48cc536c..60ca33f41 100644 --- a/packages/stack/src/supabase/query-builder-v3.ts +++ b/packages/stack/src/supabase/query-builder-v3.ts @@ -19,7 +19,6 @@ import { } from './query-builder' import type { DbName, - DbPendingOrCondition, DbSelect, FilterOp, SupabaseClientLike, @@ -447,28 +446,6 @@ export class EncryptedQueryBuilderV3Impl< return this.queryTypeForRawOp(op) } - /** - * Rewrite the structured form's `contains` to the PostgREST operator token - * `cs` before the or-string is rebuilt. String-form callers already write - * `cs` — PostgREST syntax — so those pass through untouched. - * - * Operator shaping stays here rather than in `toDbSpace` because it depends - * on `wasEncrypted`, which is only known after encryption. Column names - * arrive already in DB-space. - */ - protected override transformOrConditions( - conditions: DbPendingOrCondition[], - encryptedIndexes: Set, - ): DbPendingOrCondition[] { - return conditions.map((cond, j) => { - const op = - encryptedIndexes.has(j) && cond.op === 'contains' - ? ('cs' as FilterOp) - : cond.op - return op === cond.op ? cond : { ...cond, op } - }) - } - /** Rebuild `Date` values from the encrypt-config `cast_as` (date/timestamp), * mirroring the typed v3 client's decrypt-model path. */ protected override postprocessDecryptedRow( diff --git a/packages/stack/src/supabase/query-builder.ts b/packages/stack/src/supabase/query-builder.ts index 334bfbf17..5b2424287 100644 --- a/packages/stack/src/supabase/query-builder.ts +++ b/packages/stack/src/supabase/query-builder.ts @@ -16,6 +16,7 @@ import type { import { logger } from '@/utils/logger' import { addJsonbCasts, + formatInListOperand, getEncryptedColumnNames, isEncryptableTerm, isEncryptedColumn, @@ -596,6 +597,30 @@ export class EncryptedQueryBuilderImpl< const column = tableColumns[nf.column] if (!column) continue + if (nf.op === 'in') { + // Each element is its own ciphertext, exactly as the regular `in` and + // `or(… .in. …)` paths do. Encrypting the whole list as one value + // yields a filter that silently matches nothing. + if (!Array.isArray(nf.value)) { + throw new Error( + `not("${nf.column}", "in", …) on an encrypted column requires an array of values, ` + + `not a PostgREST list literal — each element must be encrypted separately`, + ) + } + for (let j = 0; j < nf.value.length; j++) { + if (!isEncryptableTerm(nf.op, nf.value[j])) continue + terms.push({ + value: nf.value[j] as JsPlaintext, + column, + table: this.schema, + queryType: mapFilterOpToQueryType(nf.op), + returnType: 'composite-literal', + }) + termMap.push({ source: 'not', notIndex: i, inIndex: j }) + } + continue + } + terms.push({ value: nf.value as JsPlaintext, column, @@ -749,9 +774,11 @@ export class EncryptedQueryBuilderImpl< } } - /** `encryptedIndexes` is deliberately NOT precomputed here — it stays derived - * at apply time from the substitution maps, so this pass never has to agree - * with the encryption predicate about which conditions were encrypted. */ + /** Column names only. Which conditions were encrypted is never decided here: + * it stays derived at apply time from the substitution maps, so this pass + * never has to agree with the encryption predicate. The operator token is + * settled later still, in `rebuildOrString`, where `contains` becomes `cs` + * for encrypted and plaintext conditions alike. */ private orFilterToDbSpace(of_: PendingOrFilter): DbPendingOrFilter { const toDbCondition = (c: PendingOrCondition): DbPendingOrCondition => ({ ...c, @@ -903,6 +930,7 @@ export class EncryptedQueryBuilderImpl< const filterInMap = new Map() // "filterIndex:inIndex" -> value const matchValueMap = new Map() // "matchIndex:column" -> value const notValueMap = new Map() + const notInMap = new Map() // "notIndex:inIndex" -> value const rawValueMap = new Map() const orStringConditionMap = new Map() // "orIndex:condIndex" -> value const orStructuredConditionMap = new Map() @@ -926,7 +954,11 @@ export class EncryptedQueryBuilderImpl< matchValueMap.set(`${mapping.matchIndex}:${mapping.column}`, encValue) break case 'not': - notValueMap.set(mapping.notIndex, encValue) + if (mapping.inIndex !== undefined) { + notInMap.set(`${mapping.notIndex}:${mapping.inIndex}`, encValue) + } else { + notValueMap.set(mapping.notIndex, encValue) + } break case 'raw': rawValueMap.set(mapping.rawIndex, encValue) @@ -990,7 +1022,15 @@ export class EncryptedQueryBuilderImpl< q = q.is(column, value) break case 'in': - q = q.in(column, value as unknown[]) + // `wasEncrypted` above is false for in-lists: their ciphertexts land + // in `filterInMap`, keyed per element. + q = this.applyInFilter( + q, + column, + value as unknown[], + Array.isArray(f.value) && + f.value.some((_, j) => filterInMap.has(`${i}:${j}`)), + ) break } } @@ -1013,6 +1053,15 @@ export class EncryptedQueryBuilderImpl< // Apply not filters for (let i = 0; i < dbSpace.notFilters.length; i++) { const nf = dbSpace.notFilters[i] + + if (nf.op === 'in' && Array.isArray(nf.value)) { + const values = nf.value.map((v, j) => + notInMap.has(`${i}:${j}`) ? notInMap.get(`${i}:${j}`) : v, + ) + q = q.not(nf.column, 'in', formatInListOperand(values)) + continue + } + const wasEncrypted = notValueMap.has(i) const value = wasEncrypted ? notValueMap.get(i) : nf.value q = q.not(nf.column, this.notFilterOperator(nf.op, wasEncrypted), value) @@ -1025,35 +1074,28 @@ export class EncryptedQueryBuilderImpl< if (of_.kind === 'string') { // Already parsed (once) and translated by `toDbSpace`. const parsed = [...of_.conditions] - const encryptedIndexes = new Set() for (let j = 0; j < parsed.length; j++) { const sub = substituteOrValue(orStringConditionMap, i, j, parsed[j]) if (sub) { parsed[j] = { ...parsed[j], value: sub.value } - encryptedIndexes.add(j) } } // Rebuild whenever a condition REFERENCES an encrypted column — not // merely when a value was encrypted. An `is`/null operand on an - // encrypted column encrypts nothing, so keying on `encryptedIndexes` - // would send that condition down the verbatim path below and forward - // the caller's JS property name to a DB that only knows the column's - // real name. `toDbSpace` has already translated `parsed`. + // encrypted column encrypts nothing, so keying on "was a value + // substituted" would send that condition down the verbatim path below + // and forward the caller's JS property name to a DB that only knows the + // column's real name. `toDbSpace` has already translated `parsed`. const referencesEncrypted = parsed.some((c) => isEncryptedColumn(c.column, this.encryptedColumnNames), ) if (referencesEncrypted) { - q = q.or( - rebuildOrString( - this.transformOrConditions(parsed, encryptedIndexes), - ), - { - referencedTable: of_.referencedTable, - }, - ) + q = q.or(rebuildOrString(parsed), { + referencedTable: of_.referencedTable, + }) } else { // Every condition names a plaintext column, whose property name IS // its DB name — nothing to map. Forward the caller's ORIGINAL string @@ -1065,21 +1107,12 @@ export class EncryptedQueryBuilderImpl< } } else { // Structured: convert to string - const encryptedIndexes = new Set() const conditions = of_.conditions.map((cond, j) => { const sub = substituteOrValue(orStructuredConditionMap, i, j, cond) - if (sub) { - encryptedIndexes.add(j) - return { ...cond, value: sub.value } - } - return cond + return sub ? { ...cond, value: sub.value } : cond }) - q = q.or( - rebuildOrString( - this.transformOrConditions(conditions, encryptedIndexes), - ), - ) + q = q.or(rebuildOrString(conditions)) } } @@ -1157,6 +1190,26 @@ export class EncryptedQueryBuilderImpl< return 'equality' } + /** + * Apply an `in` filter. + * + * A plaintext list goes to postgrest-js's `in()`, which quotes elements that + * contain `,()`. An ENCRYPTED list cannot: every element is a + * `JSON.stringify`d envelope, and `in()` wraps it in `"…"` without escaping + * the quotes inside it, so PostgREST terminates the value at the envelope's + * first `"`. Emit the operand ourselves and hand it to `filter()`, which + * forwards it verbatim. + */ + protected applyInFilter( + q: SupabaseQueryBuilder, + column: DbName, + values: unknown[], + wasEncrypted: boolean, + ): SupabaseQueryBuilder { + if (!wasEncrypted) return q.in(column, values) + return q.filter(column, 'in', formatInListOperand(values)) + } + /** * Apply a `like`/`ilike` filter. v2 relies on the `~~` operator defined on * `eql_v2_encrypted`; the v3 dialect overrides this for encrypted columns @@ -1207,18 +1260,6 @@ export class EncryptedQueryBuilderImpl< return op } - /** - * Transform `.or()` conditions before the or-string is rebuilt. The v3 - * dialect maps property names to DB names and `like`/`ilike` on encrypted - * conditions to `cs`. - */ - protected transformOrConditions( - conditions: DbPendingOrCondition[], - _encryptedIndexes: Set, - ): DbPendingOrCondition[] { - return conditions - } - /** * Post-process a decrypted result row. The v3 dialect reconstructs `Date` * values from the encrypt-config `cast_as`; v2 returns rows unchanged. @@ -1389,7 +1430,7 @@ export class EncryptedQueryBuilderImpl< type TermMapping = | { source: 'filter'; filterIndex: number; inIndex?: number } | { source: 'match'; matchIndex: number; column: string } - | { source: 'not'; notIndex: number } + | { source: 'not'; notIndex: number; inIndex?: number } | { source: 'raw'; rawIndex: number } | { source: 'or-string' diff --git a/packages/stack/src/supabase/types.ts b/packages/stack/src/supabase/types.ts index 70ea544fc..b58e2b302 100644 --- a/packages/stack/src/supabase/types.ts +++ b/packages/stack/src/supabase/types.ts @@ -120,6 +120,40 @@ export type V3FreeTextSearchableKeys< Row extends Record, > = Exclude, NonFreeTextSearchV3Keys> +/** + * The operand `contains()` accepts on a PLAINTEXT column, mirroring + * postgrest-js's own untyped `contains` overload: a jsonb literal, an array, or + * the raw string form. + * + * Deliberately NOT `ReadonlyArray` (postgrest-js's *typed* overload): + * for `tags: string[]` that resolves to `string[][]` and rejects the very call + * it exists to allow, `contains('tags', ['vip'])`. + */ +type NativeContainsValue = string | readonly unknown[] | Record + +/** + * The `contains()` operand for column `K`. + * + * A DECLARED encrypted column reaching `contains` necessarily carries a + * `freeTextSearch` capability — only `public.text_match` and + * `public.text_search` do, and both `cast_as` to `string` — so its operand is + * the string to tokenize into a bloom-filter query term. + * + * Any other key is a plaintext passthrough, where the runtime forwards the + * operand untouched to `q.contains` and `contains` means PostgREST's native + * jsonb/array containment. A blanket `value: string` made that half of + * {@link V3FreeTextSearchableKeys} unreachable from TypeScript. + * + * `[K] extends [...]` rather than a bare conditional: a naked type parameter + * distributes over unions, which would silently widen a union key to the union + * of both operand types instead of demanding every member be a declared column. + */ +export type V3ContainsValue
= [ + K, +] extends [Extract, string>] + ? string + : NativeContainsValue + /** * Row keys a v3 builder accepts in `order()`: every row key that is NOT an * encrypted v3 column. `ORDER BY` on an EQL v3 domain sorts the raw ciphertext @@ -156,7 +190,7 @@ export interface EncryptedQueryBuilderV3< > { contains & StringKeyOf>( column: K, - value: string, + value: V3ContainsValue, ): EncryptedQueryBuilderV3 } @@ -166,6 +200,11 @@ export interface EncryptedQueryBuilderV3< * runtime guard in the term-encryption path is the only protection — but the * DIALECT is still v3, so `like`/`ilike` are absent here too. Typing this as * {@link EncryptedQueryBuilder} would hand back the v2 surface. + * + * For the same reason nothing here can tell an encrypted match column from a + * plaintext jsonb one, so `contains` accepts the full native operand union + * (which subsumes the encrypted column's `string`); the runtime resolves the + * column and picks the encoding. */ export interface EncryptedQueryBuilderV3Untyped< Row extends Record, @@ -176,7 +215,7 @@ export interface EncryptedQueryBuilderV3Untyped< > { contains>( column: K, - value: string, + value: NativeContainsValue, ): EncryptedQueryBuilderV3Untyped } @@ -573,7 +612,23 @@ export interface EncryptedQueryBuilderCore< gte(column: K, value: T[K]): Self lt(column: K, value: T[K]): Self lte(column: K, value: T[K]): Self + /** + * `IS NULL` / `IS TRUE` / `IS FALSE`. + * + * The `null` form is widened to EVERY row key, not just the filterable ones. + * `is` is the one predicate never encrypted — `isEncryptableTerm` rejects it + * outright, so no term is collected and no capability guard runs — and a NULL + * plaintext is stored as a SQL NULL rather than a ciphertext. On a v3 + * storage-only column (`types.Boolean`, `types.Integer`, …) `IS NULL` is + * therefore not merely legal but the ONLY predicate available, so narrowing it + * to `FK` would deny the sole query those columns support. + * + * The boolean form stays on `FK`: `IS TRUE` against a jsonb ciphertext column + * compares an envelope to a plaintext boolean, which is a type error in the + * database, not a filter. + */ is(column: K, value: null | boolean): Self + is>(column: K, value: null): Self in(column: K, values: T[K][]): Self filter(column: K, operator: string, value: T[K]): Self not(column: K, operator: string, value: T[K]): Self From 8cd485d246b6f5dc4f92ed019329626163ed4b8e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 22:51:20 +1000 Subject: [PATCH 2/4] fix(stack): correct the supabase .or() string parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An `.or()` string is only rebuilt from its parse when it references an encrypted column — otherwise the caller's string is forwarded verbatim — so each of these corrupts precisely the mixed encrypted/plaintext case. Quotes were tracked only at brace depth 0, so a `}` inside a quoted array element or jsonb string value closed the literal early and the next `"` re-opened quoting: the following top-level comma never split and its condition was absorbed into the operand. Quotes are now opaque at every depth. A stray `}` or `)` drove the depth counter negative, after which no comma split again. Neither is a PostgREST reserved character, so `a}b` is a valid unquoted operand. Depth now floors at zero. `in`-list elements were split on every comma, ignoring quotes, and the quotes were left embedded in the fragments. On an encrypted column each fragment became its own term, so `in.("Doe, John",Smith)` never matched. Elements are now split on top-level commas and unquoted — the inverse of what `rebuildOrString` emits. A parenthesized operand was read as a list for every operator, so `eq.(foo)` encrypted a JS array rather than the string. Only `in` and the range operators take a paren-delimited operand. A string operand spelling `null`/`true`/`false` is now quoted: PostgREST reads a bare `null` as SQL NULL. Finally, `contains(col, …)` on a union key spanning an encrypted and a plaintext column accepted an array or object. A union is only as permissive as its strictest member; any declared column in it pins the operand to `string`. A literal column argument was never affected. --- .changeset/supabase-or-string-parser.md | 19 +++ .../stack/__tests__/supabase-helpers.test.ts | 154 ++++++++++++++++++ .../stack/__tests__/supabase-v3.test-d.ts | 23 +++ packages/stack/src/supabase/helpers.ts | 88 ++++++++-- packages/stack/src/supabase/types.ts | 20 ++- 5 files changed, 281 insertions(+), 23 deletions(-) create mode 100644 .changeset/supabase-or-string-parser.md diff --git a/.changeset/supabase-or-string-parser.md b/.changeset/supabase-or-string-parser.md new file mode 100644 index 000000000..25db66e67 --- /dev/null +++ b/.changeset/supabase-or-string-parser.md @@ -0,0 +1,19 @@ +--- +'@cipherstash/stack': patch +--- + +Fix the Supabase adapter's `.or()` string parser mis-splitting conditions, and pin `contains()` on a mixed union column key to the encrypted operand. + +An `.or()` string is only rebuilt from its parse when it references an encrypted column — otherwise the caller's string is forwarded verbatim — so each of these corrupts precisely the mixed encrypted/plaintext case. + +**Quotes were tracked only at brace depth 0.** A `}` inside a quoted array element or jsonb string value closed the literal early, and the next `"` re-opened quoting, so the following top-level comma never split: `.or('tags.cs.{"a}b"},email.eq.secret')` parsed as a single condition and silently absorbed `email.eq.secret` into the operand. Quotes are now opaque at every depth. + +**A stray `}` or `)` drove the depth counter negative**, after which no comma split again. `}` and `)` are not PostgREST reserved characters, so `a}b` is a valid unquoted operand and `.or('nickname.eq.a}b,id.eq.1')` dropped `id.eq.1`. Depth now floors at zero. + +**`in`-list elements were split on every comma, ignoring quotes.** `.or('email.in.("a,b",c)')` parsed as three elements with the quotes still embedded; on an encrypted column each fragment was encrypted as its own term, so the intended element never matched. Elements are now split on top-level commas and unquoted, the inverse of what the rebuild emits. + +**A parenthesized operand was read as a list for every operator.** Only `in` and the range operators (`ov`, `sl`, `sr`, `nxr`, `nxl`, `adj`) take a paren-delimited operand; elsewhere `(` is an ordinary character. `email.eq.(foo)` parsed as `['foo']` and encrypted a JS array rather than the string, matching nothing. + +**A string operand spelling `null`, `true` or `false` is now quoted.** PostgREST reads a bare `null` as SQL NULL, so `.or([{ column: 'name', op: 'eq', value: 'null' }])` emitted `name.eq.null` and compared against NULL instead of the three-character string. + +**`contains(col, …)` where `col` is a union spanning an encrypted and a plaintext column** accepted an array or object operand. The union is now only as permissive as its strictest member: any declared encrypted column in the union pins the operand to `string`. A literal column argument was never affected. diff --git a/packages/stack/__tests__/supabase-helpers.test.ts b/packages/stack/__tests__/supabase-helpers.test.ts index 2782897e3..18991e99b 100644 --- a/packages/stack/__tests__/supabase-helpers.test.ts +++ b/packages/stack/__tests__/supabase-helpers.test.ts @@ -256,6 +256,160 @@ describe('parseOrString containment literals', () => { }) }) +// A quoted operand is opaque at EVERY depth, and a stray brace or paren in an +// unquoted value is a literal character, not structure. Tracking quotes only at +// depth 0 let a `}` inside a quoted array element close the literal early; an +// unmatched `}` in a plain value drove the depth counter negative, after which no +// top-level comma ever split again. Both silently absorb the following condition +// into the preceding operand — and only or-strings that also reference an +// encrypted column are rebuilt from the parse, so it corrupts precisely the +// mixed encrypted/plaintext case. +describe('parseOrString structural characters inside values', () => { + it('does not close an array literal on a brace inside a quoted element', () => { + expect(parseOrString('tags.cs.{"a}b"},email.eq.secret')).toEqual([ + { column: 'tags', op: 'cs', negate: false, value: '{"a}b"}' }, + { column: 'email', op: 'eq', negate: false, value: 'secret' }, + ]) + }) + + it('does not close a jsonb literal on a brace inside a quoted value', () => { + expect(parseOrString('meta.cs.{"a":"v}"},id.eq.1')).toEqual([ + { column: 'meta', op: 'cs', negate: false, value: '{"a":"v}"}' }, + { column: 'id', op: 'eq', negate: false, value: '1' }, + ]) + }) + + // Every structural character, quoted as a jsonb VALUE, with the encrypted + // column ahead of the literal and a plaintext condition behind it — the shape + // that actually reaches `rebuildOrString`, since the encrypted `email` is what + // forces the group to be rebuilt rather than forwarded verbatim. + it.each([ + '}', + '{', + ')', + '(', + ])('keeps a quoted %s inside a jsonb literal out of the depth count', (char) => { + expect( + parseOrString(`email.eq.x,meta.cs.{"a":"${char}"},note.eq.y`), + ).toEqual([ + { column: 'email', op: 'eq', negate: false, value: 'x' }, + { column: 'meta', op: 'cs', negate: false, value: `{"a":"${char}"}` }, + { column: 'note', op: 'eq', negate: false, value: 'y' }, + ]) + }) + + it('keeps an escaped quote inside a jsonb value opaque', () => { + // `\"` must not close the element, or the `}` behind it decrements depth. + expect(parseOrString('a.eq.1,meta.cs.{"a":"\\"}"},b.eq.2')).toHaveLength(3) + }) + + it('splits after an unmatched brace in an unquoted value', () => { + // `}` is not a PostgREST reserved character, so `a}b` is a valid unquoted + // scalar operand. + expect(parseOrString('nickname.eq.a}b,id.eq.1')).toEqual([ + { column: 'nickname', op: 'eq', negate: false, value: 'a}b' }, + { column: 'id', op: 'eq', negate: false, value: '1' }, + ]) + }) + + it('splits after an unmatched paren in an unquoted value', () => { + expect(parseOrString('a.eq.x),b.eq.y')).toEqual([ + { column: 'a', op: 'eq', negate: false, value: 'x)' }, + { column: 'b', op: 'eq', negate: false, value: 'y' }, + ]) + }) +}) + +// An `in`-list element is quoted exactly like any other operand, so the list must +// be split on top-level commas and each element unquoted. Splitting the raw +// string on every comma tore `("a,b",c)` into three fragments and left the quotes +// embedded in them — on an encrypted column each fragment is encrypted as its own +// term, so the intended element never matches. +describe('parseOrString in-list elements', () => { + it('does not split on a comma inside a quoted element', () => { + expect(parseOrString('email.in.("a,b",c)')).toEqual([ + { column: 'email', op: 'in', negate: false, value: ['a,b', 'c'] }, + ]) + }) + + it('unescapes a quoted element', () => { + expect(parseOrString('a.in.("x\\"y",z)')).toEqual([ + { column: 'a', op: 'in', negate: false, value: ['x"y', 'z'] }, + ]) + }) + + it('round-trips a comma-bearing element through rebuild', () => { + const s = 'name.in.("Doe, John",Smith)' + expect(rebuildOrString(parseOrString(s) as DbPendingOrCondition[])).toBe(s) + }) + + it('round-trips an encrypted envelope element', () => { + const parsed = parseOrString( + rebuildOrString([cond('email', 'in', [ENVELOPE, 'x'])]), + ) + expect(parsed).toEqual([ + { column: 'email', op: 'in', negate: false, value: [ENVELOPE, 'x'] }, + ]) + }) + + it('splits a negated list on top-level commas only', () => { + expect(parseOrString('email.not.in.("a,b",c)')).toEqual([ + { column: 'email', op: 'in', negate: true, value: ['a,b', 'c'] }, + ]) + }) + + // Only the operators whose operand PostgREST delimits with parens take a list. + // A parenthesized operand anywhere else is a scalar that happens to start with + // `(`: parsed as an array, an encrypted `eq` operand is encrypted as a JS array + // rather than the intended string, and the filter matches nothing. + it('does not read a parenthesized scalar as a list for a scalar operator', () => { + expect(parseOrString('email.eq.(foo)')).toEqual([ + { column: 'email', op: 'eq', negate: false, value: '(foo)' }, + ]) + }) + + // The range operators take a paren-delimited operand too. Excluding them would + // re-emit `period.ov.(1,10)` as a quoted scalar — a wire-format change on a + // plaintext column that merely shares an `.or()` with an encrypted one. + it.each([ + 'ov', + 'sl', + 'sr', + 'nxr', + 'nxl', + 'adj', + ])('round-trips a paren-delimited %s operand', (op) => { + const s = `period.${op}.(1,10)` + expect(parseOrString(s)).toEqual([ + { column: 'period', op, negate: false, value: ['1', '10'] }, + ]) + expect(rebuildOrString(parseOrString(s) as DbPendingOrCondition[])).toBe(s) + }) +}) + +// PostgREST reads a bare `null` / `true` / `false` operand as the SQL value, not +// as the string spelling it. A string operand that happens to spell one must be +// quoted, or `name.eq.null` compares against SQL NULL and matches nothing. +describe('rebuildOrString reserved words', () => { + it.each(['null', 'true', 'false'])('quotes the string %s', (word) => { + expect(rebuildOrString([cond('name', 'eq', word)])).toBe( + `name.eq."${word}"`, + ) + }) + + it('leaves the SQL values unquoted', () => { + expect(rebuildOrString([cond('a', 'is', null)])).toBe('a.is.null') + expect(rebuildOrString([cond('a', 'is', true)])).toBe('a.is.true') + expect(rebuildOrString([cond('a', 'is', false)])).toBe('a.is.false') + }) + + it('quotes a reserved word inside an in-list', () => { + expect(rebuildOrString([cond('a', 'in', ['null', 'x'])])).toBe( + 'a.in.("null",x)', + ) + }) +}) + describe('parseOrString negation', () => { it('lifts a not. prefix off the operator', () => { expect(parseOrString('nickname.not.eq.ada')).toEqual([ diff --git a/packages/stack/__tests__/supabase-v3.test-d.ts b/packages/stack/__tests__/supabase-v3.test-d.ts index 9b598a816..f8c4698b2 100644 --- a/packages/stack/__tests__/supabase-v3.test-d.ts +++ b/packages/stack/__tests__/supabase-v3.test-d.ts @@ -33,6 +33,11 @@ declare const mixedBuilder: EncryptedQueryBuilderV3< UserRow & { tags: string[]; meta: Record } > +/** A column key that is a UNION spanning an encrypted and a plaintext column. */ +declare const mixedKey: 'email' | 'tags' +/** A column key that is a union of plaintext columns only. */ +declare const plaintextKey: 'tags' | 'meta' + describe('encryptedSupabaseV3 typed surface (with schemas)', () => { it('rows carry each column its domain plaintext type', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { @@ -144,6 +149,24 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { mixedBuilder.contains('bio', { a: 1 }) }) + // A union column key is only as permissive as its STRICTEST member. If any + // member is a declared encrypted column the operand must be the string term: + // that member's runtime path hands the operand to `encrypt()`, which has no + // plaintext-type guard, so an array reaches protect-ffi as the plaintext for a + // `cast_as: text` column. + it('pins a mixed union key to the encrypted string operand', () => { + mixedBuilder.contains(mixedKey, 'ada') + // @ts-expect-error — the union includes email (public.text_search) + mixedBuilder.contains(mixedKey, ['vip']) + // @ts-expect-error — the union includes email (public.text_search) + mixedBuilder.contains(mixedKey, { plan: 'pro' }) + }) + + it('leaves a union of plaintext keys on the native operand', () => { + mixedBuilder.contains(plaintextKey, ['vip']) + mixedBuilder.contains(plaintextKey, { plan: 'pro' }) + }) + it('does not expose like/ilike on the v3 builder, at any chain depth', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { schemas: { users }, diff --git a/packages/stack/src/supabase/helpers.ts b/packages/stack/src/supabase/helpers.ts index 5a700b5e3..a6895bafd 100644 --- a/packages/stack/src/supabase/helpers.ts +++ b/packages/stack/src/supabase/helpers.ts @@ -220,7 +220,7 @@ export function mapFilterOpToQueryType(op: FilterOp): QueryTypeName { export function parseOrString(orString: string): PendingOrCondition[] { const conditions: PendingOrCondition[] = [] // Split on commas that are not inside parentheses (nested or/and) - const parts = splitOrString(orString) + const parts = splitTopLevel(orString) for (const part of parts) { const trimmed = part.trim() @@ -252,7 +252,7 @@ export function parseOrString(orString: string): PendingOrCondition[] { const value = rest.slice(secondDot + 1) // Handle special value formats - const parsedValue = parseOrValue(value) + const parsedValue = parseOrValue(value, op) conditions.push({ column, op, negate, value: parsedValue }) } @@ -310,7 +310,22 @@ export function rebuildOrString( // Internal helpers // --------------------------------------------------------------------------- -function splitOrString(input: string): string[] { +/** + * Split on the commas that separate top-level tokens, leaving those inside a + * quoted operand or a `(…)` / `{…}` literal alone. + * + * Quotes are tracked at EVERY depth. A quoted string is opaque wherever it + * appears, and an array literal quotes any element carrying a reserved character + * (see {@link arrayLiteralElement}) — so `{"a}b"}` closes at the LAST brace, not + * the one inside the element. Tracking quotes only at depth 0 ended the literal + * early and swallowed the following condition into this operand. + * + * `depth` never goes below zero. `}` and `)` are not PostgREST reserved + * characters, so `a}b` is a valid unquoted scalar; letting a stray one decrement + * past zero meant no later comma ever split, silently absorbing every remaining + * condition. + */ +function splitTopLevel(input: string): string[] { const parts: string[] = [] let current = '' let depth = 0 @@ -328,7 +343,7 @@ function splitOrString(input: string): string[] { } else if (char === '\\' && inQuotes) { escaped = true current += char - } else if (char === '"' && depth === 0) { + } else if (char === '"') { inQuotes = !inQuotes current += char } else if ((char === '(' || char === '{') && !inQuotes) { @@ -340,7 +355,7 @@ function splitOrString(input: string): string[] { depth++ current += char } else if ((char === ')' || char === '}') && !inQuotes) { - depth-- + depth = Math.max(0, depth - 1) current += char } else if (char === ',' && depth === 0 && !inQuotes) { parts.push(current) @@ -350,14 +365,43 @@ function splitOrString(input: string): string[] { } } - if (current) { - parts.push(current) - } + parts.push(current) return parts } -function parseOrValue(value: string): unknown { +/** + * One element of an `in`-list operand, undoing {@link formatOrValue}'s quoting. + * Unquoted elements are trimmed, as PostgREST ignores whitespace around them; + * inside quotes it is significant. + */ +function parseInListElement(element: string): string { + const trimmed = element.trim() + if (trimmed.length >= 2 && trimmed.startsWith('"') && trimmed.endsWith('"')) { + return unescapeOrValue(trimmed.slice(1, -1)) + } + return trimmed +} + +/** + * PostgREST operators whose operand is delimited by parentheses: the `in` list + * and the range operators. Everywhere else `(` is an ordinary character, and a + * parenthesized operand is a scalar that happens to start with one. + * + * The range operators earn their place by round-trip fidelity rather than by + * encryption: none is supported on an encrypted column, but an or-string is + * rebuilt whole as soon as ANY of its conditions names one, so a plaintext + * `period.ov.(1,10)` sharing the group must re-emit byte-for-byte. + */ +const PAREN_OPERAND_OPS = new Set(['in', 'ov', 'sl', 'sr', 'nxr', 'nxl', 'adj']) + +/** + * @param op the operator the value belongs to, already stripped of any `not.` + * prefix. Parsing a parenthesized scalar as an array meant an encrypted `eq` + * operand was encrypted as a JS array rather than the intended string, and the + * filter matched nothing. + */ +function parseOrValue(value: string, op?: string): unknown { // Handle double-quoted values (PostgREST quoting for reserved characters). // Must undo `escapeOrValue`, or a parse → rebuild round-trip doubles every // backslash. The two functions are only correct as a pair. @@ -365,12 +409,16 @@ function parseOrValue(value: string): unknown { return unescapeOrValue(value.slice(1, -1)) } - // Handle parenthesized lists: (val1,val2,val3) - if (value.startsWith('(') && value.endsWith(')')) { - return value - .slice(1, -1) - .split(',') - .map((v) => v.trim()) + // Handle parenthesized lists: (val1,val2,val3). Elements are quoted exactly as + // any other operand, so the split must respect those quotes: `("a,b",c)` is + // two elements, not three. + if ( + op !== undefined && + PAREN_OPERAND_OPS.has(op) && + value.startsWith('(') && + value.endsWith(')') + ) { + return splitTopLevel(value.slice(1, -1)).map(parseInListElement) } // Handle booleans @@ -395,6 +443,14 @@ function parseOrValue(value: string): unknown { */ const POSTGREST_RESERVED = /["\\,().]/ +/** + * Operands PostgREST reads as SQL values rather than as the string spelling + * them. A STRING operand that happens to spell one must be quoted, or + * `name.eq.null` compares against SQL NULL — a filter that matches nothing — + * instead of against the three-character string. + */ +const POSTGREST_RESERVED_WORDS = new Set(['null', 'true', 'false']) + /** Escape `\` first, then `"` — the reverse order would double-escape. */ function escapeOrValue(str: string): string { return str.replace(/\\/g, '\\\\').replace(/"/g, '\\"') @@ -460,7 +516,7 @@ function formatOrValue(value: unknown, op?: string): string { // Wrap in double quotes if the value contains reserved characters. // This is required for encrypted values (JSON with commas, braces, etc.) // and is safe for all string values per PostgREST spec. - if (POSTGREST_RESERVED.test(str)) { + if (POSTGREST_RESERVED.test(str) || POSTGREST_RESERVED_WORDS.has(str)) { return `"${escapeOrValue(str)}"` } diff --git a/packages/stack/src/supabase/types.ts b/packages/stack/src/supabase/types.ts index b58e2b302..8b00f527d 100644 --- a/packages/stack/src/supabase/types.ts +++ b/packages/stack/src/supabase/types.ts @@ -144,15 +144,21 @@ type NativeContainsValue = string | readonly unknown[] | Record * jsonb/array containment. A blanket `value: string` made that half of * {@link V3FreeTextSearchableKeys} unreachable from TypeScript. * - * `[K] extends [...]` rather than a bare conditional: a naked type parameter - * distributes over unions, which would silently widen a union key to the union - * of both operand types instead of demanding every member be a declared column. + * A UNION key is only as permissive as its strictest member: if ANY member is a + * declared column, the operand is the string term. `[K] extends [declared]` gets + * this backwards — a mixed `'email' | 'tags'` fails that test and falls to + * `NativeContainsValue`, so an array typechecks for a key that may resolve to + * the encrypted `email` at runtime. Nothing downstream catches it: the operand + * goes straight to `encrypt()`, which has no plaintext-type guard. + * + * So test the INTERSECTION instead, wrapped in a tuple to stop the naked type + * parameter distributing (which would rebuild the same permissive union). */ export type V3ContainsValue
= [ - K, -] extends [Extract, string>] - ? string - : NativeContainsValue + Extract, string>>, +] extends [never] + ? NativeContainsValue + : string /** * Row keys a v3 builder accepts in `order()`: every row key that is NOT an From 55d96461751f413d53c0f80bec99f327df808a20 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:45:22 +1000 Subject: [PATCH 3/4] fix(stack): close the audited v3 supabase review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes every finding from the PR #609 review that survived verification against HEAD. Each was reproduced with a failing test before the fix. `.or()` dropped a condition after an unbalanced opening brace or paren. The depth floor added in 8cd485d2 recovers from a stray CLOSING brace, but a stray OPENING one strands the counter above zero and no later comma ever splits, swallowing every condition behind it. With a plaintext column first, `referencesEncrypted` then reads false over the surviving conditions and the group is forwarded verbatim — running the swallowed condition against a ciphertext column with a plaintext operand. Braces are now quoted on emit (PostgREST treats them as structure inside `or=(…)`, so an unquoted scalar brace was malformed on the wire regardless), and `splitTopLevel` re-splits honouring quotes alone when its depth tracking does not balance. Containment literals keep the narrow reserved set, so `tags.cs.{vip}` stays unquoted. Direct `contains()` / `not(col, 'contains', …)` did not serialize their operand. postgrest-js joins array elements on `,` without quoting, so `contains('tags', ['with,comma'])` reached Postgres as two elements; its `not()` interpolates with `String(value)`, emitting `not.contains.with,comma` — no braces, wrong operator token — and `[object Object]` for a jsonb operand. Both now build the same containment literal the `.or()` path builds, and emit `cs`. `is(col, true)` compiled on queryable encrypted columns. The boolean overload was gated on the filterable keys, which exclude only the storage-only columns, so a `text_search`/`text_eq`/`*_ord` column matched it and emitted `IS TRUE` against a jsonb ciphertext. It now takes a dedicated `BK` parameter, kept distinct from the orderable `OK` so the two capability axes can diverge. The type test asserted the bug; it now asserts the compile error. `contains()` admitted arrays on plaintext SCALAR columns, emitting `@>` on `text` (42883). The plaintext operand now follows the column's own shape, mapping scalars to `never`. It distributes over `Row[K]`, which is sound only because the tuple guard already excludes every encrypted member; the residual case is documented. In-list operands spent one ZeroKMS crossing per element. Terms are now grouped by column — mandatory, since `bulkEncrypt` carries one `{table, column}` for the whole payload — and each group takes a single call, scattered back onto its original indices. Mirrors the Drizzle v3 path, keeps a per-term fallback, and rejects a length mismatch rather than silently truncating the predicate. `types` is now `satisfies Record`. The derived `DOMAIN_REGISTRY` calls every value at module load, so a non-factory export would throw and take the supabase introspect/schema-build/verify path with it; that is now a compile error at the offending line. The blind `Object.values(types) as V3ColumnFactory[]` cast that would have silenced it is gone. In-list term collection is unified behind `collectInListTerms` — the three copies had already drifted once, leaving the `not` path unfixed. --- .changeset/supabase-in-list-operands.md | 44 ++++- .../stack/__tests__/helpers/supabase-mock.ts | 13 ++ .../stack/__tests__/supabase-helpers.test.ts | 50 +++++ .../__tests__/supabase-v3-builder.test.ts | 159 +++++++++++++++ .../stack/__tests__/supabase-v3-wire.test.ts | 66 +++++++ .../stack/__tests__/supabase-v3.test-d.ts | 41 +++- packages/stack/src/eql/v3/columns.ts | 9 + packages/stack/src/eql/v3/domain-registry.ts | 14 +- packages/stack/src/eql/v3/types.ts | 8 +- packages/stack/src/supabase/helpers.ts | 73 ++++++- .../stack/src/supabase/query-builder-v3.ts | 168 ++++++++++++---- packages/stack/src/supabase/query-builder.ts | 186 +++++++++++------- packages/stack/src/supabase/types.ts | 75 +++++-- 13 files changed, 749 insertions(+), 157 deletions(-) diff --git a/.changeset/supabase-in-list-operands.md b/.changeset/supabase-in-list-operands.md index 2753b5ea8..dfd1fb2e5 100644 --- a/.changeset/supabase-in-list-operands.md +++ b/.changeset/supabase-in-list-operands.md @@ -33,10 +33,42 @@ and a NULL plaintext is stored as a SQL NULL, so `IS NULL` is not merely legal there but the only predicate those columns support. `is(col, true)` remains a compile error on encrypted columns. -**`contains()` accepts native operands on plaintext columns.** A plaintext -jsonb/array column falls through to PostgREST's native containment, so +**`contains()` accepts native operands on plaintext array and jsonb columns.** A +plaintext jsonb/array column falls through to PostgREST's native containment, so `contains('tags', ['vip'])` and `contains('meta', { plan: 'pro' })` now -typecheck. Encrypted match columns still take a `string` token. Relatedly, -`.or([{ op: 'contains' }])` now emits PostgREST's `cs` operator for plaintext -columns too — previously only encrypted conditions were translated, so a -plaintext containment reached the wire as `.contains.` and failed to parse. +typecheck. A plaintext SCALAR column does not: `@>` is undefined on `text`, so +the operand type follows the column's own shape and a scalar rejects every +containment operand. Encrypted match columns still take a `string` token. +Relatedly, `.or([{ op: 'contains' }])` now emits PostgREST's `cs` operator for +plaintext columns too — previously only encrypted conditions were translated, so +a plaintext containment reached the wire as `.contains.` and failed to parse. + +**Direct `contains()` / `not(col, 'contains', …)` now serialize their operand.** +postgrest-js builds an array operand as `cs.{a,b}` with no element quoting, so +`contains('tags', ['with,comma'])` reached Postgres as two elements; and its +`not()` stringifies the operand outright, emitting `not.contains.with,comma` +(no braces, and the wrong operator token) or `[object Object]` for a jsonb +operand. Both paths now build the containment literal the `.or()` path already +built, and emit the `cs` token. + +**`.or()` no longer drops a condition after an unbalanced brace or paren.** A +scalar operand containing `{` left the parser's depth counter stranded above +zero, so no later comma separated a condition and everything behind it was +swallowed into that operand. With a plaintext column first, the group was then +forwarded verbatim — running the swallowed condition against a ciphertext column +with a plaintext operand. Braces are now quoted on emit (they are structural to +PostgREST inside `or=(…)`), and the parser falls back to quote-only splitting +when its depth tracking does not balance. + +**`is(col, true)` is now rejected on every encrypted column, not just the +storage-only ones.** The boolean form was gated on the filterable keys, which +exclude storage-only columns but keep queryable encrypted ones — so +`is(emailTextSearchColumn, true)` compiled and emitted `IS TRUE` against a jsonb +ciphertext. + +**In-list operands encrypt in one crossing per column.** The element-wise `in` / +`not.in` encoding above spent one ZeroKMS round-trip per element; terms are now +grouped by column and each group takes a single `bulkEncrypt` call, matching the +Drizzle v3 path. Falls back to per-term encryption for clients without +`bulkEncrypt`, and rejects a bulk response whose length does not match the list +rather than silently truncating the predicate. diff --git a/packages/stack/__tests__/helpers/supabase-mock.ts b/packages/stack/__tests__/helpers/supabase-mock.ts index 9c8acb474..e799170a9 100644 --- a/packages/stack/__tests__/helpers/supabase-mock.ts +++ b/packages/stack/__tests__/helpers/supabase-mock.ts @@ -83,6 +83,19 @@ export function createMockEncryptionClient() { encrypt: (value: unknown, opts: { column: { getName(): string } }) => operation(fakeEnvelope(value, opts.column.getName())), + // v3 filter path: one FFI crossing per (table, column) group. Position- + // stable and envelope-identical to `encrypt`, so every wire assertion holds + // whichever path the builder takes. + bulkEncrypt: ( + payloads: Array<{ plaintext: unknown }>, + opts: { column: { getName(): string } }, + ) => + operation( + payloads.map((p) => ({ + data: fakeEnvelope(p.plaintext, opts.column.getName()), + })), + ), + // v2 filter path: batch query terms as composite literals encryptQuery: (terms: Array<{ value: unknown }>) => operation(terms.map((t) => `("${String(t.value)}")`)), diff --git a/packages/stack/__tests__/supabase-helpers.test.ts b/packages/stack/__tests__/supabase-helpers.test.ts index 18991e99b..236834e3b 100644 --- a/packages/stack/__tests__/supabase-helpers.test.ts +++ b/packages/stack/__tests__/supabase-helpers.test.ts @@ -107,6 +107,17 @@ describe('rebuildOrString quoting', () => { it('leaves a value with no reserved characters unquoted', () => { expect(rebuildOrString([cond('a', 'eq', 'plain')])).toBe('a.eq.plain') }) + + // A brace is structural to PostgREST's own logic-tree parser inside `or=(…)`, + // so an unquoted `a{b` scalar is malformed on the wire — and it desynchronises + // our parser on the way back in. Emit and parse must agree on what is structure. + it('quotes a scalar value containing an opening brace', () => { + expect(rebuildOrString([cond('a', 'eq', 'a{b')])).toBe('a.eq."a{b"') + }) + + it('quotes a scalar value containing a closing brace', () => { + expect(rebuildOrString([cond('a', 'eq', 'a}b')])).toBe('a.eq."a}b"') + }) }) // --------------------------------------------------------------------------- @@ -214,6 +225,25 @@ describe('parseOrString / rebuildOrString round-trip', () => { expect(parsed[0].value).toBe(ENVELOPE) expect(parsed[1].value).toBe('7') }) + + // The emit side must never produce a string its own parser mis-reads. A scalar + // brace was the one character that escaped quoting, so rebuild → parse dropped + // the condition behind it. + it.each([ + 'a{b', + 'a}b', + 'a(b', + 'a)b', + ])('round-trips a scalar value containing %s', (value) => { + const conditions = [ + { column: 'note', op: 'eq', negate: false, value }, + { column: 'id', op: 'eq', negate: false, value: '7' }, + ] + const s = rebuildOrString( + conditions.map((c) => cond(c.column, c.op, c.value)), + ) + expect(parseOrString(s)).toEqual(conditions) + }) }) // --------------------------------------------------------------------------- @@ -318,6 +348,26 @@ describe('parseOrString structural characters inside values', () => { { column: 'b', op: 'eq', negate: false, value: 'y' }, ]) }) + + // The mirror image of the two cases above, and the one the depth floor cannot + // catch: an unmatched OPENING brace or paren leaves `depth` above zero for the + // rest of the string, so no later comma ever splits. Every following condition + // is swallowed into this operand. With a plaintext column first the group is + // then forwarded VERBATIM (nothing looks encrypted), so PostgREST runs the + // swallowed `email.eq.ada` with a plaintext operand against a ciphertext column. + it('splits after an unmatched opening brace in an unquoted value', () => { + expect(parseOrString('note.eq.a{b,email.eq.ada')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'a{b' }, + { column: 'email', op: 'eq', negate: false, value: 'ada' }, + ]) + }) + + it('splits after an unmatched opening paren in an unquoted value', () => { + expect(parseOrString('note.eq.a(b,email.eq.ada')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'a(b' }, + { column: 'email', op: 'eq', negate: false, value: 'ada' }, + ]) + }) }) // An `in`-list element is quoted exactly like any other operand, so the list must diff --git a/packages/stack/__tests__/supabase-v3-builder.test.ts b/packages/stack/__tests__/supabase-v3-builder.test.ts index 68d5acc88..8946aaf88 100644 --- a/packages/stack/__tests__/supabase-v3-builder.test.ts +++ b/packages/stack/__tests__/supabase-v3-builder.test.ts @@ -8,6 +8,7 @@ import { createMockSupabase, fakeEnvelope, isFakeEnvelope, + operation, } from './helpers/supabase-mock' // --------------------------------------------------------------------------- @@ -1653,3 +1654,161 @@ describe('v3 raw filter() resolves the query type from the operator', () => { expect(error?.message).toContain('not supported on scalar EQL v3 columns') }) }) + +// --------------------------------------------------------------------------- +// In-list encryption batching +// +// The element-wise `in`/`not.in` fix (each element its own term, so the list is +// never encrypted whole) fed N same-column terms into `encryptCollectedTerms`, +// which spent one ZeroKMS/FFI round-trip on each. `bulkEncrypt` takes ONE +// `{table, column}` for a whole list, so terms must be grouped by column before +// the crossing — a single bulk call over a multi-column term array would stamp +// one column onto every plaintext. +// --------------------------------------------------------------------------- + +describe('v3 in-list term encryption batches by column', () => { + function batchingInstance() { + const supabase = createMockSupabase() + const encryption = createMockEncryptionClient() + const bulkEncrypt = vi.spyOn( + encryption as unknown as { bulkEncrypt: (...a: unknown[]) => unknown }, + 'bulkEncrypt', + ) + const encrypt = vi.spyOn( + encryption as unknown as { encrypt: (...a: unknown[]) => unknown }, + 'encrypt', + ) + const builder = () => + new EncryptedQueryBuilderV3Impl( + 'users', + users, + encryption, + supabase.client, + USERS_ALL_COLUMNS, + ) + return { supabase, builder, bulkEncrypt, encrypt } + } + + /** The plaintext each emitted operand actually encrypts, in call order. */ + const plaintextsOf = (calls: { args: unknown[] }[], operandIndex: number) => + calls.map((c) => JSON.parse(c.args[operandIndex] as string).pt) + + it('spends one bulk crossing on an in-list, not one per element', async () => { + const { builder, bulkEncrypt, encrypt } = batchingInstance() + + await builder().select('id').in('nickname', ['ada', 'grace', 'hopper']) + + expect(bulkEncrypt).toHaveBeenCalledTimes(1) + expect(encrypt).not.toHaveBeenCalled() + + const [payloads, opts] = bulkEncrypt.mock.calls[0] as [ + Array<{ plaintext: unknown }>, + { column: { getName(): string } }, + ] + expect(payloads).toEqual([ + { plaintext: 'ada' }, + { plaintext: 'grace' }, + { plaintext: 'hopper' }, + ]) + expect(opts.column.getName()).toBe('nickname') + }) + + it('groups a multi-column query into one crossing per column', async () => { + const { builder, bulkEncrypt } = batchingInstance() + + await builder() + .select('id') + .eq('email', 'ada@lovelace.dev') + .eq('nickname', 'ada') + .in('nickname', ['grace', 'hopper']) + + // Two columns, two crossings — NOT four terms, four crossings. + expect(bulkEncrypt).toHaveBeenCalledTimes(2) + + const byColumn = new Map( + bulkEncrypt.mock.calls.map((call) => { + const [payloads, opts] = call as [ + Array<{ plaintext: unknown }>, + { column: { getName(): string } }, + ] + return [opts.column.getName(), payloads.map((p) => p.plaintext)] + }), + ) + expect(byColumn.get('email')).toEqual(['ada@lovelace.dev']) + expect(byColumn.get('nickname')).toEqual(['ada', 'grace', 'hopper']) + }) + + // Grouping reorders the crossings; the operands must still land on the filter + // that asked for them. This is the assertion that catches a bad scatter. + it('scatters each envelope back onto its own filter', async () => { + const { supabase, builder } = batchingInstance() + + await builder() + .select('id') + .eq('email', 'ada@lovelace.dev') + .eq('nickname', 'ada') + .in('nickname', ['grace', 'hopper']) + + expect(plaintextsOf(supabase.callsFor('eq'), 1)).toEqual([ + 'ada@lovelace.dev', + 'ada', + ]) + + const inCall = supabase.callsFor('filter')[0] + expect(inCall.args[1]).toBe('in') + const elements = (inCall.args[2] as string) + .slice(1, -1) + .split(/","/) + .map((e) => JSON.parse(e.replace(/^"|"$/g, '').replace(/\\"/g, '"')).pt) + expect(elements).toEqual(['grace', 'hopper']) + }) + + it('falls back to per-term encryption when the client has no bulkEncrypt', async () => { + const supabase = createMockSupabase() + const encryption = createMockEncryptionClient() + // biome-ignore lint/performance/noDelete: exercising the capability probe + delete (encryption as unknown as { bulkEncrypt?: unknown }).bulkEncrypt + const encrypt = vi.spyOn( + encryption as unknown as { encrypt: (...a: unknown[]) => unknown }, + 'encrypt', + ) + + await new EncryptedQueryBuilderV3Impl( + 'users', + users, + encryption, + supabase.client, + USERS_ALL_COLUMNS, + ) + .select('id') + .eq('email', 'ada@lovelace.dev') + .in('nickname', ['grace', 'hopper']) + + expect(encrypt).toHaveBeenCalledTimes(3) + expect(plaintextsOf(supabase.callsFor('eq'), 1)).toEqual([ + 'ada@lovelace.dev', + ]) + }) + + it('rejects a bulk response whose length does not match the list', async () => { + const supabase = createMockSupabase() + const encryption = createMockEncryptionClient() + ;( + encryption as unknown as { bulkEncrypt: (...a: unknown[]) => unknown } + ).bulkEncrypt = () => operation([{ data: fakeEnvelope('ada', 'nickname') }]) + + const { error, status } = await new EncryptedQueryBuilderV3Impl( + 'users', + users, + encryption, + supabase.client, + USERS_ALL_COLUMNS, + ) + .select('id') + .in('nickname', ['ada', 'grace']) + + // Silently truncating would widen the `in` predicate to one element. + expect(status).toBe(500) + expect(error?.message).toMatch(/1 term(s)? for 2 value(s)?/) + }) +}) diff --git a/packages/stack/__tests__/supabase-v3-wire.test.ts b/packages/stack/__tests__/supabase-v3-wire.test.ts index 5d33d3274..5ce8468e6 100644 --- a/packages/stack/__tests__/supabase-v3-wire.test.ts +++ b/packages/stack/__tests__/supabase-v3-wire.test.ts @@ -129,3 +129,69 @@ describe('encrypted not(col, in, …) emits a parseable PostgREST operand', () = expect(error?.message).toMatch(/requires an array of values/) }) }) + +// postgrest-js serializes a `contains` array as `cs.{${value.join(',')}}` — no +// element quoting — and a `not(col, 'contains', …)` operand as a bare +// `String(value)` coercion, which loses the braces entirely and renders an object +// as `[object Object]`. The `.or()` path already formats containment operands +// correctly; these are the direct paths, and they disagreed with it. +describe('plaintext contains() emits a parseable containment literal', () => { + it('quotes an array element containing a comma', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', ['with,comma']) + + // Unquoted, Postgres reads `{with,comma}` as TWO elements. + expect(wire.operandFor('note')).toBe('cs.{"with,comma"}') + }) + + it('leaves a comma-free array operand as a bare array literal', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', ['vip', 'admin']) + + expect(wire.operandFor('note')).toBe('cs.{vip,admin}') + }) + + it('emits a jsonb literal for an object operand', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', { a: 1 }) + + expect(wire.operandFor('note')).toBe('cs.{"a":1}') + }) + + it('forwards a string operand verbatim', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', 'plain') + + expect(wire.operandFor('note')).toBe('cs.plain') + }) +}) + +describe('plaintext not(col, contains, …) emits a parseable containment literal', () => { + it('quotes an array element containing a comma', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').not('note', 'contains', ['with,comma']) + + expect(wire.operandFor('note')).toBe('not.cs.{"with,comma"}') + }) + + it('emits a jsonb literal for an object operand', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').not('note', 'contains', { a: 1 }) + + expect(wire.operandFor('note')).toBe('not.cs.{"a":1}') + }) + + it('emits the `cs` token, never the `contains` method name', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').not('note', 'contains', ['vip']) + + expect(wire.operandFor('note')).toBe('not.cs.{vip}') + }) +}) diff --git a/packages/stack/__tests__/supabase-v3.test-d.ts b/packages/stack/__tests__/supabase-v3.test-d.ts index f8c4698b2..bf5fb5a03 100644 --- a/packages/stack/__tests__/supabase-v3.test-d.ts +++ b/packages/stack/__tests__/supabase-v3.test-d.ts @@ -24,13 +24,13 @@ type UserRow = InferPlaintext /** * A declared table whose ROW also carries plaintext passthrough columns — - * `tags` (text[]) and `meta` (jsonb). `InferPlaintext` alone yields only the - * declared encrypted columns, so this is the shape that exercises the - * plaintext half of `V3FreeTextSearchableKeys`. + * `tags` (text[]), `meta` (jsonb) and `note` (a SCALAR text column). + * `InferPlaintext` alone yields only the declared encrypted columns, so this is + * the shape that exercises the plaintext half of `V3FreeTextSearchableKeys`. */ declare const mixedBuilder: EncryptedQueryBuilderV3< typeof users, - UserRow & { tags: string[]; meta: Record } + UserRow & { tags: string[]; meta: Record; note: string } > /** A column key that is a UNION spanning an encrypted and a plaintext column. */ @@ -75,6 +75,24 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { builder.is('active', true) }) + // Every encrypted column stores a jsonb envelope, whether or not it carries a + // query capability. `IS TRUE` compares that envelope to a plaintext boolean — + // a database type error, not a filter. Gating the boolean form on the + // FILTERABLE keys only excluded the storage-only columns, so a queryable + // encrypted column like `email` slipped through. + it('rejects is(col, true) on queryable encrypted columns', async () => { + const supabase = await encryptedSupabaseV3(supabaseClient, { + schemas: { users }, + }) + const builder = supabase.from('users') + // @ts-expect-error — email is public.text_search: a jsonb ciphertext + builder.is('email', true) + // @ts-expect-error — nickname is public.text_eq: a jsonb ciphertext + builder.is('nickname', false) + // @ts-expect-error — amount is public.integer_ord: a jsonb ciphertext + builder.is('amount', true) + }) + // `IS NULL` is forwarded unencrypted (a NULL plaintext is stored as a SQL // NULL, not a ciphertext), and it is the ONLY predicate a storage-only column // supports — so it must not be gated behind the filterable-key narrowing. @@ -85,7 +103,6 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { const builder = supabase.from('users') builder.is('active', null) builder.is('email', null) - builder.is('email', true) }) it('rejects order() on every encrypted column at the type level', async () => { @@ -167,6 +184,20 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => { mixedBuilder.contains(plaintextKey, { plan: 'pro' }) }) + // `@>` is defined on arrays and jsonb, not on a scalar. Postgres answers a + // containment query against a plaintext `text` column with 42883 + // (operator does not exist), so the operand type must follow the column's own + // shape rather than admitting every native containment value on every + // plaintext key. + it('rejects containment on a plaintext scalar column', () => { + // @ts-expect-error — note is plaintext text: `text @> text[]` does not exist + mixedBuilder.contains('note', ['vip']) + // @ts-expect-error — note is plaintext text: `text @> jsonb` does not exist + mixedBuilder.contains('note', { a: 1 }) + // @ts-expect-error — a scalar column supports no containment operand at all + mixedBuilder.contains('note', 'vip') + }) + it('does not expose like/ilike on the v3 builder, at any chain depth', async () => { const supabase = await encryptedSupabaseV3(supabaseClient, { schemas: { users }, diff --git a/packages/stack/src/eql/v3/columns.ts b/packages/stack/src/eql/v3/columns.ts index a48fa0b33..f929dace8 100644 --- a/packages/stack/src/eql/v3/columns.ts +++ b/packages/stack/src/eql/v3/columns.ts @@ -613,6 +613,15 @@ export type AnyEncryptedV3Column = | EncryptedDoubleOrdOreColumn | EncryptedDoubleOrdColumn +/** + * A factory that builds a concrete v3 column for a given DB column name. + * + * Declared here rather than beside `DOMAIN_REGISTRY` so that `./types` can + * constrain its own export against it without importing from `./domain-registry`, + * which imports `./types` back as a value. + */ +export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column + /** * Shape of v3 table columns: every value is a v3 concrete column builder. * (Nested fields are deferred to later increments.) diff --git a/packages/stack/src/eql/v3/domain-registry.ts b/packages/stack/src/eql/v3/domain-registry.ts index b99a8fb67..6c255b1ab 100644 --- a/packages/stack/src/eql/v3/domain-registry.ts +++ b/packages/stack/src/eql/v3/domain-registry.ts @@ -1,8 +1,7 @@ -import type { AnyEncryptedV3Column } from './columns' +import type { V3ColumnFactory } from './columns' import { types } from './types' -/** A factory that builds a concrete v3 column for a given DB column name. */ -export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column +export type { V3ColumnFactory } from './columns' /** * Unqualified Postgres `domain_name` → the `eql/v3/types.ts` factory that @@ -18,6 +17,13 @@ export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column * `getEqlType()`, no test that also derives them can detect a corrupted domain * constant — `eql-v3-domain-registry.test.ts` pins them against a hand-written * literal list for exactly that reason. + * + * `factory('_probe')` runs at module load, so a non-factory value in `types` + * would throw here and take the importing modules (`../../supabase/introspect.ts`, + * `schema-builder.ts`, `verify.ts`) down with it. `types` is declared + * `satisfies Record` so that mistake is a compile error + * at its source instead. Do not reintroduce a cast on `Object.values` below: it + * would silence exactly the check that keeps this loop total. */ // NULL PROTOTYPE — load-bearing. A plain object literal inherits from // Object.prototype, so `DOMAIN_REGISTRY['constructor']` returns a *function* @@ -30,7 +36,7 @@ export type V3ColumnFactory = (name: string) => AnyEncryptedV3Column // exact: on a plain object `'constructor' in registry` is spuriously true. export const DOMAIN_REGISTRY: Record = (() => { const registry = Object.create(null) as Record - for (const factory of Object.values(types) as V3ColumnFactory[]) { + for (const factory of Object.values(types)) { // Probe name only: the constructors store their arguments and nothing else, // so building one column per domain at module load is free of side effects. const key = stripDomainSchema(factory('_probe').getEqlType()) diff --git a/packages/stack/src/eql/v3/types.ts b/packages/stack/src/eql/v3/types.ts index ecbdea3e2..f6550564f 100644 --- a/packages/stack/src/eql/v3/types.ts +++ b/packages/stack/src/eql/v3/types.ts @@ -76,6 +76,7 @@ import { TIMESTAMP_EQ, TIMESTAMP_ORD, TIMESTAMP_ORD_ORE, + type V3ColumnFactory, } from './columns' /** @@ -183,4 +184,9 @@ export const types = { DoubleOrdOre: (name: string) => new EncryptedDoubleOrdOreColumn(name, DOUBLE_ORD_ORE), DoubleOrd: (name: string) => new EncryptedDoubleOrdColumn(name, DOUBLE_ORD), -} as const + // `satisfies` is load-bearing, not decoration: `DOMAIN_REGISTRY` derives itself + // by calling every value here at module load. A non-factory export would throw + // during module evaluation and take the supabase introspect/schema-build/verify + // path down with it. This turns that into a compile error at the offending line. + // `as const` applies first, so literal key inference is preserved. +} as const satisfies Record diff --git a/packages/stack/src/supabase/helpers.ts b/packages/stack/src/supabase/helpers.ts index a6895bafd..b57ff044c 100644 --- a/packages/stack/src/supabase/helpers.ts +++ b/packages/stack/src/supabase/helpers.ts @@ -324,8 +324,21 @@ export function rebuildOrString( * characters, so `a}b` is a valid unquoted scalar; letting a stray one decrement * past zero meant no later comma ever split, silently absorbing every remaining * condition. + * + * The floor only covers a stray CLOSING character. A stray OPENING one leaves + * `depth` above zero for the rest of the input, and no later comma splits either + * — the same silent absorption, mirrored. `depth !== 0` at the end proves the + * counting was fooled by a brace or paren that was never structure, so the whole + * pass is discarded and the input re-split honouring quotes alone. That can only + * recover conditions, never lose them: a real containment literal reaches this + * point balanced, and the multi-element ones (the only kind carrying an internal + * comma) are quoted by {@link formatOrValue} and therefore already opaque. + * + * `trackDepth` is the recursion's own flag, never passed by callers. NEVER + * throws — `query-builder.ts` relies on `parseOrString` being total so that + * capability errors surface in filter order. */ -function splitTopLevel(input: string): string[] { +function splitTopLevel(input: string, trackDepth = true): string[] { const parts: string[] = [] let current = '' let depth = 0 @@ -346,7 +359,7 @@ function splitTopLevel(input: string): string[] { } else if (char === '"') { inQuotes = !inQuotes current += char - } else if ((char === '(' || char === '{') && !inQuotes) { + } else if (trackDepth && (char === '(' || char === '{') && !inQuotes) { // `{` as well as `(`: a containment operand is an array (`{vip,admin}`) or // a jsonb (`{"a":1,"b":2}`) literal, whose top-level commas are part of // the value. PostgREST's own logic-tree parser tracks these braces; @@ -354,7 +367,7 @@ function splitTopLevel(input: string): string[] { // dotless fragment `admin}` that the loop below silently drops. depth++ current += char - } else if ((char === ')' || char === '}') && !inQuotes) { + } else if (trackDepth && (char === ')' || char === '}') && !inQuotes) { depth = Math.max(0, depth - 1) current += char } else if (char === ',' && depth === 0 && !inQuotes) { @@ -367,6 +380,10 @@ function splitTopLevel(input: string): string[] { parts.push(current) + // An opener that was never structure (`note.eq.a{b,…`) left depth stranded and + // swallowed every condition behind it. Re-split without depth: quotes alone. + if (trackDepth && depth !== 0) return splitTopLevel(input, false) + return parts } @@ -443,6 +460,23 @@ function parseOrValue(value: string, op?: string): unknown { */ const POSTGREST_RESERVED = /["\\,().]/ +/** + * The reserved set for a SCALAR operand: {@link POSTGREST_RESERVED} plus the + * braces. + * + * A brace is structure to PostgREST's logic-tree parser inside `or=(…)`, and to + * {@link splitTopLevel} on the way back in, so an unquoted `a{b` is malformed on + * the wire AND desynchronises our own parse — the condition behind it is absorbed + * into this operand and silently dropped. Every character the parser reacts to + * must be quoted here. + * + * Deliberately NOT used for containment literals: those are `{…}` by + * construction, and quoting them on the brace alone would turn `tags.cs.{vip}` + * into `tags.cs."{vip}"`. Both spellings parse, but the bare one is what + * PostgREST documents and what the tests pin. + */ +const POSTGREST_RESERVED_SCALAR = /["\\,(){}.]/ + /** * Operands PostgREST reads as SQL values rather than as the string spelling * them. A STRING operand that happens to spell one must be quoted, or @@ -501,7 +535,14 @@ function formatOrValue(value: unknown, op?: string): string { // the top level. PostgREST accepts a quoted `"{vip,admin}"` inside `or=(…)`. if (op !== undefined && CONTAINMENT_OPS.has(op)) { const literal = containmentLiteral(value) - if (literal !== null) return formatOrValue(literal) + // Quoted on the NARROW set, not the scalar one: a containment literal is + // always brace-delimited, so the scalar set would quote every one of them. + // Its own braces are balanced and `splitTopLevel` counts them correctly. + if (literal !== null) { + return POSTGREST_RESERVED.test(literal) + ? `"${escapeOrValue(literal)}"` + : literal + } } if (Array.isArray(value)) { @@ -516,7 +557,10 @@ function formatOrValue(value: unknown, op?: string): string { // Wrap in double quotes if the value contains reserved characters. // This is required for encrypted values (JSON with commas, braces, etc.) // and is safe for all string values per PostgREST spec. - if (POSTGREST_RESERVED.test(str) || POSTGREST_RESERVED_WORDS.has(str)) { + if ( + POSTGREST_RESERVED_SCALAR.test(str) || + POSTGREST_RESERVED_WORDS.has(str) + ) { return `"${escapeOrValue(str)}"` } @@ -536,3 +580,22 @@ function formatOrValue(value: unknown, op?: string): string { export function formatInListOperand(values: readonly unknown[]): string { return formatOrValue([...values]) } + +/** + * The operand for a direct `cs`/`contains` filter: `{a,b}` for an array, + * `{"a":1}` for a jsonb object, `null` for a scalar. + * + * `null` means "not a structured operand" — the caller forwards the value as it + * stands. That covers both a plaintext string (`cs.plain`) and the v3 encrypted + * envelope, which is already `JSON.stringify`d and must not be re-serialized. + * + * Required because postgrest-js builds an array operand as `{${value.join(',')}}` + * with no element quoting, so an element carrying a comma becomes two elements; + * and its `not()` stringifies the operand outright, dropping the braces and + * rendering an object as `[object Object]`. The `.or()` path formats containment + * operands through the same {@link containmentLiteral}; emit them identically + * here, or the two paths disagree on what the same call means. + */ +export function formatContainmentOperand(value: unknown): string | null { + return containmentLiteral(value) +} diff --git a/packages/stack/src/supabase/query-builder-v3.ts b/packages/stack/src/supabase/query-builder-v3.ts index 60ca33f41..03a0342a8 100644 --- a/packages/stack/src/supabase/query-builder-v3.ts +++ b/packages/stack/src/supabase/query-builder-v3.ts @@ -323,36 +323,136 @@ export class EncryptedQueryBuilderV3Impl< } /** - * Encrypt every filter operand as a full storage envelope (see the class - * doc for why `encryptQuery` terms cannot be used), serialized to jsonb + * Validate a term's query type against its column's declared capabilities. + * Pure validation: `encrypt`/`bulkEncrypt` never receive the query type — the + * v3 filter operand is a full storage envelope (see the class doc for why + * `encryptQuery` terms cannot be used). + */ + private assertTermQueryable(term: ScalarQueryTerm): V3ColumnLike { + const column = term.column as unknown as V3ColumnLike + const queryType = term.queryType ?? 'equality' + + if ( + queryType !== 'equality' && + queryType !== 'orderAndRange' && + queryType !== 'freeTextSearch' + ) { + throw new Error( + `[supabase v3]: query type "${queryType}" is not supported on scalar EQL v3 columns`, + ) + } + + if (!column.getQueryCapabilities()[queryType]) { + throw new Error( + `[supabase v3]: column "${column.getName()}" (${column.getEqlType()}) does not support ${queryType} queries — declare the column with a domain that carries that capability`, + ) + } + + return column + } + + private encryptionFailure(message: string, cause?: unknown): never { + logger.error( + `Supabase: failed to encrypt query terms for table "${this.tableName}"`, + ) + throw new EncryptionFailedError( + `Failed to encrypt query terms: ${message}`, + cause, + ) + } + + /** + * Encrypt every filter operand as a full storage envelope, serialized to jsonb * text for the PostgREST filter value. + * + * Terms are grouped by column and each group takes ONE `bulkEncrypt` crossing. + * `in(col, [a, b, c])` collects one term per element (the list must never be + * encrypted whole), so encrypting per term spent N ZeroKMS/FFI round-trips + * where one would do. `bulkEncrypt` carries a single `{table, column}` for the + * whole payload, so the grouping is mandatory, not an optimisation: one bulk + * call over a mixed-column term array would stamp one column onto every + * plaintext. Results are scattered back onto the terms' original indices, + * which is the contract `termMap` downstream relies on. + * + * Mirrors `eql/v3/drizzle/operators.ts` `encryptOperands` — same batching + * contract, same length assertion, same fallback. Kept separate because that + * one encrypts a single-column operand list and returns `SQL[]`, while this + * must group a multi-column term array and preserve positions. */ protected override async encryptCollectedTerms( terms: ScalarQueryTerm[], ): Promise { - return Promise.all( - terms.map(async (term) => { - const column = term.column as unknown as V3ColumnLike - const queryType = term.queryType ?? 'equality' - const capabilities = column.getQueryCapabilities() - - if ( - queryType !== 'equality' && - queryType !== 'orderAndRange' && - queryType !== 'freeTextSearch' - ) { - throw new Error( - `[supabase v3]: query type "${queryType}" is not supported on scalar EQL v3 columns`, - ) - } + const groups = new Map< + V3ColumnLike, + { indices: number[]; values: ScalarQueryTerm['value'][] } + >() + terms.forEach((term, index) => { + const column = this.assertTermQueryable(term) + const group = groups.get(column) ?? { indices: [], values: [] } + group.indices.push(index) + group.values.push(term.value) + groups.set(column, group) + }) + + const bulkEncrypt = this.encryptionClient.bulkEncrypt?.bind( + this.encryptionClient, + ) + const results = new Array(terms.length) - if (!capabilities[queryType]) { - throw new Error( - `[supabase v3]: column "${column.getName()}" (${column.getEqlType()}) does not support ${queryType} queries — declare the column with a domain that carries that capability`, - ) - } + await Promise.all( + Array.from(groups, async ([column, { indices, values }]) => { + const encrypted = bulkEncrypt + ? await this.bulkEncryptGroup(bulkEncrypt, column, values) + : await this.encryptGroupPerTerm(column, values) + + encrypted.forEach((envelope, i) => { + results[indices[i]] = JSON.stringify(envelope) + }) + }), + ) + + return results + } + + /** One FFI crossing for a column's whole operand list. */ + private async bulkEncryptGroup( + bulkEncrypt: NonNullable, + column: V3ColumnLike, + values: ScalarQueryTerm['value'][], + ): Promise { + const baseOp = bulkEncrypt( + values.map((plaintext) => ({ plaintext })) as never, + { column, table: this.v3Table } as never, + ) + const op = this.lockContext + ? baseOp.withLockContext(this.lockContext) + : baseOp + if (this.auditConfig) op.audit(this.auditConfig) + + const result = await op + if (result.failure) + this.encryptionFailure(result.failure.message, result.failure) + + // `bulkEncrypt` is position-stable, so a length mismatch means the contract + // was violated. Truncating instead would silently widen an `in` predicate + // (or narrow a `not.in`) to whatever came back. + const encrypted = result.data as Array<{ data: unknown }> | undefined + if (!Array.isArray(encrypted) || encrypted.length !== values.length) { + this.encryptionFailure( + `bulk encryption returned ${Array.isArray(encrypted) ? encrypted.length : 0} terms for ${values.length} values on column "${column.getName()}".`, + ) + } + return encrypted.map((term) => term.data) + } - const baseOp = this.encryptionClient.encrypt(term.value, { + /** Fallback for a client that predates `bulkEncrypt`. */ + private async encryptGroupPerTerm( + column: V3ColumnLike, + values: ScalarQueryTerm['value'][], + ): Promise { + return Promise.all( + values.map(async (value) => { + const baseOp = this.encryptionClient.encrypt(value, { column, table: this.v3Table, }) @@ -363,17 +463,9 @@ export class EncryptedQueryBuilderV3Impl< const result = await op if (result.failure) { - logger.error( - `Supabase: failed to encrypt query terms for table "${this.tableName}"`, - ) - - throw new EncryptionFailedError( - `Failed to encrypt query terms: ${result.failure.message}`, - result.failure, - ) + this.encryptionFailure(result.failure.message, result.failure) } - - return JSON.stringify(result.data) + return result.data }), ) } @@ -424,16 +516,6 @@ export class EncryptedQueryBuilderV3Impl< return super.applyContainsFilter(q, column, value, wasEncrypted) } - protected override notFilterOperator( - op: FilterOp, - wasEncrypted: boolean, - ): string { - if (wasEncrypted && op === 'contains') { - return 'cs' - } - return op - } - /** * `.or()` string conditions carry raw PostgREST operators, so a free-text * condition arrives as `cs` — not a {@link FilterOp}. Resolve it through the diff --git a/packages/stack/src/supabase/query-builder.ts b/packages/stack/src/supabase/query-builder.ts index 5b2424287..7f9d84af5 100644 --- a/packages/stack/src/supabase/query-builder.ts +++ b/packages/stack/src/supabase/query-builder.ts @@ -16,6 +16,7 @@ import type { import { logger } from '@/utils/logger' import { addJsonbCasts, + formatContainmentOperand, formatInListOperand, getEncryptedColumnNames, isEncryptableTerm, @@ -532,6 +533,48 @@ export class EncryptedQueryBuilderImpl< const tableColumns = this.getColumnMap() + const pushTerm = ( + value: JsPlaintext, + column: ScalarQueryTerm['column'], + queryType: QueryTypeName, + mapping: TermMapping, + ) => { + terms.push({ + value, + column, + table: this.schema, + queryType, + returnType: 'composite-literal', + }) + termMap.push(mapping) + } + + /** + * Collect one term per element of an `in`-list operand. + * + * Element-wise is the only correct encoding: encrypting the array as ONE + * value collapses `(a,b)` into a single ciphertext that matches nothing. A + * null element is SQL NULL and passes through unencrypted; the applier + * restores it by index, which is why the mapping carries `inIndex`. + * + * Shared by the regular-`in`, `not(…,'in',…)` and or-condition paths. They + * drifted apart once already — the `not` path went unfixed while the other + * two encrypted element-wise — so they are kept in lockstep here rather than + * spelled out three times. + */ + const collectInListTerms = ( + op: FilterOp, + values: readonly unknown[], + column: ScalarQueryTerm['column'], + queryType: QueryTypeName, + mappingFor: (inIndex: number) => TermMapping, + ) => { + for (let j = 0; j < values.length; j++) { + if (!isEncryptableTerm(op, values[j])) continue + pushTerm(values[j] as JsPlaintext, column, queryType, mappingFor(j)) + } + } + // Regular filters for (let i = 0; i < dbSpace.filters.length; i++) { const f = dbSpace.filters[i] @@ -541,30 +584,20 @@ export class EncryptedQueryBuilderImpl< if (!column) continue if (f.op === 'in' && Array.isArray(f.value)) { - // For `in` filters, encrypt each value separately. A null element is - // SQL NULL and passes through; the applier restores it by index. - for (let j = 0; j < f.value.length; j++) { - if (!isEncryptableTerm(f.op, f.value[j])) continue - terms.push({ - value: f.value[j] as JsPlaintext, - column, - table: this.schema, - queryType: mapFilterOpToQueryType(f.op), - returnType: 'composite-literal', - }) - termMap.push({ source: 'filter', filterIndex: i, inIndex: j }) - } + collectInListTerms( + f.op, + f.value, + column, + mapFilterOpToQueryType(f.op), + (inIndex) => ({ source: 'filter', filterIndex: i, inIndex }), + ) } else if (!isEncryptableTerm(f.op, f.value)) { // `is` predicate or null operand — forwarded unencrypted. } else { - terms.push({ - value: f.value as JsPlaintext, - column, - table: this.schema, - queryType: mapFilterOpToQueryType(f.op), - returnType: 'composite-literal', + pushTerm(f.value as JsPlaintext, column, mapFilterOpToQueryType(f.op), { + source: 'filter', + filterIndex: i, }) - termMap.push({ source: 'filter', filterIndex: i }) } } @@ -578,14 +611,11 @@ export class EncryptedQueryBuilderImpl< const column = tableColumns[colName] if (!column) continue - terms.push({ - value: value as JsPlaintext, - column, - table: this.schema, - queryType: 'equality', - returnType: 'composite-literal', + pushTerm(value as JsPlaintext, column, 'equality', { + source: 'match', + matchIndex: i, + column: colName, }) - termMap.push({ source: 'match', matchIndex: i, column: colName }) } } @@ -598,37 +628,29 @@ export class EncryptedQueryBuilderImpl< if (!column) continue if (nf.op === 'in') { - // Each element is its own ciphertext, exactly as the regular `in` and - // `or(… .in. …)` paths do. Encrypting the whole list as one value - // yields a filter that silently matches nothing. + // A PostgREST list literal (`'(a,b)'`) cannot be encrypted element-wise, + // and encrypting it whole matches nothing. Refuse it rather than emit a + // filter that silently returns no rows. if (!Array.isArray(nf.value)) { throw new Error( `not("${nf.column}", "in", …) on an encrypted column requires an array of values, ` + `not a PostgREST list literal — each element must be encrypted separately`, ) } - for (let j = 0; j < nf.value.length; j++) { - if (!isEncryptableTerm(nf.op, nf.value[j])) continue - terms.push({ - value: nf.value[j] as JsPlaintext, - column, - table: this.schema, - queryType: mapFilterOpToQueryType(nf.op), - returnType: 'composite-literal', - }) - termMap.push({ source: 'not', notIndex: i, inIndex: j }) - } + collectInListTerms( + nf.op, + nf.value, + column, + mapFilterOpToQueryType(nf.op), + (inIndex) => ({ source: 'not', notIndex: i, inIndex }), + ) continue } - terms.push({ - value: nf.value as JsPlaintext, - column, - table: this.schema, - queryType: mapFilterOpToQueryType(nf.op), - returnType: 'composite-literal', + pushTerm(nf.value as JsPlaintext, column, mapFilterOpToQueryType(nf.op), { + source: 'not', + notIndex: i, }) - termMap.push({ source: 'not', notIndex: i }) } // Or filters — conditions were parsed once, in `toDbSpace`. The string and @@ -644,30 +666,23 @@ export class EncryptedQueryBuilderImpl< const column = tableColumns[cond.column] if (!column) continue - const pushTerm = (value: JsPlaintext, inIndex?: number) => { - terms.push({ - value, - column, - table: this.schema, - queryType: this.queryTypeForOrOp(cond.op), - returnType: 'composite-literal', - }) - termMap.push({ source, orIndex: i, conditionIndex: j, inIndex }) - } + // `queryTypeForOrOp`, not `mapFilterOpToQueryType`: an or-condition may + // carry a raw PostgREST operator (`cs`), which is not a `FilterOp`. + const queryType = this.queryTypeForOrOp(cond.op) + const mappingFor = (inIndex?: number): TermMapping => ({ + source, + orIndex: i, + conditionIndex: j, + inIndex, + }) - // Mirror the regular filter path: each element of an `in` list is its - // own term. Encrypting the array as one value collapses `(a,b)` into a - // single ciphertext that matches nothing. if (cond.op === 'in' && Array.isArray(cond.value)) { - for (let k = 0; k < cond.value.length; k++) { - if (!isEncryptableTerm(cond.op, cond.value[k])) continue - pushTerm(cond.value[k] as JsPlaintext, k) - } + collectInListTerms(cond.op, cond.value, column, queryType, mappingFor) continue } if (!isEncryptableTerm(cond.op, cond.value)) continue - pushTerm(cond.value as JsPlaintext) + pushTerm(cond.value as JsPlaintext, column, queryType, mappingFor()) } } @@ -679,14 +694,12 @@ export class EncryptedQueryBuilderImpl< const column = tableColumns[rf.column] if (!column) continue - terms.push({ - value: rf.value as JsPlaintext, + pushTerm( + rf.value as JsPlaintext, column, - table: this.schema, - queryType: this.queryTypeForRawOp(rf.operator), - returnType: 'composite-literal', - }) - termMap.push({ source: 'raw', rawIndex: i }) + this.queryTypeForRawOp(rf.operator), + { source: 'raw', rawIndex: i }, + ) } if (terms.length === 0) { @@ -1064,6 +1077,19 @@ export class EncryptedQueryBuilderImpl< const wasEncrypted = notValueMap.has(i) const value = wasEncrypted ? notValueMap.get(i) : nf.value + + // `contains` is a supabase-js METHOD name, not a PostgREST operator, and + // `q.not()` interpolates its operand with `String(value)` — so an array + // arrives brace-less and an object as `[object Object]`. Build the + // containment literal ourselves and emit the `cs` token, exactly as the + // `.or()` path does. A scalar (including the encrypted envelope, already + // serialized) yields `null` and is forwarded untouched. + if (nf.op === 'contains') { + const literal = formatContainmentOperand(value) + q = q.not(nf.column, 'cs', literal ?? value) + continue + } + q = q.not(nf.column, this.notFilterOperator(nf.op, wasEncrypted), value) } @@ -1233,6 +1259,10 @@ export class EncryptedQueryBuilderImpl< * jsonb/array containment. The v3 dialect overrides it for encrypted columns, * where `cs` resolves to the `@>` operator the EQL bundle declares on the * domain, backed by `eql_v3.contains` (bloom-filter containment). + * + * A structured operand is serialized here rather than by postgrest-js, which + * joins array elements on `,` without quoting them — so `['with,comma']` would + * reach Postgres as two elements. Scalars keep the native path. */ protected applyContainsFilter( q: SupabaseQueryBuilder, @@ -1240,7 +1270,10 @@ export class EncryptedQueryBuilderImpl< value: unknown, _wasEncrypted: boolean, ): SupabaseQueryBuilder { - return q.contains(column, value) + const literal = formatContainmentOperand(value) + return literal !== null + ? q.filter(column, 'cs', literal) + : q.contains(column, value) } /** @@ -1253,8 +1286,9 @@ export class EncryptedQueryBuilderImpl< } /** - * The PostgREST operator to use for a `.not()` filter. The v3 dialect maps - * `like`/`ilike` on encrypted columns to `cs` (see applyPatternFilter). + * The PostgREST operator to use for a `.not()` filter. Every {@link FilterOp} + * except `contains` spells the same as its PostgREST operator; `contains` is + * handled before this is reached, because it also needs its operand rewritten. */ protected notFilterOperator(op: FilterOp, _wasEncrypted: boolean): string { return op diff --git a/packages/stack/src/supabase/types.ts b/packages/stack/src/supabase/types.ts index 8b00f527d..247998b26 100644 --- a/packages/stack/src/supabase/types.ts +++ b/packages/stack/src/supabase/types.ts @@ -131,6 +131,22 @@ export type V3FreeTextSearchableKeys< */ type NativeContainsValue = string | readonly unknown[] | Record +/** + * The `contains()` operand for a PLAINTEXT column, derived from the column's own + * declared shape. + * + * `@>` is defined on arrays and on jsonb, never on a scalar: `contains('note', + * ['vip'])` against a plaintext `text` column emits `note.cs.{vip}` and Postgres + * answers 42883 (operator does not exist). A scalar therefore maps to `never`, + * which costs no legitimate call. `string` stays available on the container + * columns for the raw-literal form (`contains('tags', '{vip}')`). + */ +type PlaintextContainsValue = V extends readonly unknown[] + ? V | string + : V extends Record + ? V | string + : never + /** * The `contains()` operand for column `K`. * @@ -139,25 +155,36 @@ type NativeContainsValue = string | readonly unknown[] | Record * `public.text_search` do, and both `cast_as` to `string` — so its operand is * the string to tokenize into a bloom-filter query term. * - * Any other key is a plaintext passthrough, where the runtime forwards the - * operand untouched to `q.contains` and `contains` means PostgREST's native - * jsonb/array containment. A blanket `value: string` made that half of - * {@link V3FreeTextSearchableKeys} unreachable from TypeScript. + * Any other key is a plaintext passthrough, where `contains` means PostgREST's + * native jsonb/array containment and the operand follows the column + * ({@link PlaintextContainsValue}). A blanket `value: string` made that half of + * {@link V3FreeTextSearchableKeys} unreachable from TypeScript; a blanket + * {@link NativeContainsValue} then over-corrected, admitting an array on a + * plaintext scalar. * * A UNION key is only as permissive as its strictest member: if ANY member is a * declared column, the operand is the string term. `[K] extends [declared]` gets - * this backwards — a mixed `'email' | 'tags'` fails that test and falls to - * `NativeContainsValue`, so an array typechecks for a key that may resolve to - * the encrypted `email` at runtime. Nothing downstream catches it: the operand - * goes straight to `encrypt()`, which has no plaintext-type guard. + * this backwards — a mixed `'email' | 'tags'` fails that test and falls to the + * plaintext branch, so an array typechecks for a key that may resolve to the + * encrypted `email` at runtime. Nothing downstream catches it: the operand goes + * straight to `encrypt()`, which has no plaintext-type guard. * * So test the INTERSECTION instead, wrapped in a tuple to stop the naked type * parameter distributing (which would rebuild the same permissive union). + * + * `PlaintextContainsValue` DOES distribute over `Row[K]`, which is safe only + * because the tuple guard above has already excluded every encrypted member: a + * union of plaintext keys (`'tags' | 'meta'`) must accept the operands of each. + * The residual imprecision is a plaintext scalar unioned with a container column + * (`'note' | 'tags'`), where an array still typechecks — and yields a loud 42883 + * rather than a silent mis-encryption. */ -export type V3ContainsValue
= [ - Extract, string>>, -] extends [never] - ? NativeContainsValue +export type V3ContainsValue< + Table extends AnyV3Table, + Row extends Record, + K extends string, +> = [Extract, string>>] extends [never] + ? PlaintextContainsValue : string /** @@ -192,11 +219,15 @@ export interface EncryptedQueryBuilderV3< Row, V3FilterableKeys & StringKeyOf, EncryptedQueryBuilderV3, + V3OrderableKeys & StringKeyOf, + // `is(col, true)` is legal only on a plaintext column. That is exactly the + // orderable set today — every encrypted column is excluded from both — but + // the two are threaded separately so they can diverge. V3OrderableKeys & StringKeyOf > { contains & StringKeyOf>( column: K, - value: V3ContainsValue, + value: V3ContainsValue, ): EncryptedQueryBuilderV3 } @@ -581,6 +612,12 @@ export interface EncryptedQueryBuilderCore< /** Keys `order()` accepts. Defaults to `FK`, so the v2 surface is unchanged; * v3 narrows it to plaintext columns (see {@link V3OrderableKeys}). */ OK extends StringKeyOf = FK, + /** Keys the BOOLEAN form of `is()` accepts. Defaults to `FK`, so the v2 + * surface is unchanged; v3 narrows it to plaintext columns. Distinct from + * `OK` on purpose: "sortable" and "IS TRUE-able" are different capability + * axes that happen to select the same keys today, and narrowing `order()` + * later must not silently narrow `is()` with it. */ + BK extends StringKeyOf = FK, > extends PromiseLike> { /** `columns` defaults to `'*'`, matching supabase-js. A `'*'` select expands * to the introspected column list when one is available (v3), and otherwise @@ -629,11 +666,15 @@ export interface EncryptedQueryBuilderCore< * therefore not merely legal but the ONLY predicate available, so narrowing it * to `FK` would deny the sole query those columns support. * - * The boolean form stays on `FK`: `IS TRUE` against a jsonb ciphertext column - * compares an envelope to a plaintext boolean, which is a type error in the - * database, not a filter. + * The boolean form narrows to `BK`: `IS TRUE` against a jsonb ciphertext + * column compares an envelope to a plaintext boolean, which is a type error in + * the database, not a filter. `FK` is the wrong gate for it — that set + * excludes only the STORAGE-ONLY columns, so a queryable encrypted column + * (`types.TextSearch`, `types.TextEq`, any `*_ord`) is in `FK` and would still + * compile `is(col, true)`. Every encrypted column stores an envelope, + * capability or not, so `BK` excludes them all. */ - is(column: K, value: null | boolean): Self + is(column: K, value: null | boolean): Self is>(column: K, value: null): Self in(column: K, values: T[K][]): Self filter(column: K, operator: string, value: T[K]): Self From 4b27b7a529cdc61773639f7ca667d9bf5a205535 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 9 Jul 2026 23:56:03 +1000 Subject: [PATCH 4/4] fix(stack): count only structural braces when splitting an or-string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `splitTopLevel` treated every unquoted brace and paren as structure. Neither `{`/`(` mid-operand nor `}`/`)` is a PostgREST reserved character, so `a{b` and `a}b` are valid unquoted scalars — and counting them desynchronised the split. A stray closer drove the depth negative, a stray opener stranded it above zero, and either way no later comma split: every remaining condition was absorbed into the preceding operand. Flooring the depth at zero fixed the closer. The opener was handled by discarding the whole depth pass whenever the count ended unbalanced and re-splitting on quotes alone — which trades the bug for its mirror image. Given both a stray opener and a real containment literal, the re-split lands inside the literal: note.eq.a{b,tags.cs.{vip,admin} -> ["note.eq.a{b", "tags.cs.{vip", "admin}"] `admin}` carries no dot, so `parseOrString` drops it. A brace or paren is structure only where the grammar can put one: opening a logic group (`and(`, `or(`, and their `not.` forms), opening an operand right after the operator dot, or nested inside a literal already open. Count those and nothing else. The unbalanced-depth re-split stays as a backstop for the one case the rule still misreads — a scalar whose brace follows an in-value dot, `x.eq.a.{b` — where it is a recovery rather than the primary mechanism. Both paths are reachable only through a raw-string `.or()`: `rebuildOrString` quotes braces in scalars, so the adapter never emits the trigger itself. Also pins the two containment operands `arrayLiteralElement` special-cases but no wire test drove: an empty element must emit `{""}`, not the empty array `{}`, and an element spelling `null` must emit `{"null"}`, not a SQL NULL. --- .../stack/__tests__/supabase-helpers.test.ts | 51 +++++++++++++++++ .../stack/__tests__/supabase-v3-wire.test.ts | 20 +++++++ packages/stack/src/supabase/helpers.ts | 55 +++++++++++++------ 3 files changed, 110 insertions(+), 16 deletions(-) diff --git a/packages/stack/__tests__/supabase-helpers.test.ts b/packages/stack/__tests__/supabase-helpers.test.ts index 236834e3b..bb5b870b8 100644 --- a/packages/stack/__tests__/supabase-helpers.test.ts +++ b/packages/stack/__tests__/supabase-helpers.test.ts @@ -368,6 +368,57 @@ describe('parseOrString structural characters inside values', () => { { column: 'email', op: 'eq', negate: false, value: 'ada' }, ]) }) + + // A stray opener must not cost the or-string its REAL containment literals. + // Discarding the depth pass wholesale on an unbalanced count re-splits inside + // `{vip,admin}`, and the dotless `admin}` fragment is then dropped by + // `parseOrString` — the same silent condition loss, moved one operand along. + // A structural brace opens a group or an operand; anywhere else it is data. + it('keeps a sibling array literal intact past a stray opening brace', () => { + expect(parseOrString('note.eq.a{b,tags.cs.{vip,admin}')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'a{b' }, + { column: 'tags', op: 'cs', negate: false, value: '{vip,admin}' }, + ]) + }) + + it('keeps an array literal intact when the stray opener follows it', () => { + expect(parseOrString('tags.cs.{vip,admin},note.eq.a{b')).toEqual([ + { column: 'tags', op: 'cs', negate: false, value: '{vip,admin}' }, + { column: 'note', op: 'eq', negate: false, value: 'a{b' }, + ]) + }) + + it('keeps a sibling jsonb literal intact past a stray opening brace', () => { + expect(parseOrString('note.eq.a{b,meta.cs.{"a":1,"b":2}')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'a{b' }, + { column: 'meta', op: 'cs', negate: false, value: '{"a":1,"b":2}' }, + ]) + }) + + it('keeps a sibling array literal intact past a stray opening paren', () => { + expect(parseOrString('note.eq.a(b,tags.cs.{vip,admin}')).toEqual([ + { column: 'note', op: 'eq', negate: false, value: 'a(b' }, + { column: 'tags', op: 'cs', negate: false, value: '{vip,admin}' }, + ]) + }) + + // The boundary rule reads any `{` after a dot as an operand opener, so a + // scalar carrying an in-value dot still fools it. The unbalanced-depth + // re-split is what recovers this one; both mechanisms are load-bearing. + it('recovers a scalar whose brace follows an in-value dot', () => { + expect(parseOrString('x.eq.a.{b,y.eq.1')).toEqual([ + { column: 'x', op: 'eq', negate: false, value: 'a.{b' }, + { column: 'y', op: 'eq', negate: false, value: '1' }, + ]) + }) + + it('treats and/or group parens as structure', () => { + expect(parseOrString('and(a.eq.1,b.eq.2),c.eq.3')).toHaveLength(2) + expect(parseOrString('not.and(a.eq.1,b.eq.2),c.eq.3')).toHaveLength(2) + expect(parseOrString('and(a.eq.1,or(b.eq.2,c.eq.3)),d.eq.4')).toHaveLength( + 2, + ) + }) }) // An `in`-list element is quoted exactly like any other operand, so the list must diff --git a/packages/stack/__tests__/supabase-v3-wire.test.ts b/packages/stack/__tests__/supabase-v3-wire.test.ts index 5ce8468e6..f9f0716db 100644 --- a/packages/stack/__tests__/supabase-v3-wire.test.ts +++ b/packages/stack/__tests__/supabase-v3-wire.test.ts @@ -168,6 +168,26 @@ describe('plaintext contains() emits a parseable containment literal', () => { expect(wire.operandFor('note')).toBe('cs.plain') }) + + // An empty element is not an empty array: `{}` is a zero-length array literal, + // so the containment would test for nothing at all. + it('quotes an empty array element', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', ['']) + + expect(wire.operandFor('note')).toBe('cs.{""}') + }) + + // `NULL` is a keyword inside an array literal, case-insensitively. A string + // that spells it must be quoted or the element becomes a SQL NULL. + it('quotes an element spelling null', async () => { + const { wire, builder } = wireInstance() + + await builder().select('id').contains('note', ['null']) + + expect(wire.operandFor('note')).toBe('cs.{"null"}') + }) }) describe('plaintext not(col, contains, …) emits a parseable containment literal', () => { diff --git a/packages/stack/src/supabase/helpers.ts b/packages/stack/src/supabase/helpers.ts index b57ff044c..91ca28453 100644 --- a/packages/stack/src/supabase/helpers.ts +++ b/packages/stack/src/supabase/helpers.ts @@ -310,6 +310,9 @@ export function rebuildOrString( // Internal helpers // --------------------------------------------------------------------------- +/** A logic-group token: the only non-operand position an opener may follow. */ +const OR_GROUP_TOKEN = /^(?:not\.)?(?:and|or)$/ + /** * Split on the commas that separate top-level tokens, leaving those inside a * quoted operand or a `(…)` / `{…}` literal alone. @@ -320,19 +323,25 @@ export function rebuildOrString( * the one inside the element. Tracking quotes only at depth 0 ended the literal * early and swallowed the following condition into this operand. * - * `depth` never goes below zero. `}` and `)` are not PostgREST reserved - * characters, so `a}b` is a valid unquoted scalar; letting a stray one decrement - * past zero meant no later comma ever split, silently absorbing every remaining - * condition. + * Braces and parens count as STRUCTURE only where PostgREST's grammar can put + * them: opening a logic group (`and(`, `or(`, and their `not.` forms), opening + * an operand (immediately after the operator dot — `col.cs.{…}`, `col.in.(…)`), + * or nested inside a literal already open. `}` and `)` are not PostgREST + * reserved characters and `{`/`(` are not reserved mid-operand, so `a}b` and + * `a{b` are valid unquoted scalars. Counting those as structure desynchronised + * the split: a stray closer drove `depth` negative and a stray opener stranded + * it above zero, and either way no later comma split — every remaining condition + * was silently absorbed into this operand. + * + * `current === ''` admits a literal at the start of a token, which is how the + * `in`-list reuse below sees `{"a":1},{"b":2}`. * - * The floor only covers a stray CLOSING character. A stray OPENING one leaves - * `depth` above zero for the rest of the input, and no later comma splits either - * — the same silent absorption, mirrored. `depth !== 0` at the end proves the - * counting was fooled by a brace or paren that was never structure, so the whole - * pass is discarded and the input re-split honouring quotes alone. That can only - * recover conditions, never lose them: a real containment literal reaches this - * point balanced, and the multi-element ones (the only kind carrying an internal - * comma) are quoted by {@link formatOrValue} and therefore already opaque. + * The rule still reads a `{` after an in-value dot (`x.eq.a.{b`) as an operand + * opener. `depth !== 0` at the end proves the counting was fooled, so the pass + * is discarded and the input re-split honouring quotes alone — a backstop, not + * the primary mechanism. It must stay narrow: applied to an input whose braces + * WERE structure, it re-splits inside `{vip,admin}` and `parseOrString` then + * drops the dotless `admin}` fragment. * * `trackDepth` is the recursion's own flag, never passed by callers. NEVER * throws — `query-builder.ts` relies on `parseOrString` being total so that @@ -359,16 +368,30 @@ function splitTopLevel(input: string, trackDepth = true): string[] { } else if (char === '"') { inQuotes = !inQuotes current += char - } else if (trackDepth && (char === '(' || char === '{') && !inQuotes) { + } else if ( + trackDepth && + (char === '(' || char === '{') && + !inQuotes && // `{` as well as `(`: a containment operand is an array (`{vip,admin}`) or // a jsonb (`{"a":1,"b":2}`) literal, whose top-level commas are part of // the value. PostgREST's own logic-tree parser tracks these braces; // without them a condition splits mid-literal into `tags.cs.{vip` and a - // dotless fragment `admin}` that the loop below silently drops. + // dotless fragment `admin}` that the loop below silently drops. Only at a + // token boundary, though — mid-operand the character is just data. + (depth > 0 || + current === '' || + current.endsWith('.') || + OR_GROUP_TOKEN.test(current)) + ) { depth++ current += char - } else if (trackDepth && (char === ')' || char === '}') && !inQuotes) { - depth = Math.max(0, depth - 1) + } else if ( + trackDepth && + (char === ')' || char === '}') && + !inQuotes && + depth > 0 + ) { + depth -= 1 current += char } else if (char === ',' && depth === 0 && !inQuotes) { parts.push(current)