ci: enforce the image-size parser mitigation through a test-app gate - #2269
Conversation
|
Size Report
npm unpacked components
Startup median (7 runs, lower is better):
Top changed chunks:
Top changed packed files
|
|
BLOCKED at exact head The new gate claims patch-file edits as an owned input, but Include the patch files in the cache key or force frozen reinstall/patch-hash verification, then plant a patch-only change under a cache hit and show the gate fails before restoring it. |
24d621d to
fc61b04
Compare
|
Confirmed — that was a real hole, and it made the enforcement vacuous for a defining input. Fixed at the cache key, plus two more weaknesses the exercise surfaced. New head: 1. Cache key now includes the patch files (the fix you named). 2. Observed the hole, then the fix (your requested evidence).
So a patch-only regression is now caught instead of silently passing on a stale cache. 3. Two more issues the exercise surfaced (quality):
Validation at |
|
Code review is clean at exact head fc61b04. The patch-only cache-key hole is fixed for restore and save; the planted weakened-patch run discriminates correctly; lockfile-derived artifact selection, gate ownership, and the matching jxlp fixture are sound. The exact-head fingerprint lane has already exercised test-app-security green. No device evidence is owed for this CI/security-only change. Marking ready-for-human while the remaining exact-head CI finishes; merge readiness still depends on those checks completing successfully. |
Summary
image-size(a transitive of the test app's Metro) has two open Dependabot advisories — GHSA-w3rx-r6r6-pgpr (ICNS zero-length entry) and GHSA-5p2g-fcmc-qvqq (JXL/HEIF zero-size box), both DoS via infinite parser loops — and no fixed version exists (latestis still2.0.2, inside the vulnerable range). The mitigation already landed in #2182: an in-tree pnpm patch (examples/test-app/patches/image-size@1.2.1.patch) plus a budget test (examples/test-app/security/image-size-security.test.mjs, run by thesecurity:testscript).The gaps this closes: that proof was a manual script — nothing in CI ran it, and the dependency-graph cache the test depends on could restore a stale patched artifact for a patch-only change, so even a wired-up gate would have tested the previous patch.
This registers the security suite as a first-class gate,
test-app-security, and makes the enforcement sound end to end:test-app:security→pnpm --dir examples/test-app security:test.CHECK_CATALOG(CI-authoritative,localRunnable: false, mirroringtest-app-typecheck— the test app owns an isolated Expo dependency graph and must not be installed by every root-checkout validation).Test App Build Cachefingerprintjob (runs on every PR and already restores the test app dependency graph, so the patched package is present).setup-test-app-dependenciesnow hashesexamples/test-app/patches/**into its restore/save key alongside the manifests. Before, a patch-only edit left the key invariant, so a cache hit restored the previous patchednode_modulesand skippedpnpm install— the gate would run against the prior patch. Any patch change now forces a fresh install that applies the current patch.patchedDependencieshash recorded inpnpm-lock.yamlinstead of the firstimage-size@1.2.1_patch_hash=*directory it finds — the declared artifact is the one under test, and a missing declaration fails loudly.jxlp(the boxextractPartialStreamssearches for). Ground truth: on unpatched 1.2.1 the prior JXL fixture (zero-size box namedjunk) terminates viafindBox's+8advance, so it never exercised the advisory vector — only the new fixture and the ICNS fixture hang. A patch regression that drops theboxSize < 8guard is now caught by the JXL fixture; the ICNS break is still caught by the ICNS fixture.examples/test-app/patches/,examples/test-app/security/, andexamples/test-app/pnpm-workspace.yaml, which carries thepatchedDependenciesdeclaration).The patch itself is verified against both advisory vectors: the
boxSize < 8guard inreadBoxstops the JXLextractPartialStreamsstall on a size-0jxlp(the guard, notfindBox's advance, is what hides a matching zero-size box), and theimageHeader[1] < 8break in the ICNS parser stops the zero-length-entry loop. The HEIF fixture is a termination budget pin: unpatched 1.2.1 has no unbounded loop in the HEIF path (a singlefindBoxchain over the+8-advancingfindBox), so it asserts the format named in the advisory stays within budget.Closes Dependabot alerts #69 and #70 at the mitigation level (they cannot be cleared by a version bump; see residual note below).
Validation
boxSize < 8guard from the patch file), the pre-fix key hash was byte-identical (000c575f…) — a cache hit would restore the stale guarded artifact and the suite passed against it (installedutils.jscarried the guard the tree patch no longer declared). With the fix, the same patch-only edit changes the key (a7004046…→24f5bd59…); the resulting install applied the weakened patch and the gate failed —JXL zero-size jxlp box exceeded the parser budget(spawnSync … ETIMEDOUT) — before the patch was restored.pnpm gate test-app-securitygreen (1 test, 4 fixtures: ICNS entry, HEIF box, JXL box, JXL zero-size jxlp box, each under a 1s budget), artifact resolved via the lockfile patch hash.pnpm check:affected --rungreen at this commit (format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package, integration, gate-manifest, affected-selector,vitest relatedover the changed surface).pnpm gate gate-manifestreports 52 checks wired across 27 lanes (was 51) — the new gate is owned by a lane, so the "every gate owned and wired" invariant holds.309a5360f5; main picked up no commits touching this PR's surface.Residual: Dependabot reports by resolved version, so #69/#70 will remain "open" in the dashboard until upstream ships a fix or the maintainers mark them dismissed-as-mitigated; the patch + gate above are the durable control.