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..b42b7ef 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,122 @@ 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. + """ + # 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") + assert git, "git is not on PATH, so the claim this test makes cannot 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" diff --git a/src/system/tests/test_web_surface.py b/src/system/tests/test_web_surface.py index 037100c..d409cae 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"] @@ -1500,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. + + 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". - 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. + 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): @@ -2287,6 +2329,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"]