diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 3756e7abf..f2cf2d4ab 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/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 ` 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 `, `sha512 mismatch`, `http `, `fetch error `, `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 ``; 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. | diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index f3afa2262..6874ab71d 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/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 diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index bd3377f85..6ae1f6c16 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/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, ®istry, 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`). diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 04b0c6d65..966ff41f9 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/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 { } } +/// 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, + rewrite: &RewriteResult, + overrides: &[DepOverride], +) -> Vec { + 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 diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index b9e81d265..593907689 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/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; diff --git a/crates/socket-patch-core/src/patch/redirect/npm_allow_scripts.rs b/crates/socket-patch-core/src/patch/redirect/npm_allow_scripts.rs new file mode 100644 index 000000000..fe4e9e123 --- /dev/null +++ b/crates/socket-patch-core/src/patch/redirect/npm_allow_scripts.rs @@ -0,0 +1,480 @@ +//! npm 12's `package.json#allowScripts` install-script policy (#1595). +//! +//! npm >= 12 blocks dependency install scripts unless the root +//! `package.json` approves them, and `npm approve-scripts` writes a +//! version-pinned key (`"esbuild@0.19.12": true`). npm matches a pinned +//! key only against the trusted version it reads from a registry-shaped +//! `resolved` URL (`…//-/-.tgz`, arborist's +//! `script-allowed.js` / `versionFromTgz`). A hosted redirect (patch +//! server URL) and a vendored rewire (`file:.socket/vendor/…`) both leave +//! that shape, so the trusted version becomes unknown: a pinned allow +//! stops matching, a pinned deny fails closed (it matches whatever its +//! version), and name-only keys (`pkg`, `pkg@*`) match as before. An +//! approved script then silently stops running (or, under +//! `strict-allow-scripts`, every install fails). Neither mode can keep the +//! approval valid without widening the policy or writing a +//! machine-specific key, so both warn. + +use std::cmp::Ordering; + +use semver::Version; +use serde_json::Value; + +/// Hosted-mode warning code (`warnings[]`). +pub const HOSTED_WARNING: &str = "redirect_npm_allow_scripts"; +/// Vendored-mode advisory code (`skipped` event). +pub const VENDOR_WARNING: &str = "vendor_npm_allow_scripts"; + +/// The only npm lock npm >= 12 reads (it never reads npm-shrinkwrap.json), +/// so the only one whose rewired entries can lose an approval. +pub const NPM12_LOCK: &str = "package-lock.json"; + +/// How an `allowScripts` key can match a registry package, mirroring +/// arborist's `matchRegistry` for the key types that name one. +enum KeyKind { + /// A bare name or `name@*`: matches whatever the lock resolves to. + NameOnly, + /// An exact version or `||`-joined exact versions: matches only when + /// npm reads a covered version from a registry-shaped `resolved`. + Pinned(Pin), +} + +/// The versions a pinned key covers, as arborist compares them. +enum Pin { + /// One version (npa `type: 'version'`): the trimmed spec must equal + /// the registry version as a string, so `v1.2.3`, `=1.2.3` and + /// `1.2.3+build` never match `1.2.3`. + Exact(String), + /// A `||` disjunction (npa `type: 'range'`): every part must be a + /// strict `semver.valid` version, matched by `semver.satisfies` (loose, + /// so build metadata is ignored). + AnyOf(Vec), +} + +impl Pin { + fn parse(spec: &str) -> Option { + if !spec.contains("||") { + // npa only calls a loose-valid version a version; anything + // else is a range or tag that matchRegistry rejects. + loose_version(spec)?; + return Some(Self::Exact(spec.to_string())); + } + spec.split("||") + .map(|part| { + let part = part.trim(); + Version::parse(part.strip_prefix('v').unwrap_or(part)).ok() + }) + .collect::>>() + .map(Self::AnyOf) + } + + fn covers(&self, version: &str) -> bool { + match self { + Self::Exact(spec) => spec == version, + Self::AnyOf(pins) => loose_version(version).is_some_and(|v| { + pins.iter() + .any(|pin| pin.cmp_precedence(&v) == Ordering::Equal) + }), + } + } +} + +/// The approval npm 12 gives one package under the policy. +#[derive(PartialEq)] +enum Verdict { + Allowed, + Denied, + Unreviewed, +} + +/// The parsed root `allowScripts` policy: `(key, name, kind, allow)` for +/// every key that can name a registry package. Aliases, ranges npm +/// rejects (`^1`, `latest`) and git / file / remote specs never match a +/// registry package, before or after the rewrite, and are dropped. +pub struct AllowScripts { + entries: Vec<(String, String, KeyKind, bool)>, +} + +/// What the rewrite of one package costs under the policy: the pinned +/// allows that stop matching and the pinned denies that start to. +pub struct LostApproval { + pub allows: Vec, + pub denies: Vec, +} + +/// Split an npm package-arg key into `(name, spec)`; `spec` is `None` for +/// a bare name. A scoped name's leading `@` is part of the name. +fn split_key(key: &str) -> (&str, Option<&str>) { + let search_from = usize::from(key.starts_with('@')); + match key[search_from..].find('@') { + Some(at) => { + let at = at + search_from; + (&key[..at], Some(&key[at + 1..])) + } + None => (key, None), + } +} + +/// `semver.valid(v, { loose: true })`, which also accepts surrounding +/// whitespace and a leading `=` / `v`: how npa decides a spec is a version. +fn loose_version(text: &str) -> Option { + let text = text.trim(); + let text = text.strip_prefix('=').unwrap_or(text).trim_start(); + let text = text + .strip_prefix('v') + .or_else(|| text.strip_prefix('V')) + .unwrap_or(text); + Version::parse(text).ok() +} + +impl AllowScripts { + /// The policy in the root `package.json` text, or `None` when it has + /// no `allowScripts` key that could name a registry package (or the + /// file is unreadable): nothing can be lost. + pub fn parse(package_json: &str) -> Option { + let manifest: Value = + serde_json::from_str(crate::formats::text::strip_bom(package_json)).ok()?; + let policy = manifest.get("allowScripts")?.as_object()?; + let entries: Vec<_> = policy + .iter() + .filter_map(|(key, value)| { + let allow = value.as_bool()?; + let (name, spec) = split_key(key); + let kind = match spec.map(str::trim) { + None | Some("") | Some("*") => KeyKind::NameOnly, + Some(spec) => KeyKind::Pinned(Pin::parse(spec)?), + }; + Some((key.clone(), name.to_string(), kind, allow)) + }) + .collect(); + (!entries.is_empty()).then_some(Self { entries }) + } + + /// npm's verdict for `name`, at `version` when npm can read one from a + /// registry `resolved`, or `None` once it cannot (deny wins). + fn verdict(&self, name: &str, version: Option<&str>) -> Verdict { + let mut allowed = false; + for (_, key_name, kind, allow) in &self.entries { + if key_name != name { + continue; + } + let matches = match (kind, version) { + (KeyKind::NameOnly, _) => true, + (KeyKind::Pinned(pin), Some(v)) => pin.covers(v), + // Unverifiable version: a deny fails closed, an allow is + // refused. + (KeyKind::Pinned(_), None) => !allow, + }; + if matches && !allow { + return Verdict::Denied; + } + allowed |= matches; + } + if allowed { + Verdict::Allowed + } else { + Verdict::Unreviewed + } + } + + /// The approval `name@version`'s install scripts lose once its lock + /// entries resolve outside the registry, or `None` when npm 12 runs + /// them as before (or never did: denied or unreviewed already). + pub fn lost(&self, name: &str, version: &str) -> Option { + loose_version(version)?; + if self.verdict(name, Some(version)) != Verdict::Allowed + || self.verdict(name, None) == Verdict::Allowed + { + return None; + } + let pinned = |allow: bool| -> Vec { + self.entries + .iter() + .filter(|(_, key_name, kind, a)| { + key_name == name && *a == allow && matches!(kind, KeyKind::Pinned(_)) + }) + .filter(|(_, _, kind, _)| match kind { + // Every pinned deny matches after the rewrite; only the + // allows that covered this version were lost. + KeyKind::Pinned(pin) => !allow || pin.covers(version), + KeyKind::NameOnly => false, + }) + .map(|(key, ..)| key.clone()) + .collect() + }; + Some(LostApproval { + allows: pinned(true), + denies: pinned(false), + }) + } +} + +/// Does a rewired `package-lock.json` entry for `name@version` run an +/// install script? `is_rewired(key, entry)` picks the entries the run +/// points outside the registry. lockfileVersion 2/3 entries say so with +/// `hasInstallScript`; a lockfileVersion 1 lock never records it, so a +/// rewired v1 entry counts as having one. Bundled copies never run +/// scripts. +pub fn rewired_runs_install_script( + lock: &Value, + name: &str, + version: &str, + is_rewired: impl Fn(&str, &Value) -> bool, +) -> bool { + if let Some(packages) = lock.get("packages").and_then(Value::as_object) { + return packages.iter().any(|(key, entry)| { + let Some(idx) = key.rfind("node_modules/") else { + return false; + }; + let folder = &key[idx + "node_modules/".len()..]; + entry.get("name").and_then(Value::as_str).unwrap_or(folder) == name + && entry.get("version").and_then(Value::as_str) == Some(version) + && entry.get("hasInstallScript").and_then(Value::as_bool) == Some(true) + && entry.get("inBundle").and_then(Value::as_bool) != Some(true) + && is_rewired(key, entry) + }); + } + fn walk_v1( + deps: Option<&Value>, + name: &str, + version: &str, + is_rewired: &dyn Fn(&str, &Value) -> bool, + ) -> bool { + let Some(deps) = deps.and_then(Value::as_object) else { + return false; + }; + deps.iter().any(|(key, entry)| { + (key == name + && entry.get("version").and_then(Value::as_str) == Some(version) + && entry.get("bundled").and_then(Value::as_bool) != Some(true) + && is_rewired(key, entry)) + || walk_v1(entry.get("dependencies"), name, version, is_rewired) + }) + } + walk_v1(lock.get("dependencies"), name, version, &is_rewired) +} + +/// The warning detail for `name@version`, whose approval is `lost` once +/// its lock entries point at `target` (the hosted patch server or the +/// vendored `file:` tarball). +pub fn lost_detail(name: &str, version: &str, lost: &LostApproval, target: &str) -> String { + let quote = |keys: &[String], value: bool| { + keys.iter() + .map(|key| format!("`\"{key}\": {value}`")) + .collect::>() + .join(", ") + }; + let mut why = Vec::new(); + if !lost.allows.is_empty() { + why.push(format!( + "package.json `allowScripts` approves it only by version ({}), which npm matches \ + only against a registry tarball URL", + quote(&lost.allows, true) + )); + } + if !lost.denies.is_empty() { + why.push(format!( + "npm applies its version-pinned deny ({}) to every version it cannot read from a \ + registry tarball URL", + quote(&lost.denies, false) + )); + } + let remedy = if lost.denies.is_empty() { + format!("approve the package by name (`\"{name}\": true`) in package.json `allowScripts`") + } else { + format!( + "remove its version-pinned deny entries and approve the package by name \ + (`\"{name}\": true`) in package.json `allowScripts`" + ) + }; + format!( + "npm >= 12 stops running {name}@{version}'s approved install scripts once the lock \ + resolves it to {target}: {}. The next `npm ci` installs the patched files but blocks \ + the install script (with `strict-allow-scripts=true` every install fails). To keep it \ + running, {remedy}; npm <= 11 is unaffected", + why.join("; and ") + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn lost_for(policy: &str, name: &str, version: &str) -> Option { + let text = format!(r#"{{ "name": "app", "allowScripts": {policy} }}"#); + AllowScripts::parse(&text)?.lost(name, version) + } + + #[test] + fn exact_pin_is_lost() { + let lost = lost_for(r#"{"esbuild@0.19.12": true}"#, "esbuild", "0.19.12").unwrap(); + assert_eq!(lost.allows, vec!["esbuild@0.19.12".to_string()]); + assert!(lost.denies.is_empty()); + } + + #[test] + fn scoped_pin_and_disjunction_are_lost() { + let policy = r#"{"@img/sharp@0.33.0 || 0.33.5": true, "@img/sharp@ 0.33.5 ": true, + "@img/sharp@v0.33.0 || 0.33.5+b": true}"#; + let lost = lost_for(policy, "@img/sharp", "0.33.5").unwrap(); + assert_eq!(lost.allows.len(), 3); + } + + /// npm compares a single-version pin as a string, so loose spellings + /// never approved the registry copy: nothing is lost, and the remedy + /// must not approve a script nobody approved. + #[test] + fn loose_single_version_spellings_never_matched() { + for spec in ["v0.19.12", "=0.19.12", "0.19.12+b"] { + let policy = format!(r#"{{"esbuild@{spec}": true}}"#); + assert!(lost_for(&policy, "esbuild", "0.19.12").is_none(), "{spec}"); + } + // A disjunction part npm's strict semver.valid rejects voids the key. + assert!(lost_for( + r#"{"esbuild@=0.19.12 || 0.20.0": true}"#, + "esbuild", + "0.19.12" + ) + .is_none()); + } + + /// A loosely spelled deny never matched the registry copy but fails + /// closed after the rewrite, overriding a name-only allow. + #[test] + fn loose_single_version_deny_starts_matching() { + let lost = lost_for( + r#"{"esbuild": true, "esbuild@v0.19.12": false}"#, + "esbuild", + "0.19.12", + ) + .unwrap(); + assert!(lost.allows.is_empty()); + assert_eq!(lost.denies, vec!["esbuild@v0.19.12".to_string()]); + } + + #[test] + fn name_only_approval_still_covers_it() { + for policy in [ + r#"{"esbuild@0.19.12": true, "esbuild": true}"#, + r#"{"esbuild@0.19.12": true, "esbuild@*": true}"#, + r#"{"esbuild": true}"#, + ] { + assert!(lost_for(policy, "esbuild", "0.19.12").is_none(), "{policy}"); + } + } + + /// npm fails a pinned deny closed on an unknown version, so a deny for + /// ANOTHER version starts matching the rewired copy, even beside a + /// name-only allow. + #[test] + fn other_version_deny_starts_matching() { + let lost = lost_for( + r#"{"esbuild": true, "esbuild@0.18.0": false}"#, + "esbuild", + "0.19.12", + ) + .unwrap(); + assert!(lost.allows.is_empty()); + assert_eq!(lost.denies, vec!["esbuild@0.18.0".to_string()]); + let detail = lost_detail("esbuild", "0.19.12", &lost, "x"); + assert!( + detail.contains("remove its version-pinned deny"), + "{detail}" + ); + + let lost = lost_for( + r#"{"esbuild@0.19.12": true, "esbuild@0.18.0": false}"#, + "esbuild", + "0.19.12", + ) + .unwrap(); + assert_eq!(lost.allows, vec!["esbuild@0.19.12".to_string()]); + assert_eq!(lost.denies, vec!["esbuild@0.18.0".to_string()]); + } + + #[test] + fn denied_unreviewed_or_unrelated_lose_nothing() { + for policy in [ + r#"{"esbuild@0.19.12": true, "esbuild": false}"#, + r#"{"esbuild@0.19.12": false}"#, + r#"{"esbuild@0.19.11": true}"#, + r#"{"esbuild@^0.19.0": true}"#, + r#"{"esbuild-wasm@0.19.12": true}"#, + r#"{"alias@npm:esbuild@0.19.12": true}"#, + r#"{"esbuild@0.19.12": "yes"}"#, + r#"{"esbuild": true, "esbuild@^0.18": false}"#, + r#"[]"#, + ] { + assert!(lost_for(policy, "esbuild", "0.19.12").is_none(), "{policy}"); + } + assert!(AllowScripts::parse("not json").is_none()); + assert!(AllowScripts::parse(r#"{"name": "app"}"#).is_none()); + assert!(AllowScripts::parse(r#"{"allowScripts": {}}"#).is_none()); + } + + #[test] + fn bom_prefixed_package_json_is_read() { + let text = "\u{feff}{\"allowScripts\": {\"esbuild@0.19.12\": true}}"; + assert!(AllowScripts::parse(text) + .unwrap() + .lost("esbuild", "0.19.12") + .is_some()); + } + + #[test] + fn install_script_read_from_rewired_v3_entries_only() { + let lock: Value = serde_json::from_str( + r#"{"lockfileVersion": 3, "packages": { + "": {"name": "app"}, + "node_modules/esbuild": {"version": "0.19.12", "resolved": "R", "hasInstallScript": true}, + "node_modules/x/node_modules/esbuild": {"version": "0.19.12", "resolved": "U", "hasInstallScript": true}, + "node_modules/plain": {"version": "1.0.0", "resolved": "R"}, + "node_modules/p/node_modules/bundled": {"version": "1.0.0", "resolved": "R", "hasInstallScript": true, "inBundle": true}, + "node_modules/al": {"name": "aliased", "version": "2.0.0", "resolved": "R", "hasInstallScript": true} + }}"#, + ) + .unwrap(); + let rewired = |_: &str, e: &Value| e["resolved"] == "R"; + assert!(rewired_runs_install_script( + &lock, "esbuild", "0.19.12", rewired + )); + // Only a copy the run left on the registry has a script: no loss. + assert!(!rewired_runs_install_script( + &lock, + "esbuild", + "0.19.12", + |k, _| { k == "node_modules/plain" } + )); + assert!(!rewired_runs_install_script( + &lock, "esbuild", "0.19.11", rewired + )); + assert!(!rewired_runs_install_script( + &lock, "plain", "1.0.0", rewired + )); + assert!(!rewired_runs_install_script( + &lock, "bundled", "1.0.0", rewired + )); + assert!(rewired_runs_install_script( + &lock, "aliased", "2.0.0", rewired + )); + assert!(!rewired_runs_install_script(&lock, "app", "1.0.0", rewired)); + } + + #[test] + fn rewired_v1_entry_counts_as_running_a_script() { + let lock: Value = serde_json::from_str( + r#"{"lockfileVersion": 1, "dependencies": { + "a": {"version": "1.0.0", "resolved": "U", "dependencies": { + "esbuild": {"version": "0.19.12", "resolved": "R"}}}, + "b": {"version": "1.0.0", "resolved": "R", "bundled": true} + }}"#, + ) + .unwrap(); + let rewired = |_: &str, e: &Value| e["resolved"] == "R"; + assert!(rewired_runs_install_script( + &lock, "esbuild", "0.19.12", rewired + )); + assert!(!rewired_runs_install_script(&lock, "a", "1.0.0", rewired)); + assert!(!rewired_runs_install_script(&lock, "b", "1.0.0", rewired)); + } +} diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 95db7b1c9..133f7e560 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -497,8 +497,9 @@ pub async fn vendor_npm_any<'a>( entry.flavor = Some(flavor.as_str().to_string()); } // npm >= 11.14's `allow-file` gates the `file:` specs the - // package-lock wiring writes (#969); the other flavors' package - // managers do not read it. + // package-lock wiring writes (#969), and npm 12's `allowScripts` + // pins stop matching them (#1595); the other flavors' package + // managers read neither. if flavor == NpmLockFlavor::PackageLock && result.success { if let Some((name, version)) = super::npm_common::parse_npm_purl(purl) { if let Some(detail) = @@ -506,6 +507,16 @@ pub async fn vendor_npm_any<'a>( { warnings.push(VendorWarning::new(npm_lock::ALLOW_FILE_WARNING, detail)); } + // npm 12 stops honoring a version-pinned allowScripts + // approval for the rewired `file:` tarball (#1595). + if let Some(detail) = + npm_lock::allow_scripts_lost(project_root, &name, &version).await + { + warnings.push(VendorWarning::new( + crate::patch::redirect::npm_allow_scripts::VENDOR_WARNING, + detail, + )); + } } } } diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index 6f1d95ec4..58e4f81e3 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -1776,6 +1776,42 @@ pub(super) async fn allow_file_refusal_with( )) } +/// The `vendor_npm_allow_scripts` advisory for `name@version` (#1595), or +/// `None`: npm >= 12 matches a version-pinned `package.json#allowScripts` +/// approval only against a registry tarball URL (and fails a pinned deny +/// closed), so once vendoring rewires package-lock.json to the `file:` +/// tarball an approved install script can stop running. Only the +/// instances vendoring rewires count (the [`scan_lock_matches`] set), and +/// only package-lock.json: npm >= 12 never reads npm-shrinkwrap.json. The +/// policy is the user's and is never rewritten. +pub(super) async fn allow_scripts_lost( + project_root: &Path, + name: &str, + version: &str, +) -> Option { + use crate::patch::redirect::npm_allow_scripts::{ + lost_detail, rewired_runs_install_script, AllowScripts, NPM12_LOCK, + }; + use crate::utils::fs::read_regular_to_string; + let package_json = read_regular_to_string(&project_root.join("package.json")) + .await + .ok()?; + let lost = AllowScripts::parse(&package_json)?.lost(name, version)?; + let bytes = read_regular_to_bytes(&project_root.join(NPM12_LOCK)) + .await + .ok()?; + let lock = parse_json_manifest(&bytes).ok()?; + let overrides = NpmOverrides::read(project_root).await; + let LockScan::Matches(matches) = + scan_lock_matches(&lock, &overrides, name, version, &mut Vec::new()) + else { + return None; + }; + let rewired = |key: &str, _: &Value| matches.iter().any(|m| m.key == key); + rewired_runs_install_script(&lock, name, version, rewired) + .then(|| lost_detail(name, version, &lost, "the vendored `file:` tarball")) +} + /// [`allow_file_refusal_with`] against this process's npm config layers. /// Unit tests see only the project `.npmrc`: the developer's or runner's /// own layers (an `npm_config_allow_file` variable, a `~/.npmrc`) must not diff --git a/docs/ecosystems.md b/docs/ecosystems.md index deda930d5..cc2ab1201 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -133,6 +133,14 @@ Each Legacy format has an upgrade path and an undo path. Both work in v5: E404: the hosted run reads it from the same env / project / user / global / builtin layers and warns `redirect_npm_replace_registry_host` (set `replace-registry-host=npmjs` in the project `.npmrc`, or use vendored mode). + npm 12 runs a dependency's install script only when `package.json` + `allowScripts` approves it, and matches a version-pinned key + (`"esbuild@0.19.12": true`, what `npm approve-scripts` writes) only against a + registry tarball URL. Hosted and vendored lock entries are not one, so when a + patched package with an install script is approved only by version, the run + leaves the policy alone and warns `redirect_npm_allow_scripts` / + `vendor_npm_allow_scripts`: approve it by name (`"esbuild": true`) to keep + the script running (#1595). npm 6 ignores `resolved` for registry dependencies, so a redirected lockfileVersion 1 lock fails closed with EINTEGRITY under npm 6 (`redirect_npm_legacy_client`) and installs under npm diff --git a/docs/testing/npm-compatibility.md b/docs/testing/npm-compatibility.md index 297a45779..711d72513 100644 --- a/docs/testing/npm-compatibility.md +++ b/docs/testing/npm-compatibility.md @@ -72,6 +72,13 @@ npm 12 notes: install unchanged. An explicit `none` (or `root` with a transitive vendored copy) refuses them with EALLOWFILE; the vendored run warns `vendor_npm_allow_file` and `vendor --check` fails (#969). +- `allowScripts` (npm 12): a version-pinned approval (`"pkg@x.y.z": true`) + matches only a registry-shaped `resolved`, so after a hosted or vendored + rewrite npm blocks the package's approved install script (every install + fails under `strict-allow-scripts=true`). A name-only approval keeps + working. The run leaves the policy untouched and warns + `redirect_npm_allow_scripts` / `vendor_npm_allow_scripts` when the patched + entry has `hasInstallScript` and no name-only approval covers it (#1595). Dependencies that ship their own `npm-shrinkwrap.json` (#753): the lock marks such a package `"hasShrinkwrap": true` (`firebase-tools`, `netlify-cli`), and