fix(cli): validate captured Lottie archives and previews

[jrusso1020]

Lottie capture currently saves corrupt archive bytes as .lottie files and accepts JSON with only truthy layers and w. Its automatic preview page also loads animation assets without capture’s request policy. This change rejects invalid archives/animation data before persistence and installs request interception before loading the preview shell.

• fitpro-painel-mestre.html #363: remove the raw-on-error archive write. Read v1 animations/*.json and v2 a/*.json in memory, preflight the raw classic central directory and entry count before constructing adm-zip (names at most 512 bytes and 16 path segments; ambiguous alternate/ZIP64 footer selection rejected), cap declared expansion, reject unsafe paths/zero-size selected entries, and verify actual output size. The existing adm-zip dependency bounds inflation using the positive entry size.
• docs(website-to-hyperframes): add load-bearing GSAP authoring rules #364: bound JSON to 10 MiB, require record-valued semantic layers and a finite duration of at most one hour (defaults: ip/op=0, fr=30), and bound canvas dimensions/tree size/depth. Discovery uses response metadata only and re-fetches public JSON candidates through a bounded 5 MB stream; it never calls Puppeteer response.buffer(). This deliberately uses the public-asset fetch contract without replaying browser credentials or POST bodies. Response events synchronously collect at most 32 deduplicated candidates. Explicit .lottie and .json URLs displace the lowest-priority generic JSON candidate with deterministic insertion-order ties. One awaited sequential phase closes collection before fetching and finishes before save, bounded to 10 seconds/the capture deadline, 20 MiB, and 10 results. Shared accounting avoids charging retained discovery data twice. Move the shared capture download budget before the Lottie pass so it also accounts for later fonts/images.
• Preview requests allow the pinned lottie-web runtime and public image/font/stylesheet resources, plus data/blob assets. Every request and redirect target passes the existing scheme/private-address policy; other scripts and navigations are blocked. This retains public assets and makes no DNS-rebinding protection claim beyond the existing literal/hostname policy.

Validation: full workspace build, CLI typecheck, all 301 capture tests; two end-to-end persistence regressions fail against original main and four archive/semantic review regressions fail the first PR head. Missing/lying Content-Length discovery tests verify cancellation without invoking the Puppeteer body API. A real Chromium/local-server experiment allowed a simulated public asset and rejected both direct loopback and public-to-loopback redirects before they hit the target server. Delayed response, late event, duplicate URL, 100 generic candidate, and ordered priority replacement tests cover discovery ownership. The ordered priority regression fails the preceding head. The exact 5,000-segment ZIP regression and alternate footer probes cover pre-materialization rejection; the independent reviewer measured the deep-path probe dropping from approximately 196 ms/129 MiB extra RSS to 0 ms/0 MiB. Tests also cover valid v1/v2 archives, corrupt archives, unsafe declared expansion sizes, JSON shape/prototype/nonfinite-number controls, request classifications, and existing capture-budget behavior. Fallow passes with one test-fixture clone warning, accepted as nonblocking by the independent reviewer.

Please independently review security and compatibility (archive limits, animation timing/tree limits, allowed preview request classes) and classify #363/#364 after the final PR CodeQL scan. No alerts dismissed or claimed closed before merge/main verification.

[miguel-heygen]

The raw corrupt-archive fallback is correctly removed, the archive output stays in memory, and guardLottiePreviewRequests() is installed before the shell/runtime request. The exact runtime allow, public image/font/stylesheet classes, and script/document/private-target denials match the isolated preview threat model; redirects re-enter Puppeteer interception.

Blocker — intercepted JSON is still fully buffered before any enforceable cap. packages/cli/src/capture/index.ts:231-236 trusts Content-Length and then calls response.buffer(). A missing or lying header makes Puppeteer materialize the entire attacker response before the post-hoc 5 MB check; saveLottieAnimations() only receives the already-parsed object later and cannot undo that allocation. This is the same OOM shape the bounded reader fixed for the re-fetch path, so the PR body's “downloaded/intercepted JSON” bound is not true. Use a genuinely bounded discovery path (or fail closed before reading when no trustworthy bounded stream is available) and add missing/lying-header regressions at this source.

Blocker — the archive entry cap runs after attacker-controlled entry materialization. packages/cli/src/capture/lottieValidation.ts:9-10 calls new AdmZip(bytes).getEntries() before checking entries.length. In adm-zip 0.6.0, getEntries() allocates new Array(mainHeader.diskEntries) and constructs every ZipEntry; a 10 MB central directory can encode well over 100k minimum-size entries, so memory is spent before the 256-entry guard. AdmZip#getEntryCount() reads the EOCD count without loading entries: reject on that first, then call getEntries(). The size/inflation/path checks after that are sound.

Blocker — the advertised layer/timing validation still accepts malformed values that reach lottie-web. lottieValidation.ts:55-67 only proves root layers is an array and fr > 0; it does not validate layer entries or a finite derived duration. I executed both controls at this head:

{layers:[true,null,"x"], fr:30, ip:0, op:30} -> true
{layers:[], fr:5e-324, ip:0, op:10000000} -> true; duration -> Infinity

The second becomes Infinity at mediaCapture.ts:134-135 and serializes as null in the manifest at :203-214. Require every semantic layers collection the validator accepts to contain records, and validate a finite, bounded (op-ip)/fr (with an explicit default when timing fields are omitted). Add these exact positive/negative controls.

Local exact-head validation: the three focused files pass 25/25; those tests currently do not cover the counterexamples above. My local Fallow audit reports zero duplicate groups in changed files; if hosted Fallow retains one test-fixture clone, it is nonblocking because it duplicates no production decision. The browser experiment log path is not present in this session, so I relied on the source policy and tests rather than claiming that artifact.

#363 should close as fixed once the early entry-count guard is in: its raw .lottie sink is gone. #364 is not ready for disposition because the intercepted source and JSON semantic boundary above remain open; no dismissal is warranted. CI/CodeQL were still running at review time.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The sink hardening is directionally correct, but unbounded interception and post-materialization archive limits leave the original resource attack live, while malformed layer/timing values still cross the new validator into lottie-web.

[miguel-heygen]

The first-round blockers are closed: discovery no longer materializes the Puppeteer body, EOCD count is checked before getEntries(), semantic layers collections require record entries, and derived duration is finite and capped at one hour. The public GET/no-browser-credentials/POST tradeoff is acceptable as a documented fail-closed choice by itself.

Blocker — response events launch untracked, unbounded refetch work that races the only save pass. packages/cli/src/capture/index.ts:215-226 registers an async EventEmitter listener; Puppeteer/Node does not await the returned promise. Every .json, application/json, or text/plain response starts safeFetch() at lottieDiscovery.ts:13-25, with no URL dedupe or candidate/concurrency cap. saveLottieAnimations() runs once at index.ts:356-364 without awaiting those promises. A late/slow discovery can therefore (a) append after the save iteration and be silently lost, (b) keep refetching after the phase/capture has advanced, and (c) debit the shared budget concurrently with later font/image passes. A JSON-heavy SPA also gets an extra public GET per response and can exhaust the 100 MiB budget on non-Lottie API payloads before real assets. Collect bounded/deduplicated response metadata synchronously, then perform a capped/awaited discovery batch before the save pass (or explicitly track/await/abort the promises). Add delayed-response and many-ordinary-JSON route-level controls that prove no discovery or budget mutation occurs after the save boundary.

Blocker — one entry can still blow up adm-zip before path validation. The EOCD count check at lottieValidation.ts:9-12 closes the many-entry allocation, but getEntries() calls adm-zip's makeTemporaryFolders() before returning. safeArchivePath() at :39-46 therefore runs too late for a deeply segmented single name: adm-zip constructs every prefix with repeated slice().join() and synthetic ZipEntry objects. Executed at this head, one 20,124-byte ZIP with a single a/ + 5,000 x/ segments + demo.json entry returns null only after ~196 ms and a 129 MiB RSS increase; larger names remain within ZIP's 65,535-byte filename field and can be much worse. Preflight central-directory name length/depth/path spelling before getEntries() (or use a parser that does not synthesize folders), then keep the existing post-parse defense. Add this exact one-entry/deep-path regression; the current mocked count test cannot see it.

Exact-head local verification: the five focused suites pass 33/33, and the previous malformed-layer and subnormal-framerate probes now reject. Those successes do not cover the event-lifecycle or deep-path cases above. The refreshed body correctly discloses the authenticated/POST capture loss.

Fallow remains nonblocking: the only claimed clone is test-fixture code and duplicates no production decision. #363's raw-file sink is removed, but I would not stamp the archive path until pre-materialization name bounds are complete. #364 remains undispositioned while discovery can race and outlive its owner. No alerts should be dismissed. Fresh CI/CodeQL are still running/canceled-restarting at this head.

— Magi

Verdict: REQUEST CHANGES
Reasoning: The original fixes are correct, but asynchronous discovery currently has no lifecycle/concurrency owner and adm-zip can still allocate path-prefix structures before the validator sees the name.

[miguel-heygen]

The two second-round blockers are closed at 42eceb8257c7: response events now collect metadata synchronously, one awaited sequential run owns all fetches and shared-budget debits before save, late events are refused, and the 20 MiB/10-result/10-second bounds are explicit. Raw central-directory name/count/path bounds now run before adm-zip; my exact 5,000-segment probe fell from ~196 ms/+129 MiB RSS to 0 ms/0 MiB.

Blocker — candidate replacement evicts the first worse item, not the worst-priority item. packages/cli/src/capture/lottieDiscovery.ts:50-57 uses .find(candidate => candidate.priority > priority). Because Map preserves response order, a common ordering loses the explicit .json candidate this policy says it protects: collect real.json (priority 1), then 31 ordinary JSON APIs (priority 2), then an .lottie URL (priority 0). The new .lottie evicts real.json, retains all 31 generic APIs, and run() never fetches the real JSON animation. I executed that shape at this head: jsonFetched=false, 31 generic fetches, only the .lottie metadata result survives. Select the maximum numeric priority (with a deterministic tie policy), and add this ordered regression.

Important — external report/body mismatch. The current GitHub body still describes the previous 294-test head and does not document the 32-candidate priority, sequential awaited owner, 20 MiB discovery sub-budget, 10-result cap, or 512-byte/16-segment preflight. The Slack report says the body was refreshed immediately, but GitHub is the underlying source and remains stale. Update it before the next verdict.

Local exact-head verification: five focused suites pass 39/39; delayed completion, duplicate URL, late collect, generic-candidate cap, deep path, alternate footer, nested layers, and duration controls all pass. The public GET/no credentials/POST replay loss remains an acceptable, explicitly fail-closed tradeoff. Fallow's test-only clone remains nonblocking.

#363 is structurally fixed: raw persistence is removed and archive allocation/path bounds now precede adm-zip. #364's security boundary is also sound in source, but I am holding its final disposition and approval until the priority regression is fixed, the body matches, and the fresh CodeQL/native gates settle. No dismissal.

— Magi

Verdict: REQUEST CHANGES
Reasoning: Discovery and archive resource ownership are now correct, but the advertised candidate-priority policy deterministically discards a real .json animation while retaining lower-priority generic API responses.

[miguel-heygen]

Reviewed exact head ad84cdb2f9c3f7e10e11c0144b8e2190c36b8bef.

The priority-cap regression is resolved: replacement now evicts the numerically worst candidate, and stable ordering preserves deterministic oldest-first ties. The exact real.json → 31 generic JSON APIs → anim.lottie case now retains the real candidate and evicts api/0. Earlier lifecycle, bounded discovery, archive preflight, semantic validation, preview request interception, and shared-budget findings remain closed.

Verification: 40/40 focused local tests passed; direct ordered probe fetched real.json, did not fetch api/0, and retained anim.lottie. All eight required checks are green after the unrelated unchanged lint timing test passed on retry; Windows and CodeQL are green, with zero open PR-ref alerts/results. The disclosed credential-less public refetch fail-closed tradeoff and one test-fixture clone warning are acceptable and nonblocking.

— Magi

Verdict: APPROVE

Reasoning: The implementation now enforces the intended bounded, deterministic candidate policy without weakening the Lottie security boundary, all prior blocking findings are resolved, and native gates are fully green at the reviewed head.