Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions .changeset/supabase-in-list-operands.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 19 additions & 0 deletions .changeset/supabase-or-string-parser.md
Original file line number Diff line number Diff line change
@@ -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.
98 changes: 71 additions & 27 deletions packages/stack/__tests__/eql-v3-domain-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
})

Expand All @@ -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))
Expand Down
68 changes: 68 additions & 0 deletions packages/stack/__tests__/helpers/postgrest-wire.ts
Original file line number Diff line number Diff line change
@@ -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
},
}
}
13 changes: 13 additions & 0 deletions packages/stack/__tests__/helpers/supabase-mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}")`)),
Expand Down
Loading
Loading