Fix npm rewiring git/URL/file lock entries (#326) (#345)

* Start fix for #326

Assisted-by: Claude Code:claude-opus-5-5

* Skip npm lock entries installed from git or URLs

npm installs a git, remote-tarball or file: dependency from the
dependent's spec and ignores the lock entry's resolved. Hosted and
vendored mode rewired those entries anyway, so the scan reported the
package patched and vex attested it while npm ci installed the
original bytes.

Both rewriters now skip such entries with a loud stays-UNPATCHED
warning (vendored refuses when no registry copy is left), and vex no
longer attests a name@version while a non-registry copy of it is in
the lock.

Fixes #326

Assisted-by: Claude Code:claude-opus-5-5

* Document the npm non-registry entry skip

Assisted-by: Claude Code:claude-opus-5-5

* Let the npm 12 e2e install a URL dependency

npm 12 refuses remote-tarball specs unless allow-remote permits them,
so the new vendored e2e now passes --allow-remote=all there. The VEX
test messages and the new diagnostic no longer print patch URLs or
uuids.

Assisted-by: Claude Code:claude-opus-5-5

* Skip the v2 legacy mirror of non-registry npm entries

The hosted rewriter and the vendored backend skipped a git, URL or
file: packages entry, but still rewired its lockfileVersion 2 legacy
dependencies mirror when that mirror stored the plain version. Each
legacy node now maps to the packages key it mirrors, and a node whose
twin is non-registry is left alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LycgFGuki2BqZ25VNhwxJ5

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Mikola Lysenko <mik@socket.dev>

Author: 3 people

CHANGELOG.md

@@ -102,6 +102,17 @@ limits, and required install commands.
 
 ### Fixed
 
+- **npm dependencies installed from git, a URL or `file:` are no longer
+  reported patched.** npm installs such a dependency from the dependent's
+  spec (`github:user/repo`, `https://…/x.tgz`, `file:…`) and ignores the
+  lock entry's `resolved`, so `npm ci` kept installing the original bytes
+  after `scan --mode hosted` or `vendor` rewired the entry and `vex`
+  attested it. Both modes now skip such an entry with a loud
+  stays-UNPATCHED warning (`redirect_npm_non_registry_entry_skipped` /
+  `vendor_non_registry_entry_skipped`; vendoring refuses with
+  `vendor_lock_entry_not_rewritable` when no registry copy is left), and
+  `vex` attests nothing for a `name@version` while such a copy is in the
+  lock (#326).
 - **Agent mode finds Poetry's virtualenv in more setups.** Three cases
   missed the virtualenv Poetry installed into. Each fell back to the
   wrong interpreter, skipped the patch as `package_not_installed` and

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -356,7 +356,7 @@ Discovery is read-only, never touches the network, and never fails the run: a ma
 
 | Ecosystem | Files read | Hosted reference | Vendored reference | Hosted pin (`integrity_required`) |
 |---|---|---|---|---|
-| npm | `package-lock.json` and `npm-shrinkwrap.json` (both when both exist) | `resolved` on the patch host (`packages` in v2/v3; `dependencies` only in v1; `link` / `inBundle` / `bundled` entries skipped) | `resolved: file:.socket/vendor/npm/<uuid>/<name>-<ver>.tgz` | `integrity`, required |
+| npm | `package-lock.json` and `npm-shrinkwrap.json` (both when both exist) | `resolved` on the patch host (`packages` in v2/v3; `dependencies` only in v1; `link` / `inBundle` / `bundled` entries skipped, and so is any entry npm installs from a git, URL or `file:` spec, together with every ref for the same `name@version`) | `resolved: file:.socket/vendor/npm/<uuid>/<name>-<ver>.tgz` | `integrity`, required |
 | pnpm | `pnpm-lock.yaml` (every `lockfileVersion`); `shrinkwrap.yaml` only when there is no `pnpm-lock.yaml`; with `rush.json`, `common/config/rush/pnpm-lock.yaml` + `common/config/subspaces/*/pnpm-lock.yaml` | `packages:` `resolution.tarball` on the patch host | `file:.socket/vendor/npm/…` tarball + key | `integrity`, required |
 | yarn | `yarn.lock` (classic and berry) | classic `resolved`; berry `resolution: …::__archiveUrl=<url>` | classic `resolved "file:./.socket/vendor/npm/…#<sha1>"`; berry `file:` entry **plus** a root `package.json` `resolutions` mapping onto the same artifact (without it the entry is orphaned: diagnosed, no ref) | classic `integrity` / `#sha1`, berry `checksum`, required |
 | bun | `bun.lock`; `bun.lockb` only when there is no `bun.lock` (bun reads exactly one) | URL tuple / binary remote-tarball resolution; version from the URL leaf | `.socket/vendor/npm/<uuid>/<name>-<ver>.tgz` tuple / local-tarball resolution | `sha512-…`, required. A 2-tuple that Bun < 1.3.10 re-saved without its digest is still a reference, but it attests only from an installed tree. |

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

@@ -596,6 +596,119 @@ fn npm_vendor_vex_attests_against_vendored_tarball() {
     );
 }
 
+/// #326: a dependency installed from a remote-tarball spec (`"left-pad":
+/// "https://…/left-pad-1.3.0.tgz"`) is fetched from that url by `npm ci`,
+/// whatever the lock's `resolved` says. Vendoring used to rewire the entry
+/// and report `applied` (and `vex` attested it) while `npm ci` installed
+/// the original bytes. It must refuse, leave the lock untouched, and give
+/// `vex` nothing to attest.
+#[test]
+fn npm_vendor_refuses_a_remote_tarball_dependency() {
+    let suite = "e2e_vendor_npm_build (remote tarball)";
+    let Some(major) = npm_major_or_skip(suite) else {
+        return;
+    };
+    let tmp = tempfile::tempdir().unwrap();
+    let proj = tmp.path().join("proj");
+    std::fs::create_dir_all(&proj).unwrap();
+    std::fs::write(
+        proj.join("package.json"),
+        r#"{"name":"vendor-url-spec","version":"0.0.0","private":true}"#,
+    )
+    .unwrap();
+    let cache = tmp.path().join("npm-cache");
+    let url = format!("https://registry.npmjs.org/{DEP}/-/{DEP}-{DEP_VERSION}.tgz");
+    // npm 12 refuses remote-tarball specs unless `allow-remote` permits them.
+    let spec = if major >= 12 {
+        format!("{url} --allow-remote=all")
+    } else {
+        url.clone()
+    };
+    let mut args = vec!["install", "--no-audit", "--no-fund", "--cache"];
+    args.push(cache.to_str().unwrap());
+    args.extend(spec.split(' '));
+    let out = npm(&proj, &args);
+    if !out.status.success() {
+        npm_e2e_common::skip(
+            suite,
+            &format!(
+                "`npm install {spec}` failed (registry unreachable?):\n{}",
+                String::from_utf8_lossy(&out.stderr)
+            ),
+        );
+        return;
+    }
+    let pkg: serde_json::Value =
+        serde_json::from_slice(&std::fs::read(proj.join("package.json")).unwrap()).unwrap();
+    assert_eq!(
+        pkg["dependencies"][DEP], url,
+        "the fixture depends on the url spec: {pkg}"
+    );
+
+    let installed_index = proj.join("node_modules").join(DEP).join("index.js");
+    let orig = std::fs::read(&installed_index).expect("installed index.js");
+    let patched: Vec<u8> = [MARKER.as_bytes(), orig.as_slice()].concat();
+    let purl = format!("pkg:npm/{DEP}@{DEP_VERSION}");
+    const GHSA: &str = "GHSA-vend-npm-url";
+    stage_patch_with_vuln(&proj, &purl, "package/index.js", &orig, &patched, GHSA);
+    if v1_lock_is_refused(&proj, major) {
+        return;
+    }
+    let lock_before = std::fs::read(proj.join("package-lock.json")).unwrap();
+
+    let (_code, stdout, stderr) = run_socket(
+        &proj,
+        &[
+            "vendor",
+            "--json",
+            "--offline",
+            "--cwd",
+            proj.to_str().unwrap(),
+        ],
+    );
+    let env = parse_envelope(&stdout);
+    assert_eq!(
+        env["summary"]["applied"], 0,
+        "a url-spec dependency must not be vendored: {env}\nstderr:\n{stderr}"
+    );
+    assert!(
+        stdout.contains("vendor_lock_entry_not_rewritable") && stdout.contains("UNPATCHED"),
+        "the refusal must say why: {env}"
+    );
+    assert_eq!(
+        std::fs::read(proj.join("package-lock.json")).unwrap(),
+        lock_before,
+        "the lock is untouched"
+    );
+    assert!(
+        !proj.join(format!(".socket/vendor/npm/{UUID}")).exists(),
+        "no artifact is written"
+    );
+
+    let vex_path = proj.join("out.vex.json");
+    let (code, stdout, stderr) = run_socket(
+        &proj,
+        &[
+            "vex",
+            "--cwd",
+            proj.to_str().unwrap(),
+            "--output",
+            vex_path.to_str().unwrap(),
+            "--product",
+            "pkg:npm/app@1.0.0",
+        ],
+    );
+    let attested = std::fs::read(&vex_path)
+        .ok()
+        .and_then(|b| serde_json::from_slice::<serde_json::Value>(&b).ok())
+        .and_then(|doc| doc["statements"].as_array().map(|s| !s.is_empty()))
+        .unwrap_or(false);
+    assert!(
+        !attested,
+        "vex must not attest an unwired patch (exit {code}).\nstdout:\n{stdout}\nstderr:\n{stderr}"
+    );
+}
+
 /// get-driven twin of the capstone (v3.6): instead of hand-staging
 /// `.socket/` (manifest + blob) and running `vendor --offline`,
 /// `get <uuid> --mode vendored` resolves the SAME patch from a mocked