feat(cachet): add encrypt feature for authenticated value encryption - #558
feat(cachet): add encrypt feature for authenticated value encryption#558schgoo wants to merge 33 commits into
encrypt feature for authenticated value encryption#558Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional encrypt feature to cachet that introduces an authenticated encryption boundary for cache values (AES-256-GCM) before data reaches an untrusted fallback tier, binding ciphertext to the storage key via GCM AAD.
Changes:
- Introduces
AeadCipher/Aes256GcmCipherand anEncryptedTierthat encrypts values and treats undecryptable entries as cache misses. - Adds
.encrypt(&[u8; 32])to the serialized builder pipeline viaEncryptedTransformBuilder, supporting fallback chaining andbuild(). - Adds unit + integration tests, docs/README updates, and feature-gated dependencies (
aes-gcm,getrandom).
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/cachet/tests/encrypt.rs | Integration tests for .serialize().encrypt().fallback() behavior and key-binding relocation defense. |
| crates/cachet/src/transform/mod.rs | Wires in the encrypt transform module behind the encrypt feature gate. |
| crates/cachet/src/transform/encrypt.rs | Implements AES-256-GCM cipher + EncryptedTier wrapper for value encryption/decryption at the storage boundary. |
| crates/cachet/src/lib.rs | Documents the new encrypt feature and re-exports EncryptedTransformBuilder when enabled. |
| crates/cachet/src/builder/transform.rs | Adjusts TransformBuilder field visibility to enable the .encrypt() builder transition. |
| crates/cachet/src/builder/mod.rs | Adds the encrypt builder module and exports EncryptedTransformBuilder behind the feature. |
| crates/cachet/src/builder/encrypt.rs | Implements .encrypt(&key) and the EncryptedTransformBuilder fallback/build pipeline. |
| crates/cachet/README.md | Regenerated README content to document the new feature. |
| crates/cachet/Cargo.toml | Adds the encrypt feature and its optional deps. |
| Cargo.toml | Adds workspace dependency entries for aes-gcm and getrandom. |
| Cargo.lock | Locks new crypto/randomness transitive dependencies. |
| .spelling | Adds crypto-related terminology used by new docs/tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (64.0%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage.
Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
=========================================
- Coverage 100.0% 64.0% -36.0%
=========================================
Files 473 115 -358
Lines 45493 7475 -38018
=========================================
- Hits 45493 4791 -40702
- Misses 0 2684 +2684
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| #[cfg(any(feature = "logs", test))] | ||
| if self.logging_enabled { | ||
| tracing::warn!(cache.name = cache_name, cache.event = attributes::EVENT_UNPROTECT_FAILED); | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/cachet/src/transform/encrypt/mock.rs:36
MockValueProtectoris documented as being available/gated only behindtest-util, but it also requires theencryptfeature (the whole module is#[cfg(feature = "encrypt")]). This can mislead consumers about which features they need to enable.
/// Available with the `test-util` feature. Use it to exercise a
/// [`protect_with`](crate::TransformBuilder::protect_with) pipeline — round-trips, key
/// binding, and unprotect failures — without a real cryptographic library or a source
/// of entropy, keeping tests fast and reproducible.
| // Build the post-transform tier chain and wrap it so values are protected | ||
| // (and key-bound) before reaching it. | ||
| let post_tier = self.post.build_tier(clock.clone(), telemetry.clone(), true); | ||
| let protected = ProtectedTier::new( |
There was a problem hiding this comment.
🤖: Wrapping the already-built post_tier in a single ProtectedTier means unprotect soft-failures happen outside the post chain's own FallbackCache. With chained post tiers, if the first post tier returns tampered/corrupt bytes that inner fallback treats it as a hit and never consults the next post tier; only afterward does the outer ProtectedTier turn the result into None. That breaks the promised "soft failure = miss, fall through" behavior for chained post tiers. Consider applying the protection decorator to each post storage tier before composing the fallback chain (e.g. FallbackCache<ProtectedTier<L2>, ProtectedTier<L3>>, sharing the protector via Arc), or add validated-hit handling that lets the chain continue after an unprotect soft-failure. Worth covering with an integration test that nests an unbuilt ProtectedTransformBuilder as a .fallback(...) across multiple post tiers and forces a tampered read on the first store — the current chained-fallback test only adds tiers inside one protected builder, so it would not catch this.
| /// chain is wrapped in an internal `ProtectedTier`, which protects values and binds | ||
| /// each value to its storage key. Add post tiers with [`fallback`](Self::fallback) and | ||
| /// finish with [`build`](Self::build), exactly as with `TransformBuilder`. | ||
| pub struct ProtectedTransformBuilder<K, V, Pre, Post = ()> { |
There was a problem hiding this comment.
🤖: Design/maintainability: ProtectedTransformBuilder re-implements almost the entire TransformBuilder surface (both fallback impls, Sealed, CacheTierBuilder, Buildable, build, Debug, and the field set). After .protect_with, the storage types are already fixed to BytesView, BytesView and TransformBuilder::fallback already constrains post tiers to that same pair, so the parallel builder buys little extra type-state safety — it mainly carries the Box<dyn ValueProtector>. Consider instead giving TransformBuilder a feature-gated Option<Box<dyn ValueProtector>> field that .protect_with sets, and branching in build_tier to wrap the post tier in ProtectedTier when present. That removes ~200 lines of parallel surface that otherwise must be kept in lockstep as fallback/refresh/stampede wiring evolves. Tradeoff: it threads an optional crypto concern through the general builder and adds some cfg-gated field noise — a maintainability-vs-clarity call for a maintainer, not a correctness issue.
| pub(crate) fn record_unprotect_failure(&self, cache_name: CacheName) { | ||
| #[cfg(any(feature = "logs", test))] | ||
| if self.logging_enabled { | ||
| tracing::warn!(cache.name = cache_name, cache.event = attributes::EVENT_UNPROTECT_FAILED); |
There was a problem hiding this comment.
🤖: This emits cache.unprotect_failed at WARN, but the public event-level table in src/lib.rs (the ### Event types table, ~lines 370-374) has only ERROR/INFO/DEBUG rows — no WARN row — and omits this stable event, and its generated README.md mirror is likewise missing it. Please add a WARN row for cache.unprotect_failed to the table and regenerate the README so the documented telemetry inventory matches what this feature emits.
| tracing::warn!(cache.name = cache_name, cache.event = attributes::EVENT_UNPROTECT_FAILED); | ||
| } | ||
|
|
||
| self.emit_tier_event( |
There was a problem hiding this comment.
🤖: Consider a builder-level test that drives an unprotect soft-failure (malformed or relocated ciphertext) through a CacheEventHandler and asserts the cache.unprotect_failed tier callback carries fallback = true and the same nonzero request ID as the completed get. The existing focused test only greps tracing text for the event name, so regressions in the public structured telemetry fields / request-id correlation would still pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
crates/cachet/src/builder/encrypt.rs:20
- The docs say protection happens "before they reach any storage tier", but
.protect_with()is configured on aSerializeBuilderand only applies to the post-serialize()fallback tier(s); the pre-serialize tier still stores plaintext values.
/// Protects values with the given [`ValueProtector`] before they reach any storage
/// tier, binding each to its storage key.
crates/cachet/src/transform/encrypt/codec.rs:47
record_unprotect_failureis being emitted withstd::any::type_name::<Self>()as the tier name, so thecache.name/tier_namefield won’t identify the actual cache tier that observed the authentication failure. This makes the newcache.unprotect_failedsignal harder to attribute in logs/handlers when multiple tiers are present.
Unprotected::Rejected(Rejection::AuthenticationFailed) => {
self.telemetry.record_unprotect_failure(std::any::type_name::<Self>());
Ok(DecodeOutcome::SoftFailure)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/cachet/src/builder/encrypt.rs:20
- The doc comment says values are protected “before they reach any storage tier”, but
.protect_with()only applies to the next byte-speaking fallback tier introduced by this serialization boundary (the pre-serialize tier remains unprotected). Wording this as “any storage tier” is misleading for users building multi-tier caches.
/// Protects values with the given [`ValueProtector`] before they reach any storage
/// tier, binding each to its storage key.
crates/cachet/src/transform/encrypt/codec.rs:47
record_unprotect_failureis emitted as a tier event, but thetier_namebeing recorded istype_name::<ProtectorCodec>()rather than the actual cache tier name (e.g. "l2"). This makes it hard to attribute unprotect failures to a specific tier and becomes ambiguous when multiple protected tiers exist in a fallback chain.
Unprotected::Rejected(Rejection::AuthenticationFailed) => {
self.telemetry.record_unprotect_failure(std::any::type_name::<Self>());
Ok(DecodeOutcome::SoftFailure)
|
Summary
Adds an optional
encryptfeature tocachetthat protects cache valuesbefore they reach a fallback/remote tier, binding each value to its storage key so
it cannot be read back under a different key.
The feature ships only the protection mechanism and carries no cryptographic
dependency of its own — callers plug in a protector backed by their approved
cryptographic library via
.protect_with(protector):Motivation
When a cache tiers down to an untrusted store (Redis, S3, etc.), values are
exposed at rest. This feature keeps the ergonomic typed cache API while
transparently protecting values on the way out and recovering them on the way
back, with no hand-rolled serialization/encryption pipeline. Shipping the
mechanism without any bundled crypto lets each consumer satisfy its own
cryptographic-library compliance requirements and keeps the crate dependency-free
and portable across all CI targets.
What it does
.protect_with(protector)— available after.serialize()(once values areBytesView). Protects each value with a caller-suppliedValueProtector.ValueProtectortrait — the pluggableprotect(context, plaintext)/unprotect(context, protected)contract. The verb pair mirrors OSdata-protection APIs (Windows DPAPI
CryptProtectData, .NETIDataProtector);contextis the AEAD-associated-data / DPAPI-entropy role.contextand must be bound,so a value protected for one key fails to recover under any other key. This
prevents an attacker with write access to the backing store from relocating or
swapping values between keys.
deterministic and lookupable, so secrets/PII must not be placed in cache keys.
(corrupt, truncated, wrong key, tampered, or relocated) is treated as a cache
miss (
Ok(None)) and emits acache.unprotect_failedtelemetry event (taggedfallback = true, since the protected tier always sits on the fallback side) sotampering is observable, consistent with the serialization codec's soft-failure
behavior.
Design
ProtectedTierinstalled at the storageboundary, where both the key and value are in scope — this is what makes the
key-as-context binding possible.
.protect_with()returns a dedicatedProtectedTransformBuilder(storage typesfixed to
BytesView) supporting.fallback()and.build(), mirroringTransformBuilder. It lives in its ownbuilder/encrypt.rs, matching theserializefeature's file layout.ValueProtectortrait is the pluggable seam; the crate provides nocryptographic implementation. A complete reference
ValueProtectorbacked bySymCrypt (FIPS-certifiable AES-256-GCM) is documented in the crate-level docs as
a copy-paste example, since SymCrypt requires a native library at build/run time
and pulling it into the workspace would force it onto all
--all-featuresCI.encryptfeature namesthe capability (discoverable; avoids overloading the existing
stampede_protection), while the API speaksprotect/unprotect— the same way.NET's
DataProtectionpackage exposes anIDataProtector.Protectmethod.Testability
Per the Microsoft Pragmatic Rust Guidelines (M-MOCKABLE-SYSCALLS,
M-DESIGN-FOR-AI, M-TEST-UTIL), the crate abstracts the non-deterministic
crypto/entropy work behind the
ValueProtectorseam and ships a mock sodownstream consumers can test their protected-cache pipelines without a crypto
dependency:
MockValueProtector— a deterministic, crypto-freeValueProtectorgatedbehind
test-utiland re-exported at the crate root next toMockCache. Itbinds
contextand soft-fails on mismatch/corruption, but provides noconfidentiality (documented as test-only).
Dependencies
None. The
encryptfeature adds no cryptographic dependency; consumers bringtheir own approved library.
Testing
ProtectedTiermechanism and theMockValueProtector..serialize().protect_with().fallback()pipeline via
MockValueProtector: stored bytes are ciphertext with theplaintext never appearing verbatim, values round-trip through the protected tier,
a fresh nonce is used per insert, the boundary is reachable on a
FallbackBuilderand through chained post-transform fallbacks, and a value relocated to a
different key reads as a miss.
transform/encrypt.rsandbuilder/encrypt.rs.