fix(vendor): pre-revert live hosted redirects when vendoring npm-family purls

[Mikola Lysenko (mikolalysenko)]

Root cause

The cross-mode takeover #196 built for cargo (C1–C7) was hard-gated to pkg:cargo/ in BOTH directions:

• crates/socket-patch-cli/src/commands/vendor.rs:993 — the vendor dispatch loop's pre-revert ("Cross-mode takeover (cargo)" block, L982–1099). Vendoring an npm purl over a LIVE hosted redirect therefore skipped the hosted-edit pre-revert and the redirect-ledger drop entirely.
• crates/socket-patch-cli/src/commands/scan/hosted.rs:336/346 — the reverse direction (hosted scan over live vendored wiring).
• crates/socket-patch-core/src/patch/redirect/mod.rs:34 / takeover.rs:110 — core exported only revert_cargo_redirect_purl; no npm-family FileEdit replay existed anywhere.

Matrix evidence (adversarially CONFIRMED P1; cells convy1-hosted2vendored + E5, yarn 1.22 and yarn 4.6)

On npm/yarn, vendor / scan --mode vendored over a live hosted redirect:

• (a) recorded the grant-tokenized HOSTED patch.socket.dev lock fragment as the vendor ledger's unrecoverable pre-vendor "original" (E5: state.json wiring[].original = the __archiveUrl= fragment),
• (b) left redirect-state.json records+edits in place forever — the vendor_supersedes_redirect warning's promised auto-reconcile was byte-for-byte non-convergent (re-run left the ledger identical and re-fired the warning), leaving the stale pre-redirect edits as the documented revert-replay hazard,
• (c) made vendor --revert land back on the expiring hosted URL with no CLI path back to registry state (grep -c patch.socket.dev yarn.lock = 1, registry.yarnpkg.com = 0 after the round trip).

Fix (ports the cargo C1–C7 pattern to the npm family)

• core patch/redirect/takeover.rs: new revert_npm_redirect_purl replays each recorded FileEdit.original over its new — text-fragment kinds (redirect_yarn_classic_entry, redirect_yarn_berry_entry, redirect_pnpm_resolution, 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. Same fail-closed contract as cargo: every inverse resolves against a staged view, drift refuses with the project byte-identical, and only a fully clean replay drops the record+edits (the caller persists — ledger transactionality per the fix(redirect): atomic fail-closed ledger, ledger-aware updates[] #192/fix(hosted): persist redirect ledger before lockfile writes #187 precedent, and a persist failure fails that purl closed). 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: takeover gate widened from pkg:cargo/ to redirect_revert_supported (cargo + npm); the redirect_ledger_corrupt refusal and the dry-run vendor_would_revert_redirect warning now cover npm too; ecosystem-appropriate vendor_takeover_reverted_redirect advisory text.
• cli scan/hosted.rs: reverse takeover gate widened to cargo + npm (dispatch_revert_one already handles npm — the exact per-purl vendor --revert machinery); the no-ledger .cargo/config.toml wired-check stays cargo-only; refused-override filtering now keys by (ecosystem, coordinate, version).

Net effect: the vendor_supersedes_redirect warning's advertised auto-reconcile is now TRUE and convergent for npm, the vendor ledger records the pristine registry originals, vendor --revert round-trips to registry state, and E5's hosted→vendored path reaches a clean vendored state on berry.

Tests (TDD, RED first — commit 377799d, fix b04ad42)

• crates/socket-patch-cli/tests/mode_migration_npm.rs (new, twin of mode_migration_cargo.rs): drives the real binary + corepack yarn (classic 1.22.22 AND berry 4.12.0) with a wiremock hosted server through hosted → vendored → vendor --revert, asserting: takeover advisory, redirect-state.json dropped, vendor-ledger originals are the registry fragments, fresh-checkout install carries the patched bytes (classic --frozen-lockfile --offline, berry --immutable --check-cache), and the revert restores the registry yarn.lock byte-identically; plus a classic vendored → hosted reverse-takeover leg. All three FAILED on main (RED: takeover advisory missing / redirect_supersedes_vendored fired instead), all GREEN with the fix.
• core takeover.rs units: real-rewriter round-trip fixtures for classic, berry, and a lockfileVersion-2 package-lock (both packages + legacy dependencies trees restored byte-identically), drift fail-closed refusal, bun-edit refusal, missing-record error, gate coverage.
• Manual e2e on a throwaway copy of the confirmed repro (yarn1 minimist@1.2.2, live hosted redirect vs production): old binary (yarn-matrix-e2e worktree) → stale redirect-state.json, hosted original in state.json, revert lands on patch.socket.dev; new binary → ledger dropped, registry original recorded, vendor_takeover_reverted_redirect emitted, revert lands on registry.yarnpkg.com.
• Suites run green: cargo test -p socket-patch-core --lib (2271), -p socket-patch-cli --lib (388), mode_migration_cargo, mode_migration_npm, in_process_redirect, in_process_redirect_pnpm, in_process_vendor, in_process_scan, scan_invariants, scan_vendor_e2e, scan_vendor_step_error_e2e, e2e_redirect_{npm,yarn_classic,yarn_berry,bun}_build, e2e_vendor_{npm,pnpm,bun,yarn_classic,yarn_berry}_build, e2e_vendor_yarn_classic_dev_flow, repair_vendor_e2e, e2e_vex_redirect; cargo clippy --workspace --all-features -- -D warnings clean.

Residuals (noted, out of scope)

• bun.lock hosted redirects have no replay yet — a bun edit claiming the purl now refuses fail-closed instead of silently half-migrating.
• The npm vendor backends have no hosted_redirect_live backstop (cargo-only, vendor/cargo.rs:614): vendoring over hosted wiring whose ledger was DELETED still proceeds recording hosted originals. Follow-up candidate.
• Warning-text/visibility polish for mode_takeover_detail belongs to lane ADE.

Sibling-PR conflict notes

Parallel lanes from the same campaign:

• Lane C fix/vendor-revert-driftskip-keep-artifacts — touches core vendor/yarn_classic_lock.rs + npm-family twins, vendor/mod.rs RevertOutcome, and cli vendor.rs revert path → likely textual conflict in cli/src/commands/vendor.rs (this PR edits the dispatch-loop takeover block and comments only) and possible semantic overlap on the revert path my new e2e exercises.
• Lane ADE fix/mode-conversion-visibility-warnings — touches cli scan/mod.rs + core redirect/mod.rs berry warning branch → this PR adds 4 lines to core redirect/mod.rs (the takeover re-export block at L31–37) — trivial to merge; no scan/mod.rs changes here.
• PR fix(scan): surface yarn PnP refusal instead of silent success-0 across all modes #201 (scan/mod.rs empty-discovery path) and PR test(yarn): version-matrix e2e capstones — yarn2/3 refusal pins, berry workspaces + pnpm-linker install proofs #200 (new e2e_yarn* test files): no file overlap with this PR.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes lockfile/ledger migration paths for npm hosted⇄vendored mode; behavior is heavily tested but touches critical state transitions and redirect ledger persistence.

Overview
Extends the hosted ⇄ vendored mode takeover that previously only applied to cargo so npm-family packages get the same treatment in both directions.

Core: Adds revert_npm_redirect_purl to replay redirect-ledger FileEdits for yarn classic/berry, pnpm, and package-lock JSON/text lockfiles (fail-closed on drift, staged writes). Introduces redirect_revert_supported and revert_redirect_purl as the shared dispatcher; CargoRedirectRevert becomes an alias of RedirectRevert.

CLI: The vendor dispatch loop and hosted scan takeover gate use the dispatcher for cargo + npm instead of hardcoding pkg:cargo/. Hosted scan refused-override filtering keys by (ecosystem, coordinate, version).

Tests: New mode_migration_npm.rs e2e (yarn classic + berry) plus unit tests for npm revert round-trips, drift refusal, and bun.lock hard refusal.

^{Reviewed by Cursor Bugbot for commit b04ad42. Configure here.}

[cursor]

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

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Lock edit claim ignores version
  • Added version checking to redirect_npm_lock_dep and redirect_npm_lock_entry claiming logic to match only edits for the specific package version being reverted.

Or push these changes by commenting:

@cursor push 46b3bf7a3a

Preview (46b3bf7a3a)

diff --git a/crates/socket-patch-core/src/patch/redirect/takeover.rs b/crates/socket-patch-core/src/patch/redirect/takeover.rs
--- a/crates/socket-patch-core/src/patch/redirect/takeover.rs
+++ b/crates/socket-patch-core/src/patch/redirect/takeover.rs
@@ -343,6 +343,28 @@
     "redirect_pnpm_resolution",
 ];
 
+/// Check if the legacy npm v2 `dependencies` tree contains any entry with the
+/// given name and version (recursively).
+fn deps_contains_name_version(deps: &Value, name: &str, version: &str) -> bool {
+    let Some(deps_obj) = deps.as_object() else {
+        return false;
+    };
+    for (dep_name, entry) in deps_obj {
+        if dep_name == name
+            && entry.get("version").and_then(Value::as_str) == Some(version)
+            && entry.get("bundled").and_then(Value::as_bool) != Some(true)
+        {
+            return true;
+        }
+        if let Some(nested) = entry.get("dependencies") {
+            if deps_contains_name_version(nested, name, version) {
+                return true;
+            }
+        }
+    }
+    false
+}
+
 /// Revert every hosted-redirect edit the ledger records for `purl` (an npm
 /// package), then drop that purl's record and edits from `state`. The caller
 /// persists the mutated ledger (see `persist_redirect_state`).
@@ -369,14 +391,17 @@
     let (name, version) = (name.to_string(), version.to_string());
     let lock_key = format!("{name}@{version}");
 
-    // The package-lock/shrinkwrap files any `redirect_npm_lock_entry` edits
-    // touch, parsed once from disk: an ALIAS install (`npm i alias@npm:name`)
-    // keys its entry by the alias, so ownership is resolved through the
-    // entry's `name` field — exactly how the rewriter matched it (the rewrite
-    // never touches name/version, so the probe is symmetric).
+    // The package-lock/shrinkwrap files any `redirect_npm_lock_entry` or
+    // `redirect_npm_lock_dep` edits touch, parsed once from disk: an ALIAS
+    // install (`npm i alias@npm:name`) keys its entry by the alias, so
+    // ownership is resolved through the entry's `name` field — exactly how
+    // the rewriter matched it (the rewrite never touches name/version, so the
+    // probe is symmetric).
     let mut disk_locks: BTreeMap<String, Option<Value>> = BTreeMap::new();
     for e in &state.edits {
-        if e.kind == "redirect_npm_lock_entry" && !disk_locks.contains_key(&e.path) {
+        if (e.kind == "redirect_npm_lock_entry" || e.kind == "redirect_npm_lock_dep")
+            && !disk_locks.contains_key(&e.path)
+        {
             let parsed = read_rel(project_root, &e.path)
                 .await?
                 .and_then(|c| serde_json::from_str::<Value>(&c).ok());
@@ -386,9 +411,9 @@
 
     // Claim this purl's edits. Text-fragment kinds and the berry/classic/pnpm
     // rewriters key edits by `<name>@<version>`; the legacy npm v2
-    // `dependencies` tree keys by bare name; the v3 `packages` map keys by
-    // the lock path. A bun.lock edit that may belong to this purl is a hard
-    // refusal: bun edits key by the lock's package key (not name@version)
+    // `dependencies` tree and the v3 `packages` map also match by version
+    // (not just bare name). A bun.lock edit that may belong to this purl is a
+    // hard refusal: bun edits key by the lock's package key (not name@version)
     // and their revert is not implemented, so vendoring over one would drop
     // the record while stranding its edits — half a takeover.
     let mut mine: Vec<usize> = Vec::new();
@@ -396,13 +421,20 @@
         let key = e.key.as_deref().unwrap_or_default();
         let claimed = match e.kind.as_str() {
             k if NPM_TEXT_KINDS.contains(&k) => key == lock_key,
-            "redirect_npm_lock_dep" => key == name,
+            "redirect_npm_lock_dep" => {
+                key == name
+                    && disk_locks
+                        .get(&e.path)
+                        .and_then(|l| l.as_ref())
+                        .and_then(|l| l.get("dependencies"))
+                        .is_some_and(|deps| deps_contains_name_version(deps, &name, &version))
+            }
             "redirect_npm_lock_entry" => {
                 let key_name = key
                     .rsplit_once("node_modules/")
                     .map(|(_, n)| n)
                     .unwrap_or(key);
-                key_name == name
+                (key_name == name
                     || disk_locks
                         .get(&e.path)
                         .and_then(|l| l.as_ref())
@@ -410,6 +442,14 @@
                         .and_then(|p| p.get(key))
                         .is_some_and(|entry| {
                             entry.get("name").and_then(Value::as_str) == Some(name.as_str())
+                        }))
+                    && disk_locks
+                        .get(&e.path)
+                        .and_then(|l| l.as_ref())
+                        .and_then(|l| l.get("packages"))
+                        .and_then(|p| p.get(key))
+                        .is_some_and(|entry| {
+                            entry.get("version").and_then(Value::as_str) == Some(version.as_str())
                         })
             }
             "redirect_bun_lock_package" => {

diff --git a/crates/socket-patch-core/src/vendor/cargo.rs b/crates/socket-patch-core/src/vendor/cargo.rs
--- a/crates/socket-patch-core/src/vendor/cargo.rs
+++ b/crates/socket-patch-core/src/vendor/cargo.rs
@@ -1434,7 +1434,9 @@
             "marker must survive"
         );
         assert_eq!(
-            tokio::fs::read(root.join(".cargo/config.toml")).await.unwrap(),
+            tokio::fs::read(root.join(".cargo/config.toml"))
+                .await
+                .unwrap(),
             cfg1,
             "config untouched"
         );
@@ -1519,10 +1521,15 @@
         let copy = dir.path().join("cfg-if-1.0.4");
         let stage = stage_dir_for(&copy);
         tokio::fs::create_dir_all(&stage).await.unwrap();
-        tokio::fs::write(stage.join("lib.rs"), b"new\n").await.unwrap();
+        tokio::fs::write(stage.join("lib.rs"), b"new\n")
+            .await
+            .unwrap();
 
         swap_stage_into_place(&stage, &copy).await.unwrap();
-        assert_eq!(tokio::fs::read(copy.join("lib.rs")).await.unwrap(), b"new\n");
+        assert_eq!(
+            tokio::fs::read(copy.join("lib.rs")).await.unwrap(),
+            b"new\n"
+        );
         assert!(!backup_dir_for(&copy).exists());
         assert!(!stage.exists());
     }
@@ -1576,7 +1583,9 @@
              [[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nsource = \"{SOURCE}\"\nchecksum = \"{CHECKSUM}\"\n\n\
              [[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nsource = \"git+https://example.com/fork/cfg-if#abcdef\"\n"
         );
-        tokio::fs::write(root.join("Cargo.lock"), &lock).await.unwrap();
+        tokio::fs::write(root.join("Cargo.lock"), &lock)
+            .await
+            .unwrap();
 
         let detail = expect_refused(
             run_vendor(PURL, root, &blobs, &pristine, &record, false).await,
@@ -1603,7 +1612,9 @@
     async fn test_refuses_user_entry_through_foreign_socket_dir() {
         let (dir, blobs, pristine, record) = fixture().await;
         let root = dir.path();
-        tokio::fs::create_dir_all(root.join(".cargo")).await.unwrap();
+        tokio::fs::create_dir_all(root.join(".cargo"))
+            .await
+            .unwrap();
         let user_cfg = format!(
             "[patch.crates-io]\ncfg-if = {{ path = \"../shared-fork/.socket/vendor/cargo/{UUID2}/cfg-if-1.0.4\" }}\n"
         );

diff --git a/crates/socket-patch-core/src/vendor/cargo_config.rs b/crates/socket-patch-core/src/vendor/cargo_config.rs
--- a/crates/socket-patch-core/src/vendor/cargo_config.rs
+++ b/crates/socket-patch-core/src/vendor/cargo_config.rs
@@ -236,10 +236,12 @@
     if segments.contains(&"..") {
         return false;
     }
-    [CARGO_VENDOR_DIR, LEGACY_CARGO_PATCHES_DIR].iter().any(|dir| {
-        let prefix: Vec<&str> = dir.split('/').collect();
-        segments.len() > prefix.len() && segments[..prefix.len()] == prefix[..]
-    })
+    [CARGO_VENDOR_DIR, LEGACY_CARGO_PATCHES_DIR]
+        .iter()
+        .any(|dir| {
+            let prefix: Vec<&str> = dir.split('/').collect();
+            segments.len() > prefix.len() && segments[..prefix.len()] == prefix[..]
+        })
 }
 
 /// The `path` string of a `[patch]` entry (inline table or sub-table), if any.
@@ -384,9 +386,7 @@
         ));
         // A `..` INSIDE the owned prefix escapes it.
         assert!(!path_is_socket_owned(".socket/vendor/cargo/../../../etc"));
-        assert!(!path_is_socket_owned(
-            ".socket/cargo-patches/../../secrets"
-        ));
+        assert!(!path_is_socket_owned(".socket/cargo-patches/../../secrets"));
         // A nested sub-checkout's socket dir is not THIS project's.
         assert!(!path_is_socket_owned("sub/.socket/vendor/cargo/u/x-1.0.0"));
         // The bare owned dir itself (no copy segment) is not an entry we write.

diff --git a/crates/socket-patch-core/src/vendor/pnpm_lock.rs b/crates/socket-patch-core/src/vendor/pnpm_lock.rs
--- a/crates/socket-patch-core/src/vendor/pnpm_lock.rs
+++ b/crates/socket-patch-core/src/vendor/pnpm_lock.rs
@@ -194,8 +194,7 @@
     if let Err(detail) = check_lock_override(&lines, name, version, &effective_key) {
         return refused("vendor_override_conflict", detail);
     }
-    if let Err(detail) =
-        check_workspace_override(ws_text.as_deref(), name, version, &effective_key)
+    if let Err(detail) = check_workspace_override(ws_text.as_deref(), name, version, &effective_key)
     {
         return refused("vendor_override_conflict", detail);
     }
@@ -282,11 +281,11 @@
     // The pnpm >= 11 override surface. Mirrors the package.json override
     // key-for-key so whichever surface the installed pnpm reads matches the
     // lock's `overrides:` section.
-    let ws_edit = match apply_workspace_override(ws_text.as_deref(), &effective_key, &spec, &mut wiring)
-    {
-        Ok(edit) => edit,
-        Err(e) => return done_failure(purl, format!("{PNPM_WORKSPACE} surgery failed: {e}")),
-    };
+    let ws_edit =
+        match apply_workspace_override(ws_text.as_deref(), &effective_key, &spec, &mut wiring) {
+            Ok(edit) => edit,
+            Err(e) => return done_failure(purl, format!("{PNPM_WORKSPACE} surgery failed: {e}")),
+        };
 
     if !pkg_changed && !lock_changed && ws_edit.new_text.is_none() {
         // Everything already carries this uuid + the packed integrity: the
@@ -573,7 +572,10 @@
         .find(|r| r.file == PNPM_WORKSPACE && r.kind == KIND_WS_OVERRIDE)
     {
         let (created_file, created_overrides) = match &entry.pnpm {
-            Some(meta) => (meta.created_workspace_file, meta.created_workspace_overrides),
+            Some(meta) => (
+                meta.created_workspace_file,
+                meta.created_workspace_overrides,
+            ),
             None => (false, false),
         };
         if let Err(e) = revert_workspace(
@@ -688,7 +690,11 @@
         }
         match rec.original.as_ref().and_then(Value::as_str) {
             Some(orig) => {
-                lines[i] = format!("{}{}: {orig}", " ".repeat(indent), yaml_key_like(key, &repr));
+                lines[i] = format!(
+                    "{}{}: {orig}",
+                    " ".repeat(indent),
+                    yaml_key_like(key, &repr)
+                );
             }
             None => {
                 lines.remove(i);
@@ -1326,7 +1332,10 @@
             lines[i] = format!("{pad}{}: {spec}", yaml_key_like(our_key, &repr));
             wiring.push(ws_record(our_key, spec, WiringAction::Rewritten, original));
         } else {
-            lines.insert(last_entry + 1, format!("{pad}{}: {spec}", yaml_key(our_key)));
+            lines.insert(
+                last_entry + 1,
+                format!("{pad}{}: {spec}", yaml_key(our_key)),
+            );
             wiring.push(ws_record(our_key, spec, WiringAction::Added, None));
         }
         return Ok(WorkspaceEdit {
@@ -1346,7 +1355,10 @@
         .unwrap_or(lines.len());
     lines.splice(
         anchor..anchor,
-        ["overrides:".to_string(), format!("  {}: {spec}", yaml_key(our_key))],
+        [
+            "overrides:".to_string(),
+            format!("  {}: {spec}", yaml_key(our_key)),
+        ],
     );
     wiring.push(ws_record(our_key, spec, WiringAction::Added, None));
     Ok(WorkspaceEdit {
@@ -4103,7 +4115,10 @@
     #[tokio::test]
     async fn workspace_file_is_created_with_root_scaffold_and_revert_deletes_it() {
         let fx = fixture_with(P1_BEFORE_PKG, P1_BEFORE_LOCK).await;
-        assert!(!ws_exists(&fx).await, "fixture starts with no workspace file");
+        assert!(
+            !ws_exists(&fx).await,
+            "fixture starts with no workspace file"
+        );
 
         let (_, entry, _) = expect_done(fx.vendor(false).await);
         let entry = entry.unwrap();
@@ -4114,10 +4129,11 @@
             "created workspace carries `packages: ['.']` + the override"
         );
         // The three surfaces agree on the same key → value (no config mismatch).
-        assert!(fx.read(PNPM_WORKSPACE).await.contains(&format!(
-            "overrides:\n  left-pad@1.3.0: {spec}"
-        )));
         assert!(fx
+            .read(PNPM_WORKSPACE)
+            .await
+            .contains(&format!("overrides:\n  left-pad@1.3.0: {spec}")));
+        assert!(fx
             .read(PNPM_LOCK)
             .await
             .contains(&format!("overrides:\n  left-pad@1.3.0: {spec}")));
@@ -4240,7 +4256,10 @@
         assert!(pnpm_meta.created_pnpm_table && pnpm_meta.created_overrides_table);
         assert!(prev.wiring.iter().any(|r| r.file == PACKAGE_JSON));
         assert!(prev.wiring.iter().any(|r| r.file == PNPM_LOCK));
-        assert!(!ws_exists(&fx).await, "downgraded state has no workspace file");
+        assert!(
+            !ws_exists(&fx).await,
+            "downgraded state has no workspace file"
+        );
 
         // 2. Re-vendor under the current code: package.json + lock are already
         //    in sync, so ONLY the workspace mirror is written and the fresh
@@ -4267,7 +4286,11 @@
             P1_BEFORE_PKG,
             "package.json byte-restored"
         );
-        assert_eq!(fx.read(PNPM_LOCK).await, P1_BEFORE_LOCK, "lock byte-restored");
+        assert_eq!(
+            fx.read(PNPM_LOCK).await,
+            P1_BEFORE_LOCK,
+            "lock byte-restored"
+        );
         assert!(
             !ws_exists(&fx).await,
             "the workspace file the re-vendor created is deleted"

_{You can send follow-ups to the cloud agent here.}

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

^{Reviewed by Cursor Bugbot for commit b04ad42. Configure here.}

[cursor]

Lock edit claim ignores version

High Severity

revert_npm_redirect_purl claims redirect_npm_lock_entry and redirect_npm_lock_dep edits by package name only. The rewriter that recorded them matched name and version (and alias name fields). A takeover for one version can therefore claim, replay, and drop another version’s or alias-key sibling’s edits while only removing one ledger record, leaving corrupted hosted state or refusing a valid vendor.

 

^{Reviewed by Cursor Bugbot for commit b04ad42. Configure here.}