fix(pdf): fail closed when the outline / AcroForm walks hit their node cap

[douglasmun]

External security review (Codex, 2026-08-17) found one validated CDR bypass and two runbook drift issues. All three are fixed here.

High — validated CDR bypass

_strip_pdf_outlines() ended its walk with continue and _strip_acroform_fields() with break on reaching _MAX_WALK_NODES (100,000), then returned normally. cdr_pdf() shipped the file to the sanitised bucket carrying whatever lived past the cap.

Reproduced at production cap size, not just monkeypatched:

input 11.6 MiB, under _MAX_FILE_BYTES: True
before: sanitised output returned, live /A JavaScript present
after:  REJECTED: PDF outline tree exceeds the 100000-node walk cap;
        cannot prove all outline actions were stripped

Same for a 100,002-node AcroForm /Kids chain. Both are far under the 100 MB _MAX_FILE_BYTES, so no size guard stands in the way. Both now raise CdrReject, matching the fail-closed bound already in _walk_pdf_nodes (pitfall #59).

Why the #60 audit missed it

Pitfall #60 claimed every cap's failure direction had been audited. Its table is a hand-written enumeration of ten constants, and _MAX_WALK_NODES — shared by two functions, named nothing like _PDF_WALK_MAX_NODES — was never on it. check_fail_closed.py could not catch it either: that guard mutates rejections that exist and asks whether a test notices, so it is structurally blind to a cap that never raises at all.

Recorded as pitfall #61, with the cap list re-derived from the source rather than from recall (grep -n '_MAX_[A-Z_]*\|_BUDGET' src/lambda_function.py → 10 constants, 9 previously audited + this one). The #60 table now carries the missed row explicitly rather than reading as complete.

Low — deployment runbook drift

• The Terraform EventBridge rule is ${resource_prefix}-s3-object-created (verified at terraform/main.tf:363), not the literal cdr-s3-object-created. Operators using the recommended non-default prefix were inspecting a rule that does not exist. Fixed at both sites.
• MaxMemoryUsed is not an AWS/Lambda CloudWatch metric — the query returns an empty Datapoints list, which reads as "no memory pressure" but means "no such metric". Replaced with the Logs Insights REPORT-line query docs/benchmark.py already uses, plus a pointer to the script. The tuning table made the same claim and is corrected too.

Tests

4 new regression tests: two pin the exception, two pin the outcome (assert b"CAPTAIL" not in clean) so replacing CdrReject with any other silent-completion path still fails. The existing deep-chain tests covered 5,000 nodes but never the cap's failure direction.

check_fail_closed.py now reports 11 pinned rejections, up from 9 — both new ones are load-bearing.

Verification

• 454 tests pass (was 450)
• All 7 CI guards pass, including the full check_fail_closed.py mutation run
• Bypass reproduced pre-fix and confirmed closed post-fix at real cap size

Not done

The review also suggested a CI guard for $PREFIX-parameterized runbook commands. Not implemented — it needs a new script with a real false-positive surface (the SAM path legitimately looks up an auto-generated physical name), and it is scope beyond the two drift fixes.

🤖 Generated with Claude Code