Skip to content

feat(stack-encrypt)!: derive owned record declarations with EncryptTarget and DecryptSource - #1504

Open
coderdan wants to merge 10 commits into
feat/stack-encrypt-owned-collectionsfrom
feat/stack-encrypt-owned-records
Open

coderdan wants to merge 10 commits into
feat/stack-encrypt-owned-collectionsfrom
feat/stack-encrypt-owned-records

Conversation

@coderdan

@coderdan coderdan commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Stack Encrypt (the Rust crate stack-encrypt) encrypts a value under a per-value ZeroKMS data key. Until now a user could encrypt one scalar the owned way (keyset.encrypt(value) consumes the value and wipes it afterwards) but a record (a struct whose fields are encrypted individually, some kept in the clear) could only be declared the older structural way, over a borrowed value, with no way to say "carry this field through in the clear".

With this PR a user writes:

#[derive(Debug, PartialEq, Zeroize)]
struct User { id: u64, name: String, email: String, created_at: i64 }

#[derive(EncryptTarget, DecryptSource, Serialize, Deserialize)]
#[stash(struct = User, context = "users")]
struct EncryptedUser {
    #[stash(passthrough)] id: u64,          // kept in the clear, moved into the record
    name: Ciphertext,                       // sealed under ("users", "name")
    email: Ciphertext,                      // sealed under ("users", "email")
    #[stash(passthrough)] created_at: i64,
}

let encrypted: EncryptedUser = user.encrypt_into(&keyset).await?;           // one batched key request
let restored: User = User::decrypt_from(encrypted, &cipher).await?;
let rows: Vec<SearchableUser> = users.encrypt_into(&keyset).await?;       // a whole column, one batch

The conversion wipes the plaintext fields it seals and moves (never copies) the fields the caller keeps in the clear. A struct record must name every field of its plaintext (an unaccounted field is a compile error, not a field dropped unwiped), and a field opened before a later field is refused is wiped; a caller's context extends every field's pair (encrypt_into_with_context(&keyset, row_id)) and the same extension opens it. Records nest, a #[stash(plaintext = String)] record is a searchable scalar (ciphertext plus equality term) usable as a field, a record can store its own context, and a field whose target cannot take the record's context is a compile error at that field.

The structural derives (EncryptFrom / DecryptInto) stay; the owned derives emit them as a bridge so an owned record is also a field of a structural record or a typed plan field. Their removal is a separate issue.

Changes

Core (packages/stack-encrypt)

  • EncryptTarget gains owned_encryption() (the description over the plaintext by value, defaulting to guard-with-Protected, run the borrowed encryption(), wipe) and indexes(). EncryptInto and the owned Vec<T> column now run owned_encryption; the column runs every element even after an earlier element fails, so none is dropped unwiped.
  • New Decryption::zip_wiping: zip that wipes the left side's plaintext when the right side fails or panics; the owned record derive opens its fields through it. The owned Vec column keeps the elements it has not yet handed over in a wiping guard, so a panicking element declaration does not drop the rest unwiped.
  • New Encryption::split (two descriptions over two different parts of one owned value, one context, one batch) and Encryption::project_owned: how a record divides its struct between its fields without cloning. split also takes a wipe: fn(&mut U) for the part that waits while the first description runs. If that description panics, the waiting part is wiped during the unwind. The derive's wipe zeroizes only the sealed fields, so passthrough field types get no Zeroize bound.
  • EncryptInto::encrypt_into (context omitted) now guards the plaintext with Protected until the declaration and its context are built, as encrypt_into_with_context already did. A declaration that panics while it is being built wipes the plaintext.
  • New caller inputs: ContextExtension<C> (omit = declared contexts stand; supply = extend each) and ExpectedContext<T> as a ContextInput (omit = open under the stored context; supply = also check it). Conversions so a record field is handed its pair: RequiredContext<C>: From<CallerContext>, ContextExtension<C>: From<CallerContext>, RequiredContext<NonEmpty<T>> into the core contexts, and the reverse for the bridge.
  • Owned declarations on the core leaves: StackCipherText (both ways, RequiredContext<AeadContext>), Encrypted<Terms> (seal under RequiredContext<CallerContext>, open under RequiredContext<AeadContext>), EqualityTerm, MatchTerms, OreTerm, OpeTerm, OreBlockTerm as EncryptTarget alone. Ciphertext is Decryptable and a DecryptField, so it is a record field.
  • FieldsBuilder::target::<T, _>(): a typed plan field laid out by an owned-only target (e.g. Ciphertext).
  • Breaking: the structural Vec<T>: DecryptInto<Vec<P>> / DecryptField<Vec<P>, _> impls open through Decryption::all_wiping and so require P: Zeroize; previously the elements recovered before a late element failure were dropped unwiped. A new drop-witness test fails without the wipe.

Derive (packages/stack-encrypt-derive)

  • #[derive(EncryptTarget)] and #[derive(DecryptSource)], taking the existing attributes plus #[stash(passthrough)] (with struct = .. only; refused on the structural derives, on plaintext records and beside identity/default/decrypt, each with its own message). The owned form destructures the plaintext exhaustively, so the plaintext struct cannot implement Drop. struct records get both a borrowed and an owned form; plaintext records, context_type = AeadContext records and context_field records are supported. Both derives also emit the structural EncryptFrom / DecryptInto / DecryptField bridge and Decryptable.

Tests

  • tests/owned_records.rs: the public mixed-record and searchable-record programs as executable tests (serde round trip, descriptors per field, row-binding extension, passthrough moved not cloned — a Clone that panics — sealed fields wiped before any await and on a refused context, Vec<User> into Vec<SearchableUser> as one generate_keys call and the matched rows opened in one retrieve_keys call, nested/renamed/pinned fields and their composed descriptors, stored context with expectation, plaintext records over structural and opaque leaves, the bridge into a structural record and a plan).
  • tests/owned_equivalence.rs: one record sealed by a hand-written EncryptTarget, the derive and the dynamic data plan against one recording provider: same descriptors, same equality-term bytes, every author opens every author's record.
  • tests/ui: field-specific compile-fail cases (NoCallerContext / RequiredContext<u64> field in a struct record, a term in an AeadContext-only record, omitted required context, non-Zeroize plaintext, every passthrough misuse) and a must-compile file of the owned forms; eight existing snapshots re-recorded for rustc's longer implementor lists.
  • tests/collections.rs: the structural Vec path wipes its recovered prefix after a late element failure, as a column and as a column field of a structural record; a panicking element declaration wipes the elements after it.
  • tests/owned_records.rs also pins that a record field opened before a later refusal is wiped; tests/ui/owned_struct_unmapped_field.rs pins the unaccounted-field refusal.
  • tests/owned_records.rs panic witnesses: a_panicking_field_declaration_wipes_the_sealed_fields_after_it (a panic in the outer split and in a nested split) and a_declaration_that_panics_while_constructed_wipes_the_plaintext. Both are enrolled in the owned-records lifecycle suite.
  • Mutant coverage: each_owned_term_target_names_its_index and a_context_extension_is_inspected_as_supplied_or_omitted. ExpectedContext's omitted -> Default::default() is an equivalent mutant (its body is Self::default()), so it is excluded in .cargo/mutants.toml with the reason.
  • lifecycle-profiles.json: a new owned-records suite enrolls the record drop witnesses; the new collection witnesses join collections.

Docs

  • Crate docs "Records" section (doctest), target module docs with the trait/ownership table, derive docs "Owned records" section and the passthrough attribute row, ADR-0012 (signature/import/ownership map and the bridge decision; numbered 0012 because feat(stack-encrypt)!: add OpaqueBlob whole-value encryption #1503 takes 0011), conformance/stack-encrypt/coverage.md rows, CHANGELOG [Unreleased] (Breaking / Added / Deprecated).

Verification

All run from the worktree on the rebased head, no credentials:

  • cargo fmt --all --check — clean.
  • STACK_CONFORMANCE_REQUIRED=1 mise x --env test -- cargo nextest run -p stack-encrypt -p stack-encrypt-derive --all-features — 1130 passed, 1 skipped (the ui trybuild suite included: 67 compile-fail cases, 11 pass files).
  • cargo clippy -p stack-encrypt -p stack-encrypt-derive --all-features --all-targets -- -D warnings — clean.
  • cargo test -p stack-encrypt --doc --all-features — 57 passed; cargo test -p stack-encrypt-derive --doc — 12 passed (the new owned-record example included).
  • mise run doc:stack-encrypt — builds with warnings as errors.
  • cargo check -p eql-bindings --features stack-encrypt from packages/eql — compiles unchanged.
  • cargo check from languages/golang/encrypt/guest (CARGO_TARGET_DIR unset) — compiles unchanged.
  • npx vitest run scripts/__tests__/stack-conformance-ci.test.mjs scripts/__tests__/stack-lifecycle-conformance.test.mjs scripts/__tests__/stack-eql-conformance.test.mjs — 17 passed.
  • node scripts/stack-lifecycle-conformance.mjs rust — passes, owned-records suite included.
  • Not run: cargo-mutants, live ZeroKMS examples.

Related

Review notes

Start with packages/stack-encrypt/docs/adr/0012-owned-record-declarations.md, then src/target/conversion.rs (owned_encryption), src/target/input.rs (the inputs and conversions) and packages/stack-encrypt-derive/src/owned_encrypt.rs (split chain). tests/owned_records.rs is the executable version of the public examples.

Deliberately deferred, with reasons:

  • Configured nested targets (a JSON index with options as a record field, the plan's target(options = factory) attribute): the only configurable target's plaintext is serde_json::Value, which is not Zeroize, so it cannot be an owned source yet. Nested targets with and without caller context are supported; a typed-options attribute waits on a zeroizing document plaintext.
  • Option<T> and tuple targets stay structural-only in this slice (nullable scalars are deferred by the scalar slice too).
  • EQL adapter owned impls: eql-bindings compiles unchanged and keeps its structural declarations. Adding EncryptTarget impls there would build against the crates.io stack-encrypt, which lacks this surface until the next release (see AGENTS.md on the stack-encrypt feature's registry gate).
  • No #[deprecated] attribute on EncryptFrom / DecryptInto: the crate's own plan and dynamic modules implement them, so the attribute would only be noise until stack-encrypt refactor: remove transitional traits and redundant execution APIs #1488; the CHANGELOG Deprecated section carries the notice.
  • A #[stash(passthrough)] field type must be Clone: the record's borrowed form (what a plan or a structural record runs) copies it; the owned form never does, and a test with a panicking Clone holds it to that.
  • The dynamic lowering and the Go guest are untouched by design: the data grammar already expresses passthrough and sealed fields, and the equivalence test is what ties the three authors to one engine.

Summary by CodeRabbit

  • New Features
    • Added owned record encryption and decryption derives, supporting nested and renamed fields, stored or caller-provided contexts, and searchable indexes.
    • Added passthrough fields that move into records without encryption, plus owned targets for plan fields and batched collections.
  • Bug Fixes
    • Recovered plaintext is wiped when later decryption fails; owned plaintext is also wiped in covered failure and panic cases.
    • Structural vector decryption now requires plaintext elements to support zeroization.
  • Documentation
    • Added guides and examples for owned records, context handling, passthrough fields, and migration between owned and structural APIs.

@changeset-bot

changeset-bot Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b3f71ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

Changes

Owned record declarations

Layer / File(s) Summary
Owned target and context runtime
packages/stack-encrypt/src/target/*, packages/stack-encrypt/src/plan/build.rs, packages/stack-encrypt/src/scalar.rs
Adds owned encryption and decryption operations, context conversions, collection handling, index support, and typed plan fields.
Owned record derive generation
packages/stack-encrypt-derive/src/*
Adds EncryptTarget and DecryptSource derives with passthrough fields, field accounting, context checks, structural bridges, and wiping-aware reconstruction.
Wiping and failure cleanup
packages/stack-encrypt/tests/collections.rs, packages/stack-encrypt/tests/owned_records.rs, conformance/stack-encrypt/*
Adds cleanup behavior and evidence for late failures, refused contexts, declaration panics, collection conversion, and passthrough moves.
Record integration and conformance evidence
packages/stack-encrypt/tests/owned_equivalence.rs, packages/stack-encrypt/tests/ui/*, packages/stack-encrypt/docs/*, packages/stack-encrypt/CHANGELOG.md, conformance/stack-encrypt/*
Adds integration, UI, documentation, lifecycle enrollment, and equivalence coverage for owned records and their structural bridges.

Estimated code review effort: 5 (Critical) | ~90 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant EncryptTarget
  participant FieldTarget
  participant Pending
  participant DecryptSource
  Caller->>EncryptTarget: encrypt owned record with context
  EncryptTarget->>FieldTarget: split or project fields
  FieldTarget->>Pending: create field operations
  Pending-->>Caller: return encrypted record
  Caller->>DecryptSource: decrypt stored record
  DecryptSource->>FieldTarget: open field groups
  FieldTarget->>Pending: combine results with wiping
  Pending-->>Caller: return plaintext record
Loading

Possibly related issues

Possibly related PRs

Suggested reviewers: freshtonic, calvinbrewer

Merge Risk: Minimal · up to b3f71

The conformance enrollment is supported by the referenced tests and presents no actionable merge-blocking risk.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Issue #1483 is open and directly linked. The PR implements the owned derives, passthrough mapping, context diagnostics, cleanup, batching, executable record tests, and typed/dynamic/generated equivale… Implement configured nested targets and migrate the remaining plan, dynamic/guest, and EQL consumers against the owned declaration surface, or split those requirements into follow-up issues and revise #1483 and the PR description so that th…
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the primary change: owned record declarations for stack-encrypt, built on the EncryptTarget and DecryptSource derives. The raw summary confirms this is the main change. It adds owned d…
Out of Scope Changes check Passed The changed core APIs, derives, context inputs, cleanup behavior, collection bounds, tests, lifecycle evidence, documentation, ADR, changelog, and mutation configuration support the owned-record objec…
Full details: Title check

Explanation

The title names the primary change: owned record declarations for stack-encrypt, built on the EncryptTarget and DecryptSource derives. The raw summary confirms this is the main change. It adds owned derive macros, owned encryption and decryption, passthrough fields, and context inputs, and the title covers that focus. The title is 91 characters, which is a little long but still within a readable single sentence. It uses the conventional commit prefix with the breaking-change marker, which matches the summary's breaking note on structural vector decryption requiring Zeroize.

Full details: Out of Scope Changes check

Explanation

The changed core APIs, derives, context inputs, cleanup behavior, collection bounds, tests, lifecycle evidence, documentation, ADR, changelog, and mutation configuration support the owned-record objectives in #1483 or the stated migration bridge. The structural Vec cleanup change supports the collection cleanup requirement. The PR does not show an unrelated change. Cargo.lock was intentionally excluded and was not inspected.

Full details: Linked Issues check

Explanation

Issue #1483 is open and directly linked. The PR implements the owned derives, passthrough mapping, context diagnostics, cleanup, batching, executable record tests, and typed/dynamic/generated equivalence. The PR does not meet all coding requirements in #1483. Configured nested targets remain deferred. Plan lowering gains FieldsBuilder::target, but dynamic/guest consumers and EQL adapters remain on structural declarations. These gaps match the PR's stated deferrals.

Resolution

Implement configured nested targets and migrate the remaining plan, dynamic/guest, and EQL consumers against the owned declaration surface, or split those requirements into follow-up issues and revise #1483 and the PR description so that this PR does not claim to close #1483. Reassess after the issue scope and implementation agree.

Full details: Docstring Coverage

Explanation

Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 33 functions across 29 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI; Enforced overrides: Organization
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5933b899-0626-4711-a7be-4222e47483f7
Commits

Reviewing files that changed from the base of the PR and between bfa3d58 and b3f71ad.

Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing: 2
  • conformance/stack-encrypt/coverage.md
  • conformance/stack-encrypt/v1/lifecycle-v1.enrollment.json
Files skipped from review as they are similar to previous changes: 1
  • conformance/stack-encrypt/coverage.md

Limit details: You’ve used all 2 included reviews currently available.

@coderdan
coderdan force-pushed the feat/stack-encrypt-owned-records branch from 88737dc to 55ff30f Compare November 2, 2026 12:59
@coderdan

coderdan commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Review

Rebase first. The branch is rebased onto feat/stack-encrypt-owned-collections at 4b42186 (now 55ff30f). This is a rebase, not a content change. The one conflict was tests/ui/owned_collection_elements.stderr. I kept the base's UFCS form (E0277, no $RUST/ snippet) and re-blessed it. The only difference is that Zeroize now prints unqualified. grep -l 'RUST/' tests/ui/*.stderr prints nothing, and none of this PR's .stderr files depend on E0599.

Checked by running code: cargo check -p eql-bindings --features stack-encrypt and the Go guest (default and --features eql) both compile.

Standards

  • packages/stack-encrypt/docs/adr/0011-owned-record-declarations.md: number collision. feat(stack-encrypt)!: add OpaqueBlob whole-value encryption #1503 (feat/stack-encrypt-opaque-blob) also adds 0011-a-whole-value-is-one-storage-blob-sealed-as-one-leaf.md. This one moves to 0012. Nothing in the tree links to it, but the PR body does. CONFIRMED.
  • packages/stack-encrypt/CHANGELOG.md:81: ### Deprecated sits in the middle of [Unreleased] ### Added. The existing "Owned collections" (stack-encrypt refactor: extend owned target conversions to collections #1482) bullets and everything after them now read as deprecations. CONFIRMED.
  • src/target/operations.rs (DecryptField<Vec<P>, Ctx> for Vec<T>): if this impl's all_wiping goes back to all, no test fails. I ran the full stack-encrypt nextest suite. The new collections test reaches only the DecryptInto impl. That is a surviving mutant on a Breaking-labelled line, and mutants.yml --in-diff would flag it. CONFIRMED.
  • src/plan/build.rs FieldsBuilder::target repeats the FieldOpener construction from encrypt_into almost line for line (duplicated code). Judgement call; left.
  • ADR §2, the CHANGELOG Added bullet and the split docs all say the conversion "wipes what it seals and transfers what the caller chose to keep". As written that is not true: see the first Spec bullet. CONFIRMED.

Spec

  • packages/stack-encrypt-derive/src/owned_encrypt.rs (divide, let #source { .., .. } = __source): plaintext fields the record does not name are dropped unwiped. The owned override replaces the default guard over the whole struct. Any field of User that is neither sealed nor passthrough falls through .. and is freed as it is. I reproduced it with a Wide { sealed, unmapped } drop witness: unmapped is never zeroized. The default path and the borrowed path both wipe it. DecryptSource already has to name every field to rebuild the struct, so an exhaustive destructure costs a both-derive record nothing. CONFIRMED.
  • src/target/collections.rs:63-70 (owned Vec column): a panicking element drops the rest of the column unwiped. source.into_iter().map(..).collect() unwinds through vec::IntoIter's drop. Before this PR the whole vector sat in one Protected guard, so this is a regression on the panic path that the base branch otherwise hardens. I reproduced it with a target that panics on element 0: elements 1 and 2 are never wiped. CONFIRMED.
  • owned_decrypt.rs struct_body → Decryption::zip → Pending::zip (pending.rs:300): a field opened before a later field fails is dropped unwiped. Ok((first(responses)?, second(responses)?)) discards first. The column has collect_wiping, but the record has nothing equivalent, so the acceptance point "preserve owned plaintext cleanup" holds for columns and not for records. The structural derive has the same gap. CONFIRMED (by reading; a fix needs a test).
  • Encryption::split (operations.rs): while the first field's description builds, theirs is held unguarded, so a panic there drops the later fields unwiped. Guarding it would put U: Zeroize on passthrough field types. PLAUSIBLE.
  • A source struct that implements Drop (ZeroizeOnDrop) cannot be destructured (E0509), so it cannot be an owned struct record source. The derive docs don't say so. CONFIRMED (language rule).
  • Acceptance bullets:
    • Met: the signature/ownership map (ADR table); explicit passthrough, source-field mapping and decrypt reconstruction; context capabilities with field-spanned diagnostics (owned_record_field_context.stderr points at fixed: Fixed and bound: Bound); executable examples and three-author equivalence (owned_equivalence.rs, which also opens across authors).
    • Partial: cleanup (the three CONFIRMED items above); nested targets (configured targets are deferred); EQL adapters (deferred); decrypt-side field diagnostics (only the encrypt side is pinned in tests/ui).
  • Deferrals:
    • Acceptable: configured nested targets (serde_json::Value is not Zeroize); EQL owned impls (the registry gate in AGENTS.md applies); Option/tuple staying structural-only; no #[deprecated]; passthrough needing Clone (pinned by the panicking Clone).
    • Not acceptable: skipping lifecycle-profile enrollment. The two parent slices enrolled their drop witnesses in lifecycle-profiles.json. This slice's record witnesses (wiped before await, wiped on a refused context) are the same evidence for records and should be enrolled the same way.

Standards: 4 confirmed, 1 judgement call. Worst: the CHANGELOG Deprecated heading reclassifies the #1482 Added bullets.
Spec: 4 confirmed, 1 plausible. Worst: an owned struct record drops the plaintext fields it does not name, unwiped.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

caught missed unviable timeout
14 0 41 0

Every mutant in the changed lines was caught by a test.

@coderdan

coderdan commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Review findings → fixes

Finding Resolution
Rebase onto 4b42186 (the owned_collection_elements.stderr conflict) 55ff30f. Rebase only: I kept the base's UFCS form and re-blessed it. No $RUST/ remains.
ADR 0011 collides with #1503 15ba61d: renamed to 0012-owned-record-declarations.md. Nothing in the tree linked to it by number. The PR body now links the new name.
CHANGELOG Deprecated heading in the middle of Added 15ba61d: moved after the full Added list. The owned-collections bullet no longer reads as a deprecation.
DecryptField<Vec<P>, _>'s all_wiping untested f56c542: added a_late_element_failure_in_a_column_field_wipes_the_recovered_prefix, a structural record with a column field. It fails when all_wiping goes back to all.
Owned struct record drops plaintext fields it does not name a80c98b: the destructure is now exhaustive. An unaccounted field is E0027 at the struct = .. path, pinned in tests/ui/owned_struct_unmapped_field.rs.
Owned column: a panicking declaration drops the rest unwiped f56c542: the elements not yet handed over sit in a guard that wipes them. An element is taken out only once its description and context exist. a_panicking_element_declaration_wipes_the_elements_after_it fails without the guard.
Owned record decrypt drops already-opened fields on a later refusal 1830fbd: added Decryption::zip_wiping / Pending::zip_wiping (a guard holds the left side). The owned derive opens its fields through it. a_field_opened_before_a_later_field_is_refused_is_wiped fails with plain zip. The structural DecryptInto derive is left as it is: its field plaintexts carry no Zeroize bound, and adding one would be a separate breaking change.
Docs claimed "wipes what it seals and transfers what it keeps" 15ba61d: the ADR, the derive's "Owned records" docs and the CHANGELOG now say what is true after the fixes above.
A source with Drop / ZeroizeOnDrop cannot be an owned struct source 15ba61d: documented in the derive docs, the ADR and the CHANGELOG. Not changed: lifting it needs a Default bound on every moved field (mem::take) or unsafe.
Lifecycle-profile enrollment skipped 3c23ed0: added a new owned-records suite with three witnesses, plus two collection witnesses. node scripts/stack-lifecycle-conformance.mjs rust passes.
Encryption::split holds later fields unguarded during an earlier field's build (PLAUSIBLE) Declined. It is reachable only through a panicking field declaration. Guarding the later fields needs U: Zeroize on split, which would extend to passthrough field types. That is a design decision for you, not a mechanical fix.
FieldsBuilder::target duplicates encrypt_into's opener (judgement call) Declined. Left as it is.

c4784c9 is rustfmt only.

Verification on c4784c9:

  • cargo fmt --all --check: clean.
  • clippy -D warnings (both crates, all targets): clean.
  • STACK_CONFORMANCE_REQUIRED=1 nextest (both crates): 1126 passed, 1 skipped.
  • Doctests: 57 passed in stack-encrypt, 12 in stack-encrypt-derive.
  • mise run doc:stack-encrypt: builds.
  • eql-bindings --features stack-encrypt, the Go guest and the Go guest with --features eql: all compile.
  • Conformance vitest (three files): 17 passed.

@coderdan
coderdan marked this pull request as ready for review November 2, 2026 13:16
@coderdan
coderdan requested a review from a team as a code owner November 2, 2026 13:16
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T13:19:34.053478Z c4784c9 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-02T13:22:57.030079Z c4784c9 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderdan

coderdan commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor
Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4784c9329

ℹ️ 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".

Comment on lines +429 to +430
let (mine, theirs) = divide(source);
(self.build)(mine, cipher, cx.clone()).zip((other.build)(theirs, cipher, cx))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard unvisited fields while executing an owned split

When an earlier field's declaration panics during synchronous execution, theirs is dropped without being wiped because its target has not yet received it. Derived owned struct records use this combinator for every field, so a panic in the first field's borrowed encryption() (called inside its default owned guard), for example, leaves later sealed plaintext fields unwiped. The new collection implementation explicitly guards unvisited elements against this same scenario, but the record path does not. Keep the remaining parts guarded until their descriptions take ownership.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 9d3729a, without a new bound on passthrough types.

Encryption::split now takes a fourth argument, wipe: fn(&mut U). The waiting part sits in a guard while the first part's description runs. If that description panics, the guard hands the part to wipe during the unwind. The part leaves the guard only once the first description has returned.

The derive writes each wipe itself. It zeroizes the sealed fields still waiting and leaves the passthrough ones alone. A sealed field's plaintext is already Zeroize, because every EncryptTarget source is, so no bound is added anywhere. A passthrough field is stored in the clear and gets no bound. split is new in this PR, so the extra argument breaks no released caller.

I rejected the other two designs. (a) does not work: guarding the whole struct means the struct cannot be destructured until every field has run, but each owned field build consumes its field. (c) cannot close the gap either: the panic can come from running a declaration, not only from constructing one (for example a project selector), and that has to happen after the field has been moved out.

The witness is a_panicking_field_declaration_wipes_the_sealed_fields_after_it in tests/owned_records.rs. The record has three sealed fields with a passthrough field between them. The test runs twice: once with the first field's selector panicking, which exercises the outer split, and once with the third field's selector panicking, which exercises a nested split. It fails before the fix. It is enrolled in the owned-records lifecycle profile.

source.risky_ref(),
T::ContextInput::omitted(),
)
cipher.run(T::owned_encryption(), self, T::ContextInput::omitted())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard plaintext while constructing the owned description

For the context-omitting conversion, self is now unguarded while T::owned_encryption() is constructed. If that construction panics—for example, while a derived struct target constructs one of its field descriptions—the consumed plaintext is dropped without calling Zeroize. Previously the Protected guard was established before constructing the description, and the supplied-context method still establishes that guard. Retain a guard here until description construction and omitted-context initialization have succeeded, then transfer ownership to the description.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in b40cf93. The context-omitting encrypt_into now holds the plaintext in Protected until T::owned_encryption() and T::ContextInput::omitted() have both returned. Only then does it hand the plaintext to the declaration. encrypt_into_with_context now also builds the declaration under its guard. Before, the declaration was built as a call argument, which was safe only because of argument evaluation order.

The witness is a_declaration_that_panics_while_constructed_wipes_the_plaintext. It uses a target whose owned_encryption() panics while it is constructed, and checks both entry points. It failed before the fix, on the omitted-context path. It is enrolled in the owned-records lifecycle profile.

@cipherstash-agent cipherstash-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review attestation

Reviewed c4784c9 · posted from Claude Code · claude-sonnet-5-5
Sources: code-review (medium) · claude-sonnet-5-5
Skills: code-review, review-cross-check

No findings.

Six candidate findings were verified against c4784c9 and cross-checked against existing feedback: two are already raised in open threads (unguarded plaintext in encrypt_into at conversion.rs:80; unguarded remainder in Encryption::split), and the others were dismissed as documented design or left undecided, so none is posted.


Generated by Claude Code

@coderdan

coderdan commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Codex findings and Mutants gate: fixes

Finding Resolution
Codex P2: Encryption::split drops the waiting fields unwiped if an earlier field's declaration panics Fixed in 9d3729a. I had declined this earlier. split now takes a wipe: fn(&mut U) and keeps the waiting part in a guard until the first description returns. The derive writes each wipe: it zeroizes the sealed fields still waiting and leaves the passthrough ones alone. Sealed sources are already Zeroize, as every EncryptTarget source is, so passthrough field types get no new bound. Witness: a_panicking_field_declaration_wipes_the_sealed_fields_after_it, which covers both the outer split and a nested split. The ADR, CHANGELOG and derive docs are updated.
Codex P2: encrypt_into leaves self unguarded while the declaration is built Fixed in b40cf93. The plaintext stays in Protected until the declaration and the omitted context are built. Witness: a_declaration_that_panics_while_constructed_wipes_the_plaintext, which checks both entry points.
Mutant: MatchTerms EncryptTarget::indexes → vec![] bfa3d58: each_owned_term_target_names_its_index. It covers every owned term target.
Mutant: ContextExtension::into_inner → None bfa3d58: a_context_extension_is_inspected_as_supplied_or_omitted.
Mutant: ExpectedContext::omitted → Default::default() bfa3d58: equivalent, because the body is Self::default(). It is excluded in .cargo/mutants.toml next to the matching From<()> entry. The omitted behaviour is pinned by a_stored_context_opens_as_stored_and_checks_an_expectation.

I confirmed the two killed mutants by applying each one by hand: the new test failed under each. Both panic witnesses failed before their fixes and are enrolled in the owned-records lifecycle suite.

CodeRabbit finished its review with no actionable comments.

Verification on bfa3d58:

  • cargo fmt --all --check and clippy -D warnings on both crates (all targets): clean.
  • STACK_CONFORMANCE_REQUIRED=1 nextest on both crates: 1130 passed, 1 skipped.
  • Doctests: 57 passed in stack-encrypt, 12 in stack-encrypt-derive.
  • mise run doc:stack-encrypt: builds.
  • eql-bindings --features stack-encrypt and the Go guest: both compile.
  • node scripts/stack-lifecycle-conformance.mjs rust: passes.
  • Conformance vitest: 17 passed.

@coderdan coderdan linked an issue Nov 3, 2026 that may be closed by this pull request
6 tasks
@coderdan
coderdan added this pull request to stack #1514 November 3, 2026 11:48
…rget and DecryptSource

Records, derives and plans still spoke the structural `EncryptFrom` /
`DecryptInto` pair over a borrowed plaintext while the scalar API had moved
to `EncryptTarget` / `DecryptSource` with explicit caller-context
capabilities. A user could not write the mixed record the public examples
show (`id` in the clear, `name` and `email` sealed) as an owned conversion
that consumes and wipes the plaintext, and nothing said how the two pairs
relate.

`EncryptTarget` gains `owned_encryption()`, the description over the
plaintext by value, defaulting to guard-and-wipe over the borrowed
`encryption()`, and `indexes()`. `EncryptInto` and the owned column run
it. A new `#[derive(EncryptTarget, DecryptSource)]` composes the field
targets directly: a `struct = User` record projects each field out of a
borrow in its borrowed form and divides the struct between its fields with
the new `Encryption::split` in its owned form, so a `#[stash(passthrough)]`
field is moved into the record (never cloned, never encrypted or
authenticated) and each sealed field is guarded by its own target's owned
description. Caller context is a capability composed through the tree:
`ContextExtension<C>` for a record whose fields carry their own pairs,
`ExpectedContext<T>` (now a `ContextInput`) for a stored context, and
`RequiredContext` of the core contexts for `plaintext` records and the core
leaves, which all implement the owned pair now (terms as `EncryptTarget`
alone). A field whose target cannot take the record's pair is refused at
that field. Both owned derives also emit the structural declarations as a
bridge, so an owned record is a field of a structural record or a typed
plan field; `FieldsBuilder::target` admits an owned-only target such as a
`Ciphertext`. The dynamic lowering is untouched: the equivalence test seals
one record through a hand-written target, the derive and the data plan
against one recording provider and compares descriptors, term bytes and
cross-opening.

Breaking: the structural `Vec<T>: DecryptInto<Vec<P>>` and
`DecryptField<Vec<P>, _>` impls open through the wiping collection path and
so require `P: Zeroize`; before, the elements recovered before a late
element failure were dropped unwiped (the CodeRabbit finding on the owned
collections PR). A drop-witness test fails without the wipe.

ADR-0011 records the signature and ownership map. The structural derives
stay for the plans and the dynamic engine; their removal is #1488.

Closes #1483
Refs #1488
…declaration panics

The owned `Vec<S>` conversion hands each element to its own declaration
by value. A declaration that panicked unwound through `vec::IntoIter`'s
drop, which freed every element not yet handed over without wiping it.
The whole vector's guard did wipe them before this branch moved the
column to per-element owned declarations. The elements not yet handed
over now stay in a guard that wipes them. An element is taken out only
once its description and context exist.

This also pins the structural `Vec<T>: DecryptField<Vec<P>, _>` impl.
Swapping its `all_wiping` back to `all` failed no test, because the
existing test reaches only the `DecryptInto` impl. A structural record
with a column field now opens a recovered prefix and a late refusal
through it.
…a plaintext field unaccounted for

The owned form of a `struct = ..` record replaces the default guard over
the whole plaintext. It destructures the struct with `..`, so any field
that the record neither sealed nor carried through was freed unwiped.
The destructure is now exhaustive. Such a field is a compile error at
the `struct = ..` path ("pattern does not mention field"), just as the
decrypt side already refuses a plaintext it cannot rebuild.
…eld is refused

A `struct = ..` record deriving `DecryptSource` zipped its field openings
with `Pending::zip`. When a later field failed authentication after its
key arrived, `zip` dropped the plaintext of the fields already opened
without wiping it. `Decryption::zip_wiping` (backed by
`Pending::zip_wiping`) holds the left side in a guard, which wipes it
when the right side fails or panics. The owned derive opens its fields
through it.

The structural `DecryptInto` derive is unchanged. Its field plaintexts
carry no `Zeroize` bound today.
… the changelog sections

ADR-0011 is taken by the opaque-blob ADR on a sibling branch, so the
owned-record ADR becomes 0012. Nothing in the tree linked to it by number.

The `[Unreleased]` `Deprecated` heading sat in the middle of `Added`,
which filed the owned-collections bullet under deprecations. It now
follows the full `Added` list. The owned-record bullet, the derive's
"Owned records" docs and the ADR now cover what the fixes on this
branch made true: a `struct` record names every plaintext field, its
plaintext cannot implement `Drop`, and a field opened before a later
refusal is wiped. A new bullet adds `Decryption::zip_wiping`. The
owned-collections bullet now describes the per-element owned form.
…fecycle profile

The owned-column slice enrolled its drop witnesses. The owned-record
witnesses are the same evidence for records and now go in the same place.
A new `owned-records` suite (`tests/owned_records.rs`) adds three witnesses:

- under buffer cleanup, the record wipes its sealed fields before any
  await and on a refused context, and moves its passthrough fields;
- under partial output, a field opened before a later field is refused
  is wiped.

Two new collection witnesses join the existing `collections` suite. One
checks the elements after a panicking declaration. The other checks a
structural record's column field.
… its declaration

The context-omitting EncryptInto::encrypt_into handed self to the declaration only after T::owned_encryption() had been constructed, with nothing guarding it meanwhile, so a declaration that panicked while constructed (a record building its fields' declarations) dropped the plaintext unwiped. It now holds the plaintext in Protected until the declaration and the omitted context exist, as encrypt_into_with_context already did for its context conversion; that variant now also constructs the declaration under the guard rather than as an argument.
… over

Encryption::split ran the first part's description while the other part sat in a local, so a panic in that description (a field declaration's selector, say) dropped the later sealed fields of an owned struct record unwiped. split now takes a wipe for the waiting part and holds it in a guard that calls it during the unwind; the part leaves the guard only once the first description has returned.

The derive writes each wipe to zeroize the sealed fields still waiting and leave the passthrough ones. A sealed field's plaintext is already Zeroize, as every EncryptTarget source is, so no bound is added, and passthrough field types take none.

BREAKING CHANGE: Encryption::split takes a fourth argument, wipe: fn(&mut U).
…ion inspection

Covers two mutants that survived the in-diff gate: MatchTerms' EncryptTarget::indexes replaced with vec![] (each_owned_term_target_names_its_index, which also pins the other owned terms) and ContextExtension::into_inner replaced with None (a_context_extension_is_inspected_as_supplied_or_omitted). The third survivor, ExpectedContext's omitted replaced with Default::default(), is equivalent: the body is Self::default(). It is excluded in .cargo/mutants.toml beside the matching From<()> entry, with the test that pins the omitted behaviour named.
@coderdan
coderdan force-pushed the feat/stack-encrypt-owned-records branch from bfa3d58 to b3f71ad Compare November 3, 2026 12:04
@coderdan

coderdan commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the rebased #1501 (57a9b210c). This was a force push. One conflict came up: the first commit extends two coverage.md rows in which #1491 had renamed v1/declarations.json to v1/record-declarations-v1.cases.json. I kept this PR's text with the new path. Compared line by line, the PR diff otherwise matches the one before the rebase: 52 files, +4097/−84. Review feedback is not addressed in this push.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stack-encrypt refactor: migrate record derives and plans to owned declarations

2 participants