Skip to content

Commit e27ce9d

Browse files
committed
refactor(stack): review wording/hygiene for v3 JSON (no behaviour change)
Addresses the PR #621 review's wording/classification/test-hygiene points (design changes split to #622/#623): - Sweep `ste_vec` out of user-facing comments/test names/skills → "encrypted JSONB document" / `eql_v3_json`; keep `ste_vec` only where it is literally the protect-ffi config key (`indexes.ste_vec`) or the `SteVecDocument` payload type. - json-contains suite: drop the `makeEqlV3Column` wrapper (use the Drizzle `types.Json`), which lets both `as never` casts go (the pgTable and the bulkEncryptModels call) — the table is now properly typed; un-alias the Drizzle `eq`/`asc` imports. - operators unit: rename `storageDomains` → `nonScalarQueryDomains` and fix the comment — json is a QUERYABLE (containment) domain that answers no scalar op, not a storage-only one. - Reframe json's `deferred` marker across catalog/families/run-family-suite/ test: it means "not run by the scalar op-matrix" (covered by dedicated suites), NOT unimplemented — distinct from the ORE domains' superuser-only deferral. - Rewrite the opaque introspect comment as the explicit three-way case. - Skill: describe JSON as containment-only today; mark JSONPath selector as not-yet-implemented (tracked in #623); note `JsonDocument` (no top-level scalar). Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
1 parent 58b9ef8 commit e27ce9d

11 files changed

Lines changed: 101 additions & 77 deletions

File tree

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,18 +115,21 @@ const orderDomains = matrixEntries.filter(
115115
([, spec]) => spec.indexes.ore || spec.indexes.ope,
116116
)
117117
const matchDomains = matrixEntries.filter(([, spec]) => spec.indexes.match)
118-
const storageDomains = matrixEntries.filter(
118+
// Domains with NO scalar query index (unique/ore/ope/match), so `eq`/`ne`/
119+
// ordering all throw. This is the truly storage-only scalar domains PLUS
120+
// `eql_v3_json` — json is a QUERYABLE domain (containment), it just answers no
121+
// scalar op, so it lands here for the eq/order rejection checks.
122+
const nonScalarQueryDomains = matrixEntries.filter(
119123
([, spec]) =>
120124
!spec.indexes.unique &&
121125
!spec.indexes.ore &&
122126
!spec.indexes.ope &&
123127
!spec.indexes.match,
124128
)
125-
// Storage-only domains that also lack an ste_vec index: these reject `contains`
126-
// outright. A json (`ste_vec`) column is storage-only for eq/order but DOES
127-
// answer `contains` via `@>`, so it is excluded from the containment-rejection
128-
// group and covered by its own positive assertion below.
129-
const noContainmentDomains = storageDomains.filter(
129+
// Of those, the ones that also reject `contains` — i.e. everything except json.
130+
// json answers `contains` via `@>`, so it is excluded here and gets its own
131+
// positive assertion in the JSON containment describe above.
132+
const noContainmentDomains = nonScalarQueryDomains.filter(
130133
([, spec]) => !spec.indexes.ste_vec,
131134
)
132135

@@ -477,8 +480,8 @@ describe('createEncryptionOperatorsV3 - JSON containment', () => {
477480
})
478481
})
479482

480-
describe('createEncryptionOperatorsV3 - storage-only domains', () => {
481-
it.each(storageDomains)('%s eq throws', async (eqlType, spec) => {
483+
describe('createEncryptionOperatorsV3 - domains with no scalar query', () => {
484+
it.each(nonScalarQueryDomains)('%s eq throws', async (eqlType, spec) => {
482485
const { ops } = setup()
483486
await expect(
484487
ops.eq(matrixColumn(eqlType), sampleFor(spec)),
@@ -492,7 +495,7 @@ describe('createEncryptionOperatorsV3 - storage-only domains', () => {
492495
).rejects.toBeInstanceOf(EncryptionOperatorError)
493496
})
494497

495-
it.each(storageDomains)('%s asc throws synchronously', (eqlType) => {
498+
it.each(nonScalarQueryDomains)('%s asc throws synchronously', (eqlType) => {
496499
const { ops } = setup()
497500
expect(() => ops.asc(matrixColumn(eqlType))).toThrow(
498501
EncryptionOperatorError,

packages/stack/__tests__/test-kit-families.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ describe('test-kit families partition the v3 catalog', () => {
7070
'text_ord_ore',
7171
'timestamp_ord_ore',
7272
])
73-
// ORE domains defer for the superuser-only opclass; json defers because
74-
// ste_vec containment isn't in the scalar op matrix.
73+
// ORE domains defer for the superuser-only opclass; json defers because it
74+
// is queried by containment, not the scalar op-matrix (covered by dedicated
75+
// suites — see catalog reason).
7576
for (const { bare, reason } of deferred) {
76-
expect(reason).toMatch(bare === 'json' ? /ste_vec/ : /superuser-only/)
77+
expect(reason).toMatch(bare === 'json' ? /containment/ : /superuser-only/)
7778
}
7879
})
7980

packages/stack/integration/drizzle-v3/json-contains.integration.test.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,24 @@
22
* Live JSON containment for the v3 `types.Json()` column through the Drizzle
33
* operators. Seeds encrypted JSONB documents and queries them with
44
* `ops.contains(col, subObject)` — the same operator text search uses, but on a
5-
* `json` column it emits the `@>` operator over the ste_vec document (json has
6-
* no `eql_v3.contains` overload) with a `query_jsonb` needle from
5+
* `json` column it emits the `@>` operator over the encrypted JSONB document
6+
* (json has no `eql_v3.contains` overload) with a `query_jsonb` needle from
77
* `encryptQuery` — asserting it returns exactly the rows whose document contains
88
* the sub-object (jsonb `@>` semantics), and excludes the rest.
99
*/
1010
import { databaseUrl, unwrapResult } from '@cipherstash/test-kit'
11-
import { and, asc as drizzleAsc, eq as drizzleEq, type SQL } from 'drizzle-orm'
11+
import { and, asc, eq, type SQL } from 'drizzle-orm'
1212
import { integer, pgTable, text } from 'drizzle-orm/pg-core'
1313
import { drizzle } from 'drizzle-orm/postgres-js'
1414
import postgres from 'postgres'
1515
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
16-
import { EncryptionV3, types } from '@/encryption/v3'
16+
import { EncryptionV3 } from '@/encryption/v3'
17+
import type { JsonDocument } from '@/eql/v3'
1718
import {
1819
createEncryptionOperatorsV3,
1920
extractEncryptionSchemaV3,
21+
types,
2022
} from '@/eql/v3/drizzle'
21-
import { makeEqlV3Column } from '@/eql/v3/drizzle/column'
2223

2324
const sqlClient = postgres(databaseUrl(), { prepare: false })
2425

@@ -29,13 +30,13 @@ const docTable = pgTable(TABLE_NAME, {
2930
id: integer('id').primaryKey().generatedAlwaysAsIdentity(),
3031
rowKey: text('row_key').notNull(),
3132
testRunId: text('test_run_id').notNull(),
32-
doc: makeEqlV3Column(types.Json('doc')),
33-
} as never)
33+
doc: types.Json('doc'),
34+
})
3435

3536
const schema = extractEncryptionSchemaV3(docTable)
3637

3738
// Distinct documents so each containment query has a definite expected set.
38-
const DOCS: Record<string, Record<string, unknown>> = {
39+
const DOCS: Record<string, JsonDocument> = {
3940
ada: { user: 'ada@example.com', roles: ['admin', 'eng'], active: true },
4041
grace: { user: 'grace@example.com', roles: ['eng'] },
4142
zoe: { user: 'zoe@example.com', roles: ['ops'], active: false },
@@ -50,8 +51,8 @@ async function matching(condition: SQL): Promise<string[]> {
5051
const rows = (await db
5152
.select({ rowKey: docTable.rowKey })
5253
.from(docTable)
53-
.where(and(drizzleEq(docTable.testRunId, RUN), condition))
54-
.orderBy(drizzleAsc(docTable.rowKey))) as SelectRow[]
54+
.where(and(eq(docTable.testRunId, RUN), condition))
55+
.orderBy(asc(docTable.rowKey))) as SelectRow[]
5556
return rows.map((row) => row.rowKey)
5657
}
5758

@@ -77,7 +78,7 @@ beforeAll(async () => {
7778
doc,
7879
}))
7980
const encrypted = unwrapResult(
80-
await client.bulkEncryptModels(rows as never, schema),
81+
await client.bulkEncryptModels(rows, schema),
8182
) as Array<Record<string, unknown>>
8283
await db.insert(docTable).values(encrypted as never)
8384
}, 120000)

packages/stack/integration/shared/json-crypto.integration.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/**
22
* Live round-trip for the v3 `types.Json()` column — an encrypted JSONB document
3-
* stored as an ste_vec `SteVecDocument`. Proves the new json column model
4-
* encrypts and decrypts a real document through protect-ffi (no DB query here;
5-
* containment is exercised by the Drizzle json suite).
3+
* (`public.eql_v3_json`). Proves the new json column model encrypts and decrypts
4+
* a real document through protect-ffi (no DB query here; containment is exercised
5+
* by the Drizzle json suite). The stored payload is protect-ffi's
6+
* `SteVecDocument`, carrying an `sv` array rather than a scalar `c` ciphertext.
67
*/
78
import { unwrapResult } from '@cipherstash/test-kit'
89
import { beforeAll, describe, expect, it } from 'vitest'
@@ -12,7 +13,7 @@ const docs = encryptedTable('v3_json_docs', {
1213
profile: types.Json('profile'),
1314
})
1415

15-
describe('v3 typed client — JSON (ste_vec) round-trip', () => {
16+
describe('v3 typed client — encrypted JSONB round-trip', () => {
1617
let client: Awaited<ReturnType<typeof EncryptionV3<[typeof docs]>>>
1718

1819
beforeAll(async () => {
@@ -30,7 +31,8 @@ describe('v3 typed client — JSON (ste_vec) round-trip', () => {
3031
const encrypted = unwrapResult(
3132
await client.encrypt(value, { table: docs, column: docs.profile }),
3233
)
33-
// An ste_vec document carries an `sv` array, not a scalar `c` ciphertext.
34+
// An encrypted JSONB document carries an `sv` array, not a scalar `c`
35+
// ciphertext (the stored shape is protect-ffi's `SteVecDocument`).
3436
expect(Array.isArray((encrypted as { sv?: unknown }).sv)).toBe(true)
3537

3638
const decrypted = unwrapResult(await client.decrypt(encrypted))
@@ -46,7 +48,7 @@ describe('v3 typed client — JSON (ste_vec) round-trip', () => {
4648

4749
// A JSON document is not only an object: `types.Json` (and the `JsonDocument`
4850
// plaintext type) also admit a top-level array and `null`. Pin both so a
49-
// regression in ste_vec encoding of non-object roots turns a test red.
51+
// regression in encrypted-JSONB encoding of non-object roots turns a test red.
5052
it.each([
5153
['array root', [1, 'two', { three: 3 }]],
5254
['null root', null],

packages/stack/integration/supabase/introspect.integration.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,13 @@ beforeAll(async () => {
2828
// as unmodelled — it is an ordinary plaintext passthrough.
2929
await sql.unsafe(`DROP DOMAIN IF EXISTS public.${USER_DOMAIN}`)
3030
await sql.unsafe(`CREATE DOMAIN public.${USER_DOMAIN} AS jsonb`)
31-
// Mixed EQL columns: `score` is typed with an EQL domain the SDK has NO types
32-
// factory for (unmodelled → reported); `doc` is `eql_v3_json`, which `types.Json`
33-
// DOES model (so it must be synthesized, not reported); `own` is a user's plain
34-
// jsonb domain (plaintext passthrough, never reported).
31+
// This table exercises all three cases the introspector must tell apart:
32+
// score → EQL domain (eql_v3_integer_ord_ope) the SDK has no factory for
33+
// → "unmodelled", MUST be reported so the caller knows it can't query it
34+
// doc → EQL domain (eql_v3_json) the SDK models via types.Json
35+
// → modelled, MUST be synthesized and NOT reported
36+
// own → a user's own plain jsonb domain with no EQL marker
37+
// → plaintext passthrough, MUST never be reported
3538
await sql.unsafe(`
3639
CREATE TABLE ${UNMODELLED} (
3740
id INTEGER PRIMARY KEY GENERATED ALWAYS AS IDENTITY,

packages/stack/src/eql/v3/columns.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,10 @@ function indexesForCapabilities(
369369
}
370370

371371
if (capabilities.searchableJson) {
372-
// Encrypted-JSONB (ste_vec) index. `prefix: 'enabled'` is a sentinel the
373-
// table's `build()` rewrites to `${tableName}/${columnName}` — the same
374-
// scheme v2 `searchableJson` uses — so each document's selectors are scoped
375-
// to their column.
372+
// Encrypted-JSONB index (the protect-ffi config kind is named `ste_vec`).
373+
// `prefix: 'enabled'` is a sentinel the table's `build()` rewrites to
374+
// `${tableName}/${columnName}` — the same scheme v2 `searchableJson` uses —
375+
// so each document's selectors are scoped to their column.
376376
//
377377
// `array_index_mode` indexes array elements by identity (`item`) and enables
378378
// `$[*]` wildcard selectors, but deliberately drops `position`: `@>`
@@ -381,8 +381,8 @@ function indexesForCapabilities(
381381
// With positional terms emitted (v2 `searchableJson`'s `'all'`), the needle
382382
// for `x`-at-index-0 would miss a document holding `x` at index 1.
383383
//
384-
// `mode: 'compat'` is REQUIRED for eql-3.0.0: `public.eql_v3_json` orders
385-
// ste_vec entries by the CLLW-OPE `op` term, so the index must emit `op`
384+
// `mode: 'compat'` is REQUIRED for eql-3.0.0: `public.eql_v3_json` orders the
385+
// document's entries by the CLLW-OPE `op` term, so the index must emit `op`
386386
// (compat) terms. `'standard'` emits v2's CLLW-ORE `oc` terms, which the v3
387387
// domain rejects at encrypt time.
388388
indexes.ste_vec = {
@@ -633,10 +633,11 @@ const JSON_DOMAIN = {
633633
} as const
634634

635635
/**
636-
* Builder for a `public.eql_v3_json` column — an encrypted JSONB document,
637-
* stored as an ste_vec `SteVecDocument` and queried by containment. The document
638-
* (any {@link JsonValue}) round-trips through `encrypt`/`decrypt`; structural
639-
* queries use the ste_vec index emitted by `build()`.
636+
* Builder for a `public.eql_v3_json` column — an encrypted JSONB document
637+
* (a {@link JsonDocument}: object, array, or null). It round-trips through
638+
* `encrypt`/`decrypt`, and containment queries use the encrypted-JSONB index
639+
* emitted by `build()`. (The stored payload is protect-ffi's `SteVecDocument`,
640+
* and the config index kind is `ste_vec` — both protect-ffi names.)
640641
*/
641642
export class EncryptedJsonColumn extends EncryptedV3Column<typeof JSON_DOMAIN> {
642643
constructor(columnName: string) {

packages/stack/src/eql/v3/drizzle/operators.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,11 @@ const MAX_IN_ARRAY_CONCURRENCY = 4
4343
*
4444
* `bulkEncrypt` is optional so a `{ encrypt }`-only client stays valid; the
4545
* list operators fall back to bounded-concurrency single encryption without it.
46-
* `encryptQuery` is likewise optional — only JSON containment (`@>`) needs it,
47-
* to build a `query_jsonb` needle; the `contains()` branch guards its absence.
46+
* `encryptQuery` is optional only because today it is used by a single operator
47+
* — JSON containment (`@>`), which needs a ciphertext-free `query_jsonb` needle;
48+
* the `contains()` branch guards its absence. Every OTHER operator still encrypts
49+
* its operand with `encrypt` (a full storage envelope). Unifying all operands
50+
* onto `encryptQuery` — after which it stops being optional — is tracked in #622.
4851
*/
4952
type OperandEncryptionClient = {
5053
encrypt(
@@ -233,9 +236,10 @@ export function createEncryptionOperatorsV3(
233236
const ORDERING_INDEXES = ['ore', 'ope'] as const
234237
// `contains` answers two shapes: bloom free-text (`match`, a `text_search`/
235238
// `text_match` column), which emits `eql_v3.contains(col, operand)`, and
236-
// encrypted-JSONB containment (`ste_vec`, a `json` column), which emits the
237-
// `@>` operator instead — json has no `eql_v3.contains` overload. The branch
238-
// in `contains()` picks the right SQL by index kind.
239+
// encrypted-JSONB containment (an `eql_v3_json` column, whose config carries
240+
// the `ste_vec` index kind), which emits the `@>` operator instead — json has
241+
// no `eql_v3.contains` overload. The branch in `contains()` picks the right SQL
242+
// by index kind.
239243
const CONTAINMENT_INDEXES = ['match', 'ste_vec'] as const
240244

241245
function applyOperationOptions(
@@ -448,7 +452,7 @@ export function createEncryptionOperatorsV3(
448452

449453
/**
450454
* Build a `query_jsonb` containment needle for a `json` column. Uses
451-
* `encryptQuery` (not `encrypt`): the ste_vec query term carries no ciphertext
455+
* `encryptQuery` (not `encrypt`): the JSON query term carries no ciphertext
452456
* and satisfies the `eql_v3.query_jsonb` CHECK the `@>` overload needs.
453457
*/
454458
async function encryptJsonContainmentTerm(

packages/test-kit/src/catalog.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -668,11 +668,13 @@ export const V3_MATRIX = {
668668
errorSamples: NUM_ERR,
669669
},
670670

671-
// Encrypted JSONB document (ste_vec). DEFERRED from the scalar family driver:
672-
// it is queried by containment/selector, not the eq/ord/match ops the oracle
673-
// models, so it is covered by a dedicated json integration suite. The row
674-
// still pins the built shape (`cast_as: 'json'` + the ste_vec index) and
675-
// carries representative document samples.
671+
// Encrypted JSONB document. Excluded from the SCALAR family driver (that is
672+
// what `deferred` marks here) — NOT unimplemented: JSON is fully covered by
673+
// its own live suites (`json-crypto`, `json-contains`). It is queried by
674+
// containment (the `@>` operator), not the eq/ord/match ops the scalar oracle
675+
// models, so `runFamilySuite` cannot exercise it. The row still pins the built
676+
// shape (`cast_as: 'json'` + the encrypted-JSONB index) and carries
677+
// representative document samples.
676678
'public.eql_v3_json': {
677679
builder: types.Json,
678680
ColumnClass: EncryptedJsonColumn,
@@ -695,7 +697,7 @@ export const V3_MATRIX = {
695697
{ user: 'grace@example.com', roles: ['eng'] },
696698
],
697699
deferred:
698-
'json is queried by ste_vec containment/selector, not the scalar op ' +
699-
'matrix — covered by a dedicated json integration suite',
700+
'json is covered by dedicated live suites (json-crypto, json-contains), ' +
701+
'not the scalar op-matrix: it is queried by containment, not eq/ord/match',
700702
},
701703
} as const satisfies Record<EqlV3TypeName, DomainSpec>

packages/test-kit/src/families.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,12 @@ const FAMILY_PREFIXES: Readonly<Record<FamilyName, readonly string[]>> = {
4747
timestamp: ['timestamp'],
4848
text: ['text'],
4949
boolean: ['boolean'],
50-
// The json family's only domain (`eql_v3_json`) is `deferred`: ste_vec
51-
// containment doesn't fit the scalar oracle, so it's covered by a dedicated
52-
// suite, not `runFamilySuite`. The prefix still owns it for coverage
53-
// accounting.
50+
// `json` is here only for coverage accounting — its one domain (`eql_v3_json`)
51+
// is marked `deferred`, meaning "not run by the scalar op-matrix", because JSON
52+
// is queried by containment (`@>`), not the eq/ord/match ops the oracle models.
53+
// It is NOT unimplemented: dedicated live suites (`json-crypto`, `json-contains`)
54+
// cover it. (Contrast the ORE domains, also `deferred` but because their opclass
55+
// is superuser-only and cannot run on managed Postgres.)
5456
json: ['json'],
5557
}
5658

packages/test-kit/src/run-family-suite.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,13 @@ export function runFamilySuite(
151151
const adapter = makeAdapter()
152152
const domains = domainsForFamily(family)
153153

154-
// A family whose every domain is deferred (e.g. `json`: ste_vec containment
155-
// and selector queries don't fit the scalar oracle this driver runs). It
156-
// carries no op-matrix coverage here — its real coverage lives in dedicated
157-
// suites (`json-crypto`, `json-contains`). Emit one assertion documenting the
158-
// deferral so the family stays visible and is not a silent skip, rather than
159-
// letting `planTable` throw on an empty column set. A family with NO domains
160-
// at all (covered or deferred) is a wiring bug and still falls through to that
161-
// throw.
154+
// A family whose every domain is deferred (e.g. `json`: containment queries
155+
// don't fit the scalar oracle this driver runs). It carries no op-matrix
156+
// coverage here — its real coverage lives in dedicated suites (`json-crypto`,
157+
// `json-contains`). Emit one assertion documenting the deferral so the family
158+
// stays visible and is not a silent skip, rather than letting `planTable` throw
159+
// on an empty column set. A family with NO domains at all (covered or deferred)
160+
// is a wiring bug and still falls through to that throw.
162161
if (domains.length === 0) {
163162
const deferred = deferredForFamily(family)
164163
if (deferred.length > 0) {

0 commit comments

Comments
 (0)