feat(raftengine): Stage 6C-2c — encryptionScanner against etcdraft.MemoryStorage (#783)

bootjp · web-flow · commit 12ec8ba75460 · 2026-05-18T03:46:22.000+09:00
## Summary Stage 6C-2c per the [PR #762 plan](#762) and the 6C sub-decomposition landed in [PR #781](#781) / [PR #782](#782). Ships the **raftengine-side scanner** that implements the `encryption.EncryptionRelevantScanner` interface defined in [PR #782 (Stage 6C-2b)](#782). The `main.go` startup-guard wiring that ties this scanner to the encryption-side guard is split into a new **6C-2d** sub-milestone per the design doc update in this PR. ## What this PR ships ### `encryptionScanner` against `etcdraft.MemoryStorage` ```go func (e *Engine) EncryptionScanner() encryption.EncryptionRelevantScanner ``` Walks `raftpb.Entry` rows in `(startExclusive, endInclusive]` via `storage.Entries(lo, hi, scanMaxBytes)`, filters out: - `EntryConfChange` / `EntryConfChangeV2` — config changes never carry FSM payloads; their `Data[0]` could coincidentally fall in the encryption opcode range without this gate - Zero-length normal entries — etcd-raft leadership bumps after election; would `panic: index out of range` on a naive `Data[0]` read Returns `IsEncryptionRelevantOpcode(ent.Data[0])` over surviving entries — i.e., asks whether any entry's opcode is in `[OpBootstrap, OpEncryptionMax]` = `[0x04, 0x07]`. Note: **OpRegistration (0x03) is correctly excluded** per the predicate's narrowed range from PR #782. ### Error semantics | Path | Behavior | |---|---| | Compacted range (sidecar below snapshot horizon) | Wrapped `ErrCompacted` propagated; caller routes as scanner-error refusal **distinct** from `ErrSidecarBehindRaftLog` | | `nil` storage (engine not opened) | Returns error rather than nil-deref | | Empty range | `(false, nil)` fast path; storage NOT consulted | ### Tests (8 cases covering every branch) | Test | Pins | |---|---| | `EmptyRange` (3 sub-cases) | equal / start-ahead / both-zero — storage never consulted | | `NoRelevantEntries` | OpRegistration (0x03) explicitly NOT hit — pins the PR #782 predicate narrowing end-to-end | | `BootstrapHit` | OpBootstrap in range → hit | | `RotationHit` | OpRotation in range → hit | | `NonNormalEntriesSkipped` | ConfChange / ConfChangeV2 with `Data[0]` in encryption range → NOT hit | | `EmptyDataSkipped` | nil / empty `Data` → NOT hit AND no panic | | `CompactedRange` | Snapshot at index 10, scan `[0, 5)` → error, hit=false | | `NilStorageError` | Zero-value scanner returns an error | ## Design doc updates - **6C-2c** row rewritten to reflect the scanner-only scope of this PR (engine implementation + `Engine.EncryptionScanner()` accessor, **no** `main.go` wiring). - **6C-2d** row added — `main.go` startup-guard phase that ties 6C-2b's pure-function guard to 6C-2c's engine scanner. Will ship as its own PR. - Shipping order updated: `6C-1 ✅ → 6C-2 ✅ → 6C-2b ✅ → 6C-2c (this PR) → 6C-2d → 6D (bundles 6C-3) → 6E (bundles 6C-4)`. - Rationale section updated with the 6C-2c / 6C-2d split. ## Why split 6C-2c into "scanner" vs "wiring" Keeping the engine scanner separate from the `main.go` wiring lets: - Reviewers focus on one concern per PR. - Stage 6D-and-later code paths that need the scanner (capability fan-out gap-coverage check) depend on the engine package without waiting for 6C-2d. - 6C-2d's main.go integration touches the multi-shard lifecycle, which is a different concern from the engine-internal scanner implementation here. ## Caller audit No public API changes outside `Engine.EncryptionScanner()`, which is net-additive. `encryptionScanner` and `isEncryptionRelevantEntry` are package-internal. No existing caller references the new accessor. The implementation depends only on: - `encryption.EncryptionRelevantScanner` (interface from PR #782) - `encryption.IsEncryptionRelevantOpcode` (predicate from PR #782, OpRegistration-excluded) - `etcdraft.MemoryStorage.Entries` (existing engine internal) - `raftpb.Entry` / `raftpb.EntryConfChange` / V2 (existing types) No changes to the engine's apply path or any other lifecycle. ## Five-lens self-review 1. **Data loss** — net-positive. The scanner enables the §9.1 refusal that catches partial-write crashes between an encryption-relevant Raft commit and the sidecar update. Until 6C-2d wires the scanner into main.go the scanner is operator-inert, but its failure modes (compacted-range error, conf-change skip, empty-data skip, nil-storage error) are all correct. 2. **Concurrency / distributed failures** — scanner reads from `MemoryStorage` (thread-safe per etcd-raft contract). Scanner itself is stateless. 3. **Performance** — zero hot-path impact. Called once per shard per process start in 6C-2d, with at most one `Entries()` call in the typical case (gap size << 64 MiB). 4. **Data consistency** — the scanner respects MemoryStorage's compaction boundary: never silently returns "no hit" for a range below the snapshot horizon; propagates the error so the guard routes it as a distinct scanner-error refusal. 5. **Test coverage** — 8 tests cover all 6 predicate paths, all 2 filter gates (type, length), AND the 2 error paths (compacted, nil-storage). The OpRegistration exclusion is pinned end-to-end with a dedicated test case. ## Test plan - [x] `go test -race -timeout=120s ./internal/raftengine/etcd/... ./internal/encryption/...` — PASS - [x] `golangci-lint run ./internal/raftengine/etcd/...` — 0 issues - [ ] Full Jepsen suite — not run; scanner has no caller in this PR (operator-inert until 6C-2d) ## Plan Per the design doc updates in this PR, next sub-milestones: - **6C-2d** — `main.go` startup-guard phase that calls `encryption.GuardSidecarBehindRaftLog` with the scanner from this PR - **6C-3** — `ErrNodeIDCollision` + `ErrLocalEpochRollback` (bundles with Stage 6D) - **6D** — `enable-storage-envelope` admin RPC + §7.1 Phase-1 cutover (unblocked once 6C-2d lands)  ## Summary by CodeRabbit * **New Features** * Advances encryption-at-rest implementation with internal validation mechanisms to ensure system consistency during startup. * **Documentation** * Updated design documentation clarifying the encryption-at-rest implementation phases and dependency chain. * **Tests** * Added comprehensive test coverage for encryption validation logic, including edge cases and error scenarios.  [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/bootjp/elastickv/pull/783?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
diff --git a/docs/design/2026_04_29_partial_data_at_rest_encryption.md b/docs/design/2026_04_29_partial_data_at_rest_encryption.md
@@ -23,7 +23,8 @@ Date: 2026-04-29
 | 6C-1 | Subset of §9.1 startup refusal guards that depend only on flags + sidecar state: sidecar present without `--encryption-enabled` (`ErrSidecarPresentWithoutFlag`, downgrade prevention), `--encryption-enabled` without `--kekFile` (`ErrKEKRequiredWithFlag`, fail-fast), KEK loaded but sidecar wrapped DEKs do not unwrap (`ErrKEKMismatch`, operator-error catch). `internal/encryption/startup.go`'s `CheckStartupGuards` runs once in `main.go run()` before `buildShardGroups`. | shipped | PR #778 |
 | 6C-2 | Process-local §9.1 guards that read only the encryption package's own state: `ErrLocalEpochExhausted` (any active DEK with `local_epoch == 0xFFFF` — refuses to start until rotated, preventing GCM nonce reuse on the next write), `ErrUnsupportedFilesystem` startup probe (`ProbeSidecarFilesystem` exercises the §5.1 write+rename+dir.Sync sequence on a sentinel file in the sidecar directory at startup so filesystem-capability failures surface BEFORE the first encryption-relevant Raft entry, not on the first WriteSidecar call hours into operation). Also includes the gemini-r3 medium parseErr-annotation fix deferred from PR #778 (`ErrKEKMismatch` now wraps both `parseErr` and the unwrap error when the defensive non-decimal-key_id branch fires). The asymmetric `ErrLocalEpochRollback` peer is split into 6C-3 because it requires the writer-registry record (Stage 7). The `ErrSidecarBehindRaftLog` guard moves to 6C-2b because it requires raftengine integration — outside the encryption package boundary. | open | — |
 | 6C-2b | `ErrSidecarBehindRaftLog` guard PRIMITIVE — encryption-side sentinel, `IsEncryptionRelevantOpcode` predicate (§5.5), `EncryptionRelevantScanner` cross-package interface, and `GuardSidecarBehindRaftLog` pure-function guard that takes both indices + a scanner callback. Tests use a fake scanner to exercise every branch (caught-up / gap-not-covered / gap-covered / scanner-error / nil-scanner-fails-closed). Wires nothing into main.go; the raftengine-side implementation lands in 6C-2c. | shipped-in-this-PR | this PR |
-| 6C-2c | Raftengine implementation of `EncryptionRelevantScanner` (WAL-based scan of Raft entries in the supplied index range against the §5.5 predicate) + `main.go` startup-guard phase that runs after `buildShardGroups` for each shard with encryption enabled, before any gRPC server starts serving. Inherits the encryption-side primitive from 6C-2b. | open | — |
+| 6C-2c | Raftengine implementation of `EncryptionRelevantScanner` against `etcdraft.MemoryStorage.Entries`. `Engine.EncryptionScanner()` exposes the scanner as the encryption-side interface so callers (Stage 6C-2d, 6D, 6E) inherit it without coupling to the engine internals. Tests pin every branch: empty range, no-relevant entries (with OpRegistration explicitly), bootstrap hit, rotation hit, conf-change skipped, empty-data skipped, compacted-range error propagation, nil-storage error. | shipped-in-this-PR | this PR |
+| 6C-2d | `main.go` startup-guard phase that runs after `buildShardGroups` for each shard with encryption enabled, before any gRPC server starts serving. Calls `encryption.GuardSidecarBehindRaftLog` with the shard's engine `AppliedIndex()` + sidecar `RaftAppliedIndex` + the engine's `EncryptionScanner()` from 6C-2c. The guard's existing branches (caught-up / gap-not-covered / gap-covered / scanner-error / nil-scanner-fails-closed) determine refusal behavior. | open | — |
 | 6C-3 | Cluster-wide §9.1 guards that require the Voters ∪ Learners membership view: `ErrNodeIDCollision` (two members hash to the same 16-bit `node_id`), `ErrLocalEpochRollback` (sidecar `local_epoch` ≤ writer-registry record). Naturally bundles with the Stage 6D capability fan-out which already needs that membership view. | open | — |
 | 6C-4 | Phase-2-specific §9.1 guards: `ErrEnvelopeCutoverDivergence` (sidecar `raft_envelope_cutover_index` disagrees with snapshot header), `ErrEncryptionNotBootstrapped` (`enable-raft-envelope` before bootstrap), `ErrLocalEpochOutOfRange` on the wire side of `GetCapability` / `GetSidecarState`. Bundles with Stage 6E (`enable-raft-envelope` admin RPC + Phase-2 cutover). | open | — |
 | 6D | §6.6 `enable-storage-envelope` admin RPC + §7.1 Phase-1 storage cutover (§6.2 toggle ON) + Voters ∪ Learners capability gate (depends on 6B for mutator wiring AND 6C-1+6C-2 for §9.1 startup-refusal guards; bundles 6C-3 for the membership-view-dependent collision check — see rationale) | open | — |
@@ -200,18 +201,28 @@ production-safe state, and unblocks the next one.
       ONLY on the encryption package without waiting on the
       raftengine-side scan helper.
 
-    * **6C-2c** — Raftengine implementation of the
-      `EncryptionRelevantScanner` interface defined in 6C-2b. Walks
-      the WAL across the supplied entry-index range, decodes each
-      entry's opcode byte, and applies the §5.5 predicate. Wires
-      a new startup-guard phase into `main.go` that runs AFTER
+    * **6C-2c** — Raftengine-side implementation of the
+      `EncryptionRelevantScanner` interface defined in 6C-2b.
+      Implemented against `etcdraft.MemoryStorage.Entries` so the
+      scan reads in-memory state that the engine has just replayed
+      from the WAL + snapshot at Open() time. Filters out
+      EntryConfChange / EntryConfChangeV2 and zero-length normal
+      entries (etcd-raft leadership bumps) so those don't falsely
+      trigger the predicate. Compacted-range errors propagate
+      wrapped — the caller (Stage 6C-2d's main.go phase) surfaces
+      them as a scanner-error refusal distinct from
+      ErrSidecarBehindRaftLog. Exposed via the new
+      `Engine.EncryptionScanner()` accessor.
+
+    * **6C-2d** — `main.go` startup-guard phase that ties 6C-2b's
+      pure-function guard to 6C-2c's engine scanner. Runs AFTER
       `buildShardGroups` returns and each shard's engine has opened,
       but BEFORE any gRPC server starts serving — a different
       lifecycle phase from the existing 6C-1 / 6C-2 guards (which
       run before engine startup). For each shard with encryption
-      enabled, the phase calls `GuardSidecarBehindRaftLog` with the
-      shard's engine `AppliedIndex()` and a scanner backed by the
-      engine's WAL.
+      enabled, calls `GuardSidecarBehindRaftLog` with the shard's
+      engine `AppliedIndex()` + sidecar `RaftAppliedIndex` + the
+      engine's `EncryptionScanner()`.
 
     * **6C-3** — Membership-view guards. `ErrNodeIDCollision`
       requires a Voters ∪ Learners enumeration of every Raft
@@ -229,11 +240,12 @@ production-safe state, and unblocks the next one.
       `local_epoch` values to the §4.1 16-bit field. Bundles with
       Stage 6E's `enable-raft-envelope` admin RPC.
 
-  **Shipping order:** 6C-1 → 6C-2 → 6C-2b (primitive) → 6C-2c
-  (raftengine implementation + main.go wiring) → 6D (bundles
-  6C-3) → 6E (bundles 6C-4). Stage 6D is unblocked once 6C-1
-  + 6C-2 + 6C-2b + 6C-2c have all shipped (the latter pair
-  is the load-bearing `ErrSidecarBehindRaftLog` work); 6D
+  **Shipping order:** 6C-1 → 6C-2 → 6C-2b (encryption-side
+  primitive) → 6C-2c (raftengine scanner) → 6C-2d (main.go
+  startup-guard wiring) → 6D (bundles 6C-3) → 6E (bundles
+  6C-4). Stage 6D is unblocked once 6C-1 + 6C-2 + 6C-2b +
+  6C-2c + 6C-2d have all shipped (the latter three together
+  are the load-bearing `ErrSidecarBehindRaftLog` work); 6D
   does not need to wait for the 6C-3/6C-4 work that bundles
   into it.
 
diff --git a/internal/raftengine/etcd/encryption_scanner.go b/internal/raftengine/etcd/encryption_scanner.go
@@ -0,0 +1,165 @@
+package etcd
+
+import (
+	"math"
+
+	"github.com/bootjp/elastickv/internal/encryption"
+	"github.com/cockroachdb/errors"
+	etcdraft "go.etcd.io/raft/v3"
+	raftpb "go.etcd.io/raft/v3/raftpb"
+)
+
+// scanMaxBytes caps the byte size of a single Entries() fetch from
+// MemoryStorage during a gap scan. The §9.1 ErrSidecarBehindRaftLog
+// guard runs once per process start, so a comfortable margin over
+// any expected gap (typical: 1-N entries from a partial-write
+// crash) keeps the scan to a single Entries() call without the
+// per-iteration ceremony of a pagination loop. If a real gap ever
+// exceeds this size the iteration falls back to additional calls
+// (see scanEntries() below) so the cap is not a correctness
+// boundary, only a per-call memory cap.
+const scanMaxBytes uint64 = 64 << 20 // 64 MiB
+
+// encryptionScanner implements encryption.EncryptionRelevantScanner
+// against the engine's in-memory raft storage. It wraps the
+// engine's *MemoryStorage; a fresh value is constructed per
+// Engine.EncryptionScanner() call (cheap — no internal state).
+// Callers get access to it via Engine.EncryptionScanner().
+//
+// The scan walks raftpb.Entry rows in (startExclusive, endInclusive]
+// via etcdraft.MemoryStorage.Entries(lo, hi, maxSize), filters out
+// non-normal entries (conf-change, empty leadership bump) since the
+// §5.5 IsEncryptionRelevantOpcode predicate only applies to normal
+// FSM entries, and reports whether any survives the predicate.
+//
+// Compacted-out-of-MemoryStorage ranges return raft's
+// ErrCompacted (or ErrUnavailable) which the scanner wraps and
+// propagates. GuardSidecarBehindRaftLog routes that as a scanner
+// error (NOT as ErrSidecarBehindRaftLog), so the operator sees a
+// distinct refusal: "the gap exists but is below the snapshot
+// horizon; run encryption resync-sidecar to advance past the
+// compacted entries". A sidecar that far behind is itself the
+// operator's primary problem; the scanner just reports honestly.
+type encryptionScanner struct {
+	storage *etcdraft.MemoryStorage
+}
+
+// HasEncryptionRelevantEntryInRange satisfies the
+// encryption.EncryptionRelevantScanner contract: returns true iff
+// at least one raftpb.Entry with index in (startExclusive,
+// endInclusive] is a §5.5 sidecar-mutating opcode per
+// encryption.IsEncryptionRelevantOpcode.
+//
+// Empty range (startExclusive >= endInclusive) returns (false, nil)
+// per the interface contract. The encryption.GuardSidecarBehindRaftLog
+// caller precomputes the non-empty-gap branch and only calls scan
+// when there's a range to inspect, but the defensive guard stays.
+func (s *encryptionScanner) HasEncryptionRelevantEntryInRange(startExclusive, endInclusive uint64) (bool, error) {
+	// The EncryptionRelevantScanner contract (audit.go) requires
+	// the empty-range case to return (false, nil) UNCONDITIONALLY
+	// — including on nil-storage receivers. Reorder the guards so
+	// an empty range short-circuits before any state-dependent
+	// failure path. GuardSidecarBehindRaftLog pre-filters the
+	// empty-gap case in production, so this is contract conformance
+	// rather than a reachable bug; pinning the order keeps any
+	// future caller (6C-2d, capability fan-out) from triggering
+	// the surprising "empty range on nil scanner returns error"
+	// path.
+	if startExclusive >= endInclusive {
+		return false, nil
+	}
+	if s.storage == nil {
+		return false, errors.New("encryption scanner: storage is nil (engine not opened?)")
+	}
+	// MemoryStorage.Entries(lo, hi, ...) returns indices [lo, hi).
+	// We want (startExclusive, endInclusive] = [startExclusive+1,
+	// endInclusive+1). Guard against uint64 wraparound at
+	// MaxUint64 (otherwise hi = 0 and the loop never runs).
+	if endInclusive == math.MaxUint64 {
+		return false, errors.New("encryption scanner: endInclusive == math.MaxUint64 cannot be expressed as half-open Entries() range")
+	}
+	lo := startExclusive + 1
+	hi := endInclusive + 1
+	cursor := lo
+	for cursor < hi {
+		batch, err := s.storage.Entries(cursor, hi, scanMaxBytes)
+		if err != nil {
+			return false, errors.Wrapf(err,
+				"encryption scanner: read raft entries [%d, %d) from MemoryStorage",
+				cursor, hi)
+		}
+		if len(batch) == 0 {
+			// Fail closed: an empty batch with cursor < hi means
+			// MemoryStorage didn't advance even though the requested
+			// range is non-empty. Treating this as "no hit" would
+			// silently classify an unscanned range as harmless, so
+			// we surface it as a scanner error. The caller routes
+			// scanner errors as a refusal distinct from
+			// ErrSidecarBehindRaftLog.
+			return false, errors.WithStack(errors.Newf(
+				"encryption scanner: Entries(cursor=%d, hi=%d) returned no entries and no error (unscanned gap)",
+				cursor, hi))
+		}
+		for _, ent := range batch {
+			if isEncryptionRelevantEntry(ent) {
+				return true, nil
+			}
+		}
+		// Continue from one past the last returned index.
+		cursor = batch[len(batch)-1].Index + 1
+	}
+	return false, nil
+}
+
+// isEncryptionRelevantEntry centralises the per-Entry predicate.
+// Returns false for:
+//
+//   - EntryConfChange / EntryConfChangeV2 — never carry encryption
+//     FSM payloads.
+//   - Entries whose Data does not decode as a valid proposal
+//     envelope (zero-length leadership bumps, malformed entries).
+//   - Envelopes wrapping an empty FSM payload.
+//
+// For valid envelopes, returns IsEncryptionRelevantOpcode(payload[0])
+// where payload is the post-envelope-strip FSM byte sequence. The
+// engine wraps every EntryNormal in a 9-byte proposal envelope
+// (1 byte version + 8 byte proposal ID) via encodeProposalEnvelope
+// at propose time, so the raw Data[0] is always the envelope
+// version 0x01 — NEVER the FSM opcode. A scanner that misreads
+// the layout would return false for every entry in a real Raft
+// log, silently defeating the §9.1 ErrSidecarBehindRaftLog guard.
+func isEncryptionRelevantEntry(ent raftpb.Entry) bool {
+	if ent.Type != raftpb.EntryNormal {
+		return false
+	}
+	_, payload, ok := decodeProposalEnvelope(ent.Data)
+	if !ok || len(payload) == 0 {
+		return false
+	}
+	return encryption.IsEncryptionRelevantOpcode(payload[0])
+}
+
+// EncryptionScanner returns the engine's
+// encryption.EncryptionRelevantScanner implementation. The
+// returned value is safe to call after Open() has populated
+// MemoryStorage from the on-disk WAL + snapshot, and only after
+// — calling it before Open() returns a nil-storage error from
+// HasEncryptionRelevantEntryInRange.
+//
+// Used by main.go's startup-guard phase (Stage 6C-2d) to invoke
+// encryption.GuardSidecarBehindRaftLog against the engine's
+// applied index and the sidecar's raft_applied_index.
+//
+// Nil-receiver-safe: a nil *Engine returns a zero-value
+// encryptionScanner whose HasEncryptionRelevantEntryInRange will
+// surface the nil-storage error. This matches the rest of the
+// package's defensive nil-receiver posture (Status() / AppliedIndex()
+// return zero values on nil receivers) and lets call sites in
+// the lifecycle that may forward a maybe-nil engine pointer
+// avoid a panic during startup/refusal triage.
+func (e *Engine) EncryptionScanner() encryption.EncryptionRelevantScanner {
+	if e == nil {
+		return &encryptionScanner{}
+	}
+	return &encryptionScanner{storage: e.storage}
+}
diff --git a/internal/raftengine/etcd/encryption_scanner_test.go b/internal/raftengine/etcd/encryption_scanner_test.go
