size limits are enforced post-hoc for an archive that understates its central directory: the bytes are already on disk

[kiki830621]

Split out of #130 (PR #154), from cross-model review.

What #130 actually bounds

ZipHelper.unzip checks the archive's declarations before extracting, and measures the extracted tree afterwards. Both refuse. But they refuse at different moments, and only the first one is a bound:

archive	when it is refused	bytes written first
declares more than the limit	before extraction	none
understates its entries	after extraction, from the walk	all of them

The central directory can lie — ZipHelper.swift says so in its own comment — so a package that declares 1 KB and writes 4 MB gets its 4 MB onto disk, and is only then refused. Scale that: a package can consume the volume, or the 1.7 GB from #130's own PoC, and the refusal arrives afterwards. That is post-hoc validation, not "how far a package may expand".

The actual compression ratio is not checked at all. An entry declaring a modest size under the 500× declared-ratio limit can still emit 200 MiB from a tiny compressed stream, pass the 256 MiB actual entry limit, and have a real ratio well over 500×.

What it needs

Count bytes as the decompressor produces them and abort the extraction as soon as a per-entry or aggregate limit is crossed. The post-extraction walk can stay as defence in depth; it just cannot be the bound.

The easy route does not work — measured

ZIPFoundation's unzipItem(at:to:…, progress: Progress?) looks sufficient: its helpers check progress?.isCancelled and throw ArchiveError.cancelledOperation, and Archive+Helpers.swift adds actual bytes (progress?.completedUnitCount += Int64(data.count)).

But unzipItem builds totalUnitCount from archive.totalUnitCountForReading(_:) — the declared sizes — and aggregates per-entry children with addChild(_:withPendingUnitCount:). For an archive that understates, each child saturates against its own declared total, so the parent's count stops rising exactly in the case that matters. Observing the parent progress therefore cannot detect the overflow it exists to detect.

So this needs per-entry Archive.extract(_:bufferSize:consumer:) with our own counter, which means re-doing the extraction path that carries #137–#139's hardening (symlink refusal, owner-only descriptors, the private copy, the strict walk). That is why it is not folded into #130 — it is a rewrite of security-critical code, and #130's own history is twelve fix rounds of exactly that kind of change going wrong one round at a time.

Meanwhile

PR #154's CHANGELOG now states the boundary rather than implying a bound: a declaration over the limit is refused before any byte is written; an understating archive is caught after those bytes have landed.

Refs #130

[kiki830621]

Correction: the conclusion here was wrong, and it is now implemented

I filed this saying streaming enforcement needed a rewrite of the extraction path. That was wrong. Round-2 review reproduced the saturation I described and found the way past it, which I then implemented — aec0413 on idd/130-io-size-limits.

What was right

The saturation, reproduced against ZIPFoundation 0.9.20 with a runnable probe:

child.completedUnitCount  = 4194304      ← actual bytes, unclamped
child.fractionCompleted   = 4096.0
parent.completedUnitCount = 1024         ← stops at the declaration
parent.fractionCompleted  = 1.0          ← saturated

totalUnitCountForReading does return Int64(entry.uncompressedSize), and unzipItem does aggregate with addChild. So observing the parent genuinely cannot detect an understating archive.

What was wrong

Saturation only blocks the counting half. Cancellation is a separate path, and the check sits before each chunk reaches the consumer (Archive+Helpers.swift, readCompressed): check → write → account. So the overshoot is exactly one defaultReadChunkSize = 16 KiB.

And the children are reachable: unzipItem calls addChild on the object you hand it, so a Progress subclass that overrides addChild captures every per-entry child and can observe the unclamped completedUnitCount directly. No extract(_:consumer:), no touching the #137–#139 hardening — one extra argument at the unzipItem call.

Measured, on a package declaring 1 KB and writing 8 MB with a 1 MB limit: 512 chunks written before, 65 after.

Two things the design did not predict

1. Progress.cancel() on the parent did not propagate. A child attached through the overridden addChild still read isCancelled == false immediately after — and ZIPFoundation checks the child. The budget cancels each child directly.
2. The interception rides on an implementation detail. Foundation has another way to attach a child (Progress(totalUnitCount:parent:pendingUnitCount:)) that does not call addChild. If a future ZIPFoundation switches, nothing is captured, the total stays zero, and the bound disappears with no error at all — the same silent-failure shape as the Int64 trap round 1 shipped. testTheExtractionBudgetActuallySeesTheEntries asserts the captured count equals the archive's file entries, so that upgrade turns a test red instead of turning the limit off.

Directory entries contribute 1 unit each (assigned, not added) — below any limit worth setting. Symlinks are already refused in the pre-scan.

Leaving this open until PR #154 merges; it closes with that.

Refs #130