fix(import): cap GitHub zip expansion before inflate

[SebTardif]

What Problem This Solves

Authenticated Convex GitHub import (previewGitHubImport, previewGitHubImportCandidate, importGitHubSkill) calls unzipSync(zipBytes) with no filter, then checks file count / single-file / total size after the archive has already been inflated.

fetchGitHubZipBytes only caps the compressed download at 25MB. A highly compressible 25MB GitHub zip can expand far past the 80MB unzipped budget inside the Convex action before those post-checks run.

Sibling convex/githubSkillSync.ts already uses createZipEntryLimitFilter so originalSize is enforced before inflate. This change applies the same filter to githubImport.ts unzipToEntries.

Evidence

Live bun against production unzipToEntries with a zip whose uncompressed originalSize is 10MB+1:

$ bun -e "import { zipSync } from 'fflate'; import { __test } from './convex/githubImport.ts'; ..."
THREW ConvexError: Repo archive contains a file that is too large

Same-repo: githubSkillSync unzip filter (createZipEntryLimitFilter). F006 #3612 is the same file's blob-store leak, not unzip bounds.

Real behavior proof

• Behavior or issue addressed: GitHub import unzip rejects oversized entries from zip headers before fflate inflates them.
• Real environment tested: Windows 11, bun 1.4.1, worktree C:\tmp\wt-ch-f012 at 786ce7d.
• Exact steps or command run after this patch: Built an in-memory zip with zipSync containing model.bin of length 10 * 1024 * 1024 + 1 and called __test.unzipToEntries.
• Evidence after fix: terminal output THREW ConvexError: Repo archive contains a file that is too large.
• Observed result after fix: Import unzip fails closed on the single-file cap during the filter callback instead of inflating first and skipping later.
• What was not tested: A 25MB highly compressible archive on live Convex. File-count and 80MB total caps use the same filter and are not separately exercised in this live command.

Tracker

Ref #3678

That issue stays open if this PR is closed without landing on main.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[vercel]

@SebTardif is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel.

A member of the Team first needs to authorize it.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 11, 2026, 10:13 AM ET / 14:13 UTC.

ClawSweeper review

What this changes

Adds ZIP-header limits before GitHub import decompression and a regression test for rejecting oversized files.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 3 items remain

The hardening remains necessary on main and in v0.23.3, but the patch introduces a compatibility regression for repositories containing oversized, unselected files.

Priority: P2
Reviewed head: 786ce7d7dd3e19d4fe64aad4466f3fcae8a7485d

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Focused real runtime evidence supports the hardening, but an avoidable compatibility regression blocks merge.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.
Evidence reviewed	9 items	Verified introduced change: The pinned base-to-head diff adds the filter and changes oversized entries from being skipped to throwing an archive-wide error; only the import implementation and its tests change. Current and shipped behavior: Inspected main and v0.23.3: both call unzipSync without a filter and subsequently skip files larger than 10 MiB. The release resolves to the recorded release SHA, so neither inspected version contains this hardening. Compatibility and caller boundary: Root imports extract the whole repository before candidate selection. Path-scoped imports still skip oversized blobs at lines 724–726. Consequently the new throw can reject a valid root skill because of an unrelated large repository file.
Findings	1 actionable finding	[P1] [P1] Preserve oversized-file skipping before decompression
Security	None	None.

How this fits together

ClawHub imports selected skill files from a signed-in user's public GitHub repositories. Archive extraction feeds candidate discovery, file previews, and publishing.

flowchart TD
  A[Signed-in user] --> B[Owned public repository check]
  B --> C[Download bounded GitHub archive]
  C --> D[Check entry headers]
  D --> E[Decompress accepted entries]
  D --> F[Reject over-limit archive]
  E --> G[Preview and select skill files]
  G --> H[Publish skill]

Loading

Before merge

• [P1] Preserve oversized-file skipping before decompression (P1) - A repository with a valid small skill and an unrelated file over 10 MiB previously imported successfully because extraction skipped that file. This throw now aborts the entire archive before candidate or file selection, breaking root previews/imports even when the large file would never be published. Path-scoped imports still skip oversized blobs at lines 724–726. Return false here before adding to the accepted-byte total, and update the regression test to verify that the small skill survives without inflating the oversized entry.
• Resolve merge risk (P1) - After deployment, previously importable repositories with an unrelated file over 10 MiB would fail root preview/import unless the contributor changes the repository or import path.
• Complete next step (P2) - Preserve oversized-file skipping in the pre-inflate filter and verify compatible imports alongside the count and aggregate-size limits.

Findings

• [P1] [P1] Preserve oversized-file skipping before decompression — convex/githubImport.ts:926-928

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Diff size	+38/-5 across 2 files	The patch is confined to GitHub import extraction and its regression coverage.
Production versus tests	production +28/-5, tests +10/-0	Production growth implements pre-inflate accounting; the added test covers only oversized-file rejection.

Merge-risk options

Maintainer options:

1. Skip oversized entries before inflation (recommended)
   Return false for oversized entries, retain aggregate and count limits, and verify that valid selected skill files remain importable.

Technical review

Best possible solution:

Apply limits before decompression while preserving selective import's oversized-file skipping, and document that distinction from complete-content Skill Sync verification.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: main inflates every ZIP entry before checking limits, and a valid skill alongside a compressible file over 10 MiB demonstrates the patch's skip-to-error regression. No reviewer-side runtime reproduction was executed.

Is this the best way to solve the issue?

Not as written: the pre-inflate filter is the right layer, but returning false for oversized entries provides the same allocation protection without breaking selective imports; post-inflate checks alone cannot prevent excessive allocation.

Full review comments:

• [P1] [P1] Preserve oversized-file skipping before decompression — convex/githubImport.ts:926-928
  A repository with a valid small skill and an unrelated file over 10 MiB previously imported successfully because extraction skipped that file. This throw now aborts the entire archive before candidate or file selection, breaking root previews/imports even when the large file would never be published. Path-scoped imports still skip oversized blobs at lines 724–726. Return false here before adding to the accepted-byte total, and update the regression test to verify that the small skill survives without inflating the oversized entry.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cbfee7343ddc.

Labels

Label changes:

• add P2: This addresses authenticated import resource exhaustion with a limited subsystem scope and no demonstrated production outage.
• add merge-risk: 🚨 compatibility: The new archive-wide error blocks repositories whose oversized files were previously skipped.
• add proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.

Label justifications:

• P2: This addresses authenticated import resource exhaustion with a limited subsystem scope and no demonstrated production outage.
• merge-risk: 🚨 compatibility: The new archive-wide error blocks repositories whose oversized files were previously skipped.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows/Bun terminal trace exercises the changed production unzip helper with real fflate data and observes the oversized-entry error; upstream source confirms this occurs before that entry's inflation. This focused proof does not establish aggregate/count limits or upgrade-compatible success.

Evidence

What I checked:

• Verified introduced change: The pinned base-to-head diff adds the filter and changes oversized entries from being skipped to throwing an archive-wide error; only the import implementation and its tests change. (convex/githubImport.ts:926, 786ce7d7dd3e)
• Current and shipped behavior: Inspected main and v0.23.3: both call unzipSync without a filter and subsequently skip files larger than 10 MiB. The release resolves to the recorded release SHA, so neither inspected version contains this hardening. (convex/githubImport.ts:894, 87ca030c30f3)
• Compatibility and caller boundary: Root imports extract the whole repository before candidate selection. Path-scoped imports still skip oversized blobs at lines 724–726. Consequently the new throw can reject a valid root skill because of an unrelated large repository file. (convex/githubImport.ts:700, 786ce7d7dd3e)
• Sibling filter has a different completeness contract: Skill Sync's oversized-file test explicitly prevents verified content hashes from omitting files. Selective GitHub import instead permits choosing a bounded subset, as documented in specs/github-import.md; copying the sibling's whole-archive rejection is unnecessary to prevent inflation. (convex/githubSkillSync.test.ts:270, 786ce7d7dd3e)
• Affirmative compression dependency: The changed production function calls fflate.unzipSync with a filter; package.json pins fflate 0.8.3. Its filter ordering is therefore directly relevant to the security and compatibility assessment. (package.json:131, 786ce7d7dd3e)
• Upstream filter runs before allocation and inflation: The inspected fflate 0.8.3 source evaluates the filter before copying stored bytes or allocating the inflate output. Returning false safely skips an oversized entry. Repository ownership was checked through the upstream package.json repository field. (src/index.ts:3639)

Likely related people:

• Patrick Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Preserve pre-inflate skipping and add a regression showing a small skill remains importable beside an oversized unselected file.
• Record compatible before/after success and cover file-count and aggregate-size boundaries; document the import-specific safety contract in specs/github-import.md.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[Patrick-Erichsen]

Closing as superseded by the merged #3711, commit 6f92d9e12b33717d16101ca4d3776d9878da3846.

Thank you for the pre-inflation hardening proposal. The landed follow-up retains that approach and fixes the compatibility regression where an unrelated oversized file caused rejection of an otherwise valid skill import. The same three real ZIP regressions were compared against main, this proposal, and the landed fix; current hosted CI is green. Your branch has not been modified.