From a9bcc0bf29a64afe1bd95a722d32bca7fa6461bc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 22:37:01 +0000 Subject: [PATCH 01/12] Revert a vendored npm/Bun package after upgrade Moving a vendored package to another version (`npm install pkg@x`, `bun update`, a Dependabot bump) left the vendored copy and its ledger entry stuck. `scan --prune`, `vendor --revert`, `remove` and `rollback` called the moved lock entry "drift" and kept everything, so `vendor --check` stayed red and every remedy it named looped. A lock entry that now locks a different version than the one vendored means the vendored version left the lock graph, the same as after `npm uninstall`. The npm and Bun text-lock reverts now report it as `vendor_lock_entry_removed`, leave the user's lock untouched, and delete the artifact once no lock resolves through it. A re-resolution at the same version is still drift and still keeps the artifact. Fixes #1155 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/in_process_vendor.rs | 82 +++++++++ .../socket-patch-core/src/vendor/bun_lock.rs | 98 ++++++++++- .../socket-patch-core/src/vendor/npm_lock.rs | 158 ++++++++++++++++++ 3 files changed, 336 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 759f0c20a..edeab8488 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -543,6 +543,88 @@ async fn rollback_after_dependency_removed_cleans_up_and_converges() { assert!(events(&env).is_empty(), "nothing left to revert: {env:#}"); } +/// What `npm install left-pad@1.3.1` leaves behind after vendoring 1.3.0: +/// the same lock key, now resolving the new version from the registry. +fn upgrade_vendored_left_pad(fx: &NpmFixture) -> Vec { + let mut lock = fx.lock_value(); + lock["packages"]["node_modules/left-pad"] = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut upgraded = serde_json::to_vec_pretty(&lock).unwrap(); + upgraded.push(b'\n'); + std::fs::write(fx.lock_path(), &upgraded).unwrap(); + upgraded +} + +/// #1155: moving a vendored package off its patched version (`npm install +/// left-pad@1.3.1`, a Dependabot bump) takes the vendored version out of +/// the lock graph just like `npm uninstall`. `rollback` used to call that +/// drift, keep the artifact and ledger entry and exit 1 on every run, so +/// `vendor --check` stayed red. Now the first rollback cleans up, leaves +/// the user's upgraded lock alone, and later runs are clean no-ops. +#[tokio::test] +async fn rollback_after_version_upgrade_cleans_up_and_converges() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + let upgraded = upgrade_vendored_left_pad(&fx); + + let cwd = fx.root().to_str().unwrap(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &["rollback", "--json", "--yes", "--offline", "--cwd", cwd], + &[], + ); + assert_eq!(code, 0, "rollback must succeed:\n{stdout}\n{stderr}"); + assert!( + !stdout.contains("vendor_artifact_kept") && !stdout.contains("drifted"), + "nothing is kept as drift:\n{stdout}" + ); + assert!( + !fx.vendor_dir().exists(), + "the unreferenced artifact and ledger are cleaned up:\n{stdout}" + ); + assert_eq!(fx.lock_bytes(), upgraded, "the user's lock is untouched"); + + let (code, env) = vendor_cli(fx.root(), &["--revert"]); + assert_eq!(code, 0, "{env:#}"); + assert!(events(&env).is_empty(), "nothing left to revert: {env:#}"); + let (code, env) = vendor_cli(fx.root(), &["--check"]); + assert_eq!(code, 0, "vendor --check is green again: {env:#}"); +} + +/// #1155, `remove` leg: it used to end on "drift-kept …; re-run `scan +/// --mode vendored` to normalize, then remove again", a remedy that +/// changes nothing. Now it reverts the entry and exits 0. +#[tokio::test] +async fn remove_after_version_upgrade_reverts_vendoring() { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + let upgraded = upgrade_vendored_left_pad(&fx); + + let (code, stdout, stderr) = run_cli( + fx.root(), + &[ + "remove", + PURL, + "--json", + "--offline", + "--yes", + "--cwd", + fx.root().to_str().unwrap(), + ], + &[], + ); + assert_eq!(code, 0, "remove must succeed:\n{stdout}\n{stderr}"); + let env: Value = serde_json::from_str(&stdout).unwrap(); + let reverted = find_event(&env, "removed", Some("vendor_reverted")); + assert_eq!(reverted["purl"], PURL); + assert!(!stdout.contains("drift-kept"), "{env:#}"); + assert!(!fx.vendor_dir().exists(), "vendor tree fully removed"); + assert_eq!(fx.lock_bytes(), upgraded, "the user's lock is untouched"); +} + // ───────────────────────────────────────────────────────────────────── // 5. revert works without a manifest // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index 713ee8142..f9cefa27a 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -1037,6 +1037,22 @@ fn revert_one_record( .and_then(|path| parse_vendor_path(&path)) .is_some_and(|p| p.eco == "npm" && p.uuid == entry_uuid); if !exact && !ours_uuid { + // MOVED OFF THE VENDORED VERSION, not drifted (#1155): `bun + // update` / `bun add pkg@other` re-locked the entry at another + // registry version, so the vendored version left the lock graph + // exactly as after `bun remove`. Nothing to restore; the caller + // keeps the artifact only while the lock still resolves through + // it. A same-version re-resolution stays drift. + if let Some(live_version) = version_moved_off(rec, &parsed) { + warnings.push(VendorWarning::new( + super::LOCK_ENTRY_REMOVED_CODE, + format!( + "lock entry `{key}` now locks version {live_version}; the vendored \ + version is no longer installed, so there is nothing to restore" + ), + )); + return; + } warnings.push(drifted(format!( "lock entry `{key}` was re-resolved since vendoring; left alone" ))); @@ -1068,6 +1084,30 @@ fn revert_one_record( )); } +/// The registry version an entry tuple locks (`"left-pad@1.3.1"` → `1.3.1`), +/// or `None` for any other spec (a vendored path, a URL, `file:`/`github:`). +fn registry_version(entry: &BunEntry) -> Option { + let spec = decode_json_string(entry.elems.first()?)?; + let (_, version) = split_name_spec(&spec)?; + let plain = !version.is_empty() + && version + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')); + plain.then(|| version.to_string()) +} + +/// The live entry's registry version when it differs from the pre-vendor +/// one in `rec.original`: the user moved the package to another version +/// since vendoring. `None` when either side has no registry version to +/// compare (an `original: None` record, a URL or `file:` spec), which keeps +/// the caller's drift verdict. +fn version_moved_off(rec: &WiringRecord, live: &BunEntry) -> Option { + let live_version = registry_version(live)?; + let original = rec.original.as_ref().and_then(Value::as_str)?; + let original_version = registry_version(&parse_entry_line(original).ok()?)?; + (live_version != original_version).then_some(live_version) +} + // ───────────────────────── vendor-specific classification ───────────────── // The conservative line grammar (`BunEntry`, `parse_*`, `scan_*`, …) lives in // `crate::vendor::bun_lock_text`; this module keeps only the vendor tuple @@ -3127,14 +3167,68 @@ mod tests { ); } + /// #1155: `bun update` / `bun add left-pad@1.3.1` moved the vendored + /// package off its patched version. The vendored version left the lock + /// graph exactly as after `bun remove`, so the revert has nothing to + /// restore and the artifact goes once nothing resolves through it, + /// instead of a drift-keep that `vendor --check` can never clear. + #[tokio::test] + async fn revert_after_version_change_drops_the_unreferenced_artifact() { + let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let upgraded_line = " \"left-pad\": [\"left-pad@1.3.1\", \"\", {}, \"sha512-other==\"],"; + let live = fx.read_lock().await; + let new_line = entry.wiring[0] + .new + .as_ref() + .and_then(Value::as_str) + .unwrap(); + let upgraded_lock = live.replace(new_line, upgraded_line); + assert_ne!(upgraded_lock, live, "test setup must move the entry"); + tokio::fs::write(fx.root().join(BUN_LOCK), &upgraded_lock) + .await + .unwrap(); + + let outcome = revert_bun(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_removed" + && w.detail.contains("left-pad") + && w.detail.contains("1.3.1")), + "the version change is surfaced: {:?}", + outcome.warnings + ); + assert_eq!( + fx.read_lock().await, + upgraded_lock, + "the user's upgraded lock is left byte-identical" + ); + assert!( + !fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "nothing resolves through the artifact, so it is removed" + ); + } + #[tokio::test] async fn revert_leaves_drifted_entries_alone_with_warning() { let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; let (_, entry, _) = expect_done(fx.vendor(false).await); let entry = entry.unwrap(); - // The user re-resolved the entry behind our back (`bun update`). - let drifted_line = " \"left-pad\": [\"left-pad@1.3.1\", \"\", {}, \"sha512-other==\"],"; + // The user re-resolved the entry behind our back at the SAME + // version (a fork tarball). A version change is not drift (#1155); + // see `revert_after_version_change_drops_the_unreferenced_artifact`. + let drifted_line = + " \"left-pad\": [\"left-pad@https://example.com/left-pad-1.3.0.tgz\", {}],"; let live = fx.read_lock().await; let new_line = entry.wiring[0] .new diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index f1f49ffef..67165dcfa 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1163,6 +1163,20 @@ fn drifted_resolved_note(resolved: Option<&str>) -> String { } } +/// The live entry's `version` when it names neither the version we wired +/// (`rec.new`) nor the pre-vendor one (`rec.original`): the user moved the +/// package to another version since vendoring. `None` when either side has +/// no `version` to compare, which keeps the caller's drift verdict. +fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> { + let live_version = live.get("version").and_then(Value::as_str)?; + let recorded: Vec<&str> = [rec.new.as_ref(), rec.original.as_ref()] + .into_iter() + .flatten() + .filter_map(|v| v.get("version").and_then(Value::as_str)) + .collect(); + (!recorded.is_empty() && !recorded.contains(&live_version)).then_some(live_version) +} + /// Apply one wiring record in reverse: restore `original` iff the live /// fragment is still ours (drift = third party re-resolved it; leave theirs /// alone, with a warning). @@ -1227,6 +1241,23 @@ fn revert_one_record( None => false, }; if !ours { + // MOVED OFF THE VENDORED VERSION, not drifted (#1155): `npm install + // pkg@other` (or Dependabot) re-locked the entry at another + // version, so the vendored version left the lock graph exactly as + // after `npm uninstall`. Nothing to restore; the caller keeps the + // artifact only while a lock still resolves through it. A + // same-version re-resolution stays drift: re-vendoring can undo it. + if let Some(live_version) = version_moved_off(rec, live) { + warnings.push(VendorWarning::new( + super::LOCK_ENTRY_REMOVED_CODE, + format!( + "lock entry `{key}` now locks version {live_version} ({}); the vendored \ + version is no longer installed, so there is nothing to restore", + drifted_resolved_note(live_resolved) + ), + )); + return; + } warnings.push(VendorWarning::new( "vendor_lock_entry_drifted", format!( @@ -3370,6 +3401,133 @@ mod tests { ); } + /// #1155: the user moved the vendored package off its patched version + /// (`npm install left-pad@1.3.1`, or Dependabot doing the same), so + /// every recorded entry still exists but now resolves another version + /// from the registry. The vendored version left the lock graph just as + /// it does after `npm uninstall`: nothing to restore, and the artifact + /// goes once nothing resolves through it. Keeping it as drift would + /// leave `vendor --check` red with remedies that can never converge. + #[tokio::test] + async fn revert_after_version_change_drops_the_unreferenced_artifact() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let upgraded = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = upgraded.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = upgraded; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + let after_upgrade = tokio::fs::read(fx.lock_path()).await.unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + assert!( + outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_removed" + && w.detail.contains("node_modules/left-pad") + && w.detail.contains("1.3.1")), + "the version change is surfaced: {:?}", + outcome.warnings + ); + assert!( + !fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "nothing resolves through the artifact, so it is removed" + ); + assert_eq!( + tokio::fs::read(fx.lock_path()).await.unwrap(), + after_upgrade, + "the user's upgraded lock is left byte-identical" + ); + } + + /// #1155, downgrade of one instance: the direct copy moved to 1.2.0 + /// while the nested copy is still wired. The nested copy is restored, + /// the direct one is left as the user locked it, and the artifact goes + /// because the restore leaves nothing resolving through it. + #[tokio::test] + async fn revert_after_partial_downgrade_restores_the_rest_and_drops_artifact() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let downgraded = json!({ + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.2.0.tgz", + "integrity": "sha512-older==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = downgraded.clone(); + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + let after = fx.read_lock().await; + assert_eq!(after["packages"]["node_modules/left-pad"], downgraded); + assert_eq!( + after["packages"]["node_modules/foo/node_modules/left-pad"], + default_lock()["packages"]["node_modules/foo/node_modules/left-pad"], + "the still-wired instance is restored" + ); + assert!(!fx + .root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); + } + + /// #1155 guard: the recorded entry moved to another version, but the + /// lock still resolves through the artifact under a key the wiring + /// never recorded. The artifact may be the only copy that install + /// needs, so it is kept. + #[tokio::test] + async fn revert_keeps_artifact_when_version_changed_but_uuid_still_referenced() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let mut live = fx.read_lock().await; + let packages = live["packages"].as_object_mut().unwrap(); + let wired = packages["node_modules/left-pad"].clone(); + packages.insert("node_modules/bar/node_modules/left-pad".into(), wired); + packages.insert( + "node_modules/left-pad".into(), + json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }), + ); + packages.remove("node_modules/foo/node_modules/left-pad"); + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(outcome.kept_artifact, "{:?}", outcome.warnings); + assert!(fx + .root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); + } + /// #665 guard: a recorded entry vanished but the lock still resolves /// through the artifact under a key the wiring never recorded (npm /// re-hoisted it). The artifact may be the only copy that install From 8c6b688d476485ee5d70add27050fe01b677d6a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 22:49:19 +0000 Subject: [PATCH 02/12] Test scan --prune after a vendored npm upgrade Covers the `scan --prune` leg of #1155 end to end: after `npm install left-pad@1.3.1` over a vendored 1.3.0, one prune run reverts the entry, removes the artifact and leaves the user's lock byte-identical. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/tests/scan_vendor_e2e.rs | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs index 2f0208055..718e8018b 100644 --- a/crates/socket-patch-cli/tests/scan_vendor_e2e.rs +++ b/crates/socket-patch-cli/tests/scan_vendor_e2e.rs @@ -1153,6 +1153,97 @@ async fn scan_prune_reverts_unused_vendored_entry() { ); } +/// #1155: `npm install left-pad@1.3.1` after vendoring 1.3.0 keeps the +/// `node_modules/left-pad` key but locks the new version from the +/// registry. The vendored version left the lock graph just as after +/// `npm uninstall`, so `scan --prune` must revert the entry in one run. +/// It used to call the moved entry drift and keep it forever, so the +/// prune remedy `vendor --check` names never converged. +#[tokio::test] +async fn scan_prune_reverts_vendored_entry_after_version_upgrade() { + let mock = MockServer::start().await; + mount_patch_api(&mock, UUID).await; + let tmp = tempfile::tempdir().unwrap(); + write_fixture(tmp.path()); + + let (code, stdout, stderr) = run_scan_vendor(tmp.path(), &mock.uri(), &[]); + assert_eq!(code, 0, "stdout={stdout}; stderr={stderr}"); + assert!(tmp + .path() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); + + // What `npm install left-pad@1.3.1` leaves behind. + let lock = serde_json::json!({ + "name": "scan-vendor-test", + "version": "0.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "scan-vendor-test", + "version": "0.0.0", + "dependencies": { "left-pad": "^1.3.1" } + }, + "node_modules/left-pad": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==", + "license": "WTFPL" + } + } + }); + let mut lock_bytes = serde_json::to_vec_pretty(&lock).unwrap(); + lock_bytes.push(b'\n'); + std::fs::write(tmp.path().join("package-lock.json"), &lock_bytes).unwrap(); + std::fs::write( + tmp.path().join("node_modules/left-pad/package.json"), + br#"{"name":"left-pad","version":"1.3.1"}"#, + ) + .unwrap(); + + let out = Command::new(binary()) + .args([ + "scan", + "--json", + "--prune", + "--yes", + "--api-url", + &mock.uri(), + "--api-token", + "fake-token", + "--org", + ORG_SLUG, + ]) + .current_dir(tmp.path()) + .output() + .expect("run"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + assert_eq!(out.status.code(), Some(0), "stdout={stdout}"); + let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); + assert_eq!( + v["gc"]["revertedVendoredEntries"], + serde_json::json!([PURL]), + "gc must revert the upgraded-away entry: {v}" + ); + assert_eq!( + v["gc"]["keptVendoredEntries"], + serde_json::json!([]), + "nothing resolves through the artifact, so nothing is kept: {v}" + ); + assert!( + !tmp.path() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + "artifact dir removed" + ); + assert_eq!( + std::fs::read(tmp.path().join("package-lock.json")).unwrap(), + lock_bytes, + "the user's upgraded lock is left exactly as they wrote it" + ); +} + /// #541, npm package-lock flavor: after `npm uninstall left-pad` re-locks /// the project without the vendored dependency, a vendored rescan skips /// the stale ledger entry with a `vendor_ledger_entry_unwired` warning From 8294b9496caff43b409bb8d3b6bdfdbe083355a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:02:38 +0000 Subject: [PATCH 03/12] Only treat same-registry version moves as upgrades A lock entry that kept the vendored key but changed its version was reverted as an upgrade no matter where it resolved. An edited lock could point that entry at any tarball, and `scan --prune`, `remove` or `rollback` would then delete the vendored copy and turn `vendor --check` green. An upgrade now has to be the same package from the registry the pre-vendor entry used: for npm the new `resolved` must share the original's `//-/` tarball directory, and for Bun the spec's package name and the tuple's registry field must match. Anything else stays drift, keeps the artifact and leaves `vendor --check` red. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/bun_lock.rs | 66 ++++++++++++---- .../socket-patch-core/src/vendor/npm_lock.rs | 78 +++++++++++++++++-- 2 files changed, 125 insertions(+), 19 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index f9cefa27a..8866854d9 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -1084,28 +1084,34 @@ fn revert_one_record( )); } -/// The registry version an entry tuple locks (`"left-pad@1.3.1"` → `1.3.1`), -/// or `None` for any other spec (a vendored path, a URL, `file:`/`github:`). -fn registry_version(entry: &BunEntry) -> Option { +/// The package name and registry version an entry tuple locks +/// (`"left-pad@1.3.1"` → `("left-pad", "1.3.1")`), or `None` for any other +/// spec (a vendored path, a URL, `file:`/`github:`). +fn registry_name_version(entry: &BunEntry) -> Option<(String, String)> { let spec = decode_json_string(entry.elems.first()?)?; - let (_, version) = split_name_spec(&spec)?; + let (name, version) = split_name_spec(&spec)?; let plain = !version.is_empty() && version .chars() .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')); - plain.then(|| version.to_string()) + plain.then(|| (name.to_string(), version.to_string())) } -/// The live entry's registry version when it differs from the pre-vendor -/// one in `rec.original`: the user moved the package to another version -/// since vendoring. `None` when either side has no registry version to -/// compare (an `original: None` record, a URL or `file:` spec), which keeps -/// the caller's drift verdict. +/// The live entry's registry version when the user moved the package to +/// another version from the same registry since vendoring: same package +/// name, same registry field (the tuple's second element) as the pre-vendor +/// tuple in `rec.original`, different version. `None` otherwise, which +/// keeps the caller's drift verdict: an `original: None` record, a URL or +/// `file:` spec, another package or another registry is not a plain upgrade +/// and must keep the artifact and leave `vendor --check` red. fn version_moved_off(rec: &WiringRecord, live: &BunEntry) -> Option { - let live_version = registry_version(live)?; - let original = rec.original.as_ref().and_then(Value::as_str)?; - let original_version = registry_version(&parse_entry_line(original).ok()?)?; - (live_version != original_version).then_some(live_version) + let (live_name, live_version) = registry_name_version(live)?; + let original = parse_entry_line(rec.original.as_ref().and_then(Value::as_str)?).ok()?; + let (original_name, original_version) = registry_name_version(&original)?; + let same_registry = matches!((live.elems.get(1), original.elems.get(1)), + (Some(l), Some(o)) if decode_json_string(l).is_some() && decode_json_string(l) == decode_json_string(o)); + (same_registry && live_name == original_name && live_version != original_version) + .then_some(live_version) } // ───────────────────────── vendor-specific classification ───────────────── @@ -3218,6 +3224,38 @@ mod tests { ); } + /// #1155 provenance guard: a version change counts as an upgrade only + /// for the same package from the same registry field as the pre-vendor + /// tuple. Another registry or another package name stays drift. + #[tokio::test] + async fn revert_keeps_version_change_from_another_registry_as_drift() { + for moved_line in [ + " \"left-pad\": [\"left-pad@1.3.1\", \"https://evil.example.com/\", {}, \"sha512-other==\"],", + " \"left-pad\": [\"not-left-pad@1.3.1\", \"\", {}, \"sha512-other==\"],", + ] { + let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let live = fx.read_lock().await; + let new_line = entry.wiring[0] + .new + .as_ref() + .and_then(Value::as_str) + .unwrap(); + let moved_lock = live.replace(new_line, moved_line); + assert_ne!(moved_lock, live, "test setup must move the entry"); + tokio::fs::write(fx.root().join(BUN_LOCK), &moved_lock) + .await + .unwrap(); + + let outcome = revert_bun(&entry, fx.root(), false).await; + assert!(outcome.success, "{moved_line}: {:?}", outcome.error); + assert!(outcome.drift_skipped(), "{moved_line}: {:?}", outcome.warnings); + assert!(outcome.kept_artifact, "{moved_line}: {:?}", outcome.warnings); + assert_eq!(fx.read_lock().await, moved_lock, "left alone"); + } + } + #[tokio::test] async fn revert_leaves_drifted_entries_alone_with_warning() { let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 67165dcfa..c3ea75bc9 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1163,10 +1163,15 @@ fn drifted_resolved_note(resolved: Option<&str>) -> String { } } -/// The live entry's `version` when it names neither the version we wired -/// (`rec.new`) nor the pre-vendor one (`rec.original`): the user moved the -/// package to another version since vendoring. `None` when either side has -/// no `version` to compare, which keeps the caller's drift verdict. +/// The live entry's `version` when the user moved the package to another +/// version from the same registry since vendoring: the version differs +/// from both the one we wired (`rec.new`) and the pre-vendor one +/// (`rec.original`), and `resolved` is the same package's tarball on the +/// registry the pre-vendor entry used (same `//-/` prefix). +/// `None` otherwise, which keeps the caller's drift verdict: a missing +/// version or pre-vendor `resolved`, or a resolution anywhere else (another +/// host, another package, a URL or `file:` spec) is not a plain upgrade +/// and must keep the artifact and leave `vendor --check` red. fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> { let live_version = live.get("version").and_then(Value::as_str)?; let recorded: Vec<&str> = [rec.new.as_ref(), rec.original.as_ref()] @@ -1174,7 +1179,25 @@ fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> .flatten() .filter_map(|v| v.get("version").and_then(Value::as_str)) .collect(); - (!recorded.is_empty() && !recorded.contains(&live_version)).then_some(live_version) + if recorded.is_empty() || recorded.contains(&live_version) { + return None; + } + let original_resolved = rec.original.as_ref()?.get("resolved")?.as_str()?; + let prefix = registry_tarball_prefix(original_resolved)?; + let live_resolved = live.get("resolved").and_then(Value::as_str)?; + let leaf = live_resolved.strip_prefix(prefix)?; + (leaf.ends_with(".tgz") && !leaf.contains('/')).then_some(live_version) +} + +/// `https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz` → +/// `https://registry.npmjs.org/left-pad/-/`: the registry tarball directory +/// of one package, which every version of it shares. +fn registry_tarball_prefix(resolved: &str) -> Option<&str> { + if !(resolved.starts_with("https://") || resolved.starts_with("http://")) { + return None; + } + let at = resolved.rfind("/-/")?; + Some(&resolved[..at + 3]) } /// Apply one wiring record in reverse: restore `original` iff the live @@ -3492,6 +3515,51 @@ mod tests { .exists()); } + /// #1155 provenance guard: a version change is only an upgrade when the + /// new tarball is the same package on the registry the pre-vendor entry + /// used. A version change that resolves anywhere else (another host, + /// another package's tarball, a bare URL) is not something `npm + /// install pkg@x` writes, so it stays drift: the artifact is kept and + /// `vendor --check` stays red. + #[tokio::test] + async fn revert_keeps_version_change_resolved_off_the_recorded_registry_as_drift() { + for resolved in [ + "https://evil.example.com/left-pad/-/left-pad-1.3.1.tgz", + "https://registry.npmjs.org/not-left-pad/-/not-left-pad-1.3.1.tgz", + "https://example.com/left-pad-1.3.1.tgz", + "file:../left-pad-1.3.1.tgz", + ] { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let moved = json!({ + "version": "1.3.1", + "resolved": resolved, + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = moved.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = moved; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{resolved}: {:?}", outcome.error); + assert!( + outcome.drift_skipped(), + "{resolved}: {:?}", + outcome.warnings + ); + assert!(outcome.kept_artifact, "{resolved}: {:?}", outcome.warnings); + assert!(fx + .root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists()); + } + } + /// #1155 guard: the recorded entry moved to another version, but the /// lock still resolves through the artifact under a key the wiring /// never recorded. The artifact may be the only copy that install From d1428a3eaa11735052c156bb459d23a9efbf3969 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:13:19 +0000 Subject: [PATCH 04/12] Accept http-to-https registry upgrades Older npm locks record `http://` registry tarball URLs, and the next `npm install` rewrites them to `https://`. An upgrade made that way was still called drift, so the vendored copy stayed stuck as in #1155. A move from `http` to `https` on the same registry now counts as an upgrade; a move from `https` down to `http` stays drift. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 90 ++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index c3ea75bc9..84fbac7a7 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1183,21 +1183,35 @@ fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> return None; } let original_resolved = rec.original.as_ref()?.get("resolved")?.as_str()?; - let prefix = registry_tarball_prefix(original_resolved)?; + let (original_scheme, prefix) = registry_tarball_prefix(original_resolved)?; let live_resolved = live.get("resolved").and_then(Value::as_str)?; - let leaf = live_resolved.strip_prefix(prefix)?; + let (live_scheme, live_rest) = split_http_scheme(live_resolved)?; + // npm rewrites an old lock's `http://` registry URLs to `https://` on + // the next install, so that upgrade is the same registry; a move from + // `https://` down to `http://` is not. + if live_scheme != original_scheme && (original_scheme, live_scheme) != ("http", "https") { + return None; + } + let leaf = live_rest.strip_prefix(prefix)?; (leaf.ends_with(".tgz") && !leaf.contains('/')).then_some(live_version) } /// `https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz` → -/// `https://registry.npmjs.org/left-pad/-/`: the registry tarball directory -/// of one package, which every version of it shares. -fn registry_tarball_prefix(resolved: &str) -> Option<&str> { - if !(resolved.starts_with("https://") || resolved.starts_with("http://")) { - return None; - } - let at = resolved.rfind("/-/")?; - Some(&resolved[..at + 3]) +/// `("https", "registry.npmjs.org/left-pad/-/")`: the scheme and the +/// registry tarball directory of one package, which every version of it +/// shares. +fn registry_tarball_prefix(resolved: &str) -> Option<(&str, &str)> { + let (scheme, rest) = split_http_scheme(resolved)?; + let at = rest.rfind("/-/")?; + Some((scheme, &rest[..at + 3])) +} + +/// `https://host/path` → `("https", "host/path")`; `None` for anything but +/// an `http`/`https` URL. +fn split_http_scheme(url: &str) -> Option<(&str, &str)> { + ["https", "http"] + .into_iter() + .find_map(|scheme| Some((scheme, url.strip_prefix(scheme)?.strip_prefix("://")?))) } /// Apply one wiring record in reverse: restore `original` iff the live @@ -3515,6 +3529,62 @@ mod tests { .exists()); } + /// #1155, old lock: npm 6 recorded `http://registry.npmjs.org/...` + /// tarball URLs, and the next `npm install` writes `https://`. That + /// upgrade is from the same registry, so it reverts like any other. + /// The reverse (an `https` entry moved to `http`) stays drift. + #[tokio::test] + async fn revert_after_version_change_accepts_http_to_https_upgrade_only() { + for (original, moved, reverts) in [ + ( + "http://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + true, + ), + ( + "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + "http://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + false, + ), + ] { + let mut lock = default_lock(); + lock["packages"]["node_modules/left-pad"]["resolved"] = json!(original); + lock["packages"]["node_modules/foo/node_modules/left-pad"]["resolved"] = + json!(original); + let fx = fixture_with("left-pad", "1.3.0", lock).await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let upgraded = json!({ + "version": "1.3.1", + "resolved": moved, + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = upgraded.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = upgraded; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{moved}: {:?}", outcome.error); + assert_eq!( + outcome.drift_skipped(), + !reverts, + "{moved}: {:?}", + outcome.warnings + ); + assert_eq!( + fx.root() + .join(format!(".socket/vendor/npm/{UUID}")) + .exists(), + !reverts, + "{moved}" + ); + } + } + /// #1155 provenance guard: a version change is only an upgrade when the /// new tarball is the same package on the registry the pre-vendor entry /// used. A version change that resolves anywhere else (another host, From 4b1630c13df1b94e23a874e162f858b866945a3e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:18:46 +0000 Subject: [PATCH 05/12] Require the exact tarball name for upgrades An upgraded npm entry was accepted when its `resolved` sat under the recorded registry directory and ended in `.tgz`. A URL written with backslashes or `..` passed that check, but npm normalizes it before fetching, so it could point at another package's tarball while the vendored copy was deleted. The tarball file must now be exactly `-.tgz`, with the name taken from the pre-vendor tarball and a plain version; anything else stays drift. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 84fbac7a7..3fbb51397 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1192,8 +1192,25 @@ fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> if live_scheme != original_scheme && (original_scheme, live_scheme) != ("http", "https") { return None; } + // The tarball file must be exactly `-.tgz`, with the + // basename taken from the pre-vendor tarball and a plain version, so + // no separator, `..`, `\`, query or fragment can steer npm's fetch to + // another package after it normalizes the URL. + let original_leaf = split_http_scheme(original_resolved)? + .1 + .strip_prefix(prefix)?; + let original_version = rec + .original + .as_ref()? + .get("version") + .and_then(Value::as_str)?; + let basename = original_leaf.strip_suffix(&format!("-{original_version}.tgz"))?; + let plain_version = !live_version.is_empty() + && live_version + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')); let leaf = live_rest.strip_prefix(prefix)?; - (leaf.ends_with(".tgz") && !leaf.contains('/')).then_some(live_version) + (plain_version && leaf == format!("{basename}-{live_version}.tgz")).then_some(live_version) } /// `https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz` → @@ -3598,6 +3615,9 @@ mod tests { "https://registry.npmjs.org/not-left-pad/-/not-left-pad-1.3.1.tgz", "https://example.com/left-pad-1.3.1.tgz", "file:../left-pad-1.3.1.tgz", + "https://registry.npmjs.org/left-pad/-/..\\..\\not-left-pad\\-\\not-left-pad-1.3.1.tgz", + "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz?x=/../../evil", + "https://registry.npmjs.org/left-pad/-/other-1.3.1.tgz", ] { let fx = fixture().await; let (_, entry, _) = expect_done(fx.vendor(false).await); From 264b6ff1155937326767f22fc795309a7a1becc3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:33:29 +0000 Subject: [PATCH 06/12] Read legacy npm alias versions on upgrade Lockfile v1 alias rows store their version as `npm:left-pad@1.3.0`, so the new exact tarball-name check never matched them and a real alias upgrade was still kept as drift. The tarball version is now read from after the last `@` of an `npm:` spec before the same strict check runs. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 67 ++++++++++++++++--- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 3fbb51397..5686c86a7 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1199,18 +1199,30 @@ fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> let original_leaf = split_http_scheme(original_resolved)? .1 .strip_prefix(prefix)?; - let original_version = rec - .original - .as_ref()? - .get("version") - .and_then(Value::as_str)?; + let original_version = tarball_version( + rec.original + .as_ref()? + .get("version") + .and_then(Value::as_str)?, + ); let basename = original_leaf.strip_suffix(&format!("-{original_version}.tgz"))?; - let plain_version = !live_version.is_empty() - && live_version + let tarball = tarball_version(live_version); + let plain_version = !tarball.is_empty() + && tarball .chars() .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')); let leaf = live_rest.strip_prefix(prefix)?; - (plain_version && leaf == format!("{basename}-{live_version}.tgz")).then_some(live_version) + (plain_version && leaf == format!("{basename}-{tarball}.tgz")).then_some(live_version) +} + +/// The version a lock `version` field names in its tarball file: itself, +/// or for a legacy (lockfile v1) alias row's `npm:left-pad@1.3.0` / +/// `npm:@scope/pkg@1.0.0` spelling, the part after the last `@`. +fn tarball_version(version: &str) -> &str { + match version.strip_prefix("npm:") { + Some(spec) => spec.rsplit_once('@').map_or(spec, |(_, v)| v), + None => version, + } } /// `https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz` → @@ -3602,6 +3614,45 @@ mod tests { } } + /// #1155, legacy alias: a lockfile v1 alias row spells its version + /// `npm:left-pad@1.3.0`. Upgrading the alias (`npm install + /// pad@npm:left-pad@1.3.1`) writes `npm:left-pad@1.3.1` with the 1.3.1 + /// registry tarball, which is the same plain upgrade. An alias pointed + /// at another package's tarball stays drift. + #[test] + fn version_moved_off_reads_legacy_alias_versions() { + let rec = WiringRecord { + file: PACKAGE_LOCK.to_string(), + kind: KIND_LOCK_LEGACY_ENTRY.to_string(), + action: WiringAction::Rewritten, + key: Some("/dependencies/pad".to_string()), + original: Some(json!({ + "version": "npm:left-pad@1.3.0", + "resolved": REG_RESOLVED, + "integrity": "sha512-orig==" + })), + new: Some(json!({ + "version": "npm:left-pad@1.3.0", + "resolved": format!("file:.socket/vendor/npm/{UUID}/left-pad-1.3.0.tgz"), + })), + }; + let upgraded = json!({ + "version": "npm:left-pad@1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + }); + assert_eq!( + version_moved_off(&rec, &upgraded), + Some("npm:left-pad@1.3.1") + ); + let elsewhere = json!({ + "version": "npm:left-pad@1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/other-1.3.1.tgz", + }); + assert_eq!(version_moved_off(&rec, &elsewhere), None); + assert_eq!(tarball_version("npm:@scope/pkg@1.0.0"), "1.0.0"); + assert_eq!(tarball_version("1.0.0"), "1.0.0"); + } + /// #1155 provenance guard: a version change is only an upgrade when the /// new tarball is the same package on the registry the pre-vendor entry /// used. A version change that resolves anywhere else (another host, From f379b2d3db105a663b5b60ff52848abe63b7b5b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 23:53:41 +0000 Subject: [PATCH 07/12] Keep non-registry installs out of upgrades An entry npm installs from a git, URL or `file:` spec could still be read as a registry upgrade when its lock `resolved` was written in the registry tarball shape. npm ci installs such an edge from the spec, so the revert deleted the vendored copy while something else got installed. The revert now reads the same non-registry edge set the vendor scan uses (including a registry override's rescue, #490) and keeps any such entry as drift. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/npm_lock.rs | 74 ++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 5686c86a7..2b16ce474 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -748,6 +748,9 @@ pub async fn revert_npm_opts( } } + // An entry npm installs from a non-registry spec (git, URL, `file:`) is + // never a registry upgrade, whatever its `resolved` says (#326). + let overrides = NpmOverrides::read(project_root).await; for lock_name in lock_files { let lock_path = project_root.join(lock_name); let lock_bytes = match read_regular_to_bytes(&lock_path).await { @@ -774,12 +777,14 @@ pub async fn revert_npm_opts( } }; + let non_registry = npm_non_registry_entries(&lock, &overrides); let mut changed = false; // Reverse application order, like every backend's revert. for rec in entry.wiring.iter().rev().filter(|r| r.file == lock_name) { revert_one_record( &mut lock, rec, + &non_registry, &entry.uuid, &mut changed, &mut outcome.warnings, @@ -1215,6 +1220,22 @@ fn version_moved_off<'a>(rec: &WiringRecord, live: &'a Value) -> Option<&'a str> (plain_version && leaf == format!("{basename}-{tarball}.tgz")).then_some(live_version) } +/// The `packages` key a legacy `dependencies` wiring pointer mirrors +/// (`/dependencies/a/dependencies/b` → `node_modules/a/node_modules/b`), or +/// `None` for a pointer of any other shape. +fn legacy_pointer_packages_key(pointer: &str) -> Option { + let mut tokens = pointer.strip_prefix('/')?.split('/'); + let mut key = String::new(); + while let Some(field) = tokens.next() { + if field != "dependencies" { + return None; + } + let name = tokens.next()?.replace("~1", "/").replace("~0", "~"); + key = legacy_packages_key(&key, &name); + } + (!key.is_empty()).then_some(key) +} + /// The version a lock `version` field names in its tarball file: itself, /// or for a legacy (lockfile v1) alias row's `npm:left-pad@1.3.0` / /// `npm:@scope/pkg@1.0.0` spelling, the part after the last `@`. @@ -1249,6 +1270,7 @@ fn split_http_scheme(url: &str) -> Option<(&str, &str)> { fn revert_one_record( lock: &mut Value, rec: &WiringRecord, + non_registry: &BTreeMap, entry_uuid: &str, changed: &mut bool, warnings: &mut Vec, @@ -1313,7 +1335,12 @@ fn revert_one_record( // after `npm uninstall`. Nothing to restore; the caller keeps the // artifact only while a lock still resolves through it. A // same-version re-resolution stays drift: re-vendoring can undo it. - if let Some(live_version) = version_moved_off(rec, live) { + let packages_key = match rec.kind.as_str() { + KIND_LOCK_LEGACY_ENTRY => legacy_pointer_packages_key(key), + _ => Some(key.to_string()), + }; + let registry_install = packages_key.is_some_and(|k| !non_registry.contains_key(&k)); + if let Some(live_version) = version_moved_off(rec, live).filter(|_| registry_install) { warnings.push(VendorWarning::new( super::LOCK_ENTRY_REMOVED_CODE, format!( @@ -3558,6 +3585,51 @@ mod tests { .exists()); } + /// #1155 provenance guard: an edge npm installs from a git, URL or + /// `file:` spec is not a registry upgrade even when its `packages` entry + /// is written in the registry tarball shape: npm ci installs it from the + /// spec. It stays drift and the artifact is kept. + #[tokio::test] + async fn revert_keeps_version_change_behind_a_non_registry_spec_as_drift() { + for spec in [ + "github:stevemao/left-pad#v1.3.1", + "https://example.com/left-pad-1.3.1.tgz", + "file:../left-pad", + ] { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + + let forged = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"][""]["dependencies"]["left-pad"] = json!(spec); + live["packages"]["node_modules/left-pad"] = forged.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = forged; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{spec}: {:?}", outcome.error); + assert!(outcome.drift_skipped(), "{spec}: {:?}", outcome.warnings); + assert!(outcome.kept_artifact, "{spec}: {:?}", outcome.warnings); + } + } + + #[test] + fn legacy_pointer_maps_to_its_packages_key() { + assert_eq!( + legacy_pointer_packages_key("/dependencies/foo/dependencies/@s~1pad").as_deref(), + Some("node_modules/foo/node_modules/@s/pad") + ); + assert_eq!(legacy_pointer_packages_key("/packages/x"), None); + assert_eq!(legacy_pointer_packages_key("/dependencies"), None); + } + /// #1155, old lock: npm 6 recorded `http://registry.npmjs.org/...` /// tarball URLs, and the next `npm install` writes `https://`. That /// upgrade is from the same registry, so it reverts like any other. From 5c52a322eccbfbcf9888666f31f628bf64dabc9e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 00:15:31 +0000 Subject: [PATCH 08/12] Distrust upgrades a project config can redirect Two more ways an edited project could make the revert delete a vendored copy while something else gets installed: - npm: a package.json override can swap a registry edge for a git, URL or file: spec, which npm ci installs instead of the lock's resolved tarball. While any override names the vendored package, a version change now stays drift. - Bun: an empty registry field means the default registry, which a committed bunfig.toml or .npmrc can rebind. When either file names a registry, an upgrade from the default registry now stays drift. Both fall back to the pre-#1155 behavior, which keeps the vendored copy, whenever the project's own config could change the install source. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/bun_lock.rs | 82 +++++++++++++++++-- .../socket-patch-core/src/vendor/npm_lock.rs | 66 ++++++++++++++- .../src/vendor/npm_origin.rs | 17 ++++ 3 files changed, 159 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index 8866854d9..d94df0e9b 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -928,10 +928,22 @@ pub(crate) async fn revert_bun_opts( } } + // An empty registry field means "the default registry", which a + // committed bunfig.toml or .npmrc can rebind; with either naming a + // registry, an empty field proves nothing about where an upgrade + // installs from (#1155). + let default_registry_pinned = !project_configures_a_registry(project_root).await; let mut dirty = false; if let Some(lines) = lines.as_mut() { for rec in entry.wiring.iter().rev().filter(|r| r.file == BUN_LOCK) { - revert_one_record(lines, rec, &entry.uuid, &mut dirty, &mut outcome.warnings); + revert_one_record( + lines, + rec, + &entry.uuid, + default_registry_pinned, + &mut dirty, + &mut outcome.warnings, + ); } if dirty { if let Err(e) = atomic_write_bytes_preserving_mode( @@ -986,6 +998,7 @@ fn revert_one_record( lines: &mut [String], rec: &WiringRecord, entry_uuid: &str, + default_registry_pinned: bool, dirty: &mut bool, warnings: &mut Vec, ) { @@ -1043,7 +1056,7 @@ fn revert_one_record( // exactly as after `bun remove`. Nothing to restore; the caller // keeps the artifact only while the lock still resolves through // it. A same-version re-resolution stays drift. - if let Some(live_version) = version_moved_off(rec, &parsed) { + if let Some(live_version) = version_moved_off(rec, &parsed, default_registry_pinned) { warnings.push(VendorWarning::new( super::LOCK_ENTRY_REMOVED_CODE, format!( @@ -1104,16 +1117,36 @@ fn registry_name_version(entry: &BunEntry) -> Option<(String, String)> { /// keeps the caller's drift verdict: an `original: None` record, a URL or /// `file:` spec, another package or another registry is not a plain upgrade /// and must keep the artifact and leave `vendor --check` red. -fn version_moved_off(rec: &WiringRecord, live: &BunEntry) -> Option { +fn version_moved_off( + rec: &WiringRecord, + live: &BunEntry, + default_registry_pinned: bool, +) -> Option { let (live_name, live_version) = registry_name_version(live)?; let original = parse_entry_line(rec.original.as_ref().and_then(Value::as_str)?).ok()?; let (original_name, original_version) = registry_name_version(&original)?; - let same_registry = matches!((live.elems.get(1), original.elems.get(1)), - (Some(l), Some(o)) if decode_json_string(l).is_some() && decode_json_string(l) == decode_json_string(o)); + let live_registry = decode_json_string(live.elems.get(1)?)?; + let same_registry = Some(&live_registry) == decode_json_string(original.elems.get(1)?).as_ref() + && (!live_registry.is_empty() || default_registry_pinned); (same_registry && live_name == original_name && live_version != original_version) .then_some(live_version) } +/// True when the project's own `bunfig.toml` or `.npmrc` mentions a +/// registry at all (or can't be read), so Bun's default registry may not +/// be npmjs. Deliberately coarse: any doubt keeps the drift verdict. +async fn project_configures_a_registry(project_root: &Path) -> bool { + for name in ["bunfig.toml", ".npmrc"] { + match read_regular_to_string(&project_root.join(name)).await { + Ok(text) if text.to_ascii_lowercase().contains("registry") => return true, + Ok(_) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false +} + // ───────────────────────── vendor-specific classification ───────────────── // The conservative line grammar (`BunEntry`, `parse_*`, `scan_*`, …) lives in // `crate::vendor::bun_lock_text`; this module keeps only the vendor tuple @@ -3256,6 +3289,45 @@ mod tests { } } + /// #1155 provenance guard: an empty registry field means "the default + /// registry", and a committed bunfig.toml or .npmrc can rebind that. + /// With either naming a registry, a version change from `""` is not + /// proven to come from the pre-vendor registry, so it stays drift. + #[tokio::test] + async fn revert_keeps_default_registry_upgrade_as_drift_when_the_registry_is_configured() { + for (file, text) in [ + ( + "bunfig.toml", + "[install]\nregistry = \"https://evil.example.com/\"\n", + ), + (".npmrc", "registry=https://evil.example.com/\n"), + ] { + let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let upgraded_line = + " \"left-pad\": [\"left-pad@1.3.1\", \"\", {}, \"sha512-other==\"],"; + let live = fx.read_lock().await; + let new_line = entry.wiring[0] + .new + .as_ref() + .and_then(Value::as_str) + .unwrap(); + tokio::fs::write( + fx.root().join(BUN_LOCK), + live.replace(new_line, upgraded_line), + ) + .await + .unwrap(); + tokio::fs::write(fx.root().join(file), text).await.unwrap(); + + let outcome = revert_bun(&entry, fx.root(), false).await; + assert!(outcome.success, "{file}: {:?}", outcome.error); + assert!(outcome.drift_skipped(), "{file}: {:?}", outcome.warnings); + assert!(outcome.kept_artifact, "{file}: {:?}", outcome.warnings); + } + } + #[tokio::test] async fn revert_leaves_drifted_entries_alone_with_warning() { let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 2b16ce474..d032048ed 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -777,7 +777,26 @@ pub async fn revert_npm_opts( } }; - let non_registry = npm_non_registry_entries(&lock, &overrides); + let mut non_registry = npm_non_registry_entries(&lock, &overrides); + // An override can also swap a registry edge for a git / URL / + // `file:` spec, which that set does not report. Any override that + // names the vendored package keeps every record off the upgrade + // path; the revert then drift-keeps as before #1155. + let overridden = super::npm_common::parse_npm_purl(&entry.base_purl) + .is_none_or(|(name, _)| overrides.mentions(&name)); + if overridden { + for rec in entry.wiring.iter().filter(|r| r.file == lock_name) { + if let Some(key) = rec.key.as_deref() { + let key = match rec.kind.as_str() { + KIND_LOCK_LEGACY_ENTRY => legacy_pointer_packages_key(key), + _ => Some(key.to_string()), + }; + if let Some(key) = key { + non_registry.insert(key, "an override names the package".to_string()); + } + } + } + } let mut changed = false; // Reverse application order, like every backend's revert. for rec in entry.wiring.iter().rev().filter(|r| r.file == lock_name) { @@ -3620,6 +3639,51 @@ mod tests { } } + /// #1155 provenance guard: an override can swap a registry edge for a + /// git / URL / `file:` spec that npm ci installs instead of the lock's + /// `resolved`. While any override names the vendored package, a version + /// change is not trusted as a registry upgrade and stays drift. + #[tokio::test] + async fn revert_keeps_version_change_as_drift_while_an_override_names_the_package() { + for overrides in [ + json!({ "left-pad": "file:../left-pad" }), + json!({ "foo": { "left-pad": "github:evil/left-pad" } }), + json!({ "left-pad@1.3.0": "https://example.com/left-pad.tgz" }), + ] { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + let manifest = json!({ "name": "fixture", "version": "1.0.0", "overrides": overrides }); + tokio::fs::write( + fx.root().join("package.json"), + serialize_json(&manifest, " ").unwrap(), + ) + .await + .unwrap(); + + let upgraded = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = upgraded.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = upgraded; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{overrides}: {:?}", outcome.error); + assert!( + outcome.drift_skipped(), + "{overrides}: {:?}", + outcome.warnings + ); + assert!(outcome.kept_artifact, "{overrides}: {:?}", outcome.warnings); + } + } + #[test] fn legacy_pointer_maps_to_its_packages_key() { assert_eq!( diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index 9c8a31840..17badb8ca 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -89,6 +89,23 @@ impl NpmOverrides { } } + /// True when any override rule, at any nesting depth, names the package + /// `name` (bare or with a selector). Coarser than [`Self::replacement`] + /// on purpose: a caller that must not trust an edge's registry + /// resolution while an override might swap it uses this. + pub(crate) fn mentions(&self, name: &str) -> bool { + fn walk(rules: &Map, name: &str) -> bool { + rules.iter().any(|(key, value)| { + let at = key + .char_indices() + .find_map(|(i, c)| (c == '@' && i > 0).then_some(i)) + .unwrap_or(key.len()); + &key[..at] == name || value.as_object().is_some_and(|nested| walk(nested, name)) + }) + } + walk(&self.rules, name) + } + /// The spec an override makes npm install for the edge `dep_name@spec` /// whose dependent sits at the lock key `from`, or `None` when no /// override clearly applies. From c5c0736ab3c1d302f596a847ef98969c2f7aa29d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 00:40:47 +0000 Subject: [PATCH 09/12] Trust upgrades only without redirecting config Review found more ways committed project config can change where an "upgraded" package installs from, while the revert deletes the vendored copy: a project .npmrc registry or replace-registry-host rewrites npmjs dist URLs at fetch time, a Bun [install.scopes] table rebinds a scope, and an npm override keyed on an alias edge swaps its source. Instead of matching each spelling, the upgrade shortcut now applies only when the project has no such config at all: no `overrides` in package.json, and no .npmrc or bunfig.toml that mentions a registry or a scope (or can't be read). Otherwise the revert drift-keeps exactly as before #1155. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/vendor/bun_lock.rs | 28 +++----- .../src/vendor/npm_common.rs | 23 +++++++ .../socket-patch-core/src/vendor/npm_lock.rs | 65 +++++++++++++++---- .../src/vendor/npm_origin.rs | 18 +---- 4 files changed, 87 insertions(+), 47 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/bun_lock.rs b/crates/socket-patch-core/src/vendor/bun_lock.rs index d94df0e9b..d52e72e09 100644 --- a/crates/socket-patch-core/src/vendor/bun_lock.rs +++ b/crates/socket-patch-core/src/vendor/bun_lock.rs @@ -929,10 +929,11 @@ pub(crate) async fn revert_bun_opts( } // An empty registry field means "the default registry", which a - // committed bunfig.toml or .npmrc can rebind; with either naming a - // registry, an empty field proves nothing about where an upgrade - // installs from (#1155). - let default_registry_pinned = !project_configures_a_registry(project_root).await; + // committed bunfig.toml or .npmrc can rebind (`registry`, a scope + // table); with either configuring one, an empty field proves nothing + // about where an upgrade installs from (#1155). + let default_registry_pinned = + !super::npm_common::project_may_redirect_registry(project_root).await; let mut dirty = false; if let Some(lines) = lines.as_mut() { for rec in entry.wiring.iter().rev().filter(|r| r.file == BUN_LOCK) { @@ -1132,21 +1133,6 @@ fn version_moved_off( .then_some(live_version) } -/// True when the project's own `bunfig.toml` or `.npmrc` mentions a -/// registry at all (or can't be read), so Bun's default registry may not -/// be npmjs. Deliberately coarse: any doubt keeps the drift verdict. -async fn project_configures_a_registry(project_root: &Path) -> bool { - for name in ["bunfig.toml", ".npmrc"] { - match read_regular_to_string(&project_root.join(name)).await { - Ok(text) if text.to_ascii_lowercase().contains("registry") => return true, - Ok(_) => {} - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(_) => return true, - } - } - false -} - // ───────────────────────── vendor-specific classification ───────────────── // The conservative line grammar (`BunEntry`, `parse_*`, `scan_*`, …) lives in // `crate::vendor::bun_lock_text`; this module keeps only the vendor tuple @@ -3301,6 +3287,10 @@ mod tests { "[install]\nregistry = \"https://evil.example.com/\"\n", ), (".npmrc", "registry=https://evil.example.com/\n"), + ( + "bunfig.toml", + "[install.scopes]\n\"@s\" = \"https://evil.example.com/\"\n", + ), ] { let fx = fixture_with(BN3_BEFORE_LOCK, "node_modules/left-pad").await; let (_, entry, _) = expect_done(fx.vendor(false).await); diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index 96403cd64..a197e76be 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -64,6 +64,29 @@ pub(super) struct NpmCoords { /// vendor, arbitrary delete on revert) — reject fail-closed before any disk /// access. `Err` carries a ready [`VendorOutcome::Refused`] to bubble /// verbatim. +/// True when the project's own `.npmrc` or `bunfig.toml` could make npm or +/// Bun fetch a registry package from somewhere other than the URL its lock +/// records (a `registry` / `@scope:registry` / `replace-registry-host` +/// line, a Bun scope table), or can't be read. Deliberately coarse: a +/// revert that would delete a vendored copy because a version moved +/// "within the same registry" trusts that move only when this is false +/// (#1155). +pub(super) async fn project_may_redirect_registry(project_root: &Path) -> bool { + for name in [".npmrc", "bunfig.toml"] { + match crate::utils::fs::read_regular_to_string(&project_root.join(name)).await { + Ok(text) => { + let text = text.to_ascii_lowercase(); + if text.contains("registry") || text.contains("scope") { + return true; + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false +} + pub(super) fn guard_coordinates( purl: &str, record: &PatchRecord, diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index d032048ed..b688c5a2b 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -751,6 +751,8 @@ pub async fn revert_npm_opts( // An entry npm installs from a non-registry spec (git, URL, `file:`) is // never a registry upgrade, whatever its `resolved` says (#326). let overrides = NpmOverrides::read(project_root).await; + let upgrades_trusted = overrides.is_empty() + && !super::npm_common::project_may_redirect_registry(project_root).await; for lock_name in lock_files { let lock_path = project_root.join(lock_name); let lock_bytes = match read_regular_to_bytes(&lock_path).await { @@ -778,13 +780,12 @@ pub async fn revert_npm_opts( }; let mut non_registry = npm_non_registry_entries(&lock, &overrides); - // An override can also swap a registry edge for a git / URL / - // `file:` spec, which that set does not report. Any override that - // names the vendored package keeps every record off the upgrade - // path; the revert then drift-keeps as before #1155. - let overridden = super::npm_common::parse_npm_purl(&entry.base_purl) - .is_none_or(|(name, _)| overrides.mentions(&name)); - if overridden { + // Any `overrides` rule (it can swap a registry edge, alias edges + // included, for a git / URL / `file:` spec npm ci installs instead + // of the lock's `resolved`) or a project `.npmrc` that can rebind + // the registry host keeps every record off the upgrade path; the + // revert then drift-keeps as before #1155. + if !upgrades_trusted { for rec in entry.wiring.iter().filter(|r| r.file == lock_name) { if let Some(key) = rec.key.as_deref() { let key = match rec.kind.as_str() { @@ -792,7 +793,7 @@ pub async fn revert_npm_opts( _ => Some(key.to_string()), }; if let Some(key) = key { - non_registry.insert(key, "an override names the package".to_string()); + non_registry.insert(key, "project config can redirect it".to_string()); } } } @@ -3639,16 +3640,18 @@ mod tests { } } - /// #1155 provenance guard: an override can swap a registry edge for a - /// git / URL / `file:` spec that npm ci installs instead of the lock's - /// `resolved`. While any override names the vendored package, a version - /// change is not trusted as a registry upgrade and stays drift. + /// #1155 provenance guard: an override can swap a registry edge (alias + /// edges included) for a git / URL / `file:` spec that npm ci installs + /// instead of the lock's `resolved`. While the project declares any + /// override, a version change is not trusted as a registry upgrade and + /// stays drift. #[tokio::test] - async fn revert_keeps_version_change_as_drift_while_an_override_names_the_package() { + async fn revert_keeps_version_change_as_drift_while_any_override_is_declared() { for overrides in [ json!({ "left-pad": "file:../left-pad" }), json!({ "foo": { "left-pad": "github:evil/left-pad" } }), json!({ "left-pad@1.3.0": "https://example.com/left-pad.tgz" }), + json!({ "foo": { "aliased": "git+file:///evil" } }), ] { let fx = fixture().await; let (_, entry, _) = expect_done(fx.vendor(false).await); @@ -3684,6 +3687,42 @@ mod tests { } } + /// #1155 provenance guard: npm rewrites a `registry.npmjs.org` dist URL + /// to the project `.npmrc` registry at fetch time + /// (`replace-registry-host`), so with a registry configured there a + /// version move's recorded host proves nothing. It stays drift. + #[tokio::test] + async fn revert_keeps_version_change_as_drift_while_npmrc_configures_a_registry() { + for npmrc in [ + "registry=https://evil.example.com/\n", + "@s:registry=https://evil.example.com/\n", + "replace-registry-host=always\n", + ] { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + tokio::fs::write(fx.root().join(".npmrc"), npmrc) + .await + .unwrap(); + let upgraded = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = upgraded.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = upgraded; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{npmrc}: {:?}", outcome.error); + assert!(outcome.drift_skipped(), "{npmrc}: {:?}", outcome.warnings); + assert!(outcome.kept_artifact, "{npmrc}: {:?}", outcome.warnings); + } + } + #[test] fn legacy_pointer_maps_to_its_packages_key() { assert_eq!( diff --git a/crates/socket-patch-core/src/vendor/npm_origin.rs b/crates/socket-patch-core/src/vendor/npm_origin.rs index 17badb8ca..410b4439d 100644 --- a/crates/socket-patch-core/src/vendor/npm_origin.rs +++ b/crates/socket-patch-core/src/vendor/npm_origin.rs @@ -89,21 +89,9 @@ impl NpmOverrides { } } - /// True when any override rule, at any nesting depth, names the package - /// `name` (bare or with a selector). Coarser than [`Self::replacement`] - /// on purpose: a caller that must not trust an edge's registry - /// resolution while an override might swap it uses this. - pub(crate) fn mentions(&self, name: &str) -> bool { - fn walk(rules: &Map, name: &str) -> bool { - rules.iter().any(|(key, value)| { - let at = key - .char_indices() - .find_map(|(i, c)| (c == '@' && i > 0).then_some(i)) - .unwrap_or(key.len()); - &key[..at] == name || value.as_object().is_some_and(|nested| walk(nested, name)) - }) - } - walk(&self.rules, name) + /// True when the root `package.json` declares no `overrides` rule. + pub(crate) fn is_empty(&self) -> bool { + self.rules.is_empty() } /// The spec an override makes npm install for the edge `dep_name@spec` From 949593fd6185d7ecf2261af2d619eb81d54f5c43 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 01:05:44 +0000 Subject: [PATCH 10/12] Fail closed on any project or env registry config The upgrade shortcut skipped a project .npmrc that only set a proxy, `strict-ssl=false` or a CA file, since it looked for the words "registry" and "scope". Those settings can also change what npm fetches. Any setting at all in the project .npmrc or bunfig.toml, or a `NPM_CONFIG_REGISTRY` / `npm_config_registry` / `BUN_CONFIG_REGISTRY` in the environment, now keeps a version move as drift. A file of only comments or blank lines still counts as no config. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/npm_common.rs | 34 ++++++++++++++----- .../socket-patch-core/src/vendor/npm_lock.rs | 32 ++++++++++++++++- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index a197e76be..723d550f9 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -64,19 +64,35 @@ pub(super) struct NpmCoords { /// vendor, arbitrary delete on revert) — reject fail-closed before any disk /// access. `Err` carries a ready [`VendorOutcome::Refused`] to bubble /// verbatim. -/// True when the project's own `.npmrc` or `bunfig.toml` could make npm or -/// Bun fetch a registry package from somewhere other than the URL its lock -/// records (a `registry` / `@scope:registry` / `replace-registry-host` -/// line, a Bun scope table), or can't be read. Deliberately coarse: a -/// revert that would delete a vendored copy because a version moved -/// "within the same registry" trusts that move only when this is false -/// (#1155). +/// True when npm or Bun might fetch a registry package from somewhere other +/// than the URL its lock records, as far as the project and the process +/// environment can tell: the project's own `.npmrc` or `bunfig.toml` sets +/// anything at all (a registry, a scope table, a proxy, TLS settings, ...) +/// or can't be read, or `NPM_CONFIG_REGISTRY` / `npm_config_registry` / +/// `BUN_CONFIG_REGISTRY` is set. Deliberately coarse: a revert that would +/// delete a vendored copy because a version moved "within the same +/// registry" trusts that move only when this is false (#1155). User- and +/// machine-level config files (`~/.npmrc`, `~/.bunfig.toml`, the global +/// npmrc) are not read. pub(super) async fn project_may_redirect_registry(project_root: &Path) -> bool { + let env_registry = [ + "NPM_CONFIG_REGISTRY", + "npm_config_registry", + "BUN_CONFIG_REGISTRY", + ] + .iter() + .any(|var| std::env::var_os(var).is_some_and(|v| !v.is_empty())); + if env_registry { + return true; + } for name in [".npmrc", "bunfig.toml"] { match crate::utils::fs::read_regular_to_string(&project_root.join(name)).await { Ok(text) => { - let text = text.to_ascii_lowercase(); - if text.contains("registry") || text.contains("scope") { + let configures_something = text.lines().any(|line| { + let line = line.trim(); + !line.is_empty() && !line.starts_with('#') && !line.starts_with(';') + }); + if configures_something { return true; } } diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index b688c5a2b..5258aa7b2 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -3692,11 +3692,13 @@ mod tests { /// (`replace-registry-host`), so with a registry configured there a /// version move's recorded host proves nothing. It stays drift. #[tokio::test] - async fn revert_keeps_version_change_as_drift_while_npmrc_configures_a_registry() { + async fn revert_keeps_version_change_as_drift_while_a_project_npmrc_sets_anything() { for npmrc in [ "registry=https://evil.example.com/\n", "@s:registry=https://evil.example.com/\n", "replace-registry-host=always\n", + "https-proxy=http://evil.example.com:8080\nstrict-ssl=false\n", + "cafile=./evil-ca.pem\n", ] { let fx = fixture().await; let (_, entry, _) = expect_done(fx.vendor(false).await); @@ -3723,6 +3725,34 @@ mod tests { } } + /// A project `.npmrc` that holds only comments or blank lines changes + /// nothing about where npm fetches, so the upgrade is still trusted. + #[tokio::test] + async fn revert_after_version_change_ignores_a_comment_only_npmrc() { + let fx = fixture().await; + let (_, entry, _) = expect_done(fx.vendor(false).await); + let entry = entry.unwrap(); + tokio::fs::write(fx.root().join(".npmrc"), "# nothing here\n\n; nor here\n") + .await + .unwrap(); + let upgraded = json!({ + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz", + "integrity": "sha512-upgraded==" + }); + let mut live = fx.read_lock().await; + live["packages"]["node_modules/left-pad"] = upgraded.clone(); + live["packages"]["node_modules/foo/node_modules/left-pad"] = upgraded; + tokio::fs::write(fx.lock_path(), serialize_json(&live, " ").unwrap()) + .await + .unwrap(); + + let outcome = revert_npm(&entry, fx.root(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!(!outcome.drift_skipped(), "{:?}", outcome.warnings); + assert!(!outcome.kept_artifact, "{:?}", outcome.warnings); + } + #[test] fn legacy_pointer_maps_to_its_packages_key() { assert_eq!( From 0c50224098ad4bdd1ce362236aaf24c8899bf673 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 15:31:42 +0000 Subject: [PATCH 11/12] Fix the build after merging main's bun.lockb rework #1147 landed a bun.lockb revert that calls bun_lock::revert_one_record for a migrated text record, and main moved npm_lock's npm_origin import. Both broke against this branch in the merge queue (clippy: E0061, E0425). Pass `false` for the new default-registry argument from the bun.lockb path, which keeps a moved default-registry tuple there as drift: bun.lockb stays outside the #1155 upgrade path. Import legacy_packages_key again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Mo7HM9gyRkUAMxWqi62Dxz --- crates/socket-patch-core/src/vendor/bun_binary.rs | 4 ++++ crates/socket-patch-core/src/vendor/npm_lock.rs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vendor/bun_binary.rs b/crates/socket-patch-core/src/vendor/bun_binary.rs index 6137e82e0..3697e7a47 100644 --- a/crates/socket-patch-core/src/vendor/bun_binary.rs +++ b/crates/socket-patch-core/src/vendor/bun_binary.rs @@ -644,10 +644,14 @@ pub(crate) async fn revert(entry: &VendorEntry, root: &Path, opts: RevertOpts) - if let RevertLock::Migrated(lines) = &mut lock { if rec.file == TEXT_LOCK && rec.kind == super::bun_lock::KIND_LOCK_PACKAGE { let mut dirty = false; + // `false`: bun.lockb migrations stay outside the #1155 + // upgrade path, so a moved default-registry tuple keeps + // its drift verdict here. super::bun_lock::revert_one_record( lines, rec, &entry.uuid, + false, &mut dirty, &mut outcome.warnings, ); diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 04db739de..59f11b5c4 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -31,7 +31,9 @@ use super::npm_common::{ guard_revert_uuid_dir, vendor_npm_family, NpmCommit, NpmCoords, NpmLockBackend, NpmStagedPack, NpmVendorRequest, WireCx, }; -use super::npm_origin::{npm_non_registry_entries, npm_shrinkwrapped_entries, NpmOverrides}; +use super::npm_origin::{ + legacy_packages_key, npm_non_registry_entries, npm_shrinkwrapped_entries, NpmOverrides, +}; use super::parse_memo::ParseMemo; use super::path::parse_vendor_path; use super::source::PackageSource; From 7dd9ed13d45a349d27bd7ddfc20c8865167c1771 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 18:02:52 +0000 Subject: [PATCH 12/12] Repin live minimist@1.2.2 suites to republished patch 642d7f02 Port of #1301 (fixes #1293). Production withdrew the free minimist@1.2.2 patch 80630680 and republished the fix as 642d7f02, which turned hosted-e2e, e2e_safety_pnpm and every Bun native leg red here as on main. The vlt harness also now reads the republished patch's unprefixed file keys. Test-only; a no-op once main carries #1301. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Mo7HM9gyRkUAMxWqi62Dxz --- .../tests/e2e_hosted_production.rs | 4 +-- crates/socket-patch-cli/tests/e2e_npm.rs | 6 ++-- .../socket-patch-cli/tests/e2e_safety_pnpm.rs | 6 ++-- .../tests/e2e_vendored_production.rs | 4 +-- docs/testing/bun-compatibility.md | 2 +- scripts/backtest-bun.py | 2 +- scripts/backtest-vlt.py | 4 +-- scripts/tests/test_backtest_harnesses.py | 33 +++++++++++++++++++ 8 files changed, 47 insertions(+), 14 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_hosted_production.rs b/crates/socket-patch-cli/tests/e2e_hosted_production.rs index 6c7ca6f07..f293592a0 100644 --- a/crates/socket-patch-cli/tests/e2e_hosted_production.rs +++ b/crates/socket-patch-cli/tests/e2e_hosted_production.rs @@ -33,7 +33,7 @@ //! //! | Ecosystem | PURL | Patch UUID | Advisory | //! |-----------|------|------------|----------| -//! | npm | `pkg:npm/minimist@1.2.2` | `80630680-4da6-45f9-bba8-b888e0ffd58c` | GHSA-xvch-5gv4-984h (CVE-2021-44906) | +//! | npm | `pkg:npm/minimist@1.2.2` | `642d7f02-ebc1-4ab0-99e2-07f5dd8463cb` | GHSA-xvch-5gv4-984h (CVE-2021-44906) | //! | PyPI | `pkg:pypi/urllib3@1.26.18` | *any of three* (see [`PYPI_UUIDS`]) | GHSA-gm62-xv2j-4w53 &co | //! | gem | `pkg:gem/activestorage@6.0.3` | *any of* [`GEM_UUIDS`] (six today; the sixth merges three advisories) | GHSA-m42x-37p3-fv5w (CVE-2020-8162), GHSA-w749-p3v6-hccq (CVE-2022-21831), GHSA-9xrj-h377-fr87 (CVE-2026-33195), GHSA-r4mg-4433-c7g3 (CVE-2025-24293), GHSA-xr9x-r78c-5hrm (CVE-2026-66066) | //! @@ -123,7 +123,7 @@ const PATCH_HOST: &str = "patch.socket.dev"; const NPM_PURL: &str = "pkg:npm/minimist@1.2.2"; const NPM_NAME: &str = "minimist"; const NPM_VERSION: &str = "1.2.2"; -const NPM_UUID: &str = "80630680-4da6-45f9-bba8-b888e0ffd58c"; +const NPM_UUID: &str = "642d7f02-ebc1-4ab0-99e2-07f5dd8463cb"; const PYPI_PURL: &str = "pkg:pypi/urllib3@1.26.18"; const PYPI_NAME: &str = "urllib3"; diff --git a/crates/socket-patch-cli/tests/e2e_npm.rs b/crates/socket-patch-cli/tests/e2e_npm.rs index 63de6c636..4df29c4d7 100644 --- a/crates/socket-patch-cli/tests/e2e_npm.rs +++ b/crates/socket-patch-cli/tests/e2e_npm.rs @@ -1,7 +1,7 @@ //! End-to-end tests for the npm patch lifecycle. //! //! These tests exercise the full CLI against the real Socket API, using the -//! **minimist@1.2.2** patch (UUID `80630680-4da6-45f9-bba8-b888e0ffd58c`), +//! **minimist@1.2.2** patch (UUID `642d7f02-ebc1-4ab0-99e2-07f5dd8463cb`), //! which fixes CVE-2021-44906 (Prototype Pollution). //! //! # Prerequisites @@ -26,14 +26,14 @@ use common::cache_env; // Constants // --------------------------------------------------------------------------- -const NPM_UUID: &str = "80630680-4da6-45f9-bba8-b888e0ffd58c"; +const NPM_UUID: &str = "642d7f02-ebc1-4ab0-99e2-07f5dd8463cb"; const NPM_PURL: &str = "pkg:npm/minimist@1.2.2"; /// Git SHA-256 of the *unpatched* `index.js` shipped with minimist 1.2.2. const BEFORE_HASH: &str = "311f1e893e6eac502693fad8617dcf5353a043ccc0f7b4ba9fe385e838b67a10"; /// Git SHA-256 of the *patched* `index.js` after the security fix. -const AFTER_HASH: &str = "043f04d19e884aa5f8371428718d2a3f27a0d231afe77a2620ac6312f80aaa28"; +const AFTER_HASH: &str = "ec956dcafb886f14315570bf3981d44aa12c561716abb46eed8b067aaa1f6bdf"; // --------------------------------------------------------------------------- // Helpers diff --git a/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs b/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs index 783e7337a..e70b3511d 100644 --- a/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs +++ b/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs @@ -11,7 +11,7 @@ //! view and the store entry byte-identical. //! //! Fixture: minimist@1.2.2 + its Socket patch (UUID -//! `80630680-4da6-45f9-bba8-b888e0ffd58c`, CVE-2021-44906) — same +//! `642d7f02-ebc1-4ab0-99e2-07f5dd8463cb`, CVE-2021-44906) — same //! pair `e2e_npm.rs` uses, so the BEFORE/AFTER hashes are known. //! //! Network: yes (pnpm install + socket-patch get). Toolchain: pnpm. @@ -24,12 +24,12 @@ mod common; use common::{assert_run_ok, git_sha256_file, has_command, pnpm_run, write_package_json}; -const NPM_UUID: &str = "80630680-4da6-45f9-bba8-b888e0ffd58c"; +const NPM_UUID: &str = "642d7f02-ebc1-4ab0-99e2-07f5dd8463cb"; /// Git-SHA-256 of the *unpatched* `index.js` shipped with minimist 1.2.2. const BEFORE_HASH: &str = "311f1e893e6eac502693fad8617dcf5353a043ccc0f7b4ba9fe385e838b67a10"; /// Git-SHA-256 of the *patched* `index.js` after the security fix. -const AFTER_HASH: &str = "043f04d19e884aa5f8371428718d2a3f27a0d231afe77a2620ac6312f80aaa28"; +const AFTER_HASH: &str = "ec956dcafb886f14315570bf3981d44aa12c561716abb46eed8b067aaa1f6bdf"; // ── Setup helpers ───────────────────────────────────────────────────── diff --git a/crates/socket-patch-cli/tests/e2e_vendored_production.rs b/crates/socket-patch-cli/tests/e2e_vendored_production.rs index 51a34a20f..53f2f2d9c 100644 --- a/crates/socket-patch-cli/tests/e2e_vendored_production.rs +++ b/crates/socket-patch-cli/tests/e2e_vendored_production.rs @@ -49,7 +49,7 @@ //! //! | Ecosystem | PURL | Patch UUID | Marker in the patched bytes | //! |-----------|------|------------|-----------------------------| -//! | npm | `pkg:npm/minimist@1.2.2` | `80630680-4da6-45f9-bba8-b888e0ffd58c` | `Socket Community Patch` header | +//! | npm | `pkg:npm/minimist@1.2.2` | `642d7f02-ebc1-4ab0-99e2-07f5dd8463cb` | `Socket Community Patch` header | //! | PyPI | `pkg:pypi/urllib3@1.26.18` | *any of three* (see [`PYPI_UUIDS`]) | `Socket Community Patch` header | //! | gem | `pkg:gem/activestorage@6.0.3` | *any of* [`GEM_PATCHES`] | `Socket Community Patch` header | //! @@ -137,7 +137,7 @@ const PROXY: &str = "https://patches-api.socket.dev"; const NPM_PURL: &str = "pkg:npm/minimist@1.2.2"; const NPM_NAME: &str = "minimist"; const NPM_VERSION: &str = "1.2.2"; -const NPM_UUID: &str = "80630680-4da6-45f9-bba8-b888e0ffd58c"; +const NPM_UUID: &str = "642d7f02-ebc1-4ab0-99e2-07f5dd8463cb"; const PYPI_PURL: &str = "pkg:pypi/urllib3@1.26.18"; const PYPI_NAME: &str = "urllib3"; diff --git a/docs/testing/bun-compatibility.md b/docs/testing/bun-compatibility.md index 26ef8e5d5..a65b4b96d 100644 --- a/docs/testing/bun-compatibility.md +++ b/docs/testing/bun-compatibility.md @@ -5,7 +5,7 @@ projects using text `bun.lock` or native binary `bun.lockb`. Real-Bun evidence b - **The native matrix** — `scripts/backtest-bun.py` runs real Bun releases against the public free Socket patch for `minimist@1.2.2` - (`80630680-4da6-45f9-bba8-b888e0ffd58c`) with the production CLI and patch + (`642d7f02-ebc1-4ab0-99e2-07f5dd8463cb`) with the production CLI and patch service, without a token or substitute service, and checks the INSTALLED bytes, lock stability, digest rejection and rollback on Linux, macOS and Windows ([workflow](../../.github/workflows/bun-compatibility.yml)). diff --git a/scripts/backtest-bun.py b/scripts/backtest-bun.py index c7f10aa77..71983e544 100644 --- a/scripts/backtest-bun.py +++ b/scripts/backtest-bun.py @@ -117,7 +117,7 @@ # former `vendored-detached` leg collapsed into `vendored`: same footprint. MODES = ['hosted', 'vendored'] PURL = 'pkg:npm/minimist@1.2.2' -UUID = '80630680-4da6-45f9-bba8-b888e0ffd58c' +UUID = '642d7f02-ebc1-4ab0-99e2-07f5dd8463cb' # The registry slot bun writes for a non-default registry: the full tarball URL. REGISTRY_SLOT = 'https://registry.npmjs.org/minimist/-/minimist-1.2.2.tgz' LOCAL_TUPLE_SPEC = f'minimist@.socket/vendor/npm/{UUID}/minimist-1.2.2.tgz' diff --git a/scripts/backtest-vlt.py b/scripts/backtest-vlt.py index 18ed56f9d..fb8533fa2 100644 --- a/scripts/backtest-vlt.py +++ b/scripts/backtest-vlt.py @@ -88,7 +88,7 @@ VERSIONS = ['0.0.0-16', '0.0.0-32', '1.0.0-rc.14', '1.0.0-rc.32', '1.0.4', '1.0.10', '1.2.0'] MODES = ['hosted', 'vendored', 'agent'] PURL = 'pkg:npm/minimist@1.2.2' -UUID = '80630680-4da6-45f9-bba8-b888e0ffd58c' +UUID = '642d7f02-ebc1-4ab0-99e2-07f5dd8463cb' NAME = 'minimist' VERSION = '1.2.2' TARGET = f'{NAME}@{VERSION}' @@ -1069,7 +1069,7 @@ def holds(self, root, lock_text, side): ok = True for copy_dir in self.copies(root, lock_text): for key, hashes in self.record['files'].items(): - path = copy_dir / key.split('/', 1)[1] + path = copy_dir / key.removeprefix('package/') digest = git_hash(path.read_bytes()) if path.is_file() else None details[str(path.relative_to(root))] = digest ok = ok and digest == hashes.get(f'{side}Hash') diff --git a/scripts/tests/test_backtest_harnesses.py b/scripts/tests/test_backtest_harnesses.py index 5bb56de5c..fe9f7e36f 100644 --- a/scripts/tests/test_backtest_harnesses.py +++ b/scripts/tests/test_backtest_harnesses.py @@ -775,6 +775,39 @@ def test_shapes_are_depscan_capture_shapes(self): self.assertLessEqual(set(vlt.SHAPES), capture_names) +class VltInstalledBytesTests(unittest.TestCase): + def test_holds_checks_root_and_nested_files_with_optional_package_prefix(self): + contents = { + 'index.js': {'before': b'original entrypoint', 'after': b'patched entrypoint'}, + 'test/proto.js': {'before': b'original test', 'after': b'patched test'}, + } + for prefix in ('', 'package/'): + for side in ('before', 'after'): + with self.subTest(prefix=prefix, side=side), tempfile.TemporaryDirectory() as temp: + root = Path(temp) + package = root / 'node_modules' / 'minimist' + record = {'files': { + prefix + name: {s + 'Hash': vlt.git_hash(data) for s, data in sides.items()} + for name, sides in contents.items() + }} + cell = vlt.Cell({'out': root, 'record': record}, '1.2.0', 'vendored', 'direct') + expected = {} + for name, sides in contents.items(): + path = package / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(sides[side]) + expected[str(path.relative_to(root))] = vlt.git_hash(sides[side]) + self.assertEqual(cell.holds(root, '{}', side), (True, expected)) + other_side = 'after' if side == 'before' else 'before' + self.assertFalse(cell.holds(root, '{}', other_side)[0]) + + nested = package / 'test' / 'proto.js' + nested.write_bytes(b'corrupted') + self.assertFalse(cell.holds(root, '{}', side)[0]) + nested.unlink() + self.assertFalse(cell.holds(root, '{}', side)[0]) + + class VltConfigTests(unittest.TestCase): """write_vlt_json follows the DESIGN §8.3 per-era registry table."""