[dogzzdogzz]

What problem does this solve?

openab-core resolves zip 2.4.2. Several zip-rs/zip2 advisories cover that version, including CVE-2026-115905 (critical, "Dual-Header Inconsistencies / Zip Cloaking") and GHSA-wjr6-mhvj-cjvm (ZipArchive::new EOCD fallback). The table below lists all of them. The crate is used only by the pre-seed feature, which is on by default in the root binary. That feature parses archive bytes downloaded from S3 with zip::ZipArchive::new.

Closes #1579

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365158868619404/1564673005240451224

Review Contract

Goal

Move zip to a fixed release (9.1.2) and keep pre_seed zip extraction behaviour as close to 2.4.2 as possible: same flate2 backend (miniz_oxide), and escaping or root-anchored entry names are still rejected. The two remaining zip 9 path differences are listed under Accepted Residual Risks.

Non-goals

• Changing pre_seed extraction semantics (limits, permissions, overwrite).
• Hardening the size budget to count actual bytes written instead of the header-declared size(). That is a separate concern from this advisory and goes in a follow-up issue.
• Bumping other dependencies.

Accepted Residual Risks

• This is a 2.x → 9.x major jump. The APIs pre_seed.rs uses (ZipArchive::new, by_index, enclosed_name, name_raw, is_dir, size, unix_mode) compile, and the pre_seed tests pass. Path handling is now pinned by extract_rejects_path_traversal. Edge-case archives outside the tests could still behave differently. Recovery is to revert this squash commit.
• zip 9 enclosed_name() normalises two cases differently from 2.x, and both stay inside dest:
  • \ is treated as a path separator, so a\b.txt extracts to dest/a/b.txt instead of a flat file named a\b.txt.
  • A leading drive prefix is stripped, so C:/x extracts to dest/x. On 2.x under Unix it extracted to dest/C:/x.
    Rejecting drive prefixes would be a new restriction rather than parity, so it is left as is.
• zip 9.x needs Rust >= 1.88. The Dockerfiles build on rust:1-bookworm and CI's build-builder and smoke tests are green. A builder pinned to an older toolchain would fail at build time, not at runtime.

Acceptance Criteria

• Cargo.lock resolves zip 9.1.2. Compared with main, the lockfile diff is limited to zip's transitive deps: it removes arbitrary, crossbeam-utils, derive_arbitrary and zopfli, and adds typed-path. The only new edges are zip's own (getrandom 0.4.3, nix, typed-path, windows-sys 0.61.2) plus getrandom 0.4.3's wasm32-only js-sys/wasm-bindgen. No unrelated crate edges change. flate2 still depends on miniz_oxide, and zlib-rs is not in the lockfile.
• Zip entries named ../x, a/../../x, /x and \x are rejected, and nothing is written (extract_rejects_path_traversal). The test passes on both zip 2.4.2 and 9.1.2.
• cargo clippy --all-targets -- -D warnings is clean.
• cargo clippy --workspace --features unified -- -D warnings is clean.
• The pre_seed tests pass (cargo test -p openab-core --features pre-seed pre_seed).
• cargo test --bin openab passes.
• CI is green.

Follow-ups

• Extraction budget should count actual bytes written, not the header-declared size() (low priority: zip >= 9.0.1 already fails reads that exceed the declared size, fix: fail reads that exceed the declared uncompressed size zip-rs/zip2#1022).
• Add cargo audit / cargo deny to CI (separate PR).

At a Glance

S3 object ──> pre_seed::download_and_extract ──> extract_zip_budgeted
                                                   ├─ zip::ZipArchive::new   <- zip 2.4.2 -> 9.1.2
                                                   ├─ reject raw names starting with / or \   (new, keeps 2.x behaviour)
                                                   └─ enclosed_name() -> by_index + io::copy
flate2 backend (zip + tar.gz GzDecoder): miniz_oxide (unchanged)

Advisory coverage

pre_seed builds zip with default-features = false, features = ["deflate-flate2"] (no AES, no legacy-zip). It reads entries only through ZipArchive::new + by_index + std::io::copy. It never calls ZipArchive::extract() or ZipStreamReader, it never creates symlinks from zip entries, and it masks modes to 0o777.

Advisory	Affected	Fixed	Affects 2.4.2?	Reachable from pre_seed?
GHSA-m665-v7pm-7grj / CVE-2026-115905 (critical): dual-header inconsistencies	<= 9.0.4	9.0.5	yes	yes: entry metadata read via by_index
GHSA-wjr6-mhvj-cjvm: ZipArchive::new EOCD fallback	>= 2.1.4, <= 9.0.5	9.1.0	yes	yes: ZipArchive::new
GHSA-gmfj-vffv-32pw: strict stream validation, missing footer / shifted offsets	< 9.1.0	9.1.0	yes	fixed by the bump either way
GHSA-q7v9-x93c-xqv3 / CVE-2026-115909: unbounded symlink-target handling	>= 1.3.0	9.0.0	yes	no: extraction-path symlink handling, pre_seed creates no symlinks
GHSA-5qh5-c9fp-66jm: Windows symlink → SMB/NTLM	>= 1.3.0	9.0.0	yes	no: Windows + symlink extraction
GHSA-8hxf-qr8p-62x4 / CVE-2026-115906: AES HMAC bypass on premature EOF	>= 0.6.0	9.0.0	yes	no: AES disabled
GHSA-c9rm-qm35-rhqf: AES HMAC skipped on early stop	<= 8.6.0	9.0.1	yes	no: AES disabled
GHSA-4684-h2q9-2vcp: extract() applies perms through symlinks	<= 9.0.0	9.0.1	yes	no: extract() only
GHSA-4v9h-p7m9-rh26: extract() symlinks outside dest	<= 9.0.0-pre3	9.0.0	yes	no: extract() only
GHSA-vv5m-jxxh-4627: SUID/SGID preserved on extraction	<= 9.0.0-pre3	9.0.0	yes	no: pre_seed masks to 0o777
GHSA-9hj8-73fh-9963 / CVE-2026-115907: legacy Shrink/Reduce/Implode DoS	>= 5.1.0	9.0.0	no	no: legacy-zip not enabled
GHSA-hjqx-j4rw-g7xv / CVE-2026-115908: extract() lexical symlink check	>= 7.2.0	9.0.0	no	no: extract() only
GHSA-x2x8-gq67-9mq6: default symlink action creates escaping links	>= 9.0.0	9.0.2	no	no: 9.1.2 includes the fix

Source: gh api repos/zip-rs/zip2/security-advisories (2026-10-27).

Prior Art & Industry Research

Not applicable. This is a security dependency bump with no design change.

Proposed Solution

• crates/openab-core/Cargo.toml: zip = { version = "9.1.2", default-features = false, features = ["deflate-flate2"], optional = true }, then cargo update -p zip --precise 9.1.2.
  • zip 9's deflate feature turns on flate2/zlib-rs. Through Cargo feature unification that would switch the flate2 backend for both zip and the tar.gz (GzDecoder) path from miniz_oxide to zlib-rs.
  • deflate-flate2 pulls in flate2 without picking a backend, so the existing flate2 = "1" default (miniz_oxide) stays in effect. It also drops the write-only zopfli.
• pre_seed.rs: in zip 9, enclosed_name() strips a leading root, so /abs.txt would extract to dest/abs.txt. zip 2.x rejected that entry name. The new check rejects raw names that start with / or \ before calling enclosed_name(), so these archives still fail closed.
• New test extract_rejects_path_traversal pins the contract.
• Cargo.lock is regenerated from main with cargo update -p zip --precise 9.1.2 only.

Why this approach?

• 9.1.2 over 9.0.5 (the minimum fix for CVE-2026-115905): GHSA-wjr6-mhvj-cjvm is fixed only in 9.1.0. It affects ZipArchive::new, which is the exact entry point pre_seed uses. GHSA-gmfj-vffv-32pw is also fixed in 9.1.0.
• Caret requirement: "9.1.2" means ^9.1.2. Later 9.x security patches are picked up, and the vulnerable 9.0.0–9.0.5 releases are excluded.
• Fail closed on absolute names: rejecting them keeps behaviour identical to 2.4.2 and needs no reasoning about how 9.x normalises roots.

Alternatives Considered

• Pinning exactly 9.0.5: it misses GHSA-wjr6-mhvj-cjvm, which reaches ZipArchive::new.
• Keeping features = ["deflate"] and documenting the zlib-rs switch: rejected, because it silently changes the decompressor for untrusted S3 bytes on both archive formats.
• Accepting zip 9's root-stripping for absolute names: contained, but a silent behaviour change. Rejecting is simpler to reason about.
• Replacing zip or dropping .zip support from pre_seed: a larger change, not justified here.

Validation

macOS arm64, rustc 1.99.1, head 2171842:

• cargo clippy --all-targets -- -D warnings: clean
• cargo clippy --workspace --features unified -- -D warnings: clean
• cargo test -p openab-core --features pre-seed pre_seed: 34 passed, 0 failed (includes the new extract_rejects_path_traversal)
• cargo test --bin openab: 18 passed, 0 failed
• Cargo.lock: flate2 1.1.9 depends on crc32fast and miniz_oxide; no zlib-rs and no zopfli packages.

Originally authored by @dogzzdogzz; F1/F2/F3 from the round-1 review addressed by maintainers in 0d9ac3b and 2171842.

[chaodu-agent]

Pushed 0d9ac3b on top of your commit to address F1 and F2 (maintainer edit, no force-push).

F1: flate2 backend. Switched to features = ["deflate-flate2"]. flate2 now resolves to crc32fast + miniz_oxide again, and zlib-rs and zopfli are no longer in Cargo.lock. Versus main, the lockfile delta is now only -arbitrary -crossbeam-utils -derive_arbitrary -zopfli +typed-path.

F2: traversal test, plus one behaviour change it exposed. zip 9's enclosed_name() strips a leading /, so /abs.txt extracted to dest/abs.txt. On zip 2.4.2 the same entry failed with invalid zip entry name. Nothing escaped dest, but this was a second change hidden behind "no behaviour change". I added an explicit reject for raw names starting with / or \, so these archives fail closed as before. The new test extract_rejects_path_traversal covers ../escape.txt, sub/../../escape.txt, /abs_escape.txt and \abs_escape.txt. Each case must be rejected and leave dest empty.

A correction to the commit body: its last line says the test fails on main. That is not right for the final version. With the explicit reject, every case is also rejected on zip 2.4.2, so the test passes on main and pins the 2.x contract across the bump. It failed on main only in an earlier draft that asserted the 9.x strip behaviour.

Verified on macOS arm64:

• cargo test -p openab-core --features pre-seed pre_seed: 34 passed
• cargo clippy -p openab-core --features pre-seed -- -D warnings: clean
• cargo clippy --workspace -- -D warnings: clean

Still with you (description only), F3:

• In "Why this approach?", state why 9.1.2 and not 9.0.5: GHSA-wjr6-mhvj-cjvm, fixed in 9.1.0, affects ZipArchive::new.
• Add a short GHSA table: fixed by this bump vs. N/A for openab's usage, with the reason for each N/A.
• Update the "no behaviour change" wording: the backend stays on miniz_oxide, and absolute entries are rejected explicitly.

[chaodu-agent]

/review

[chaodu-agent]

Round 2 addressed:

• F8: 2171842 regenerates Cargo.lock from main with only cargo update -p zip --precise 9.1.2. The unrelated windows-sys/getrandom downgrades are gone.
• F3: the description now has the 9.1.2-vs-9.0.5 rationale, a per-advisory table with precise ranges, current wording for the feature, lockfile and test count (34), and the two remaining zip 9 path differences (\\ as separator, drive-prefix stripping) under Accepted Residual Risks.
• The inaccurate last line of 0d9ac3b's commit body will be corrected in the squash message.

/review

[chaodu-agent]

/review

[chaodu-agent]

Note

LGTM ✅ - Round 3: every finding is resolved. 2171842 re-resolves Cargo.lock from main, so the lockfile diff now touches only zip, and the updated description accurately states the remaining zip 9 path differences.

What This PR Does

openab-core resolves zip 2.4.2, which is affected by several zip-rs/zip2 advisories, including CVE-2026-115905 (critical) and GHSA-wjr6-mhvj-cjvm. The latter reaches ZipArchive::new, which the default-on pre-seed feature uses to parse S3-downloaded archives. This PR bumps zip to 9.1.2 and keeps pre_seed behaviour as close to 2.4.2 as possible.

How It Works

• 6130b84: bumps zip to 9.1.2.
• 0d9ac3b: switches the zip feature to deflate-flate2, so flate2 keeps miniz_oxide. Adds a fail-closed reject for raw entry names starting with / or \. Adds extract_rejects_path_traversal.
• 2171842 (new): regenerates Cargo.lock from main with only cargo update -p zip --precise 9.1.2.

Findings Status

#	Finding	Status
F1	deflate switched flate2 to zlib-rs	✅ Fixed in 0d9ac3b
F2	No path-traversal regression test	✅ Fixed in 0d9ac3b
F3	Version rationale / advisory coverage / behaviour wording	✅ Fixed in the description
F8	Unrelated lockfile downgrades	✅ Fixed in 2171842

Findings (Round 3)

#	Severity	Finding	Location
F15	🟢	Independently verified Cargo.lock at 2171842 against main (d59f7de). The only package changes are: zip 2.4.2 -> 9.1.2; arbitrary, crossbeam-utils, derive_arbitrary and zopfli removed; typed-path added. The only edge changes are zip's own (getrandom 0.4.3, nix, typed-path, windows-sys 0.61.2) plus the wasm32-only js-sys / wasm-bindgen on getrandom 0.4.3. No unrelated crate edges change. flate2 depends on crc32fast + miniz_oxide, and there is no zlib-rs.	Cargo.lock
F16	🟢	The Goal now claims only what holds (same backend; escaping and root-anchored names rejected). Accepted Residual Risks names the two remaining zip 9 differences: \ is treated as a separator, and a leading drive prefix is stripped. Both stay inside dest. The rationale for not rejecting drive prefixes (new restriction, not parity) is reasonable.	PR description
F12	🟢	The advisory table matches the published zip-rs/zip2 advisories: all 13 rows agree on affected range, patched version and CVE mapping. The omitted GHSA-94vh-gphv-8pm8 / CVE-2025-29787 was patched in 2.3.0 and does not affect 2.4.2.	PR description
F13	🟢	The case for 9.1.2 over 9.0.5 is explicit and correct: GHSA-wjr6-mhvj-cjvm (>= 2.1.4, <= 9.0.5) is fixed only in 9.1.0 and reaches ZipArchive::new.	PR description
F9	🟢	The / / \ guard in front of enclosed_name() is equivalent to checking the decoded name, because zip 9.1.2 name() decodes the raw bytes as UTF-8 or CP437 and ignores the Unicode Path extra field. It also returns the existing error message.	crates/openab-core/src/pre_seed.rs:228-233
F10	🟢	The regression test asserts both the error and the filesystem state, and passes on both zip 2.4.2 and 9.1.2, so it pins the 2.x path-safety contract across the bump.	crates/openab-core/src/pre_seed.rs (extract_rejects_path_traversal)
F11	🟢	The net dependency tree is smaller than on main, and the decompression backend is unchanged.	crates/openab-core/Cargo.toml:44

Non-blocking Notes

• Squash message: drop or correct the last line of the 0d9ac3b body ("Verified it fails on main's zip 2.4.2 only for cases it newly pins"), as the fix note already acknowledges.
• Optionally add one more case to the residual-risk note: zip 9 is also stricter in one direction. On 2.x under Unix, a\..\..\x extracted as a single literal filename; zip 9 now rejects it. This is not a risk.
• CI on 2171842 had not finished when this review was posted (5 success, 2 skipped, 2 in progress). The lockfile change from 0d9ac3b only moves Windows edges and tempfile -> getrandom back to main's resolution, but please confirm CI is green before merging.
• Follow-ups recorded in the description: actual-bytes size budget (lower priority, since zip >= 9.0.1 already errors on reads past the declared size), and cargo audit / cargo deny in CI.

Addressing Feedback on This PR

• Fix note (comment 6448054112): F1 and F2 claims verified in round 2. The lockfile-delta claim made there is now true at 2171842 (F15).
• Round 2 follow-up note: "F8: 2171842 regenerates Cargo.lock from main ... The unrelated windows-sys/getrandom downgrades are gone." ✅ Verified by full diff against main (F15). "F3: the description ..." ✅ Verified (F12, F13, F16).
• /review (comment 6448186065): answered by rounds 2 and 3.
• No other external comments, reviews, or inline threads exist on this PR.

Agent Team Verdicts

Reviewer	Round	Angle	Verdict	Key finding(s)
Reviewer A	1	Performance/Efficiency	CHANGES REQUESTED -> resolved	F1
Reviewer B	1	Simplicity	CHANGES REQUESTED -> resolved	F1
Reviewer C	1	General + Readability	CHANGES REQUESTED -> resolved	F1, F2
Reviewer D	1	Security/CI	CHANGES REQUESTED -> resolved	F1, F3
Reviewer E	1	Docs/UX	CHANGES REQUESTED -> resolved	F1, F3
Reviewer F	1 (late)	General + Architecture	LGTM	enclosed_name stricter in 9.x (now disclosed)
Reviewer G	1 (late)	General + Operability	CHANGES REQUESTED -> resolved	backend disclosure; \ separator change (now disclosed)
Coordinator	2	Re-review of 0d9ac3b	CHANGES REQUESTED	F3, F8
Coordinator	3	Re-review of 2171842 + description	LGTM	all resolved

Devil's Advocate: Three Reasons We Might Not Need This PR

1. Exposure is limited to operator-controlled S3 objects. Counter: pre-seed is on by default, and 2.4.2 is affected by a critical advisory that reaches the by_index path.
2. A 2.x -> 9.x jump on a parser carries edge-case risk. Counter: the backend is unchanged, path safety is pinned by a test, the remaining differences are disclosed, and recovery is a single revert.
3. Dropping .zip support from pre-seed would remove this advisory class entirely. Counter: that is a user-facing feature removal and needs its own decision; this PR is the minimal fix.

Baseline Check

• PR opened: 2026-10-27
• Head SHA: 2171842ce0ed550b8b1f158a2a5e71a3f8bc085c (3 commits: 6130b84, 0d9ac3b, 2171842)
• Base: main @ d59f7de2cccf9b06dbbf02af79c6344b93dbe0a9 (not stacked)
• Files: Cargo.lock, crates/openab-core/Cargo.toml, crates/openab-core/src/pre_seed.rs
• Net-new value: moves zip out of all advisories affecting 2.4.2 that pre_seed can reach, without changing the decompression backend or the fail-closed path contract.

What's Good (🟢)

• Each finding was fixed minimally and verifiably, and the fix notes were candid, including a self-correction on the commit body.
• The description is a model security-bump write-up: an accurate advisory table, an explicit version rationale, honest residual risks and recorded follow-ups.
• The dependency tree is smaller than on main.

[chaodu-obk]

LGTM ✅ - Round 3: all findings (F1, F2, F3, F8) resolved; zip 9.1.2 bump keeps the miniz_oxide backend, fails closed on root-anchored entry names, and the lockfile diff vs main is limited to zip.

Full review: #1580 (comment)

Please confirm CI is green on 2171842 before merging.

[thepagent]

LGTM. Round 3 bot review clean, all checks green. Approving as code owner.