Skip to content

feat(stack-encrypt)!: move the stack crates, the guests and the Go module onto vitaminc ADR 0001 - #1213

Open
coderdan wants to merge 10 commits into
feat/stack-encrypt-json-operatorsfrom
feat/stack-encrypt-vitaminc-adr0001
Open

coderdan wants to merge 10 commits into
feat/stack-encrypt-json-operatorsfrom
feat/stack-encrypt-vitaminc-adr0001

Conversation

@coderdan

@coderdan coderdan commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Stacked on #1209. This PR moves the stack repo's root Cargo workspace, the fuzz crate, both Go guests, the Go module and eql-bindings' stack-encrypt feature onto vitaminc ADR 0001: a value describes itself once (Encrypt, which vitaminc called Plaintext until cipherstash/vitaminc#456), reads back through a Decrypt sink, crosses an FFI boundary as a transport frame, and is sealed one leaf at a time by a bytes-only Cipher. It is PR 1 of 3; the next is #1140.

  • A temporary vitaminc pin, in every workspace that compiles stack-encrypt. Five blocks: the root Cargo.toml, the encrypt guest, the credential guest, packages/stack-encrypt/fuzz and the EQL workspace (packages/eql, whose tests/encryption is a member). Each carries a [patch.crates-io] block pointing every vitaminc crate they use at one commit, 2413bd0e, of feat(ore)!: compare order terms through the scheme and column, not through Ord vitaminc#464: vitaminc main (ADR 0001, merged from cipherstash/vitaminc PRs 439 to 445) plus the order-term comparison ore.rs builds against. It is pinned by rev, because the branch it first named was deleted when its stack merged; chore(deps)(deps-dev): bump the dev-dependencies group across 1 directory with 6 updates #464 must be merged with a merge commit, or 2413bd0e tagged, or it becomes unreachable the same way. vitaminc-aead-value is replaced by vitaminc-aead-frame. The blocks go together when vitaminc releases ADR 0001. The lockfile also moves toml_parser, which the git dependency needs to resolve.
  • The cipher is the bytes-only Cipher, batched as before. PendingStackCipherText::plan walks a value into vitaminc's pending tree with no I/O. The target layer's Pending still fetches every data key of an assembly in one generate_keys call (RFC 0002: the fields of a record and the rows of a column merge into one request), and then MintedKeys seals each leaf under the next generated key. Opening binds each retrieved key to its leaf and BoundKeys opens it; vitaminc derives every leaf's AAD and checks the empty markers and container rules on both sides, so there is one definition of them. vitaminc's AsyncCipher is not used: its batch is one document, and a ZeroKMS request needs the descriptor of the root context, which a leaf's derived AAD does not carry.
  • Every leaf records its type. Leaves are vitaminc's tagged wire version 2, so a Rust u32 field and a data plan's uint32 field seal the same leaf and each opens the other's, and a reader of another kind is refused instead of reading changed plaintext (an integer reader still takes any integer width the value fits). That closes stack-encrypt: a Rust String field reads a data-plan leaf as "\nalice" — the Rust chain and a data plan seal different leaf bytes #1118. The record fixture is regenerated; its terms are unchanged, and the tests that pinned the old mismatch now pin the shared leaf.
  • None is a null leaf; a passthrough is a frame. StackCipherText is Node<SealedValue>. CipherText::None, transcode::Visitor::absent, StackDecipher and BoxedPassthrough are gone, and Visitor::passthrough takes a PassthroughFrame.
  • dynamic::FfiValue and ValueKind move into stack-encrypt, unchanged in shape, with Encrypt, a Decrypt sink and to_frame/from_frame. This is the minimum that keeps today's behaviour; the rework of dynamic::Value and Scalar is stack-encrypt: delete dynamic::Value and Scalar, derive terms through vitaminc, add block ORE, drop Bool order terms #1140. vcffi now sends an 8- or 16-bit integer at its own width, and the sink reads it as the 32-bit integer of its sign, so a generated int8 field seals as it did.
  • stack-guest-abi hands the host HostValues (outputs and the last error), written as transport frames. Text is held as Zeroizing<String>, so the auth guest's access token is wiped as it was under FfiValue.
  • eql-bindings' stack-encrypt feature compiles stack-encrypt by path, so it moves with it: encryption::targets and the generated v3::targets take FfiValue/ValueKind from stack_encrypt::dynamic (the generator emits the new import), and the Ciphertext target takes Encrypt and opens into Decrypt + Send + 'static. No other EQL code names a vitaminc API. Its CHANGELOG has an entry.
  • The guests and the Go module. The encrypt guest reads inputs as frames, writes outputs through vitaminc's FrameWriter into a buffer still sized before the first byte, and carries ciphertext trees in vitaminc_aead_frame's codec. The credential guest's outputs are HostValues. The Go module moves vcffi and vcvalue to the same vitaminc commit (2413bd0e4bf7) and drops the absent leaf kind. No wasm is checked in; CI builds the guests as before.

What this PR does not do

Tests

  • stack-encrypt: the cipher, record and target tests run on the new model; new unit tests for FfiValue's frame round trip, its integer widths, the narrower integers and the refusals; HostValue round trip and refusals in stack-guest-abi. The trybuild snapshots are re-recorded under Rust 1.94.1; struct_field_must_open now uses a local borrowed type, so its diagnostic points at no vitaminc source path.
  • The tests that pinned Element, the boxed passthrough and the absent marker are removed with them.
  • Go: unchanged apart from the retired leaf kind; the fixture and hermetic round trips pass against the rebuilt deterministic guest.

Test plan

  • mise x --env test -- cargo nextest run --workspace --all-features: 1120 passed.
  • cargo mutants --in-diff over stack-auth and stack-encrypt: the nine survivors of the first run are pinned or removed in a later commit, and a rerun over those functions catches every viable mutant.
  • mise run test:doc, mise run doc, mise run wasm:wasi-check, mise run wasm:no-http-test.
  • cargo clippy --workspace --all-targets with and without --all-features, -D warnings; cargo fmt --check in the root, the fuzz crate and both guests.
  • The wasm:guest:test and wasm:auth-guest:test steps without the eql feature (clippy native and wasm32, rustdoc, nextest).
  • wasm:guest:build, wasm:guest:build:deterministic, wasm:auth-guest:build.
  • The four stack-encrypt fuzz targets replay their corpus (-runs=0, nightly).
  • mise run wasm:guest:test in full, the eql steps included.
  • wasm:guest:build:eql and wasm:guest:build:eql:deterministic.
  • scripts/go-binding-test.sh (gofmt, vet, go test ./... with every guest built, TestTextEqIsRefusedByTheBuildWithoutEQLTypes included) and golangci-lint.
  • EQL, as test-eql.yml's Rust job runs it: test:crates, test:encryption, check:encryption:wasi, types:check, and test:encryption:postgres against a local PostgreSQL 17.5. node scripts/lint-no-eql-registry-pins.mjs and the eql-suite-ci, eql-workflow-filters and crates-ci script tests.
  • cargo publish -p eql-bindings --dry-run --all-features: not run; the packaged crate resolves stack-encrypt and vitaminc from crates.io, where neither this stack-encrypt nor ADR 0001 is released, so it can only warn until both are.
  • The 386 Go run (Linux only), macOS and Windows Go jobs: CI.
  • CI.

Closes #1118
Part of #1140

On top of next

Rebased onto next, which carries the EQL v4 types. The eql-bindings scalar plaintexts (Int32, Int64, Bool, Float32, Float64) are ported from vitaminc 0.5's visitor traits to Encrypt and FromScalar, and still refuse a value of another width. v4_rows.json is regenerated, because ADR 0001 leaves are a new wire version.

@changeset-bot

changeset-bot Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: cbfbddd

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 Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 94cd19e6-7b43-4e1e-8d30-b54916d914b6
📥 Commits

Reviewing files that changed from the base of the PR and between a63d2e7 and cbfbddd.

⛔ Files ignored due to path filters: 6
  • Cargo.lock is excluded by !**/*.lock
  • languages/golang/auth/guest/Cargo.lock is excluded by !**/*.lock
  • languages/golang/encrypt/guest/Cargo.lock is excluded by !**/*.lock
  • languages/golang/go.sum is excluded by !**/*.sum
  • packages/eql/Cargo.lock is excluded by !**/*.lock
  • packages/stack-encrypt/fuzz/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 82
  • Cargo.toml
  • languages/golang/auth/guest/Cargo.toml
  • languages/golang/auth/guest/src/lib.rs
  • languages/golang/auth/guest/src/ops.rs
  • languages/golang/auth/guest/tests/leak.rs
  • languages/golang/encrypt/ciphertext.go
  • languages/golang/encrypt/guest/Cargo.toml
  • languages/golang/encrypt/guest/src/abi.rs
  • languages/golang/encrypt/guest/src/config.rs
  • languages/golang/encrypt/guest/src/lib.rs
  • languages/golang/encrypt/guest/src/ops.rs
  • languages/golang/encrypt/guest/src/options.rs
  • languages/golang/encrypt/guest/src/status.rs
  • languages/golang/encrypt/guest/src/targets.rs
  • languages/golang/encrypt/guest/tests/leak.rs
  • languages/golang/encrypt/guest/tests/native_ops.rs
  • languages/golang/encrypt/unit_test.go
  • languages/golang/go.mod
  • languages/golang/stashgen/emit.go
  • packages/eql/Cargo.toml
  • packages/eql/crates/eql-bindings/CHANGELOG.md
  • packages/eql/crates/eql-bindings/Cargo.toml
  • packages/eql/crates/eql-bindings/README.md
  • packages/eql/crates/eql-bindings/src/encryption.rs
  • packages/eql/crates/eql-bindings/src/encryption/plaintext.rs
  • packages/eql/crates/eql-bindings/src/encryption/targets.rs
  • packages/eql/crates/eql-bindings/src/v4/targets.rs
  • packages/eql/crates/eql-codegen/src/targets.rs
  • packages/eql/tests/encryption/Cargo.toml
  • packages/eql/tests/encryption/fixtures/v4_rows.json
  • packages/eql/tests/encryption/tests/targets.rs
  • packages/eql/tests/encryption/tests/text_eq.rs
  • packages/eql/tests/encryption/tests/v4_types.rs
  • packages/stack-encrypt-derive/src/lib.rs
  • packages/stack-encrypt/CHANGELOG.md
  • packages/stack-encrypt/CONTEXT.md
  • packages/stack-encrypt/Cargo.toml
  • packages/stack-encrypt/examples/mixed_user.rs
  • packages/stack-encrypt/fuzz/Cargo.toml
  • packages/stack-encrypt/fuzz/fuzz_targets/check_record.rs
  • packages/stack-encrypt/src/cipher.rs
  • packages/stack-encrypt/src/dynamic/context.rs
  • packages/stack-encrypt/src/dynamic/ffi_value.rs
  • packages/stack-encrypt/src/dynamic/kind.rs
  • packages/stack-encrypt/src/dynamic/mod.rs
  • packages/stack-encrypt/src/dynamic/record.rs
  • packages/stack-encrypt/src/dynamic/target.rs
  • packages/stack-encrypt/src/dynamic/term.rs
  • packages/stack-encrypt/src/dynamic/value.rs
  • packages/stack-encrypt/src/lib.rs
  • packages/stack-encrypt/src/plan/build.rs
  • packages/stack-encrypt/src/plan/chain.rs
  • packages/stack-encrypt/src/plan/mod.rs
  • packages/stack-encrypt/src/plan/value.rs
  • packages/stack-encrypt/src/target/core.rs
  • packages/stack-encrypt/src/target/index.rs
  • packages/stack-encrypt/src/target/mod.rs
  • packages/stack-encrypt/src/target/operations.rs
  • packages/stack-encrypt/src/target/transcode.rs
  • packages/stack-encrypt/tests/common/mod.rs
  • packages/stack-encrypt/tests/fixtures/README.md
  • packages/stack-encrypt/tests/fixtures/record_lowering.json
  • packages/stack-encrypt/tests/index.rs
  • packages/stack-encrypt/tests/plan_grammar.rs
  • packages/stack-encrypt/tests/record_lowering.rs
  • packages/stack-encrypt/tests/roundtrip.rs
  • packages/stack-encrypt/tests/source_mode.rs
  • packages/stack-encrypt/tests/transcode.rs
  • packages/stack-encrypt/tests/ui/decrypt_field_not_decryptable.stderr
  • packages/stack-encrypt/tests/ui/drop_record.stderr
  • packages/stack-encrypt/tests/ui/owned_fan_out_needs_clone.rs
  • packages/stack-encrypt/tests/ui/pass/owned_without_clone.rs
  • packages/stack-encrypt/tests/ui/pass/query_only_and_borrowed.rs
  • packages/stack-encrypt/tests/ui/plaintext_needs_encrypt.stderr
  • packages/stack-encrypt/tests/ui/struct_field_must_open.rs
  • packages/stack-encrypt/tests/ui/struct_field_must_open.stderr
  • packages/stack-guest-abi/Cargo.toml
  • packages/stack-guest-abi/src/abi.rs
  • packages/stack-guest-abi/src/call.rs
  • packages/stack-guest-abi/src/last_error.rs
  • packages/stack-guest-abi/src/lib.rs
  • packages/stack-guest-abi/src/value.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

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

caught missed unviable timeout
192 0 121 0

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

…scribed once and every leaf records its type

Pins the root workspace's vitaminc crates to the branch that implements
vitaminc ADR 0001 (cipherstash/vitaminc #439 to #445) with a temporary
`[patch.crates-io]`, and moves stack-encrypt and the guest ABI onto it.

- A value describes itself with vitaminc's `Plaintext` and reads back
  through `Decrypt`. `PendingStackCipherText::plan` walks it into
  vitaminc's pending tree with no I/O; the target layer's `Pending`
  still fetches every key of an assembly in one batched call, and the
  leaves are then sealed and opened through the bytes-only `Cipher`:
  `MintedKeys` seals each leaf under the next generated key, `BoundKeys`
  opens a leaf under the key bound to it, and vitaminc derives every
  leaf's AAD and checks the markers and container rules on both sides.
- Every leaf is tagged (wire version 2), so a Rust `u32` field and a
  data plan's `uint32` field seal the same leaf and each opens the
  other's. The record fixture is regenerated; its terms are unchanged.
- `None` is a null leaf, a passthrough is a `PassthroughFrame`, and
  `StackCipherText` is `Node<SealedValue>`.
- `dynamic::FfiValue` and `ValueKind` move into this crate, unchanged in
  shape, with `Plaintext` and a `Decrypt` sink, and cross a binding's
  boundary as transport frames. The deeper rework of `dynamic::Value`
  and `Scalar` is #1140.
- stack-guest-abi's outputs and last error are a `HostValue` written as
  a transport frame.

Closes #1118
…with no absent marker to fuzz

The fuzz crate is its own workspace and compiles stack-encrypt by path, so
it carries the same temporary vitaminc pin. check_record's model loses the
None node, which ADR 0001 retires, and builds its passthroughs as frames.
…guests

Both WASI guests build against the same temporary vitaminc pin as the
root workspace, and every value crosses as a transport frame.

- The encrypt guest decodes inputs with `FfiValue::from_frame`, writes
  outputs through vitaminc's `FrameWriter` into a buffer still sized
  before the first byte, and reads and writes ciphertext trees with
  `vitaminc_aead_frame`'s codec, a passthrough as its frame.
- The credential guest's outputs are `HostValue`s.
- The Go module moves vcffi and vcvalue to the same vitaminc commit,
  whose container tags and leaf kinds match the frame. The absent leaf
  kind is gone from the binding's leaf set. vcffi sends an 8- or 16-bit
  integer at its own width, which the engine reads as the 32-bit integer
  of its sign, so a generated field seals as it did.
…and drop the unpinnable capacity hint

cargo-mutants found the unused half of MintedKeys and BoundKeys, and the
UINT8 arm of FfiValue's sink, untested: no Rust source walks a u8 as an
int, so the test reads a host's frame of one. The sink's capacity hint
changed nothing a test can see, so it is gone.
… 0001 so the EQL jobs and the EQL guest stay green

eql-bindings' `stack-encrypt` feature and tests/encryption compile
stack-encrypt by path, so they move with it.

- The EQL workspace carries the same temporary vitaminc pin as the root,
  the fuzz crate and both guests: every workspace that compiles
  stack-encrypt now resolves one vitaminc.
- `encryption::targets` and the generated `v3::targets` take `FfiValue`
  and `ValueKind` from `stack_encrypt::dynamic` (the feature turns on
  stack-encrypt's `dynamic`); the generator emits the new import.
  `vitaminc-aead-value` is gone from both manifests.
- The `Ciphertext` target takes `Plaintext` in place of `Encrypt` and
  `Decrypt + Send + 'static` in place of `Decrypt<'static>`.
- The encrypt guest's lockfile drops `vitaminc-aead-value`, which only
  its `eql` build still pulled in.

No other EQL code names a vitaminc API.

Rebased onto next, which carries the EQL v4 types:

- `encryption::plaintext`'s `Int32`, `Int64`, `Bool`, `Float32` and
  `Float64` sealed vitaminc 0.5's tagged leaves through `Encrypt` and
  `DecipherVisitor`, which ADR 0001 removes. They now describe themselves
  as the primitive they wrap (`Plaintext`), so they seal the leaf a data
  plan's field of that type seals, and read back through `FromScalar`
  only a scalar of their own kind and width, as before: vitaminc's
  primitive targets would convert any integer that fits and widen an
  f32. `vitaminc-aead-value` and `vitaminc-protected` leave eql-bindings;
  `vitaminc-aead` stays for the scalar kinds.
- `tests/encryption/tests/v4_types.rs` takes `FfiValue` and `ValueKind`
  from `stack_encrypt::dynamic`.
- `v4_rows.json` is regenerated by the Go test over the deterministic EQL
  guest: ADR 0001 leaves are wire version 2, so the committed ciphertexts
  no longer opened.
…s Encrypt rename

The vitaminc crates came from `feat/aead-remove-legacy-traits` by branch
name. That stack has merged into vitaminc main and the branch is gone, so
a lockfile refresh could no longer resolve it. Every workspace that
compiles stack-encrypt (the root, both Go guests, packages/eql and the
fuzz crate) now pins one commit of cipherstash/vitaminc#464, vitaminc main
plus the order-term comparison, which is the commit ore.rs's vitaminc
implementation (cipherstash/ore.rs#115) builds against. Pinned by rev, not
branch, so a merge cannot pull it out from under the build.

Between the two commits vitaminc renamed the source trait `Plaintext` to
`Encrypt`, to pair with `Decrypt`. stack-encrypt re-exports it under the
new name, and every bound, impl, import and doc link in the stack crates,
the guests, stack-guest-abi and eql-bindings follows. `TargetError::
Plaintext`, `Supports`'s `In::Plaintext` and eql-bindings' own
`targets::Plaintext` are different things and keep their names. The
trybuild expectations follow the rename.

The other breaking changes since (Timestamp as plaintext, Copy
normalizers, vitaminc-ore's column handle and compare) reach no code at
this layer. v4_rows.json is unchanged: the leaves are the same format.
@coderdan
coderdan force-pushed the feat/stack-encrypt-vitaminc-adr0001 branch from f21a238 to 6e46b87 Compare October 16, 2026 13:11
@coderdan
coderdan marked this pull request as ready for review October 16, 2026 13:11
@coderdan
coderdan requested a review from a team as a code owner October 16, 2026 13:11
@coderdan
coderdan requested a balanced review from Copilot October 16, 2026 13:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 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-10-16T13:15:05.656202Z 6e46b87 Draft marked ready
🔒 Security Review ✅ Completed 2026-10-16T13:20:06.065176Z 6e46b87 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

Copy link
Copy Markdown
Contributor Author

Restacked onto next, with two changes beyond resolving conflicts:

  • vitaminc is now pinned to feat(ore)!: compare order terms through the scheme and column, not through Ord vitaminc#464 by commit (2413bd0), in all five workspaces. That's vitaminc main plus the order-term comparison ore.rs builds against. The old pin named feat/aead-remove-legacy-traits, a branch that no longer exists. vitaminc renamed the source trait Plaintext to Encrypt in between, so stack-encrypt re-exports it under the new name and every use follows (b08eda0d9).
  • The EQL v4 scalar plaintexts from next (Int32, Int64, Bool, Float32, Float64) are ported from vitaminc 0.5's visitor traits to Encrypt + FromScalar. They still refuse a leaf of another width. v4_rows.json is regenerated because ADR 0001 leaves are a new wire version (6e46b87's parent).

All checks were run locally at this tip: workspace tests, clippy, rustdoc, the EQL workspace (not the SQLx suite), all guest builds and the Go module.

@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

Source: automated /code-review medium plus cross-check of the findings, run by Claude Code on head 6e46b87.

Critical or blocking issues: none found.

Scope read:

  • The cipher rewrite in packages/stack-encrypt/src/cipher.rs.
  • The [patch.crates-io] pins.
  • dynamic/ffi_value.rs.
  • stack-guest-abi/src/value.rs.

Not read in full: the lockfiles, the trybuild snapshots, the Go module, the generated EQL targets, and the rest of the plan/ and target/ rewrites. Treat those as not covered by this attestation.

What I checked:

  • The old per-node AAD rules now come from vitaminc's plan and seal_all.
    • Those rules are element and entry AAD, empty-marker handling, duplicate keys, and the all-passthrough refusal.
    • MintedKeys::open and BoundKeys::seal both return Unspecified, so a minted key cannot open a leaf and a retrieved key cannot seal one.
    • I did not verify vitaminc's own enforcement of the empty markers and duplicate keys. This PR depends on it.
  • Batching is unchanged: one generate_keys call per pending, with keys consumed in walk order. An exhausted iterator is refused.
  • FfiValue::Debug redacts text and bytes. Zeroize is kept, and Passthrough is skipped as before.

Non-blocking (inline):

  1. 🟡 The pin comment, the PR description, and the CHANGELOG disagree about which vitaminc commit is pinned. The description names a branch and #439 to #445, but the code pins a rev from #464.
  2. 🟡 ValueMap::entry does an O(n²) duplicate-key scan on host-supplied frames.

Still open from the PR's own test plan: the CI run, the Go 386, macOS and Windows jobs, and cargo publish --dry-run.


Generated by Claude Code

Self: 's;

fn entry(&mut self, key: &str) -> Result<ValueSink<Entry<'_>>, DecodeError> {
if self.entries.iter().any(|(k, _)| k == key) {

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.

🟡 (non-blocking) The duplicate-key check scans self.entries on every entry, so decoding an object with n keys from a host frame is O(n²). from_frame takes host-supplied bytes. A frame with many thousands of tiny keys costs quadratic CPU inside the guest. A HashSet<String> kept beside entries would make it linear. Fine to defer if the frame size is capped upstream.

Source: packages/stack-encrypt/src/dynamic/ffi_value.rs ValueMap::entry


Generated by Claude Code

Comment thread Cargo.toml
vitaminc = { git = "https://github.com/cipherstash/vitaminc", rev = "2413bd0e4bf7eb8932a31101c635e2373c1096c6" }
vitaminc-aead = { git = "https://github.com/cipherstash/vitaminc", rev = "2413bd0e4bf7eb8932a31101c635e2373c1096c6" }
vitaminc-aead-frame = { git = "https://github.com/cipherstash/vitaminc", rev = "2413bd0e4bf7eb8932a31101c635e2373c1096c6" }
vitaminc-encrypt = { git = "https://github.com/cipherstash/vitaminc", rev = "2413bd0e4bf7eb8932a31101c635e2373c1096c6" }

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.

🟡 (non-blocking) The PR description is out of date. It says the pins point at branch feat/aead-remove-legacy-traits (vitaminc #439 to #445). The code pins rev = 2413bd0… from vitaminc #464, which the comment above says includes the ore.rs order-term comparison. The rev pin is the better choice, since a branch is deleted when its stack merges. Please correct the description so a reviewer checks the right vitaminc commit. Also confirm that rev is reachable once #464 merges or is squashed, because a [patch] rev that disappears breaks every workspace that carries this block.


Generated by Claude Code

Copilot AI 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.

🟡 Changes recommended

Credential text and oversized output buffers can leave sensitive copies unwiped in freed memory.

2 open findings
What changed in this PR

Migrates Stack Encrypt, its Go guests, and EQL integration to vitaminc ADR 0001, unifying typed Rust and data-plan leaf encodings.

Changes:

  • Adopts tagged leaves, sink-based decryption, and transport frames while preserving batched key requests.
  • Updates runtime values, guest serialization, and dependency pins.
  • Refreshes tests, fixtures, examples, and breaking-change documentation.
File Description
packages/​stack-guest-abi/​src/​lib.rs Exposes host values.
packages/​stack-guest-abi/​src/​last_error.rs Encodes errors as host-value frames.
packages/​stack-guest-abi/​src/​call.rs Updates error-decoding tests.
packages/​stack-guest-abi/​src/​abi.rs Updates error-format documentation.
packages/​stack-guest-abi/​Cargo.toml Adds frame-model dependencies.
packages/​stack-encrypt/​tests/​ui/​struct_field_must_open.stderr Refreshes trait diagnostics.
packages/​stack-encrypt/​tests/​ui/​struct_field_must_open.rs Uses a local non-decryptable type.
packages/​stack-encrypt/​tests/​ui/​plaintext_needs_encrypt.stderr Refreshes encryption diagnostics.
packages/​stack-encrypt/​tests/​ui/​pass/​query_only_and_borrowed.rs Updates a trait comment.
packages/​stack-encrypt/​tests/​ui/​pass/​owned_without_clone.rs Reorders imports.
packages/​stack-encrypt/​tests/​ui/​owned_fan_out_needs_clone.rs Reorders imports.
packages/​stack-encrypt/​tests/​ui/​drop_record.stderr Updates ciphertext diagnostic types.
packages/​stack-encrypt/​tests/​ui/​decrypt_field_not_decryptable.stderr Updates decryption diagnostics.
packages/​stack-encrypt/​tests/​transcode.rs Tests frame-based passthroughs.
packages/​stack-encrypt/​tests/​source_mode.rs Adapts plaintext implementations.
packages/​stack-encrypt/​tests/​roundtrip.rs Updates round trips and marker coverage.
packages/​stack-encrypt/​tests/​record_lowering.rs Adapts shared-record serialization.
packages/​stack-encrypt/​tests/​plan_grammar.rs Updates decryption bounds.
packages/​stack-encrypt/​tests/​index.rs Updates decryption bounds.
packages/​stack-encrypt/​tests/​fixtures/​record_lowering.json Regenerates tagged ciphertext leaves.
packages/​stack-encrypt/​tests/​fixtures/​README.md Documents shared leaf encoding.
packages/​stack-encrypt/​tests/​common/​mod.rs Adds passthrough-frame helpers.
packages/​stack-encrypt/​src/​target/​transcode.rs Replaces absent markers and boxed passthroughs.
packages/​stack-encrypt/​src/​target/​operations.rs Updates opening contracts.
packages/​stack-encrypt/​src/​target/​mod.rs Exposes internal key binding.
packages/​stack-encrypt/​src/​target/​index.rs Updates indexed decryption bounds.
packages/​stack-encrypt/​src/​target/​core.rs Adopts planned sealing and bound-key opening.
packages/​stack-encrypt/​src/​plan/​value.rs Updates plan opening bounds.
packages/​stack-encrypt/​src/​plan/​mod.rs Documents unified leaves.
packages/​stack-encrypt/​src/​plan/​chain.rs Updates field trait bounds.
packages/​stack-encrypt/​src/​plan/​build.rs Updates field opening contracts.
packages/​stack-encrypt/​src/​lib.rs Revises public exports and documentation.
packages/​stack-encrypt/​src/​dynamic/​value.rs Implements plaintext visits and decryption sinks.
packages/​stack-encrypt/​src/​dynamic/​term.rs Uses the local runtime value.
packages/​stack-encrypt/​src/​dynamic/​target.rs Uses local value types.
packages/​stack-encrypt/​src/​dynamic/​mod.rs Exports the runtime value model.
packages/​stack-encrypt/​src/​dynamic/​kind.rs Uses local kind definitions.
packages/​stack-encrypt/​src/​dynamic/​context.rs Updates value imports and frame references.
packages/​stack-encrypt/​fuzz/​fuzz_targets/​check_record.rs Adapts fuzzed ciphertext shapes.
packages/​stack-encrypt/​fuzz/​Cargo.toml Pins the vitaminc revision.
packages/​stack-encrypt/​examples/​mixed_user.rs Demonstrates derives and passthrough frames.
packages/​stack-encrypt/​CONTEXT.md Updates architectural terminology.
packages/​stack-encrypt/​CHANGELOG.md Records breaking API and wire changes.
packages/​stack-encrypt/​Cargo.toml Replaces the value-codec dependency.
packages/​stack-encrypt-derive/​src/​lib.rs Reorders documentation-example imports.
packages/​eql/​tests/​encryption/​tests/​v4_types.rs Uses Stack Encrypt runtime types.
packages/​eql/​tests/​encryption/​tests/​text_eq.rs Adapts custom plaintext implementations.
packages/​eql/​tests/​encryption/​tests/​targets.rs Updates runtime-type imports.
packages/​eql/​tests/​encryption/​fixtures/​v4_rows.json Regenerates encrypted row fixtures.
packages/​eql/​tests/​encryption/​Cargo.toml Enables dynamic values and sink APIs.
packages/​eql/​crates/​eql-codegen/​src/​targets.rs Generates updated runtime imports.
packages/​eql/​crates/​eql-bindings/​src/​v4/​targets.rs Updates generated target imports.
packages/​eql/​crates/​eql-bindings/​src/​encryption/​targets.rs Uses Stack Encrypt value types.
packages/​eql/​crates/​eql-bindings/​src/​encryption.rs Updates ciphertext opening bounds.
packages/​eql/​crates/​eql-bindings/​README.md Updates the target example.
packages/​eql/​crates/​eql-bindings/​CHANGELOG.md Records integration changes.
packages/​eql/​crates/​eql-bindings/​Cargo.toml Revises optional encryption dependencies.
packages/​eql/​Cargo.toml Pins compatible vitaminc crates.
packages/​eql/​Cargo.lock Locks migrated dependencies.
languages/​golang/​stashgen/​emit.go Clarifies integer-width handling in comments.
languages/​golang/​go.sum Adds updated codec checksums.
languages/​golang/​go.mod Updates Go codec dependencies.
languages/​golang/​encrypt/​unit_test.go Removes retired absent-leaf coverage.
languages/​golang/​encrypt/​guest/​tests/​native_ops.rs Adapts native guest operation tests.
languages/​golang/​encrypt/​guest/​tests/​leak.rs Updates frame-based leak tests.
languages/​golang/​encrypt/​guest/​src/​targets.rs Uses Stack Encrypt runtime types.
languages/​golang/​encrypt/​guest/​src/​status.rs Updates test imports.
languages/​golang/​encrypt/​guest/​src/​options.rs Updates runtime-value imports.
languages/​golang/​encrypt/​guest/​src/​ops.rs Migrates value and ciphertext serialization.
languages/​golang/​encrypt/​guest/​src/​lib.rs Documents transport formats.
languages/​golang/​encrypt/​guest/​src/​config.rs Updates runtime-value imports.
languages/​golang/​encrypt/​guest/​src/​abi.rs Decodes inputs as frames.
languages/​golang/​encrypt/​guest/​Cargo.toml Pins vitaminc and adds the frame codec.
languages/​golang/​encrypt/​ciphertext.go Documents sealed-null behavior.
languages/​golang/​auth/​guest/​tests/​leak.rs Adapts credential leak tests.
languages/​golang/​auth/​guest/​src/​ops.rs Serializes credential outputs through HostValue.
languages/​golang/​auth/​guest/​src/​lib.rs Documents host-value frames.
languages/​golang/​auth/​guest/​Cargo.toml Updates credential-guest dependencies.
languages/​golang/​auth/​guest/​Cargo.lock Locks migrated dependencies.
Cargo.toml Pins the shared vitaminc revision.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +79 to +80
fn string(value: impl Into<String>) -> HostValue {
HostValue::text(value)

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 cc5ca95. HostValue::Text now holds a Zeroizing<String> on every path that builds one: the constructor, JSON and a decoded frame. A test pins it. 1574655 also closes the other half: HostValue::encode now copies the frame into a buffer with no spare capacity, so registering it doesn't reallocate and leave an unwiped copy of the token behind.

Comment on lines +392 to +394
(&value)
.visit(FrameWriter::new(&mut out))
.map_err(|_| STATUS_ENCODING)?;

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 1574655. encode_value writes into a Zeroizing staging buffer and copies the frame into an exactly sized output. The auth guest's HostValue::encode does the same. Both guests have a test that every encoded output's capacity equals its length, across every value shape the guest writes.

…dule to the same vitaminc

Review follow-ups for the move onto vitaminc ADR 0001.

`HostValue::Text` held a plain `String`. Before this PR the auth guest
built its output from `FfiValue::String`, whose bytes live in a
`Protected` buffer, so the access token it returns was wiped when dropped;
after it, every `token` call left a bearer token in freed guest memory.
`Text` now holds `Zeroizing<String>` on every path that builds one (the
constructor, JSON, a decoded frame), and a test pins it.

The Go module still named vcffi and vcvalue at 2e895cb9caed, the head of
the deleted `feat/aead-remove-legacy-traits` branch, which builds only
while the Go proxy has it cached. It now names the commit the five Rust
workspaces pin, 2413bd0e4bf7 (cipherstash/vitaminc#464); the Go code under
bindings/go is the same at both.

Docs that still called vitaminc's source trait `Plaintext` (CONTEXT.md,
the mixed_user example, two test headers and two manifests) say
`Encrypt`. The CHANGELOG no longer says a reader of any other type is
refused: an integer reader takes any integer width the value fits, and an
`f64` reader an `f32`, which change no value.
…registering one copies nothing unwiped

Copilot review. The registry wipes the buffer it is handed, but
`register` turns a `Vec` into a boxed slice, and a `Vec` with more
capacity than length is reallocated on the way, leaving the old
allocation unwiped. The encrypt guest reserved the exact encoded length
before writing a frame, but vitaminc's frame writer reserves an upper
bound per leaf (14 bytes for a six-byte string's 11), so a decrypt's
plaintext output could still grow past it and leave a copy behind; the
auth guest's `HostValue::encode` returned the writer's buffer as it was.

Both now write into a buffer that is wiped when dropped and copy the
frame into one whose capacity is its length. A test in each pins it over
every value shape the guest writes.
@coderdan

Copy link
Copy Markdown
Contributor Author

An independent review of this PR found no blocking issues. Fixed in cc5ca95:

  • Access token copy (security): the auth guest left an unwiped copy of the access token in guest memory once HostValue::Text became a plain String. It now wipes on drop.
  • Go pin: the Go module still named vcffi/vcvalue at the deleted branch's commit. It now names 2413bd0e4bf7, the same as the Rust workspaces.
  • Stale names: six doc mentions of Plaintext now say Encrypt, and the CHANGELOG's "refused if another type" claim is now accurate.

Copilot's output-buffer finding is fixed in 1574655.

One thing outside this repo: 2413bd0e is the head of the still-open cipherstash/vitaminc#464. If #464 is squash-merged or rebased, that commit becomes unreachable, which is how the first pin broke. Merge #464 with a merge commit, or tag 2413bd0e, or repin here once it lands.

The CRAP gate failed on `FfiValue::fmt` (complexity 15, 35% covered),
which this PR moves into stack-encrypt: the one test printed three of its
fourteen variants. A test now prints every variant and checks that text
and bytes are redacted inside an array, an object and a passthrough as
well as at the top.
@coderdan

Copy link
Copy Markdown
Contributor Author

CI follow-up in d757b27: the CRAP gate failed on FfiValue::fmt, which this PR moves into stack-encrypt (complexity 15, 35% covered). A test now prints every variant and checks that text and bytes are redacted at any depth. Running the gate locally, no function exceeds the threshold.

`cargo udeps` failed on this PR: the `dynamic` feature still enabled
`vitaminc-aead-frame`, but nothing in stack-encrypt uses it, since an
`FfiValue` frames itself through vitaminc-aead's own `to_frame` and
`Frame`. The guests and stack-guest-abi, which do carry ciphertext trees
in the frame codec, keep their own dependency. `dynamic` is now a plain
feature.
@coderdan

Copy link
Copy Markdown
Contributor Author

CI follow-up in cbfbddd: Unused dependencies failed, as it had on this PR since 15 October, because the dynamic feature still enabled vitaminc-aead-frame and nothing in stack-encrypt uses it. The dependency and the now-unused patch lines in the EQL and fuzz workspaces are gone.

The two "package cleanly for crates.io" checks still fail. They can't pass while stack-encrypt builds on unreleased vitaminc: cargo package ignores [patch] and resolves vitaminc from crates.io. That's the constraint next exists for, and they'll pass once vitaminc 0.6 is released and the pins become versions.

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: a Rust String field reads a data-plan leaf as "\nalice" — the Rust chain and a data plan seal different leaf bytes

3 participants