GeezerVison is showing github.com — fix(redirect): atomic fail-closed ledger, ledger-aware updates[] by mikolalysenko · Pull Request #192 · SocketDev/socket-patch
Conversation
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 — new save_redirect_state() persists .socket/vendor/redirect-state.json through the shared atomic_write_bytes helper (stage + fsync + rename), exactly like the sibling vendor ledger (vendor/state.rs). load_redirect_state() now returns Result<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 to redirect-state.json.corrupt (never clobbering an earlier .corrupt snapshot) so no later run can overwrite the revert data it may still hold.
crates/socket-patch-cli/src/commands/scan/hosted.rs — run_redirect loads 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-run reports 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_redirect propagates the corruption as a redirect_ledger_corrupt VEX 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-only scan update-detection consult warns on stderr.
D2 — ledger-aware update detection:
crates/socket-patch-cli/src/commands/scan/discovery.rs — new pure merge_redirect_records_for_updates() folds the redirect ledger's purl→uuid records into the manifest view detect_updates consults (an existing manifest entry wins a collision, matching VEX's augment_with_redirect). Hosted mode never writes .socket/manifest.json, so before this a pure hosted project's updates[] was structurally empty forever. The JSON envelope schema is unchanged; the hosted --json envelope reuses the same updates[].
README.md — documents that updates[] 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's unwrap_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/main and 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 old load_malformed_ledger_is_none tolerant-load pin); quarantine preserves the corrupt bytes verbatim and never clobbers an earlier .corrupt snapshot; save_redirect_state round-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 .corrupt quarantine, 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-only scan --json, no manifest, ledger records an old uuid → updates[] carries old→new); redirect_ledger_write_failure_leaves_project_files_untouched obstructs the ledger write with a read-only .socket/vendor and 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_run updated 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 unrelated docker_e2e_golang docker overlay2 teardown I/O error (the container's own log shows ===E2E PASS===) that passes on re-run.
Known limitations / deferrals
- The bun.lockb→bun.lock auto-migration still runs before the ledger write (it is an external
bun install re-lock whose FileEdit records no original bytes by design — git history is the documented restore path for the binary lock).
- After a quarantine, a subsequent hosted run starts a fresh ledger (the malformed file is no longer at the live path). This is deliberate: the hard error told the operator once, loudly, and the
.corrupt snapshot — which is never overwritten — remains the recovery artifact.
- Human (non-
--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.json and fixes read-only scan --json update detection for ledger-only projects.
Ledger durability (fail closed): load_redirect_state now returns Result — missing ledger is fine, but malformed/unreadable files raise CorruptRedirectState with optional quarantine to redirect-state.json.corrupt instead of being treated as “no ledger.” save_redirect_state writes atomically (stage + fsync + rename). run_redirect loads 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. vex hard-errors on corrupt ledgers; read-only scan warns.
Updates[] for hosted deps: merge_redirect_records_for_updates folds redirect-ledger patch records into the manifest view used by detect_updates, so pure hosted projects (no manifest.json) still get superseding patches in updates[]. README documents the behavior.
Reviewed by Cursor Bugbot for commit 4b64f46. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 <[email protected]>
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 <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
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 <[email protected]>
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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: Claude Fable 5 <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|