Facts port: the snapshot's diff and files read by one compare pinned to the head commit (#284)

Merged
DO
7d ago
+207-97

A snapshot's diff and file list now come from one compare, pinned to the head of the snapshot's first read. A head pushed away and back while the snapshot is read no longer pairs one commit's content with another head. This is the working ruling on #284, closing the hole the security pass on #278 found.

What changes

  • adapters/pull.py.
    • diff reads GET /repos/{base repo}/compare/{base_head}...{head} with the diff Accept. That is the read timeline._content already makes for a review's commit, so the two content ids are made the same way. P-29 shows the bytes equal the pull request's own diff.
    • changed_paths reads the same compare as JSON and takes files.
    • Both commits are checked as 40-hex before the path is built.
    • A 404 refuses the snapshot.
    • A 406 too_large reads as truncated, as before. That is grounded for the pull request's diff, not for compare's, and the design says so.
  • More than 300 files. A compare lists at most 300 files, as GitHub documents it; no trial records this. A change of more is read short by changed_files alone, with nothing asked, so no answer GitHub gives can refuse the snapshot. The Kernel defers it for the Maintainer.
  • adapters/facts.py. The snapshot passes the base branch's head it already read, and the known-limit sentence is gone.
  • Docs. The design's §3.3 rows for content_id and changed_paths say so, and so does a new Boundaries row on decision 0020.

Boundaries, each with a mutant:

  • A diff or a file list read at another commit is never asked for.
  • The pull request's own diff and list are never read.
  • A base head that is not a commit is refused.
  • A fork's head is compared in the base repo.
  • A compare with no files, or files that are not a list, is refused.
  • A change of more than 300 files is read short with nothing asked.
  • 200 and 300 files read whole.
  • Facts compares against the branch's head, not the pull request's last-named base.

Tests. The port tests replay P-29's recorded diff and files answers as compare reads at the head (as_compare). The diff bytes are P-29's own; the file rows have the same keys a compare gives.

Not done. The fresh review raised a hardening question: could GitHub resolve a 40-hex branch name ahead of the commit? That is unverified, and timeline._content has the same exposure today. It is left for a trial; this PR does not change it.

Reviews, all on 1638f8e, with the fix rounds after.

  • Claude review (fresh context). One major: the contract check for a_path_list_can_leave_out_a_file read made-up rows. Minors: the 300 was not marked unverified; a change of more than 300 files could be refused rather than deferred; the Router's docstring was stale. All are answered in 7b2293a.
  • Security pass. The same review covered it. No path lets an author pair a head with another commit's content or read more than 300 files whole. The branch-name question above stays open.
  • AGY (Gemini 4 Argon). No findings.
  • Muse (Muse Wonder Full). Approve, with two test gaps: the base repo for a fork, and a compare with no files. Both have a test and a mutant in ca40688.
  • Verify. make verify passes on a fresh export of the head.

Closes #284

🤖 Generated with Claude Code