fix(redirect): atomic fail-closed ledger, ledger-aware updates[] - #192
Merged
Mikola Lysenko (mikolalysenko) merged 2 commits intoAug 14, 2026
Merged
Conversation
The hosted-mode redirect ledger (redirect-state.json) is the only store of the pre-redirect lockfile originals a revert needs, yet it was written with a plain fs::write and loaded tolerantly: a torn write made the next run silently start a fresh ledger over it, permanently destroying the revert data with exit 0 (audit D1). - Persist the ledger with the shared atomic writer (stage + fsync + rename), like the sibling vendor ledger. - Write the ledger BEFORE the project files, so a crash between the two leaves a complete ledger over untouched files instead of rewritten files whose originals never reached any ledger. - Fail closed on a malformed ledger everywhere: hosted runs abort before writing anything, quarantining the corrupt bytes aside to redirect-state.json.corrupt (never clobbered) with a recovery message; vex refuses to emit a false attestation; read-only scan warns. Absent ledger stays a fresh start. Hosted mode also never writes the manifest, so updates[] — the documented read-only CI signal — was structurally empty for pure hosted projects and a superseding patch was never reported (audit D2). Update detection now folds the ledger's purl->uuid records into the manifest view it consults; the JSON envelope schema is unchanged. Assisted-by: Claude Code:claude-fable-5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Windows ignores FILE_ATTRIBUTE_READONLY on directories for file creation, so the read-only .socket/vendor obstruction in leg 4 of the write-failure envelope test never obstructed there: the atomic writer's stage file was created fine, the run exited 0, and the windows-latest CI leg failed on both the error-envelope and the lockfile-untouched assertions. Split leg 4 into its own #[cfg(unix)] test (the file's established gating for permission-obstruction tests), hoisting the shared envelope assertion and scan driver to module scope. Leg 3's read-only FILE does obstruct on Windows and stays cross-platform, as does unwritable_ledger_fails_the_run's directory-squatting obstruction. The ledger-before-files ordering pin is unchanged, just unix-only now. Also drop the never-used REDIRECT_STATE_CORRUPT_REL export flagged in review; quarantine() derives the .corrupt path from the ledger path itself. Assisted-by: Claude Code:claude-fable-5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wenxin Jiang (Wenxin-Jiang)
approved these changes
Aug 14, 2026
Mikola Lysenko (mikolalysenko)
merged commit Aug 14, 2026
77362d0
into
main
118 of 119 checks passed
Mikola Lysenko (mikolalysenko)
deleted the
fix/redirect-ledger-atomicity
branch
August 14, 2026 20:23
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Aug 14, 2026
Reconciles with #192 (fix/redirect-ledger-atomicity), which already landed the ledger-before-lockfiles ordering, the atomic ledger write (save_redirect_state), and the fail-closed corrupt-ledger load with .corrupt quarantine. Kept all of main's #192 machinery and re-expressed this PR's remaining value on top of it: - Lockfile writes go through atomic_write_bytes_preserving_mode (main still used bare fs::write for the rewrite.files loop). - FileEdit derives PartialEq and the ledger merge skips byte-identical re-planned edits from a retried partial failure. - parse_purl_simple percent-decodes the version like the name. - New test partial_lockfile_write_failure_persists_ledger_originals (mid-run multi-lock failure: originals durable, failed lock byte-untouched). - Leg 3 of the write-failure envelope test obstructs the lock's DIRECTORY (unix-gated): with atomic writes a read-only lock FILE is replaced mode-preserved instead of failing. Dropped as subsumed by #192: load_redirect_state_strict (superseded by CorruptRedirectState), this branch's own atomic ledger write and ledger-first ordering, and corrupt_ledger_is_refused_not_replaced (superseded by corrupt_ledger_fails_closed_and_preserves_the_bytes, whose quarantine semantics intentionally MOVE the corrupt file). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 14, 2026
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Aug 19, 2026
…ly purls (#206) * test(vendor): RED — npm-family mode-migration e2e twins of the cargo C1-C7 capstones mode_migration_npm.rs drives the real binary + corepack yarn (classic and berry) through hosted -> vendored -> revert, and vendored -> hosted, asserting the cross-mode takeover contract #196 shipped for cargo: - hosted -> vendored must pre-revert the hosted lock edits (so the vendor ledger records the PRISTINE registry originals), drop the purl's redirect-ledger record+edits, and surface vendor_takeover_reverted_redirect - vendor --revert afterwards must land back on REGISTRY state byte-identical - vendored -> hosted must revert the vendored wiring per purl first (redirect_takeover_reverted_vendored), not leave overlapping ledgers All three tests FAIL on main (takeover machinery is hard-gated to pkg:cargo/ at cli vendor.rs:993 and scan/hosted.rs:336): test berry_hosted_then_vendored_takeover_round_trips_to_registry ... FAILED test classic_hosted_then_vendored_takeover_round_trips_to_registry ... FAILED test classic_vendored_then_hosted_takeover_leaves_pure_hosted ... FAILED (takeover advisory missing from the vendor envelope / takeover warning missing: redirect_supersedes_vendored fired instead) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(vendor): pre-revert live hosted redirects when vendoring npm-family purls The cross-mode takeover that #196 built for cargo (C1-C7) was hard-gated to pkg:cargo/ in BOTH directions: - cli vendor.rs:993 — the vendor dispatch loop's pre-revert ("Cross-mode takeover (cargo)"), so vendoring an npm purl over a LIVE hosted redirect (a) recorded the grant-tokenized HOSTED lock fragment as the vendor ledger's unrecoverable pre-vendor original, (b) never dropped the superseded redirect records/edits — the vendor_supersedes_redirect warning's promised auto-reconcile provably never converged — and (c) made vendor --revert land back on the expiring hosted URL with no CLI path to registry state (adversarially confirmed P1, cells convy1-hosted2vendored + E5, yarn 1 and yarn 4). - scan/hosted.rs:336/346 — the reverse direction, so a hosted scan over live vendored npm wiring either hijacked the vendored resolution while the vendored ledger still claimed it (classic) or refused outright (berry file: protocol). Fix, porting the cargo pattern to the npm family: - core redirect/takeover.rs: revert_npm_redirect_purl replays each recorded FileEdit.original over its .new — text-fragment kinds (yarn classic / yarn berry / pnpm, keyed name@version) via staged replacen, package-lock JSON kinds (redirect_npm_lock_entry / redirect_npm_lock_dep, alias entries resolved through the lock's name field exactly as the rewriter matched them) via a staged JSON replay — with the same fail-closed contract as cargo: every inverse resolves against a staged view, drift refuses byte-identically, and only a fully clean replay drops the record+edits (caller persists). A bun.lock edit for the purl is a hard refusal (no bun replay yet). New: redirect_revert_supported + revert_redirect_purl dispatcher; CargoRedirectRevert kept as an alias of the renamed RedirectRevert. - cli vendor.rs: gate widened from pkg:cargo/ to redirect_revert_supported (cargo + npm); the corrupt-ledger refusal and dry-run vendor_would_revert_redirect warning now cover npm too; ecosystem-appropriate takeover advisory text. - cli scan/hosted.rs: reverse takeover gate widened to cargo + npm (dispatch_revert_one already handles npm); the no-ledger wired-check stays cargo-only; refused-override filtering now keys by (ecosystem, coordinate, version). Ledger transactionality per #192/#187 precedent is preserved: the redirect ledger is persisted before vendoring proceeds, and a persist failure fails that purl closed. Unit tests: real-rewriter round-trip fixtures for classic, berry, and a lockfileVersion-2 package-lock (both trees), drift refusal, bun refusal, gate coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(takeover): version-scope the npm package-lock edit claims The npm JSON lock-edit claim matcher was name-only: reverting pkg:npm/name@X claimed and replayed the ledger edits belonging to pkg:npm/name@Y (a sibling hosted-redirected version) and alias-keyed edits belonging to a different package whose lock path merely equals `name` — silently un-hosting the other purl (v3 `packages`) or spuriously drift-refusing the whole takeover (v2 `dependencies`), while the rewriter had matched on entry name AND version. Same bug class as the pnpm multi-version clobber: bind claims to name@version, not name-only. `redirect_npm_lock_entry` now attributes a live entry exactly the way the rewriter matched it — effective name (the `name` field npm writes for alias installs, else the key's trailing path) AND version — and a vanished entry keeps the fail-closed "no longer exists" refusal via key path + the recorded resolved URLs (which embed the version behind `/<version>/` or `-<version>.tgz` delimiters, so sibling versions never cross-match). `redirect_npm_lock_dep` (bare-name key) gains the same URL-based version discriminator. New adversarial fixtures (both RED against the old matcher): a two-version package-lock (v2, both trees) where taking over 1.3.0 must leave 1.2.0's redirect and ledger intact, and an alias collision where `npm i left-pad@npm:other` must be exonerated by the entry's `name` field while `npm i mylp@npm:left-pad` is still claimed through it; plus a guard that a pruned lock entry still fails closed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
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.
LLM Description written by Claude Code:claude-fable-5
What
Fixes audit findings D1 (redirect ledger written non-atomically; a torn write silently destroys all revert data on the next run) and D2 (
updates[]never reports newer patches for hosted-redirect-managed deps).D1 — ledger durability (fail closed):
crates/socket-patch-core/src/patch/redirect/state.rs— newsave_redirect_state()persists.socket/vendor/redirect-state.jsonthrough the sharedatomic_write_byteshelper (stage + fsync + rename), exactly like the sibling vendor ledger (vendor/state.rs).load_redirect_state()now returnsResult<Option<RedirectState>, CorruptRedirectState>: absent →Ok(None)(fresh start, fine); present-but-malformed/unreadable → a hard error that names the file, explains what is at stake, and lists the recovery options.CorruptRedirectState::quarantine()moves the malformed file aside toredirect-state.json.corrupt(never clobbering an earlier.corruptsnapshot) so no later run can overwrite the revert data it may still hold.crates/socket-patch-cli/src/commands/scan/hosted.rs—run_redirectloads the ledger before any file is written (bun.lockb migration included) and aborts on corruption with the error above (JSON envelope + stderr, exit 1), quarantining the corrupt bytes;--dry-runreports the same error but moves nothing. The merged ledger is now persisted atomically and before the project files, so a crash between the two leaves a complete ledger whose recorded originals simply match files that were never rewritten — instead of rewritten files whose pre-redirect originals never reached any ledger (a healing re-run records no edits for already-redirected entries).crates/socket-patch-cli/src/commands/vex.rs—augment_with_redirectpropagates the corruption as aredirect_ledger_corruptVEX error instead of silently attesting without the ledger's records (a false document).crates/socket-patch-cli/src/commands/scan/mod.rs— the takeover-overlap classifier treats a malformed ledger like a missing one (it only feeds warnings, and every write/attest path already hard-errors); the read-onlyscanupdate-detection consult warns on stderr.D2 — ledger-aware update detection:
crates/socket-patch-cli/src/commands/scan/discovery.rs— new puremerge_redirect_records_for_updates()folds the redirect ledger's purl→uuid records into the manifest viewdetect_updatesconsults (an existing manifest entry wins a collision, matching VEX'saugment_with_redirect). Hosted mode never writes.socket/manifest.json, so before this a pure hosted project'supdates[]was structurally empty forever. The JSON envelope schema is unchanged; the hosted--jsonenvelope reuses the sameupdates[].README.md— documents thatupdates[]covers hosted-managed deps.Why
The redirect ledger is the ONLY store of the pre-redirect lockfile originals (e.g. the cargo lock's crates.io source/checksum entries) a future revert needs, and the VEX record store. It was written with a plain
fs::write(torn on crash/ENOSPC) and loaded tolerantly (.ok()?), so the next hosted run'sunwrap_or_else(RedirectState::new)started a FRESH ledger and overwrote the corrupt file — permanently destroying every previously recorded edit, with exit 0 and no warning. Exiting 0 while destroying the only revert path is exactly the fail-open bug class this PR series removes: a refusal with a clear, actionable error is always acceptable; a silent broken success never is.Separately, bots keying off
updates[](the documented read-only CI signal) never learned that a hosted-redirected patch had been superseded, so projects kept installing the older patched artifact indefinitely.Testing
Red-first: the three new integration tests were run against unmodified
origin/mainand reproduced both findings (exit 0 with the corrupt ledger silently replaced by a fresh one;updates: []for a ledger-only project) before the fix turned them green.crates/socket-patch-core/src/patch/redirect/state.rs(unit): malformed load is a hard error naming the file (replaces the oldload_malformed_ledger_is_nonetolerant-load pin); quarantine preserves the corrupt bytes verbatim and never clobbers an earlier.corruptsnapshot;save_redirect_stateround-trips, keeps the trailing newline, creates the directory, and leaves no.socket-stage-*litter.crates/socket-patch-cli/tests/in_process_redirect.rs:corrupt_ledger_fails_closed_and_preserves_the_bytes(hosted run over a torn ledger → exit 1, parseable error envelope naming both the ledger and the.corruptquarantine, lockfile untouched, no fresh ledger written);corrupt_ledger_dry_run_errors_without_moving_the_file;scan_updates_reports_superseding_patch_for_ledger_only_project(plain read-onlyscan --json, no manifest, ledger records an old uuid →updates[]carries old→new);redirect_ledger_write_failure_leaves_project_files_untouchedobstructs the ledger write with a read-only.socket/vendorand additionally asserts the lockfile stays untouched (ledger-before-files ordering) —#[cfg(unix)]-gated per review, since Windows ignores the read-only attribute on directories for file creation;unwritable_ledger_fails_the_runupdated to pin the new fail-before-any-write ordering (its directory-squatting obstruction blocks reads on Windows too, so it stays cross-platform).crates/socket-patch-cli/src/commands/scan/discovery.rs(unit): ledger-only project reports the superseding patch; same-uuid ledger record is not an update; manifest entry wins a collision; disjoint manifest+ledger purls both detected; absent/empty ledger leaves the view untouched.Gates:
cargo test -p socket-patch-cli --all-features --test in_process_redirect— 27 passed.cargo test -p socket-patch-core --all-features redirect— all passed.cargo clippy --workspace --all-features -- -D warnings— clean.cargo test --workspace --all-features— green across every completed test binary; the one failure seen was an unrelateddocker_e2e_golangdocker overlay2 teardown I/O error (the container's own log shows===E2E PASS===) that passes on re-run.Known limitations / deferrals
bun installre-lock whoseFileEditrecords no original bytes by design — git history is the documented restore path for the binary lock)..corruptsnapshot — which is never overwritten — remains the recovery artifact.--json) hosted output still doesn't print update info;updates[]in the JSON envelope is the documented CI contract and is what D2 fixes.Note
Medium Risk
Changes fail-closed behavior and write ordering for the redirect ledger (only revert path for pre-redirect lockfile data) and VEX attestation; incorrect handling could block hosted runs or mis-report updates, but scope is redirect/hosted paths with strong test coverage.
Overview
Hardens hosted redirect handling around
.socket/vendor/redirect-state.jsonand fixes read-onlyscan --jsonupdate detection for ledger-only projects.Ledger durability (fail closed):
load_redirect_statenow returnsResult— missing ledger is fine, but malformed/unreadable files raiseCorruptRedirectStatewith optional quarantine toredirect-state.json.corruptinstead of being treated as “no ledger.”save_redirect_statewrites atomically (stage + fsync + rename).run_redirectloads the ledger before any project writes, aborts on corruption (dry-run quarantines nothing), persists the merged ledger before lockfile rewrites, and fails if the ledger cannot be saved.vexhard-errors on corrupt ledgers; read-onlyscanwarns.Updates[] for hosted deps:
merge_redirect_records_for_updatesfolds redirect-ledger patch records into the manifest view used bydetect_updates, so pure hosted projects (nomanifest.json) still get superseding patches inupdates[]. README documents the behavior.Reviewed by Cursor Bugbot for commit 4b64f46. Configure here.