Fix in-run hosted VEX attesting npm bundled copies (#325)

CHANGELOG.md

@@ -102,6 +102,14 @@ limits, and required install commands.
 
 ### Fixed
 
+- `scan --vex` in hosted mode no longer attests an npm patch as
+  `not_affected` when `package-lock.json` also lists a bundled copy of the
+  same `name@version` (`inBundle`, or `bundled` in a v1 lock). npm unpacks
+  that copy from its parent's tarball, so it stays unpatched; the run
+  already warned `redirect_npm_bundled_instance_skipped` and now leaves the
+  patch out of its attestation, like a standalone `vex` run (#325). When a
+  `packages` map exists, stale bundled flags in the legacy `dependencies`
+  mirror do not suppress an attestation for the actual install tree.
 - 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/tests/in_process_redirect.rs

@@ -1179,6 +1179,178 @@ async fn scan_redirect_bun_bundled_copy_is_not_attested_in_run() {
     );
 }
 
+/// REGRESSION (#325): npm's half of #469. The hoisted entry is redirected,
+/// but a parent also bundles the same `name@version` (`inBundle: true` in
+/// `packages`, or the legacy `bundled: true` spelling in a v1
+/// `dependencies` tree). npm unpacks that copy from the parent's tarball,
+/// so it stays unpatched; the run warns
+/// `redirect_npm_bundled_instance_skipped`, and its in-run `--vex` must
+/// not attest the purl either.
+#[tokio::test]
+#[serial]
+async fn scan_redirect_npm_bundled_copy_is_not_attested_in_run() {
+    let packages_lock = format!(
+        r#"{{
+  "name": "consumer",
+  "version": "0.0.0",
+  "lockfileVersion": 3,
+  "requires": true,
+  "packages": {{
+    "": {{ "name": "consumer", "version": "0.0.0", "dependencies": {{ "{NAME}": "{VERSION}", "parent": "2.0.0" }} }},
+    "node_modules/{NAME}": {{
+      "version": "{VERSION}",
+      "resolved": "https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz",
+      "integrity": "sha512-UPSTREAMupstream=="
+    }},
+    "node_modules/parent": {{
+      "version": "2.0.0",
+      "resolved": "https://registry.npmjs.org/parent/-/parent-2.0.0.tgz",
+      "integrity": "sha512-PARENT=="
+    }},
+    "node_modules/parent/node_modules/{NAME}": {{
+      "version": "{VERSION}",
+      "inBundle": true,
+      "integrity": "sha512-UPSTREAMupstream=="
+    }}
+  }}
+}}
+"#
+    );
+    let legacy_lock = format!(
+        r#"{{
+  "name": "consumer",
+  "version": "0.0.0",
+  "lockfileVersion": 1,
+  "requires": true,
+  "dependencies": {{
+    "{NAME}": {{
+      "version": "{VERSION}",
+      "resolved": "https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz",
+      "integrity": "sha512-UPSTREAMupstream=="
+    }},
+    "parent": {{
+      "version": "2.0.0",
+      "resolved": "https://registry.npmjs.org/parent/-/parent-2.0.0.tgz",
+      "integrity": "sha512-PARENT==",
+      "dependencies": {{
+        "{NAME}": {{
+          "version": "{VERSION}",
+          "bundled": true
+        }}
+      }}
+    }}
+  }}
+}}
+"#
+    );
+    for (shape, lock) in [("inBundle", packages_lock), ("legacy bundled", legacy_lock)] {
+        let server = MockServer::start().await;
+        mock_discovery(&server).await;
+        mock_reference(&server).await;
+        mock_view(&server).await;
+
+        let tmp = tempfile::tempdir().unwrap();
+        write_project(tmp.path());
+        std::fs::write(tmp.path().join("package-lock.json"), &lock).unwrap();
+        let copy = tmp
+            .path()
+            .join("node_modules/parent/node_modules")
+            .join(NAME);
+        std::fs::create_dir_all(&copy).unwrap();
+        std::fs::write(
+            copy.join("package.json"),
+            format!(r#"{{ "name": "{NAME}", "version": "{VERSION}" }}"#),
+        )
+        .unwrap();
+        std::fs::write(
+            tmp.path().join("node_modules/parent/package.json"),
+            format!(
+                r#"{{ "name": "parent", "version": "2.0.0", "bundleDependencies": ["{NAME}"] }}"#
+            ),
+        )
+        .unwrap();
+
+        let out = tmp.path().join("out.vex.json");
+        let mut args = redirect_args(tmp.path(), server.uri());
+        args.vex.vex = Some(out.clone());
+        args.vex.vex_product = Some("pkg:npm/consumer@0.0.0".into());
+        let _ = run(args).await;
+
+        let rewritten = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap();
+        assert!(
+            rewritten.contains(HOSTED_URL),
+            "{shape}: the regular entry is redirected:\n{rewritten}"
+        );
+        let attested = std::fs::read_to_string(&out)
+            .ok()
+            .and_then(|text| serde_json::from_str::<serde_json::Value>(&text).ok())
+            .is_some_and(|doc| doc.to_string().contains(PURL));
+        assert!(
+            !attested,
+            "{shape}: in-run VEX must not attest a purl whose bundled copy stays unpatched"
+        );
+    }
+}
+
+/// npm 7+ installs from the v2 `packages` map. An ignored legacy bundled
+/// flag must not block the hosted in-run attestation before that install.
+#[tokio::test]
+#[serial]
+async fn scan_redirect_npm_stale_legacy_bundle_mirror_still_attests() {
+    for lockfile in ["package-lock.json", "npm-shrinkwrap.json"] {
+        let server = MockServer::start().await;
+        mock_discovery(&server).await;
+        mock_reference(&server).await;
+        mock_view(&server).await;
+        let tmp = tempfile::tempdir().unwrap();
+        write_project(tmp.path());
+        std::fs::write(
+            tmp.path().join("node_modules").join(NAME).join("index.js"),
+            b"upstream bytes before the next install\n",
+        )
+        .unwrap();
+        let original = tmp.path().join("package-lock.json");
+        let mut lock: serde_json::Value =
+            serde_json::from_slice(&std::fs::read(&original).unwrap()).unwrap();
+        lock["lockfileVersion"] = serde_json::json!(2);
+        lock["dependencies"] = serde_json::json!({
+            NAME: {
+                "version": VERSION,
+                "resolved": format!("https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz"),
+                "integrity": "sha512-UPSTREAMupstream==",
+                "bundled": true
+            }
+        });
+        std::fs::write(&original, serde_json::to_vec_pretty(&lock).unwrap()).unwrap();
+        if lockfile != "package-lock.json" {
+            std::fs::rename(&original, tmp.path().join(lockfile)).unwrap();
+        }
+        let out = tmp.path().join("out.vex.json");
+        let mut args = redirect_args(tmp.path(), server.uri());
+        args.vex.vex = Some(out.clone());
+        args.vex.vex_product = Some("pkg:npm/consumer@0.0.0".into());
+        let exit = run(args).await;
+        assert_eq!(
+            exit, 0,
+            "{lockfile}: npm consumes the normal packages entry"
+        );
+        let document: serde_json::Value =
+            serde_json::from_slice(&std::fs::read(&out).unwrap()).unwrap();
+        assert!(
+            document.to_string().contains(PURL),
+            "{lockfile}: {document:#}"
+        );
+        assert_eq!(document["statements"][0]["status"], "not_affected");
+        let rewritten: serde_json::Value =
+            serde_json::from_slice(&std::fs::read(tmp.path().join(lockfile)).unwrap()).unwrap();
+        assert_eq!(
+            rewritten["packages"][format!("node_modules/{NAME}")]["resolved"],
+            HOSTED_URL
+        );
+        assert_eq!(rewritten["dependencies"][NAME]["bundled"], true);
+    }
+}
+
 /// The bun 1.4 leg: `"lockfileVersion": 2` is the SAME emitted grammar as 1
 /// (bun 1.4 bumped the integer to gate stricter parse checks — oven-sh/bun
 /// PR #31539 — same-fixture locks are byte-identical except the integer), so

crates/socket-patch-core/src/patch/redirect/mod.rs

@@ -932,6 +932,10 @@ fn rewrite_one_npm_lock(
     // Entries npm installs from a git / url / `file:` spec: see
     // `vendor::npm_origin` (#326).
     let non_registry = npm_non_registry_entries(&lock, manifest_overrides);
+    // npm 7+ reads `packages` when it exists; the legacy `dependencies`
+    // mirror must not suppress an attestation for that install tree.
+    // Match the shared npm lock inventory's object-valued-map precedence.
+    let legacy_is_install_tree = lock.get("packages").and_then(Value::as_object).is_none();
     let mut changed = false;
     for dep in npm {
         let fname = full_name(dep);
@@ -966,9 +970,12 @@ fn rewrite_one_npm_lock(
                 // here would put the hosted URL in the lockfile (confirming
                 // and VEX-attesting the patch) while the unpatched bundled
                 // bytes keep installing. Mirrors the vendored backend's
-                // `vendor_bundled_instance_skipped` refusal.
+                // `vendor_bundled_instance_skipped` refusal. The uuid is
+                // recorded so the in-run `--vex` verifies instead of
+                // assuming the patch applied (#325, as Bun's #469).
                 if entry.get("inBundle").and_then(Value::as_bool) == Some(true) {
                     matched_any = true;
+                    result.bundled_skipped_uuids.insert(dep.patch_uuid.clone());
                     result.warnings.push(RewriteWarning {
                         code: "redirect_npm_bundled_instance_skipped".into(),
                         detail: format!(
@@ -1019,6 +1026,7 @@ fn rewrite_one_npm_lock(
                 dep,
                 &sha512,
                 lockfile,
+                legacy_is_install_tree,
                 result,
                 &mut matched_any,
             ) || changed;
@@ -1100,6 +1108,7 @@ fn rewrite_npm_v2_deps(
     dep: &DepOverride,
     sha512: &str,
     lockfile: &str,
+    legacy_is_install_tree: bool,
     result: &mut RewriteResult,
     matched_any: &mut bool,
 ) -> bool {
@@ -1113,6 +1122,9 @@ fn rewrite_npm_v2_deps(
             // fail-open as the `packages` guard above.
             if entry.get("bundled").and_then(Value::as_bool) == Some(true) {
                 *matched_any = true;
+                if legacy_is_install_tree {
+                    result.bundled_skipped_uuids.insert(dep.patch_uuid.clone());
+                }
                 result.warnings.push(RewriteWarning {
                     code: "redirect_npm_bundled_instance_skipped".into(),
                     detail: format!(
@@ -1146,6 +1158,7 @@ fn rewrite_npm_v2_deps(
                 dep,
                 sha512,
                 lockfile,
+                legacy_is_install_tree,
                 result,
                 matched_any,
             ) || changed;
@@ -13643,6 +13656,10 @@ mod tests {
             "a bundled skip is a MATCH — not-found must stay quiet: {:?}",
             r.warnings
         );
+        assert!(
+            r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+            "#325: the skipped bundled copy must keep the patch out of the in-run VEX"
+        );
     }
 
     /// #326: npm installs a git, remote-tarball or `file:` dependency from
@@ -14007,6 +14024,10 @@ mod tests {
             "partial coverage must be surfaced: {:?}",
             r.warnings
         );
+        assert!(
+            r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+            "#325: a redirected sibling must not let the in-run VEX attest the patch"
+        );
     }
 
     /// The v1/v2 legacy `dependencies` tree spells the bundled flag
@@ -14054,6 +14075,62 @@ mod tests {
             "legacy bundled skip must warn: {:?}",
             r.warnings
         );
+        assert!(
+            r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+            "#325: the legacy bundled skip must be recorded like `inBundle`"
+        );
+    }
+
+    /// A stale v2 legacy mirror is not the install tree. Its bundled flag
+    /// must not suppress in-run VEX for a normal `packages` entry; genuine
+    /// bundled entries in `packages` still suppress the same patch.
+    #[test]
+    fn npm_stale_legacy_bundled_mirror_does_not_contest_packages() {
+        for nested in [false, true] {
+            for actual_bundle in [false, true] {
+                let bundled = json!({"version": "1.3.0", "bundled": true});
+                let legacy = if nested {
+                    json!({"parent": {"version": "2.0.0", "dependencies": {"left-pad": bundled}}})
+                } else {
+                    json!({"left-pad": bundled})
+                };
+                let mut lock = json!({
+                    "lockfileVersion": 2,
+                    "packages": {
+                        "": {"name": "app", "version": "1.0.0"},
+                        "node_modules/left-pad": {
+                            "version": "1.3.0",
+                            "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz",
+                            "integrity": "sha512-UPSTREAM=="
+                        }
+                    },
+                    "dependencies": legacy
+                });
+                if actual_bundle {
+                    lock["packages"]["node_modules/parent/node_modules/left-pad"] =
+                        json!({"version": "1.3.0", "inBundle": true});
+                }
+                let files = BTreeMap::from([("package-lock.json".into(), lock.to_string())]);
+                let overrides = vec![npm_override(
+                    "left-pad",
+                    "1.3.0",
+                    "http://patch.test/lp.tgz",
+                    "sha512-PATCHED==",
+                )];
+                let r = rewrite_registry_redirect(&files, &overrides);
+                assert_eq!(
+                    r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+                    actual_bundle,
+                    "nested mirror={nested}, actual bundled install={actual_bundle}"
+                );
+                let out: Value = serde_json::from_str(&r.files["package-lock.json"]).unwrap();
+                assert_eq!(
+                    out["packages"]["node_modules/left-pad"]["resolved"],
+                    "http://patch.test/lp.tgz"
+                );
+                assert_eq!(out["dependencies"], lock["dependencies"]);
+            }
+        }
     }
 
     /// An alias install (`npm i my-alias@npm:left-pad@1.3.0`) keys the lock