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
 |
|
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 6 | 0 | 0 | 0 |
Every mutant in the changed lines was caught by a test.
coderdan
left a comment
coderdan
left a comment
There was a problem hiding this comment.
Review of this PR's own diff (the typed Rust half of #1577). I compiled scratch programs against the branch for each claim below, and ran one scratch test against the frozen stored rows. None of the scratch files is committed.
Findings
- Blocker.
tests/ui/op_order_on_bytes.stderrquotes the standard library (::: $RUST/alloc/src/vec/mod.rs). rustc prints that snippet only whenrust-srcis installed. The CI toolchain in the rootmise.tomlhas norust-src, so theuitest will fail in CI while it passes locally. It is the only.stderrfile with a$RUST/line. - Should fix.
op_order_on_bytes.rsno longer tests the query refusal it is named for. The plan now fails at.with(Ore), so the.on(..)error is gone from the.stderr. No test now pins that an ordering query over bytes does not compile. - Should fix. The refusal can be bypassed through the public low-level operations. I compiled a user
Index<Vec<u8>>whoseoperation()istarget::ore()and whosespec()isIndexSpec::Ore.Plan::context(..).with::<Vec<u8>, _>((Equality, MyOre)).build()compiles. A userEncryptFrom<Vec<u8>>target that zipsciphertext()withtarget::ore()and returnsIndexSpec::Orefromindexes()also builds as a plan. TheIndexrustdoc invites exactly this: "an index defined in another crate implements it by composing this module's public operations". Sotarget::ore,opeandore_blockdo declare an index in practice. The keyset'sore_term,ope_termandore_block_termcannot be composed into a description, so bounding only the threetargetoperations onop::Orderedcloses the gap. The term-level byte tests (tests/term_bytes.rs,tests/ore_block.rs) and the dynamic path use the keyset functions, so they are not affected. - Should fix. Rustdoc that is now false or silent.
Oresays "Defined for everySthe scheme can order", but the scheme orders bytes and the index no longer takes them (OpeandOreBlocksay "the sameS"). TheIndextrait says it is "Implemented for exactly the plaintexts the index's scheme is defined over", and its diagnostic note saysOreandOpe"take the types their schemes encode".Orderabledoes not say thatVec<u8>has an order term but no order index.Ordereddoes not say it is the bound on the order indexes. - Should fix. Test gaps in
order_index_on_bytes.rs. It pins theIndexandTermSetbounds, andEncryptTargetforOpeTermandOreBlockTerm. It does not pin theEncryptFrombound on the three order terms, orEncryptTargetforOreTerm. I checked:Plan::value::<Vec<u8>>().encrypt_into::<OreTerm<Vec<u8>>>()andkeyset.encrypt_as::<Vec<u8>, Encrypted<(EqualityTerm, OreBlockTerm<Vec<u8>>)>>are refused today, but reverting theEncryptFrombounds would fail no test. - Nit. No positive test that each
Orderedtype still takes each order index through a plan, anEncrypted<..>target andEncryptFrom.tests/ui/pass/indexes.rscoversu32andStringonly. A pass case would catch a later narrowing ofOrdered. - Nit.
CHANGELOG.mdsays the dynamic interface still admits an order index on bytes "so that rows the Go SDK stored stay readable". Reading does not need that (see the dynamic notes below). The reason is that the decision is open. The entry could also say that a record which only opens can still nameEncrypted<OreTerm<Vec<u8>>>, so rows stored before stay readable from Rust.
Checked, and nothing to fix
- Every ordinary typed declaration path is refused. I compiled each of these over
Vec<u8>and each fails with theOrdereddiagnostic:.withwithOre,Ope,OreBlock,(Equality, Ore)andvec![Ope];Plan::value().with(..);Plan::value().encrypt_into::<Encrypted<(EqualityTerm, OreTerm<Vec<u8>>)>>()and::<OreTerm<Vec<u8>>>(); the fields builder'sencrypt_index,index,encrypt_intoandtarget;cipher.encrypt(..).context(..).with(Ore);target::indexed(Ope);encrypt_as;Vec<..>,Option<..>and tuple targets;#[derive(EncryptTarget)]; and#[derive(EncryptFrom)]with anOreTerm<Vec<u8>>field. Markers come from the derive, so they are covered too.&[u8],Box<[u8]>andProtected<Vec<u8>>were neverOrderable. - EQL v4 types. No
eql-bindingsv4 type stores an order term over bytes. - Decryption.
Encrypted<OreTerm<Vec<u8>>>can still be named forDecryptIntoandDecryptSource, and#[derive(DecryptSource)]with such a field compiles. That is right: rows stored before stay readable. - Query-side bounds.
QueryTerm,LabelledTargetandOperatorAnswerfor the order terms are still onOrderable. They cannot be reached for bytes, becauseAnswersandDefaultIndexare already onOrderedand nothing can store an order term over bytes. Harmless, so I would leave them. - The
other types implement traitlists in the new.stderrfiles are sorted by rustc and match the existingop_order_on_bytesoutput, so they are stable. - Mutation testing. The diff changes trait bounds, comments and tests only.
cargo mutants --in-diffhas no function body to mutate. - House rules. No issue numbers in code comments or rustdoc. The CHANGELOG and plan references are allowed.
The dynamic half (information for the decision, not a finding)
I read src/dynamic/record.rs and ran the frozen every_kind row from tests/fixtures/stored_rows_v1.json under edited plans:
record_rowtakes only the"c"node of a sealed field. It ignores every other output key, and it ignores stored fields the plan does not name. A field with no"c"(index only) is never opened (FieldSpec::opens).- With
"ore"removed fromblob, the row opens to the frozenopenedvalue, byte for byte. - With
"ore"removed frombloband the["ope"]-onlyblob_opefield removed from the plan, the row still opens to the same value. - A field with
"outputs": []is refused when the plan is parsed (no outputs).
So option 2's stated cost ("its rows could not be read without a further rule") does not hold for reading. The rule is "remove the field", and the rows open unchanged. Option 2's real costs are: a Go program whose generated plan has the declaration fails to parse until it is regenerated; the frozen-row test parses the frozen plan as it is, so it would need a v2 fixture or a test change; and the record-declarations-v1 cases change. Option 1 keeps every plan parsing and every row opening, but it adds a write-time refusal, and the conformance cases still change meaning (accepted for reading only). Option 3 keeps writing the leaking term for every new row.
| | pub struct Indexed<X>(X); | ||
| | --------------------- doesn't satisfy `Indexed<stack_encrypt::Ore>: ValueLayout<Vec<u8>>` | ||
| | | ||
| ::: $RUST/alloc/src/vec/mod.rs |
There was a problem hiding this comment.
1. Blocker. This snippet is printed only when rust-src is installed. The CI toolchain (root mise.toml: rustc,cargo,rustfmt,rust-std,clippy,rust-docs,llvm-tools-preview) has none, so the ui test fails in CI. It is the only .stderr with a $RUST/ line (grep -l 'RUST/' tests/ui/*.stderr). The E0599 on .build() is what prints it. Ending the plan before .build(), or using an index bytes do take, removes it.
There was a problem hiding this comment.
Fixed in a3073c5. op_order_on_bytes.rs no longer calls .build() on a plan that cannot build, so there is no E0599 and no $RUST/ snippet. grep -l RUST/ tests/ui/*.stderr prints nothing.
| //! Bytes have no order key the ordering operators compare, so an ordering on | ||
| //! a bytes plan does not compile, even where the plan declares an ORE index. | ||
| //! Bytes have no order key the ordering operators compare. So a plan | ||
| //! cannot declare an ORE index on bytes, and an ordering on a bytes plan |
There was a problem hiding this comment.
2. Should fix. With Ore on line 11 the plan fails first, and the query on line 14 is never type-checked: the .on(..) error is gone from the .stderr. So nothing pins that an ordering query over bytes does not compile any more. order_index_on_bytes.rs already covers the plan refusal. Declaring the plan with Equality keeps this file about the query.
There was a problem hiding this comment.
Fixed in a3073c5. The case now names the index with .context("users/blob").via(Ore), so the .stderr again pins the query refusal (Ore does not implement Supports<Lt, Vec<u8>>). I tried a plan with Equality first, but it added two noisy Select<Ore, _> errors; via gives the one error the file is about.
| term_operation!( | ||
| /// The order-revealing term of `S` under the context the tree hands it. | ||
| /// The bound is the leaf's own: it says which `S` has an order target. | ||
| /// This is the low-level operation. It declares no index, so it also |
There was a problem hiding this comment.
3. Should fix. "It declares no index" does not hold. I compiled a user Index<Vec<u8>> whose operation() is target::ore() and whose spec() is IndexSpec::Ore, and Plan::context("f/d").with::<Vec<u8>, _>((Equality, MyOre)).build() compiles. A user EncryptFrom<Vec<u8>> that zips ciphertext() with target::ore() and returns [IndexSpec::Ore] from indexes() builds as a plan too. The Index rustdoc tells users to build indexes this way. The keyset's ore_term and the others return a term, not a description, so bounding only target::ore, ope and ore_block on op::Ordered closes this. The byte tests and the dynamic path use the keyset functions.
There was a problem hiding this comment.
Fixed in a3073c5. target::ore, target::ope and target::ore_block are now bounded on op::Ordered, and order_index_on_bytes.rs pins all three. The keyset functions stay on Orderable. tests/ui/pass/owned_without_clone.rs now bounds its generic helpers on Ordered. The CHANGELOG entry says so.
| matching() | ||
| } | ||
| } | ||
| // Each order index is declared over `Ordered`, the trait its operators |
There was a problem hiding this comment.
4. Should fix. The rustdoc does not say this yet. Ore (line 228) says "Defined for every S the scheme can order", and the scheme orders bytes. The Index trait says it is "Implemented for exactly the plaintexts the index's scheme is defined over", and its on_unimplemented note says Ore and Ope "take the types their schemes encode". sem::Orderable and op::Ordered do not say which one bounds the order indexes.
There was a problem hiding this comment.
Fixed in a3073c5. Ore now says it is defined over op::Ordered, lists the types, and has a compile_fail example for bytes. The Index trait doc and its on_unimplemented note now say order indexes take the op::Ordered types. sem::Orderable and op::Ordered each say which one bounds the order indexes, and that Vec<u8> has a term but no index.
| let _ = Plan::context("files/digest").with::<Vec<u8>, _>(Ore); | ||
| let _ = Plan::context("files/digest").with::<Vec<u8>, _>(Ope); | ||
| let _ = Plan::context("files/digest").with::<Vec<u8>, _>(OreBlock); | ||
| let _ = <OpeTerm<Vec<u8>> as EncryptTarget<Vec<u8>>>::encryption::<()>(); |
There was a problem hiding this comment.
5. Should fix. This pins EncryptTarget for OpeTerm and OreBlockTerm, but not for OreTerm, and not the EncryptFrom bound on any of the three. Reverting the three EncryptFrom bounds in operations.rs would fail no test. <OreTerm<Vec<u8>> as EncryptFrom<Vec<u8>>>::indexes() and the same for the other two would pin them.
There was a problem hiding this comment.
Fixed in a3073c5. The case now also pins EncryptTarget for OreTerm, EncryptFrom for all three order terms, and the three low-level operations. A new pass case, tests/ui/pass/order_indexes.rs, checks that every Ordered type takes each order index through a plan, an Encrypted target, a term target, EncryptFrom and the low-level operation (finding 6).
| keyset, and the low-level operations `target::ore`, `ope` and | ||
| `ore_block`, still take bytes. They derive one term and declare no | ||
| index. | ||
| - The dynamic interface is unchanged. It still admits an order index on |
There was a problem hiding this comment.
7. Nit. Reading does not depend on this. I ran the frozen every_kind row with "ore" removed from blob and with blob_ope removed from the plan, and it opens to the frozen value. The dynamic path is unchanged because the decision is open. It may also help to say that a record which only opens can still name Encrypted<OreTerm<Vec<u8>>>.
There was a problem hiding this comment.
Fixed in a3073c5. The entry now says the dynamic path is unchanged because the decision is open, and that a record which only opens can still name Encrypted<OreTerm<Vec<u8>>>, so rows stored before stay readable.
coderdan
changed the title
|
Pushed |
coderdan
left a comment
coderdan
left a comment
There was a problem hiding this comment.
Review of a51efdc41 ("a data plan refuses an order index on bytes") only. The two earlier commits were reviewed before.
I found no correctness defect in the engine change. The findings are stale documentation and weak tests. Most severe first.
-
Low-medium. The declaration grammar still says bytes admit an order index.
conformance/stack-encrypt/declaration-grammar.md:178, the admission table:| bytes | Admitted | Refused | Admitted |. The corpus now refusestype-bytes-ore,type-bytes-opeandtype-bytes-ore_blockwithindex_not_admitted. This file is the specification a new runner (Python, TypeScript) is written from. A runner built from the table admits the declaration, and then fails those three cases. -
Low. The Go CHANGELOG names a tag word that does not exist.
languages/golang/CHANGELOG.md:15-16: "A[]bytefield taggedindex=ore,index=opeorindex=ore_blockis now a field error".ore_blockis not a tag word.stashgen/tag.go:317refuses it asunknown index "ore_block"on every field, before and after this commit;stashgen/declaration.go:197-200says a field reaches it only throughencrypt_into=. A reader would thinkindex=ore_blockis accepted on other types. Only a hand-builtrecord.Planwithrecord.OreBlockis newly refused. -
Low.
coverage.mdstill records the unanswered order index.conformance/stack-encrypt/coverage.md:146-147: "Unsupported combinations the corpora record rather than skip: the order index onbytesstores a term no operator reads". The corpora no longer record it: the declaration is refused and theunansweredlist is gone. -
Low. Two rustdoc lists of refused pairs leave bytes out.
packages/stack-encrypt/src/dynamic/mod.rs:559-560(Reason::IndexNotAdmitted: "match on an integer, equality or an order index on a boolean, any index on a composite") andpackages/stack-encrypt/src/dynamic/record.rs:416-419(FieldPlan::with_type,# Errors, which also names the 8-bit integer). Both are on docs.rs. A caller whose bytes field, accepted by rc.3, is now refused withindex_not_admittedlooks there and does not find the case. -
Low. A Go assertion can no longer fail.
languages/golang/stashgen/refusal_test.go:389checks that no notice contains "no generated method queries". This commit removed the only code that wrote that text (indexesAnswerinemit.go), so the check passes whatever the generator prints about those fields. -
Low. The Go fake engine keeps the list the Rust engine dropped.
languages/golang/stashgen/enginetest/enginetest.go:221-225, inansweredBy:orderable := (integer(t) && !eightBit(t)) || logical(t) || ...is the Go copy of theordered(kind)list this commit deleted fromsrc/dynamic/query.rs. It is now redundant:declaredalready callsindexApplies, which refuses an order index on bytes, a boolean and an 8-bit integer. It is two lists that must agree, the pattern the commit removed from the engine. IfindexApplieschanges andorderabledoes not, the fake admits an index and answers nothing through it, and only the casesTestRefusalsHoldAgainstTheEmbeddedEnginelists would show it.
What I checked and found sound:
- No other path stores an order term on bytes.
FieldPlan::with_typeis where a data plan checks its indexes against the kind, and a field's outputs are fixed inFieldPlan::new, before it. The guest'svalidateandencrypt_recordboth parse throughrecord::plan. No EQL type ineql-bindingshas a bytes plaintext, and a target field's kind must equal its descriptor's. The guest'sse_termstill derives an order term over bytes throughIndexSpec::defines, but as one term, and the Go host calls it only for an output the plan declares. Op::answered_byis unchanged for every kind but bytes. Order indexes answer onlylt,lte,gt,gteandbetween. For those, the deletedordered(kind)andadmits(kind, Ore | Ope | OreBlock)agree on every other kind: both take the integers of 16 bits and wider, both floats, text, decimal, date and timestamp, and both refuse bool, int8, uint8, array, object and json. There is no uuid kind.tests/stored_rows.rs. The stripping is keyed on a frozen field whosetypeisbytesand an output key ofore,opeorore_block. Onlyblobandblob_opeinevery_kindmatch, so every other field is compared as before.STACK_ENCRYPT_REFREEZE_ORDER_TERMS=1passes and leavesstored_rows_v1.jsonbyte-identical, retired terms included (git statusclean after the run).- Mutation testing.
cargo mutants -p stack-encrypt --in-diffover this commit'ssrcdiff: 6 mutants (admitstotrueand tofalse, both&&to||,==to!=, delete!), 6 caught. - Go.
record-declarations-v1.inputs.go.jsonis consistent: its bytes cases take their outcome from the shared cases file. With all four guests rebuilt,go generate ./...changes no file, andgo vet ./...andgo test ./...pass. - Rust.
cargo nextest run -p stack-encrypt -p stack-encrypt-derive --all-features: 1390 passed, 1 skipped.
I will fix all six in one commit on this branch.
|
All six findings from the review of
Checks run on 7044224:
On |
Codex Review Summary: ✅ CompletedThis 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.
Review attestation
Reviewed 7044224c · posted from Claude Code · claude-opus-5-5 · effort medium
Sources: code-review · claude-opus-5-5
Skills: code-review, company-skills:review-cross-check, company-skills:review-post
Findings (2): 2 minor, 2 inline, 0 below.
Seven findings were checked against 7044224c, and five were refuted before posting. Order terms over bytes are unreachable through query bounds and stored plans. ore_block is not a tag word. No Go caller can produce a bytes order term. The order-index lists agree today.
Generated by Claude Code
| - The dynamic interface is unchanged. It still admits an order index on | ||
| bytes. How it should treat a declaration that Go rc.3 accepted is not | ||
| decided yet. |
There was a problem hiding this comment.
minor This bullet says the dynamic interface "still admits an order index on bytes", but the Breaking entry above ("A data plan refuses an order index on bytes") and dynamic::admits (src/dynamic/kind.rs:89-93) now refuse ore, ope and ore_block on bytes with index_not_admitted. Delete the bullet or point it at that entry.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| with the case it described. To upgrade, remove the index from the tag | ||
| and generate again. Rows stored with the term still open: opening |
There was a problem hiding this comment.
minor The upgrade note does not say that a _stash.go generated by rc.3 for such a field still compiles, but the guest refuses its whole plan at parse (FieldPlan::with_type → admits). Until the tag is edited and stashgen re-run, Encrypt and Decrypt of that record type, and its bytes field's Field.Encrypt, fail with index_not_admitted, including on rows that were stored correctly. Say so before "To upgrade".
Source: code-review · claude-opus-5-5
Generated by Claude Code
4b30382 to
b24c0c7
Compare
7044224 to
bc8d1d2
Compare
b24c0c7 to
18cff59
Compare
bc8d1d2 to
d07ee6f
Compare
18cff59 to
32519a5
Compare
d07ee6f to
cad6572
Compare
Ore, Ope and OreBlock were indexes over sem::Orderable, which covers Vec<u8>. Their operators need op::Ordered, which does not. So a plan or a record could declare an order index on bytes. Each row then stored a term that reveals its order, and no query could use the term. Each order index, its term set, and its EncryptFrom and EncryptTarget impls are now over op::Ordered, so the index and its operators share one trait. The low-level operations and the keyset's term functions keep Orderable: they derive one term and declare no index. The dynamic interface still admits an order index on bytes. The frozen stored rows declare one, and rows the Go SDK stored must stay readable. How such a declaration is handled is the open question in the issue. Part of #1577
The low-level operations target::ore, target::ope and target::ore_block are now bounded on op::Ordered, as the order indexes are. An index or a target of another crate is built from them, and before this change one could declare IndexSpec::Ore over Vec<u8> and store an order term for bytes. The keyset's ore_term, ope_term and ore_block_term still take bytes: each returns a term, not a description, so nothing can be built from it. op_order_on_bytes.stderr quoted the standard library, which rustc does only when rust-src is installed. CI has no rust-src, so the ui test would fail there. The case now names the index with via(Ore) and pins the query refusal again, which the plan refusal had hidden. order_index_on_bytes.rs now also pins EncryptFrom for the three order terms, EncryptTarget for OreTerm, and the three low-level operations. A new pass case checks that every Ordered type takes each order index. The rustdoc on Index, Ore, sem::Orderable and op::Ordered now says which trait bounds the order indexes and why bytes are left out. The CHANGELOG says rows stored before stay readable, and why the dynamic path is unchanged.
The typed path already refuses an order index on bytes at compile time. The dynamic path, which the Go SDK uses through the guest, still accepted the declaration: `dynamic::admits` read the table of values the scheme has a term for, and the scheme has an order term for bytes. Each row then stored a term anyone holding two rows can compare, and every ordering query was refused as `operator_not_answered`, from a second list in `dynamic::query`. `admits` now refuses `ore`, `ope` and `ore_block` on bytes, and the second list is gone: `Op::answered_by` reads `admits` alone, so the two cannot disagree again. `IndexSpec::defines` is unchanged, because it reports what the scheme defines, and the low-level term operations still use it. A row stored under such a declaration still opens once the index is removed from the plan. The frozen stored rows declare one on two fields, so their test now opens them under the plan with those indexes removed, and the fixture file is untouched. The conformance corpus changes with the rule: the three bytes declaration cases are refused, the `unanswered` list is removed from the value profile and from both runners, and the query-refusal profile's bytes field declares equality alone. In Go, `stashgen` refuses the tag as a field error, its notice for an index no generated method queries is removed with the only case it had, and the two probe types lose their bytes field. Closes #1577
The engine change in the previous commit was sound. The review found text and tests that still described the old rule, or that no longer checked anything. - The declaration grammar's admission table said bytes admit `ore`, `ope` and `ore_block`. A runner written from it would admit a declaration the corpus refuses. The table now refuses them, and says why. - `coverage.md` listed the order index on bytes as a combination the corpora record. They no longer do. - The Go CHANGELOG named `index=ore_block` as a tag that is now refused. It is not a tag word, and stashgen refused it before this change. Only a hand-built plan with an `ore_block` output on bytes is newly refused. - The rustdoc for `Reason::IndexNotAdmitted` and `FieldPlan::with_type` lists the refused pairs. It now names an order index on bytes. - A stashgen test checked that no notice held a text the generator no longer writes, so it could not fail. It now checks that no notice names one of the test's fields. - The Go fake engine kept its own list of the kinds the orderings are answered over, beside `indexApplies`. That is the second list the previous commit removed from the engine. `answeredBy` now reads `indexApplies` alone, and the unused `logical` helper is removed.
The docs that explain why an order index refuses bytes now use short sentences with one idea each. They say that an operator accepts a type, where they said that an operator is answered over a type. The note on the Index error lists what each index accepts.
32519a5 to
9916276
Compare
cad6572 to
d8aeadb
Compare
Summary
A field of bytes can no longer declare an order index, in Rust or in Go. It used to be accepted, and it leaked: each row stored a term that reveals its order, and no query could ever use that term.
Stack Encrypt (
packages/stack-encrypt) is the Rust encryption engine under every CipherStash SDK. An order index (Ore,OpeorOreBlock) stores a term beside the ciphertext, so the database can answer<,>andBETWEENon encrypted values. The term reveals the order of the rows to anyone who holds them. That cost is accepted where a query needs it.There were two ways to declare one, and both had the same gap:
sem::Orderable, which coversVec<u8>. The ordering operators needop::Ordered, which does not. So the compiler accepted the index on bytes and refused every query on it.To upgrade: remove the order index from the bytes field. Rows stored with the term still open, because opening ignores a stored term the plan does not declare. The old terms stay in the database until the row is written again.
This PR is stacked on #1610. The base branch is the branch of #1610, so the diff shows only this change.
Changes
Typed Rust
Index<S>forOre,OpeandOreBlockis bounded onop::Ordered. The index and its operators now share one trait, so they cannot disagree.TermSet,EncryptFromandEncryptTargetimpls. Those are the other ways to declare an order term for a field..with::<Vec<u8>, _>(Ore)on a plan, a record fieldEncrypted<OreTerm<Vec<u8>>>, and an order term over bytes as a target.target::ore,opeandore_blockare bounded onop::Orderedtoo. An index or a target of another crate is built from them, so without the bound it could still declareIndexSpec::Oreover bytes.ore_term,ope_termandore_block_termstill take bytes. Each returns one term, not a description, so no plan, record or target can be built from it. Existing tests pin how bytes order at that level.DecryptInto,DecryptSource) can still nameEncrypted<OreTerm<Vec<u8>>>, so rows stored before stay readable from Rust.The dynamic interface
dynamic::admitsrefusesore,opeandore_blockon abytesfield. A data plan that declares one is refused withindex_not_admitted, before any key is requested.dynamic::query, is removed.Op::answered_bynow readsadmitsalone, so the two cannot disagree again.IndexSpec::definesis unchanged. It reports whether the scheme has a term for a value, and the low-level term operations still use it.Go
stashgenrefusesindex=ore,opeorore_blockon a[]bytefield as a field error. It asks the engine, so this follows from the engine change.stashgenno longer prints a notice for "an index no generated method queries". The order index on bytes was the only case left that produced it.ProbeandProbeOpelose their bytes field, and their generated files are regenerated.Conformance corpus
record-declarations-v1:type-bytes-ore,type-bytes-opeandtype-bytes-ore_blockare now refused withindex_not_admitted.record-values-v1: theunansweredlist is removed from the corpus and from both runners.bytes-binarydeclares equality alone.record-query-refusals-v1: the bytes field declares equality alone. The six*-on-bytes-order-indexcases are renamed*-on-bytes. They are still refused asoperator_not_answered.Tests
tests/ui/order_index_on_bytes.rsis a new compile-fail case. It covers a plan with each of the three indexes, a derived record field, each term as an owned target,EncryptFromfor each term, and the three low-level operations.tests/ui/pass/order_indexes.rsis a new pass case: everyOrderedtype takes each order index through each of those paths.op_order_on_bytesnow names the index with.via(Ore), since a plan cannot declare it on bytes. It still pins the query refusal.order_term_of_an_8_bit_integerreports the new trait for the same refusal.tests/stored_rows.rs: the frozen filestored_rows_v1.jsonis not changed. Itsevery_kindplan declares an order index on two bytes fields. The test removes those indexes from the plan, opens each row as stored under it, and compares the resealed row without the retired terms. This is the check that old rows still open after the upgrade.Docs
conformance/stack-encrypt/values-contract.md,query-refusals-contract.mdandexchange-contract.mddescribe the refusal.equalityalone for[]byte.Verification
I ran these locally, on Rust 1.94.1 and the repo's pinned Go:
cargo nextest run -p stack-encrypt -p stack-encrypt-derive --all-featuresuitest, the frozen rows and the conformance runners.cargo test -p stack-encrypt --all-features --doccargo clippy --workspace --all-features --all-targets -- -D warningscargo doc -p stack-encrypt --all-features --no-deps, warnings as errorscargo testgo generate ./...,go vet ./...,go test ./...inlanguages/golang, with all four guests rebuiltI did not run the EQL crates' tests, the Postgres suites, the live tests or mutation testing locally after the last commit. CI runs them.
Related
Closes #1577. The base branch is not
main, so GitHub will not close the issue on merge. Stacked on #1610.Review notes
Indeximpls inpackages/stack-encrypt/src/target/index.rs, thentests/ui/order_index_on_bytes.rs..stderrquotes the standard library. CI has norust-src, so a snippet from$RUST/...would differ there.grep -l RUST/ tests/ui/*.stderrprints nothing.Orderablestill covers bytes. I tried removingVec<u8>fromOrderable. It breakstests/ore_block.rsandtests/term_bytes.rs, which pin how bytes order at the term level, and the dynamic path, which derives its terms through it. Keeping the term level leaves room to make bytes queryable by order later, which the issue names as an alternative.