Fix npm 12 pinned allowScripts lost on scan (#1595)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1461,6 +1461,7 @@ command's integration tests check its output.
 | `vendor_workspace_member_skipped` | `skipped` (warning) | vendor / scan / get `--mode vendored` (npm, #688): a lock entry with the package's `name@version` is the project's own source (a workspace member or a `file:` directory), so it is left alone while the lock's registry copies are vendored; patch that source directly if it needs the fix. When it is the only instance, vendoring refuses instead. |
 | `vendor_copy_unpatched` | `skipped` (warning) | vendor / scan / get `--mode vendored` (yarn berry, #939): the registry entry was wired, but `yarn.lock` also installs the same `name@version` from a `file:` tarball / directory, URL tarball or `portal:` / `link:` dependency declared under another name (`lp2@file:…`), which the name-keyed `resolutions` entry never reaches; the detail names that entry, and that copy stays unpatched (`vex` does not attest the package beside it). |
 | `vendor_npm_allow_file` | `skipped` (warning, printed as `Warning (vendor_npm_allow_file)`) | vendor / scan / get `--mode vendored` (package-lock, #969): the effective npm `allow-file` setting (env > project `.npmrc` > user > global > builtin) is not `all`, so npm >= 11.14 refuses the vendored `file:` tarball (EALLOWFILE) on install. The setting is respected, never rewritten; the detail names where it comes from and the remedy (`allow-file=all`). `vendor --check` fails the entry for the same reason. |
+| `vendor_npm_allow_scripts` | `skipped` (warning, printed as `Warning (vendor_npm_allow_scripts)`) | vendor / scan / get `--mode vendored` (package-lock, #1595): the root package.json `allowScripts` approves a vendored package's install script only by version (`"pkg@x.y.z": true`), which npm 12 matches only against a registry tarball URL, so the next install blocks the approved script (every install fails under `strict-allow-scripts`). Raised only when a lock entry has `hasInstallScript`, no name-only approval (`pkg`, `pkg@*`) covers it and no deny matches. The policy is never rewritten; the detail names the pinned keys and the name-only remedy. |
 | `vendor_yarn_berry_unsupported` | `failed` | vendor (npm): yarn-berry Plug'n'Play layout; use its native `yarn patch` workflow. |
 | `vendor_bun_lockb_invalid` | `failed` | vendor / scan / get `--mode vendored`: the binary lock is malformed, unreadable, unsupported or cannot be rewritten safely. The detail names the parser, hash or filesystem error. Refused before patch downloads and before hosted takeover; the package's `failed` event carries `errorCode` and `error`, while `get <uuid>` also carries top-level `error.code`. Dry-run predicts the same refusal. |
 | `vendor_bun_workspace_unsupported` | `failed` | vendor / scan / get `--mode vendored` (bun): the text lock holds `workspace:` packages and its `lockfileVersion` is below 2 — Bun 1.2–1.3 resolve a workspace member's local-tarball path relative to the member; a committed version-2 lock is the proof every consumer runs Bun ≥ 1.4 (deliberate over-approximation: root-only declared packages would install on version 1 too). Detail names the version integer and a version-specific remedy: delete `bun.lock` and re-lock with Bun ≥ 1.4 (an in-place `bun install` keeps the existing version) — then, for a version-1 lock, "or use `--mode hosted`, which accepts version-1 workspace locks"; for a version-0 lock, "or delete `bun.lock`, re-lock with Bun ≥ 1.2 (which writes lockfileVersion 1) and use `--mode hosted`" (hosted refuses version-0 workspace locks, so a bare hosted pointer would send the user into a second refusal). Refused before any write — in the pre-download preflight on `get`/`scan` (see `vendor_bun_lockb_invalid` for the placements); in the shared preflight that `vendor` and the vendor step run BEFORE a hosted → vendored takeover's revert (a hosted-redirected purl stays hosted-wired, ledger and lock untouched; `vendor --dry-run` previews the same `failed` code); and in the engine when the run would write a NEW local tuple. Exempt: purls the vendor ledger wires at the selected uuid, purls whose every `bun.lock` instance is already a `.socket/vendor/npm/` tuple (any uuid), in-sync re-runs and `repair` redownloads. |
@@ -1570,6 +1571,7 @@ command's integration tests check its output.
 | `redirect_vlt_lockfile_version_missing` / `redirect_vlt_old_lockfile_ignored` / `redirect_vlt_scalar_registry_ignored` | `warnings[]` (warning) | scan/get `--mode hosted` (vlt): the lock has no `lockfileVersion` (vlt ≥ 1.0.0-rc.15 re-resolves it) / a legacy default-registry id without `"modifiers"` in `vlt.json` (vlt 0.0.0-16 … 0.0.0-24 ignore the lock) / a scalar `registry` option that vlt 1.0.0-rc.7 … rc.29 honor over the lock. The deps stay redirected, but the run's `--vex` does not attest them. |
 | `redirect_vlt_sibling_lockfiles` | `warnings[]` (warning) | scan/get `--mode hosted` (vlt): `vlt-lock.json` and another npm-family lock are both present and vlt's install state (`node_modules/.vlt-lock.json` or `node_modules/.vlt/`) is not, so both locks were rewritten and the other lock's rules confirm. |
 | `redirect_npm_replace_registry_host` | `warnings[]` (warning) | scan/get `--mode hosted` (npm): the effective npm `replace-registry-host` (env var, project `.npmrc`, or user / global / builtin config) is `always` or a pinned hosted hostname, so npm rewrites the hosted pins to the configured registry and every install fails E404; the detail names the layer and the remedies (`replace-registry-host=npmjs` in the project `.npmrc`, or vendored mode). See the npm `replace-registry-host` contract. |
+| `redirect_npm_allow_scripts` | `warnings[]` (warning) | scan/get `--mode hosted` (npm, #1595): a root npm lock pins a patched package to its hosted artifact, a lock entry for it has `hasInstallScript`, and package.json `allowScripts` approves it only by version (`"pkg@x.y.z"` or `||`-joined exact versions) with no name-only approval and no deny. npm 12 matches a version pin only against a registry tarball URL, so the approved install script stops running (every install fails under `strict-allow-scripts`). The policy is never rewritten; the detail names the pinned keys and the name-only remedy. |
 | `redirect_vlt_no_lockfile` | `warnings[]` (warning) | scan/get `--mode hosted` (vlt): `vlt.json` or vlt's install state is present without `vlt-lock.json`; replaces `redirect_npm_no_lockfile` for vlt projects. |
 | `redirect_npm_shrinkwrap_only` | `warnings[]` (warning) | scan/get `--mode hosted` (npm, #899): the root `npm-shrinkwrap.json` is the only npm lock and carries a hosted redirect (this run's or an earlier one's — every run repeats it until the project gains a `package-lock.json`). npm >= 12 never reads the shrinkwrap and installs the unpatched registry bytes; npm <= 11 installs the patch. Rename the lock to `package-lock.json` (or commit a copy under that name) and re-run. |
 | `redirect_vlt_artifact_unverifiable` | `warnings[]` (warning), `skipped` (hosted event) | scan/get `--mode hosted` (vlt): before any takeover or rewrite (dry runs included), each granted artifact with a default-registry instance in `vlt-lock.json` (or, for a purl a `flavor: "vlt"` vendored entry claims, its vendored node, probed before the takeover reverts it) is fetched once as vlt fetches it (`accept-encoding: gzip;q=1.0, identity;q=0.5`, no `Authorization`, up to 10 redirects) and must return 200 with no content encoding (or `identity`) and the granted sha512. On failure (`content-encoding <v>`, `sha512 mismatch`, `http <status>`, `fetch error <e>`, `offline`) the dep is withheld from every rewriter when vlt drives or it is vlt-vendored (which also keeps it vendored), and from the vlt rewrite only otherwise (detail "…; vlt-lock.json was not changed for {purl}"; only the sibling lock this run rewrote can confirm it). A lock already pinned by an earlier run is left pinned, and neither confirmed nor attested. Projects without `vlt-lock.json` make no such request. The detail quotes the artifact URL (and any fetch error that echoes it) with its grant-token path level, the one just before the patch uuid, spelled `<redacted>`; host, uuid and leaf stay. The in-memory hosted engine (`hosted-bundle`, the Node addon) has no network for this fetch, so it judges every in-scope artifact as `--offline` does (withheld, never pinned; the vendored takeover it refuses anyway). Exit 0. |

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

@@ -3861,6 +3861,106 @@ async fn pnpm_warning_strips_userinfo_and_names_only_spliced_hosts() {
     );
 }
 
+/// [`write_project`] whose lock records an install script for the patched
+/// package and whose root package.json approves it in npm 12's
+/// `allowScripts` with `policy`.
+fn write_project_with_allow_scripts(root: &Path, policy: serde_json::Value) {
+    write_project(root);
+    std::fs::write(
+        root.join("package.json"),
+        serde_json::json!({
+            "name": "consumer",
+            "version": "0.0.0",
+            "dependencies": { NAME: VERSION },
+            "allowScripts": policy,
+        })
+        .to_string(),
+    )
+    .unwrap();
+    let path = root.join("package-lock.json");
+    let mut lock: serde_json::Value =
+        serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap();
+    lock["packages"][format!("node_modules/{NAME}")]["hasInstallScript"] = true.into();
+    std::fs::write(&path, serde_json::to_vec_pretty(&lock).unwrap()).unwrap();
+}
+
+/// #1595: npm 12 matches a version-pinned `allowScripts` approval
+/// (`"pkg@x.y.z": true`, what `npm approve-scripts` writes) only against a
+/// registry-shaped `resolved`, which the hosted redirect replaces, so the
+/// approved install script silently stops running. The run must say so
+/// (`redirect_npm_allow_scripts`, naming the pinned key and the name-only
+/// remedy) and leave package.json untouched; a name-only approval, which
+/// npm still honors, must not warn.
+#[tokio::test]
+#[serial]
+async fn hosted_npm_warns_when_pinned_allow_scripts_approval_is_lost() {
+    let server = MockServer::start().await;
+    mock_discovery(&server).await;
+    mock_reference(&server).await;
+    mock_view(&server).await;
+
+    let tmp = tempfile::tempdir().unwrap();
+    let pinned = format!("{NAME}@{VERSION}");
+    write_project_with_allow_scripts(tmp.path(), serde_json::json!({ &pinned: true }));
+    let package_json = std::fs::read(tmp.path().join("package.json")).unwrap();
+    let env = run_redirect_subprocess(tmp.path(), &server.uri());
+    assert_eq!(vlt_hosted_common::redirected(&env), 1, "anchor: {env}");
+    let lock = std::fs::read_to_string(tmp.path().join("package-lock.json")).unwrap();
+    assert!(lock.contains("\"hasInstallScript\": true"), "{lock}");
+    assert_eq!(
+        warning_codes(&env),
+        vec![
+            "redirect_npm_allow_remote".to_string(),
+            "redirect_npm_allow_scripts".to_string()
+        ],
+        "{env}"
+    );
+    let detail = env["warnings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .find(|w| w["code"] == "redirect_npm_allow_scripts")
+        .and_then(|w| w["detail"].as_str())
+        .unwrap()
+        .to_string();
+    assert!(
+        detail.contains(&format!("`\"{pinned}\": true`"))
+            && detail.contains(&format!("`\"{NAME}\": true`"))
+            && detail.contains("strict-allow-scripts"),
+        "the warning names the lost pin and the name-only remedy: {detail}"
+    );
+    assert_eq!(
+        std::fs::read(tmp.path().join("package.json")).unwrap(),
+        package_json,
+        "the user's allowScripts policy is never rewritten"
+    );
+
+    // A BOM-prefixed lock (kept by the rewriter) is read the same way.
+    let tmp = tempfile::tempdir().unwrap();
+    write_project_with_allow_scripts(tmp.path(), serde_json::json!({ &pinned: true }));
+    let lock_path = tmp.path().join("package-lock.json");
+    let mut bom = b"\xEF\xBB\xBF".to_vec();
+    bom.extend(std::fs::read(&lock_path).unwrap());
+    std::fs::write(&lock_path, bom).unwrap();
+    let env = run_redirect_subprocess(tmp.path(), &server.uri());
+    assert_eq!(vlt_hosted_common::redirected(&env), 1, "anchor: {env}");
+    assert!(
+        warning_codes(&env).contains(&"redirect_npm_allow_scripts".to_string()),
+        "a BOM-prefixed lock still warns: {env}"
+    );
+
+    // A name-only approval keeps matching the redirected entry: no warning.
+    let tmp = tempfile::tempdir().unwrap();
+    write_project_with_allow_scripts(tmp.path(), serde_json::json!({ &pinned: true, NAME: true }));
+    let env = run_redirect_subprocess(tmp.path(), &server.uri());
+    assert_eq!(vlt_hosted_common::redirected(&env), 1, "anchor: {env}");
+    assert_eq!(
+        warning_codes(&env),
+        vec!["redirect_npm_allow_remote".to_string()],
+        "{env}"
+    );
+}
+
 /// A clean, fully-successful hosted run must carry an EXACT warning set — not
 /// merely "contains X". Presence-only assertions let spurious warnings ride
 /// along unnoticed: every pnpm/yarn/bun/Rush run shipped a bogus

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

@@ -4428,6 +4428,72 @@ snapshots:
         );
     }
 
+    /// #1595: npm 12 matches a version-pinned `allowScripts` approval only
+    /// against a registry tarball URL, so once vendoring rewires the lock
+    /// to the `file:` tarball the approved install script stops running.
+    /// The vendored run still vendors, leaves package.json untouched and
+    /// records `vendor_npm_allow_scripts`; a name-only approval (which npm
+    /// still honors) records nothing.
+    #[tokio::test]
+    #[serial]
+    async fn vendor_package_lock_warns_when_pinned_allow_scripts_approval_is_lost() {
+        let server = MockServer::start().await;
+        let registry = mock_registry(&server).await;
+        for (policy, expect_warning) in [
+            (json!({ format!("{CONV_NAME}@{CONV_VERSION}"): true }), true),
+            (
+                json!({ format!("{CONV_NAME}@{CONV_VERSION}"): true, CONV_NAME: true }),
+                false,
+            ),
+        ] {
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path();
+            write_package_lock_project(root);
+            let package_json = json!({
+                "name": "consumer",
+                "version": "0.0.0",
+                "dependencies": { CONV_NAME: CONV_VERSION },
+                "allowScripts": policy,
+            })
+            .to_string();
+            std::fs::write(root.join("package.json"), &package_json).unwrap();
+            let lock_path = root.join("package-lock.json");
+            let mut lock: Value =
+                serde_json::from_slice(&std::fs::read(&lock_path).unwrap()).unwrap();
+            lock["packages"][format!("node_modules/{CONV_NAME}")]["hasInstallScript"] = true.into();
+            std::fs::write(&lock_path, serde_json::to_vec_pretty(&lock).unwrap()).unwrap();
+
+            seed_manifest_and_blob(root);
+            let (code, env) = vendor_online_cli(root, &registry, PATCH_ORIGIN, &[]);
+            assert_eq!(code, 0, "vendoring still succeeds: {env:#}");
+            find_event(&env, "applied", None);
+            let warned = events(&env)
+                .iter()
+                .find(|e| e["errorCode"] == "vendor_npm_allow_scripts");
+            if expect_warning {
+                let warned =
+                    warned.unwrap_or_else(|| panic!("expected vendor_npm_allow_scripts: {env:#}"));
+                assert_eq!(warned["action"], "skipped", "{env:#}");
+                let text = warned.to_string();
+                assert!(
+                    text.contains(&format!("{CONV_NAME}@{CONV_VERSION}"))
+                        && text.contains("vendored `file:` tarball"),
+                    "{text}"
+                );
+            } else {
+                assert!(
+                    warned.is_none(),
+                    "a name-only approval still matches: {env:#}"
+                );
+            }
+            assert_eq!(
+                std::fs::read_to_string(root.join("package.json")).unwrap(),
+                package_json,
+                "the user's allowScripts policy is never rewritten"
+            );
+        }
+    }
+
     /// [`write_package_lock_project`] plus a parent package `bund` that
     /// BUNDLES the same `name@version` (`inBundle: true`): the hosted run
     /// rewires only the hoisted entry (`redirect_npm_bundled_instance_skipped`).

crates/socket-patch-core/src/hosted/engine.rs

@@ -2306,8 +2306,9 @@ async fn rewrite_once(
         &overrides,
         &options,
     );
-    let (npm_warnings, npmrc_config_write) =
+    let (mut npm_warnings, npmrc_config_write) =
         npm_allow_remote(view, &files, &rewrite, &overrides, &options);
+    npm_warnings.extend(npm_allow_scripts(&files, &rewrite, &overrides));
     if let Some((text, edit)) = trust_config_write {
         rewrite.files.insert(PNPM_WORKSPACE_REL.to_string(), text);
         // Appended last, after the lock edits it serves. v5 keeps no hosted
@@ -2796,6 +2797,61 @@ fn governing_workspace(view: &ProjectView<'_>) -> Option<std::path::PathBuf> {
     }
 }
 
+/// npm >= 12 blocks dependency install scripts unless the root
+/// `package.json#allowScripts` approves them, and it matches a pinned key
+/// (`"esbuild@0.19.12": true`) only against a registry-shaped `resolved`
+/// URL, which a hosted redirect replaces (#1595). Warn
+/// (`redirect_npm_allow_scripts`) for every package whose rewired
+/// package-lock.json entry runs an install script that loses its approval.
+/// The policy is the user's, so it is never rewritten: the only key npm
+/// would still honor is a name-only one, which widens it. npm >= 12 never
+/// reads npm-shrinkwrap.json, so a shrinkwrap pin cannot lose one.
+fn npm_allow_scripts(
+    files: &BTreeMap<String, String>,
+    rewrite: &RewriteResult,
+    overrides: &[DepOverride],
+) -> Vec<RewriteWarning> {
+    use crate::patch::redirect::npm_allow_scripts::{
+        lost_detail, rewired_runs_install_script, AllowScripts, HOSTED_WARNING, NPM12_LOCK,
+    };
+    let read = |rel: &str| rewrite.files.get(rel).or_else(|| files.get(rel));
+    let Some(policy) = read("package.json").and_then(|text| AllowScripts::parse(text)) else {
+        return Vec::new();
+    };
+    let Some(lock) =
+        read(NPM12_LOCK).and_then(|text| crate::vendor::common::parse_json_text(text).ok())
+    else {
+        return Vec::new();
+    };
+    let mut seen: BTreeSet<(String, String)> = BTreeSet::new();
+    let mut warnings = Vec::new();
+    for o in overrides.iter().filter(|o| o.ecosystem == "npm") {
+        let name = crate::patch::redirect::full_name(o);
+        if !seen.insert((name.clone(), o.version.clone())) {
+            continue;
+        }
+        let Some(lost) = policy.lost(&name, &o.version) else {
+            continue;
+        };
+        let rewired = |_: &str, entry: &serde_json::Value| {
+            entry.get("resolved").and_then(serde_json::Value::as_str)
+                == Some(o.artifact_url.as_str())
+        };
+        if !rewired_runs_install_script(&lock, &name, &o.version, rewired) {
+            continue;
+        }
+        let target = match url_host(&o.artifact_url) {
+            Some(host) => format!("the hosted patch tarball on {host}"),
+            None => "the hosted patch tarball".to_string(),
+        };
+        warnings.push(warning(
+            HOSTED_WARNING,
+            lost_detail(&name, &o.version, &lost, &target),
+        ));
+    }
+    warnings
+}
+
 /// npm >= 12 ships `allow-remote=none`: it refuses (EALLOWREMOTE) every
 /// tarball whose `resolved` origin is not the configured registry — which
 /// is exactly what a hosted redirect writes. npm <= 11 installs it

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

@@ -48,6 +48,7 @@ mod group_equivalence_tests;
 #[cfg(test)]
 mod lock_index_equivalence_tests;
 mod maven_index;
+pub mod npm_allow_scripts;
 pub mod npmrc;
 mod pdm;
 mod pipenv;