Skip to content

fix(archive): validate 7z entry paths before extraction - #1226

Merged
loonghao merged 4 commits into
mainfrom
fix/archive-sevenz-path-validation
Oct 16, 2026
Merged

loonghao merged 4 commits into
mainfrom
fix/archive-sevenz-path-validation

Conversation

@loonghao

@loonghao loonghao commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Path-safety fix

  • Validate 7z entry names before the public vx-runtime-archive extractor writes them, addressing the unsafe default extraction pattern in RUSTSEC-2026-0245.
  • Reject traversal, absolute/drive/ADS paths, empty or dot-only names, Win32 trailing-dot/space components, and existing symlink/reparse-point paths. Preserve ordinary nested paths with either separator style.
  • Exercise generated archives natively on Linux, Windows, and macOS with extended-formats, zero-test failures, and retries disabled.

Integration follow-ups

Rebased onto main at 5a85987d9c77a73a0e67f9141d1df19a5ad8ac06. Current head: bedcf849cf178235012a1b58bbc915facd0d0b2a.

  • Refresh one generated workspace-hack feature line: syn 3 gains visit-mut, as required by async-trait 0.1.92. This is the exact hakari-generated delta; Cargo.lock is unchanged. The previous PR job only warned about this drift, while postmerge main failed, so the new run must show no generated drift rather than merely a green check.
  • Stabilize only test_where_alias_imagemagick using an isolated managed fixture and explicit fixture version. Both alias and canonical requests must succeed and return the exact expected JSON path/version/source. This replaces the host-dependent boolean comparison, which also allowed both commands to fail. No production alias behavior, retries, or skips are changed.

Validation

  • Independent review of the path checks and both small integration follow-ups found no blocking code issues.
  • Formatting, patch checks, and 149 script tests pass.
  • On original head 2e1e0dc, real new archive regressions passed Linux 22/22, Windows 20/20, and macOS 22/22, with zero skips and retries disabled.
  • That original head's full Windows run absorbed one ImageMagick alias failure on retry. It is not treated as clean validation; the fixture change above addresses the demonstrated test-isolation gap. The old assertion discarded subprocess output, so the precise first-attempt subprocess failure cannot be established from its log.
  • Offline smoke with official vx 0.9.37 verified the fixture despite conflicting project/PATH settings; removing the fixture makes both lookups fail, and removing alias metadata makes only the alias fail. The empty fixture is not executed. This is not a substitute for exact-source cross-platform CI.
  • On rebased head 84812e3, archive regressions again passed Linux 22/22, Windows 20/20, and macOS 22/22. Code Quality reported both "no changes detected" and "no operations to perform" for hakari. Full Linux 4289/4289 and macOS 4292/4292 passed with no retry-absorbed failures.
  • That head's Windows full run passed 4312 tests and failed the new fixture's JSON assertion: the expected path mixed store/magick with native backslashes, while vx returned the correct native path. The follow-up constructs each path component with a separate join; it preserves the complete JSON assertion and changes no production code. Independent review approved this one-line correction.
  • Exact current head bedcf849cf178235012a1b58bbc915facd0d0b2a: 41 successful checks, 17 conditionally skipped, no failed/pending checks. Full Linux passed 4289/4289 (128 existing skips), full macOS passed 4292/4292 (128 existing skips), and full Windows passed 4313/4313 (130 existing skips). All three logs report zero retry-absorbed failures, including first-attempt success for the ImageMagick alias fixture.
  • Current-head native archive cases again passed Linux 22/22, Windows 20/20, and macOS 22/22, zero skips with retries disabled. Code Quality reports no hakari changes and no dependency operations; formatting and clippy passed. Its inherited inline-test migration warning remains advisory-only.
  • Immediately before merge, main remains 5a85987d9c77a73a0e67f9141d1df19a5ad8ac06; there are no review threads or outstanding review requests. The independent reviews above remain applicable. Separate compatible dependency patch work is tracked in fix(deps): patch four compatible locked dependencies #1227.

Boundaries

vx-runtime-archive is an optional-feature public API; current CLI production crates no longer depend on it. CLI HTTP/installer 7z callbacks already validate entry paths. Existing-link preflight checks are not an atomic guarantee against concurrent symlink replacement. This patch does not remove unmaintained sevenz-rust, add advisory ignores, change audit settings, or claim the workspace audit is clean. Dependency migration and existing audit failure suppression remain separate work.

The 84812e3 audit log reports 10 vulnerabilities and 10 denied warnings, despite a green job. Its preceding lock regeneration failed because all matching zip 8.x releases were yanked; the errors are suppressed by the existing recipe. These findings apply to inherited dependencies in the committed lock. Earlier 3/7 results followed successful lock regeneration and must not be used as the committed-lock total. This PR does not change Cargo.lock or use that audit job's green status as evidence of security clearance.

@loonghao
loonghao force-pushed the fix/archive-sevenz-path-validation branch from 2e1e0dc to 84812e3 Compare October 16, 2026 03:40
@loonghao
loonghao marked this pull request as ready for review October 16, 2026 05:40
@loonghao
loonghao merged commit ed36726 into main Oct 16, 2026
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant