fix(797): the BOM guard was fail-open wherever xxd is not installed (#798)

Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

.claude/hooks/pretooluse-bom-guard.sh

@@ -66,7 +66,11 @@ while IFS= read -r f; do
   esac
   p="$root/$f"
   [ -f "$p" ] || continue
-  if [ "$(head -c3 "$p" 2>/dev/null | xxd -p 2>/dev/null)" = "efbbbf" ]; then
+  # `od`, NOT `xxd`. `xxd` ships with vim and is absent on plain Linux hosts including this repo's
+  # CI runner, where the command substitution yielded empty, never equalled `efbbbf`, and this guard
+  # therefore passed every BOM in silence. It has been fail-open on any host without vim since it
+  # was written. `od -A n -t x1 -N 3` is POSIX and produces byte-identical output on macOS and Linux.
+  if [ "$(od -A n -t x1 -N 3 < "$p" 2>/dev/null | tr -d ' \n')" = "efbbbf" ]; then
     bad="${bad}  ${f}"$'\n'
   fi
 done < /tmp/.bom-guard-files.$$

docs/contributing.md

@@ -117,7 +117,7 @@ talks exclusively to the REST API. Every former Blazor route now 302-redirects t
   `io.open(..., encoding='utf-8-sig')` *writes one back*, and sed/perl round-trips keep it. This cost
   two sessions a red CI job on 2026-07-17 alone (PR #405, 6 files; PR #402, 19), which is what the
   guard is for. To check by hand:
-  `for f in $(git diff --name-only origin/main...HEAD --cached -- '*.cs'; git diff --name-only -- '*.cs') ; do head -c3 "$f" | xxd -p | grep -q '^efbbbf' && echo "BOM: $f"; done` (covers branch + staged + dirty — a brand-new staged file is absent from `origin/main...HEAD`)
+  `for f in $(git diff --name-only origin/main...HEAD --cached -- '*.cs'; git diff --name-only -- '*.cs') ; do [ "$(od -A n -t x1 -N 3 < "$f" | tr -d ' \n')" = efbbbf ] && echo "BOM: $f"; done` (covers branch + staged + dirty — a brand-new staged file is absent from `origin/main...HEAD`)
   (note `dotnet format --include` needs **bash** — `mapfile` is bash-only, and under zsh the file list
   comes out empty, which looks exactly like the tool silently doing nothing). (A one-time repo-wide normalization
   is a separate, unmade decision; the touched-file rule is the standing one.)

docs/decisions/README.md

@@ -103,7 +103,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
 | `media.remote-stream-probe` | `ValidatePlayoutItemPath` probes the Plex/Jellyfin/Emby remote-stream URL via `IRemoteStreamProber` before returning it; only a redirected 404 fails closed (`PlayoutItemNotAvailableFromMediaServer`), everything else fails open, and there is no toggle. | 2026-07-19 | [link](records/media/remote-stream-probe.md) |
 | `media.remote-stream-probe-externaljson` | External-JSON playout channels' `StreamRemotely` now probes the remote-stream URL through the same `IRemoteStreamProber` seam as the generated-playout path, closing the #473 scope gap for a channel kind with no DB `PlayoutItem` rows. | 2026-07-20 | [link](records/media/remote-stream-probe-externaljson.md) |
 | `media.source-mgmt-write-api` | Media-source management (local/Plex/Jellyfin/Emby) is a REST write API + SPA under `/app/libraries/*`, wrapping existing MediatR commands 1:1 with no new commands or DB migration; connection GETs never leak a stored `apiKey`, and each PUT-replace family's identity contract is documented per-family (not assumed uniform). | 2026-07-11 | [link](records/media/source-mgmt-write-api.md) |
-| `process.bom-format-detection-recipe` | Before any push touching `.cs`, detect BOMs with the `xxd` byte check and verify the format gate with `dotnet format --include` run under `bash -c`, never bare zsh. | 2026-07-21 | [link](records/process/bom-format-detection-recipe.md) |
+| `process.bom-format-detection-recipe` | Before any push touching `.cs`, detect BOMs with the `od -A n -t x1 -N 3` byte check and verify the format gate with `dotnet format --include` run under `bash -c`, never bare zsh. NOT `xxd`: it ships with vim and is absent on plain Linux hosts including this repo's CI runner, where the substitution yields empty, never matches, and the check reports all-clean — the same all-clean-detector failure this record was written about, in the detector it prescribed. | 2026-07-21 | [link](records/process/bom-format-detection-recipe.md) |
 | `process.branch-off-feature-branch` | To fix work on an unmerged feature branch, branch off that branch and land by fast-forward push — and after creating a worktree, drive the first Edit/Read from ITS absolute paths and `git status` it before building. | 2026-07-21 | [link](records/process/branch-off-feature-branch.md) |
 | `process.build-concurrency-limits` | Run at most 3–4 concurrent dotnet/npm builds on this Mac, gate launches on FREE RAM rather than CPU load, and never set `ETV_UPDATE_GOLDENS` / `ETV_UPDATE_PLAYOUT_GOLDENS`. | 2026-07-21 | [link](records/process/build-concurrency-limits.md) |
 | `process.codex-cheap-worker-launch` | For bounded tool-bearing selector/recon work, launch a Codex worker with `codex exec -m gpt-5.4-mini -c model_reasoning_effort=low -s read-only`; `spawn_agent` buys parallelism but no cost savings. | 2026-07-21 | [link](records/process/codex-cheap-worker-launch.md) |

docs/decisions/records/process/bom-format-detection-recipe.md

@@ -1,11 +1,11 @@
 ---
 key: process.bom-format-detection-recipe
-title: '2026-07-21 — BOM/format pre-push detection: use the `xxd` loop, and run `dotnet format --include` under `bash -c` (#542)'
+title: '2026-07-21 — BOM/format pre-push detection: use the `od` byte check, and run `dotnet format --include` under `bash -c` (#542, detector corrected #797)'
 status: active
 since: '2026-07-21'
 supersedes: none
 superseded-by: none
-rule: Before any push touching `.cs`, detect BOMs with the `xxd` byte check and verify the format gate with `dotnet format --include` run under `bash -c`, never bare zsh.
+rule: 'Before any push touching `.cs`, detect BOMs with the `od -A n -t x1 -N 3` byte check and verify the format gate with `dotnet format --include` run under `bash -c`, never bare zsh. NOT `xxd`: it ships with vim and is absent on plain Linux hosts including this repo''s CI runner, where the substitution yields empty, never matches, and the check reports all-clean — the same all-clean-detector failure this record was written about, in the detector it prescribed.'
 signals: 'UTF-8 BOM · `efbbbf` · `dotnet format --verify-no-changes` · `--include` · `mapfile` · zsh vs bash · #311 format gate · detector verification · paths: `ErsatzTV.sln`, `.editorconfig` · issues: #542, #311, #70, PR #402, PR #405'
 mechanics: Local pre-push shell; CI's format job recipe (`ci.format-gate-folder-mode`). The BOM *policy* is `release.format-as-you-touch-rebase`; this record is the mechanics only.
 ---
@@ -13,11 +13,25 @@ mechanics: Local pre-push shell; CI's format job recipe (`ci.format-gate-folder-
 Detection loop, verbatim:
 
 ```bash
-for f in $(git diff --name-only origin/main...HEAD -- '*.cs'); do head -c3 "$f" | xxd -p | grep -q "^efbbbf" && echo "BOM: $f"; done
+for f in $(git diff --name-only origin/main...HEAD -- '*.cs'); do [ "$(od -A n -t x1 -N 3 < "$f" | tr -d ' \n')" = efbbbf ] && echo "BOM: $f"; done
 ```
 
-**Verify your detector.** An `od -An -c | grep '357 273 277'` check reported all-clean while 19 files
-were actually dirty. A detector that can only say "ok" is worse than none — use the `xxd` form or read
+**The prescribed detector was itself an all-clean detector, corrected 2026-08-14 (#797).** This
+record was written because a detector that can only say "ok" is worse than none. It then prescribed
+`xxd -p`, and **`xxd` ships with vim and is absent on plain Linux hosts, including this repo's CI
+runner** — where the substitution yields the empty string, never matches `efbbbf`, and the loop
+prints nothing for a tree full of BOMs. The rule failed its own test, in the recipe it recommends,
+for three weeks.
+
+Read the original warning precisely, because it is still right and it is not about `od`: the form
+that reported all-clean over 19 dirty files was `od -An -c | grep '357 273 277'` — octal character
+output, whose spacing and escaping vary. The replacement is `od -A n -t x1 -N 3`, which emits
+hexadecimal bytes. That is a different invocation of the same tool, and it was measured across 17
+inputs (empty, sub-3-byte, exactly the BOM, BOM+NUL, binary, UTF-16 BOM, unreadable, 20 MB, awkward
+filenames) on BSD `od`, GNU `od`, and inside the actual CI runner image: identical in every cell.
+`tr -d ' \n'` is load-bearing — BSD pads to a fixed column width and GNU does not.
+
+**Verify your detector** — use the form above, or read
 the bytes directly, and distrust a clean result you didn't prove can go dirty.
 
 **`dotnet format --include` DOES work here** — an earlier note claiming it silently no-ops was WRONG.

docs/guard-inventory.md

@@ -82,7 +82,7 @@ all.
 | `.claude/hooks/pretooluse-agent-model.sh` | an Agent dispatch naming no model (asks) | GUARD | NONE | — |
 | `.claude/hooks/pretooluse-agent-ram.sh` | an Agent dispatch under 10% free RAM | GUARD | NONE | — |
 | `.claude/hooks/pretooluse-bash-guard.sh` | a Bash call setting `ETV_UPDATE_GOLDENS` | GUARD | NONE | — |
-| `.claude/hooks/pretooluse-bom-guard.sh` | a commit/push carrying a BOM in a touched `.cs` | GUARD | NONE | — |
+| `.claude/hooks/pretooluse-bom-guard.sh` | a commit/push carrying a BOM in a touched `.cs` | GUARD | MUTATION | `test_bom_guard_detection.py::test_DISARMING_the_BOM_comparison_stops_detection` |
 | `.claude/hooks/pretooluse-merge-consent.sh` | a PR merge without derived consent | GUARD | BEHAVIOUR-ONLY | `test_merge_consent_exemption.py::test_protected_path_on_a_LATER_page_is_still_seen` |
 | `.claude/hooks/pretooluse-nav-guard.sh` | a browser navigate to a streaming URL | GUARD | NONE | — |
 | `.claude/hooks/pretooluse-worktree-guard.sh` | a commit/merge in a foreign worktree | GUARD | NONE | — |
@@ -105,6 +105,7 @@ all.
 | `scripts/post-review-verdict.sh` | nothing (writes the verdict status) | GUARD | BEHAVIOUR-ONLY | `test_post_review_verdict.py::test_never_retargets_the_verdict_at_the_new_head` |
 | `scripts/pr-changed-files.sh` | the verdict exemption, on an incomplete enumeration | GUARD | BEHAVIOUR-ONLY | `test_pr_changed_files.py::test_a_SHORT_page_does_not_end_the_enumeration` |
 | `scripts/update-openapi.sh` | nothing (regenerates the spec) | TOOLING | NONE | — |
+| `scripts/tests/test_bom_guard_detection.py` | the `script-tests` job | PROOF | NONE | — |
 | `scripts/tests/test_build_catalog.py` | the `script-tests` job | PROOF | NONE | — |
 | `scripts/tests/test_check_review_verdict.py` | the `script-tests` job | PROOF | NONE | — |
 | `scripts/tests/test_ci_dropped_step_guard.py` | the `script-tests` job | PROOF | NONE | — |
@@ -123,8 +124,8 @@ all.
 
 ## What the numbers say
 
-31 guards, 4 tooling scripts, 12 proof files. **4 guards carry a mutation proof; 6 are
-behaviour-only; 21 have none.** These figures are asserted against the table by
+31 guards, 4 tooling scripts, 13 proof files. **5 guards carry a mutation proof; 6 are
+behaviour-only; 20 have none.** These figures are asserted against the table by
 `test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against
 a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it,
 which is the duplication family this change argues against. Both cold reviewers found the error
@@ -170,14 +171,25 @@ dangerous direction — a token that a reviewer means as `BLOCKED` silently read
 approval, which writes a green `review-verdict/h10`. A token added to one script and not the other
 is untested by either, which is the whole of #788.
 
-The 21 include every `PreToolUse` hook except merge-consent — the same population #776 found we
+**That unproven set carries no number here on purpose.** It restated the count, drifted the moment
+the BOM guard was regraded, and `test_the_summary_counts_match_the_table` cannot see it — the parser
+checks the formatted summary sentence and nothing else. A second hand-maintained copy of a number is
+the duplication family this file argues against, so the copy is removed rather than corrected: the
+set is every `PreToolUse` hook except merge-consent and the BOM guard, and the count is in the
+summary above. It is also the population #776 found we
 cannot even observe firing. A guard that is neither proven nor observable is an assertion about the
 past.
 
 The gaps are not uniform in cost, and the ranking that matters is *what a silent failure would let
 through*, not test count:
 
-1. `pretooluse-bom-guard.sh` — guards a defect that has recurred three times (#311, #402, #405).
+1. ~~`pretooluse-bom-guard.sh`~~ — **now proven, and it was fail-open the whole time.** Ranked
+   first here because the defect it guards has recurred three times (#311, #402, #405); that
+   ranking turned out to be right for a worse reason than intended. It detected a BOM with
+   `xxd -p`, and `xxd` ships with vim and is **absent on the Linux CI runner**, so the comparison
+   never matched and every BOM was allowed in silence. `od` now, with a clause-level mutation proof.
+   The lesson for the rows below: an unproven guard is not merely untested, it is a guard whose
+   *current* behaviour nobody has established.
 2. `pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh` — a two-file mechanism guarding
    #289, where a regression in either half is invisible and the two halves have never been tested
    together.

scripts/tests/test_bom_guard_detection.py

@@ -0,0 +1,184 @@
+"""`pretooluse-bom-guard.sh` actually detects a BOM — including where `xxd` does not exist.
+
+The guard compared `head -c3 <file> | xxd -p` against `efbbbf`. **`xxd` ships with vim and is absent
+on plain Linux hosts, including this repo's CI runner** (verified there directly). On such a host the
+command substitution yields the empty string, never equals `efbbbf`, and the guard allows every BOM
+in silence. It had been fail-open on any machine without vim since it was written, and nothing
+noticed because `docs/guard-inventory.md` graded it `NONE` — no proof it could go red.
+
+It surfaced only when an unrelated change (ersatztv#776) added an assertion that this hook must
+reach a real decision, and that assertion ran on Linux. Every prior review of this guard ran on
+macOS, where `xxd` exists — the *environment* was a sampled population, and the sample was
+unanimous.
+
+So this file exists to make the guard's detection load-bearing rather than assumed:
+
+  * it drives the real hook end to end, through the real payload shape;
+  * it drives it with `xxd` REMOVED FROM PATH, which is the regression;
+  * and it performs a clause-level mutation — the comparison is disarmed in an isolated copy and the
+    guard must stop detecting — which is what `testing.guard-ships-with-mutation-proof` asks for and
+    what this guard has never had.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import shutil
+import subprocess
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+HOOK = REPO_ROOT / ".claude" / "hooks" / "pretooluse-bom-guard.sh"
+
+BOM = b"\xef\xbb\xbf"
+
+
+def _git(cwd: Path, *args: str) -> None:
+    subprocess.run(
+        ["git", *args], cwd=str(cwd), check=True, capture_output=True,
+        env={**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@e",
+             "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@e"},
+    )
+
+
+def _repo_with(tmp_path: Path, name: str, content: bytes) -> Path:
+    # The guard scopes itself to repositories whose toplevel path matches `*ersatztv*`, so the
+    # directory name is load-bearing, not decoration.
+    repo = tmp_path / "ersatztv-scratch"
+    repo.mkdir()
+    _git(repo, "init", "-q", ".")
+    (repo / name).write_bytes(content)
+    _git(repo, "add", name)
+    return repo
+
+
+def _run(hook: Path, repo: Path, env: dict) -> tuple[int, bytes]:
+    payload = json.dumps({
+        "session_id": "s", "hook_event_name": "PreToolUse", "tool_name": "Bash",
+        "cwd": str(repo), "tool_input": {"command": "git commit -m x"},
+    })
+    p = subprocess.run(["bash", str(hook)], input=payload.encode(), capture_output=True,
+                       cwd=str(repo), env=env, timeout=60)
+    return p.returncode, p.stdout
+
+
+def _path_without_xxd(tmp_path: Path) -> dict:
+    """A PATH that resolves everything the hook needs EXCEPT `xxd`.
+
+    Rebuilding PATH from symlinks rather than just dropping directories, because `xxd` usually lives
+    in the same directory as `git` and `od`; removing that directory would starve the hook of tools
+    it legitimately needs and the test would pass for the wrong reason.
+    """
+    import re
+
+    # DERIVED from the hook, not hand-listed. The hand-listed version symlinked six tools the hook
+    # never calls and omitted `tail`, which it does call for `cd`-detection — so a payload whose cwd
+    # differs from the repo took a `tail: command not found` path and the guard ALLOWED a BOM for a
+    # reason that had nothing to do with xxd. A stand-in PATH that starves the subject is a test
+    # passing for the wrong reason, which is the whole subject of this file.
+    words = set(re.findall(r"\b([a-z][a-z0-9_-]*)\b", HOOK.read_text()))
+    bindir = tmp_path / "nobin"
+    bindir.mkdir()
+    linked = []
+    for tool in sorted(words - {"xxd"}):
+        found = shutil.which(tool)
+        if found and not (bindir / tool).exists():
+            (bindir / tool).symlink_to(found)
+            linked.append(tool)
+    # `head` is deliberately NOT here: the `od` change removed the only `head` call, and asserting a
+    # tool the hook no longer uses is how a stand-in PATH drifts from its subject.
+    for required in ("bash", "git", "od", "tr", "sort", "tail"):
+        assert shutil.which(required, path=str(bindir)), (
+            f"the stand-in PATH lost `{required}`, which the hook needs — the test would then pass "
+            "because the guard was starved, not because it detected anything"
+        )
+    assert not shutil.which("xxd", path=str(bindir)), "the stand-in PATH still resolves xxd"
+    return {**os.environ, "PATH": str(bindir)}
+
+
+# ------------------------------------------------------------------------------------------------
+# ANTI-VACUITY — if the fixture stops producing a BOM file, every assertion below is meaningless.
+# ------------------------------------------------------------------------------------------------
+
+
+def test_the_fixture_really_stages_a_BOM(tmp_path):
+    repo = _repo_with(tmp_path, "Bad.cs", BOM + b"class A {}\n")
+    assert (repo / "Bad.cs").read_bytes()[:3] == BOM
+    staged = subprocess.run(["git", "diff", "--name-only", "--cached"], cwd=str(repo),
+                            capture_output=True, text=True).stdout.split()
+    assert staged == ["Bad.cs"], f"nothing was staged, so the guard would have nothing to read: {staged}"
+
+
+# ------------------------------------------------------------------------------------------------
+# THE GUARD DECIDES
+# ------------------------------------------------------------------------------------------------
+
+
+def test_a_staged_BOM_is_DENIED(tmp_path):
+    repo = _repo_with(tmp_path, "Bad.cs", BOM + b"class A {}\n")
+    rc, out = _run(HOOK, repo, dict(os.environ))
+    assert rc == 0, "the hook communicates by printing, and must always exit 0"
+    assert b'"permissionDecision": "deny"' in out or b'"permissionDecision":"deny"' in out, (
+        f"a staged BOM-carrying .cs was not denied: {out!r}"
+    )
+
+
+def test_a_staged_BOM_is_DENIED_when_xxd_DOES_NOT_EXIST(tmp_path):
+    """THE REGRESSION. This is the case that was silently allowed on every host without vim."""
+    repo = _repo_with(tmp_path, "Bad.cs", BOM + b"class A {}\n")
+    rc, out = _run(HOOK, repo, _path_without_xxd(tmp_path))
+    assert rc == 0
+    assert b'"permissionDecision": "deny"' in out or b'"permissionDecision":"deny"' in out, (
+        "with `xxd` absent the guard allowed a BOM. That is the fail-open this file exists to "
+        f"prevent, and it is the state every non-vim host was in: {out!r}"
+    )
+
+
+def test_a_clean_file_is_ALLOWED(tmp_path):
+    """The negative control. A guard that denies everything would pass the tests above."""
+    repo = _repo_with(tmp_path, "Good.cs", b"class A {}\n")
+    rc, out = _run(HOOK, repo, _path_without_xxd(tmp_path))
+    assert rc == 0
+    assert out == b"", f"a BOM-free file was not allowed silently: {out!r}"
+
+
+# ------------------------------------------------------------------------------------------------
+# THE MUTATION PROOF — disarm the comparison alone, detection must stop
+# ------------------------------------------------------------------------------------------------
+
+
+def test_DISARMING_the_BOM_comparison_stops_detection(tmp_path):
+    """`testing.guard-ships-with-mutation-proof`, which this guard has never carried.
+
+    The clause is disarmed in an isolated copy — the comparison is pointed at a byte sequence no
+    file starts with — and the guard must go quiet. If it still denies, the deny is coming from
+    somewhere other than the clause the guard is supposed to hang on, and the tests above prove
+    nothing about it.
+    """
+    text = HOOK.read_text()
+    marker = '= "efbbbf" ]; then'
+    assert marker in text, (
+        "the BOM comparison has moved; retarget this mutation rather than loosening it — a mutation "
+        "that silently stops mutating is the failure this file is about"
+    )
+    mutated = tmp_path / "mutated-bom-guard.sh"
+    mutated.write_text(text.replace(marker, '= "deadbeef" ]; then', 1))
+
+    repo = _repo_with(tmp_path, "Bad.cs", BOM + b"class A {}\n")
+
+    # POSITIVE CONTROL FIRST. Without it this test passes when the guard detects NOTHING AT ALL —
+    # verified: run against the pre-fix hook on Linux, where xxd is absent, and "the mutant is
+    # silent" was trivially true. A green mutation proof over a dead check is the exact failure
+    # `guard-ships-with-mutation-proof` exists to stop, and the inventory row cites THIS function.
+    rc_live, out_live = _run(HOOK, repo, dict(os.environ))
+    assert rc_live == 0 and b"deny" in out_live, (
+        "the UNMUTATED guard did not detect the BOM, so 'the mutant is silent' proves nothing about "
+        f"the clause: {out_live!r}"
+    )
+
+    rc, out = _run(mutated, repo, dict(os.environ))
+    assert rc == 0
+    assert out == b"", (
+        f"disarming the BOM comparison did not stop detection, so it is not load-bearing: {out!r}"
+    )