test(cache): guard the body-cache eligibility gate against deny-list regressions (#1146) (#4742)

`full_object_plaintext_len` decides whether a body-cache hook hit may serve
bytes in place of the erasure read. It is a fail-closed allow-list: it excludes
every read whose `ReadPlan::build` applies some other transform (ranged/part,
raw/data-movement, restore, encrypted, remote) with an early `return None`,
then returns a `Some(..)` length only for the whole-plaintext cases. A newly
added `ReadPlan` branch that nobody teaches this gate about falls through to
`None` and safely bypasses the cache. Flip it to a deny-list and the same new
branch silently serves bytes in the wrong representation — the backlog#1108 /
#1109 / #1146 class of bug.

The existing unit and e2e tests only cover the branches that exist today. This
adds `scripts/check_body_cache_whitelist.sh`, a structural guard wired into
pre-commit / pre-pr / dev-check and CI, that asserts every exclusion predicate
and a `return None` still precede the first `Some(..)`. Reordering a predicate,
dropping one, moving the positive return ahead of the gate, or renaming/removing
the function all fail; wording, formatting, and adding a new exclusion in the
same gate do not. Mutation-tested against all four regression shapes.

This machine-enforces the structural invariant that backlog#1146 was kept open
to guard by hand.

Co-authored-by: heihutu <heihutu@gmail.com>

.config/make/lint-fmt.mak

@@ -55,6 +55,11 @@ extension-schema-check: ## Check extension-schema stays a lightweight contract c
 	@echo "🧩 Checking extension schema boundaries..."
 	./scripts/check_extension_schema_boundaries.sh
 
+.PHONY: body-cache-whitelist-check
+body-cache-whitelist-check: ## Check the body-cache eligibility gate stays a fail-closed allow-list
+	@echo "🧱 Checking body-cache whitelist guard..."
+	./scripts/check_body_cache_whitelist.sh
+
 .PHONY: compilation-check
 compilation-check: core-deps ## Run compilation check
 	@echo "🔨 Running compilation check..."

.config/make/pre-commit.mak

@@ -14,13 +14,13 @@ doc-paths-check: ## Check that instruction/architecture docs reference existing
 	./scripts/check_doc_paths.sh
 
 .PHONY: pre-commit
-pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check quick-check ## Run fast pre-commit checks without clippy/full tests
+pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check doc-paths-check quick-check ## Run fast pre-commit checks without clippy/full tests
 	@echo "✅ All pre-commit checks passed!"
 
 .PHONY: pre-pr
-pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check clippy-check test ## Run full pre-PR checks with clippy and tests
+pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check doc-paths-check clippy-check test ## Run full pre-PR checks with clippy and tests
 	@echo "✅ All pre-PR checks passed!"
 
 .PHONY: dev-check
-dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check doc-paths-check quick-check ## Run fast local development checks
+dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check doc-paths-check quick-check ## Run fast local development checks
 	@echo "✅ Fast development checks passed!"

.github/workflows/ci.yml

@@ -131,6 +131,9 @@ jobs:
       - name: Check extension schema boundaries
         run: ./scripts/check_extension_schema_boundaries.sh
 
+      - name: Check body-cache whitelist guard
+        run: ./scripts/check_body_cache_whitelist.sh
+
   test-and-lint:
     name: Test and Lint
     if: github.event_name != 'pull_request' || github.event.action != 'closed'

crates/ecstore/src/set_disk/ops/object.rs

@@ -48,6 +48,11 @@ use crate::object_api::GetObjectBodySource;
 /// Compressed objects ARE eligible: `ReadTransform::Compressed` returns the full
 /// plaintext and rewrites `object_info.size` to the decompressed length, which
 /// the caller must replicate with the returned length (backlog#1109).
+///
+/// The fail-closed shape here is enforced by `scripts/check_body_cache_whitelist.sh`
+/// (backlog#1146): the guard requires every exclusion predicate and a `return
+/// None` to precede the first `Some(..)`, so a refactor to a deny-list fails CI.
+/// If you rename or move this function, update that script.
 fn full_object_plaintext_len(range: &Option<HTTPRangeSpec>, opts: &ObjectOptions, object_info: &ObjectInfo) -> Option<i64> {
     if range.is_some()
         || opts.part_number.is_some()

scripts/check_body_cache_whitelist.sh

@@ -0,0 +1,96 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+# Guard: the app-layer body cache eligibility gate must stay a fail-closed
+# allow-list.
+#
+# `full_object_plaintext_len` (crates/ecstore/src/set_disk/ops/object.rs)
+# decides whether a body-cache hook hit may serve bytes IN PLACE of the erasure
+# read. A hit bypasses `ReadPlan`/`ReadTransform` entirely, so it is sound only
+# where the normal read path would return that exact plaintext under that exact
+# `object_info.size`. The function encodes this as a positive allow-list: it
+# excludes every read whose `ReadPlan::build` applies some other transform
+# (ranged/part reads, raw/data-movement reads, restore reads, encrypted and
+# remote objects) with an early `return None`, THEN — and only then — returns a
+# `Some(..)` length for the whole-plaintext cases.
+#
+# That ordering is the whole invariant. A newly added `ReadPlan` branch that
+# nobody teaches this gate about falls through to `None` and safely bypasses the
+# cache. Flip the structure to a deny-list — default `Some(..)`, subtract known
+# bad cases — and the same new branch silently serves bytes in the WRONG
+# representation. That is exactly the backlog#1108 / #1109 / #1146 class of bug
+# (an erasure read returning a decrypted/decompressed body under a stored-byte
+# `size`, or vice versa). Unit and e2e tests only cover the branches that exist
+# today; this guard is what keeps a future refactor from regressing the shape.
+#
+# The check is deliberately structural, not a byte-for-byte snapshot: it asserts
+# that each exclusion predicate and a `return None` appear BEFORE the first
+# `Some(` in the function body. Reordering a predicate, dropping one, or moving
+# the positive return ahead of the gate all fail; harmless edits (wording,
+# adding a new exclusion in the same gate, formatting) do not.
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ROOT_DIR="${CHECK_BODY_CACHE_WHITELIST_ROOT:-$(cd "${SCRIPT_DIR}/.." && pwd)}"
+
+TARGET="crates/ecstore/src/set_disk/ops/object.rs"
+FN="full_object_plaintext_len"
+
+# Exclusion predicates that MUST be checked (returning None) before any positive
+# Some(..) return. Order-independent; presence-and-position is what matters.
+REQUIRED_PREDICATES=(
+  "range.is_some()"
+  "opts.part_number.is_some()"
+  "opts.raw_data_movement_read"
+  "opts.data_movement"
+  "restore_request_active(opts)"
+  "object_info.is_encrypted()"
+  "object_info.is_remote()"
+)
+
+fail() {
+  printf 'Body-cache whitelist guard failed: %s\n' "$1" >&2
+  exit 1
+}
+
+FILE="${ROOT_DIR}/${TARGET}"
+[[ -f "$FILE" ]] || fail "${TARGET} is missing (did the file move? update this guard)"
+
+# Extract the function body: from the `fn <FN>(` signature line to the first
+# closing brace in column 0 (the function is a free fn at module scope). awk
+# prints the body with 1-based relative line numbers so we can reason about
+# ordering without absolute offsets.
+BODY="$(awk -v fn="fn ${FN}(" '
+  index($0, fn) == 1 { inside = 1; n = 0 }
+  inside {
+    n++
+    print n "\t" $0
+    if (started && $0 ~ /^}/) { exit }
+    if ($0 ~ /\{[[:space:]]*$/) started = 1
+  }
+' "$FILE")"
+
+[[ -n "$BODY" ]] || fail "fn ${FN} not found in ${TARGET} (renamed or removed? update this guard)"
+
+# Line number (relative to the body) of the first positive `Some(` return. The
+# allow-list's positive returns must all sit AFTER the exclusion gate, so this
+# is the boundary every exclusion must precede.
+some_line="$(awk -F'\t' '$2 ~ /Some\(/ { print $1; exit }' <<<"$BODY")"
+[[ -n "$some_line" ]] || fail "fn ${FN} has no Some(..) return — the allow-list positive branch vanished; review by hand"
+
+# A `return None` must gate the excluded reads before the first positive return.
+none_line="$(awk -F'\t' '$2 ~ /return None/ { print $1; exit }' <<<"$BODY")"
+[[ -n "$none_line" ]] || fail "fn ${FN} has no \`return None\` before its positive branch — fail-closed gate is gone"
+[[ "$none_line" -lt "$some_line" ]] || fail "\`return None\` gate no longer precedes the first Some(..) — structure looks like a deny-list"
+
+# Every required exclusion predicate must appear, and appear before the first
+# positive Some(..). A missing or relocated predicate means some read that must
+# bypass the cache can now reach a cache hit.
+for pred in "${REQUIRED_PREDICATES[@]}"; do
+  pred_line="$(awk -F'\t' -v p="$pred" 'index($2, p) { print $1; exit }' <<<"$BODY")"
+  [[ -n "$pred_line" ]] || fail "exclusion predicate \`${pred}\` missing from fn ${FN} — a refused read can now hit the cache"
+  [[ "$pred_line" -lt "$some_line" ]] ||
+    fail "exclusion predicate \`${pred}\` no longer precedes the first Some(..) — it is not gating the allow-list"
+done
+
+echo "Body-cache whitelist guard passed."