ci: guard that every fail-closed rejection is pinned by a test

[douglasmun]

Follow-up to #112 (pitfall #59), where a resource cap returned instead of raising and shipped a live payload to the sanitised bucket while the whole suite stayed green. Nothing asserted the bound was load-bearing, so the tests passed identically with the hole open and closed.

Two parts: audit the other caps, then make the property enforceable so it outlives the session that checked it.

Part 1 — every cap's failure direction, audited

Verified by running each one (monkeypatched low, hostile input, observe the verdict) rather than reading it — #53 is a docstring that claimed FAIL CLOSED while the code did not.

Cap	On exceeding	Correct?
_PDF_WALK_MAX_NODES	CdrReject	fixed in #112
_EXTERNAL_REF_SCAN_MAX_NODES	return True (= external → subtree removed)	yes, closed by design
_DecompressionBudget / _MAX_TOTAL_ENTRY_BYTES	CdrReject	yes
_MAX_ZIP_ENTRIES	_validate_zip_structure hard-fail	yes
_MAX_ENTRY_BYTES, _MAX_FILE_BYTES	reject	yes
_MAX_IMAGE_FRAMES, _MAX_TOTAL_IMAGE_PIXELS	CdrReject	yes
_SNS_REMOVED_BYTE_BUDGET	truncates, explicitly marked	yes — and it must

No second instance of the #59 bug.

Part 2 — the guard

That last row is why this is mutation-based rather than a grep for return/break inside a bounded loop. _SNS_REMOVED_BYTE_BUDGET caps a report about an already-sanitised file; rejecting there would discard a successful sanitisation over a long removed-list. The right failure direction depends on what the bound protects, so a syntactic rule would flag the one cap that is correct to truncate.

What is not a judgement call is whether a rejection that exists is load-bearing. scripts/check_fail_closed.py mutates each raise CdrReject into a silent pass and re-runs the suite:

  caught    lambda_function.py:893:  raise CdrReject(f"unparseable xlsb workbook: ...
  caught    lambda_function.py:1298: raise CdrReject(f"XML part '{filename}' contains a DTD ...
  caught    lambda_function.py:1409: raise CdrReject("no %PDF- header in the first 1024 bytes")
  caught    lambda_function.py:1667: raise CdrReject(
  ...
all 9 fail-closed rejections are pinned by a failing test

All 9 currently fail their mutant, so existing coverage was sound — the guard exists so that stays true for rejections added later, which is exactly the path by which #59 shipped.

The guard needed two things not to become the defect it checks for

A baseline green run before any mutant counts. Without it a broken checkout reports every mutant "caught" and passes vacuously (#57). This fired immediately and usefully: my first version exported SANITISED_BUCKET/QUARANTINE_BUCKET, but the test module defaults them with os.environ.setdefault, which yields to the environment — five tests asserting the literal test-sanitised/test-quarantine names failed. The documented "run pytest BARE" rule, rediscovered by writing a tool that broke it.

A negative control on the guard itself. Injected a deliberately unpinned raise CdrReject; the guard named it by line:

::error::1 fail-closed rejection(s) not pinned by any test
  lambda_function.py:580: raise CdrReject("decorative rejection with no test")

A checker never shown failing is an assumption.

Notes

• src/lambda_function.py is untouched — enforcement only, no behaviour change.
• Runs after the main test step in tests.yml; it costs one suite run per rejection (~9 runs).
• 450 tests pass, unchanged (the guard is a script, not a test). All other guards still pass.
• Recorded as pitfall docs: threat-coverage analysis for the JXA-Persistency macOS chain #60.

General rule: when a review finds a bug class rather than a bug, the deliverable is the executable check, not the fixed instance. This repo already had that instinct for docs (check_test_count.py, check_cap_defaults.py, check_pitfalls_index.py, check_iac_parity.py); security invariants deserve it more, because a doc drifting is embarrassing and a fail-open shipping is not.

🤖 Generated with Claude Code