fix: guard lock handoffs and ZIP read integrity - #189
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: passed. Reviewed September 1, 2026, 2:47 AM ET / 06:47 UTC. ClawSweeper reviewWhat this changesThe branch hardens sidecar-lock handoffs and synchronous lock recovery, verifies native ZIP entry lengths, and adds focused tests, documentation, and CI coverage. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessThis PR remains necessary because its fixes are not on current main. The exact reviewed head has no actionable correctness or security finding and has sufficient real-behavior proof, so it should proceed through its normal merge gates. Priority: P2 Review scores
Verification
How this fits togetherThis package provides capability-style filesystem roots and guarded helpers for Node.js applications handling untrusted paths. The changed lock and archive readers validate filesystem or archive inputs before returning a lock handle or decoded bytes to callers. flowchart LR
A[Application lock request] --> B[Root capability]
B --> C[Sidecar lock admission]
C --> D[Identity and ancestry checks]
D --> E[Held lock or safe retry]
F[ZIP archive input] --> G[Native ZIP reader]
G --> H[Size and CRC validation]
H --> I[Returned entry bytes]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the exact reviewed hardening through normal merge gates, preserving the guarded Root APIs and the focused cross-platform regressions. Do we have a high-confidence way to reproduce the issue? Yes. The supplied installed-consumer traces exercise the public lock and ZIP paths after the fix, including cross-process contention and Windows replacement rejection. Is this the best way to solve the issue? Yes. The patch strengthens the existing Root and sidecar-lock safety boundaries rather than adding a parallel filesystem mechanism, and the native ZIP check aligns the backend with the established JavaScript contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against abad7c213615. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
|
@clawsweeper re-review Please review exact head |
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Exact head: The main PR body now includes all 18 actual packaged-Windows authority trace rows across off/auto/require: real replacement before Root admission is rejected with zero held registrations, a closed descriptor, preserved replacement/original bytes after cleanup and drain, and an intact outside canary. Separate explicitly labeled open-denial fault controls prove fresh exclusive creation is required and a foreign replacement cannot yield reentrant ownership or deletion authority. Fresh installed Windows contention also passed all 1,200 acquisitions; combined Linux/Windows source+installed proofs total 4,800. The failed Windows checks were confined to a test fixture assuming |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
|
@clawsweeper automerge Special instructions: The main body contains the complete packaged Windows authority trace, actual short-path before/after proof, Linux installed-consumer ZIP/lock/release controls, and 4,800 Linux/Windows source+installed contended acquisitions. Full Linux validation passed 6,274 tests, 63 Rust tests, 78 security tests, and 150 selected native regressions. The final test-only inode-fixture correction passed 103 Windows tests and scoped-clean Codex autoreview; all production/package inputs are unchanged from the fully reviewed and proved candidate. Do not change source, rebase, bypass checks, start unrelated work, create tags, bump versions, publish packages, or modify existing 0.7.0 release artifacts. If the head changes or a gate regresses, stop rather than merging another candidate. This authorizes landing the existing reviewed fixes only; a future 0.7.1 release is separate. |
|
🦞✅ Source: What merged:
Automerge notes:
The automerge loop is complete. Automerge progress:
|
clawsweeper
Bot
added
clawsweeper:automerge
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Landed as 59b9cc4ea1b12defd7e693aa3af8d56581d52e9f. The merged tree exactly matches the reviewed final candidate ( Validation included Final CI, coverage, and CodeQL passed, including native Windows/Linux/macOS/musl and the Node 22/24 OS matrix. Earlier failures exposed the Windows short-name mismatch and a test fixture whose numeric inode increment could round away; both were corrected, not skipped. A local focused attempt later hit host-load command timeout; the isolated Windows rerun and final hosted checks passed without changing test timeouts. A remote binding-setup omission was also corrected without source changes. Production delta is net +222 lines (+342/-120). Task-owned proof VMs are released. Package versions, dated 0.7.0 notes, and published artifacts are unchanged: this merges fixes under 0.7.1 Unreleased, but does not publish a release. |
1 participant
Summary
Fix the four failures found by fresh installed-consumer testing of published 0.7.0: Root-backed async lock handoffs, synchronous stale classification, synchronous failed-release retries, and native selected ZIP entry size verification. Also fix the Windows sync-lock canonicalization and open-denial gaps exposed by the new real-process proof, and clarify the TAR error-code wording found during validation.
Production source is 342 lines added / 120 deleted / net +222 across
src/andnative/src/. ZIP and sync fixes account for net +57; the remaining growth is the bounded async observation/ownership path. Most of the overall diff is deterministic regression coverage and a standalone cross-process proof. No new public API, dependency, version bump, or publication is included.Ownership and safety
Create-only Root preflight no longer opens an existing file merely to inherit a mode it cannot use. Boundary, alias, hardlink and type checks remain. Failed opened-file observations during async acquisition may be discarded only with operation-specific provenance, an inspectable exact-identity unlinked descriptor, and rechecked canonical ancestry. Discarding is not proof of pathname absence and provides no release/reclaim authority; a fresh exclusive creation is still required, and retry/deadline budgets apply. Generic Root reads, moved-but-linked/unknown descriptors, changed ancestry and caller errors remain strict.
Post-create admission compares the original serialized bytes/token before registering a holder. Cleanup retains that creator receipt rather than adopting a replacement's stat. Sync staleness uses the validated snapshot mtime, and failed sync release keeps its final reference and cleanup receipt. Descriptor ownership is consumed before close, since a failed close can already have freed the number; retry cannot close an unrelated reused descriptor.
Windows sync parent canonicalization now agrees with Root, including 8.3 short-name expansion. Open-denial retries reuse the existing eight-denial cap, are scoped to actual lock-file opens, consume caller budgets, and preserve the original error on exhaustion. Missing or changed snapshots also consume retry budgets; parser/read/stat errors are not disguised as missing paths. Held verification, release and reclaim remain strict. The previous test expecting an ordinary valid Windows Root-sync acquisition to fail was replaced with successful default/explicit path and reentrant-identity checks, retaining outside-root/junction/error controls.
The native ZIP reader verifies decoded length after its existing bounded, CRC-checking read. Both short and long declared-size mismatches reject; valid data, CRC rejection and byte caps remain unchanged.
Executed before/after behavior
Published-package runs captured Root async
path-mismatch/Windows resolver failures, false syncfile_lock_stale, a sync release retry leaving its sidecar afterEACCES, and native ZIP reads accepting 16 decoded bytes declared as 1 or 100. The first PR CI run exposed Windows Root-syncoutside-workspace; a separate Windows VM also reproduced sync acquisitionEPERM. These failures were corrected rather than skipped or hidden by longer timeouts.The final candidate was built and packed, then installed into a fresh Linux consumer with its freshly built binding. Installed binding bytes were verified against the build; neither workspace links nor the old registry binding substituted for it. The unchanged public reproductions passed in separate
off,auto, andrequireprocesses. Actual final output:{"proof":"fixed-installed-consumer","scenarios":18,"passed":true,"node":"v24.20.0","platform":"linux","arch":"x64"}This gate asserts all four pathname/Root × async/sync lock combinations with four processes and 100 acquisitions per case, actual permission-failure release retry, stored/deflated ZIPs declaring 1/16/100 bytes, valid payload contents, CRC rejection, and enforced byte caps. A second built-public-API proof keeps children alive until final sidecar absence is checked, so process-exit cleanup cannot hide a broken release. Together the two final Linux proofs completed 2,400 contended acquisitions across all three modes, plus release-retry controls.
The actual Windows short-path probe changed from:
{"proof":"windows-root-canonicalization","shortDiffersFromLong":true,"legacyMatchesRoot":false,"nativeMatchesRoot":true,"acquired":false,"error":"outside-workspace"}To:
{"proof":"windows-root-canonicalization","shortDiffersFromLong":true,"legacyMatchesRoot":false,"nativeMatchesRoot":true,"defaultAndExplicitPassed":true,"reentrantAliasPassed":true}Windows focused source tests passed 103 tests, with one POSIX permission test skipped. The final built-source and fresh installed-consumer proofs passed 2,400 contended acquisitions across
off,auto, andrequire, plus the actual short-path/default-path/explicit-path/reentrant checks above. This dedicated Windows lane uses the published binding only for the unchanged lock ABI; it is not proof of the Rust ZIP change. Hosted native CI builds the new Windows binding for that coverage. An initial source-proof setup omitted the binding from the workspace package; installing it in the normal resolution location and verifying its bytes corrected the harness without changing source.Validation
pnpm check: 6,274 passed, 81 skipped; 178 test files passed, one skipped.pnpm native:test: 63 passed on Linux x64.pnpm test:security: 78 passed.FS_SAFE_NATIVE_MODE=require: 150 passed.node scripts/sidecar-contention-proof.mjs off,auto, andrequire: final Linux runs passed.pnpm package:smoke: root-only npm 11.19.0 and pnpm 11.24.0 consumers passed native selection, missing-binary fallback and omitted-optionals cases. Foreign-platform filtering fixtures are explicitly synthetic; they are not foreign execution proof.git diff --check: passed.Production/package proof tree:
939dd9f6771449e6be7f60773a3b72580c612615. Final PR head:cce3a4059da04cdbc1c6e55f10fedec21e82da7c, tree6e8f6bd58f43eb3d6684b2e0051385dd3481bbdf. The final commit changes only the test fixture: Windows inode numbers can round awayino + 1, so the regression now supplies explicit distinct nonzero identities. All production/package inputs remain byte-identical, verified by Git diff; the corrected Windows suite again passed 103 tests and fixture-only Codex autoreview is scoped-clean. The local host was heavily contended, so the authoritative combined gate ran on isolated Linux instead of raising test timeouts.CI explicitly includes the ZIP integrity regression and Root admission tests in native selections, plus real contention proof on fallback Node 24 and native Linux/macOS/Windows/musl jobs. Hosted exact-head validation is required before merge; this body does not claim pending jobs have passed.
Packaged Windows authority-chain proof
The final installed Windows consumer also exercised the precise forbidden effect requested by review, in separate
off,auto, andrequireprocesses. A wrapper around the public Rootopen()method schedules a real rename and exclusive replacement-file creation after creator publication but before admission. No filesystem error is injected in this replacement case. The acquisition returnspath-mismatch, the opened descriptor is closed, the manager has zero held entries, and both replacement and displaced original remain byte-identical after acquisition cleanup and manager drain. An outside canary is preserved.Separate synchronous controls explicitly inject one
EPERMat the Node lock-file open call (controlled fault injection, not a claim of a naturally timed denial). They prove that success requires another real exclusive create. When a foreign replacement occupies the path, retries exhaust without a handle, a subsequent same-owner reentrant request still contends rather than inheriting hidden ownership, and replacement bytes survive. These runtime controls supplement—not replace—the naturally contended multi-process runs. The installed artifact integrity is checked before execution; the final head differs from its frozen production/package tree only in a nonpackaged test fixture.Actual complete redacted terminal output:
{"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"root-async-replacement-before-admission","scheduling":"public-Root-open-wrapper","filesystemMutation":"real-rename-and-exclusive-create","injectedFilesystemErrors":false,"rejected":"path-mismatch","heldRegistrations":0,"openedDescriptorClosed":true,"replacementPreservedAfterCleanup":true,"displacedOriginalPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"pathname-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"pathname-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"root-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"root-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"off","case":"summary","passed":true,"outsideCanaryPreserved":true,"packageIntegrity":"sha512-qhTAnHU8V9rmYphO3En5EwNgQJrh0jDysVRiaYkzN2uRgp4K2CtjhYKgWHCvOlXpU3NZVHhUMNTRvuOBf3ymVA==","nativeBindingScope":"published-0.7.0-lock-ABI-only"} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"root-async-replacement-before-admission","scheduling":"public-Root-open-wrapper","filesystemMutation":"real-rename-and-exclusive-create","injectedFilesystemErrors":false,"rejected":"path-mismatch","heldRegistrations":0,"openedDescriptorClosed":true,"replacementPreservedAfterCleanup":true,"displacedOriginalPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"pathname-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"pathname-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"root-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"root-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"auto","case":"summary","passed":true,"outsideCanaryPreserved":true,"packageIntegrity":"sha512-qhTAnHU8V9rmYphO3En5EwNgQJrh0jDysVRiaYkzN2uRgp4K2CtjhYKgWHCvOlXpU3NZVHhUMNTRvuOBf3ymVA==","nativeBindingScope":"published-0.7.0-lock-ABI-only"} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"root-async-replacement-before-admission","scheduling":"public-Root-open-wrapper","filesystemMutation":"real-rename-and-exclusive-create","injectedFilesystemErrors":false,"rejected":"path-mismatch","heldRegistrations":0,"openedDescriptorClosed":true,"replacementPreservedAfterCleanup":true,"displacedOriginalPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"pathname-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"pathname-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"root-sync-open-denial","injectedOpenDenials":1,"exclusiveCreateAttempts":2,"grantedOnlyAfterFreshExclusiveCreate":true,"verified":true,"released":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"root-sync-denial-replacement-preservation","injectedSnapshotOpenDenials":1,"exclusiveCreateAttempts":2,"rejected":"file_lock_timeout","noReentrantHeldAuthority":true,"replacementBytesPreserved":true} {"proof":"windows-packaged-lock-authority","head":"cce3a4059da04cdbc1c6e55f10fedec21e82da7c","productionTree":"939dd9f6771449e6be7f60773a3b72580c612615","mode":"require","case":"summary","passed":true,"outsideCanaryPreserved":true,"packageIntegrity":"sha512-qhTAnHU8V9rmYphO3En5EwNgQJrh0jDysVRiaYkzN2uRgp4K2CtjhYKgWHCvOlXpU3NZVHhUMNTRvuOBf3ymVA==","nativeBindingScope":"published-0.7.0-lock-ABI-only"}Release boundary
Only
0.7.1 - Unreleasednotes change. Package versions remain 0.7.0, the dated 0.7.0 changelog remains byte-identical, and existing tag/npm/GitHub Release artifacts are untouched. These fixes are not a new npm release.