Skip to content

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

Open
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
fix/redirect-ledger-atomicity
Open

fix(redirect): atomic fail-closed ledger, ledger-aware updates[]#192
Mikola Lysenko (mikolalysenko) wants to merge 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>
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