feat(stack-drizzle): EQL v3 JSON selector-with-constraint querying#651
Conversation
Add ops.selector(col, '$.path') to the v3 Drizzle integration (#623): comparison methods (eq/ne/gt/gte/lt/lte) bound to a JSONPath into a types.Json column, emitting `eql_v3.<op>(eql_v3."->"(col,'<sel>'), eql_v3."->"('<needle>'::eql_v3_json,'<sel>'))`. Its unique power over `contains` is ORDERING at a path (col->'$.age' > 21). - Selector hash from encryptQuery(path, searchableJson) (string needle → ste_vec_selector). - Comparison entry from a STORAGE encrypt of {path: value} — its ciphertext `c` is required by the eql_v3_jsonb_entry domain (a query term can't carry it). - One new dialect fn (selectorEntry); reuses equality/comparison for the compare. - v1: dot-notation object paths; array/wildcard rejected with a clear error. Core needs no change. Supabase is a follow-up (#650) — blocked by PostgREST's inability to cast a filter value. Refs #623 Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds ChangesJSON selector comparisons
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant DrizzleOperators
participant EncryptionClient
participant PostgreSQL
Application->>DrizzleOperators: selector(column, '$.path').gt(value)
DrizzleOperators->>EncryptionClient: encrypt selector and RHS data
EncryptionClient-->>DrizzleOperators: encrypted selector entry data
DrizzleOperators->>PostgreSQL: execute selector comparison SQL
PostgreSQL-->>Application: matching rows
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedLatest commit: 36ecb3b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…aint
Rework the selector path to be entirely ciphertext-free (the storage-payload RHS
was wrong — it leaked ciphertext into the WHERE clause). Both operands are query
terms; the comparison reduces to the encrypted term on each side:
eql_v3.<term>(eql_v3.jsonb_path_query_first(col,'<sel>')) <cmp> eql_v3.<term>(<operand>)
where <term> is eq_term (=/<>) or ord_term (</<=/>/>=) — both collapse to
bytea (hmac_256 / ope_cllw) and compare natively.
- selector hash: encryptQuery(path, searchableJson) (string → ste_vec_selector).
- RHS: a scalar encryptQuery(value, { queryType: equality|orderAndRange }) term
cast to eql_v3.query_<T>_<eq|ord>, T inferred from the leaf value's JS type.
- Drops the client `encrypt` requirement and the doc reconstruction entirely.
Assumes protect-ffi keys the scalar query term with the column's key (verified by
the live tests). The bundle lacks a (jsonb_entry, query_<T>) operator, so the
direct-function form is used; adding it upstream is an EQL follow-up.
Refs #623
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
The ciphertext-free approach was non-functional: encryptQuery can't mint a
scalar/ordering query term against a ste_vec (JSON) column, so the value call
threw "Index type not configured" (caught by CI's live integration job).
Verified via FFI probes: the stored ste_vec entry at the JSONPath selector
carries c + op (ordering) / hm (equality), and encryptQuery('$.path') returns
the selector that matches it. So the RHS is a STORAGE encryption of {path: value}
whose entry is extracted with jsonb_path_query_first and compared entry-vs-entry
(eq_term/ord_term read only hm/op). Ciphertext-free ordering needs a protect-ffi
change (cipherstash/protectjs-ffi#137); until then the value's ciphertext is in
the WHERE clause — documented inline.
Also from the code review: use Promise.all for the two independent encrypts;
throw (not silently mis-bind) if the selector term isn't a bare string; fix path
normalization (leading $/.) and reject the empty/root path; drop the fragile
JS-type→query-domain inference entirely.
Refs #623 · protect-ffi gap cipherstash/protectjs-ffi#137 · EQL operator #404
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Update stash-encryption + stash-drizzle skills for the new ops.selector(col,
'$.path').{eq,ne,gt,gte,lt,lte} surface — they previously said JSONPath selector
queries were 'not yet implemented'. Clarify that direct eq/gt/asc on a Json
column still throw; selector-with-constraint is the path-scoped form, with
ordering-at-a-path its unique power over containment. Skills ship in the stash
tarball, so this carries a stash patch changeset.
Refs #623
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stack-drizzle/integration/json-selector.integration.test.ts`:
- Around line 68-76: Update the integration-test setup around TABLE_NAME to
remove the unconditional DROP TABLE operation and create the shared table
idempotently with CREATE TABLE IF NOT EXISTS. Preserve the existing RUN-specific
cleanup so setup deletes only rows belonging to the current test run.
In `@packages/stack-drizzle/src/v3/operators.ts`:
- Around line 632-643: Update the JSDoc for the selector-with-constraint flow in
packages/stack-drizzle/src/v3/operators.ts (lines 632-643) to describe the
interim storage-needle behavior and acknowledge that the encrypted RHS document
appears in generated SQL instead of calling the operation ciphertext-free. Add
the same limitation to the relevant guidance at skills/stash-drizzle/SKILL.md
(line 791) and skills/stash-encryption/SKILL.md (lines 691-700), ensuring each
skill file states that selector RHS ciphertext currently appears in the WHERE
clause and remains self-sufficient.
- Around line 115-124: Update the selector-document construction around the root
and nested objects in the segments traversal to use null-prototype objects,
preventing a `__proto__` path segment from invoking the prototype setter.
Preserve the existing cursor assignment and value placement behavior while
ensuring every intermediate and root object is created without inherited
properties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79f8944b-e042-4c46-919e-c36d9ba0417c
📒 Files selected for processing (6)
.changeset/eql-v3-json-selector.mdpackages/stack-drizzle/integration/json-selector.integration.test.tspackages/stack-drizzle/src/v3/operators.tspackages/stack-drizzle/src/v3/sql-dialect.tsskills/stash-drizzle/SKILL.mdskills/stash-encryption/SKILL.md
- reconstructSelectorDocument: use null-prototype objects so a `__proto__` path
segment becomes an own key instead of invoking the prototype setter (which
would mis-serialize the needle → no matches).
- Fix stale selectorCompare JSDoc: it still described the reverted ciphertext-free
approach ('No ciphertext, no storage encryption') and referenced the renamed
selectorConstraint. Now documents the storage-needle interim consistently.
- Disclose the interim ciphertext-in-WHERE behavior in both stash-drizzle and
stash-encryption skills (each must be self-sufficient).
Skipped: CodeRabbit's DROP-TABLE-IF-EXISTS suggestion — the sibling
json-contains integration test uses the same DROP+CREATE pattern and the RUN
discriminator isolates rows; hardening should be repo-wide, not one file.
Refs #623
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
Addressed CodeRabbit's review in b5b18c3:
Build clean, 357 unit tests pass, Biome clean. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/stash-drizzle/SKILL.md`:
- Line 791: In the selector documentation sentence, update the EQL version label
from “v1 supports” to “v3 supports” while preserving the listed supported and
rejected JSONPath forms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f51e196-6852-4206-81fd-95c43469b3f6
📒 Files selected for processing (3)
packages/stack-drizzle/src/v3/operators.tsskills/stash-drizzle/SKILL.mdskills/stash-encryption/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/stash-encryption/SKILL.md
- packages/stack-drizzle/src/v3/operators.ts
| } | ||
|
|
||
| /** | ||
| * JSONPath selector-with-constraint on an `eql_v3_json` (`ste_vec`) column: |
There was a problem hiding this comment.
Source: Claude
selectorCompare JSDoc (operators.ts:632–647) is stale — describes the abandoned ciphertext-free design
and contradicts the implementation:
- "entirely ciphertext-free" / "No ciphertext, no storage encryption" — false; the RHS is
client.encrypt(...)(storage), as the inline comment at 666–673 correctly states.- value described as
encryptQuery(..., {queryType})cast toeql_v3.query_<T>_<eq|ord>— actually
client.encryptcast topublic.eql_v3_json.- references
v3Dialect.selectorConstraint, which doesn't exist (helper isselectorEntry).Rewrite to match the storage-needle implementation (or drop it and keep the inline comment), and fix
theselectorConstraint→selectorEntryreference.
There was a problem hiding this comment.
Correction (edited after posting): My original review claimed finding #1 "breaks CI." That's wrong, and I've corrected it below. The type-double failure is real — I reproduced it by running
pnpm --filter @cipherstash/stack-drizzle test:types— but it does not turn CI red, because CI only runstest:typesfor@cipherstash/stackand@cipherstash/test-kit(.github/workflows/tests.yml:110,117), never forstack-drizzle, and theRun Testsjobs usevitest run(no--typecheck). All of this PR's actual checks are green. Apologies for the overstatement. The rest of the review stands.
Reviewed with a multi-angle pass (line-by-line, removed-behavior, cross-file tracing, reuse/simplification/efficiency/altitude/conventions), each surviving candidate independently verified against the head commit, the core @cipherstash/stack helpers, and the vendored EQL v3 domain config.
Overall: really nice piece of work. The entry-vs-entry comparison design is clever, the interim storage-needle tradeoff is honestly reasoned and documented inline, and the operator/dialect code is unusually well-commented. The selValue shape guard, the empty-object match-all guard, and the concurrent-encrypt structure are all the right instincts. I verified two of my own worries away: the ste_vec-only capability gate is sufficient (the single types.Json domain hardcodes mode: 'compat', so every ste_vec entry always carries the op ordering term), and the production clients all expose encrypt, so the runtime path is sound.
I'm keeping this at request changes for the two confirmed items below — but note that with the CI framing corrected, none of these is a hard merge-blocker in the sense of turning CI red; they're a stale regression guard and a behavioral regression that are worth fixing in this PR rather than deferring.
Should fix in this PR
1. The type-double regression guard is broken under the package's own test:types (__tests__/v3/operators.test-d.ts:57). Adding the required encrypt(value, opts) member to OperandEncryptionClient (operators.ts:76, non-optional) means the { encryptQuery }-only double at lines 50-56 is no longer assignable, so expectTypeOf(createEncryptionOperatorsV3).toBeCallableWith(double) fails to compile. Reproduced locally:
× accepts a minimal structural { encryptQuery } double
→ Property 'encrypt' is missing in type '{ encryptQuery … }' but required in type 'OperandEncryptionClient'.
__tests__/v3/operators.test-d.ts:57
This is not CI-gated (see the correction above), so it won't block the merge — but it means the package's test:types is red and the M1 regression guard no longer does its job. Two knock-ons: the @ts-expect-error on the erased double (lines 67-73) is now satisfied by the missing-encrypt error instead of the intended encryptQuery-erasure error, silently defanging the #622 guard; and the runtime doubles in operators.test.ts:77 / bigint.test.ts:28 also omit encrypt (they pass today only because those paths never call it). Fix: add a minimal encrypt to the doubles and re-verify the erased assertion still fails for the erasure reason. Separately, it may be worth adding stack-drizzle to the CI test:types matrix so this guard is actually enforced.
2. The leaf-value-type guard was dropped and not re-established (operators.ts:654). Git history shows a prior revision rejected leaf values with no eq/ord query domain (else base = null // boolean has no eq/ord query domain, throwing no ordering query domain for a <type> leaf value). The storage-needle rewrite kept only requireNonNullOperand + path-syntax validation, so ops.selector(col, '$.flag').gt(true) or .gt({ nested: 1 }) now storage-encrypts the non-scalar/boolean leaf and emits an ordering comparison instead of the clean up-front EncryptionOperatorError the removed guard gave. Re-add a scalar-leaf check before encrypting the needle.
In plain English:
What the feature does: This PR lets you query encrypted JSON by a path — e.g. "find rows where the
agefield inside the JSON is greater than 21," written asops.selector(users.doc, '$.age').gt(21). The value you compare against (here,21) is the "leaf value."What the guard used to do: An earlier version of this code checked the type of that value before doing anything with it. Some comparisons only make sense for ordered things like numbers and dates. Asking "is this field greater than
true?" (a boolean) or "is this field greater than{nested: 1}?" (an object) is nonsense — booleans and objects don't have a "greater than" ordering. So the old code caught that early and threw a clear, friendly error saying, in effect, "you can't do an ordering comparison on this kind of value."What changed: When the author reworked how the right-hand side gets encrypted (the "storage-needle rewrite"), that type check got deleted and never replaced. The only checks left are "the value isn't null" and "the path string is well-formed." Nothing looks at whether the value is a sensible thing to order.
Why that's a problem: Now if someone writes
ops.selector(col, '$.flag').gt(true), the code doesn't stop them. It goes ahead, encrypts the value, builds the SQL, and sends it to the database. One of two bad things then happens:
- The database rejects it with a cryptic, low-level error that mentions internal encryption types — the user has no idea it just means "you can't order a boolean."
- Or worse, it runs and quietly returns the wrong rows.
Either way, the user gets a confusing failure late (at the database) instead of a clear message early (from the SDK), which is a worse experience than before.
The fix I suggested: Put back a check that the value is a scalar (number/date/string — something orderable) before encrypting it, so the SDK rejects
gt(true)up front with a readable error, the way it used to.One caveat: I confirmed this one against the actual git history (the deleted lines really are there), so I'm confident it's real. But I should be honest that I did not run it against a live database to confirm exactly which of the two bad outcomes happens (opaque error vs. wrong rows) — that distinction is my inference, not something I verified end to end.
3. ne and absent-path semantics are undefined, untested, and mis-documented (operators.ts:730, 745). For a row whose document lacks the queried path, eql_v3.jsonb_path_query_first returns SQL NULL, and ne routes straight into eql_v3.neq(NULL, needle) with no COALESCE/guard — under three-valued logic those rows are silently excluded from .ne(x), which is almost certainly not what a caller expects ("not equal to x" usually includes "has no x"). ne has zero test coverage (the integration suite exercises only eq/gt/gte/lt, and every seed row has $.age), and the jsdoc advertises it as plain col->'path' <> value with no NULL caveat. Please define the absent-path semantics, add a test that seeds a row missing the path, and document the behavior — or drop ne from v1 if the semantics aren't settled. (Like #2, the exact runtime NULL behavior is my inference from standard SQL three-valued logic; the live integration job is the definitive check.)
Follow-up (fine for subsequent PRs)
-
Share core's path helpers instead of forking them (operators.ts:85/102/111).
parseSelectorSegments,jsonPathOf, andreconstructSelectorDocumentre-implement core'sparseJsonbPath,toJsonPath, andbuildNestedObject(packages/stack/src/encryption/helpers/jsonb.ts, already re-exported fromhelpers/index.ts— they're just not on a public package subpath, which is the thing to fix). This is now the third copy of these (core,packages/protect, and here), and selector correctness depends on this normalization staying byte-identical to whatever core uses to build the stored ste_vec selectors. Notably the fork also drops core'sFORBIDDEN_KEYSrejection (prototype/constructor), so$.constructoris accepted here but rejected by core. Prefer exporting these from@cipherstash/stack/adapter-kit(which stack-drizzle already imports) and deleting the fork. -
Reject
../ malformed paths instead of silently collapsing (operators.ts:85).$..age(recursive descent) normalizes to$.age,$.a..bto$.a.b, and a stray leading space ($.age) builds a garbage two-segment selector — all silently querying a different path than the user wrote and returning empty/wrong rows with no error. Since v1 already rejects array/wildcard with a clear message, extend the same treatment to.., empty interior segments, and non-$-anchored input (trim + validate). -
Cache the value-independent selector hash (operators.ts:683). The LHS
encryptQuery(jsonPathOf(segments), …)depends only on(column, path), not the value, yet it's re-encrypted on every predicate — so a bounded range.gt(21)+.lt(65)on the same path pays two identical selector round-trips, and N predicates on one path pay N. Memoize per(column, path)alongside the existingcontextCache. -
Make the ciphertext-in-WHERE caveat prominent where users actually look. The interim design puts the needle's plaintext-derived ciphertext into the WHERE clause (visible in query logs /
pg_stat_statements). It's well-explained in the code comment, but the SKILL.md capability table flipsJsonto "selector-with-constraint queries" reading as fully done, with the caveat only in prose below. Surface the interim/leakage note in the table row itself, and (as you're already planning) hold the doc flip until live CI is green and ideally until #137 lands. -
Minor:
selectorCompare'soperatorlabel param is derivable fromop+path;reconstructSelectorDocument's cursor loop reads more simply as areduceRight; and the newas nevercasts (operators.ts:686/691/697/701) match the file's existing style but don't carry the// biome-ignore lint/plugin: <reason>justification the repo CLAUDE.md asks for on type-erasing assertions insrc/.
To be clear on scope: the entry-vs-entry SQL correctness (whether the bundle ships eql_v3.<op>(eql_v3_jsonb_entry, eql_v3_jsonb_entry) overloads and whether the stored hm/op terms match the needle's) is only provable by the live integration job — which the PR correctly gates on, and which is green. My review covers the SDK layer above that.
Must-fixes:
- test:types was broken (operators.test-d.ts): adding the required `encrypt`
member to OperandEncryptionClient made the `{ encryptQuery }`-only doubles
unassignable. Add `encrypt` to both doubles — and to the `erased` double too,
so it's rejected ONLY for the #622 encryptQuery-erasure reason (restoring the
guard's teeth), not for a missing member.
- Re-add the scalar-leaf-type guard dropped in the storage rewrite: reject
non-scalar leaf values (object/array → use contains()) and ordering a boolean,
up front as EncryptionOperatorError instead of a deferred opaque DB error.
- Define + test + document `ne` absent-path semantics: a row lacking the path is
now INCLUDED in `.ne(x)` (OR jsonb_path_query_first(...) IS NULL); eq/ordering
keep SQL's exclude-on-missing. Integration test seeds a row with no `$.age`.
CI gap (root cause the type break slipped through): the adapter packages'
`test:types` was never wired into CI after the #627 split — only @cipherstash/stack
and test-kit ran. Add stack-drizzle + stack-supabase type-test steps to tests.yml
(both green today).
Toby: path-validation errors now surface as EncryptionOperatorError with column
context; add unit tests for the pure helpers (parseSelectorSegments,
reconstructSelectorDocument) + the selector guards.
Also from freshtonic: harden path parsing — reject `..`/malformed/empty-segment
paths (no longer silently query a different path) and prototype-pollution keys
(__proto__/prototype/constructor); surface the interim ciphertext-in-WHERE caveat
in the skill capability-table row.
Follow-ups (noted, not in this PR): share core's path helpers via adapter-kit;
memoize the value-independent selector hash per (column, path).
Refs #623 · protect-ffi gap cipherstash/protectjs-ffi#137
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
Thanks both — really thorough. Addressed in 36ecb3b. @freshtonic — must-fixes (all fixed)
Follow-ups: also did the path-hardening ( @tobyhede
368 unit tests + 7 type tests pass; Biome clean. |
freshtonic
left a comment
freshtonic
left a comment
There was a problem hiding this comment.
Re-reviewed at 36ecb3b0. All three items from my previous review are resolved, and I verified the fixes rather than eyeballing them — checked out the branch, installed deps, and ran the drizzle typecheck + unit suite locally; both Drizzle v3 integration jobs (postgres + supabase) are also green on this head. Approving.
Confirmed resolved
-
Type-double regression guard —
operators.test-d.tsnow suppliesencrypton both doubles (and theeraseddouble gets a correctencryptso its@ts-expect-erroris again driven only by the encryptQuery-erasure, restoring the #622 guard's teeth). You also wiredstack-drizzle(andstack-supabase) into the CItest:typesmatrix — the deeper fix, since those.test-d.tsguards moved out of@cipherstash/stackin the #627 split and were running nowhere. I ranpnpm --filter @cipherstash/stack-drizzle test:types: 7/7 pass, no type errors (it failed before this commit). -
Leaf-value-type guard —
unsupportedLeafReasonre-establishes the up-front check: non-scalars are rejected with "use contains()", and a boolean under an ordering op is rejected with a clear message, whileeq/neon a boolean is still allowed. Raised as anEncryptionOperatorErrorwith column context before any encryption. Covered byselector.test.ts(rejects object/array leaf, rejectsgt(true), allowseq(true)), which I ran green. -
ne/ absent-path semantics — now emits(eql_v3.neq(...) OR leftEntry IS NULL), so a row whose document lacks the path is included in.ne(x)("not equal" covers "has no value"), whileeq/ordering keep SQL's default exclusion. Documented in the jsdoc, and — importantly — the live integration suite now seeds anoagerow and asserts all three behaviors (eq excludes, ne includes, ordering excludes). The green integration jobs confirm end-to-end what I'd previously only inferred from three-valued-logic reasoning. Nicely, the(A OR B)fragment is self-parenthesising, soops.not(...)over it stays correct.
You also folded in several follow-ups I'd flagged for "later": malformed-path rejection (.., stray/leading/trailing dots, whitespace trim — no more silent path collapse), prototype-pollution key rejection (__proto__/prototype/constructor, realigning with core's FORBIDDEN_KEYS), and the interim ciphertext-in-WHERE caveat now surfaced inline in the SKILL.md capability-table row. All unit-tested.
Still open (non-blocking, subsequent PRs)
- Share core's path helpers via
adapter-kitinstead of the fork — you've left theparseSelectorSegments/jsonPathOf/reconstructSelectorDocumentfork in place with a comment pointing at this follow-up, which is fine. The reason it's still worth doing: selector correctness depends on this normalization staying identical to whatever core uses to build the stored ste_vec selectors, and there are now three copies (core,packages/protect, here). Exporting the core helpers and deleting the fork removes the drift risk for good. - Cache the value-independent selector hash — the LHS
encryptQuery(jsonPathOf(segments), …)depends only on(column, path), so N predicates on the same path still pay N identical selector round-trips; memoizing alongsidecontextCachewould drop that to one. - Minor / worth a quick check:
unsupportedLeafReasonacceptsbigintas a scalar leaf — worth confirming abigintJSON leaf actually round-trips through the storage-needleencrypt(a plainJSON.stringifythrows onbigint), otherwise it's exactly the kind of deferred error the guard exists to prevent. I did not verify this path, so treat it as a question, not a claim. Also cosmetic:selectorCompare'soperatorlabel arg is still derivable fromop+path, and the newas nevercasts match the file's style but lack the// biome-ignore lint/plugin: <reason>the repo CLAUDE.md asks for.
Thanks for the thorough turnaround — and again, apologies for the "breaks CI" overstatement in my first pass; the type failure was real but, as you correctly noted, not CI-gated at the time. It is now.
3 participants
Adds JSONPath selector-with-constraint querying for EQL v3 encrypted JSON to the Drizzle integration. Refs #623. Draft — pending live integration CI.
What it adds
ops.selector(col, '$.path').{eq,ne,gt,gte,lt,lte}(value)on atypes.Jsoncolumn. Complementscontains(@>); its unique power is ordering at a path (col->'$.age' > 25), which containment can't express.How it works (verified via FFI probes)
Emits an entry-vs-entry comparison:
<sel>=encryptQuery('$.path', searchableJson)— the JSONPath-string query returns the ordering selector, which matches the storedopentry.<needle>= a storageencryptof{path: value}— its ste_vec entry at that selector carriesc+op/hm.eq_term/ord_termread onlyhm/op, so the comparison is correct.Why storage (interim), not a ciphertext-free query term
The clean ciphertext-free path is blocked upstream:
encryptQuerycan't mint a scalar/ordering query term against a ste_vec column — it only produces thehmequality needle. Probes confirmed the stored side already has theopterm and thequery_jsonbdomain already permits it; the only gap is protect-ffi emitting an ordering needle → cipherstash/protectjs-ffi#137. Until that lands, the RHS is a storage needle, so the value's ciphertext appears in the WHERE clause (documented inline). When #137 ships, the RHS drops the ciphertext with no API change.An earlier revision tried the ciphertext-free form and was caught red by the live integration job ("Index type not configured") — the storage form is what actually works.
Scope
@cipherstash/stack: no change.Review fixes folded in
Promise.allfor the two independent encrypts; throw (not silently mis-bind) when the selector term isn't a bare string; correct path normalization + empty-path rejection; dropped the fragile JS-type→query-domain inference.Verification
integration/json-selector.integration.test.ts).Remaining before ready
skills/stash-drizzle+skills/stash-encryptiondocs (they currently say this is "not yet implemented" — must be corrected; holding until CI green).Refs #623 · #650 · #404 · cipherstash/protectjs-ffi#137
https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Summary by CodeRabbit
selector(col, '$.path').$.profile.name) and exposeseq,ne,gt,gte,lt,lte.contains) behavior.selector(...)query pattern, semantics, and current limitations.