Merge pull request #609 from cipherstash/feat/eql-v3-supabase-adapter-follow-up

fix(stack): escape encrypted in-list operands; widen is/contains

Author: tobyhede

.changeset/supabase-in-list-operands.md

@@ -0,0 +1,74 @@
+---
+'@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 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. 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.

.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.

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))

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<Response> => {
+    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
+    },
+  }
+}

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)}")`)),