feat(stack-drizzle): encrypt v3 query operands with encryptQuery (#622)#636
Conversation
… encrypt (#622) Every v3 Drizzle operator (eq/ne/gt/gte/lt/lte/between/notBetween/inArray/ notInArray/contains) previously encrypted its operand with client.encrypt — a full storage envelope (with ciphertext `c`) cast to `::jsonb`. A WHERE operand should be a query term, not a value to store. - All operators now use client.encryptQuery, producing a ciphertext-free term cast to the column's `eql_v3.query_<domain>` type, reaching the bundle's `(domain, query_<domain>)` overloads. The dialect no longer adds `::jsonb`; the operator layer owns the query-domain cast (`queryCastForDomain`). - encryptQuery is now a REQUIRED capability of the operand client (the optional guard and its only-JSON-uses-it branch are gone); scalar and JSON paths are unified. - inArray/notInArray encrypt the whole list in one batch encryptQuery crossing (position-stable, wrong-length rejected), replacing the bulkEncrypt/ bounded-concurrency path. - stack: add a batch `encryptQuery(terms)` overload to TypedEncryptionClient (mirrors the nominal client) so the documented `createEncryptionOperatorsV3(await EncryptionV3(...))` usage type-checks with the batch path. - Verified the query-domain mapping and the composite (hm/op/bf) query-term shape against the eql-3.0.0 bundle and a live encryptQuery probe; unit tests updated to assert query-term operands and `::eql_v3.query_<domain>` casts. The end-to-end operator behaviour is covered by the Drizzle integration matrix. Closes #622 Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
🦋 Changeset detectedLatest commit: 161f17b The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the EQL v3 Drizzle integration so all query operands are encrypted as ciphertext-free query terms via encryptQuery (instead of full storage envelopes via encrypt), aligning operand semantics with the EQL v3 (domain, query_<domain>) operator overloads and reducing payload size.
Changes:
- Switch v3 Drizzle operators (
eq/ne/gt/gte/lt/lte/between/notBetween/inArray/notInArray/contains) to encrypt operands withencryptQuery, and cast operands toeql_v3.query_<domain>at the operator layer. - Add a batch
encryptQuery(terms)overload toTypedEncryptionClient(stack v3 typing) to support single-crossing list encryption forinArray/notInArray. - Update Drizzle dialect rendering to stop appending
::jsonb, and rewrite unit/type tests to assert query-term operands andquery_<domain>casts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/stack/src/encryption/v3.ts | Extends TypedEncryptionClient typing with batch encryptQuery(terms) and bridges overloads in typedClient. |
| packages/stack-drizzle/src/v3/sql-dialect.ts | Removes dialect-level ::jsonb casts; assumes operands arrive pre-cast to query_<domain>. |
| packages/stack-drizzle/src/v3/operators.ts | Migrates operand encryption to encryptQuery, adds query_<domain> casting logic, and batches list operands. |
| packages/stack-drizzle/tests/v3/sql-dialect.test.ts | Updates expectations for dialect SQL output without ::jsonb. |
| packages/stack-drizzle/tests/v3/operators.test.ts | Rewrites operator tests to assert query-term operands, casts, and batch list encryption behavior. |
| packages/stack-drizzle/tests/v3/operators.test-d.ts | Updates type-level regression guards for the new encryptQuery-only operand client surface. |
| packages/stack-drizzle/tests/v3/bigint.test.ts | Updates bigint operator tests to assert query-term casts and encryptQuery usage. |
| .changeset/eql-v3-drizzle-encrypt-query.md | Changeset documenting the operand semantic shift and the added batch overload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…med overload Replace the object-literal arrow + `as unknown as` bridge with a named function carrying the two real overload signatures (single per-column form + batch `terms[]` form). The compiler now checks the implementation against both signatures directly, so the whole-value cast is gone; only the forwarded args keep `as never`, consistent with the sibling encrypt/encryptModel wrappers. Pure type-level restructuring — the runtime forward to the nominal client is unchanged. Verified: stack + stack-drizzle build and test:types (incl. the M1 guard) pass, code:check clean. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
coderdan
left a comment
There was a problem hiding this comment.
Code review — no correctness bugs found
Ran an xhigh-effort recall pass over the diff. Verified against the eql-3.0.0 bundle that every derived eql_v3.query_<domain> cast type exists, every (domain, query_<domain>) overload (eq/neq/gt/gte/lt/lte/contains) exists, resolveIndexType answers equality on ordering-only columns, single/batch routing + the position-stable length check are correct, and lock-context/audit propagate on both paths. The mechanism is sound. Four docs/test/cleanup findings — three inline, one below (its file isn't in the diff).
🔴 skills/stash-drizzle/SKILL.md:790 — ships wrong guidance to customers
inArray/notInArrayreject an empty list, and encrypt the whole list in a singlebulkEncryptcrossing when the client exposes one.
This PR deletes the bulkEncrypt path — inArray/notInArray now always use a single required encryptQuery batch crossing. skills/ ships inside the stash npm tarball and is copied into customer repos, so this sentence becomes wrong guidance in someone else's codebase. AGENTS.md (“Agent Skills — these ship to customers”, checklist item 8) requires updating the affected skill in the same PR. It also means the PR needs a stash patch changeset for the skills change.
Suggested: “…encrypt the whole list in a single encryptQuery batch crossing.”
- operators.ts: drop the two dead regex branches in queryCastForDomain (`ord_ope`, `search_ore` are not real column-domain suffixes — ope ordering is `_ord`, text search is `_search`); comment now ties the suffixes to the column factories. - operators.ts: fix stale docs left by the bulkEncrypt→encryptQuery move — the inArray/notInArray JSDoc and the inArrayOp comment no longer describe the removed bulkEncrypt path / deleted concurrency fallback. - operators.test.ts: the encryptQuery double now routes to batch only when `opts === undefined && Array.isArray(...)`, matching the real client — an array-valued single query WITH opts (e.g. a searchableJson array needle) no longer diverges to the batch path in the double. - stash-drizzle skill: correct the inArray/notInArray line (single encryptQuery batch crossing, not bulkEncrypt); + stash patch changeset (skills ship in the tarball). Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
All four review findings addressed in 161f17b:
Verified: stack-drizzle build + 356 unit tests + |
Targets the 1.0 candidate
feat/eql-v3-text-search-schema.Problem
The EQL v3 Drizzle operators encrypted every scalar/text WHERE operand with
client.encrypt— a full storage envelope (including the ciphertextc), cast to::jsonb. That is semantically wrong (a query is not a value to store) and wasteful (larger payloads). Only JSON containment usedclient.encryptQuery, which forcedencryptQueryto be an optional capability with a runtime guard the JSON path alone exercised.Change
Every operator (
eq/ne/gt/gte/lt/lte/between/notBetween/inArray/notInArray/contains) now encrypts its operand withclient.encryptQuery:eql_v3.query_<domain>type (derived byqueryCastForDomain), reaching the bundle's(domain, query_<domain>)operator overloads (whosequery_<domain>CHECK requiresNOT (VALUE ? 'c')). The dialect no longer appends::jsonb; the operator layer owns the cast.encryptQueryis now a required capability of the operand client — the optional guard and the JSON-only branch are gone; scalar and JSON paths are unified.inArray/notInArrayencrypt the whole list in one batchencryptQuerycrossing (position-stable; a wrong-length response is rejected, never silently truncated).@cipherstash/stack: added a batchencryptQuery(terms)overload toTypedEncryptionClient(mirrors the nominalEncryptionClient) socreateEncryptionOperatorsV3(await EncryptionV3(...))type-checks with the batch path (the M1.test-dguard).Verification
cipherstash-encrypt-v3.sql) that every queryable domain has aquery_<domain>type and(domain, query_<domain>)overloads, and that storage-only domains (boolean, bare base types) are gated out before any cast.encryptQueryprobe that a v3 column emits a composite term carrying all its configured index terms (TextSearch→{hm,op,bf},TextOrd→{hm,op}, etc.), which is exactly what the multi-keyquery_text_search/query_text_ordCHECKs require — and thatencryptaddscwhileencryptQueryomits it.::eql_v3.query_<domain>casts (356 stack-drizzle tests, dialect + bigint + matrix). Type tests (M1) pass.integration-drizzle.yml), which runs on this PR — that is the authoritative check that the narrowed terms satisfy the operators, as the issue asked.Closes #622
https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w