Repository navigation
fix(stack-encrypt)!: report a ciphertext that opens as the wrong type as OpenedType, not Aead - #1256
Conversation
… as OpenedType, not Aead A typed decrypt collapsed every failure vitaminc reports into Error::Aead, the error for a wrong key, a wrong context or a tampered ciphertext. That included a leaf that authenticated and only held another type than the target reads: an integer opened as String, say. The Go guest then reported a value sealed as another type as STATUS_AUTH, i.e. tampering. vitaminc's DecryptError already tells the sink's refusal apart from the cipher's, so open_keyed now maps DecryptError::Sink to the new Error::OpenedType, naming only the type the target reads (never the map key a DecodeError can carry). Every other failure stays Aead. Telling a wrong-type reader apart reveals nothing, because the sink runs only after every leaf has authenticated. The guest maps OpenedType to STATUS_ENCODING, as it already maps a typed record field that opens to another kind. Closes #1118.
|
|
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 |
|---|---|---|---|
| 3 | 0 | 2 | 0 |
Every mutant in the changed lines was caught by a test.
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.
Review attestation: automated /code-review medium followed by /cross-check-review-findings on head bd0fe7b. Findings are listed below even when there are none.
Critical / blocking: none.
Findings: none survived cross-check.
Checked against the checked-out tree:
open_keyed→opened_error(packages/stack-encrypt/src/cipher.rs): onlyDecryptError::SinkbecomesOpenedType.Cipher,AuthandMalformedstayAead, so wrong-key, wrong-context and tamper cases are not weakened. A wrong key or context fails authentication before the sink runs, so this adds no oracle.expectedis the vitaminc kind ortype_name::<T>(). It never carries a map key, and the unit test coversMissingField,unknown_keyandduplicate_key.- The remaining
Error::Aeadexpectations I inspected are wrong-context cases and still hold:tests/derive.rs:436,:767, andtests/plan_grammar.rs:741,:896. None is a type mismatch. Erroris#[non_exhaustive], so the new variant is not a source break for downstreammatches. The Go gueststatus_for_errorfalls through toSTATUS_INTERNALfor unlisted variants, and the PR mapsOpenedTypeexplicitly toSTATUS_ENCODING.- Code and payload tests (
codes.rs) and the CHANGELOG Breaking entry are present.
Non-blocking note: the seal-side PendingStackCipherText::plan still maps refusals to Aead. The PR description already calls this out as out of scope.
Source: automated review attestation (Claude Code), static read of the diff and tree. I did not run tests.
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: bd0fe7b2b8
ℹ️ 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".
…m Aead A container's leaves authenticate one by one, but its membership does not: a map entry's AAD binds only its key, and a sequence element's binds no index. So an entry removed or repeated, or elements reordered or dropped, still authenticate, and the target's refusal of the result (MissingField, DuplicateKey, or Expected on a reshaped container) can be tampering. open_keyed now reports OpenedType only when the tree is one leaf and the target refused its kind. Every other failure, every refusal of a container's value included, stays Aead. Found by review on #1256.
…part from Aead", and say why This reverts commit 8e904f8. A container's membership and order are deliberately not authenticated: a stored sequence is a set of rows and a stored map a row's columns, and a database returns rows in any order and a query selects some columns, not all. Authenticating position or presence would break exactly those reads. So a target refusing a container's shape is a shape mismatch, OpenedType, not tampering. The rationale now sits on open_keyed, on Error::OpenedType and in the CHANGELOG, and a test opens a Vec<u32> as u32 and expects OpenedType, so a later review that reads the missing membership check as a defect meets the reason first.
2 participants
Summary
Stack Encrypt is CipherStash's Rust encryption engine, and the Go SDK runs it as a WebAssembly guest. When a program opened an encrypted value as the wrong type (an integer opened as text, say), Stack Encrypt reported
Error::Aead. That is the error for a wrong key, a wrong context or a tampered ciphertext, and the Go guest turned it intoSTATUS_AUTH, which tells the host someone altered the data. Nobody had: the value was just sealed as another type. Now that case is a new error,Error::OpenedType, which names the type the reader expected. Real tampering is stillAead.Changes
packages/stack-encrypt/src/cipher.rs:Error::OpenedType { expected }, with the codestack_encrypt::opened_type, help text and anexpectedpayload.open_keyedmaps vitaminc'sDecryptError::Sink(every leaf authenticated, but the target refused the value) to it. A cipher failure,AuthandMalformedstayAead.expectedis the vitaminc kind the target wanted when the refusal names one, and otherwise the target's Rust type name. It is never the refusal's own text, which can contain a map key.languages/golang/encrypt/guest/src/status.rs:OpenedTypemaps toSTATUS_ENCODING, the status a typed record field that opens to another kind already gets.packages/stack-encrypt/src/codes.rs: the new variant is added to the every-variant code and payload tests.packages/stack-encrypt/CHANGELOG.md: a Breaking entry under Unreleased.Verification
cargo nextest run -p stack-encrypt --all-features: 671 passed. One existing test expectedAeadfor auint32leaf opened asString; it now expectsOpenedType { expected: "string" }. Every otherAeadexpectation in the suite is a wrong-key, wrong-context or tamper case, and still passes unchanged.only_a_sink_refusal_is_told_apart_from_aeadcovers every arm of the mapping. New testa_leaf_or_a_container_of_another_shape_is_opened_typegoes through a real open: au32leaf opened asString, and aVec<u32>opened asu32, are bothOpenedType.cargo test --locked --features eql, including the new status assertion: pass.cargo clippy -p stack-encrypt --all-features --all-targets -D warnings,mise run docandcargo fmt --check: clean.next.next's tree is identical to that base, so the code tested is the code here.Related
Closes #1118
Review notes
OpenedTypecan't come from an altered leaf, and saying "wrong type" reveals nothing.OpenedTypetoo, by design. A container's membership and order are deliberately not authenticated. A stored sequence is a set of rows and a stored map a row's columns, which a database returns in any order and a query selects in part. A refusal of a container's shape is a shape mismatch, not tampering. Codex flagged this as P1; that finding doesn't apply. The rationale is written onopen_keyedandError::OpenedType, and a test pins it.PendingStackCipherText::plan, still maps a value vitaminc refuses to encrypt (a repeated key, say) toAead. That isn't what this issue is about, so I left it for a separate change.Stringfield reads a data-plan leaf as "\nalice" — the Rust chain and a data plan seal different leaf bytes #1118, the different leaf bytes between the Rust chain and a data plan, was fixed by vitaminc 0.6's tagged leaves.mod given_the_leaf_encodingindynamic/record.rspins that.