Fix npm/Bun VEX attesting a patch a same-lock copy skips (#588)

CHANGELOG.md

@@ -102,6 +102,12 @@ limits, and required install commands.
 
 ### Fixed
 
+- `vex` no longer attests an npm or Bun patch as `not_affected` while a
+  second entry for the same `name@version` in the same lockfile still
+  resolves from the registry (for example a workspace member added after
+  vendoring). That copy installs unpatched, so the patch is now reported
+  as contested. `vendor --check` reports the same lockfile entry as drift
+  (#588).
 - Global mode (`-g`) finds npm, yarn, pnpm, bun, RubyGems and Composer on
   Windows, where they install as `.cmd` / `.bat` shims, instead of reporting
   an empty scan. The yarn and npm-family global lookups no longer run from the

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -376,7 +376,7 @@ Recognition rules that hold for every ecosystem:
 
 * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-<uuid>` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above.
 * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget `<add>`, pom `<repository>`, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `<profile>` is diagnosed, never a reference.
-* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`.
+* **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. Another entry of the **same** npm or Bun lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install`) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy.
 * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-<uuid>`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged).
 
 **Record resolution.** A candidate's record must carry the patch uuid the lockfile actually **wires**. It is taken from the first source that has one: the manifest (matched qualifier-insensitively), the hosted records above (this run's, then a pre-v5 ledger's), then the vendor ledger's embedded records. If none has it and the run is online, `vex` fetches the patch view by uuid from the patch API — for a v5 hosted checkout this is the normal path. The fetch uses `get`'s API client: the public proxy when no token is configured, and a one-shot 401/403 fallback to the proxy (free patches only). At most 10 fetches run concurrently. Fetched records stay in memory: `vex` never writes the manifest. A candidate still has no record under `--offline`, after a transport error or a 404, or when the patch is refused (paid without an entitled token); it is then omitted as `record_unavailable`, and the run is not aborted. A record whose uuid or package disagrees with the wiring is omitted as `record_mismatch`. The informational `socket-patch.vendor.json` marker is never a record source. When the lockfile wires a package to patch U, a manifest or ledger record for that package under another uuid is superseded, and a human-mode `Note:` says so.
@@ -1633,7 +1633,10 @@ See [the JVM design](../../docs/design/maven-vendoring.md) for supported shapes.
 
 `vendor --check` is an offline, read-only audit. Healthy entries emit `verified`
 with `vendor_check_ok`; drift emits `failed` with `vendor_check_failed`, a
-`partialFailure` envelope and exit 1. Missing ledger entries fail with
+`partialFailure` envelope and exit 1. For a package-lock entry, drift includes a
+`package-lock.json` / `npm-shrinkwrap.json` entry for the vendored `name@version`
+that `vendor` would rewire but that does not resolve to the vendored artifact
+(#588); the reason names that entry. Missing ledger entries fail with
 `vendor_ledger_missing`. Offline upstream metadata is reported as the run warning
 `vendor_jvm_upstream_unverified`. The check never starts an API client or writes
 lock/recovery files. `--check` conflicts with `--revert`.

crates/socket-patch-cli/src/commands/vendor.rs

@@ -955,6 +955,11 @@ async fn run_check(args: &VendorArgs) -> i32 {
         if failure.is_none() && vendor::jvm::apply::is_jvm_entry(entry) {
             failure = vendor::jvm::apply::check_entry(root, entry, local_repo.as_deref()).err();
         }
+        if failure.is_none() && entry.ecosystem == "npm" {
+            failure = vendor::npm_flavor::check_npm_wiring(entry, root)
+                .await
+                .err();
+        }
         if vendor::jvm::apply::upstream_unverified(entry) {
             env.warnings.push(RunWarning {code: "vendor_jvm_upstream_unverified".into(), detail: format!("{key}: upstream metadata was accepted offline; run vendor online to verify registry checksums")});
         }

crates/socket-patch-cli/tests/e2e_vex_vendor.rs

@@ -1472,6 +1472,142 @@ fn vendored_npm_patch_with_an_unpatched_bundled_copy_is_not_attested() {
     }
 }
 
+/// REGRESSION (#588): the lock rewires the hoisted `lodash@4.17.21` to the
+/// vendored tarball, but a workspace member added after vendoring (then
+/// `npm install`) put a SECOND `lodash@4.17.21` entry in the same lock
+/// that still resolves from the registry. `npm ci` installs that copy
+/// unpatched, so `vex` must not attest the purl and `vendor --check` must
+/// report the drift (re-running the install cannot heal it). The same lock
+/// without the second entry is the control: it attests and checks clean.
+#[test]
+fn vendored_npm_patch_with_an_unwired_registry_copy_in_the_same_lock() {
+    let purl = "pkg:npm/lodash@4.17.21";
+    let uuid = "0a0a0a0a-2222-4222-8222-0a0a0a0a0a0a";
+    let patched = b"patched npm bytes\n";
+    let after_hash = compute_git_sha256_from_bytes(patched);
+    for (label, second_copy) in [("control", false), ("second registry copy", true)] {
+        let tmp = tempfile::tempdir().expect("create tempdir");
+        let cwd = tmp.path();
+        let rel = format!(".socket/vendor/npm/{uuid}/lodash-4.17.21.tgz");
+        let sha256 = sha256_hex(&write_member_tgz(
+            &cwd.join(&rel),
+            "package/index.js",
+            patched,
+        ));
+        let record = make_record(
+            uuid,
+            "package/index.js",
+            &after_hash,
+            "GHSA-dupe-aaaa",
+            &["CVE-2026-588"],
+        );
+        let wiring = write_matrix_wiring(cwd, "npm", uuid, &rel);
+        if second_copy {
+            let lock_path = cwd.join("package-lock.json");
+            let mut lock: Value =
+                serde_json::from_str(&std::fs::read_to_string(&lock_path).unwrap()).unwrap();
+            let packages = lock["packages"].as_object_mut().unwrap();
+            packages.insert(
+                "packages/b".to_string(),
+                serde_json::json!({ "name": "b", "version": "1.0.0" }),
+            );
+            packages.insert(
+                "node_modules/b".to_string(),
+                serde_json::json!({ "resolved": "packages/b", "link": true }),
+            );
+            packages.insert(
+                "packages/b/node_modules/lodash".to_string(),
+                serde_json::json!({
+                    "version": "4.17.21",
+                    "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
+                    "integrity": "sha512-T1JJR0lOQUw="
+                }),
+            );
+            std::fs::write(&lock_path, lock.to_string()).unwrap();
+        }
+        let mut state = VendorState::new();
+        state.entries.insert(
+            purl.to_string(),
+            detached_matrix_entry("npm", purl, uuid, &rel, sha256, record, wiring),
+        );
+        let dir = cwd.join(".socket/vendor");
+        std::fs::create_dir_all(&dir).unwrap();
+        std::fs::write(
+            dir.join("state.json"),
+            serde_json::to_string_pretty(&state).unwrap(),
+        )
+        .unwrap();
+
+        let vex_path = cwd.join("out.vex.json");
+        let out = cli()
+            .args([
+                "vex",
+                "--cwd",
+                cwd.to_str().unwrap(),
+                "--json",
+                "--output",
+                vex_path.to_str().unwrap(),
+                "--product",
+                "pkg:npm/app@1.0.0",
+            ])
+            .output()
+            .expect("invoke vex");
+        let env: Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
+            panic!(
+                "{label}: vex envelope JSON on stdout ({e}): {}",
+                String::from_utf8_lossy(&out.stdout)
+            )
+        });
+        let check = cli()
+            .args([
+                "vendor",
+                "--check",
+                "--cwd",
+                cwd.to_str().unwrap(),
+                "--json",
+            ])
+            .output()
+            .expect("invoke vendor --check");
+        let check_env: Value = serde_json::from_slice(&check.stdout).unwrap_or_else(|e| {
+            panic!(
+                "{label}: vendor --check envelope JSON on stdout ({e}): {}",
+                String::from_utf8_lossy(&check.stdout)
+            )
+        });
+        if !second_copy {
+            assert!(out.status.success(), "{label}: {env}");
+            let doc: Value =
+                serde_json::from_str(&std::fs::read_to_string(&vex_path).unwrap()).unwrap();
+            assert_eq!(
+                doc["statements"].as_array().unwrap().len(),
+                1,
+                "{label}: {doc}"
+            );
+            assert!(check.status.success(), "{label}: {check_env}");
+            continue;
+        }
+        assert_eq!(out.status.code(), Some(1), "{label}: {env}");
+        assert!(
+            !vex_path.exists(),
+            "{label}: no VEX document may attest the purl: {env}"
+        );
+        assert!(
+            env.to_string().contains("packages/b/node_modules/lodash"),
+            "{label}: the envelope names the unwired copy: {env}"
+        );
+        assert_eq!(check.status.code(), Some(1), "{label}: {check_env}");
+        let event = &check_env["events"][0];
+        assert_eq!(event["errorCode"], "vendor_check_failed", "{check_env}");
+        assert!(
+            event["reason"]
+                .as_str()
+                .is_some_and(|r| r.contains("packages/b/node_modules/lodash")
+                    && r.contains("re-run `socket-patch vendor`")),
+            "{label}: the check names the unwired copy: {check_env}"
+        );
+    }
+}
+
 // ──────────────────────────────────────────────────────────────────────
 // 8. an applied, byte-verified agent-mode patch attests whether or not its
 // ecosystem has an install hook (there is no setup-state filter).

crates/socket-patch-core/src/vendor/lock_inventory/mod.rs

@@ -70,7 +70,7 @@ pub(crate) mod vlt;
 pub(crate) mod wired;
 pub(crate) mod yarn;
 
-pub(crate) use self::npm::{npm_lock_bundled_nodes, npm_lock_nodes, NpmLockNode};
+pub(crate) use self::npm::{npm_lock_bundled_nodes, npm_lock_located_nodes, NpmLockNode};
 #[cfg(test)]
 pub(crate) use self::npm_family::inventory_npm_lock;
 pub(crate) use self::pypi::pipfile_lock_entries;

crates/socket-patch-core/src/vendor/lock_inventory/npm.rs

@@ -44,12 +44,20 @@ const MAX_LEGACY_NPM_DEPTH: usize = 64;
 ///   through nested `dependencies`, `bundled: true` entries skipped (their
 ///   nested trees are still walked).
 pub(crate) fn npm_lock_nodes(doc: &Value) -> Vec<NpmLockNode<'_>> {
-    walk_npm_lock(doc, Bundled::Skip)
+    walk_npm_lock(doc, Bundled::Skip, false)
         .into_iter()
         .map(|(_, node)| node)
         .collect()
 }
 
+/// [`npm_lock_nodes`], each with where the lock puts it (the `packages`
+/// key, or the `>`-joined v1 dependency chain — the
+/// [`npm_lock_bundled_nodes`] spelling), for diagnostics that must name
+/// the entry.
+pub(crate) fn npm_lock_located_nodes(doc: &Value) -> Vec<(String, NpmLockNode<'_>)> {
+    walk_npm_lock(doc, Bundled::Skip, true)
+}
+
 /// The BUNDLED entries of a parsed npm lock, each with where the lock puts
 /// it: `inBundle: true` in `packages` (lockfileVersion 2/3; the location is
 /// the `packages` key), `bundled: true` in the v1 `dependencies` tree (the
@@ -61,7 +69,7 @@ pub(crate) fn npm_lock_nodes(doc: &Value) -> Vec<NpmLockNode<'_>> {
 /// warn `*_bundled_instance_skipped`), and lockfile discovery
 /// (`vex::discover::npm`) weighs it against the rewired entries.
 pub(crate) fn npm_lock_bundled_nodes(doc: &Value) -> Vec<(String, NpmLockNode<'_>)> {
-    walk_npm_lock(doc, Bundled::Only)
+    walk_npm_lock(doc, Bundled::Only, true)
 }
 
 /// Which side of the bundled split [`walk_npm_lock`] returns.
@@ -71,10 +79,11 @@ enum Bundled {
     Only,
 }
 
-/// [`npm_lock_nodes`] / [`npm_lock_bundled_nodes`]: one walk, split on the
-/// bundled flag. Locations are built only for [`Bundled::Only`] (empty
-/// otherwise), so the common walk allocates nothing extra.
-fn walk_npm_lock(doc: &Value, bundled: Bundled) -> Vec<(String, NpmLockNode<'_>)> {
+/// [`npm_lock_nodes`] / [`npm_lock_located_nodes`] /
+/// [`npm_lock_bundled_nodes`]: one walk, split on the bundled flag.
+/// Locations are built only when `locate` is set (empty otherwise), so the
+/// common walk allocates nothing extra.
+fn walk_npm_lock(doc: &Value, bundled: Bundled, locate: bool) -> Vec<(String, NpmLockNode<'_>)> {
     let mut out = Vec::new();
     if let Some(packages) = doc.get("packages").and_then(Value::as_object) {
         for (key, node) in packages {
@@ -85,14 +94,11 @@ fn walk_npm_lock(doc: &Value, bundled: Bundled) -> Vec<(String, NpmLockNode<'_>)
                 continue;
             }
             let name = node.get("name").and_then(Value::as_str).unwrap_or(key_name);
-            let location = match bundled {
-                Bundled::Only => key.clone(),
-                Bundled::Skip => String::new(),
-            };
+            let location = if locate { key.clone() } else { String::new() };
             out.push((location, NpmLockNode::of(name, node)));
         }
     } else if let Some(deps) = doc.get("dependencies").and_then(Value::as_object) {
-        walk_npm_legacy_dependencies(deps, 0, bundled, "", &mut out);
+        walk_npm_legacy_dependencies(deps, 0, bundled, locate, "", &mut out);
     }
     out
 }
@@ -125,23 +131,24 @@ fn walk_npm_legacy_dependencies<'a>(
     deps: &'a serde_json::Map<String, Value>,
     depth: usize,
     bundled: Bundled,
+    locate: bool,
     parent: &str,
     out: &mut Vec<(String, NpmLockNode<'a>)>,
 ) {
     if depth > MAX_LEGACY_NPM_DEPTH {
         return;
     }
     for (name, node) in deps {
-        let location = match bundled {
-            Bundled::Only if parent.is_empty() => name.clone(),
-            Bundled::Only => format!("{parent} > {name}"),
-            Bundled::Skip => String::new(),
+        let location = match locate {
+            true if parent.is_empty() => name.clone(),
+            true => format!("{parent} > {name}"),
+            false => String::new(),
         };
         if npm_flag(node, "bundled") == (bundled == Bundled::Only) {
             out.push((location.clone(), NpmLockNode::of(name, node)));
         }
         if let Some(nested) = node.get("dependencies").and_then(Value::as_object) {
-            walk_npm_legacy_dependencies(nested, depth + 1, bundled, &location, out);
+            walk_npm_legacy_dependencies(nested, depth + 1, bundled, locate, &location, out);
         }
     }
 }

crates/socket-patch-core/src/vendor/npm_flavor.rs

@@ -674,6 +674,18 @@ pub async fn revert_npm_any(
     revert_npm_any_opts(entry, project_root, RevertOpts::new(dry_run)).await
 }
 
+/// `vendor --check`'s wiring audit for an npm-family entry: `Err` (the
+/// human reason) when the lock installs a copy of the entry's
+/// `name@version` the vendored artifact does not reach (#588). Only the
+/// package-lock flavor audits its lock today; the other flavors' wiring is
+/// left to the artifact check and `vex`.
+pub async fn check_npm_wiring(entry: &VendorEntry, project_root: &Path) -> Result<(), String> {
+    match NpmLockFlavor::from_recorded(entry.flavor.as_deref()) {
+        Some(NpmLockFlavor::PackageLock) => npm_lock::check_wiring(entry, project_root).await,
+        _ => Ok(()),
+    }
+}
+
 /// [`revert_npm_any`] with full [`RevertOpts`], threaded through to the
 /// flavor backend that wired the entry.
 pub async fn revert_npm_any_opts(