Skip to content

Commit ecf273d

Browse files
committed
fix(stack): reject empty SteVec, pin ops.and positive path
Address review findings on the v3 Drizzle adapter. assertEnvelope accepted `sv: []`, which satisfies `sv !== undefined` but has no `sv[0].c` — the guard's own premise. An empty, non-array, or null `sv` carries a ciphertext key but no ciphertext, and reached decrypt as garbage. Require a non-empty array. No encrypt path produces this, so it is hardening against a corrupt or hostile row rather than a live fail-open. needleFor selected samples with `sample.length >= 3`, counting UTF-16 code units where the production guard counts codepoints. An astral sample ('👍👍': 4 units, 2 codepoints) would be picked here and then rejected by the guard, blaming the wrong line. Latent only because every match-domain sample is currently ASCII. Extract it to v3-matrix/needle-for.ts and select with matchNeedleError itself, so the two cannot diverge; it now also honours a domain's own token_length. Pairing `and` over disjoint rows proved it is not `or`, but left every live `and` assertion expecting []. A constant-false `and` would satisfy that, and the `or` tests exercise a different operator. Restore the intersecting pair alongside it, so one live test proves `and` returns its row. The ['email'] lock-context test pinned /^Failed to retrieve key/. Naming a claim does not assert the token carries it: resolveLockContext is a passthrough and ZeroKMS resolves the value server-side, so a token with no `email` claim may be refused by authorization before key derivation is attempted. Accept either denial class, reject infrastructure faults explicitly. Kept loose because CI never runs this path (USER_JWT unset, #530). Collapse exports.test.ts from three tests to two: the two typeof sweeps were one sweep artificially split. The remaining sweep iterates Object.entries, so it cannot silently skip a name the key list forgot. The v2 like/ilike path has the same bloom-containment fail-open and remains unfixed here, as the changeset already notes.
1 parent 7a76041 commit ecf273d

9 files changed

Lines changed: 176 additions & 47 deletions

File tree

.changeset/eql-v3-drizzle-fail-open-guards.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ an EQL envelope, instead of surfacing a raw `SyntaxError` for malformed JSON and
2424
passing a bare scalar through unchecked — `v3FromDriver('5')` previously returned
2525
`5` typed as `Encrypted`, which then reached `decrypt` as garbage. The guard
2626
accepts both scalar envelopes (ciphertext at `c`) and SteVec documents
27-
(ciphertext at `sv[0].c`). `EqlV3CodecError` is exported from
28-
`@cipherstash/stack/eql/v3/drizzle` so callers can catch it.
27+
(ciphertext at `sv[0].c`). A SteVec's `sv` must be a non-empty array: `sv[0]` is
28+
the decryption root, so `sv: []` carries a ciphertext key but no ciphertext, and
29+
is now rejected rather than passed to `decrypt`. `EqlV3CodecError` is exported
30+
from `@cipherstash/stack/eql/v3/drizzle` so callers can catch it.
2931

3032
Also removes an unreachable branch in `inArray`/`notInArray`, whose empty-list
3133
guard already throws before it.

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

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,21 @@ describe('v3 codec', () => {
5959
)
6060
})
6161

62+
// `sv: []` has no `sv[0]`, so the guard's premise — a ciphertext at the
63+
// decryption root — does not hold. Presence of the key is not enough.
64+
it.each([
65+
['an empty SteVec', { v: 3, k: 'sv', sv: [] }],
66+
['a non-array SteVec', { v: 3, k: 'sv', sv: {} }],
67+
['a null SteVec with no c', { v: 3, k: 'sv', sv: null }],
68+
])('rejects %s', (_label, envelope) => {
69+
expect(() => v3FromDriver(envelope as never)).toThrow(EqlV3CodecError)
70+
expect(() => v3FromDriver(envelope as never)).toThrow(/ciphertext/)
71+
})
72+
73+
it('still accepts a scalar envelope, which has c and no sv', () => {
74+
expect(v3FromDriver(ENVELOPE as never)).toBe(ENVELOPE)
75+
})
76+
6277
it('normalises null/undefined to SQL NULL (JS null) on read', () => {
6378
expect(v3FromDriver(null)).toBeNull()
6479
expect(v3FromDriver(undefined)).toBeNull()
@@ -85,20 +100,21 @@ describe('v3 codec', () => {
85100
// preserve. Flattening it to a message string discards the SyntaxError and
86101
// its stack, so a caller debugging a corrupt column loses where parsing
87102
// actually broke.
88-
it.each(['{bad', '', '{"v":3,'])(
89-
'preserves the underlying SyntaxError as `cause` for %j',
90-
(raw) => {
91-
let thrown: unknown
92-
try {
93-
v3FromDriver(raw)
94-
} catch (error) {
95-
thrown = error
96-
}
97-
98-
expect(thrown).toBeInstanceOf(EqlV3CodecError)
99-
expect((thrown as EqlV3CodecError).cause).toBeInstanceOf(SyntaxError)
100-
},
101-
)
103+
it.each([
104+
'{bad',
105+
'',
106+
'{"v":3,',
107+
])('preserves the underlying SyntaxError as `cause` for %j', (raw) => {
108+
let thrown: unknown
109+
try {
110+
v3FromDriver(raw)
111+
} catch (error) {
112+
thrown = error
113+
}
114+
115+
expect(thrown).toBeInstanceOf(EqlV3CodecError)
116+
expect((thrown as EqlV3CodecError).cause).toBeInstanceOf(SyntaxError)
117+
})
102118

103119
it.each([
104120
'5',

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,19 @@ describe('v3 drizzle barrel', () => {
2222
expect(Object.keys(barrel).sort()).toEqual([...PUBLIC_SURFACE])
2323
})
2424

25-
it('exports the public surface', () => {
26-
expect(typeof barrel.createEncryptionOperatorsV3).toBe('function')
27-
expect(typeof barrel.extractEncryptionSchemaV3).toBe('function')
28-
expect(typeof barrel.EncryptionOperatorError).toBe('function')
29-
expect(typeof barrel.EqlV3CodecError).toBe('function')
25+
// The key check above sees names, not values: a re-export that resolves to
26+
// `undefined` still lists its key. Sweep every export the barrel claims.
27+
it('exports every name as a callable, and types as a namespace', () => {
28+
const callables = Object.entries(barrel).filter(
29+
([name]) => name !== 'types',
30+
)
31+
expect(callables).toHaveLength(PUBLIC_SURFACE.length - 1)
32+
33+
for (const [name, value] of callables) {
34+
expect(typeof value, name).toBe('function')
35+
}
36+
3037
expect(typeof barrel.types.TextSearch).toBe('function')
3138
expect(barrel.types.IntegerOrd('age')).toBeDefined()
3239
})
33-
34-
it('re-exports the codec and column helpers as callables', () => {
35-
expect(typeof barrel.v3ToDriver).toBe('function')
36-
expect(typeof barrel.v3FromDriver).toBe('function')
37-
expect(typeof barrel.getEqlV3Column).toBe('function')
38-
expect(typeof barrel.isEqlV3Column).toBe('function')
39-
expect(typeof barrel.makeEqlV3Column).toBe('function')
40-
})
4140
})

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,22 @@ describeLivePg('v3 drizzle operators (live pg matrix)', () => {
666666
expect(rows).toEqual([])
667667
}, 30000)
668668

669+
// The disjoint pair above proves `and` is not `or`, but every `and` assertion
670+
// over it expects []. A constant-false `and` would satisfy that too, and the
671+
// `or` tests exercise a different operator. This pins the positive path:
672+
// an intersecting pair that must actually return its row.
673+
// text_eq = 'ada@example.com' -> ROW_B only
674+
// integer_ord < 0 -> ROW_B (-42) only
675+
it('and returns the rows satisfying both encrypted predicates', async () => {
676+
const rows = await selectRowKeys(
677+
await ops.and(
678+
ops.eq(matrixColumn('public.text_eq'), 'ada@example.com'),
679+
ops.lt(matrixColumn('public.integer_ord'), 0),
680+
),
681+
)
682+
expect(rows).toEqual([ROW_B])
683+
}, 30000)
684+
669685
it('or requires either encrypted predicate, unlike and', async () => {
670686
const rows = await selectRowKeys(await ops.or(...disjointPredicates()))
671687
expect(rows).toEqual([ROW_A, ROW_B, ROW_C])

packages/stack/__tests__/drizzle-v3/operators-lock-context-live-pg.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,27 @@ async function decryptOutcome(
116116
*/
117117
const KEY_DENIAL = /^Failed to retrieve key/
118118

119+
/**
120+
* Denial under a claim the token may not carry at all. Naming a claim does not
121+
* assert it exists: `resolveLockContext` is a passthrough, and ZeroKMS resolves
122+
* the claim's value from the authenticating token. So decrypting under
123+
* `['email']` a row sealed under `['sub']` either fails key derivation, or — if
124+
* the token has no `email` claim — is refused by the authorization layer before
125+
* key derivation is ever attempted. Both are denials; which one surfaces is a
126+
* ZeroKMS server-side detail we do not pin.
127+
*
128+
* What must NOT pass is an infrastructure fault masquerading as a denial, so
129+
* that is excluded separately. Kept loose rather than pinned to `KEY_DENIAL`
130+
* because no CI run exercises this path — `USER_JWT` is unset in CI (#530) —
131+
* and a wrong message-shape guess would only surface once that secret lands.
132+
*/
133+
const IDENTITY_DENIAL =
134+
/failed to retrieve key|unauthoriz|unauthoris|forbidden|denied|not authorized|not authorised/i
135+
136+
/** A transport/outage failure, which must never be mistaken for a denial. */
137+
const INFRA_FAULT =
138+
/ECONNREFUSED|ECONNRESET|ETIMEDOUT|ENOTFOUND|EAI_AGAIN|socket hang up|timed? ?out|network error/i
139+
119140
/** Run-scoped SELECT of row keys under an already-encrypted SQL condition. */
120141
async function selectRowKeys(condition: SQL): Promise<string[]> {
121142
const rows = (await db
@@ -271,6 +292,7 @@ describeLivePg('v3 drizzle operators with lock context (live pg)', () => {
271292
)
272293

273294
expect(denied).toBe(true)
274-
expect(message).toMatch(KEY_DENIAL)
295+
expect(message).toMatch(IDENTITY_DENIAL)
296+
expect(message).not.toMatch(INFRA_FAULT)
275297
}, 30000)
276298
})

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
typedEntries,
2424
V3_MATRIX,
2525
} from '../v3-matrix/catalog'
26+
import { needleFor } from '../v3-matrix/needle-for'
2627

2728
const TERM = { c: 'ct', v: 1 }
2829
const TERM_JSON = JSON.stringify(TERM)
@@ -99,21 +100,6 @@ const matrixColumn = (eqlType: string) =>
99100
const sampleFor = (spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX]) =>
100101
spec.samples[0]
101102

102-
// `contains` needles must reach the match tokenizer's `token_length` (3), so
103-
// they cannot come from `sampleFor` — `TEXT_S[0]` is the empty string, which
104-
// tokenizes to nothing and is rejected as unanswerable.
105-
const needleFor = (
106-
spec: (typeof V3_MATRIX)[keyof typeof V3_MATRIX],
107-
): string => {
108-
const needle = spec.samples.find(
109-
(sample) => typeof sample === 'string' && sample.length >= 3,
110-
)
111-
if (typeof needle !== 'string') {
112-
throw new Error('no searchable sample for a match domain')
113-
}
114-
return needle
115-
}
116-
117103
const equalityDomains = matrixEntries.filter(
118104
([, spec]) => spec.indexes.unique || spec.indexes.ore,
119105
)
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { matchNeedleError } from '@/schema/match-defaults'
3+
import { V3_MATRIX } from './catalog'
4+
import { needleFor } from './needle-for'
5+
6+
const MATCH_BLOCK = { tokenizer: { kind: 'ngram', token_length: 3 } } as const
7+
8+
const specWith = (samples: readonly unknown[]) =>
9+
({ samples, indexes: { match: MATCH_BLOCK } }) as never
10+
11+
describe('needleFor', () => {
12+
// The guard counts codepoints; a UTF-16 `.length` check would wave '👍👍'
13+
// through (4 code units, 2 codepoints) and the `contains` test would then
14+
// fail inside the production guard, pointing at the wrong line.
15+
it('skips an astral sample that the production guard would reject', () => {
16+
expect(needleFor(specWith(['👍👍', 'ada@example.com']))).toBe(
17+
'ada@example.com',
18+
)
19+
})
20+
21+
it('skips the empty sample', () => {
22+
expect(needleFor(specWith(['', 'Ada Lovelace']))).toBe('Ada Lovelace')
23+
})
24+
25+
it('skips non-string samples', () => {
26+
expect(needleFor(specWith([42, null, 'Ada']))).toBe('Ada')
27+
})
28+
29+
it('throws when no sample can be searched', () => {
30+
expect(() => needleFor(specWith(['', 'ab', '👍👍']))).toThrow(
31+
/no searchable sample/,
32+
)
33+
})
34+
35+
it('honours a larger token_length', () => {
36+
const spec = {
37+
samples: ['ada', 'adam'],
38+
indexes: { match: { tokenizer: { kind: 'ngram', token_length: 4 } } },
39+
} as never
40+
expect(needleFor(spec)).toBe('adam')
41+
})
42+
43+
// The contract that matters: whatever it picks, the guard must accept.
44+
it('never returns a needle the guard rejects, across every match domain', () => {
45+
const matchDomains = Object.values(V3_MATRIX).filter(
46+
(spec) => spec.indexes.match,
47+
)
48+
expect(matchDomains.length).toBeGreaterThan(0)
49+
50+
for (const spec of matchDomains) {
51+
const needle = needleFor(spec)
52+
expect(matchNeedleError(needle, spec.indexes.match ?? {})).toBeUndefined()
53+
}
54+
})
55+
})
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { matchNeedleError } from '@/schema/match-defaults'
2+
import type { V3_MATRIX } from './catalog'
3+
4+
type MatrixSpec = (typeof V3_MATRIX)[keyof typeof V3_MATRIX]
5+
6+
/**
7+
* Pick a sample from a match domain that can actually be used as a `contains`
8+
* needle. `sampleFor` is no good here — `TEXT_S[0]` is the empty string, which
9+
* tokenizes to nothing and is rejected as unanswerable.
10+
*
11+
* Selection runs the production predicate rather than a length check of our
12+
* own. A `sample.length >= 3` check would count UTF-16 code units where the
13+
* guard counts codepoints, so an astral sample ('👍👍': 4 units, 2 codepoints)
14+
* would be picked here and then thrown out by the guard, blaming the wrong
15+
* line. It would also ignore a domain that raises its own `token_length`.
16+
*/
17+
export const needleFor = (spec: MatrixSpec): string => {
18+
const match = spec.indexes.match ?? {}
19+
const needle = spec.samples.find(
20+
(sample) => typeof sample === 'string' && !matchNeedleError(sample, match),
21+
)
22+
if (typeof needle !== 'string') {
23+
throw new Error('no searchable sample for a match domain')
24+
}
25+
return needle
26+
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ export class EqlV3CodecError extends Error {
2121
* which has no top-level `c`. Checking `v` plus either carrier distinguishes an
2222
* envelope from a bare scalar, an array, or an unrelated object — without
2323
* paying a full structural validation on every decrypted row.
24+
*
25+
* The `sv` carrier must be a non-empty array: `sv[0]` is the decryption root,
26+
* so `sv: []` has a ciphertext key but no ciphertext, and would reach `decrypt`
27+
* as garbage.
2428
*/
2529
function assertEnvelope(value: unknown, source: string): Encrypted {
2630
if (typeof value !== 'object' || value === null || Array.isArray(value)) {
@@ -29,11 +33,14 @@ function assertEnvelope(value: unknown, source: string): Encrypted {
2933
)
3034
}
3135
const envelope = value as { v?: unknown; c?: unknown; sv?: unknown }
36+
// A SteVec carries its root ciphertext at `sv[0].c`, so an empty or non-array
37+
// `sv` carries no ciphertext at all — presence of the key is not enough.
38+
const hasSteVec = Array.isArray(envelope.sv) && envelope.sv.length > 0
3239
const missing =
3340
envelope.v === undefined
3441
? '"v"'
35-
: envelope.c === undefined && envelope.sv === undefined
36-
? 'a ciphertext ("c", or "sv" for a SteVec document)'
42+
: envelope.c === undefined && !hasSteVec
43+
? 'a ciphertext ("c", or a non-empty "sv" for a SteVec document)'
3744
: undefined
3845
if (missing) {
3946
throw new EqlV3CodecError(

0 commit comments

Comments
 (0)