Repository navigation
Conversation
|
|
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 configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 9 | 0 | 15 | 0 |
Every mutant in the changed lines was caught by a test.
There was a problem hiding this comment.
🔵 Needs a closer look
Changes to cryptographic term derivation, key framing, and stored-byte compatibility warrant final human review.
No open issues 🎉
2 files not reviewed
languages/golang/encrypt/internal/testusers/kinds_stash.go: Generated filelanguages/golang/encrypt/internal/testusers/probe_stash.go: Generated file
What changed in this PR
Enables folded text ordering in the Rust encryption engine and Go SDK, preparing for EQL text-order types.
Changes:
- Derives text order terms using Unicode 16 case-and-accent folding.
- Enables text comparisons and validates folded range bounds.
- Updates generated Go methods, regression tests, documentation, and dependencies.
| File | Description |
|---|---|
| packages/stack-encrypt/tests/ui/pass/plan_chain.rs | Adds compiling text queries. |
| packages/stack-encrypt/tests/ui/op_order_on_text.stderr | Removes obsolete rejection snapshot. |
| packages/stack-encrypt/tests/ui/op_order_on_text.rs | Removes text-order rejection test. |
| packages/stack-encrypt/tests/ui/op_order_on_bytes.stderr | Adds bytes-order rejection snapshot. |
| packages/stack-encrypt/tests/ui/op_order_on_bytes.rs | Checks bytes ordering remains unavailable. |
| packages/stack-encrypt/tests/ui/op_between_takes_a_pair.stderr | Updates ordering diagnostics. |
| packages/stack-encrypt/tests/term_bytes.rs | Pins folded terms and rejection behavior. |
| packages/stack-encrypt/tests/op.rs | Tests folded comparisons and ranges. |
| packages/stack-encrypt/src/sem/mod.rs | Folds text and separates its key framing. |
| packages/stack-encrypt/src/op/mod.rs | Enables typed text ordering. |
| packages/stack-encrypt/src/dynamic/term.rs | Updates text-order documentation. |
| packages/stack-encrypt/src/dynamic/query.rs | Enables and validates dynamic text queries. |
| packages/stack-encrypt/fuzz/Cargo.lock | Locks the folding dependency. |
| packages/stack-encrypt/docs/adr/0009-operators-select-the-index-and-a-query-term-is-not-the-stored-term.md | Records text ordering as decided. |
| packages/stack-encrypt/CHANGELOG.md | Documents changed text term bytes. |
| packages/stack-encrypt/Cargo.toml | Adds vitaminc-ore. |
| packages/eql/Cargo.lock | Updates engine dependencies. |
| languages/golang/stashgen/testdata/cases/orders/order_stash.go.golden | Adds expected string ordering methods. |
| languages/golang/stashgen/refusal_test.go | Updates unused-index notice coverage. |
| languages/golang/stashgen/enginetest/enginetest.go | Mirrors text-order capability. |
| languages/golang/stashgen/emit.go | Updates unused-index explanations. |
| languages/golang/encrypt/README.md | Explains folded ordering and refusals. |
| languages/golang/encrypt/order_test.go | Tests folded ordering and generated queries. |
| languages/golang/encrypt/internal/testusers/probe_stash.go | Regenerates string ordering methods. |
| languages/golang/encrypt/internal/testusers/kinds_stash.go | Adds named-string ordering methods. |
| languages/golang/encrypt/guest/Cargo.lock | Updates guest dependencies. |
| languages/golang/encrypt/doc.go | Documents text ordering semantics. |
| languages/golang/cmd/stashgen/README.md | Documents generated string comparisons. |
| languages/golang/CHANGELOG.md | Records text ordering support. |
| Cargo.toml | Declares workspace folding dependency. |
| Cargo.lock | Locks vitaminc-ore and dependencies. |
Review effort: Balanced
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Automated review attestation
Source: Claude Code scheduled review (/code-review medium + /cross-check-review-findings) at head 54a204d. This is an automated review, not a human approval.
Critical / blocking issues: none.
Findings: none survived cross-check.
Reviewed (packages/stack-encrypt/src):
sem/mod.rs:OrderSourceandtext_order_key. Text goes through the folded key. The PRF context adds the frozenTEXTdomain label only for text, so integer, float and bytes terms keep their bytes. The key is zeroizing and moved, not copied.op/mod.rs: sealedOrdered::inverted. TheStringimpl compares folded keys. Integers keeplo > hi.dynamic/query.rs:ordered(kind),bound_orderand thecheck_inputfold refusal. They agree with the typed path (("b","A")inverted,("A","a")a tie, an unassigned code point refused before a keyset).- Docs, ADR-0009, trybuild snapshots, lockfiles and the Go generated/golden files, checked for consistency with the Rust change.
Cross-check against CI on this head:
- fmt/clippy/nextest/doctests/rustdoc, the Go matrix (Linux/macOS/Windows), Go lint and live tests, the EQL suites and the fuzz regressions are green.
- The mutants gate caught 24 of 24 mutants (0 missed).
- Copilot raised no issues.
- The Codex code and security reviews were still running when this review was posted.
Notes (non-blocking):
OrderSource::ofselects the text path by runtimeAnydowncast onString,Zeroizing<String>and&'static str. A new owned text type passed to an order term would silently take the non-foldedValuepath. This is fine today becauseOrderedis sealed, and the PR notes it. A test asserting everyOrderedtext form takes the folded path would guard it.- Text order terms change bytes (the CHANGELOG notes this as Breaking, with none deployed). Confirm no stored text ORE/OPE terms exist outside tests before merge.
- Changeset-bot reports no changeset. That looks acceptable here since the touched crates and the Go module use their own CHANGELOGs, but a maintainer should confirm.
Generated by Claude Code
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54a204d924
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// not assign, whose folding is not known. | ||
| pub(crate) fn text_order_key(text: &str) -> Result<Zeroizing<Vec<u8>>, TermError> { | ||
| use vitaminc_protected::Controlled; | ||
| Ok(Zeroizing::new(vitaminc_ore::text(text)?.risky_unwrap())) |
There was a problem hiding this comment.
For any valid string containing a code point unassigned in Unicode 16—including characters introduced by later Unicode versions—propagating this error makes every ORE/OPE write and query fail. This contradicts ADR-0002's frozen-transform contract in docs/adr/0002-one-value-model-three-encodings-and-eql-v4-by-producer.md:187-195, which requires such code points to pass through unchanged so existing columns remain usable as Unicode evolves; derive the stable pass-through key rather than rejecting the value.
Useful? React with 👍 / 👎.
|
Parked until new This PR folds text inside Stack Encrypt ( No released or built version of either crate implements vitaminc-ore 0.6 yet. Once they do, this PR gets reworked:
The engine surface stays: |
Text ordering is decided: the order key is the system ADR-0002's text fold, which vitaminc 0.6 ships in vitaminc-ore, and the ORE and OPE indexes answer the orderings on text over it. The ADR no longer says "if the catalog keeps text ordering" or that the paragraph goes if text ordering is dropped, and decision 7 names text beside the integer types.
…the orderings on text
Text could not be ordered: no ordering operator was answered on a text
field, and the ORE and OPE terms stored for text went through cllw-ore's
own collation, which keeps case, so Zoe ordered before alice and é after
z. ADR-0002 defines text's order key as its fold (Unicode 16 NFC with case
and accents folded), and vitaminc 0.6 ships it in vitaminc-ore.
Text ORE and OPE terms now derive over that key on the typed and the
data-plan paths alike: the text is folded with vitaminc_ore::text and the
folded bytes go to cllw-ore's byte encryption, whose ciphertext type is
the one it gives text, so OreTerm<String> and OpeTerm<String> keep their
type. The released cllw-ore and ore-rs do not implement vitaminc-ore's
OrderScheme yet, so the CLLW schemes the engine already uses encrypt the
key. A text term's key is derived under a framing that adds the fold's
frozen label; integer, float and bytes order terms keep their bytes and
framing. Café, cafe, CAFÉ and cafe with a combining accent are one term.
Text holding a code point Unicode 16 does not assign has no fold and no
order term.
op::Ordered covers String, whose Between judges an inverted range on the
folded keys through a new sealed method, Ordered::inverted: ("b", "A") is
inverted, ("A", "a") is one value. The data path answers the orderings on
text too, refuses an inverted text range and unfoldable order text at its
boundary, and the Go generator now writes Lt, Lte, Gt, Gte and Between on
a string field with ore or ope. Its notice is left for order indexes on
bytes or a float, and for a second order index.
Closes #1234.
… schemes cllw-ore 0.5.2-rc.1 implements vitaminc-ore's `OrderScheme` behind its `vitaminc` feature, so Stack Encrypt no longer encodes anything for ordering itself. An ORE or OPE term is now cllw-ore's `vitaminc::Ore` or `vitaminc::Ope` encryption of the value: the engine names the value's order target through the new sealed `sem::Orderable` (integers at their own width, `f32`/`f64` as `F64`, text as `Text`, bytes as `Bytes`) and passes the value straight through; cllw-ore encodes it with vitaminc-ore, text by the system ADR-0002's fold. That replaces the earlier workaround, which built text's order key here and handed the folded bytes to cllw-ore 0.5 through `Any` downcasts: `OrderSource`, `text_order_key` and the downcasts are gone. The per-index key derivation is kept: the PRF block keys the scheme inside the visitor and is wiped there, and the column is opened with the value's target and the empty context, since the key is already the index's own. The key labels move to `/v2` because every order term changes bytes (new column salt, cllw-ore's marker byte for text and bytes). Nothing was deployed (ADR-0002); the pins and both fixtures are re-recorded. Every source type's term is now one ciphertext type (`OreCllw8VariableV1`, `OpeCllw8VariableV1`); `OreTerm<T>`/`OpeTerm<T>` keep `T` as a marker so typed callers compile unchanged. `Between` judges an inverted range in the order the stored terms compare, by encrypting both bounds with OPE under a fixed comparison key and comparing the bytes, on the typed and the data-plan path alike. Claude-Session: https://claude.ai/code/session_01BZhkvKLpNtKuzj3HCfMXoL
54a204d to
f915cf5
Compare
|
Unparked, and reworked onto
The Go surface and the tests from the first draft stay. The description is rewritten to match. |
An `f32` or `f64` order term is cllw-ore's vitaminc encryption of the value at the `F64` target, whose vitaminc-ore encoding folds `-0.0` into `0.0` and every NaN into one value above `+Infinity`: Postgres's `float8` order. So the ordering operators are now answered over floats, on the typed path (`op::Ordered` for `f32` and `f64`) and over a data plan's `float32` and `float64` fields, and stashgen writes `Lt`, `Lte`, `Gt`, `Gte` and `Between` on a float field with `ore` or `ope`. An `f32` is ordered as the `f64` it widens to: its `Orderable` target is `F64` and vitaminc widens it exactly, so an `f32` and its `f64` are one term and `f32` can join `op::Ordered`. That settles what the earlier draft refused with `TermError::NarrowFloat`, which is gone. `Between` judges a float range in the same order (`(0.0, -0.0)` is one value, `(NaN, 1.0)` is inverted), by the comparison #1235 added. Float pins, the float ordering baseline, the integer-width lengths and the `score` field in the cross-path fixture carry over from the earlier draft; the fixtures and UI snapshots are re-recorded. Claude-Session: https://claude.ai/code/session_01BZhkvKLpNtKuzj3HCfMXoL
|
Mutants re-run after removing the equivalent |
A `Between` whose low bound is above its high bound was refused with `TermError::InvertedRange` before either term was derived. SQL's own `BETWEEN` refuses nothing: `x BETWEEN 20 AND 10` is a valid query that matches no row, and an encrypted range now behaves the same way. The empty result leaks nothing and corrupts nothing, so a refusal bought little. It also cost more than it bought. Judging a range correctly for text and floats means reproducing, before encryption, the order the index compares in: text by its fold, floats by their canonical encoding. The check did that by encrypting both bounds under a fixed public OPE key, which tied a query-time refusal to one scheme's ciphertext. That order belongs to the ORE crates, not to this one. So `TermError::InvertedRange` (code `stack_encrypt::inverted_range`) goes, with the typed path's `Ordered::inverted`, the data path's `bound_order` and `integer_order`, and the comparison scheme behind them. The variant is under [Unreleased] and never shipped. The data path still refuses order text with no fold (a code point Unicode 16 does not assign) before a term or a keyset, for one value and now for each bound of a pair. It asks `vitaminc_ore::text` directly, which is the encoding `vitaminc_ore::order` uses for the `Text` target, so it refuses exactly what derivation would. Claude-Session: https://claude.ai/code/session_01BZhkvKLpNtKuzj3HCfMXoL
|
Removed the inverted-range check from A range whose low bound is above its high bound is now encrypted like any other and matches nothing, as
|
Summary
Stack Encrypt is CipherStash's Rust encryption engine; the Go SDK runs it as a WebAssembly guest. Next to each ciphertext it can store order terms, which let the database answer
<,>,BETWEENandORDER BYwithout decrypting. There are two kinds: ORE (order-revealing encryption, compared by a special function) and OPE (order-preserving encryption, compared as plain bytes).Until now Stack Encrypt could not order text at all. This PR makes text orderable, so a Go string field with
index=oreorindex=openow getsLt,Lte,Gt,GteandBetween. Text orders by ADR-0002's fold: case and accents are folded, under Unicode 16. SoCafé,cafeandCAFÉtie,alicecomes beforeZoe, andésorts witherather than afterz.How it works changed since the first draft. The ORE crate
cllw-orenow encodes and encrypts each type for ordering itself, throughvitaminc-ore. That shipped as a pre-release, 0.5.2-rc.1, with avitamincfeature. Stack Encrypt says which type a value is and passes it straight through. It no longer folds text or builds order keys of its own.Changes
Engine (
packages/stack-encrypt)Dependency.
cllw-oreis now=0.5.2-rc.1with thevitamincfeature, in the root workspace. The root,packages/eql, encrypt guest and stack-encrypt fuzz lockfiles are refreshed and pass--locked.New
sem::Orderabletrait. A sealed trait that maps each orderable Rust type to itsvitaminc-oretarget:f32,f64F64String,&'static str,Zeroizing<String>TextVec<u8>,&'static [u8],Zeroizing<Vec<u8>>BytesboolTermError::NoOrderTermbefore any key is made, as beforeOrder terms.
cllw_ore::vitaminc::OreorOpeand is wiped. The visitor opens a column with the value's target and an empty context, since the key already belongs to this index, and encrypts the value.ore-key/v2andope-key/v2.Removed.
OrderSource,text_order_key, theAnydowncasts, and theCllwOreEncrypt/CllwOpeEncryptre-exports.Term types.
OreCllw8VariableV1orOpeCllw8VariableV1.OreTerm<T>andOpeTerm<T>keepTto name the source, so typed callers compile unchanged.KeysetCipher::ore_termandope_termreturn the ciphertext types.Between.op::OrderedcoversString.Betweenno longer refuses a range whose low bound is above its high bound. As in SQL,BETWEEN 20 AND 10is a valid query that matches nothing.TermError::InvertedRange(codestack_encrypt::inverted_range) is gone; it was unreleased.vitaminc_ore::text, the encoding theTexttarget uses.Unencodable text. Text with a code point Unicode 16 doesn't assign has no order term (
TermError::Normalize), on write and on query.Breaking: every order term changes bytes. The causes are the
/v2labels, the column salt (which binds the scheme and the target), and cllw-ore's one marker byte in front of text and bytes. Nothing was deployed (ADR-0002). The pins interm_bytes.rsandfrozen_bytes.rsare re-recorded, as are both cross-path fixtures.Docs. ADR-0009's text-ordering section names the crate, and the CHANGELOG has the Breaking entry.
Go (
languages/golang)ore/ope. Generated files and goldens are regenerated, and the README,doc.goand the stashgen README describe the fold. These are unchanged from the first draft.alice, not 40.Verification
mise x --env test -- cargo nextest run --workspace --all-features: 1183 passed.cargo nextest run -p stack-encrypt --all-features: 644 passed. This includes the trybuild UI binary; no UI snapshot changed.cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo fmt,mise run doc(rustdoc with warnings as errors) andmise run test:doc(stack-encrypt: 40 doctests): all clean.cargo mutants --in-diffon stack-encrypt againstnext: 32 mutants, 13 caught, 19 unviable, none missed. The first run missed one equivalent mutant: deleting theBackendarm inorder_errorgives the same value, becausecllw_ore::Errorhas no content. I removed the arm, and the re-run above is clean. On the commit that removes the inverted-range check: 6 mutants, 6 caught.cargo test --locked -p eql-bindings --all-features(197 passed), andpackages/eql/tests/encryption(37 passed, 3 ignored; those need a database).mise run wasm:guest:testpasses (68 and 71 tests).mise run go:testpasses,mise run go:lintreports 0 issues, andgo generate ./...leaves no diff.u32and for text;Lt/Betweenon text, with folded ties;DATABASE_URL).Related
obterm stores, still waits on anore-rsrelease (stashgen refuses TextOrdOre and TextSearchOre — EQL stores block ORE terms and the engine derives CLLW ORE #1132).Review notes
sem/mod.rs's ORE/OPE section:Orderableand the visitors.cllw-oreis pinned to the pre-release on purpose. When 0.5.2 (or later) is final, bump the pin; the terms shouldn't change.stack-authandstack-kmsfuzz lockfiles are already stale onnext, so--lockedfails there. This PR leaves them alone.https://claude.ai/code/session_01BZhkvKLpNtKuzj3HCfMXoL