security: bound source archive extraction - #433
Merged
Conversation
Why: - network-provided ZIPs could drive unbounded inode, path, and extraction work while declaring little expanded data - failures could leave partially extracted source trees until outer cleanup What: - preflight bounded archive metadata and reject unsafe, colliding, or unsupported paths - count actual output under per-file and aggregate limits, honor cancellation, and publish from private staging - add adversarial regression coverage and document the measured extraction budget Proof: - go test -count=1 -race ./internal/sourcearchive - go vet ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - go test -count=1 -tags wago_runtime ./cli/... Docs: - docs/cli-release-hardening.md updated
Why: - archive/zip allocated every central-directory entry before the extraction entry limit ran - ordinary rename could replace a target created during extraction - Unicode and invalid-byte paths made portable collision behavior platform-dependent What: - scan and bound central-directory records and metadata before archive/zip allocation - publish with OS-native atomic no-replace operations on supported release platforms - restrict source paths to printable ASCII and add adversarial regression coverage - record central-directory headroom and the 12,288-byte stripped manager increase Proof: - go test -count=1 -race ./internal/sourcearchive - go test -count=1 ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - go test -count=1 -tags wago_runtime ./cli/... - go vet ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - TinyGo CLI build and Linux/Darwin/Windows cross-compilation Docs: - docs/cli-release-hardening.md updated
Why: - deep valid paths made prefix reconstruction consume over 1 GiB per production-shaped preflight - scanning and archive/zip reopened the pathname, metadata validation was incomplete, and cleanup/close failures could be hidden What: - validate printable-ASCII paths in one scan and model canonical path nodes without rebuilding prefixes - parse and extract from one regular-file descriptor, strictly preflight ZIP modes, methods, encryption, local headers, data ranges, and exact root go.mod - close the archive before atomic publication and join extraction, file-close, and staging-cleanup failures Proof: - go test -count=1 -race ./internal/sourcearchive - go vet ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer Docs: - deferred to the final companion commit with measured benchmark and installer cancellation policy
Why: - archive ownership, metadata rejection, exact go.mod, and linear allocation guarantees need hostile regression coverage What: - add pre-mutation cases for special directory modes, encryption, unsupported methods, malformed headers, invalid compressed ranges, stored-size mismatch, and go.mod spelling - prove one-open pathname stability, zip.ErrInsecurePath closure, single close ownership, cleanup error joining, and no stale cleanup after publication - add a 16,000-file production-shaped benchmark plus a small allocation regression gate Proof: - go test -count=1 -race ./internal/sourcearchive - go test -run '^$' -bench 'BenchmarkPreflight.*Production' -benchtime=1x -benchmem ./internal/sourcearchive - before: 1102951489 ns/op, 1078897384 B/op, 2048079 allocs/op - after: 101775091 ns/op, 19494096 B/op, 48128 allocs/op Docs: - deferred to the final companion commit so the recorded measurement and installer policy land together
Why: - Ctrl+C reached manager archive extraction but the standalone installer fallback still used a background context What: - establish an interrupt-aware installer context and pass it to ExtractContext for archive fallback - verify cancellation after staging creation removes both the target and temporary extraction tree - document strict metadata, one-open ownership, linear preflight measurements, cleanup behavior, and unchanged limits Proof: - go test -count=1 ./cli/installer ./internal/sourcearchive ./cli/manager/internal/version - make docs-check Docs: - docs/cli-release-hardening.md updated with invariants and benchmark measurements
Joshua Tenner (jtenner)
marked this pull request as ready for review
August 13, 2026 13:56
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9de51a2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Why: - central-only checks accepted mismatched local flags, methods, names, and descriptors - per-entry ZIP64 metadata bypassed the EOCD-level rejection - cancellation during the final archive close could still publish What: - cross-check bounded central and local records before staging - validate descriptors and reject all ZIP64 entry metadata - gate publication on cancellation after archive close - add adversarial regressions and correct cleanup documentation Proof: - go test -count=1 -race ./internal/sourcearchive - go test -count=1 ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - go test -count=1 -tags wago_runtime ./cli/... - go vet ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - production-shaped benchmark: 138082584 ns/op, 19593424 B/op, 64135 allocs/op Docs: - docs/cli-release-hardening.md updated
Summary: - require supported ZIP flags and versions plus exact local-record coverage - reject Windows console device aliases and cancel installer downloads - add adversarial descriptor, metadata, path, and cancellation regressions Testing: - go test -count=1 -race ./internal/sourcearchive - go test -count=1 ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - go test -count=1 -tags wago_runtime ./cli/... - go vet ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - GOTOOLCHAIN=go1.22.12 CGO_ENABLED=0 go test -count=1 ./internal/sourcearchive ./cli/manager/internal/version ./cli/installer - make docs-check - make tinygo-test - scripts/build-release-assets.sh (linux/darwin/windows amd64) Docs: - update docs/cli-release-hardening.md with the stricter invariants and benchmark
Why: - Windows trims spaces before extensions when resolving DOS device aliases - descriptor-based ZIP entries must leave local CRC and size fields zero What: - reject whitespace-normalized DOS aliases during allocation-free path preflight - require zero local descriptor fields and add adversarial preflight regressions - preserve and document GitHub's version-needed 1.0 Store/Deflate convention Proof: - go test -count=1 -race ./internal/sourcearchive - Go 1.22, TinyGo, Linux, Darwin, and Windows focused checks - production benchmark: 149437456 ns/op, 20184544 B/op, 64200 allocs/op Docs: - update docs/cli-release-hardening.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security invariants
Accepted paths remain portable printable ASCII and retain the existing duplicate, case-collision, file/directory-conflict, root, depth, component, count, and size rules. Each path is scanned once, canonicalized at most once, and represented by component-sliced canonical nodes.
The archive pathname is opened exactly once. The descriptor remains open through preflight and extraction, is closed exactly once before publication, and is closed on every error including zip.ErrInsecurePath. Central-directory metadata, local headers, signed or unsigned data descriptors, and the exact root go.mod requirement are validated before target-parent or staging creation. ZIP64 metadata is rejected at the EOCD, central-entry, and local-entry levels.
Extraction still occurs in a private sibling tree and publishes with the existing OS-native atomic no-replace operation. Cancellation is checked again after archive close and immediately before publication. Failed cleanup and archive/file close errors are returned rather than discarded.
Benchmark
Go 1.26.5, linux/amd64, Ryzen 7 7800X3D; 16,000 files with 63 shared uppercase directories and zero expanded bytes:
Validation
The unconfigured local go test -count=1 ./... run reaches only the interpreter-gated staged Core 3 cases; the pinned official interpreter cannot be built locally because dune installation requires unavailable sudo credentials. The PR native CI matrix provisions that toolchain.
Limits
Production ceilings are unchanged: 20,000 entries, 16,000 files, 4,000 directories, 16 MiB central-directory metadata, 64 relative components, 1,024 path bytes, 255 bytes per component, 128 MiB per file, and 512 MiB aggregate expanded content.
Fixes #405.