Fix npm overrides to a URL treated as registry (#1965)

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

@@ -1416,3 +1416,217 @@ async fn npm_agent_keeps_crlf_shebang_of_bin_npm_did_not_link() {
         );
     }
 }
+
+/// #1965: a local `foo` depends on left-pad by a registry range, and the
+/// project's `overrides` replace it with a remote tarball url. npm records
+/// that copy with a registry-looking `resolved`, but installs it from the
+/// override spec, so a hosted pin would never install. The hosted scan
+/// must skip it loudly (`redirect_npm_non_registry_entry_skipped`), leave
+/// the lock byte-identical, and attest nothing in the in-run VEX.
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "wall-bound real-npm install; runs on the e2e CI matrix leg"]
+async fn npm_redirect_skips_a_registry_edge_overridden_to_a_url() {
+    let suite = "e2e_redirect_npm_build (overridden-to-url)";
+    if npm_e2e_common::npm_major().is_none() {
+        npm_e2e_common::skip(suite, "`npm` not installed");
+        return;
+    }
+    if !npm_supports_overrides() {
+        println!("N/A {suite}: this npm predates `overrides` (npm 8.3)");
+        return;
+    }
+    let tmp = tempfile::tempdir().unwrap();
+    let proj = tmp.path().join("proj");
+    let foo = tmp.path().join("foo");
+    let cache = tmp.path().join("npm-cache");
+    std::fs::create_dir_all(&proj).unwrap();
+    std::fs::create_dir_all(&foo).unwrap();
+    std::fs::write(
+        foo.join("package.json"),
+        format!(
+            r#"{{"name":"foo","version":"1.0.0","dependencies":{{"{DEP}":"^{DEP_VERSION}"}}}}"#
+        ),
+    )
+    .unwrap();
+    std::fs::write(
+        foo.join("index.js"),
+        format!("module.exports = require('{DEP}');\n"),
+    )
+    .unwrap();
+    let packed = npm_e2e_common::npm(&foo, &["pack", "--cache", cache.to_str().unwrap()]);
+    assert!(
+        packed.status.success(),
+        "npm pack foo: {}",
+        npm_e2e_common::output_text(&packed)
+    );
+    std::fs::rename(foo.join("foo-1.0.0.tgz"), proj.join("foo-1.0.0.tgz")).unwrap();
+    let tarball = format!("https://registry.npmjs.org/{DEP}/-/{DEP}-{DEP_VERSION}.tgz");
+    std::fs::write(
+        proj.join("package.json"),
+        format!(
+            r#"{{"name":"app","version":"1.0.0","private":true,"dependencies":{{"foo":"file:foo-1.0.0.tgz"}},"overrides":{{"{DEP}":"{tarball}"}}}}"#
+        ),
+    )
+    .unwrap();
+    // npm >= 12 refuses a url override unless remote installs are allowed.
+    std::fs::write(proj.join(".npmrc"), "allow-remote=all\n").unwrap();
+    let out = npm_e2e_common::npm(
+        &proj,
+        &[
+            "install",
+            "--no-audit",
+            "--no-fund",
+            "--cache",
+            cache.to_str().unwrap(),
+        ],
+    );
+    if !out.status.success() {
+        npm_e2e_common::skip(
+            suite,
+            &format!(
+                "`npm install` of the overrides fixture failed (registry unreachable?):\n{}",
+                npm_e2e_common::output_text(&out)
+            ),
+        );
+        return;
+    }
+    // The #1965 premise: the copy's `resolved` looks like a registry entry,
+    // and `foo`'s entry keeps its registry range.
+    let lock_before = std::fs::read(proj.join("package-lock.json")).unwrap();
+    let lock: serde_json::Value = serde_json::from_slice(&lock_before).unwrap();
+    assert_eq!(
+        lock["packages"][format!("node_modules/{DEP}")]["resolved"],
+        tarball.as_str(),
+        "{lock}"
+    );
+    assert_eq!(
+        lock["packages"]["node_modules/foo"]["dependencies"][DEP],
+        format!("^{DEP_VERSION}"),
+        "{lock}"
+    );
+
+    let server = MockServer::start().await;
+    let hosted_url = format!(
+        "{}/patch/npm/{DEP}/{DEP_VERSION}/{TOKEN}/{UUID}/{DEP}-{DEP_VERSION}.tgz",
+        server.uri()
+    );
+    Mock::given(method("POST"))
+        .and(path(format!("/v0/orgs/{ORG}/patches/batch")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "packages": [{
+                "purl": PURL,
+                "patches": [{
+                    "uuid": UUID, "purl": PURL, "tier": "free",
+                    "cveIds": [], "ghsaIds": [], "severity": "high",
+                    "title": "overridden-to-url fixture"
+                }]
+            }],
+            "canAccessPaidPatches": false,
+        })))
+        .mount(&server)
+        .await;
+    Mock::given(method("GET"))
+        .and(path_regex(format!(
+            "^/v0/orgs/{ORG}/patches/by-package/.+$"
+        )))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "patches": [{
+                "uuid": UUID, "purl": PURL,
+                "publishedAt": "2026-01-01T00:00:00Z",
+                "description": "x", "license": "MIT", "tier": "free",
+                "vulnerabilities": {}
+            }],
+            "canAccessPaidPatches": false,
+        })))
+        .mount(&server)
+        .await;
+    Mock::given(method("POST"))
+        .and(path(format!("/v0/orgs/{ORG}/patches/package")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "results": {
+                UUID: {
+                    "status": "granted",
+                    "url": hosted_url,
+                    "purl": PURL,
+                    "artifacts": [{
+                        "kind": "tarball",
+                        "url": hosted_url,
+                        "integrity": { "sha512": "sha512-UEFUQ0hFRA==" }
+                    }],
+                    "registryOverride": null
+                }
+            }
+        })))
+        .mount(&server)
+        .await;
+    Mock::given(method("GET"))
+        .and(path(format!("/v0/orgs/{ORG}/patches/view/{UUID}")))
+        .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
+            "uuid": UUID,
+            "purl": PURL,
+            "publishedAt": "2026-01-01T00:00:00Z",
+            "files": {},
+            "vulnerabilities": {
+                GHSA: {
+                    "cves": ["CVE-2026-1111"],
+                    "summary": "overridden-to-url vuln",
+                    "severity": "high",
+                    "description": "d"
+                }
+            },
+            "description": "x", "license": "MIT", "tier": "free"
+        })))
+        .mount(&server)
+        .await;
+
+    let uri = server.uri();
+    let argv = [
+        "scan",
+        "--mode=hosted",
+        "--json",
+        "--yes",
+        "--cwd",
+        proj.to_str().unwrap(),
+        "--api-url",
+        &uri,
+        "--org",
+        ORG,
+        "--api-token",
+        "fake",
+        "--vex",
+        "out.vex.json",
+        "--vex-product",
+        PRODUCT,
+    ];
+    let (_code, stdout, stderr) = run_socket(&proj, &argv);
+    let env: serde_json::Value = serde_json::from_str(&stdout)
+        .unwrap_or_else(|e| panic!("not JSON ({e}):\n{stdout}\nstderr:\n{stderr}"));
+    let skipped = env["warnings"]
+        .as_array()
+        .and_then(|w| {
+            w.iter()
+                .find(|w| w["code"] == "redirect_npm_non_registry_entry_skipped")
+        })
+        .unwrap_or_else(|| panic!("the overridden copy must be skipped loudly: {env}"));
+    assert!(
+        skipped["detail"]
+            .as_str()
+            .is_some_and(|d| d.contains("overrides")),
+        "the skip names the override: {skipped}"
+    );
+    assert_eq!(
+        std::fs::read(proj.join("package-lock.json")).unwrap(),
+        lock_before,
+        "the lock is left byte-identical: {env}"
+    );
+    let vex_path = proj.join("out.vex.json");
+    if vex_path.exists() {
+        let vex: serde_json::Value =
+            serde_json::from_slice(&std::fs::read(&vex_path).unwrap()).unwrap();
+        let not_affected = vex["statements"].as_array().is_some_and(|s| {
+            s.iter()
+                .any(|s| s["status"].as_str() == Some("not_affected"))
+        });
+        assert!(!not_affected, "nothing is attested not_affected: {vex}");
+    }
+}

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

@@ -20871,6 +20871,87 @@ mod tests {
         assert!(!r.files.contains_key("package.json"));
     }
 
+    /// #1965: `foo` depends on left-pad by a registry range and the root
+    /// `overrides` swap it for a remote tarball url, so npm installs that
+    /// url whatever the lock's (registry-looking) `resolved` says. The
+    /// rewriter must skip the entry loudly and keep it out of the VEX.
+    #[test]
+    fn issue_1965_a_registry_edge_overridden_to_a_url_is_unredirected() {
+        let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";
+        let lock = json!({
+            "name": "app",
+            "lockfileVersion": 3,
+            "packages": {
+                "": { "name": "app", "version": "0.0.0", "dependencies": { "foo": "file:foo-1.0.0.tgz" } },
+                "node_modules/foo": {
+                    "version": "1.0.0",
+                    "resolved": "file:foo-1.0.0.tgz",
+                    "dependencies": { "left-pad": "^1.3.0" }
+                },
+                "node_modules/left-pad": {
+                    "version": "1.3.0",
+                    "resolved": url,
+                    "integrity": "sha512-UPSTREAM=="
+                }
+            }
+        });
+        let overrides = vec![npm_override(
+            "left-pad",
+            "1.3.0",
+            "http://patch.test/lp.tgz",
+            "sha512-PATCHED==",
+        )];
+        let files_with = |manifest_overrides: Value| {
+            let mut files = BTreeMap::new();
+            files.insert(
+                "package-lock.json".to_string(),
+                serde_json::to_string_pretty(&lock).unwrap(),
+            );
+            files.insert(
+                "package.json".to_string(),
+                json!({
+                    "name": "app",
+                    "dependencies": { "foo": "file:foo-1.0.0.tgz" },
+                    "overrides": manifest_overrides
+                })
+                .to_string(),
+            );
+            files
+        };
+        for manifest_overrides in [
+            json!({ "left-pad": url }),
+            json!({ "foo": { "left-pad": url } }),
+            json!({ "left-pad": "github:someone/left-pad" }),
+        ] {
+            let r = rewrite_registry_redirect(&files_with(manifest_overrides.clone()), &overrides);
+            assert!(r.files.is_empty(), "{manifest_overrides}: {:?}", r.edits);
+            let skipped = r
+                .warnings
+                .iter()
+                .find(|w| w.code == "redirect_npm_non_registry_entry_skipped")
+                .unwrap_or_else(|| panic!("{manifest_overrides}: {:?}", r.warnings));
+            assert!(
+                skipped.detail.contains("overrides"),
+                "the warning names the override: {}",
+                skipped.detail
+            );
+            assert!(
+                r.bundled_skipped_uuids.contains(&overrides[0].patch_uuid),
+                "{manifest_overrides}: the skipped copy keeps the patch out of the in-run VEX"
+            );
+        }
+        // Control: a registry override (or none) still redirects.
+        for manifest_overrides in [json!({}), json!({ "left-pad": "1.3.0" })] {
+            let r = rewrite_registry_redirect(&files_with(manifest_overrides.clone()), &overrides);
+            let rewritten: Value = serde_json::from_str(&r.files["package-lock.json"]).unwrap();
+            assert_eq!(
+                rewritten["packages"]["node_modules/left-pad"]["resolved"],
+                "http://patch.test/lp.tgz",
+                "{manifest_overrides}"
+            );
+        }
+    }
+
     #[test]
     fn npm_non_registry_entries_are_skipped_with_loud_warning() {
         let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";

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

@@ -2696,6 +2696,56 @@ mod tests {
         );
     }
 
+    /// #1965: a registry range the root `overrides` swap for a remote
+    /// tarball url is installed from that url by `npm ci`, so vendoring
+    /// the copy would report `applied` over unpatched bytes. With no other
+    /// copy the vendor refuses and writes nothing.
+    #[tokio::test]
+    async fn issue_1965_a_registry_edge_overridden_to_a_url_is_refused() {
+        let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";
+        let lock = json!({
+            "name": "fixture",
+            "version": "1.0.0",
+            "lockfileVersion": 3,
+            "packages": {
+                "": { "name": "fixture", "version": "1.0.0",
+                      "dependencies": { "foo": "file:foo-1.0.0.tgz" } },
+                "node_modules/foo": {
+                    "version": "1.0.0",
+                    "resolved": "file:foo-1.0.0.tgz",
+                    "dependencies": { "left-pad": "^1.3.0" }
+                },
+                "node_modules/left-pad": {
+                    "version": "1.3.0",
+                    "resolved": url,
+                    "integrity": "sha512-orig=="
+                }
+            }
+        });
+        let fx = fixture_with("left-pad", "1.3.0", lock.clone()).await;
+        tokio::fs::write(
+            fx.root().join("package.json"),
+            format!(
+                r#"{{"name":"fixture","version":"1.0.0","dependencies":{{"foo":"file:foo-1.0.0.tgz"}},"overrides":{{"left-pad":"{url}"}}}}"#
+            ),
+        )
+        .await
+        .unwrap();
+        expect_refused(fx.vendor(false).await, "vendor_lock_entry_not_rewritable");
+        assert_eq!(fx.read_lock().await, lock, "nothing is written");
+
+        // Control: a registry override is vendored.
+        tokio::fs::write(
+            fx.root().join("package.json"),
+            r#"{"name":"fixture","version":"1.0.0","dependencies":{"foo":"file:foo-1.0.0.tgz"},"overrides":{"left-pad":"1.3.0"}}"#,
+        )
+        .await
+        .unwrap();
+        let (result, entry, _warnings) = expect_done(fx.vendor(false).await);
+        assert!(result.success, "{:?}", result.error);
+        assert!(entry.is_some());
+    }
+
     #[tokio::test]
     async fn non_registry_only_instances_refuse_and_write_nothing() {
         let url = "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz";