[HIGH] A08 Integrity: CRLF bots break bot_sha256, unbinding provenance and failing league-wide load

[stephschofield]

Summary

open_submission_pr copies the bot with text-mode IO, so a bot authored with CRLF line
endings is committed as LF. The capture-time hash and the committed bytes then disagree,
store.py silently restamps bot_sha256, the provenance token stops binding, and
load_submissions() raises ValueError — rejecting the whole league, not just that row.

Fails closed: an availability/integrity bug, not forgery or an authz bypass.

The chain

1. src/atv_bench/submit.py:154-155 — capture hashes the original bytes:

   data = Path(bot_path).read_bytes()
   bot_sha256 = hashlib.sha256(data).hexdigest()

   This hash is what the provenance token is signed against.

2. src/atv_bench/submit.py:367 — the copy into the PR is text-mode:

   (dest / "main.py").write_text(Path(bot_path).read_text())

   read_text() applies universal-newline translation (\r\n → \n) and write_text()
   writes back with os.linesep handling. On top of the newline bug, the bare
   read_text() here also has no encoding=, so it decodes with the locale
   codepage — the same class of defect PR fix(cli): survive Windows cp1252 consoles instead of crashing on status marks #29 fixed elsewhere.

3. src/atv_bench/store.py:324-326 — the trusted path re-hashes the committed bytes
   and, on mismatch, does not reject but silently overwrites:

   trusted_sha = hashlib.sha256(bot_bytes).hexdigest()
   if data.get("bot_sha256") != trusted_sha:
       data = {**data, "bot_sha256": trusted_sha}

4. src/atv_bench/store.py:339-355 — verify_provenance(...) is then called with
   bot_sha256=trusted_sha. The token was signed against the pre-translation hash, so
   it no longer binds and prov_res.ok is false:

   raise ValueError(f"submission {d.name!r} provenance does not verify: ...")

   Raised from load_submissions() → the leaderboard build fails league-wide.

Reproduction

import hashlib, tempfile, pathlib
d = pathlib.Path(tempfile.mkdtemp())
src = d / "bot.py"; src.write_bytes(b"# bot\r\nimport sys\r\nprint('hi')\r\n")
orig = hashlib.sha256(src.read_bytes()).hexdigest()          # submit.py:155
dest = d / "main.py"; dest.write_text(src.read_text())       # submit.py:367 verbatim
after = hashlib.sha256(dest.read_bytes()).hexdigest()        # store.py:324
assert orig != after

Three independent reproductions, all divergent (different test-bot content, hence
different digests — the divergence itself is the invariant):

Run	capture hash	committed hash
/atv-security scan (crlf-hash-29.png)	cd03fe07…	172014ed…
manual re-check	5df3efe8…	aeed901a…
this filing	c8c02e22…	4a74a803…

Impact

A contributor on Windows (or anyone with core.autocrlf/a CRLF editor) submits a
perfectly legitimate bot. It merges. The next leaderboard build raises ValueError and
every submission stops loading until the row is hand-repaired. Nothing is forged and
no authz is bypassed — but availability is league-wide.

Fix

Use binary IO for the copy, so the committed bytes are byte-identical to the hashed bytes:

(dest / "main.py").write_bytes(Path(bot_path).read_bytes())

This also removes the locale-codepage decode at that line as a side effect.

Consider additionally: rather than silently restamping at store.py:326, distinguish
"hash absent" (stamp) from "hash present but mismatched" (reject loudly, or skip the row
rather than failing the whole load) — a single bad row should not take the league down.

Test coverage to add

• CRLF bot round-trips through open_submission_pr with bot_sha256 unchanged.
• A CRLF submission with a provenance token loads without ValueError.
• Non-UTF-8-decodable bytes survive the copy unchanged (locale-codepage regression).

Provenance of this finding

Surfaced by the /atv-security scan of PR #29 (graded B · Critical 0 · High 1 ·
Medium 1 · Low 1), written up in §3 of docs/proof/pr-review-2930/REVIEW_REPORT.md:177
and flagged as a follow-up at §6 line 378. It survived five adversarial santa-loop rounds
with two independent reviewers.

Not introduced by PR #29 — origin/main fails identically. But #29 rewrote this exact
line for encoding correctness and left the newline half unfixed, which is why a
Windows-focused PR is the natural place this got caught.