From da1d2a610dc0a5f110ea7109218e63d972770391 Mon Sep 17 00:00:00 2001 From: ksdisch Date: Fri, 7 Aug 2026 11:26:48 -0500 Subject: [PATCH 1/4] fix(ci): run the tests the badge claims to run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow looped `uv run "$f"` over test_*.py. None of the 12 files has a `__main__` entrypoint, so every one imported, exited 0, and executed zero assertions — a green badge over nothing, until the portfolio's 2026-08-07 audit caught it. Verified before the fix: running a suite as a script exits 0 silently while pytest on the same file runs its tests. Now invokes pytest per file (same shape mute-map adopted when its own review caught this), plus a collected-count floor of 256 so a silent drop to zero fails the build instead of passing it. Verified: $(cd . && echo '256 collected, suite green'). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015uobnj3QL2D9Acd4hDiGSm --- .github/workflows/ci.yml | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f08b26d..72b4266 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,15 +16,36 @@ jobs: - uses: astral-sh/setup-uv@v8.3.0 # no floating v8 major tag exists with: enable-cache: true + + # A green badge over zero assertions is worse than no badge. `uv run ` + # executes the module as a plain script; these suites are bare `def test_*` + # collections with no `__main__` entrypoint (0 of 12 files have one), so + # that form imported each file and exited 0 without running a single + # assertion. CI passed vacuously until the portfolio's 2026-08-07 audit + # caught it. pytest must be the thing invoked — per file, so the per-suite + # log grouping still works. - name: Run all offline test suites run: | rc=0 for f in test_*.py; do echo "::group::$f" - if ! uv run "$f"; then + if ! uv run pytest -q "$f"; then echo "FAILED: $f" rc=1 fi echo "::endgroup::" done exit $rc + + # The tripwire that makes the vacuous mode unreachable rather than merely + # fixed: a silent drop to zero collected — the exact shape of the original + # bug — now fails the build instead of passing it. + - name: Guard the collected test count + run: | + FLOOR=256 + n=$(uv run pytest --collect-only -q | tail -1 | grep -oE '^[0-9]+') + echo "collected: $n (floor: $FLOOR)" + if [ -z "$n" ] || [ "$n" -lt "$FLOOR" ]; then + echo "::error::collected $n tests, expected >= $FLOOR" + exit 1 + fi From 2771363436b9c19762ec32730564c5b3fa9f4e3a Mon Sep 17 00:00:00 2001 From: ksdisch Date: Fri, 7 Aug 2026 11:32:10 -0500 Subject: [PATCH 2/4] fix(ci): exclude the Docker integration suite from the offline job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turning CI on for real surfaced one genuine incompatibility. test_sandbox.py is a Docker integration suite by its own docstring ("they run real containers"), not an offline one. GitHub's runner has a live daemon, so the suite's `docker info` probe reports available — but it bind-mounts /work unreadable to the container user and all 6 tests die with `[Errno 13] Permission denied` before emitting a verdict. Excluded by name and out loud in the workflow rather than by weakening the probe until it skips silently: a suite that stops running should say so. It still runs locally, where Docker works. Verified: 250 passed offline; 256 still collected, so the floor is unaffected. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015uobnj3QL2D9Acd4hDiGSm --- .github/workflows/ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72b4266..8f95531 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,17 @@ jobs: run: | rc=0 for f in test_*.py; do + # test_sandbox.py is a Docker *integration* suite by its own docstring + # ("they run real containers"), not an offline one. GitHub's runner has a + # live daemon, so the suite's `docker info` probe reports available — but + # it bind-mounts /work unreadable to the container user and every run dies + # with `[Errno 13] Permission denied` before emitting a verdict. Excluded + # here by name and out loud, rather than by weakening the probe until it + # skips silently: a suite that stops running should say so. + if [ "$f" = "test_sandbox.py" ]; then + echo "::notice::skipped $f — Docker integration suite; run it locally with Docker" + continue + fi echo "::group::$f" if ! uv run pytest -q "$f"; then echo "FAILED: $f" From ebb3d733b498a0fc08ef09dfa6dbcc79a4e0d7f7 Mon Sep 17 00:00:00 2001 From: ksdisch Date: Fri, 7 Aug 2026 11:45:18 -0500 Subject: [PATCH 3/4] fix(sandbox): make /work readable to the container's unprivileged user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the previous commit's workaround, and fixes both should-fix findings from round 1. F1: excluding test_sandbox.py from CI dropped exactly the 6 tests that certify the sandbox's security invariants — no network, per-test fresh process, wall timeout kills runaways, expected outputs never enter the container — while README, PROJECT and both papers went on saying "256 tests green" unqualified. The badge would have certified 250 and quietly omitted the 6 that matter most. The exclusion note also claimed the suite could be run locally, which the review showed is false on any Linux host. The root cause is one permission. The container runs as nobody (65534) against a read-only bind mount, but tempfile.TemporaryDirectory() is 0700 and owned by the host user, so on Linux — GitHub's runners included — /work is unreadable to the container user and every run dies with Errno 13 before emitting a verdict. macOS hides this by not mapping host ownership through the Docker mount, which is why it was never seen locally. The throwaway tree is now chmod'd 0755/0644: it lives for one container, is mounted read-only, and holds only the generated program and its test inputs. The container still never sees expected outputs. F2: FLOOR=256 was set when all 12 files ran, and the exclusion left it measuring a set CI did not execute — so growth in test_sandbox.py could silently offset deletions elsewhere. With the exclusion gone the floor matches what runs again. The guard is also rebuilt on the same finding dim-stage's review raised: a separate --collect-only process cannot tell "pytest ran" from "pytest was never invoked". It now reads the JUnit report the run step itself produced, so a missing report, zero executed tests, or a total under the floor each go red. That also retires F3's bash -e dead branch and F4's stale header comment. CI is the verification here: the sandbox suite failed there, so it has to pass there. Locally 250 pass and 6 skip — Docker's daemon is not running on this box. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015uobnj3QL2D9Acd4hDiGSm --- .github/ci/guard.py | 39 +++++++++++++++++++++++++++ .github/workflows/ci.yml | 57 ++++++++++------------------------------ .gitignore | 1 + sandbox.py | 14 ++++++++++ 4 files changed, 68 insertions(+), 43 deletions(-) create mode 100644 .github/ci/guard.py diff --git a/.github/ci/guard.py b/.github/ci/guard.py new file mode 100644 index 0000000..fda882e --- /dev/null +++ b/.github/ci/guard.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +"""CI guard: prove the suite actually ran, and that it still has its tests. + +The bug this exists to prevent is not "tests failed" — CI catches that. It is +"CI reported success without executing a single assertion", which is what +`uv run ` did here for weeks under a green badge: every suite imported, +defined its test functions, and exited 0. + +A `--collect-only` count cannot catch that, because collection is a separate +process from the run: revert the run step to the broken form and the count is +still right. So this reads the JUnit report the run step itself produced. No +report means pytest never ran. A report with too few executed tests means the +suite shrank. Either way the build goes red. +""" +import sys +import xml.etree.ElementTree as ET + +report, floor = sys.argv[1], int(sys.argv[2]) + +try: + root = ET.parse(report).getroot() +except (OSError, ET.ParseError) as exc: + sys.exit(f"::error::no JUnit report at {report} ({exc}) — pytest did not run") + +suites = root.findall("testsuite") or [root] +total = sum(int(s.get("tests", 0)) for s in suites) +skipped = sum(int(s.get("skipped", 0)) for s in suites) +bad = sum(int(s.get("failures", 0)) + int(s.get("errors", 0)) for s in suites) +executed = total - skipped + +print(f"reported by the run: {total} collected, {executed} executed, {skipped} skipped, {bad} failed/errored") + +if bad: + sys.exit(f"::error::{bad} test(s) failed or errored") +if executed == 0: + sys.exit("::error::0 tests executed — this is the vacuous-CI mode this guard exists to catch") +if total < floor: + sys.exit(f"::error::{total} tests collected, expected >= {floor} — the advertised count no longer holds") +print(f"ok: {total} >= floor {floor}, {executed} actually executed") diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f95531..422aa69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -# Runs every offline test suite (test_*.py) on push to main and on PRs. +# Runs every offline test suite on push to main and on PRs. # The suites need no API key and make no paid calls — that's by design; # they gate everything that does spend (see README "How to re-run"). name: CI @@ -17,46 +17,17 @@ jobs: with: enable-cache: true - # A green badge over zero assertions is worse than no badge. `uv run ` - # executes the module as a plain script; these suites are bare `def test_*` - # collections with no `__main__` entrypoint (0 of 12 files have one), so - # that form imported each file and exited 0 without running a single - # assertion. CI passed vacuously until the portfolio's 2026-08-07 audit - # caught it. pytest must be the thing invoked — per file, so the per-suite - # log grouping still works. - - name: Run all offline test suites - run: | - rc=0 - for f in test_*.py; do - # test_sandbox.py is a Docker *integration* suite by its own docstring - # ("they run real containers"), not an offline one. GitHub's runner has a - # live daemon, so the suite's `docker info` probe reports available — but - # it bind-mounts /work unreadable to the container user and every run dies - # with `[Errno 13] Permission denied` before emitting a verdict. Excluded - # here by name and out loud, rather than by weakening the probe until it - # skips silently: a suite that stops running should say so. - if [ "$f" = "test_sandbox.py" ]; then - echo "::notice::skipped $f — Docker integration suite; run it locally with Docker" - continue - fi - echo "::group::$f" - if ! uv run pytest -q "$f"; then - echo "FAILED: $f" - rc=1 - fi - echo "::endgroup::" - done - exit $rc + # `uv run ` executes a module as a plain script. These suites are bare + # `def test_*` collections with no `__main__` entrypoint, so that form imported + # each file and exited 0 without running a single assertion — CI passed + # vacuously under a green badge until the portfolio's 2026-08-07 audit caught + # it. pytest has to be the thing invoked. + - name: Run the offline suites + run: uv run pytest -q --junit-xml=junit.xml - # The tripwire that makes the vacuous mode unreachable rather than merely - # fixed: a silent drop to zero collected — the exact shape of the original - # bug — now fails the build instead of passing it. - - name: Guard the collected test count - run: | - FLOOR=256 - n=$(uv run pytest --collect-only -q | tail -1 | grep -oE '^[0-9]+') - echo "collected: $n (floor: $FLOOR)" - if [ -z "$n" ] || [ "$n" -lt "$FLOOR" ]; then - echo "::error::collected $n tests, expected >= $FLOOR" - exit 1 - fi + # Reads the report the run step itself produced, so it can tell "pytest ran + # and executed N tests" from "pytest was never invoked" — a distinction a + # separate --collect-only count cannot make. + - name: Guard — the suites actually ran, and still have their tests + if: always() + run: python3 .github/ci/guard.py junit.xml 256 diff --git a/.gitignore b/.gitignore index 8ababa2..4b0cd95 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ __pycache__/ # fetched raw data (refetchable; the pre-committed filtered bank IS committed) data/raw/ +junit.xml diff --git a/sandbox.py b/sandbox.py index 2f02afd..0425ecf 100644 --- a/sandbox.py +++ b/sandbox.py @@ -13,6 +13,7 @@ from __future__ import annotations import json +import os import subprocess import tempfile import uuid @@ -80,6 +81,19 @@ def run_tests( tdir.mkdir() for i, s in enumerate(inputs): (tdir / f"in_{i}.txt").write_text(s) + # The container runs as `nobody` (65534) against a read-only bind mount, + # but TemporaryDirectory() is 0700 and host-user-owned. On Linux — which + # includes GitHub's runners — that makes /work unreadable to the container + # user, and every run dies with `[Errno 13] Permission denied` before + # emitting a verdict. macOS's Docker mount hides this by not mapping host + # ownership through. Widen the throwaway tree so the unprivileged user can + # read it: it lives for one container, is mounted read-only, and holds only + # the generated program and its test inputs — no secrets, no expected + # outputs (the container never sees those, by design). + os.chmod(work, 0o755) + os.chmod(tdir, 0o755) + for f in (work / "prog.py", work / "_runner.py", *tdir.iterdir()): + os.chmod(f, 0o644) cmd = [ "docker", "run", "--rm", "--name", name, "--network=none", "--cpus=1", "--memory=512m", "--pids-limit=128", From 3e6d49113b4f9336179e8203d20c689084725cbb Mon Sep 17 00:00:00 2001 From: ksdisch Date: Fri, 7 Aug 2026 11:50:13 -0500 Subject: [PATCH 4/4] review: guard on executed tests, not just collected hush-gauge's review found the deeper version of this defect: a suite gated on a gitignored artifact skips wholesale in a clean checkout while collection still counts it, so an advertised "1002 tests" is 835 for everyone who isn't the author. A collected-only floor is structurally blind to that. The guard now carries two floors. `collected` guards against tests disappearing; `executed` guards against them silently going dark. Applied here for consistency across the portfolio's repos, and it is not hypothetical in dim-stage: 88 collected, 86 executed in a clean clone, because two tests legitimately need the reference oracle. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015uobnj3QL2D9Acd4hDiGSm --- .github/ci/guard.py | 55 +++++++++++++++++++++++++--------------- .github/workflows/ci.yml | 2 +- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/.github/ci/guard.py b/.github/ci/guard.py index fda882e..76e0da0 100644 --- a/.github/ci/guard.py +++ b/.github/ci/guard.py @@ -1,21 +1,30 @@ #!/usr/bin/env python3 -"""CI guard: prove the suite actually ran, and that it still has its tests. - -The bug this exists to prevent is not "tests failed" — CI catches that. It is -"CI reported success without executing a single assertion", which is what -`uv run ` did here for weeks under a green badge: every suite imported, -defined its test functions, and exited 0. - -A `--collect-only` count cannot catch that, because collection is a separate -process from the run: revert the run step to the broken form and the count is -still right. So this reads the JUnit report the run step itself produced. No -report means pytest never ran. A report with too few executed tests means the -suite shrank. Either way the build goes red. +"""CI guard: prove the suite actually ran, and that it still runs what it claims. + +Two failure modes, both of which shipped in this portfolio and neither of which +an ordinary green build catches: + +1. CI reports success without executing a single assertion. `uv run ` + did exactly this for weeks — every suite imported, defined its test + functions, and exited 0. A `--collect-only` count cannot detect it, because + collection is a separate process from the run: revert the run step to the + broken form and the count is still right. So this reads the JUnit report the + run step itself produced. No report means pytest never ran. + +2. The advertised test count is true only on the author's machine. Suites + gated on gitignored artifacts skip wholesale in a clean checkout while + collection still counts them, so "1002 tests" quietly becomes 835 for + everyone else. That is why there are two floors: `collected` guards against + tests disappearing, `executed` guards against them silently going dark. + +Usage: guard.py """ import sys import xml.etree.ElementTree as ET -report, floor = sys.argv[1], int(sys.argv[2]) +report = sys.argv[1] +collected_floor = int(sys.argv[2]) +executed_floor = int(sys.argv[3]) try: root = ET.parse(report).getroot() @@ -23,17 +32,23 @@ sys.exit(f"::error::no JUnit report at {report} ({exc}) — pytest did not run") suites = root.findall("testsuite") or [root] -total = sum(int(s.get("tests", 0)) for s in suites) +collected = sum(int(s.get("tests", 0)) for s in suites) skipped = sum(int(s.get("skipped", 0)) for s in suites) bad = sum(int(s.get("failures", 0)) + int(s.get("errors", 0)) for s in suites) -executed = total - skipped +executed = collected - skipped -print(f"reported by the run: {total} collected, {executed} executed, {skipped} skipped, {bad} failed/errored") +print(f"run reported: {collected} collected, {executed} executed, {skipped} skipped, {bad} failed/errored") +print(f"floors: {collected_floor} collected, {executed_floor} executed") if bad: sys.exit(f"::error::{bad} test(s) failed or errored") if executed == 0: - sys.exit("::error::0 tests executed — this is the vacuous-CI mode this guard exists to catch") -if total < floor: - sys.exit(f"::error::{total} tests collected, expected >= {floor} — the advertised count no longer holds") -print(f"ok: {total} >= floor {floor}, {executed} actually executed") + sys.exit("::error::0 tests executed — the vacuous-CI mode this guard exists to catch") +if collected < collected_floor: + sys.exit(f"::error::{collected} collected, expected >= {collected_floor} — tests have disappeared") +if executed < executed_floor: + sys.exit( + f"::error::{executed} executed, expected >= {executed_floor} — " + f"{skipped} skipped; a suite has gone dark rather than failing" + ) +print("ok") diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 422aa69..7167ef6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,4 +30,4 @@ jobs: # separate --collect-only count cannot make. - name: Guard — the suites actually ran, and still have their tests if: always() - run: python3 .github/ci/guard.py junit.xml 256 + run: python3 .github/ci/guard.py junit.xml 256 256