Fix npm file: copy attested by in-run VEX (#1481)

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

@@ -1613,6 +1613,87 @@ async fn scan_redirect_npm_bundled_copy_is_not_attested_in_run() {
     }
 }
 
+/// REGRESSION (#1481): npm's half of #935. A workspace member depends on a
+/// `file:` copy of the patched `name@version` (a packed tarball, or a
+/// source directory npm links), beside the registry entry the run pins.
+/// npm installs that copy from its spec, so it stays unpatched: the run
+/// keeps pinning the registry copy (the warnings are pinned by the core
+/// rewriter tests), and its in-run `--vex` must not attest the purl.
+#[tokio::test]
+#[serial]
+async fn scan_redirect_npm_file_copy_is_not_attested_in_run() {
+    let registry = format!(
+        r#""node_modules/{NAME}": {{
+      "version": "{VERSION}",
+      "resolved": "https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz",
+      "integrity": "sha512-UPSTREAMupstream=="
+    }}"#
+    );
+    // npm 10's lock for `"{NAME}": "file:../../vendor/{NAME}-{VERSION}.tgz"`.
+    let tarball = format!(
+        r#""packages/b": {{ "version": "1.0.0", "dependencies": {{ "{NAME}": "file:../../vendor/{NAME}-{VERSION}.tgz" }} }},
+    "packages/b/node_modules/{NAME}": {{
+      "version": "{VERSION}",
+      "resolved": "file:vendor/{NAME}-{VERSION}.tgz",
+      "integrity": "sha512-UPSTREAMupstream=="
+    }}"#
+    );
+    // npm 10's lock for `"{NAME}": "file:../../lpdir"`: a link entry and
+    // the directory's own entry, which alone carries the version.
+    let directory = format!(
+        r#""lpdir": {{ "name": "{NAME}", "version": "{VERSION}" }},
+    "packages/b": {{ "version": "1.0.0", "dependencies": {{ "{NAME}": "file:../../lpdir" }} }},
+    "packages/b/node_modules/{NAME}": {{ "resolved": "lpdir", "link": true }}"#
+    );
+    for (shape, copy) in [("tarball", tarball), ("directory", directory)] {
+        let lock = format!(
+            r#"{{
+  "name": "consumer",
+  "version": "0.0.0",
+  "lockfileVersion": 3,
+  "requires": true,
+  "packages": {{
+    "": {{ "name": "consumer", "version": "0.0.0", "workspaces": ["packages/*"], "dependencies": {{ "{NAME}": "{VERSION}" }} }},
+    "node_modules/b": {{ "resolved": "packages/b", "link": true }},
+    {registry},
+    {copy}
+  }}
+}}
+"#
+        );
+        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 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 registry entry is redirected:\n{rewritten}"
+        );
+        // No document is written when no statement survives, so a missing
+        // file is the expected "attests nothing" outcome here.
+        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 file: 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]

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

@@ -1382,6 +1382,29 @@ impl From<NpmLockEntry<'_>> for NpmLockTarget {
     }
 }
 
+/// The `(name, version)` of the source directory an npm `packages` link
+/// entry (at `pointer`) points at: the entry its `resolved` path names,
+/// by that entry's `name` field, else its folder name, which keeps an
+/// `@scope/` parent as npm's `nameFromFolder` does (npm omits `name` when
+/// they agree). `None` when the lock records no version.
+fn npm_link_source_identity(lock: &Value, pointer: &str) -> Option<(String, String)> {
+    let resolved = lock.pointer(pointer)?.get("resolved")?.as_str()?;
+    let source = lock.get("packages")?.get(resolved)?;
+    let version = source.get("version")?.as_str()?;
+    let name = match source.get("name").and_then(Value::as_str) {
+        Some(name) => name.to_string(),
+        None => {
+            let mut segments = resolved.rsplit('/');
+            let base = segments.next().unwrap_or(resolved);
+            match segments.next() {
+                Some(scope) if scope.starts_with('@') => format!("{scope}/{base}"),
+                _ => base.to_string(),
+            }
+        }
+    };
+    Some((name, version.to_string()))
+}
+
 /// Rewrite a single npm lockfile (`package-lock.json` or `npm-shrinkwrap.json`)
 /// in place. Factored out of `rewrite_npm_lock` so co-present locks each get
 /// the identical override rewrite (see the dual-lock note there).
@@ -1409,14 +1432,27 @@ fn rewrite_one_npm_lock(
     // for every dep. Sound because a rewrite only ever touches an entry's
     // `resolved`/`integrity` (never a key, `name`, `version` or flag), so
     // addresses and identities hold.
-    let targets: Vec<NpmLockTarget> = npm_lock_entries(&lock)
+    let mut targets: Vec<NpmLockTarget> = npm_lock_entries(&lock)
         .into_iter()
         .filter(NpmLockEntry::is_dependency)
         .map(NpmLockTarget::from)
         .collect();
     // Entries npm installs from a git / url / `file:` spec: see
     // `vendor::npm_origin` (#326).
     let non_registry = npm_non_registry_entries(&lock, manifest_overrides);
+    // A `file:` directory dependency is a versionless link entry whose
+    // source directory's own entry carries the package (#1481): give the
+    // link that identity, so a copy of a patched release is seen. A plain
+    // workspace-member link is first-party source, never a copy.
+    for target in targets
+        .iter_mut()
+        .filter(|t| t.link && t.version.is_none() && non_registry.contains_key(&t.key))
+    {
+        if let Some((name, version)) = npm_link_source_identity(&lock, &target.pointer) {
+            target.name = name;
+            target.version = Some(version);
+        }
+    }
     // Entries npm 7–11 install from a dependency's own shrinkwrap (#753).
     let shrinkwrapped = npm_shrinkwrapped_entries(&lock);
     // The legacy mirror of an entry the `packages` scan skips (and warns
@@ -1448,11 +1484,19 @@ fn rewrite_one_npm_lock(
             matched_any = true;
             let (kind, edit_key) = match target.section {
                 NpmLockSection::Packages => {
+                    // npm links a workspace member or `file:` directory
+                    // from its source and never fetches `resolved`, so that
+                    // copy keeps the unpatched source bytes. Recorded so the
+                    // in-run `--vex` verifies instead of assuming (#1481).
                     if target.link {
+                        result.bundled_skipped_uuids.insert(dep.patch_uuid.clone());
                         result.warnings.push(RewriteWarning {
                             code: "redirect_npm_link_entry_skipped".into(),
                             detail: format!(
-                                "lock entry `{key}` is a link (npm workspaces/file: dir); skipped"
+                                "lock entry `{key}` is a link (npm workspaces/file: dir) \
+                                 and CANNOT be redirected — npm links it from its source \
+                                 directory, so that copy stays UNPATCHED; depend on the \
+                                 registry release to patch it"
                             ),
                         });
                         continue;
@@ -1500,8 +1544,10 @@ fn rewrite_one_npm_lock(
                     // npm installs a git / url / `file:` dependency from the
                     // dependent's spec and ignores `resolved`, so a rewrite
                     // here would confirm (and VEX-attest) a patch that never
-                    // installs.
+                    // installs. Recorded so the in-run `--vex` verifies
+                    // instead of assuming (#1481).
                     if let Some(reason) = non_registry.get(key) {
+                        result.bundled_skipped_uuids.insert(dep.patch_uuid.clone());
                         result.warnings.push(RewriteWarning {
                             code: "redirect_npm_non_registry_entry_skipped".into(),
                             detail: format!(
@@ -19292,6 +19338,145 @@ mod tests {
         }
     }
 
+    /// #1481: beside the registry entry the rewrite pins, a workspace
+    /// member installs a `file:` copy of the same release, as a tarball or
+    /// as a linked source directory (whose version lives only on the link
+    /// target's entry). npm installs that copy unpatched, so the registry
+    /// entry is still pinned, the copy is warned about, and the uuid is
+    /// recorded so the in-run `--vex` verifies instead of assuming.
+    #[test]
+    fn issue_1481_npm_file_copy_beside_a_pin_is_warned_and_recorded() {
+        let registry = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";
+        let tarball = json!({
+            "packages/b": { "version": "1.0.0", "dependencies": { "left-pad": "file:../../vendor/left-pad-1.3.0.tgz" } },
+            "packages/b/node_modules/left-pad": {
+                "version": "1.3.0",
+                "resolved": "file:vendor/left-pad-1.3.0.tgz",
+                "integrity": "sha512-UPSTREAM=="
+            }
+        });
+        let directory = json!({
+            "lpdir": { "name": "left-pad", "version": "1.3.0" },
+            "packages/b": { "version": "1.0.0", "dependencies": { "left-pad": "file:../../lpdir" } },
+            "packages/b/node_modules/left-pad": { "resolved": "lpdir", "link": true }
+        });
+        for (shape, copy, code) in [
+            (
+                "tarball",
+                tarball,
+                "redirect_npm_non_registry_entry_skipped",
+            ),
+            ("directory", directory, "redirect_npm_link_entry_skipped"),
+        ] {
+            let mut packages = json!({
+                "": { "name": "app", "version": "0.0.0", "workspaces": ["packages/*"] },
+                "node_modules/b": { "resolved": "packages/b", "link": true },
+                "packages/a": { "version": "1.0.0", "dependencies": { "left-pad": "1.3.0" } },
+                "packages/a/node_modules/left-pad": {
+                    "version": "1.3.0",
+                    "resolved": registry,
+                    "integrity": "sha512-UPSTREAM=="
+                }
+            });
+            for (k, v) in copy.as_object().unwrap() {
+                packages[k] = v.clone();
+            }
+            let lock = json!({ "name": "app", "lockfileVersion": 3, "packages": packages });
+            let mut files = BTreeMap::new();
+            files.insert(
+                "package-lock.json".to_string(),
+                serde_json::to_string_pretty(&lock).unwrap(),
+            );
+            let overrides = vec![npm_override(
+                "left-pad",
+                "1.3.0",
+                "http://patch.test/lp.tgz",
+                "sha512-PATCHED==",
+            )];
+            let r = rewrite_registry_redirect(&files, &overrides);
+            let rewritten: Value = serde_json::from_str(&r.files["package-lock.json"])
+                .unwrap_or_else(|e| panic!("{shape}: registry entry pinned: {e}"));
+            assert_eq!(
+                rewritten["packages"]["packages/a/node_modules/left-pad"]["resolved"],
+                "http://patch.test/lp.tgz",
+                "{shape}"
+            );
+            assert_eq!(
+                rewritten["packages"]["packages/b/node_modules/left-pad"],
+                lock["packages"]["packages/b/node_modules/left-pad"],
+                "{shape}: the file: copy is left alone"
+            );
+            let warned = r
+                .warnings
+                .iter()
+                .find(|w| w.code == code)
+                .unwrap_or_else(|| panic!("{shape}: warns {code}: {:?}", r.warnings));
+            assert!(
+                warned.detail.contains("UNPATCHED")
+                    && warned.detail.contains("packages/b/node_modules/left-pad"),
+                "{shape}: {}",
+                warned.detail
+            );
+            assert!(
+                r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+                "{shape}: the in-run VEX must not assume the patch applied"
+            );
+        }
+    }
+
+    /// #1481: a scoped `file:` directory whose source entry has no `name`
+    /// (npm omits it when the folder spells the name) is still matched,
+    /// while a workspace member that shares a patched package's identity
+    /// is first-party source, not a copy.
+    #[test]
+    fn issue_1481_npm_link_identity_scoped_folder_and_workspace_member() {
+        let ovr = npm_override(
+            "pkg",
+            "1.0.0",
+            "http://patch.test/pkg.tgz",
+            "sha512-PATCHED==",
+        );
+        let mut ovr_scoped = ovr.clone();
+        ovr_scoped.namespace = Some("@scope".into());
+        let scoped = json!({
+            "lockfileVersion": 3,
+            "packages": {
+                "": { "name": "app", "dependencies": { "@scope/pkg": "file:vendor/@scope/pkg" } },
+                "vendor/@scope/pkg": { "version": "1.0.0" },
+                "node_modules/@scope/pkg": { "resolved": "vendor/@scope/pkg", "link": true }
+            }
+        });
+        let mut files = BTreeMap::new();
+        files.insert("package-lock.json".to_string(), scoped.to_string());
+        let r = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr_scoped));
+        assert!(r.edits.is_empty(), "{:?}", r.edits);
+        assert_eq!(
+            warning_codes(&r),
+            vec!["redirect_npm_link_entry_skipped"],
+            "{:?}",
+            r.warnings
+        );
+        assert!(r.bundled_skipped_uuids.contains(&ovr_scoped.patch_uuid));
+
+        let workspace = json!({
+            "lockfileVersion": 3,
+            "packages": {
+                "": { "name": "app", "workspaces": ["packages/*"] },
+                "packages/pkg": { "version": "1.0.0" },
+                "node_modules/pkg": { "resolved": "packages/pkg", "link": true }
+            }
+        });
+        files.insert("package-lock.json".to_string(), workspace.to_string());
+        let r = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr));
+        assert_eq!(
+            warning_codes(&r),
+            vec!["redirect_npm_entry_not_found"],
+            "{:?}",
+            r.warnings
+        );
+        assert!(r.bundled_skipped_uuids.is_empty());
+    }
+
     /// #326, lockfileVersion 2: the legacy `dependencies` mirror of a
     /// non-registry `packages` entry is left alone too, even when it stores
     /// the plain version, while the registry copy's mirror is rewired.