From 86f8e56d860237fca6e943a0119f63e355b0e203 Mon Sep 17 00:00:00 2001 From: Deep-Sixed <281237415+Deep-Sixed@users.noreply.github.com> Date: Sun, 20 Sep 2026 04:26:50 +0000 Subject: [PATCH 1/4] Make the checkout part of the publication contract `sha256sum -c docs/ui-design/SHA256SUMS` did not verify in a Windows clone. Git checks text out as CRLF there under the default core.autocrlf=true, the recorded digests are of the committed LF form, and every frozen artifact reported as modified in a tree nobody had touched. A checkout artefact reported as tampering is the worst way for an integrity gate to fail: loud, wrong, and it teaches the reader to distrust the gate rather than the tree. #37 worked around this one layer up, folding line endings in the test before comparing. That kept the suite honest and left the plain shell command still wrong, which is the half that a person checking the baseline from a clone actually runs. This closes it where it happens: * text=auto eol=lf *.png binary `eol=lf` is the load-bearing half. `text=auto` alone normalises what git stores and then defers to the client's core.autocrlf for what it writes out -- exactly the case that broke the check. The PNG rule changes nothing today, since git finds a NUL byte in every PNG and classifies them correctly already; it is there so that stays a rule rather than a heuristic. Adding the file made it an unowned root file, so verify_tree owns it, with its reason recorded beside it: this is a publication guarantee, not an editor preference. 98 -> 99 publishable text files. The test that matters runs real git rather than simulating it, because the claim is about what git writes. It commits the frozen artifacts into a fresh repository under core.autocrlf=true, deletes them, checks them out again, and compares against the digests in SHA256SUMS. Its control is the point: the same round trip with no .gitattributes must produce bytes that do NOT verify, or this platform is not reproducing the situation and the passing half proves nothing about it. The PNG survives both, which is why that rule is insurance. Mutations, each alone: eol=lf dropped, text=auto kept FAIL (declaration and behaviour) the *.png rule removed FAIL .gitattributes removed FAIL (behaviour and ownership) the gate stops owning it FAIL, and verify_tree reports "Unowned root file: .gitattributes" The first is the plausible half-fix, and it fails the behavioural test rather than only the one that reads the file. Nothing was renormalised: all 98 tracked text files were already LF in the index and the 6 binaries were already -text, so no recorded digest moves. Verified after the change, from docs/ui-design/, that `sha256sum -c SHA256SUMS` reports OK for all eight. Gates: 1676 -> 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. docs/ui-design/ untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo --- .gitattributes | 21 ++++ ops/tools/verify_tree.py | 12 +- src/system/tests/test_publication_contract.py | 117 ++++++++++++++++++ 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..c1bb97b --- /dev/null +++ b/.gitattributes @@ -0,0 +1,21 @@ +# How git must materialise this tree, on every platform. +# +# `docs/ui-design/SHA256SUMS` records the bytes that were accepted, and those digests are of +# the committed form, which is LF. With no rule here, git on Windows checks text files out as +# CRLF under the default `core.autocrlf=true`, and `sha256sum -c SHA256SUMS` then reports the +# frozen design baseline as corrupt in a clone where nobody has edited anything. That is a +# checkout artefact reported as tampering: loud, wrong, and the fastest way to teach a reader +# to distrust an integrity gate rather than the tree it guards. +# +# `eol=lf` is what makes this a guarantee rather than a preference. `text=auto` on its own +# still defers to whatever `core.autocrlf` the client has set; `eol=lf` overrides it, so the +# working tree matches the index everywhere and the recorded digests are checkable from any +# clone with nothing but `sha256sum`. +* text=auto eol=lf + +# The reviewed binaries: five frozen design renders and one product screenshot. Git decides +# text-or-binary by looking for a NUL byte near the start of the file, and every PNG has one, +# so this line changes nothing today -- it is here to stop that from being a heuristic. A +# render re-encoded by a checkout would fail `verify_tree`'s exact byte comparison with no +# edit behind it, which is the same false alarm as above wearing different clothes. +*.png binary diff --git a/ops/tools/verify_tree.py b/ops/tools/verify_tree.py index cf80c74..6374de2 100644 --- a/ops/tools/verify_tree.py +++ b/ops/tools/verify_tree.py @@ -16,7 +16,17 @@ DATABASE_ENGINES = {"sqlite3", "libsql", "turso", "pysqlite3"} DATABASE_OWNER = "src/data/store.py" EXCLUDED = {".git", ".venv", ".pytest_cache", ".ruff_cache", "__pycache__", "dist", "build", "var"} -ROOT_FILES = {"README.md", "AGENTS.md", "pyproject.toml", "uv.lock", ".gitignore"} +ROOT_FILES = { + "README.md", + "AGENTS.md", + "pyproject.toml", + "uv.lock", + ".gitignore", + # How git must materialise the tree. It is owned here rather than tolerated because + # it is what makes `sha256sum -c docs/ui-design/SHA256SUMS` answer the same on every + # platform, which is a publication guarantee rather than an editor preference. + ".gitattributes", +} # RFC 2606 reserves these names, and the .example top-level domain, for documentation and # synthetic fixtures. A subdomain of a reserved name is reserved with it, so a fixture may # say alerts@jobs.example.com without that being a real address anyone can receive mail at. diff --git a/src/system/tests/test_publication_contract.py b/src/system/tests/test_publication_contract.py index fbb69f8..15549e5 100644 --- a/src/system/tests/test_publication_contract.py +++ b/src/system/tests/test_publication_contract.py @@ -26,6 +26,8 @@ import importlib.util import json import re +import shutil +import subprocess import sys from pathlib import Path from types import SimpleNamespace @@ -1465,3 +1467,118 @@ def test_a_re_exported_render_is_still_caught_exactly(tmp_path): nudged = tmp_path / "dashboard.png" nudged.write_bytes(raw[:-1] + bytes([raw[-1] ^ 0x01])) assert hashlib.sha256(as_committed(nudged)).hexdigest() != hashlib.sha256(raw).hexdigest() + + +# --- the checkout contract ----------------------------------------------------------------- +# +# Everything above compares bytes this process read off the disk. What put them there is git, +# and on Windows git rewrites text files as it writes them out. That is why these live here: +# the frozen baseline's integrity claim is only as good as the checkout that materialised it. + +GITATTRIBUTES = ROOT / ".gitattributes" +FROZEN_TEXT = "CareerSignal-Mock.dc.html" + + +def declared_rules(): + """The rules `.gitattributes` states, with comments and blank lines dropped.""" + lines = [ + line.strip() + for line in GITATTRIBUTES.read_text(encoding="utf-8").splitlines() + if line.strip() and not line.lstrip().startswith("#") + ] + assert lines, "the checkout contract declares no rules" + return lines + + +def test_the_checkout_contract_pins_both_line_endings_and_binaries(): + """Two rules, and `eol=lf` is the load-bearing half. + + `text=auto` alone normalises what git *stores* and then defers to the client's + `core.autocrlf` for what it *writes out*, which is exactly the case that broke the + baseline check. `eol=lf` is what makes the working tree match the index everywhere. + """ + rules = declared_rules() + assert "* text=auto eol=lf" in rules, f"line endings are not pinned to LF: {rules}" + assert "*.png binary" in rules, f"the reviewed binaries are not declared: {rules}" + + +def git_roundtrip(git, workspace, sources, autocrlf, attributes): + """Commit `sources` into a fresh repository and read back what a checkout materialises. + + The repository is built rather than cloned because the question is what git *writes*, + and the only honest way to ask it is to let git write. + """ + workspace.mkdir() + run = lambda *args: subprocess.run( # noqa: E731 - one call shape, used four times + [git, "-C", str(workspace), *args], check=True, capture_output=True + ) + run("init", "-q", "-b", "main") + run("config", "core.autocrlf", autocrlf) + run("config", "user.email", "verification@example.com") + run("config", "user.name", "CareerSignal verification") + if attributes is not None: + (workspace / ".gitattributes").write_bytes(attributes) + for name, raw in sources.items(): + (workspace / name).write_bytes(raw) + run("add", "-A") + run("commit", "-q", "-m", "baseline") + + # Delete and restore, so the bytes on disk are ones git wrote under these settings + # rather than the ones handed to it. + for name in sources: + (workspace / name).unlink() + run("checkout", "--", ".") + return {name: (workspace / name).read_bytes() for name in sources} + + +def test_a_windows_clone_verifies_the_frozen_baseline(tmp_path): + """The carried defect, closed and proved against real git rather than a simulation. + + `sha256sum -c docs/ui-design/SHA256SUMS` did not verify in a Windows clone: git checks + text out as CRLF there by default, the digests record the committed LF form, and every + frozen artifact reported as modified in a tree nobody had touched. The guards above + worked around it by folding line endings before comparing; this closes it at the layer + that caused it, so the plain shell command answers correctly too. + + The control is the point. Without `.gitattributes`, the same round trip under the same + setting must produce bytes that do NOT verify -- otherwise this platform is not + reproducing the situation and the passing half proves nothing about it. + """ + git = shutil.which("git") + if git is None: + pytest.skip("git is not installed, so no checkout behaviour can be observed") + + baseline = ROOT / "docs" / "ui-design" + sources = { + FROZEN_TEXT: (baseline / FROZEN_TEXT).read_bytes(), + "dashboard.png": (baseline / "renders" / "dashboard.png").read_bytes(), + } + recorded = { + FROZEN_TEXT: recorded_digest(FROZEN_TEXT), + "dashboard.png": recorded_digest("renders/dashboard.png"), + } + attributes = GITATTRIBUTES.read_bytes() + + # Windows' default. Without the contract it rewrites text on the way out. + bare = git_roundtrip(git, tmp_path / "bare", sources, "true", attributes=None) + assert hashlib.sha256(bare[FROZEN_TEXT]).hexdigest() != recorded[FROZEN_TEXT], ( + "this platform did not convert line endings, so the control proves nothing" + ) + assert b"\r\n" in bare[FROZEN_TEXT] + # A binary is left alone even with no rule, which is why the PNG half of the contract is + # insurance rather than a fix: git's own detection already gets this one right. + assert hashlib.sha256(bare["dashboard.png"]).hexdigest() == recorded["dashboard.png"] + + # The same checkout, with the contract this repository ships. + governed = git_roundtrip(git, tmp_path / "governed", sources, "true", attributes) + for name, raw in governed.items(): + assert hashlib.sha256(raw).hexdigest() == recorded[name], ( + f"{name} does not match its recorded digest in a governed Windows checkout" + ) + assert b"\r\n" not in governed[FROZEN_TEXT] + + +def test_the_tree_gate_owns_the_checkout_contract(gate): + """A root file the gate does not know about is unowned, and unowned stops publication.""" + assert ".gitattributes" in gate.ROOT_FILES + assert GITATTRIBUTES.is_file(), "the gate owns a file that is not committed" From dcbdd1f99a315e7a3c29361645e1391d1bbc7a6b Mon Sep 17 00:00:00 2001 From: Deep-Sixed <281237415+Deep-Sixed@users.noreply.github.com> Date: Sun, 20 Sep 2026 04:35:33 +0000 Subject: [PATCH 2/4] Withhold the body where the surface refuses before reading it Windows 3.13 failed on this head, in a test this PR does not touch: test_a_command_needs_a_length_it_can_be_held_to ConnectionAbortedError: [WinError 10053] The cause is already documented in this repository, in Client.unread()'s docstring and again in #36's second commit: a surface that settles a refusal without touching the body leaves the sent bytes unread in the receive buffer, and closing on unread bytes is a reset rather than a clean shutdown on Windows. Whether the refusal or the reset reaches the client first is a race. So this is a latent defect on main that this head surfaced, not one it introduced -- and the fix already exists here. #36 removed the same race from the intake client by declaring a length and sending nothing. This ports `transmit` to the surface client and tags the cases that qualify. It also states each claim more exactly. The request that proves the server refused without reading is the request whose body never arrived, and "refused on what it declares" is a stronger sentence when the oversized body demonstrably never travelled. Tagged, all verified to answer before the body is reached: the missing Content-Length (the failure) the media type, refusing loop (not the charset case, which is a 200) the declared oversize, command the declared oversize, decision the query string Deliberately not tagged: test_a_body_that_is_not_a_json_object_is_refused -- the server must read that body to refuse it, so withholding it would hang; test_the_command_refuses_before_reading_the_body_in_the_order_that_- matters -- its last assertion requires the body to be read, and its stated method is to send a body that would itself be a 400. Rewriting that proof is more than porting a flag, so it is left for a decision rather than changed in passing. The distinction is mechanical rather than judged. Mis-tagging a case whose body IS read does not pass quietly -- it waits for the client timeout. Mis-tagging test_a_body_that_is_not_a_json_object_is_refused on purpose: 6 failed in 61s, every one of them a timeout. The five tagged here run in 9s with no timeout, which is the evidence they qualify. That check is the one #36 learned by getting it wrong once. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo --- src/system/tests/test_web_surface.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/system/tests/test_web_surface.py b/src/system/tests/test_web_surface.py index 037100c..5923c12 100644 --- a/src/system/tests/test_web_surface.py +++ b/src/system/tests/test_web_surface.py @@ -91,6 +91,7 @@ def send( length=None, headers=(), body=None, + transmit=True, ): connection = http.client.HTTPConnection(web.LOOPBACK, self.surface.server_port, timeout=10) try: @@ -111,7 +112,12 @@ def send( # `length` lets a test declare something other than the truth, which is the # only way to reach the ceiling without actually sending the bytes. connection.putheader("Content-Length", str(len(body) if length is None else length)) - connection.endheaders(body) + # `transmit=False` sends the headers and none of the body. It is only for an + # address the surface refuses before reading, where the bytes would otherwise sit + # unread in the receive buffer -- and closing on unread bytes is a reset rather + # than a clean shutdown on Windows, which makes the response a race. Anywhere the + # server does read the body, withholding it hangs until the timeout instead. + connection.endheaders(body if transmit else None) response = connection.getresponse() return response.status, response.read(), dict(response.getheaders()) finally: @@ -1395,6 +1401,8 @@ def test_a_command_must_be_sent_as_json(client, repository, opportunity): origin=True, content_type=content_type, body=body, + # Refused on the media type alone, before a byte is read; see `transmit`. + transmit=False, ) assert status == 400, content_type assert "application/json" in json.loads(payload)["error"], content_type @@ -1423,6 +1431,8 @@ def test_an_oversized_command_is_refused_on_what_it_declares(client, repository, origin=True, content_type="application/json", body=padded, + # The point of refusing on the declaration: the oversized body never travels. + transmit=False, ) assert status == 413 assert str(web.MAX_BODY) in json.loads(payload)["error"] @@ -1430,7 +1440,12 @@ def test_an_oversized_command_is_refused_on_what_it_declares(client, repository, def test_a_command_needs_a_length_it_can_be_held_to(client, repository, opportunity): - """No declared length means nothing to bound, and this surface decodes no chunked body.""" + """No declared length means nothing to bound, and this surface decodes no chunked body. + + The body is declared and never sent. The refusal is decided by the missing header alone, + before anything is read, so bytes on the wire would only sit unread in the receive buffer + -- and Windows 3.13 reset the connection over the response that proves the point. + """ status, payload, _ = client.send( f"/api/v1/opportunities/{opportunity}/status", method="POST", @@ -1438,6 +1453,7 @@ def test_a_command_needs_a_length_it_can_be_held_to(client, repository, opportun content_type="application/json", body=b'{"status": "applied", "expected_event_id": 1}', length="omit", + transmit=False, ) assert status == 400 and "Content-Length" in json.loads(payload)["error"] @@ -1493,6 +1509,8 @@ def test_a_command_refuses_a_query_string(client, repository, opportunity): origin=True, content_type="application/json", body=json.dumps({"status": "applied", "expected_event_id": event}).encode(), + # Refused on the address, before the body is reached. + transmit=False, ) assert status == 400 and "Unknown parameter" in json.loads(payload)["error"] @@ -2287,6 +2305,7 @@ def test_an_oversized_decision_is_refused_on_what_it_declares(addressed, reposit content_type="application/json", body=b'{"approved": false}', length=web.MAX_BODY + 1, + transmit=False, ) assert status == 413 assert str(web.MAX_BODY) in json.loads(body)["error"] From 2aa7c0bc5bec5d23e0d02a1aba14823ad12859f3 Mon Sep 17 00:00:00 2001 From: Deep-Sixed <281237415+Deep-Sixed@users.noreply.github.com> Date: Sun, 20 Sep 2026 04:42:17 +0000 Subject: [PATCH 3/4] Require git for the checkout test rather than skipping without it The matrix went green on dcbdd1f and I could not establish, from the logs, that the test which proves the whole point of this PR had actually run on Windows rather than skipping. Checking took three log fetches and still did not reach the pytest summary line, which is its own argument: a claim that costs that much to verify is one nobody will re-verify. A skip reads as a pass. On the one platform this contract exists for, a green cell that silently covered nothing is worse than a red one -- it is the same failure as the screenshot attestation two PRs ago, where the test passed while proving something other than what it claimed. So git is required instead of detected. That is honest rather than strict: these tests ship only in the source tree, which is obtained by cloning, so an environment running them without git is one that cannot have this repository in the first place. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo --- src/system/tests/test_publication_contract.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/system/tests/test_publication_contract.py b/src/system/tests/test_publication_contract.py index 15549e5..b42b7ef 100644 --- a/src/system/tests/test_publication_contract.py +++ b/src/system/tests/test_publication_contract.py @@ -1544,9 +1544,13 @@ def test_a_windows_clone_verifies_the_frozen_baseline(tmp_path): setting must produce bytes that do NOT verify -- otherwise this platform is not reproducing the situation and the passing half proves nothing about it. """ + # Required rather than skipped. These tests ship only in the source tree, which is + # obtained by cloning, so an environment running them without git is one that cannot + # have this repository in the first place. A skip here would read as a pass while + # proving nothing about the one platform the contract exists for -- and a green cell + # that silently covered nothing is worse than a red one. git = shutil.which("git") - if git is None: - pytest.skip("git is not installed, so no checkout behaviour can be observed") + assert git, "git is not on PATH, so the claim this test makes cannot be observed" baseline = ROOT / "docs" / "ui-design" sources = { From 44c831e9dfe62b7cea4faf5d700ee487ab0549d1 Mon Sep 17 00:00:00 2001 From: Deep-Sixed <281237415+Deep-Sixed@users.noreply.github.com> Date: Sun, 20 Sep 2026 04:57:22 +0000 Subject: [PATCH 4/4] Prove the body is unread, not merely unparsed, before provenance Review is right, and the old test proved the weaker property. It sent the malformed body on every request, which establishes only that the bytes were not PARSED before Host, Origin and token were checked. A server that read the whole body into memory and then checked provenance would have passed it unchanged. The three refusals now declare a body and send none. Each must still answer, which it can only do without reading -- a server reaching for those bytes waits until the client times out. The fourth request sends the same body with provenance in order and must read it to find the malformed JSON, so the pair separates "refused without reading" from "refused after reading" rather than assuming the difference. Demonstrated against a server mutated to buffer the whole body before any provenance check -- the exact design the old test could not see: old test (body transmitted every time) PASS in 0.35s new test (three refusals withhold it) FAIL, 10s timeout The first attempt at that mutation was not faithful and I nearly recorded its result. A bare read-and-discard made the OLD test fail too, but for an artifact rather than the property: the body was consumed, so the fourth request blocked re-reading it. Buffering the bytes so later reads still see them is what isolates the ordering claim, and only then does the old test pass while the new one fails. An earlier attempt was wrong in a different way and also caught: the mutation went into _resolve(), which is the GET path, so it changed nothing a POST test could observe and the suite passed. _command() is where a command settles provenance. This also removes the last instance of the unread-body reset race from this file, which dcbdd1f left behind deliberately because rewriting a load-bearing proof is not porting a flag. Gates: 1679 tests, ruff clean, verify_tree 99/5/1, verify_secrets and verify_wheel PASS. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B8kjhqYENShpeZ16QDnsyo --- src/system/tests/test_web_surface.py | 42 ++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/system/tests/test_web_surface.py b/src/system/tests/test_web_surface.py index 5923c12..d409cae 100644 --- a/src/system/tests/test_web_surface.py +++ b/src/system/tests/test_web_surface.py @@ -1518,19 +1518,43 @@ def test_a_command_refuses_a_query_string(client, repository, opportunity): def test_the_command_refuses_before_reading_the_body_in_the_order_that_matters( client, repository, opportunity ): - """Host, then Origin, then token -- each settled before a byte of the body is parsed. + """Host, then Origin, then token -- each settled before a byte of the body is read. - Proved by sending a body that would itself be a 400: whichever provenance check is - reached first must answer instead, which is only true if the body is never looked at. + The earlier version of this sent the malformed body every time, and that proves the + weaker property. A server that read the whole body into memory and only then checked + Host, Origin and token would have passed it: transmitting the bytes establishes that + they were not *parsed* before provenance, not that they were never *read*. + + So the three refusals declare a body and send none. Each must still answer, which it + can only do without reading -- a server that reached for those bytes would wait for + them until the client timed out. The final request then sends the same body with + provenance in order, and must read it to find the malformed JSON, so the pair + distinguishes "refused without reading" from "refused after reading". + + It also removes the last instance of the unread-body reset race from this file. """ nonsense = b"{not json at all" address = f"/api/v1/opportunities/{opportunity}/status" - common = {"method": "POST", "content_type": "application/json", "body": nonsense} - assert client.send(address, host="evil.example.com", origin=True, **common)[0] == 403 - assert client.send(address, origin=False, **common)[0] == 403 - assert client.send(address, origin=True, token=False, **common)[0] == 401 - # With provenance in order, the same body is finally read -- and refused. - assert client.send(address, origin=True, **common)[0] == 400 + refused = { + "method": "POST", + "content_type": "application/json", + "body": nonsense, + "transmit": False, + } + assert client.send(address, host="evil.example.com", origin=True, **refused)[0] == 403 + assert client.send(address, origin=False, **refused)[0] == 403 + assert client.send(address, origin=True, token=False, **refused)[0] == 401 + # Provenance in order, so the body finally arrives, is read, and is refused for itself. + assert ( + client.send( + address, + method="POST", + content_type="application/json", + body=nonsense, + origin=True, + )[0] + == 400 + ) def test_the_status_vocabulary_is_served_rather_than_copied(client, repository):