Skip to content

Commit f4d45dd

Browse files
committed
Merge remote-tracking branch 'origin/feat/eql-v3-text-search-schema' into feat/eql-v3-json
# Conflicts: # packages/stack/src/eql/v3/drizzle/operators.ts
2 parents e27ce9d + 7c10347 commit f4d45dd

19 files changed

Lines changed: 471 additions & 110 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
'@cipherstash/stack': minor
3+
---
4+
5+
Restore the EQL v3 envelope and `Result` types the adapters were erasing.
6+
7+
Both v3 adapters typed their operand-encryption paths as `unknown` and dropped
8+
the `Result` wrapper, so the query-type encoding and the failure channel were
9+
invisible to the type system:
10+
11+
- `eql/v3/drizzle/operators.ts` typed the client's `encrypt`/`bulkEncrypt` as
12+
returning `unknown`, collapsed the operation's `Result` to
13+
`{ data?: unknown; failure?: { message } }`, and cast the bulk response to
14+
`Array<{ data: unknown }>`.
15+
- `supabase/query-builder-v3.ts` returned `Promise<unknown[]>` from
16+
`encryptCollectedTerms`, `bulkEncryptGroup` and `encryptGroupPerTerm`, and the
17+
base `query-builder.ts` did the same.
18+
19+
These now carry the SDK's real types — `Encrypted` (the storage envelope union,
20+
which includes every v3 per-domain payload), `BulkEncryptedData`, and
21+
`EncryptedQueryResult` — threaded through a properly-typed operation surface that
22+
resolves `Result<T, EncryptionError>`. The Supabase divergence the erasure hid is
23+
now explicit: the v2 path yields `encryptQuery` composite literals and the v3
24+
path yields `JSON.stringify`'d envelope strings, and both are `EncryptedQueryResult`.
25+
26+
Bumped `minor`, not `patch`: `createEncryptionOperatorsV3` is a public export
27+
(`@cipherstash/stack/eql/v3/drizzle`), and tightening its client contract from
28+
`unknown` to a typed operation surface is a compile-time breaking change — a
29+
downstream consumer passing a loosely-typed (`unknown`-returning) client double
30+
will now fail `tsc`. That tightening has teeth: `operators.test-d.ts` pins it
31+
with a negative type-test asserting an `unknown`-returning `{ encrypt }` double
32+
is rejected (a positive "correctly-typed double is accepted" assertion cannot
33+
catch a re-erasure, since a correct value is assignable to `unknown`).
34+
35+
Behaviour is otherwise unchanged, with one addition: the Supabase v3 bulk path
36+
now rejects a `null` envelope returned by `bulkEncrypt` (the restored
37+
`Encrypted | null` type makes that arm reachable, and a `null` would otherwise
38+
be `JSON.stringify`'d to the literal `"null"` and sent as a filter operand).

.changeset/supabase-v3-order-by-ope-term.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ The guard is now on the ordering FLAVOUR, not on encryption:
2626
- **`ore` present → rejected.** The `ob` term is an array of ORE blocks whose
2727
comparison needs the superuser-only operator class, which no jsonb path can
2828
reach. (Such a column cannot hold data on managed Postgres anyway: its domain
29-
CHECK raises `ore_domain_unavailable`.)
29+
CHECK raises `ore_domain_unavailable`.) ORE columns are now excluded from
30+
`order()` at COMPILE time too, not only at runtime — `.order(oreColumn)` is a
31+
type error, matching the rejection.
3032
- **neither → rejected.** Storage-only, equality-only and match-only columns
3133
carry no ordering term.
3234

@@ -38,6 +40,8 @@ numeric and date domains, and per-character (16 hex chars each) for text, so
3840
lexicographic order reproduces plaintext order including the prefix case
3941
(`ada` < `adam`). `ope-term.integration.test.ts` pins that shape.
4042

41-
`V3OrderableKeys` widens to match, so `order()` on an ordering column
42-
typechecks. `is(col, true)` is unaffected — it stays plaintext-only, and now has
43-
its own `V3PlaintextKeys` rather than borrowing the orderable set.
43+
`V3OrderableKeys` widens to admit OPE-backed ordering columns (`*_ord`,
44+
`text_ord`, `text_search`) while still excluding ORE (`*_ord_ore`) columns, so
45+
`order()` typechecks exactly where it works. `is(col, true)` is unaffected — it
46+
stays plaintext-only, and now has its own `V3PlaintextKeys` rather than
47+
borrowing the orderable set.

.github/workflows/integration-drizzle.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ on:
1818
branches: [main]
1919
paths:
2020
- 'packages/stack/src/eql/v3/**'
21+
# Source layers the adapter's encoding/round-trip rests on: a break here
22+
# (not just under src/eql/v3) can produce wrong rows, so trigger the live
23+
# suite that would catch it.
24+
- 'packages/stack/src/encryption/**'
25+
- 'packages/stack/src/schema/**'
26+
- 'packages/schema/**'
2127
- 'packages/stack/integration/**'
2228
- 'packages/test-kit/**'
2329
- 'packages/cli/src/installer/**'
@@ -31,6 +37,12 @@ on:
3137
# Repeated verbatim: GitHub Actions does not support YAML anchors/aliases.
3238
paths:
3339
- 'packages/stack/src/eql/v3/**'
40+
# Source layers the adapter's encoding/round-trip rests on: a break here
41+
# (not just under src/eql/v3) can produce wrong rows, so trigger the live
42+
# suite that would catch it.
43+
- 'packages/stack/src/encryption/**'
44+
- 'packages/stack/src/schema/**'
45+
- 'packages/schema/**'
3446
- 'packages/stack/integration/**'
3547
- 'packages/test-kit/**'
3648
- 'packages/cli/src/installer/**'

.github/workflows/integration-supabase.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ on:
1414
paths:
1515
- 'packages/stack/src/supabase/**'
1616
- 'packages/stack/src/eql/v3/**'
17+
# Source layers the adapter's encoding/round-trip rests on: a break here
18+
# (not just under src/supabase) can produce wrong wire output or rows, so
19+
# trigger the live suite that would catch it.
20+
- 'packages/stack/src/encryption/**'
21+
- 'packages/stack/src/schema/**'
22+
- 'packages/schema/**'
1723
- 'packages/stack/integration/**'
1824
- 'packages/test-kit/**'
1925
- 'packages/cli/src/installer/**'
@@ -26,6 +32,12 @@ on:
2632
paths:
2733
- 'packages/stack/src/supabase/**'
2834
- 'packages/stack/src/eql/v3/**'
35+
# Source layers the adapter's encoding/round-trip rests on: a break here
36+
# (not just under src/supabase) can produce wrong wire output or rows, so
37+
# trigger the live suite that would catch it.
38+
- 'packages/stack/src/encryption/**'
39+
- 'packages/stack/src/schema/**'
40+
- 'packages/schema/**'
2941
- 'packages/stack/integration/**'
3042
- 'packages/test-kit/**'
3143
- 'packages/cli/src/installer/**'
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { afterEach, describe, expect, it } from 'vitest'
2+
import { clerkJwtProvider } from '../integration/helpers/clerk'
3+
4+
/**
5+
* `clerkJwtProvider`'s "FAIL rather than skip" guard is the load-bearing half of
6+
* the identity suites' no-silent-skip contract: if it were ever weakened to a
7+
* skip or a silent `undefined`, the entire identity suite would go dark while CI
8+
* stayed green. The throw is pure logic — it fires before `createClerkClient`,
9+
* so it needs no live Clerk and belongs here in the credential-free unit suite
10+
* rather than the integration config (which only globs `*.integration.test.ts`).
11+
*/
12+
describe('clerkJwtProvider', () => {
13+
const saved = process.env.CLERK_MACHINE_TOKEN
14+
const savedB = process.env.CLERK_MACHINE_TOKEN_B
15+
afterEach(() => {
16+
if (saved === undefined) delete process.env.CLERK_MACHINE_TOKEN
17+
else process.env.CLERK_MACHINE_TOKEN = saved
18+
if (savedB === undefined) delete process.env.CLERK_MACHINE_TOKEN_B
19+
else process.env.CLERK_MACHINE_TOKEN_B = savedB
20+
})
21+
22+
it('throws (does not skip) when the default machine token env var is unset', () => {
23+
delete process.env.CLERK_MACHINE_TOKEN
24+
expect(() => clerkJwtProvider()).toThrow(/missing CLERK_MACHINE_TOKEN/)
25+
})
26+
27+
it('names the custom env var it was handed', () => {
28+
delete process.env.CLERK_MACHINE_TOKEN_B
29+
expect(() => clerkJwtProvider('CLERK_MACHINE_TOKEN_B')).toThrow(
30+
/CLERK_MACHINE_TOKEN_B/,
31+
)
32+
})
33+
})

packages/stack/__tests__/drizzle-v3/operators.test-d.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1+
import type { Result } from '@byteslice/result'
12
import { describe, expectTypeOf, it } from 'vitest'
23
import type { EncryptionClient } from '@/encryption'
4+
import type { AuditConfig } from '@/encryption/operations/base-operation'
35
import type { EncryptionV3 } from '@/encryption/v3'
46
import { createEncryptionOperatorsV3 } from '@/eql/v3/drizzle'
7+
import type { EncryptionError } from '@/errors'
8+
import type { LockContext } from '@/identity'
9+
import type { Encrypted } from '@/types'
510

611
/**
712
* Static regression guard for M1: `createEncryptionOperatorsV3` must accept the
@@ -27,9 +32,34 @@ describe('createEncryptionOperatorsV3 - client parameter (M1)', () => {
2732
})
2833

2934
it('accepts a minimal structural { encrypt } double', () => {
35+
// The double now models what the real client returns: an operation whose
36+
// `then` resolves a `Result<Encrypted, …>`, not `unknown`. That is the point
37+
// of the un-erasure — a `{ encrypt }` returning `unknown` no longer
38+
// typechecks, because the factory reads `result.data` as an `Encrypted`
39+
// envelope rather than casting it.
40+
type Op = {
41+
withLockContext(lc: LockContext): Op
42+
audit(cfg: AuditConfig): Op
43+
then: PromiseLike<Result<Encrypted, EncryptionError>>['then']
44+
}
3045
const double = {
31-
encrypt: (_plaintext: never, _opts: never) => ({}) as unknown,
46+
encrypt: (_plaintext: never, _opts: never) => ({}) as Op,
3247
}
3348
expectTypeOf(createEncryptionOperatorsV3).toBeCallableWith(double)
3449
})
50+
51+
it('rejects a { encrypt } double that returns `unknown` (the erasure regression)', () => {
52+
// The complement of the test above, and the one with teeth: `toBeCallableWith`
53+
// a correctly-typed double keeps passing even if the client contract
54+
// regressed to `unknown` (a correct value is assignable to `unknown`), so it
55+
// cannot catch re-erasure. This can. `encrypt` returning `unknown` must NOT
56+
// satisfy the factory, whose contract is `ChainableOperation<Encrypted>`; if
57+
// the erasure ever comes back, the `@ts-expect-error` goes unused and fails.
58+
const erased = {
59+
encrypt: (_plaintext: never, _opts: never): unknown => ({}),
60+
}
61+
// @ts-expect-error — `encrypt` returning `unknown` does not satisfy the
62+
// factory's `ChainableOperation<Encrypted>` client contract.
63+
createEncryptionOperatorsV3(erased)
64+
})
3565
})

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,4 +1901,29 @@ describe('v3 in-list term encryption batches by column', () => {
19011901
expect(status).toBe(500)
19021902
expect(error?.message).toMatch(/1 term(s)? for 2 value(s)?/)
19031903
})
1904+
1905+
it('rejects a bulk response with a null envelope rather than sending "null"', async () => {
1906+
const supabase = createMockSupabase()
1907+
const encryption = createMockEncryptionClient()
1908+
// A length-matched response, but position 0 is a null envelope. Without the
1909+
// guard, `JSON.stringify(null)` would send the literal `"null"` as the `in`
1910+
// operand — matching whatever `"null"` encodes to instead of failing.
1911+
;(
1912+
encryption as unknown as { bulkEncrypt: (...a: unknown[]) => unknown }
1913+
).bulkEncrypt = () =>
1914+
operation([{ data: null }, { data: fakeEnvelope('grace', 'nickname') }])
1915+
1916+
const { error, status } = await new EncryptedQueryBuilderV3Impl(
1917+
'users',
1918+
users,
1919+
encryption,
1920+
supabase.client,
1921+
USERS_ALL_COLUMNS,
1922+
)
1923+
.select('id')
1924+
.in('nickname', ['ada', 'grace'])
1925+
1926+
expect(status).toBe(500)
1927+
expect(error?.message).toMatch(/null envelope at position 0/)
1928+
})
19041929
})

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const users = encryptedTable('users', {
1818
active: types.Boolean('active'),
1919
nickname: types.TextEq('nickname'),
2020
bio: types.TextMatch('bio'),
21+
score: types.IntegerOrdOre('score'),
2122
})
2223

2324
type UserRow = InferPlaintext<typeof users>
@@ -130,6 +131,11 @@ describe('encryptedSupabaseV3 typed surface (with schemas)', () => {
130131
builder.order('nickname')
131132
// @ts-expect-error — bio is public.eql_v3_text_match: match only
132133
builder.order('bio')
134+
// @ts-expect-error — score is public.eql_v3_integer_ord_ore: ORE-backed, so
135+
// orderAndRange-capable but NOT sortable through a jsonb path (its `ob` term
136+
// needs the superuser-only ORE opclass). Excluded at compile time to match
137+
// the runtime rejection.
138+
builder.order('score')
133139
})
134140

135141
it('still allows order() on a plaintext row key', async () => {

packages/stack/integration/drizzle-v3/relational.integration.test.ts

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
eqlTypeSlug as slug,
2929
sortedKeysFor as sortedKeysForKit,
3030
typedEntries,
31+
unwrapResult,
3132
V3_MATRIX,
3233
} from '@cipherstash/test-kit'
3334
import {
@@ -52,9 +53,15 @@ import { makeEqlV3Column } from '@/eql/v3/drizzle/column'
5253

5354
const sqlClient = postgres(databaseUrl(), { prepare: false })
5455

55-
const TABLE_NAME = 'protect_ci_v3_drizzle_matrix'
56-
const ACCOUNT_TABLE_NAME = 'protect_ci_v3_drizzle_matrix_accounts'
57-
const RUN = `run-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`
56+
// Per-run table suffix so two runs sharing a database (a persistent/reused CI
57+
// database, a developer's local DB, or re-enabled file parallelism) never
58+
// operate on the same physical table — one run's `beforeAll` DROP would
59+
// otherwise blow away a table another run is mid-query on. Mirrors the family
60+
// suites' run-scoped naming (`rows.ts` `planTable`).
61+
const RID = Math.random().toString(36).slice(2, 8)
62+
const TABLE_NAME = `protect_ci_v3_drizzle_matrix_${RID}`
63+
const ACCOUNT_TABLE_NAME = `protect_ci_v3_drizzle_matrix_accounts_${RID}`
64+
const RUN = `run-${Date.now()}-${RID}`
5865
const ROW_A = 'row-a'
5966
const ROW_B = 'row-b'
6067
// A third row. With only two rows every predicate can return just [A], [B],
@@ -103,7 +110,7 @@ const accountsTable = pgTable(ACCOUNT_TABLE_NAME, {
103110
// known at compile time, so it exercises A3 end-to-end with ZERO casts: the
104111
// insert takes envelope rows and the select yields `Encrypted` values ready for
105112
// decrypt.
106-
const BIGINT_TABLE_NAME = 'protect_ci_v3_drizzle_bigint'
113+
const BIGINT_TABLE_NAME = `protect_ci_v3_drizzle_bigint_${RID}`
107114
const bigintTable = pgTable(BIGINT_TABLE_NAME, {
108115
id: integer('id').primaryKey().generatedAlwaysAsIdentity(),
109116
rowKey: text('row_key').notNull(),
@@ -190,11 +197,6 @@ async function selectRowKeys(condition: SQL | undefined): Promise<string[]> {
190197
return rows.map((row) => row.rowKey)
191198
}
192199

193-
function unwrap<T>(result: { data?: T; failure?: { message: string } }): T {
194-
if (result.failure) throw new Error(result.failure.message)
195-
return result.data as T
196-
}
197-
198200
function encryptedInsertRows(): MatrixPlainRow[] {
199201
return ROWS.map((rowKey) => {
200202
const row: MatrixPlainRow = {
@@ -218,12 +220,13 @@ beforeAll(async () => {
218220
.map(([eqlType]) => `"${slug(eqlType)}" ${eqlType} NOT NULL`)
219221
.join(',\n ')
220222

221-
// DROP, not `CREATE TABLE IF NOT EXISTS`. A table left by an earlier run keeps
222-
// its old columns, so a change to the catalog silently reuses the stale schema.
223-
// That bit: after the `_ord_ore` domains were filtered out of `matrixEntries`,
224-
// the leftover table still carried its nine ORE columns, and on managed
225-
// Postgres every INSERT raised `ore_domain_unavailable` — even leaving those
226-
// columns NULL, because the domain CHECK calls a function that RAISEs.
223+
// Table names are run-scoped (see RID), so DROP IF EXISTS is normally a no-op;
224+
// it stays as a belt-and-braces guard against an id collision. Recreating from
225+
// scratch each run also means a catalog change can never silently reuse a
226+
// stale schema — the bug that bit once when the `_ord_ore` domains were
227+
// filtered out of `matrixEntries` but a leftover fixed-name table kept its nine
228+
// ORE columns, making every INSERT raise `ore_domain_unavailable` on managed
229+
// Postgres (the domain CHECK RAISEs even for a NULL value).
227230
await sqlClient.unsafe(`
228231
DROP TABLE IF EXISTS ${TABLE_NAME}
229232
`)
@@ -260,7 +263,7 @@ beforeAll(async () => {
260263
)
261264
`)
262265

263-
const encryptedRows = unwrap(
266+
const encryptedRows = unwrapResult(
264267
await client.bulkEncryptModels(encryptedInsertRows(), schema),
265268
)
266269
await db.insert(matrixTable).values(encryptedRows)
@@ -276,7 +279,7 @@ beforeAll(async () => {
276279
// ROW_B exists so the filter proofs below have a row they must EXCLUDE. On a
277280
// one-row table `gt(balance, 0n)` returning every row is indistinguishable
278281
// from it returning the right row.
279-
const bigintRows = unwrap(
282+
const bigintRows = unwrapResult(
280283
await client.bulkEncryptModels(
281284
[
282285
{
@@ -299,9 +302,11 @@ beforeAll(async () => {
299302
}, 120000)
300303

301304
afterAll(async () => {
302-
await sqlClient`DELETE FROM ${sqlClient(TABLE_NAME)} WHERE test_run_id = ${RUN}`
303-
await sqlClient`DELETE FROM ${sqlClient(ACCOUNT_TABLE_NAME)} WHERE test_run_id = ${RUN}`
304-
await sqlClient`DELETE FROM ${sqlClient(BIGINT_TABLE_NAME)} WHERE test_run_id = ${RUN}`
305+
// Tables are run-scoped, so drop them outright rather than DELETE-ing rows —
306+
// no other run shares them, and this leaves nothing behind on a persistent DB.
307+
await sqlClient.unsafe(`DROP TABLE IF EXISTS ${TABLE_NAME}`)
308+
await sqlClient.unsafe(`DROP TABLE IF EXISTS ${ACCOUNT_TABLE_NAME}`)
309+
await sqlClient.unsafe(`DROP TABLE IF EXISTS ${BIGINT_TABLE_NAME}`)
305310
await sqlClient.end()
306311
}, 30000)
307312

@@ -449,6 +454,25 @@ describe('v3 drizzle — relational, needle guards, pagination', () => {
449454
expect(rows).toEqual([ROW_B, ROW_C])
450455
}, 30000)
451456

457+
it('not(between()) negates the whole range, not just the lower bound', async () => {
458+
// integer_ord: ROW_A=0, ROW_B=-42, ROW_C=2147483647. `not(between(0, 0))`
459+
// must return every row whose value != 0 → ROW_B and ROW_C.
460+
//
461+
// `between` emits a TWO-clause conjunction, and Drizzle's passthrough `not`
462+
// renders a bare `NOT <cond>`. Postgres binds NOT tighter than AND, so this
463+
// only works because `v3Dialect.range` already parenthesises `(gte AND lte)`.
464+
// Without those parens, `NOT gte(0) AND lte(0)` parses as
465+
// `value < 0 AND value <= 0` = `value < 0` = ROW_B alone, silently dropping
466+
// ROW_C. Asserting ROW_C is present is what discriminates the two — a
467+
// single-bound complement would satisfy the buggy form too.
468+
const rows = await selectRowKeys(
469+
ops.not(
470+
await ops.between(matrixColumn('public.eql_v3_integer_ord'), 0, 0),
471+
),
472+
)
473+
expect(rows).toEqual([ROW_B, ROW_C])
474+
}, 30000)
475+
452476
it('isNull and isNotNull work on nullable encrypted columns', async () => {
453477
expect(await selectRowKeys(ops.isNull(matrixTable.nullableTextEq))).toEqual(
454478
[ROW_A],
@@ -578,7 +602,7 @@ describe('v3 drizzle — relational, needle guards, pagination', () => {
578602
)
579603
expect(encrypted).toHaveLength(1)
580604

581-
const decrypted = unwrap(
605+
const decrypted = unwrapResult(
582606
await client.decryptModel(encrypted[0], bigintSchema),
583607
)
584608
expect(decrypted.balance).toBe(BIGINT_BALANCE)

0 commit comments

Comments
 (0)