Skip to content

fix(cargo): fully migrate hosted<->vendored modes (C1-C7) - #196

Open
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
fix/cargo-mode-takeover-cleanup
Open

fix(cargo): fully migrate hosted<->vendored modes (C1-C7)#196
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
fix/cargo-mode-takeover-cleanup

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

What

Makes a cargo mode migration (vendored ↔ hosted) a real takeover: the flow reverts the displaced mode's wiring per purl BEFORE applying the new mode, so the project ends FULLY in the new mode — or the purl is refused with an actionable error. Fixes audit findings C1–C7.

  • Hosted scan over a vendored crate (C1): run_redirect now runs the exact per-purl vendor --revert machinery first (restore the Cargo.lock originals from the vendored ledger, drop the [patch.crates-io] entry, remove the committed tree and the ledger entry), persisting the ledger per purl, and only then redirects. A purl whose vendored state cannot be cleanly reverted (revert failure, ledger unreadable/unwritable) is refused — skipped with redirect_vendored_revert_failed — never half-migrated. Dry runs preview the takeover with redirect_would_revert_vendored.
  • Vendored flows over a hosted redirect (C2, C7): both the standalone vendor command and scan --mode vendored now revert the purl's hosted edits from the redirect ledger's recorded FileEdits first — Cargo.toml loses its registry = "socket-patch-…" pin, Cargo.lock gets the original crates.io source/checksum back, the now-unused [registries.socket-patch-…] block is dropped — then drop the ledger record and vendor. New core module patch/redirect/takeover.rs (revert_cargo_redirect_purl) unwinds re-redirect chains newest-first and fails closed on drifted files (neither the recorded redirected fragment nor the original matches ⇒ Err ⇒ the purl is refused with redirect_revert_failed). The plain vendor command also emits the same vendor_supersedes_redirect envelope advisory the scan flow does (C7).
  • Missing/corrupt ledger backstop: vendor_cargo_crate itself refuses (hosted_redirect_live) when live hosted wiring is present with no redirect-ledger record to revert it — with manual remediation steps — instead of baking the hosted registry values into the vendor ledger as "originals".
  • Lock-originals preservation (C3): the pre-revert ordering means the vendor detach records the PRISTINE crates.io fragment; additionally persist_vendor_entry carries the cargo lock originals forward on re-vendor (entry.lock was previously dropped whenever the re-detach found nothing to detach), so an A→B→A round trip can no longer overwrite the ledger's only copy of the unrecoverable crates.io source/checksum, and vendor --revert restores the true pre-vendor lock.
  • Classifier cargo blindness (C4/C4b): classify_overlap_takeover gains a cargo-specific probe (classify_cargo_overlap) that reads the Cargo.lock entry's actual shape — source matching a config-declared [registries.socket-patch-*] sparse index (or patch.socket.dev) proves hosted; a detached entry with [patch.crates-io] pointing into the entry's committed .socket/vendor/cargo/<uuid>/ copy proves vendored; anything else stays silent. The previous generic probe could never prove hosted for cargo (lock_inventory records resolved: None), so it inverted the direction and told users to delete the live redirect ledger.
  • GC/prune reclamation (C5): dispatch_in_use_one gains the cargo probe (vendored_entry_in_use): a detached lock entry routed through the entry's [patch] copy is in use; a registry-sourced or missing entry is reclaimable, so a vendored entry displaced by a hosted takeover no longer survives every scan --prune forever.
  • Remediation text (C6): both takeover warnings now recommend only the safe sequence (vendor --revert / re-run the vendored flow) and explicitly warn against hand-deleting .socket/vendor/ (the [patch.crates-io] entry may still reference it — deleting it hard-fails every cargo command) or redirect-state.json (it may hold the only revert data and VEX records for other still-redirected packages).

Why

Before this change, every cargo mode migration exited 0 while leaving the project broken — the exact fail-open bug class the audit flagged:

  • vendored→hosted left both wirings in place; cargo refuses to update the lock over the now-unused [patch] entry, so every CI cargo build --locked fails (C1) — with warnings: [] because the classifier could not see hosted-liveness for cargo (C4);
  • hosted→vendored left the Cargo.toml registry pin that [patch.crates-io] cannot patch over — unbuildable online and offline (C2), and completely silent via the primary vendor entry point (C7);
  • A→B→A silently destroyed the crates.io lock originals, the one piece of state documented as "not recoverable offline; the ledger entry is their only home", so revert restored a dead grant-tokenized sparse-index URL (C3);
  • the inverted warning actively advised deleting the live hosted ledger — destroying the VEX record store and the only hosted revert path (C4b/C6);
  • and nothing (GC, prune, repair) could ever reclaim the stale vendored residue (C5).

Testing

New real-binary + real-cargo migration suite crates/socket-patch-cli/tests/mode_migration_cargo.rs (adapted from the audit probes in mode_migration_cargo_audit.rs), red on the pre-fix code and green here. Each scenario proves the terminal state with a fresh-checkout cargo build --locked:

  • vendored_then_hosted_takeover_leaves_pure_hosted — vendored state fully gone (no [patch], no tree, no ledger entry), redirect live, fresh checkout builds --locked (C1/C4);
  • hosted_then_vendored_takeover_leaves_pure_vendored — registry pin and [registries] block gone, redirect record dropped, fresh checkout builds --locked --offline (C2/C7);
  • double_takeover_a_b_a_preserves_lock_originals — ledger keeps the crates.io fragment through A→B→A, and vendor --revert restores the pristine lock byte-identically (C3);
  • vendor_over_hosted_without_ledger_is_refused — live hosted wiring with a deleted ledger: vendor refuses (hosted_redirect_live), project untouched (fail-closed).

Plus a GC unit test (vendor_gc_reclaims_cargo_entry_displaced_by_hosted_takeover, C5) and classifier/remediation coverage in the scan tests.

Gates run locally, all green:

  • cargo test -p socket-patch-cli --all-features --test mode_migration_cargo (4/4, no skips)
  • cargo test -p socket-patch-cli --all-features --test in_process_redirect --test rollback_invariants --test repair_invariants --test e2e_vendor_cargo_build
  • cargo clippy --workspace --all-features -- -D warnings
  • cargo test --workspace --all-features

Note

High Risk
Changes critical cargo wiring, lockfile originals, and ledger persistence across hosted/vendored migrations; incorrect revert or classifier logic could leave projects unbuildable or destroy unrecoverable lock state.

Overview
Cargo hosted ↔ vendored mode switches are now takeovers: the displaced wiring is reverted per package before applying the new mode, so the repo ends fully in one mode—or that package is refused with an actionable error instead of reporting success on a broken --locked build.

Vendored → hosted: scan --mode hosted runs the same per-purl revert as vendor --revert (lock originals, [patch.crates-io], committed tree, ledger) before redirecting; corrupt/missing vendored ledger with live wiring is skipped.

Hosted → vendored: vendor and vendored scan call new revert_cargo_redirect_purl to unwind hosted edits from the redirect ledger, then vendor; drift or missing ledger refuses the purl (hosted_redirect_live in the cargo backend).

Classifier & GC: Cargo overlap direction uses Cargo.lock shape (sparse index vs detached + patch path), not generic patch.socket.dev resolution; cargo vendored entries displaced by hosted wiring are reclaimable on prune/GC.

Originals chain: Re-vendor carries forward cargo lock originals in the ledger; remediation text warns against hand-deleting vendor trees or redirect-state.json.

New e2e mode_migration_cargo.rs proves fresh-checkout cargo build --locked after each scenario.

Reviewed by Cursor Bugbot for commit 8e784ca. Configure here.

A cargo mode takeover used to exit 0 while leaving the project
unbuildable in both modes (audit findings C1-C7):

- hosted scan over a vendored crate left the vendored [patch.crates-io]
  entry behind, so every `cargo build --locked` failed (C1);
- vendor over a hosted redirect left the Cargo.toml
  registry = "socket-patch-..." pin, which [patch.crates-io] cannot
  apply over - unbuildable online and offline (C2), silently via the
  plain `vendor` command (C7);
- a vendored->hosted->vendored round trip overwrote the unrecoverable
  crates.io lock originals in the vendored ledger with the hosted
  sparse-index URL + patched checksum, so revert restored a dead
  grant-tokenized URL (C3);
- the takeover classifier could never prove hosted for cargo
  (lock-inventory resolved is None), so it INVERTED the direction and
  told users to delete the LIVE redirect ledger (C4/C4b);
- the displaced vendored entry was unreclaimable by GC/prune forever
  (no cargo in-use probe, C5);
- the remediation text offered deleting the .socket/vendor/<eco>/ tree
  while [patch.crates-io] still referenced it - hard resolution
  failure on every cargo command (C6).

A takeover now leaves the project FULLY in the new mode, or refuses:

- hosted scan reverts each claimed purl's vendored state per purl
  (lock originals restored, [patch] entry dropped, tree + ledger entry
  removed) before redirecting, and refuses the purl when the vendored
  ledger is missing/corrupt;
- every vendored flow (vendor, scan --mode vendored) reverts the
  purl's hosted edits from the redirect ledger FileEdits first
  (Cargo.toml pin, Cargo.lock source/checksum, registries block) and
  drops the ledger record; drifted files refuse fail-closed, and the
  cargo backend refuses (hosted_redirect_live) when hosted wiring is
  live with no ledger to revert it;
- the classifier gains a cargo lock-shape probe (socket-patch registry
  index vs detached-with-[patch]), so the direction follows the lock;
- persist_vendor_entry carries the cargo lock originals forward on
  re-vendor, keeping the crates.io fragment as the ledger's only home;
- dispatch_in_use_one gains the cargo probe so GC/prune reclaims
  genuinely stale vendored entries;
- both takeover remediation texts now recommend only the safe
  sequence (vendor --revert / re-run the vendored flow) and warn
  against hand-deleting ledgers or the vendor tree.

Red-then-green: the new real-cargo migration suite
(tests/mode_migration_cargo.rs, adapted from the audit probes) fails
all four scenarios on the previous code and passes with this change,
proving fresh-checkout `cargo build --locked` in the terminal state of
every migration direction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8e784ca. Configure here.

or restore the crates.io wiring manually, then re-run",
edit.path
));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial revert before fail-closed

Medium Severity

revert_cargo_redirect_purl writes each inverse immediately while unwinding newest-first, then returns Err on a later drifted edit without rolling those writes back. The ledger stays unchanged, so disk can be half-reverted (for example Cargo.lock restored while Cargo.toml still pinned) while the caller refuses the purl as if nothing was touched. That breaks the fail-closed takeover contract and can leave an inconsistent hosted state that retries keep failing on.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8e784ca. Configure here.

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