feat(aead): derive macros for Encrypt and Decrypt - #287
Conversation
The decrypt side had `Decrypt` — the trait a plaintext type implements to describe how it decodes itself — living alongside `Decipher`, the trait a cipher backend implements to drive that decoding. The encrypt side already keeps the two apart (`encrypt` vs `cipher`), so the asymmetry made the pairing harder to see than it is. `Decrypt` and its impls now sit in `decrypt`, opposite `encrypt`, leaving `decipher` as the mirror of `cipher`. Public paths are unchanged: everything is re-exported flat from the crate root as before. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
…n impls Writing `Encrypt`/`Decrypt` by hand for a plain struct was boilerplate with a security-relevant detail buried in it: values must be sealed as a map so each field's key is bound into its AAD. Getting that wrong — encoding as a sequence, say — leaves same-typed fields interchangeable in stored ciphertext. The derive makes the safe shape the default. A derived struct is a map keyed by field name, wire-compatible with the equivalent `HashMap`, with `#[aead(rename = "...")]` to pin an existing key and `#[aead(crate = "...")]` for re-exports. Newtype structs are transparent, tuple structs are keyed by index, and field-less structs use the authenticated empty-map marker. Decoding rejects missing, unknown, and duplicate keys rather than defaulting or skipping them. Enums are refused outright: a ciphertext carries no authenticated variant discriminator, so any encoding would either leak the variant or leave it forgeable. Decoding a heterogeneous struct needs the value type chosen after the key is known, so `MapAccess` gains `next_key`/`next_value`, with `next_entry` now defaulted over them. Reading key-first also makes the decode order-independent, which matters because entry order in a stored ciphertext is not authenticated. Skipping a value is refused: an entry never decrypted is one whose AAD binding is never verified. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
ee5e2f0 to
4fc6738
Compare
🧬 Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 22 | 0 | 18 | 0 |
✅ Every mutant in the changed lines was caught by a test.
✅ No CRAP threshold violations532 function(s) analyzed · threshold 30 |
Proc-macro logic runs at compile time, not inside the instrumented test binary, so cargo-mutants cannot reach it: every mutant in a derive crate survives because the code is unreachable from `cargo test`, not because a test is missing. `packages/protected-derive` and `packages/random-derives` are already excluded for this reason; the new derive crate joins them. The derive is exercised where it actually runs — the round-trip and rejection tests in `packages/encrypt/tests/derive.rs`, which fail to compile if the macro stops emitting an impl. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
`next_entry` became a default method over `next_key`/`next_value`, and nothing in `vitaminc-aead` covered it — cargo-mutants scopes each mutant's test run to the package it lives in, so the round-trip coverage over in `vitaminc-encrypt` never ran for it. A `next_entry` that reported the map exhausted survived: a `HashMap` would have decoded silently to empty rather than failing. Adds a `MockMapAccess` test double and covers the default's happy path plus both contract violations the key/value split introduces — `next_value` with no pending key, and a second `next_key` that would discard one. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
…om the gates
The derive crate was headed for a CRAP-gate failure and was excluded from
mutation testing, both on the premise that proc-macro code cannot be
covered. That premise only holds for the `#[proc_macro_derive]` entry
points: they run at compile time, not in the instrumented test binary.
The expansion each one delegates to is an ordinary
`fn(DeriveInput) -> Result<TokenStream>` that a unit test can call and
llvm-cov can score.
So cover it. 18 tests pin the wire shape the macros generate — per-field
key binding, `rename`, tuple-index keys, newtype transparency, the
empty-map marker, the `'__c` bounds, the visitor's `PhantomData`,
`#[aead(crate = "...")]` redirection, and the attribute-error paths.
Expansions are compared as tokens, not text: `test_support` renders the
expected fragment through `quote!` too, so an assertion is written as
ordinary Rust and cannot fail on `TokenStream` spacing.
decrypt::derive CRAP 90.0 (0% covered) -> 9.0 (100%)
encrypt::derive CRAP 56.0 (0% covered) -> 7.0 (100%)
mutants 18 tested, 14 caught, 4 unviable, 0 missed
The two entry points stay uncovered at CC 1, scoring 2.0 — under the
threshold because they are trivial by construction. That is the shape
worth keeping: a thin shim over a testable function.
Config follows the evidence. `aead-derive` needs no cargo-crap `allow`
entry at all, and its mutants exclusion narrows from the whole crate to
`src/lib.rs`. The comments in both files now name `random-derives` and
`protected-derive` specifically and record the split as the way off those
lists, rather than implying proc-macro crates are inherently unscorable.
Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
A derived struct maps onto a database row, and a row usually has columns
that are not secret and must stay queryable: an id to join on, a tenant to
filter by, a timestamp to sort on. Encrypting those makes the row useless
to every query that does not hold the key, so the derive needs a way to
leave a field alone.
`#[aead(passthrough)]` stores the field as a cleartext map entry. The wire
format already had the concept — `MapCipher::passthrough_entry` — but not
in a form generated code could reach: it takes the cipher's own
`Passthrough` associated type, which an impl generic over every cipher
cannot name. `Cipher::passthrough_boxed` solves exactly that problem for
whole ciphertexts, so this adds the missing pair:
MapCipher::passthrough_entry_boxed — write a type-erased entry
MapAccess::next_passthrough — read one back, caller downcasts
`AesMapAccess::next_passthrough` refuses an encrypted node rather than
unwrapping it, so a sealed value cannot be laundered into the caller with
its tag unchecked.
The security trade is total and deliberate, and the docs say so plainly. A
passthrough value is not encrypted and not authenticated: no tag covers
it, and unlike an encrypted entry's key, nothing binds its key either. It
can be edited, retargeted, added or deleted and every other field still
decrypts. That is precisely what makes it usable as an independent column
— an `UPDATE tenant = …` must not break the encrypted fields beside it —
and it is why such a field is untrusted input on the way back, never
something the program then trusts to make an authorization decision.
Two shapes are compile errors rather than runtime surprises: an
all-passthrough struct (nothing would be encrypted, so `MapCipher::end`
would refuse to seal a container with no tag at all) and `passthrough` on
a newtype (transparent, so there is no map entry to hold it).
Tested at both levels: expansion units pin which channel each field is
routed through, and eight end-to-end tests against real AES-256-GCM pin
the properties — a cleartext column readable straight out of the stored
ciphertext with no key, an independent rewrite that still decrypts and
reads back the new value, an encrypted column that still fails when moved
to another key, a wrong-typed payload rejected by the downcast, a deleted
column caught by the missing-field rule, and a sealed node refused when
read through the passthrough path.
Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
…s with trybuild Three defects found reviewing the derive macros, plus the test layer that would have caught two of them. **`#[aead(rename = "...")]` on a newtype was silently discarded.** The attribute was parsed and folded into the field's map key, but a newtype is transparent: it opens no map, and its expansion never reads that key. So `struct Token(#[aead(rename = "token")] String)` compiled clean and produced a value with no key binding at all, while the author had every reason to believe it was key-bound. It is now the same hard error `#[aead(passthrough)]` on a newtype already was — the two share one guard, since the reasoning is identical. **A non-`usize` const parameter could not derive `Decrypt`.** The visitor must use every generic parameter it redeclares, and the old encoding named them one at a time — a const parameter as an array length, `[u8; N]`. That silently requires `N: usize`, so `struct S<const N: u32>` failed with "the constant N is not of type usize", blamed on the macro rather than the user's code. `PhantomData<fn() -> Self::Value>` passes every parameter along exactly as declared, whatever its kind. A function pointer because the visitor owns nothing: it imposes no drop obligation and is `Send` and `Sync` regardless of the value type. **`MapAccess::next_key` returns an unauthenticated key.** A key is bound into the AAD its value is sealed against, but that binding is only checked when `next_value` opens the value — the key itself is the raw stored one, verified by nothing. Safe for choosing which field to decode next, since a wrong choice fails at `next_value`; not safe to act on before then. Now documented, alongside the same warning `next_passthrough` already carries. No code change: in-tree callers all reach `next_value`. **trybuild.** The derive's unit tests assert that `Shape::parse` returns an error containing a substring. They cannot check what a developer actually meets: that the derive fails to compile, and that the diagnostic points at the right struct and explains itself. For guards that exist to stop an unsound ciphertext — an enum with no authenticated discriminator, a struct that would seal no tag — that message is the feature. Six cases now pin it, following the `vitaminc-protected` pattern (`tests/ui/`, `#[cfg(not(miri))]`, checked-in `.stderr`). The unit tests stay: they are fast and mutation- covered, and this is the layer above them. Also ignores the `.profraw` files `cargo llvm-cov` drops into the tree. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
…rywhere The attribute documentation was a four-line list in the derive crate's module docs, which is both too thin for `passthrough` — an attribute whose whole story is what protection it gives up — and invisible to anyone reading `vitaminc_aead` or `vitaminc_encrypt`, where the macros are actually used from. The reference now lives in `packages/aead-derive/docs/attributes.md`: a table of the three options, then a section each covering what it does, the wire consequences, and what it refuses to compile. Reuse is by `#[doc(inline)]` on the re-exports, not by sharing the file. Cross-crate `include_str!` would not survive publishing — the file is not in the depending crate's `.crate` archive, so docs.rs could not build it. Inlining makes rustdoc render the macros' own documentation at each re-export site, so there is exactly one copy of the text and it cannot drift. Within the derive crate the file is included by the crate docs and by both macros; the other two crates get it for free. This also adds `vitaminc_encrypt::Decrypt`, which was simply missing: the crate re-exported `Encrypt` (one `use` carries both the trait and the derive macro of that name) but not its counterpart, so a caller depending only on `vitaminc-encrypt` could derive `Encrypt` and not `Decrypt`. The two now travel together on their own inlined re-export. Verified: the passthrough warning renders on the derive pages of all three crates, the workspace doc-warning count is unchanged at 6, and the wasm32 build gate still passes. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
`cargo doc` was emitting eight warnings across the workspace. rustdoc warns and exits 0, so nothing caught them and nothing would have caught the next one — there is no docs job in CI at all. Both halves are fixed here. The links themselves fall into three groups: - **Links to private items.** `Aad::for_marker` is deliberately private, so two public methods linked at something a reader cannot reach. They now describe the derivation and point at its public surface instead. - **Links with no path.** `Cipher::passthrough`, `Decipher::decrypt_passthrough` and `Aad::for_map_entry` were written unqualified from modules where the name is not in scope, so they rendered as plain text. Now qualified. `AesCipherText` cannot be linked at all — it lives in `vitaminc-encrypt`, which `vitaminc-aead` does not depend on — so it is code text, not a link. - **A redundant explicit target**, where the label already resolved. One of these was hiding a real loss. `Element`'s documentation — why a row seals as a sequence element, and the `Vec<Element<T>>` trap that seals under a double derivation — lived in `element`'s module docs, and `mod element` is private, so none of it rendered anywhere. The link to `self` was the symptom. That narrative now sits on the `Element` type, where a reader of `vitaminc_aead::Element` will actually meet it. The gate is a `cargo doc --workspace --all-features --no-deps` step with `RUSTDOCFLAGS: -D warnings`. `--all-features` matters: the `AesCipherText` link was inside the `hlist` module and invisible to a default-feature run. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
The guest is a standalone workspace, deliberately outside the main one, so the docs gate added alongside the workspace fmt check does not reach it — its intra-doc links were checked by nothing. Its own job now runs the same gate, on wasm32-wasip1 to match the clippy step beside it, since the ABI module only builds there. Clean as it stands; this keeps it that way. Claude-Session: https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc
freshtonic
left a comment
There was a problem hiding this comment.
Standards
The implementation follows the repository’s documented conventions, keeps the derive logic well factored, documents the security tradeoffs clearly, and adds strong unit, UI, integration, mutation, and documentation coverage. I found no blocking standards violation.
Spec
The requested derive/re-export/release integration, map and tuple shapes, transparent newtypes, enum rejection, strict missing/unknown/duplicate handling, key-first order-independent decoding, rename/crate attributes, defaulted next_entry, and explicit field-swap tamper test are all present.
Changes are required because the added passthrough facility conflicts with #292’s defining guarantee: the issue says the derive should always key-bind every field, whereas #[aead(passthrough)] deliberately emits fields with neither encryption nor authentication and permits undetectable retargeting. It also introduces a third required MapAccess method unrelated to the requested compatibility-preserving key/value split. Please move passthrough to a separately specified PR, or redesign it so the authenticated derive contract and existing external MapAccess implementations remain intact.
`MapAccess::next_passthrough` was a required method, so every implementor had to answer for passthrough entries even when its format has no representation for them. Making it a provided method that returns `Unspecified` means "unsupported" reads as a refusal rather than boilerplate: a decipher that does not store passthrough entries simply cannot hand one back, and only a decipher that writes them through `MapCipher::passthrough_entry_boxed` needs to override it. `MapAccess::Error` gains a `From<Unspecified>` bound so the default can build its error. `MockMapAccess` stores every entry encrypted, so it drops its hand-written rejection and inherits the default, which a new test pins. Claude-Session: https://claude.ai/code/session_01Jz5KYiooZazq5UxLqyogSi
freshtonic
left a comment
There was a problem hiding this comment.
Standards
The new commit is clean, documented, tested, and reduces boilerplate by making unsupported passthrough reads refuse safely. No standards or maintainability blocker remains; current CI is fully green.
Spec
The required-method concern is improved but only partially resolved: next_passthrough now has a rejecting default, while every MapAccess implementation still inherits a new Error: From<Unspecified> requirement solely for the out-of-scope passthrough feature.
The main blocker remains unchanged. #292 specifies that the derive always uses field-key AAD so every field receives the anti-swap guarantee. #[aead(passthrough)] is an explicit, well-documented opt-out, but it was not part of that issue and normal derived Decrypt still accepts those fields without a tag or key binding. The fact that passthrough has no AAD is precisely why it should be separated from this security-focused derive contract or exposed through a distinctly untrusted API.
Changes remain requested: split passthrough into its own specification/PR, or redesign the API boundary so #292’s derived contract remains true and MapAccess implementors do not acquire unrelated requirements.
freshtonic
left a comment
There was a problem hiding this comment.
My previous rejection reasons were invalid.
Closes #292
Adds
#[derive(Encrypt)]/#[derive(Decrypt)]forvitaminc-aead, and moves theDecrypttrait into its own module so the decrypt side mirrors the encrypt side.Why
Hand-writing
Encrypt/Decryptfor a plain struct is boilerplate with a security-relevant detail buried in it: values have to be sealed as a map, becauseMapCipherbinds each entry's key into the AAD its value is sealed against. Encoding a struct as a sequence instead compiles fine and round-trips fine — and leaves two same-typed fields freely interchangeable in stored ciphertext, since element AAD carries no positional component. The derive makes the safe shape the default.Commits
refactor(aead)—Decryptand its impls move topackages/aead/src/decrypt/, oppositeencrypt, leavingdecipheras the mirror ofcipher. PreviouslyDecrypt(what a plaintext type implements) sat alongsideDecipher(what a cipher backend implements), which the encrypt side already keeps apart. Public paths are unchanged — everything is re-exported flat from the crate root as before.feat(aead)— the derive itself, in a newvitaminc-aead-derivecrate re-exported fromvitaminc_aead, following theprotected/protected-deriveconvention. Added to the release group, so it releases with the other crates.Wire shape
Aad::for_map_entrybinding, so a stored field cannot be renamed or moved onto another key undetected. Interchangeable with the equivalentHashMap<String, _>ciphertext.#[aead(rename = "...")]pins the old wire key.Protected<T>/Equatable<T>already behave."0","1", …), so they get the same per-field binding.#[aead(crate = "::vitaminc::aead")]on the container for use through a re-export.MapAccessgains a key/value splitnext_entry::<T>()fixes the value type before the key is known, so it cannot decode a struct with heterogeneous field types at all.MapAccessnow has:with
next_entrydefaulted over the two — soHashMap's existing impl, and any homogeneous consumer, is untouched.AesMapAccessis the only implementor of the trait in the workspace.Reading key-first also makes the decode order-independent, which matters because entry order in a stored ciphertext is not authenticated. Both misuse paths are refused rather than tolerated: a second
next_keywithout consuming the pending value, andnext_valuewith no pending key. A skipped value is one whose AAD binding is never verified.Tests
26 units in
vitaminc-aead-derive: 7 shape tests — enum/union rejection, duplicate-key rejection, newtype vs named-single-field, tuple index keys, field-less shapes — plus 19 expansion tests that pin the generated code itself: per-field key binding,rename, tuple index keys, newtype transparency, the empty-map marker, the'__cbounds, the visitor'sPhantomData,#[aead(crate = "...")]redirection, and the attribute-error paths. Expansions are compared as tokens, not text — the expected fragment is rendered throughquote!too, so an assertion is written as ordinary Rust and cannot fail onTokenStreamspacing.This puts the derive crate under both quality gates rather than exempting it:
decrypt::derivegoes from CRAP 90.0 at 0% covered to 9.0 at 100%,encrypt::derivefrom 56.0 to 7.0, andcargo mutantsreports 14 caught / 0 missed. Only the two#[proc_macro_derive]shims inlib.rsstay uncovered — they run at compile time, and at CC 1 they score under the threshold anyway.13 end-to-end tests in
packages/encrypt/tests/derive.rsagainst real AES-256-GCM — round-trips for every shape,HashMapwire compatibility, rename, missing/unknown field rejection, order independence, wrong-AAD failure, and a field-swap tamper test: two equal-lengthStringfields transposed in the stored ciphertext must fail to decrypt, which is the property the map shape exists for.4
MapAccesscontract tests inpackages/encrypt/tests/map_access.rs.Full workspace green;
cargo fmt --checkandcargo clippy --all-targets --all-features -- -D warningsclean.https://claude.ai/code/session_011kjxjgxWmT4yi23ZqufSEc