Skip to content

fix(redirect): atomic fail-closed ledger, ledger-aware updates[] - #192

Merged
Mikola Lysenko (mikolalysenko) merged 2 commits into
mainfrom
fix/redirect-ledger-atomicity
Aug 14, 2026
Merged

fix(redirect): atomic fail-closed ledger, ledger-aware updates[]#192
Mikola Lysenko (mikolalysenko) merged 2 commits into
mainfrom
fix/redirect-ledger-atomicity

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

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.rsrun_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.rsaugment_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.

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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 77362d0 into main Aug 14, 2026
118 of 119 checks passed
@mikolalysenko
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>
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>
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.

2 participants