diff --git a/.changeset/eql-v3-supabase-adapter.md b/.changeset/eql-v3-supabase-adapter.md index 43e5a4a67..f690a896a 100644 --- a/.changeset/eql-v3-supabase-adapter.md +++ b/.changeset/eql-v3-supabase-adapter.md @@ -56,10 +56,12 @@ of always encrypting an equality term, so `filter('bio', 'cs', …)` on a `public.text_match` column works rather than being rejected, and an unsupported operator throws instead of silently encrypting the wrong term. -Substring `contains` still matches only when the needle equals the stored value -or is exactly the tokenizer's window (3 characters): the operand is a storage -envelope whose bloom carries the whole needle as an `include_original` token. -This is shared with v3 Drizzle's `contains` and tracked upstream in EQL. +Substring `contains` matches any needle whose trigrams are all present in the +stored value; needles shorter than the tokenizer's window (3 characters) bloom to +nothing and are rejected rather than silently matching every row. The v3 match +index now emits `include_original: false` — the flag is inert in protect-ffi (the +bloom is trigram-only either way), so this moves no ciphertext and only pins the +value a substring-search domain wants. v2 (`encryptedSupabase`) is unchanged: it keeps `like`/`ilike` (`eql_v2.like`, `~~`) and its raw-`filter` query-type mapping, so no v2 ciphertext moves. diff --git a/.changeset/stash-supabase-contains-substrings.md b/.changeset/stash-supabase-contains-substrings.md new file mode 100644 index 000000000..ca0df909a --- /dev/null +++ b/.changeset/stash-supabase-contains-substrings.md @@ -0,0 +1,14 @@ +--- +"stash": patch +--- + +Correct the bundled `stash-supabase` agent skill: EQL v3 `contains()` matches +substrings. The skill previously carried the reverse — that `contains()` matched +only exact values because the query's bloom filter appended the whole search term +as an extra token. That was never true: `include_original` is inert in +protect-ffi (the match bloom is trigram-only either way), so any substring of at +least the tokenizer's `token_length` (3 characters) matches, and shorter terms are +rejected rather than silently matching every row. The skills directory ships +inside the `stash` tarball and is copied into the user's `.claude/skills/` / +`.codex/skills/` (or inlined into `AGENTS.md`) at handoff time, so the stale +sentence was shipping wrong guidance into customer repos. diff --git a/docs/reference/supabase-sdk.md b/docs/reference/supabase-sdk.md index aa084aa9f..5017c3511 100644 --- a/docs/reference/supabase-sdk.md +++ b/docs/reference/supabase-sdk.md @@ -212,12 +212,11 @@ These are internal to the adapter but explain observable behaviour. Envelopes downcased and `%` is tokenized like any other character, so a `like` pattern is a category error. On plaintext columns `like`/`ilike` (and native `contains`) pass through unchanged. -- **`contains()` matches exact values, not general substrings.** The match - index always uses its default configuration (`types.TextSearch` takes no - match options), in which the operand's bloom carries the whole needle as an - extra token — so `contains()` matches when the needle equals the stored - value, or is exactly one token (3 characters) long; longer substrings do - not match. Known limitation, tracked upstream in EQL. +- **`contains()` matches substrings.** The search term blooms to its own + trigrams, and a row matches when the stored value's bloom contains all of + them — so any substring of at least 3 characters (the tokenizer's + `token_length`) matches. Shorter terms bloom to nothing and would match every + row, so they are rejected with an error rather than answered. - **`select('*')` (and bare `select()`) works on v3** — it expands to the introspected column list, with encrypted columns aliased and `::jsonb`-cast. - **Mutations send the raw encrypted payload** (the domains are diff --git a/packages/stack/__tests__/match-bloom-live.test.ts b/packages/stack/__tests__/match-bloom-live.test.ts new file mode 100644 index 000000000..c9c5b8a74 --- /dev/null +++ b/packages/stack/__tests__/match-bloom-live.test.ts @@ -0,0 +1,140 @@ +import 'dotenv/config' +import { encrypt, encryptQuery, newClient } from '@cipherstash/protect-ffi' +import { beforeAll, describe, expect, it } from 'vitest' +import { defaultMatchOpts } from '@/schema/match-defaults' + +/** + * Pins what protect-ffi's match index ACTUALLY emits, against real ffi. + * + * This suite exists because a claim about ffi's bloom filter — that + * `include_original: true` appends a whole-value token, so a substring `contains` + * can never match — was asserted in a code comment, propagated into the docs and + * the changeset, and then "confirmed" by `supabase-v3-pgrest-live.test.ts`, whose + * credential-free fake built the extra token itself. Nothing tied that fake to + * ffi, so the fiction round-tripped. It cost a bug report (CIP-3483) and a + * limitation callout in the published docs. + * + * The invariant can only be observed where the bloom is actually built, so these + * tests call ffi directly rather than going through a stack builder. They need + * CipherStash credentials but no database. + */ +const TABLE = 'match_bloom_probe' +const COLUMN = 'col' + +/** A match column, tokenized exactly as the shared defaults specify. */ +const encryptConfigWith = (includeOriginal: boolean) => ({ + v: 1, + tables: { + [TABLE]: { + [COLUMN]: { + cast_as: 'text' as const, + indexes: { + match: { ...defaultMatchOpts(), include_original: includeOriginal }, + }, + }, + }, + }, +}) + +type Client = Awaited> + +describe('protect-ffi match bloom', () => { + let withOriginal: Client + let withoutOriginal: Client + + beforeAll(async () => { + withOriginal = await newClient({ + encryptConfig: encryptConfigWith(true), + eqlVersion: 3, + }) + withoutOriginal = await newClient({ + encryptConfig: encryptConfigWith(false), + eqlVersion: 3, + }) + }) + + const bloomOf = async (client: Client, plaintext: string) => { + const payload = await encrypt(client, { + plaintext, + table: TABLE, + column: COLUMN, + }) + return (payload as { bf?: number[] }).bf ?? [] + } + + // The bloom of a QUERY term, produced the way a `contains` query actually + // produces its needle — `encryptQuery` with the `match` index type, not + // `encrypt`. This is the operand `eql_v3.contains` binds on the right of `@>`, + // so a subset test against a stored `bloomOf` value is the real query path, + // not a same-function artefact comparing `encrypt` to itself. + const queryBloomOf = async (client: Client, plaintext: string) => { + const term = await encryptQuery(client, { + plaintext, + table: TABLE, + column: COLUMN, + indexType: 'match', + }) + return (term as { bf?: number[] }).bf ?? [] + } + + /** + * ffi emits the bloom's bits in a nondeterministic ORDER — two encrypts of the + * same plaintext on the same client differ in sequence while carrying the same + * bits. Only the bit SET is meaningful (`@>` is `smallint[]` containment), so + * every comparison here sorts first. + */ + const sorted = (bits: number[]) => [...bits].sort((a, b) => a - b) + + // The claim, killed at its source. `include_original` is accepted by the + // config and ignored: the bloom is the tokenizer's trigrams, nothing more. + // If a future ffi starts honouring the flag, THIS fails — and the v3 domains + // already emit `false` (see `eql/v3/columns.ts`), so `contains` keeps working. + it.each([ + 'Ada Lovelace', + 'ada@example.com', + ])('emits the same bloom for %j whether include_original is on or off', async (plaintext) => { + const on = await bloomOf(withOriginal, plaintext) + const off = await bloomOf(withoutOriginal, plaintext) + + expect(sorted(on)).toEqual(sorted(off)) + // A trigram-only bloom, not one carrying an extra whole-value token: 6 + // bits (k) per DISTINCT trigram, so never more than 6 × the trigram count. + expect(on.length).toBeLessThanOrEqual(6 * (plaintext.length - 2)) + expect(on.length).toBeGreaterThan(0) + }) + + // The corollary, and the reason `matchNeedleError` must reject short needles + // instead of trusting `include_original` to make them matchable: below + // `token_length` there are no trigrams, so the bloom is EMPTY under either + // setting — and `stored_bf @> '{}'` is true for every row. + it.each([ + ['with include_original', true], + ['without include_original', false], + ] as const)('blooms a sub-trigram value to nothing, %s', async (_label, on) => { + const bloom = await bloomOf(on ? withOriginal : withoutOriginal, 'ad') + + expect(bloom).toEqual([]) + }) + + // The query needle's bloom is a subset of the STORED value's bloom — which is + // exactly what `eql_v3.contains` (`match_term(stored) @> query_term(needle)`) + // tests. The haystack is a stored `encrypt` value; each needle is an + // `encryptQuery` `match` term, the real operand the containment query binds — + // so this exercises the query path, not `encrypt` compared to itself. This is + // the substring case CIP-3483 reported as broken, proven at the bloom layer; + // `drizzle-v3/operators-live-pg.test.ts` proves the same thing through SQL. + it('blooms a substring query needle into a subset of the stored value bloom', async () => { + const haystack = new Set(await bloomOf(withOriginal, 'ada@example.com')) + + for (const needle of ['ada', 'example', 'ada@example.com']) { + const bits = await queryBloomOf(withOriginal, needle) + expect(bits.length).toBeGreaterThan(0) + expect(bits.filter((bit) => !haystack.has(bit))).toEqual([]) + } + + // A needle sharing no trigram must NOT be a subset, or the assertion above + // would hold for anything. + const absent = await queryBloomOf(withOriginal, 'zzz') + expect(absent.some((bit) => !haystack.has(bit))).toBe(true) + }) +}) diff --git a/packages/stack/__tests__/schema-v3.test.ts b/packages/stack/__tests__/schema-v3.test.ts index 72f442d7b..17c66b170 100644 --- a/packages/stack/__tests__/schema-v3.test.ts +++ b/packages/stack/__tests__/schema-v3.test.ts @@ -11,7 +11,7 @@ import { encryptConfigSchema, encryptedColumn } from '@/schema' import { type DomainSpec, typedEntries, V3_MATRIX } from './v3-matrix/catalog' describe('eql_v3 text_search column', () => { - it('LOAD-BEARING: default build() matches the v2 equality+order+match column modulo the ordering index', () => { + it('LOAD-BEARING: default build() matches the v2 equality+order+match column modulo the ordering index and include_original', () => { // eql-3.0.0 pins text_search's ordering to CLLW-OPE (`ope`/`op` term); the // v2 fluent builder keeps block-ORE (`ore`/`ob`) — its wire format must not // change. Everything else (unique, match, cast_as) stays byte-identical. @@ -25,12 +25,29 @@ describe('eql_v3 text_search column', () => { expect(v3.indexes.ore).toBeUndefined() expect(v2.indexes.ore).toEqual({}) expect(v2.indexes.ope).toBeUndefined() - const { ope: _v3Ord, ...v3Rest } = v3.indexes - const { ore: _v2Ord, ...v2Rest } = v2.indexes + + // The second deliberate divergence. protect-ffi ignores `include_original` + // (`match-bloom-live.test.ts` pins that against real ffi), so v3 emits the + // value a substring-search domain wants while v2 holds its historical + // `true` — no v2 wire movement. Asserted on BOTH sides, then excluded from + // the byte-identity check below, so neither can drift unnoticed. + expect(v3.indexes.match?.include_original).toBe(false) + expect(v2.indexes.match?.include_original).toBe(true) + + const { ope: _v3Ord, match: v3Match, ...v3Rest } = v3.indexes + const { ore: _v2Ord, match: v2Match, ...v2Rest } = v2.indexes + const withoutIncludeOriginal = (match: typeof v3Match) => { + if (!match) return match + const { include_original: _drop, ...rest } = match + return rest + } // toStrictEqual: byte-identical, no extra/undefined keys on either side. - expect({ ...v3, indexes: v3Rest }).toStrictEqual({ + expect({ + ...v3, + indexes: { ...v3Rest, match: withoutIncludeOriginal(v3Match) }, + }).toStrictEqual({ ...v2, - indexes: v2Rest, + indexes: { ...v2Rest, match: withoutIncludeOriginal(v2Match) }, }) }) @@ -43,7 +60,7 @@ describe('eql_v3 text_search column', () => { token_filters: [{ kind: 'downcase' }], k: 6, m: 2048, - include_original: true, + include_original: false, }) }) @@ -197,7 +214,7 @@ describe('eql_v3 encryptedTable', () => { token_filters: [{ kind: 'downcase' }], k: 6, m: 2048, - include_original: true, + include_original: false, }, }, }, diff --git a/packages/stack/integration/supabase/wire.integration.test.ts b/packages/stack/integration/supabase/wire.integration.test.ts index eb14958a4..70230b83d 100644 --- a/packages/stack/integration/supabase/wire.integration.test.ts +++ b/packages/stack/integration/supabase/wire.integration.test.ts @@ -67,11 +67,26 @@ const COLUMN_TERMS: Record< active: {}, // boolean — storage only } -/** Deterministic 3-gram token set, plus the whole value as one extra token — - * emulating the default `include_original: true`. That is precisely why a - * SUBSTRING `like` does not match: the pattern's bloom carries the whole - * pattern as a token the stored value's bloom lacks (see the class doc on - * `query-builder-v3.ts`). */ +/** + * Deterministic stand-in for protect-ffi's match tokenizer: `ngram` + * (`token_length: 3`) + `downcase`, hashed into the bloom's bit domain. + * + * TRIGRAMS ONLY, deliberately. This suite has no CipherStash credentials and + * mints its own envelopes, so this function IS the bloom oracle for both the + * seeded rows and the query needles — anything it invents, the tests below will + * dutifully confirm. It previously prepended the whole value as an extra token + * to emulate `include_original: true`, which made a substring `contains` fail + * and pinned that failure as expected behaviour. + * + * protect-ffi does no such thing: `include_original` is accepted and ignored + * (measured across 0.24 and 0.29, EQL v2 and v3 — the emitted bloom is + * trigram-only under either setting). Substring `contains` works, as + * `drizzle-v3/operators-live-pg.test.ts` proves against real ffi and real + * Postgres. + * + * A value shorter than `token_length` yields NO tokens, matching real ffi's + * empty bloom — the fail-open `matchNeedleError` exists to reject. + */ function bloomTokens(value: string): number[] { const hash = (s: string) => { let h = 2166136261 @@ -80,7 +95,7 @@ function bloomTokens(value: string): number[] { return h % 2048 } const lower = value.toLowerCase() - const tokens = [hash(lower)] // include_original + const tokens: number[] = [] for (let i = 0; i + 3 <= lower.length; i++) tokens.push(hash(lower.slice(i, i + 3))) return tokens @@ -324,13 +339,11 @@ describe('supabase v3 adapter over real PostgREST (wire + grants)', () => { // whose body is `match_term(a) @> match_term(b::public.eql_v3_text_search)` — a // smallint[] containment of the two BLOOM FILTERS. It is NOT the built-in // `jsonb @> jsonb`, which would compare whole envelopes and so could only ever - // match on an identical ciphertext. The three tests below discriminate: a - // 3-char needle matches while a 7-char one does not, and neither shares the - // stored `c`. Only bloom containment explains that. + // match on an identical ciphertext. // - // With the default `include_original: true` the needle's bloom carries the - // WHOLE needle as an extra token, so only an exact-value needle matches. - // Asserts the DOCUMENTED semantics, not the intuitive ones. + // The four tests below discriminate: substrings that share no `c` with the + // stored row match, and a trigram present in no row does not. Only bloom + // containment explains both. it('resolves contains() through cs containment for an exact value', async () => { const { data, error } = await from() .select('row_key') @@ -340,22 +353,20 @@ describe('supabase v3 adapter over real PostgREST (wire + grants)', () => { expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) }) - // A needle LONGER than the tokenizer's 3-gram window contributes an - // `include_original` token no stored trigram can supply, so containment - // fails. (A needle of exactly 3 characters is the degenerate case: its - // whole-value token IS a trigram, so `contains('email','ada')` DOES match.) - // This is the KNOWN-BROKEN substring defect, shared with v3 Drizzle's - // `contains` and tracked upstream in EQL. Pinned so a fix is a visible change. - it('does not match a longer substring under include_original', async () => { + // The needle blooms to its own trigrams — `exa,xam,amp,mpl,ple` — every one of + // which is a trigram of `ada@example.com`, so containment holds. This once + // asserted the opposite, on the strength of an `include_original` token that + // protect-ffi never emits; `bloomTokens` above no longer invents one. + it('matches a longer substring through bloom containment', async () => { const { data, error } = await from() .select('row_key') .contains('email', 'example') expect(error).toBeNull() - expect(data).toEqual([]) + expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) }) - it('matches a 3-character substring, the degenerate include_original case', async () => { + it('matches a substring exactly one trigram long', async () => { const { data, error } = await from() .select('row_key') .contains('email', 'ada') @@ -364,6 +375,19 @@ describe('supabase v3 adapter over real PostgREST (wire + grants)', () => { expect(data.map((r: { row_key: string }) => r.row_key)).toEqual(['ada']) }) + // The discriminator. Every assertion above is satisfied by a `contains` that + // matches EVERYTHING — an empty needle bloom, a broken `@>`, a fail-open. A + // trigram held by no stored row must come back empty. (`drizzle-v3`'s live + // suite pins the same guarantee against real ffi with `'qqqzzz'`.) + it('does not match a trigram absent from every stored value', async () => { + const { data, error } = await from() + .select('row_key') + .contains('email', 'zzz') + + expect(error).toBeNull() + expect(data).toEqual([]) + }) + // The reason `like` is gone: `~~` is not defined on public.eql_v3_text_search, so // had the adapter emitted it, PostgREST/Postgres would answer 42883. The // client-side guard turns that into an actionable error before the round-trip. diff --git a/packages/stack/src/eql/v3/columns.ts b/packages/stack/src/eql/v3/columns.ts index 823bc5d2e..6a1e9a46a 100644 --- a/packages/stack/src/eql/v3/columns.ts +++ b/packages/stack/src/eql/v3/columns.ts @@ -328,7 +328,8 @@ export const DOUBLE_ORD = { * operator), so those emit no `hm`. Text order domains emit BOTH `unique` * and the ordering index. * - `match` (the `bf` bloom-filter index) for free-text search, deep-cloned from - * the per-call defaults so no nested object is ever shared across columns. + * the per-call defaults so no nested object is ever shared across columns, and + * emitted with `include_original: false` (see below). */ function indexesForCapabilities( capabilities: QueryCapabilities, @@ -354,7 +355,15 @@ function indexesForCapabilities( if (capabilities.freeTextSearch) { // The factory returns fresh, unaliased nested objects per call, so no // column's emitted match block ever shares state with another's. - indexes.match = defaultMatchOpts() + // + // `include_original` is overridden off the v2 builder's `true`. protect-ffi + // ignores the flag entirely (measured across 0.24 and 0.29, EQL v2 and v3: + // the emitted bloom is trigram-only either way, and a value shorter than + // `token_length` blooms to nothing regardless), so this is inert today. It + // is set to the value a substring-search domain actually wants — matching + // the zod schema's own default — so that a protect-ffi release which starts + // honouring the flag cannot silently break `contains`. + indexes.match = { ...defaultMatchOpts(), include_original: false } } return indexes diff --git a/packages/stack/src/schema/match-defaults.ts b/packages/stack/src/schema/match-defaults.ts index f2181c79b..45fd5232f 100644 --- a/packages/stack/src/schema/match-defaults.ts +++ b/packages/stack/src/schema/match-defaults.ts @@ -20,9 +20,18 @@ export type BuiltMatchIndexOpts = { /** * Default match-index parameters — the single source of truth shared by the - * v2 `freeTextSearch()` builder and the v3 domain builders (note - * `include_original: true`, which is the v2 builder default rather than the - * zod-schema default of `false`). + * v2 `freeTextSearch()` builder and the v3 domain builders. + * + * `include_original: true` is the v2 builder's historical default rather than + * the zod-schema default of `false`. It is INERT: protect-ffi accepts the flag + * and ignores it (measured across 0.24 and 0.29, EQL v2 and v3 — the emitted + * bloom is trigram-only either way). It is kept here only so the v2 emitted + * config does not move. The v3 domain builders override it to `false`, the + * value a substring-search domain wants if ffi ever starts honouring it. + * + * In particular it does NOT let a value shorter than the tokenizer's + * `token_length` be matched: such a value blooms to nothing under either + * setting, which is exactly the fail-open {@link matchNeedleError} guards. * * This is a FACTORY (not a shared `const`) so every caller gets fresh, unaliased * nested objects (`tokenizer`, `token_filters` and the `{ kind: 'downcase' }` diff --git a/packages/stack/src/supabase/query-builder-v3.ts b/packages/stack/src/supabase/query-builder-v3.ts index 2825de019..8b11cb8aa 100644 --- a/packages/stack/src/supabase/query-builder-v3.ts +++ b/packages/stack/src/supabase/query-builder-v3.ts @@ -123,12 +123,11 @@ function assertNoPropertyDbNameCollision( * we. The typed builder has no `like`; the runtime methods throw on an * encrypted column and pass through on a plaintext one. * - * KNOWN BROKEN for real substrings, and not fixable from this file. The - * operand is a storage payload, so its bloom carries the whole needle as an - * extra `include_original` token, which the haystack's bloom cannot contain - * unless the needle equals the stored value or is exactly `token_length` (3) - * characters. v3 Drizzle's `contains` has the same defect for the same - * reason. Tracked in EQL; do not paper over it here. + * Substrings DO match: the needle blooms to its own trigrams, and containment + * holds whenever every one of them is present in the stored value's bloom — + * i.e. for any substring of at least `token_length` (3) characters. Shorter + * needles bloom to nothing (`bf @> '{}'` is true for every row) and are + * rejected up front by `matchNeedleError`, not answered. * * Decrypted rows additionally get `Date` reconstruction from the * encrypt-config `cast_as`, mirroring the typed v3 client. diff --git a/packages/stack/src/supabase/types.ts b/packages/stack/src/supabase/types.ts index ef78199be..869eb9b47 100644 --- a/packages/stack/src/supabase/types.ts +++ b/packages/stack/src/supabase/types.ts @@ -58,9 +58,7 @@ export type EncryptedSupabaseV3Options< * Declaring a `text_search` column does NOT change its match behaviour: a * declared and a synthesized `text_search` column build byte-identically, and * neither `types.TextSearch` nor `EncryptedTextSearchColumn` accepts match - * options. `include_original: true` is therefore always in force, so a - * substring `contains` matches nothing on either. See the `contains` note on - * `EncryptedQueryBuilderV3Impl`. + * options. See the `contains` note on `EncryptedQueryBuilderV3Impl`. */ schemas?: S } diff --git a/packages/test-kit/src/catalog.ts b/packages/test-kit/src/catalog.ts index 93f085fd0..2dcb3929e 100644 --- a/packages/test-kit/src/catalog.ts +++ b/packages/test-kit/src/catalog.ts @@ -217,7 +217,7 @@ const MATCH_BLOCK: NonNullable['match'] = { token_filters: [{ kind: 'downcase' }], k: 6, m: 2048, - include_original: true, + include_original: false, } const MATCH_IDX: Indexes = { match: MATCH_BLOCK } const TEXT_SEARCH_IDX: Indexes = { diff --git a/skills/stash-supabase/SKILL.md b/skills/stash-supabase/SKILL.md index 44034ae92..ff783cfca 100644 --- a/skills/stash-supabase/SKILL.md +++ b/skills/stash-supabase/SKILL.md @@ -523,12 +523,11 @@ All envelopes (stored payloads and filter operands) are versioned `v: 3`. other character, so a `like` pattern is a category error. Calling `like` or `ilike` on an encrypted column throws an error pointing at `contains()`; on plaintext columns both pass through unchanged. -- **`contains()` matches exact values, not general substrings.** The match - index always uses its default configuration, in which the query's bloom - filter carries the whole search term as an extra token — so `contains()` - matches when the term equals the stored value (or is exactly one token, - 3 characters, long); longer substrings do not match. This is a known - limitation tracked upstream in EQL. +- **`contains()` matches substrings.** The search term blooms to its own + trigrams, and a row matches when the stored value's bloom contains all of + them — so any substring of at least 3 characters (the tokenizer's + `token_length`) matches. Shorter terms bloom to nothing and would match every + row, so they are rejected with an error rather than answered. - **INTERIM — filter operands are full storage envelopes.** EQL ships term-only query domains (`eql_v3.query_`, which accept envelopes with no ciphertext) and the encryption client can mint those narrowed terms, but