Files
ersatztv/scripts/tests/test_bom_guard_detection.py
T
8fd9eae0bf
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 12s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 30s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m6s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m12s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m3s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
fix(891): a sourced path is code, so every hook resolves it from its own tree (#903)
Every hook under `.claude/hooks/` assigned `ETV_HOOK_FIRE_LIB` from `${CLAUDE_PROJECT_DIR:-<self>}`
and then `. `-SOURCED it. Sourcing is execution, so a file of that name in an env-designated tree ran
as code inside the hook before stdin was read and before it could decide anything. Measured on the
merge gate before #858 fixed that one hook: a decoy tree's copy printed an `allow` and exited 0.

Reachable without an attacker, because husky is a different launcher: `.husky/pre-push` invokes
`./.claude/hooks/…` relative to the PUSHED tree, independent of the variable, so a push from one
worktree while the environment names another sources the other tree's code into a gate.

Sweeps the remaining twelve hooks together (population derived from `git ls-files`), reconciles the
second resolution inside `scripts/hook-fire-log.sh` itself, and requires the root to OWN the sink
(`-ef`, not `-e`). The static guard pins the preamble BYTE-FOR-BYTE — a withdrawal, after a lexical
rule was defeated by five successive shapes.

Also pins two arms of the checker that were unsubsumed AND unpinned: the begin call's presence and
its missing stdout-mode token. `…_LOSES_its_instrumentation_…` looked like their proof and was not —
it asserts only that the fault list is non-empty, and a stripped hook trips four arms, so deleting
either left the suite green.

fixes #891
refs #858, #859, #776

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
2026-08-30 21:41:19 +00:00

210 lines
9.8 KiB
Python

"""`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"
)
# REPO-SHAPED, so the mutant differs from its subject in ONE clause and not two. The hooks
# self-locate `scripts/hook-fire-log.sh` from `${BASH_SOURCE[0]}` (ersatztv#891,
# `process.hook-resolves-inputs-from-repo-root`); a copy dropped at the root of `tmp_path`
# resolves two directories above a temporary filename, finds no sink, and silently runs
# UNINSTRUMENTED while the subject runs instrumented. Measured: 2 fire records for the subject,
# 0 for a tmp-root copy. The confound points toward a false RED here rather than a false green,
# but "differs in one clause" is the property this file's whole argument rests on.
mutant_root = tmp_path / "mutant-root-bom-guard"
(mutant_root / ".claude" / "hooks").mkdir(parents=True, exist_ok=True)
(mutant_root / "scripts").mkdir(parents=True, exist_ok=True)
shutil.copy2(REPO_ROOT / "scripts" / "hook-fire-log.sh", mutant_root / "scripts" / "hook-fire-log.sh")
mutated = mutant_root / ".claude" / "hooks" / HOOK.name
mutated.write_text(text.replace(marker, '= "deadbeef" ]; then', 1))
mutated.chmod(0o755)
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}"