fix(archive): support bounded local PAX metadata - #147
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
|
Codex review: needs real behavior proof before merge. Reviewed August 27, 2026, 7:29 PM ET / 23:29 UTC. ClawSweeper reviewWhat this changesThis PR adds bounded local PAX TAR metadata support to JavaScript and native archive processing, with documentation, regression coverage, and native CI coverage. Merge readiness⛔ Blocked until stronger real behavior proof is added - 4 items remain The bounded PAX implementation is necessary because current main rejects local PAX headers, and the reviewed JavaScript/native paths retain a narrow fail-closed design. The external PR still needs inspectable after-fix runtime evidence, and the current Node 24 failure needs resolution before merge. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — step 1 Assertions:
How this fits togetherArchive inputs pass through metadata preflight before JavaScript or native extraction reads entries and writes into guarded destinations. Local PAX metadata can change the next TAR member’s path or framing, so both implementations validate it before existing path, size, and link policies run. flowchart LR
A[Archive input] --> B[Metadata preflight]
B --> C[Bounded local PAX validation]
C --> D[Effective member framing]
D --> E[Existing path and limit checks]
E --> F[Guarded extraction or entry read]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Attach a redacted public-API terminal transcript or equivalent artifact showing extraction and bounded entry reads of the cited PAX archive in both JavaScript-only and required-native modes, then resolve the failing Node 24 check before landing. Do we have a high-confidence way to reproduce the issue? Yes, at source level: pinned current main rejects every local PAX header before either parser can process it, while this PR changes that exact branch. The supplied PR description also names a concrete public archive fixture, but no local execution was performed in this read-only review. Is this the best way to solve the issue? Yes, provisionally: bounded byte-level parsing ahead of the existing path, link, byte-budget, and staging checks is a maintainable way to support the needed subset without accepting ambiguous extension chains. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ed6b6d4b988d. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(archive): support bounded local PAX metadata This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
1 participant
Summary
Replace the blanket local-PAX rejection with bounded byte-level interpretation in both JavaScript and native TAR meters. PAX size overrides now drive framing, while effective paths and sizes continue through the existing extraction/read policy. Original archive bytes are not rewritten.
This fixes standard release archives that contain fractional timestamps and binary macOS provenance metadata. The implementation intentionally supports a narrow, documented local-PAX subset: ambiguous records, duplicate keys, global/old headers, mixed extension chains, unsupported structural text, and sparse semantics still fail closed. Xattrs and ownership metadata are accepted only as inert metadata and are not restored. Existing limits, guarded staging, link rejection, and package dependencies are unchanged.
Keep complete-input native validation and terminal cancellation alongside PAX support: the TAR reader stops at end markers and buffers PAX through a retrying read helper. Also return JS TAR-read traversal failures through the public promise. Add native-required PAX coverage to Linux, macOS, Windows, and musl CI lanes.
Verification
mode: "off"andmode: "require". Download verified at 30,134,912 bytes, SHA-2562216da0acbcc6e822ee341ec313aaab58875db951fa1daf0d13dd710ebfba9b8. Both extracted member hashes match independent expectations; sizes are 109,941,296 and 6,871,408 bytes, with executable modes retained.FS_SAFE_PAX_REQUIRE_NATIVE=1 pnpm check: 1,191 passed, 61 baseline skips, including build/docs/package/public-API checks.pnpm native:test: 23 passed; native build, Clippy with warnings denied, workflow lint, andgit diff --checkpassed.No consumer overrides, cloud credentials/resources, downloaded executable execution, or dependency changes were used. This PR does not publish a package release.
Recorded public-API runtime output
The following is the observed local runtime output from the built public exports for the exact source committed as
2fea6a51a7f6834aa07a01f6f68e1f6c5980fe1a, not an agent transcript. The real archive is unchanged; both native and JavaScript modes are explicit.Observed extraction and bounded reads: mode off
{ "phase": "preland", "mode": "off", "success": true, "files": [ { "name": "crabbox", "size": 109941296, "sha256": "9a4a0b312c26e40e1f4da897b284d6f2ac3f9ac73bbb5e3fe1c957200cf06452" }, { "name": "crabbox-apple-vm-helper", "size": 6871408, "sha256": "ba38d42445d202c843bff099476e115312555733f6d6a8a7a4de0ecbd585c45e" } ] }{ "phase": "preland", "mode": "off", "success": true, "checks": [ { "check": "read-exact-bytes", "name": "crabbox", "success": true, "size": 109941296, "sha256": "9a4a0b312c26e40e1f4da897b284d6f2ac3f9ac73bbb5e3fe1c957200cf06452" }, { "check": "read-exact-bytes", "name": "crabbox-apple-vm-helper", "success": true, "size": 6871408, "sha256": "ba38d42445d202c843bff099476e115312555733f6d6a8a7a4de0ecbd585c45e" }, { "check": "read-over-limit-rejected", "success": true }, { "check": "extraction-meta-limit-rejected", "success": true, "code": "archive-meta-entry-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-count-limit-rejected", "success": true, "code": "archive-entry-count-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-entry-limit-rejected", "success": true, "code": "archive-entry-extracted-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-total-limit-rejected", "success": true, "code": "archive-extracted-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-exact-all-limits", "success": true } ] }Observed extraction and bounded reads: mode require
{ "phase": "preland", "mode": "require", "success": true, "files": [ { "name": "crabbox", "size": 109941296, "sha256": "9a4a0b312c26e40e1f4da897b284d6f2ac3f9ac73bbb5e3fe1c957200cf06452" }, { "name": "crabbox-apple-vm-helper", "size": 6871408, "sha256": "ba38d42445d202c843bff099476e115312555733f6d6a8a7a4de0ecbd585c45e" } ] }{ "phase": "preland", "mode": "require", "success": true, "checks": [ { "check": "read-exact-bytes", "name": "crabbox", "success": true, "size": 109941296, "sha256": "9a4a0b312c26e40e1f4da897b284d6f2ac3f9ac73bbb5e3fe1c957200cf06452" }, { "check": "read-exact-bytes", "name": "crabbox-apple-vm-helper", "success": true, "size": 6871408, "sha256": "ba38d42445d202c843bff099476e115312555733f6d6a8a7a4de0ecbd585c45e" }, { "check": "read-over-limit-rejected", "success": true }, { "check": "extraction-meta-limit-rejected", "success": true, "code": "archive-meta-entry-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-count-limit-rejected", "success": true, "code": "archive-entry-count-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-entry-limit-rejected", "success": true, "code": "archive-entry-extracted-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-total-limit-rejected", "success": true, "code": "archive-extracted-size-exceeds-limit", "destinationEmpty": true }, { "check": "extraction-exact-all-limits", "success": true } ] }The automatic review environment could not build its native binding because no Rust toolchain was configured. That is not a runtime result for this change. A working local Rust toolchain built the binding for the results above; all four GitHub native lanes also passed. Build JavaScript before staging the binding (
pnpm build, thenpnpm native:build), since the JavaScript build clearsdist.