fix: enforce a shared PNG decompression budget

[FHoffarth]

Provenance

Antigravity performed the resource-budget audit, found the gap and wrote the original
fix. Claude reviewed it independently against the code and the diff, and rewrote the
test suite. This split is part of the evidence, not a courtesy: the review was only worth
something because it did not start from the assumption that the fix was right — and it
was not right about everything.

The defect, reproduced before touching anything

Text metadata was budgeted at 8 MiB per file. ICC profiles had their own 16 MiB ceiling.
Nothing counted the two together. Measured against 71f320b with the fix reverted:

 23 KiB in | 15 MiB ICC + 7.5 MiB text     -> ACCEPTED (10 findings)
 16 KiB in | ICC just under 16 MiB + text  -> ACCEPTED (3 findings)
 17 KiB in | 8 MiB text + 8.5 MiB ICC      -> ACCEPTED (10 findings)

A 23 KiB file unpacked to about 22.5 MiB, passed inspection, and was cleaned and verified
without complaint. profileBytes() had no budget parameter at all.

Same inputs with the fix in place:

 23 KiB in | 15 MiB ICC + 7.5 MiB text     -> refused: metadata … more than FilePass will read
 16 KiB in | ICC just under 16 MiB + text  -> refused
 17 KiB in | 8 MiB text + 8.5 MiB ICC      -> refused
 16 KiB in | 8 MiB ICC + 8 MiB text        -> ACCEPTED   ← the boundary, correct

Both orderings behave identically. The budget is one counter, not a sequence of them.

The fix

One DecompressionBudget per file carries both paths: 16 MiB in total, with the 8 MiB
text budget and the 1 MiB single-chunk ceiling unchanged inside it, and the 16 MiB profile
ceiling unchanged but no longer additive. Bytes are charged as the reader hands them over,
so a stream that dies late has still spent what it delivered; the reader is cancelled when
the budget refuses. Text-only bombs keep the sentence they had — a combined overflow says
metadata, because saying text would be untrue.

The chunk that breaks a per-item ceiling is deliberately not charged. That is only sound
while every caller turns an overflow into a refusal, and the invariant is now written down
where it has to hold.

What the review changed

Nine of the ten proposed test cases passed against the unfixed code. Only one detected
the defect, and the reported 15 MiB + 7.5 MiB attack was not among them. After the rewrite,
five of fourteen fail without the fix.

Change	Why
profileBytes(rest, budget?) → required	an optional budget is a bypass waiting for the next call site
the reported attack added as an end-to-end regression	via runBytes, asserting no verdict and no download
order-independence, both directions	the previous file only tested ICC-first
"ICC fills the budget, no room for 64 KiB of text"	a case where no individual ceiling is breached and only the total refuses
the two cleaner tests rewritten	see below
unused cleanAndVerify import removed	—

The removed case claimed cleanFile and runBytes enforce the shared budget. It used a
16 MiB + 100 KiB profile, which trips the pre-existing per-profile ceiling, so it was
green without the fix; runBytes was never called; and it built an artificial report
object, the anti-pattern already removed from resource.test.ts once.

One claim the review disproved

clean() receives a budget, but that budget cannot change any outcome there. The
cleaner never inflates text — it drops those chunks whole — so only the profile is charged,
and MAX_PROFILE_BYTES equals the total budget. A test asserting that cleanFile rejects
the 22.5 MiB file failed: the cleaner accepts it. Harmless, because clean() is only
reached after inspection has already refused, but it is future-proofing rather than a second
enforced gate, and the tests now say so instead of claiming protection that is not there.

Verified independently

Claim	Verdict
text and ICC previously accounted separately	confirmed
the 22.5 MiB case is real	reproduced
iCCP cardinality is capped at one	confirmed — and checked before any decompression
IDAT is not a metadata decompression path	confirmed — only text chunks and iCCP are inflated
JPEG unaffected	confirmed — no decompression call exists in jpeg.ts or exif.ts
PDF unaffected	only for FilePass's own code; see follow-ups
"ICC keeps its own 16 MiB ceiling"	misleading — with text present the room is 16 MiB minus text. Intended, but not an unchanged ceiling

Evidence

Windows, core.autocrlf=true:

Gate	Result
Product tests	5 files / 43 tests passed
Audit tests	22 files / 310 tests passed (was 21 / 296)
tsc --noEmit	clean
vite build	built in 951ms
npm audit --audit-level=high	0 vulnerabilities
git diff --check	clean
git diff audit/	empty — no tracked evidence file changed
evidence-ownership guard	green; the new file writes nothing
git status --porcelain after a full audit run on the committed tree	empty

npm ci again hit a Windows EPERM lock on @rolldown/binding-win32-x64-msvc in the working
checkout. No unattributable processes were killed; the lockfile was verified by running
npm ci against a copy in a scratch directory — 125 packages, 0 vulnerabilities. CI is the
authoritative clean-runner gate.

Scope

No JPEG, PDF, verification semantics, dependencies or build-system changes. The only new
product copy is the metadata overflow message, which is required because the text message
would be a false statement in that case.

Follow-ups, deliberately not in this PR

1. reader.cancel() can in principle reject and thereby mask the original budget error,
   which would downgrade a refusal to "unreadable" and let the file through. Pre-existing
   pattern, unlikely, tracked separately.
2. PDF decompression happens inside pdf-lib and pdfjs-dist, outside this PNG budget.
   Real, but its own scope.

No tag, release or deploy proposed.

🤖 Generated with Claude Code