Skip to content

Commit 8cd485d

Browse files
committed
fix(stack): correct the supabase .or() string parser
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.
1 parent 99f8b0a commit 8cd485d

5 files changed

Lines changed: 281 additions & 23 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@cipherstash/stack': patch
3+
---
4+
5+
Fix the Supabase adapter's `.or()` string parser mis-splitting conditions, and pin `contains()` on a mixed union column key to the encrypted operand.
6+
7+
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.
8+
9+
**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.
10+
11+
**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.
12+
13+
**`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.
14+
15+
**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.
16+
17+
**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.
18+
19+
**`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__/supabase-helpers.test.ts

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,160 @@ describe('parseOrString containment literals', () => {
256256
})
257257
})
258258

259+
// A quoted operand is opaque at EVERY depth, and a stray brace or paren in an
260+
// unquoted value is a literal character, not structure. Tracking quotes only at
261+
// depth 0 let a `}` inside a quoted array element close the literal early; an
262+
// unmatched `}` in a plain value drove the depth counter negative, after which no
263+
// top-level comma ever split again. Both silently absorb the following condition
264+
// into the preceding operand — and only or-strings that also reference an
265+
// encrypted column are rebuilt from the parse, so it corrupts precisely the
266+
// mixed encrypted/plaintext case.
267+
describe('parseOrString structural characters inside values', () => {
268+
it('does not close an array literal on a brace inside a quoted element', () => {
269+
expect(parseOrString('tags.cs.{"a}b"},email.eq.secret')).toEqual([
270+
{ column: 'tags', op: 'cs', negate: false, value: '{"a}b"}' },
271+
{ column: 'email', op: 'eq', negate: false, value: 'secret' },
272+
])
273+
})
274+
275+
it('does not close a jsonb literal on a brace inside a quoted value', () => {
276+
expect(parseOrString('meta.cs.{"a":"v}"},id.eq.1')).toEqual([
277+
{ column: 'meta', op: 'cs', negate: false, value: '{"a":"v}"}' },
278+
{ column: 'id', op: 'eq', negate: false, value: '1' },
279+
])
280+
})
281+
282+
// Every structural character, quoted as a jsonb VALUE, with the encrypted
283+
// column ahead of the literal and a plaintext condition behind it — the shape
284+
// that actually reaches `rebuildOrString`, since the encrypted `email` is what
285+
// forces the group to be rebuilt rather than forwarded verbatim.
286+
it.each([
287+
'}',
288+
'{',
289+
')',
290+
'(',
291+
])('keeps a quoted %s inside a jsonb literal out of the depth count', (char) => {
292+
expect(
293+
parseOrString(`email.eq.x,meta.cs.{"a":"${char}"},note.eq.y`),
294+
).toEqual([
295+
{ column: 'email', op: 'eq', negate: false, value: 'x' },
296+
{ column: 'meta', op: 'cs', negate: false, value: `{"a":"${char}"}` },
297+
{ column: 'note', op: 'eq', negate: false, value: 'y' },
298+
])
299+
})
300+
301+
it('keeps an escaped quote inside a jsonb value opaque', () => {
302+
// `\"` must not close the element, or the `}` behind it decrements depth.
303+
expect(parseOrString('a.eq.1,meta.cs.{"a":"\\"}"},b.eq.2')).toHaveLength(3)
304+
})
305+
306+
it('splits after an unmatched brace in an unquoted value', () => {
307+
// `}` is not a PostgREST reserved character, so `a}b` is a valid unquoted
308+
// scalar operand.
309+
expect(parseOrString('nickname.eq.a}b,id.eq.1')).toEqual([
310+
{ column: 'nickname', op: 'eq', negate: false, value: 'a}b' },
311+
{ column: 'id', op: 'eq', negate: false, value: '1' },
312+
])
313+
})
314+
315+
it('splits after an unmatched paren in an unquoted value', () => {
316+
expect(parseOrString('a.eq.x),b.eq.y')).toEqual([
317+
{ column: 'a', op: 'eq', negate: false, value: 'x)' },
318+
{ column: 'b', op: 'eq', negate: false, value: 'y' },
319+
])
320+
})
321+
})
322+
323+
// An `in`-list element is quoted exactly like any other operand, so the list must
324+
// be split on top-level commas and each element unquoted. Splitting the raw
325+
// string on every comma tore `("a,b",c)` into three fragments and left the quotes
326+
// embedded in them — on an encrypted column each fragment is encrypted as its own
327+
// term, so the intended element never matches.
328+
describe('parseOrString in-list elements', () => {
329+
it('does not split on a comma inside a quoted element', () => {
330+
expect(parseOrString('email.in.("a,b",c)')).toEqual([
331+
{ column: 'email', op: 'in', negate: false, value: ['a,b', 'c'] },
332+
])
333+
})
334+
335+
it('unescapes a quoted element', () => {
336+
expect(parseOrString('a.in.("x\\"y",z)')).toEqual([
337+
{ column: 'a', op: 'in', negate: false, value: ['x"y', 'z'] },
338+
])
339+
})
340+
341+
it('round-trips a comma-bearing element through rebuild', () => {
342+
const s = 'name.in.("Doe, John",Smith)'
343+
expect(rebuildOrString(parseOrString(s) as DbPendingOrCondition[])).toBe(s)
344+
})
345+
346+
it('round-trips an encrypted envelope element', () => {
347+
const parsed = parseOrString(
348+
rebuildOrString([cond('email', 'in', [ENVELOPE, 'x'])]),
349+
)
350+
expect(parsed).toEqual([
351+
{ column: 'email', op: 'in', negate: false, value: [ENVELOPE, 'x'] },
352+
])
353+
})
354+
355+
it('splits a negated list on top-level commas only', () => {
356+
expect(parseOrString('email.not.in.("a,b",c)')).toEqual([
357+
{ column: 'email', op: 'in', negate: true, value: ['a,b', 'c'] },
358+
])
359+
})
360+
361+
// Only the operators whose operand PostgREST delimits with parens take a list.
362+
// A parenthesized operand anywhere else is a scalar that happens to start with
363+
// `(`: parsed as an array, an encrypted `eq` operand is encrypted as a JS array
364+
// rather than the intended string, and the filter matches nothing.
365+
it('does not read a parenthesized scalar as a list for a scalar operator', () => {
366+
expect(parseOrString('email.eq.(foo)')).toEqual([
367+
{ column: 'email', op: 'eq', negate: false, value: '(foo)' },
368+
])
369+
})
370+
371+
// The range operators take a paren-delimited operand too. Excluding them would
372+
// re-emit `period.ov.(1,10)` as a quoted scalar — a wire-format change on a
373+
// plaintext column that merely shares an `.or()` with an encrypted one.
374+
it.each([
375+
'ov',
376+
'sl',
377+
'sr',
378+
'nxr',
379+
'nxl',
380+
'adj',
381+
])('round-trips a paren-delimited %s operand', (op) => {
382+
const s = `period.${op}.(1,10)`
383+
expect(parseOrString(s)).toEqual([
384+
{ column: 'period', op, negate: false, value: ['1', '10'] },
385+
])
386+
expect(rebuildOrString(parseOrString(s) as DbPendingOrCondition[])).toBe(s)
387+
})
388+
})
389+
390+
// PostgREST reads a bare `null` / `true` / `false` operand as the SQL value, not
391+
// as the string spelling it. A string operand that happens to spell one must be
392+
// quoted, or `name.eq.null` compares against SQL NULL and matches nothing.
393+
describe('rebuildOrString reserved words', () => {
394+
it.each(['null', 'true', 'false'])('quotes the string %s', (word) => {
395+
expect(rebuildOrString([cond('name', 'eq', word)])).toBe(
396+
`name.eq."${word}"`,
397+
)
398+
})
399+
400+
it('leaves the SQL values unquoted', () => {
401+
expect(rebuildOrString([cond('a', 'is', null)])).toBe('a.is.null')
402+
expect(rebuildOrString([cond('a', 'is', true)])).toBe('a.is.true')
403+
expect(rebuildOrString([cond('a', 'is', false)])).toBe('a.is.false')
404+
})
405+
406+
it('quotes a reserved word inside an in-list', () => {
407+
expect(rebuildOrString([cond('a', 'in', ['null', 'x'])])).toBe(
408+
'a.in.("null",x)',
409+
)
410+
})
411+
})
412+
259413
describe('parseOrString negation', () => {
260414
it('lifts a not. prefix off the operator', () => {
261415
expect(parseOrString('nickname.not.eq.ada')).toEqual([

packages/stack/__tests__/supabase-v3.test-d.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ declare const mixedBuilder: EncryptedQueryBuilderV3<
3333
UserRow & { tags: string[]; meta: Record<string, unknown> }
3434
>
3535

36+
/** A column key that is a UNION spanning an encrypted and a plaintext column. */
37+
declare const mixedKey: 'email' | 'tags'
38+
/** A column key that is a union of plaintext columns only. */
39+
declare const plaintextKey: 'tags' | 'meta'
40+
3641
describe('encryptedSupabaseV3 typed surface (with schemas)', () => {
3742
it('rows carry each column its domain plaintext type', async () => {
3843
const supabase = await encryptedSupabaseV3(supabaseClient, {
@@ -144,6 +149,24 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => {
144149
mixedBuilder.contains('bio', { a: 1 })
145150
})
146151

152+
// A union column key is only as permissive as its STRICTEST member. If any
153+
// member is a declared encrypted column the operand must be the string term:
154+
// that member's runtime path hands the operand to `encrypt()`, which has no
155+
// plaintext-type guard, so an array reaches protect-ffi as the plaintext for a
156+
// `cast_as: text` column.
157+
it('pins a mixed union key to the encrypted string operand', () => {
158+
mixedBuilder.contains(mixedKey, 'ada')
159+
// @ts-expect-error — the union includes email (public.text_search)
160+
mixedBuilder.contains(mixedKey, ['vip'])
161+
// @ts-expect-error — the union includes email (public.text_search)
162+
mixedBuilder.contains(mixedKey, { plan: 'pro' })
163+
})
164+
165+
it('leaves a union of plaintext keys on the native operand', () => {
166+
mixedBuilder.contains(plaintextKey, ['vip'])
167+
mixedBuilder.contains(plaintextKey, { plan: 'pro' })
168+
})
169+
147170
it('does not expose like/ilike on the v3 builder, at any chain depth', async () => {
148171
const supabase = await encryptedSupabaseV3(supabaseClient, {
149172
schemas: { users },

packages/stack/src/supabase/helpers.ts

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ export function mapFilterOpToQueryType(op: FilterOp): QueryTypeName {
220220
export function parseOrString(orString: string): PendingOrCondition[] {
221221
const conditions: PendingOrCondition[] = []
222222
// Split on commas that are not inside parentheses (nested or/and)
223-
const parts = splitOrString(orString)
223+
const parts = splitTopLevel(orString)
224224

225225
for (const part of parts) {
226226
const trimmed = part.trim()
@@ -252,7 +252,7 @@ export function parseOrString(orString: string): PendingOrCondition[] {
252252
const value = rest.slice(secondDot + 1)
253253

254254
// Handle special value formats
255-
const parsedValue = parseOrValue(value)
255+
const parsedValue = parseOrValue(value, op)
256256

257257
conditions.push({ column, op, negate, value: parsedValue })
258258
}
@@ -310,7 +310,22 @@ export function rebuildOrString(
310310
// Internal helpers
311311
// ---------------------------------------------------------------------------
312312

313-
function splitOrString(input: string): string[] {
313+
/**
314+
* Split on the commas that separate top-level tokens, leaving those inside a
315+
* quoted operand or a `(…)` / `{…}` literal alone.
316+
*
317+
* Quotes are tracked at EVERY depth. A quoted string is opaque wherever it
318+
* appears, and an array literal quotes any element carrying a reserved character
319+
* (see {@link arrayLiteralElement}) — so `{"a}b"}` closes at the LAST brace, not
320+
* the one inside the element. Tracking quotes only at depth 0 ended the literal
321+
* early and swallowed the following condition into this operand.
322+
*
323+
* `depth` never goes below zero. `}` and `)` are not PostgREST reserved
324+
* characters, so `a}b` is a valid unquoted scalar; letting a stray one decrement
325+
* past zero meant no later comma ever split, silently absorbing every remaining
326+
* condition.
327+
*/
328+
function splitTopLevel(input: string): string[] {
314329
const parts: string[] = []
315330
let current = ''
316331
let depth = 0
@@ -328,7 +343,7 @@ function splitOrString(input: string): string[] {
328343
} else if (char === '\\' && inQuotes) {
329344
escaped = true
330345
current += char
331-
} else if (char === '"' && depth === 0) {
346+
} else if (char === '"') {
332347
inQuotes = !inQuotes
333348
current += char
334349
} else if ((char === '(' || char === '{') && !inQuotes) {
@@ -340,7 +355,7 @@ function splitOrString(input: string): string[] {
340355
depth++
341356
current += char
342357
} else if ((char === ')' || char === '}') && !inQuotes) {
343-
depth--
358+
depth = Math.max(0, depth - 1)
344359
current += char
345360
} else if (char === ',' && depth === 0 && !inQuotes) {
346361
parts.push(current)
@@ -350,27 +365,60 @@ function splitOrString(input: string): string[] {
350365
}
351366
}
352367

353-
if (current) {
354-
parts.push(current)
355-
}
368+
parts.push(current)
356369

357370
return parts
358371
}
359372

360-
function parseOrValue(value: string): unknown {
373+
/**
374+
* One element of an `in`-list operand, undoing {@link formatOrValue}'s quoting.
375+
* Unquoted elements are trimmed, as PostgREST ignores whitespace around them;
376+
* inside quotes it is significant.
377+
*/
378+
function parseInListElement(element: string): string {
379+
const trimmed = element.trim()
380+
if (trimmed.length >= 2 && trimmed.startsWith('"') && trimmed.endsWith('"')) {
381+
return unescapeOrValue(trimmed.slice(1, -1))
382+
}
383+
return trimmed
384+
}
385+
386+
/**
387+
* PostgREST operators whose operand is delimited by parentheses: the `in` list
388+
* and the range operators. Everywhere else `(` is an ordinary character, and a
389+
* parenthesized operand is a scalar that happens to start with one.
390+
*
391+
* The range operators earn their place by round-trip fidelity rather than by
392+
* encryption: none is supported on an encrypted column, but an or-string is
393+
* rebuilt whole as soon as ANY of its conditions names one, so a plaintext
394+
* `period.ov.(1,10)` sharing the group must re-emit byte-for-byte.
395+
*/
396+
const PAREN_OPERAND_OPS = new Set(['in', 'ov', 'sl', 'sr', 'nxr', 'nxl', 'adj'])
397+
398+
/**
399+
* @param op the operator the value belongs to, already stripped of any `not.`
400+
* prefix. Parsing a parenthesized scalar as an array meant an encrypted `eq`
401+
* operand was encrypted as a JS array rather than the intended string, and the
402+
* filter matched nothing.
403+
*/
404+
function parseOrValue(value: string, op?: string): unknown {
361405
// Handle double-quoted values (PostgREST quoting for reserved characters).
362406
// Must undo `escapeOrValue`, or a parse → rebuild round-trip doubles every
363407
// backslash. The two functions are only correct as a pair.
364408
if (value.length >= 2 && value.startsWith('"') && value.endsWith('"')) {
365409
return unescapeOrValue(value.slice(1, -1))
366410
}
367411

368-
// Handle parenthesized lists: (val1,val2,val3)
369-
if (value.startsWith('(') && value.endsWith(')')) {
370-
return value
371-
.slice(1, -1)
372-
.split(',')
373-
.map((v) => v.trim())
412+
// Handle parenthesized lists: (val1,val2,val3). Elements are quoted exactly as
413+
// any other operand, so the split must respect those quotes: `("a,b",c)` is
414+
// two elements, not three.
415+
if (
416+
op !== undefined &&
417+
PAREN_OPERAND_OPS.has(op) &&
418+
value.startsWith('(') &&
419+
value.endsWith(')')
420+
) {
421+
return splitTopLevel(value.slice(1, -1)).map(parseInListElement)
374422
}
375423

376424
// Handle booleans
@@ -395,6 +443,14 @@ function parseOrValue(value: string): unknown {
395443
*/
396444
const POSTGREST_RESERVED = /["\\,().]/
397445

446+
/**
447+
* Operands PostgREST reads as SQL values rather than as the string spelling
448+
* them. A STRING operand that happens to spell one must be quoted, or
449+
* `name.eq.null` compares against SQL NULL — a filter that matches nothing —
450+
* instead of against the three-character string.
451+
*/
452+
const POSTGREST_RESERVED_WORDS = new Set(['null', 'true', 'false'])
453+
398454
/** Escape `\` first, then `"` — the reverse order would double-escape. */
399455
function escapeOrValue(str: string): string {
400456
return str.replace(/\\/g, '\\\\').replace(/"/g, '\\"')
@@ -460,7 +516,7 @@ function formatOrValue(value: unknown, op?: string): string {
460516
// Wrap in double quotes if the value contains reserved characters.
461517
// This is required for encrypted values (JSON with commas, braces, etc.)
462518
// and is safe for all string values per PostgREST spec.
463-
if (POSTGREST_RESERVED.test(str)) {
519+
if (POSTGREST_RESERVED.test(str) || POSTGREST_RESERVED_WORDS.has(str)) {
464520
return `"${escapeOrValue(str)}"`
465521
}
466522

0 commit comments

Comments
 (0)