From fa2dbb97249083f4b13d9d6e978e9dd69e2d3c58 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:37:14 +0100 Subject: [PATCH 1/2] fix(hygiene): close the rename blind spot in the blob guard's staged mode `git diff --cached --diff-filter=AM` reports a rename as a single R entry, so the AM filter dropped it entirely. An oversized blob already in the index could therefore be moved to a new path and sail past the pre-commit hook without check_one() ever being called on it. This was a real hole, not a theoretical one. Measured against a 5 MiB blob renamed with `git mv`: pre-fix hook: "blob hygiene: ok" rc=0 <- false green fixed hook: refuses the 5 MiB blob, rc=1 `--no-renames` decomposes the rename into D + A, and the A is examined like any other addition. The CI `--tree` mode was never affected -- it walks the whole tree rather than a diff -- and that is precisely why the gap was invisible: one of the two callers stayed correct, so nothing ever reported a problem. This is the drift the one-implementation-two-callers design was meant to prevent, and it appeared anyway because the two callers ask the index and the tree different questions. Controls run (4/4 correct): - 5 MiB blob renamed -> REFUSED (was admitted) - allowlisted fixture renamed out of it -> REFUSED - ordinary small file renamed -> admitted - fixture renamed within the allowlist -> admitted Also dates the "verified by mutant" cell in standards-alignment.md. It was an undated one-off manual battery presented as ongoing status -- the same shape as a step name asserting a comparison the script never makes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- docs/compliance/standards-alignment.md | 2 +- scripts/check-blob-hygiene.sh | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/compliance/standards-alignment.md b/docs/compliance/standards-alignment.md index b124fbf..ecbb3d0 100644 --- a/docs/compliance/standards-alignment.md +++ b/docs/compliance/standards-alignment.md @@ -54,7 +54,7 @@ Reference: `hyperpolymath/standards@main` (in particular | Expectation | Here | Status | |---|---|---| -| Large/dead blobs kept out | `scripts/check-blob-hygiene.sh` — one implementation, two callers: `.githooks/pre-commit` (local) and the CI repo-hygiene `Blob hygiene check` (binding on this repo). Primary rule is a 4 MiB size ceiling, not a path list; the six `data/MiSeq_SOP/run_[AB]/*.fastq.gz` fixtures are allowlisted | ✅ verified by mutant — five reintroduction attempts refused, two legitimate files admitted | +| Large/dead blobs kept out | `scripts/check-blob-hygiene.sh` — one implementation, two callers: `.githooks/pre-commit` (local) and the CI repo-hygiene `Blob hygiene check` (binding on this repo). Primary rule is a 4 MiB size ceiling, not a path list; the six `data/MiSeq_SOP/run_[AB]/*.fastq.gz` fixtures are allowlisted | ✅ verified by mutant **2026-09-21** — five reintroduction attempts refused, two legitimate files admitted. ⚠ a one-off manual battery, not an enforced control: the date is here so this cell cannot read as ongoing status. Re-run `scripts/check-blob-hygiene.sh` against fresh mutants after any change to its rules | | Diff/linguist markings | `.gitattributes` marks `*.fastq{,.gz}`, `*.fq{,.gz}`, `*.fasta`, `*.fa`, `*.sam`, `*.bam` binary `-diff linguist-generated=true` | ✅ hygiene only, not the gate | ## Branch conventions diff --git a/scripts/check-blob-hygiene.sh b/scripts/check-blob-hygiene.sh index 898b4b1..54708f3 100755 --- a/scripts/check-blob-hygiene.sh +++ b/scripts/check-blob-hygiene.sh @@ -94,7 +94,14 @@ case "$mode" in [ -n "$blob" ] || continue size="$(git cat-file -s "$blob")" check_one "$path" "$size" - done < <(git diff --cached --name-only --diff-filter=AM -z) + # --no-renames is load-bearing, not a tidy-up. With rename detection on, + # `git mv pool.fastq other.bin` is one R entry, and --diff-filter=AM drops + # it -- so an oversized blob already in the index can be moved past this + # hook without check_one() ever seeing it. --no-renames decomposes the + # rename into D + A, and the A is examined like any other addition. + # The --tree mode is immune (it walks the whole tree), which is exactly + # why the gap was invisible: CI stayed correct while the hook did not. + done < <(git diff --cached --no-renames --name-only --diff-filter=AM -z) ;; --tree) # Every tracked file at a ref, rather than a commit range. A range needs From 745ec01954bd5059dc782336ac0acdd7555f855b Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:50:30 +0100 Subject: [PATCH 2/2] fix(hygiene): make the blob guard fail when it examines nothing The guard could report success without looking at a single file. $ scripts/check-blob-hygiene.sh --tree refs/heads/no-such-branch fatal: Not a valid object name refs/heads/no-such-branch blob hygiene: ok rc=0 `git ls-tree` on an unresolvable ref writes to stderr and emits no paths. A CI `run:` step does not fail on stderr, so the loop body never executed and the script fell through to its success line. A typo in the ref would have produced a permanently green gate that inspected nothing -- and a tree of 337 files and a tree of 0 files printed byte-identical output. Three changes: - resolve the ref with `git rev-parse --verify "$ref^{tree}"` BEFORE walking it - count what was examined, and in --tree mode refuse when the count is zero - print the count, so a vacuous run is visible in the log rather than inferred No emptiness guard in --staged mode: a commit that only DELETES files legitimately stages zero additions, so zero is a real state there. The asymmetry is deliberate and commented. Controls (7 designed + 1 corrected): bad ref -> rc=2 FATAL (was rc=0 "ok") empty tree, valid object -> rc=2 FATAL (was rc=0 "ok") real tree, 337 files -> rc=0, count matches `git ls-tree | wc -l` nothing staged -> rc=0 (legal) oversized blob renamed -> rc=1 fixture renamed out of allowlist -> rc=1 uncompressed .fastq added -> rc=1 small file / fixture moved in-allowlist -> rc=0 This is the defect class the repo already had a note about -- a gate whose success is indistinguishable from never having run. It was shipped in the guard that was written to prevent exactly that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- scripts/check-blob-hygiene.sh | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/scripts/check-blob-hygiene.sh b/scripts/check-blob-hygiene.sh index 54708f3..715953c 100755 --- a/scripts/check-blob-hygiene.sh +++ b/scripts/check-blob-hygiene.sh @@ -83,6 +83,12 @@ check_one() { fi } +# How many files the guard actually looked at. Without this, a run that examined +# 337 files and a run that examined 0 produced byte-identical output ("blob +# hygiene: ok", rc=0) -- so a guard aimed at nothing reported exactly the same +# success as a guard that checked everything. +examined=0 + mode="${1:---staged}" case "$mode" in @@ -93,6 +99,7 @@ case "$mode" in blob="$(git ls-files -s -- "$path" | awk '{print $2}')" [ -n "$blob" ] || continue size="$(git cat-file -s "$blob")" + examined=$((examined + 1)) check_one "$path" "$size" # --no-renames is load-bearing, not a tidy-up. With rename detection on, # `git mv pool.fastq other.bin` is one R entry, and --diff-filter=AM drops @@ -113,12 +120,32 @@ case "$mode" in # paired with only ONE of the paths it is reachable under, which is # exactly how the duplicated pool went unnoticed. ref="${2:-HEAD}" + # Resolve the ref BEFORE walking it. `git ls-tree` on a bad ref writes + # "fatal: Not a valid object name" to STDERR and emits no paths -- and a + # CI `run:` step does not fail on stderr. The loop below then never + # executes and the script falls through to its success line, so a typo'd + # ref yields a permanently green gate that inspects nothing. Measured: + # `--tree refs/heads/no-such-branch` printed "blob hygiene: ok" rc=0. + git rev-parse --quiet --verify "$ref^{tree}" >/dev/null 2>&1 || { + echo "blob hygiene: FATAL -- '$ref' does not resolve to a tree" >&2 + exit 2 + } while IFS= read -r -d '' path; do blob="$(git rev-parse --quiet --verify "$ref:$path" 2>/dev/null || true)" [ -n "$blob" ] || continue size="$(git cat-file -s "$blob")" + examined=$((examined + 1)) check_one "$path" "$size" done < <(git ls-tree -r -z --name-only "$ref") + # Emptiness guard. A tree with zero tracked files is never a legitimate + # state for this repository, so it means the walk failed, not that the + # repo is clean. Two empty sets compare equal; this is what stops that + # from reading as a pass. (No such guard in --staged mode: a commit that + # only DELETES files legitimately stages zero additions.) + if [ "$examined" -eq 0 ]; then + echo "blob hygiene: FATAL -- examined 0 files at '$ref'; the guard checked nothing" >&2 + exit 2 + fi ;; *) echo "usage: $0 --staged | --tree [ref]" >&2 @@ -133,4 +160,4 @@ if [ "$violations" -gt 0 ]; then exit 1 fi -echo "blob hygiene: ok" +echo "blob hygiene: ok -- $examined file(s) examined, $violations violation(s)"