fix(portability): vet zip inventory before import

[leonlaiyc]

Problem / Motivation

The dashboard portability import paths constructed zipfile.ZipFile before
checking their member caps. ZipFile materializes the central-directory
inventory during construction, so a crafted import could consume memory before
the existing post-open guard ran.

This is the exact portability residual counted and deferred by the merged review
on #6501.

Why it matters

Portability archives are uploaded input. A cap that runs only after inventory
allocation does not protect preview or apply from the allocation it is meant to
bound.

What changed (motivation → approach → change)

Uploaded archives could materialize an over-cap central-directory inventory
before rejection → ZipFile materializes that inventory during construction,
ahead of the existing post-open cap → run the existing shared
vet_zip_inventory preflight before ZipFile construction in both
validate_import_zip and the defense-in-depth apply_import_zip path.
Preflight rejections use each function's existing return/exception channel, and
the post-open member and uncompressed-size checks remain in place.

Tests

• Red-before: the new regression failed because ZipFile was constructed for
  an archive whose declared member count exceeded the import cap.
• Green: the regression plus the existing member-cap, uncompressed-size-cap,
  and invalid-ZIP tests pass (4/4).
• Touched-file gates pass: isort, flake8, production-file Black, test-line
  Black, and git diff --check.
• Local mypy was unavailable in the reused dependency environment; no packages
  were installed solely for reassurance, so lockfile-complete server CI owns
  the typecheck.

Manual verification

N/A — focused unit coverage directly asserts the pre-construction ordering and
both public import entry points.

Related Issues

No linked issue. This is the reviewer-counted residual from merged #6501.

Pattern harvest

Rule candidate: review-prompt
Pattern: Archive safety caps must run before a parser/materializer performs the
allocation the cap is intended to bound.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## What changed

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## What changed

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6066 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6066: REBASE. Different layers of the same refusal; 6066's contract is prose-agnostic, so 7273's new messages need no follow-up in 6066. Files: src/kiro_crew/dashboard/handlers/portability.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[bolichen97]

Rebased onto main 53987e756 by a maintainer as part of the 2026-09-08 open-PR audit (this branch was ~1000 commits behind).

Conflicts: none, clean rebase. Both insertion points (validate_import_zip, apply_import_zip) and the test anchors around test_import_zip_bomb_member_cap survived unchanged on main, and the merged vet_zip_inventory(source, *, max_members=...) signature plus the too_many_members / cdir_too_large / zip64_saturated reasons still match your call sites. No behaviour was changed.

Gates run locally (changed files only): black --check (pre-existing baseline diff in test/test_portability.py, unrelated to this PR; your added lines are clean), isort --check-only clean, flake8 clean, pytest test/test_portability.py 91 passed / 5 skipped.

Please review the rebase. A maintainer push makes the maintainer the last pusher, so under the repo's last-push rule a second approver is needed. Reply if anything looks wrong.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of d94797e9c26ee2d8838d9bc790d106c4c1631236 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Both dashboard callers run these functions via asyncio.to_thread (src/kiro_crew/dashboard/handlers/portability.py:115,125,175), so the vet's sync-I/O caveat is satisfied. The change reuses the shared zip_vet helper exactly as its module docstring prescribes ("has to run before ZipFile is constructed"), places the guard inside the portability functions so every caller is covered, maps rejection reasons onto each function's pre-existing error channel (return-tuple vs ValueError), and keeps the post-open caps as defense-in-depth mirroring the existing duplicated check at portability.py:734. The new test pins the pre-construction ordering. No fidelity gaps between description and diff; the "Manual verification: N/A" branch is fully CI-exercised.

Design-Verdict: PASS

Closes a counted residual by moving the existing cap ahead of the allocation it bounds, via the shared vet built for exactly this — right layer, right channel.

[DESIGN-REVIEWED] d94797e

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of d94797e9c26ee2d8838d9bc790d106c4c1631236 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence is gathered. Both remaining unvetted ZipFile read sites are digest-pinned downloads (stt decoder verifies a digest on every open; papyrus tectonic is sha256-pinned per platform), so they are not siblings of the untrusted-inventory root cause. The base confirms the defect (ZipFile at portability.py:493/732 before the member cap at :504/:735), the fix reuses the existing shared zip_vet mechanism exactly as its module docstring prescribes, and the new test genuinely fails on base since the monkeypatched AssertionError is not caught by except zipfile.BadZipFile.

First-Principles-Verdict: PASS

Verify the #6501 claim: the description's only provenance link is that merged review; the red-on-base test otherwise carries the defect on its own.

What this change ships

Intent: stop an uploaded import archive from allocating its zip inventory before the size cap can refuse it — a FIX.

Inventory (4 items)

1. Import preview now refuses an over-declared archive inventory before the zip is opened — justified
2. Import apply refuses the same archive before opening it or asking to confirm — justified
3. Oversized/ZIP64 inventories get a distinct "possible zip bomb" rejection message — justified
4. A regression test pins that ZipFile is never constructed for a refused archive — justified

Counted: zipfile.ZipFile( has 12 sites in src/; every untrusted read site is vetted (doc_parser, knowledge, files, now portability ×2), mochi's import blob is capped pre-construction (routes.py:1228), and the two download sites are digest-pinned — 0 unfixed siblings. No new public surface; vet_zip_inventory gains its 4th consumer.

[FIRST-PRINCIPLES-REVIEWED] d94797e

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed d94797e9c26ee2d8838d9bc790d106c4c1631236 via the fork AI-review pipeline; updated in place on each push.

Review details

The candidate requires a legitimate import archive whose central directory exceeds 50000 * 512 = 25.6 MB while carrying ≤ 50 000 entries — i.e. member paths averaging ~466 characters each. Data-home backup contents (session/memory/manifest files) do not produce path names anywhere near that average, and the candidate itself could not establish that the export side ever emits such names. Input (a) does not occur in practice; the finding dies under falsification. No other grounded defect exists in these small, fail-closed hunks.

No findings.

[OPUS-REVIEWED] d94797e

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed d94797e9c26ee2d8838d9bc790d106c4c1631236 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d94797e

[hoang-phan98]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: the two untrusted-archive import parse sites still constructed zipfile.ZipFile before the shared inventory cap ran, so the member-count and central-directory caps were not enforced ahead of the parse; this calls the repo's pre-existing shared vet first and maps its rejection reasons onto the import error paths, one source file plus its test. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.