feat(pm): support npm approve-scripts/deny-scripts in approve-builds (#1733)

## Summary

npm 11.16.0 ([npm/cli#9360](npm/cli#9360),
"Phase 1 of `allowScripts` opt-in install-script policy") adds `npm
approve-scripts` and `npm deny-scripts`, which manage an advisory
`allowScripts` field in `package.json`. This is the npm equivalent of
`pnpm approve-builds` / `bun pm trust`.

`vp pm approve-builds` previously warned and exited 0 (no-op) on npm. It
now forwards to npm's real commands when the detected npm is `>=
11.16.0`.

## Mapping (npm >= 11.16.0)

| `vp pm approve-builds` invocation | npm command |
| ----------------------------------- |
--------------------------------------------- |
| `<pkg>...` (approves) | `npm approve-scripts <pkg>...` |
| `--all` | `npm approve-scripts --all` |
| (no args) | `npm approve-scripts --allow-scripts-pending` (read-only
list) |
| `!<pkg>...` (denies, `!` stripped) | `npm deny-scripts <pkg>...` |
| mixed approves + `!denies` | rejected with an actionable error |
| npm < 11.16.0 | warn + exit 0 (no-op), advise upgrade |

## Notes

- **Mixed approve+deny is rejected** rather than silently split: npm
separates approve vs. deny into two commands, so `vp pm approve-builds
esbuild !core-js` returns a clear message asking the user to run the two
operations separately (pnpm handles the mixed case in one command). This
keeps the single-command return type intact.
- **Advisory caveat surfaced:** npm 11.x's `allowScripts` is advisory
only (install scripts still run; npm just warns about unreviewed
packages). A one-line note is shown after an approve/deny write so users
aren't misled. Not shown on the read-only `--allow-scripts-pending`
listing.
- Version gating reuses the existing `version_satisfies`/`node_semver`
pattern (`npm_supports_allow_scripts` = `>=11.16.0`), matching pnpm's
prerelease semantics.
- Help text for the deny prefix and `--all` updated from "pnpm only" to
reflect pnpm + npm support.

## Tests

- 9 new unit tests in `approve_builds.rs` (approve-by-name, `--all`,
pending-list, deny-only, multi-deny, mixed-rejected, pass-through,
below-gate no-op, prerelease no-op). The `Option` return type is
unchanged, so existing tests are untouched.
- New global snap test `command-pm-approve-builds-npm11/` (npm@11.16.0)
exercising the real npm commands end-to-end.
- 4 existing approve-builds snaps regenerated for the help-text wording
change and the updated npm warn message.

## Validation

- `cargo test -p vite_install -p vite_pm_cli` (510 passed)
- `just check`
- `cargo clippy -p vite_install -p vite_pm_cli -- -D warnings`
- `pnpm bootstrap-cli` + local/global approve-builds snap tests
regenerated and reviewed

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Changes are localized to PM command resolution and user messaging; npm
below 11.16.0 and yarn/pnpm/bun paths stay the same aside from help
text.
> 
> **Overview**
> **`vp pm approve-builds` now forwards to npm on npm ≥ 11.16.0**
instead of always warning and no-op’ing. Older npm still gets the legacy
warn + exit 0, with copy that mentions upgrading to 11.16.0.
> 
> For supported npm versions, invocations map to **`npm
approve-scripts`** (packages, `--all`, or no-args →
`--allow-scripts-pending` pending list) and **`npm deny-scripts`** when
only `!pkg` tokens are passed (`!` stripped). Mixed approve + deny in
one call is **rejected** with guidance to run two separate commands.
Package names passed only after `--` on the pending-list path are also
rejected.
> 
> After writes that change **`allowScripts`**, a **note** explains npm
11.x policy is advisory (scripts still run; enforcement is future).
Pass-through args are forwarded on the npm path like pnpm/bun.
> 
> CLI help and the approve-builds RFC are updated for pnpm + npm parity
on `!pkg`, `--all`, and no-args behavior. Coverage adds many npm 11.16
unit tests, a global snap fixture for npm@11.16.0, and regenerated snaps
for help/warn text.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
2a34ce3. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: fengmk2

crates/vite_install/src/commands/approve_builds.rs

@@ -10,6 +10,12 @@ use crate::package_manager::{
     PackageManager, PackageManagerType, ResolveCommandResult, format_path_env,
 };
 
+/// Note shown after an npm `approve-scripts`/`deny-scripts` write, so users
+/// aren't misled into thinking lifecycle scripts are now blocked.
+const NPM_ADVISORY_NOTE: &str = "npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only \
+     warns about unreviewed packages at install time. Enforcement is planned for a future \
+     npm release.";
+
 /// Options for the approve-builds command.
 #[derive(Debug, Default)]
 pub struct ApproveBuildsCommandOptions<'a> {
@@ -24,7 +30,7 @@ pub struct ApproveBuildsCommandOptions<'a> {
 impl PackageManager {
     /// Run the approve-builds command with the package manager.
     /// Returns `ExitStatus` with success (0) when the command is a no-op
-    /// (npm, yarn, or bun with only deny tokens / no positionals).
+    /// (npm < 11.16.0, yarn, or bun with only deny tokens / no positionals).
     pub async fn run_approve_builds_command(
         &self,
         options: &ApproveBuildsCommandOptions<'_>,
@@ -38,9 +44,10 @@ impl PackageManager {
 
     /// Resolve the approve-builds command.
     /// Returns `None` when the command is a no-op for the detected PM
-    /// (npm/yarn warn; bun with no approve-tokens prints a contextual hint).
-    /// Returns `Err(Error::InvalidArgument)` when `--all` or `!pkg` is requested
-    /// on a pnpm version that does not support it.
+    /// (npm < 11.16.0 / yarn warn; bun with no approve-tokens prints a contextual hint).
+    /// npm >= 11.16.0 forwards to `npm approve-scripts` / `deny-scripts`.
+    /// Returns `Err(Error::InvalidArgument)` when `--all` or `!pkg` is requested on a
+    /// pnpm version that does not support it, or when approves and denies are mixed on npm.
     pub fn resolve_approve_builds_command(
         &self,
         options: &ApproveBuildsCommandOptions,
@@ -121,13 +128,79 @@ impl PackageManager {
                 args.extend(approves.into_iter().cloned());
             }
             PackageManagerType::Npm => {
-                output::warn(
-                    "npm runs lifecycle scripts by default. To restrict them, set \
-                     `ignore-scripts=true` in .npmrc and rebuild approved packages with \
-                     `vp pm rebuild <package>`.",
-                );
-                warn_dropped_pass_through(options.pass_through_args);
-                return Ok(None);
+                // npm < 11.16.0 has no approve-scripts/deny-scripts: keep the legacy warn
+                // no-op (scripts run by default; advise restricting via .npmrc), enhanced to
+                // point at the upgrade. `vp pm approve-builds` always runs in a project
+                // context, so npm's global-only EGLOBAL error is never hit.
+                if !npm_supports_allow_scripts(&self.version) {
+                    output::warn(
+                        "npm runs lifecycle scripts by default. Upgrade to npm >= 11.16.0 for \
+                         `npm approve-scripts`/`deny-scripts`, or set `ignore-scripts=true` in \
+                         .npmrc and rebuild approved packages with `vp pm rebuild <package>`.",
+                    );
+                    warn_dropped_pass_through(options.pass_through_args);
+                    return Ok(None);
+                }
+
+                // npm splits approve vs. deny into two separate subcommands. vp accepts both
+                // in one invocation (pnpm runs them as one command); reject the mixed case
+                // rather than widen the single-command return type. Mirror the bun branch's
+                // partition idiom.
+                let (denies, approves): (Vec<&String>, Vec<&String>) =
+                    options.packages.iter().partition(|p| p.starts_with('!'));
+                let has_denies = !denies.is_empty();
+                let has_approves = !approves.is_empty();
+
+                if has_approves && has_denies {
+                    return Err(Error::InvalidArgument(
+                        "npm manages approvals and denials separately. Run them as two \
+                         invocations, e.g. `vp pm approve-builds <approve-pkg>...` then \
+                         `vp pm approve-builds !<deny-pkg>...`."
+                            .into(),
+                    ));
+                }
+
+                bin_name = "npm".into();
+                // Every branch except the read-only pending listing writes the
+                // allowScripts policy and warrants the advisory note.
+                let writes_policy;
+                if has_denies {
+                    // `deny-scripts <pkg...>` — strip the leading `!`, like the bun branch.
+                    args.push("deny-scripts".into());
+                    args.extend(
+                        denies.into_iter().map(|p| p.strip_prefix('!').unwrap_or(p).to_string()),
+                    );
+                    writes_policy = true;
+                } else {
+                    args.push("approve-scripts".into());
+                    if options.all {
+                        args.push("--all".into());
+                        writes_policy = true;
+                    } else if has_approves {
+                        args.extend(approves.into_iter().cloned());
+                        writes_policy = true;
+                    } else if options
+                        .pass_through_args
+                        .is_some_and(|extras| extras.iter().any(is_positional_arg))
+                    {
+                        // npm's read-only `--allow-scripts-pending` listing rejects
+                        // positionals; a package name passed via `--` was almost
+                        // certainly meant to be approved.
+                        return Err(Error::InvalidArgument(
+                            "Pass package names as positionals \
+                             (`vp pm approve-builds <pkg>...`), not after `--`."
+                                .into(),
+                        ));
+                    } else {
+                        // No args, no --all: npm has no interactive picker — list
+                        // pending (read-only). Nothing is written.
+                        args.push("--allow-scripts-pending".into());
+                        writes_policy = false;
+                    }
+                }
+                if writes_policy {
+                    output::note(NPM_ADVISORY_NOTE);
+                }
             }
             PackageManagerType::Yarn => {
                 // Yarn 1 (Classic) runs lifecycle scripts by default; Berry (2+) blocks them.
@@ -148,7 +221,8 @@ impl PackageManager {
             }
         }
 
-        // Append pass-through args to the underlying PM (pnpm/bun branches only).
+        // Append pass-through args to the underlying PM (pnpm, npm, and bun reach here;
+        // yarn and npm < 11.16.0 returned early above).
         if let Some(extra) = options.pass_through_args {
             args.extend_from_slice(extra);
         }
@@ -173,6 +247,13 @@ fn pnpm_supports_deny_syntax(version: &str) -> bool {
     version_satisfies(version, ">=11.0.0")
 }
 
+fn npm_supports_allow_scripts(version: &str) -> bool {
+    // `npm approve-scripts` / `deny-scripts` / `--allow-scripts-pending` shipped in npm
+    // v11.16.0 (npm/cli #9360, Phase 1). Same npm prerelease semantics as the pnpm gates:
+    // an `11.16.0-rc.x` tag does NOT satisfy `>=11.16.0`.
+    version_satisfies(version, ">=11.16.0")
+}
+
 fn version_satisfies(version: &str, range: &'static str) -> bool {
     // Static range strings always parse; unparsable user-supplied versions
     // are treated as not-satisfying (strict), since the production path
@@ -505,6 +586,7 @@ mod tests {
 
     #[test]
     fn npm_warns_and_noop() {
+        // npm < 11.16.0 has no approve-scripts/deny-scripts: legacy warn no-op.
         let pm = create_mock_package_manager(PackageManagerType::Npm, "11.0.0");
         let packages = vec!["esbuild".to_string()];
         let result = pm
@@ -516,6 +598,163 @@ mod tests {
         assert!(result.is_none());
     }
 
+    #[test]
+    fn npm_below_11_16_warns_noop() {
+        // The release immediately before approve-scripts/deny-scripts shipped.
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.15.0");
+        let packages = vec!["esbuild".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect("resolves");
+        assert!(result.is_none());
+    }
+
+    #[test]
+    fn npm_11_16_prerelease_noop() {
+        // npm semver convention: `11.16.0-rc.0` does not satisfy `>=11.16.0`, so it
+        // falls back to the legacy no-op (mirrors pnpm_all_rejects_v11_prerelease).
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0-rc.0");
+        let packages = vec!["esbuild".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect("resolves");
+        assert!(result.is_none());
+    }
+
+    #[test]
+    fn npm_v11_16_approve_by_name() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let packages = vec!["esbuild".to_string(), "fsevents".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.bin_path, "npm");
+        assert_eq!(result.args, vec!["approve-scripts", "esbuild", "fsevents"]);
+    }
+
+    #[test]
+    fn npm_v11_16_all() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                all: true,
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.args, vec!["approve-scripts", "--all"]);
+    }
+
+    #[test]
+    fn npm_v11_16_no_args_lists_pending() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions::default())
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.args, vec!["approve-scripts", "--allow-scripts-pending"]);
+    }
+
+    #[test]
+    fn npm_v11_16_pending_forwards_flags() {
+        // Flags passed via `--` are still forwarded to the read-only listing.
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let extra = vec!["--json".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                pass_through_args: Some(&extra),
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.args, vec!["approve-scripts", "--allow-scripts-pending", "--json"]);
+    }
+
+    #[test]
+    fn npm_v11_16_pending_rejects_positional_pass_through() {
+        // npm's `--allow-scripts-pending` listing rejects positionals; a package name
+        // slipped in via `--` (with no leading positionals) is rejected up-front with a
+        // clean message instead of building an invalid `npm approve-scripts` command.
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let extra = vec!["esbuild".to_string()];
+        let err = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                pass_through_args: Some(&extra),
+                ..Default::default()
+            })
+            .expect_err("a positional via `--` on the pending path should be rejected");
+        assert!(matches!(err, Error::InvalidArgument(_)));
+    }
+
+    #[test]
+    fn npm_v11_16_deny_only() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let packages = vec!["!core-js".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        // The leading `!` is stripped for npm deny-scripts.
+        assert_eq!(result.args, vec!["deny-scripts", "core-js"]);
+    }
+
+    #[test]
+    fn npm_v11_16_multiple_denies() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let packages = vec!["!core-js".to_string(), "!esbuild".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.args, vec!["deny-scripts", "core-js", "esbuild"]);
+    }
+
+    #[test]
+    fn npm_v11_16_mixed_rejected() {
+        // npm splits approve vs deny into two commands; vp rejects a mixed invocation.
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let packages = vec!["esbuild".to_string(), "!core-js".to_string()];
+        let err = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                ..Default::default()
+            })
+            .expect_err("mixed approve+deny should be rejected on npm");
+        assert!(matches!(err, Error::InvalidArgument(_)));
+    }
+
+    #[test]
+    fn npm_v11_16_appends_pass_through() {
+        let pm = create_mock_package_manager(PackageManagerType::Npm, "11.16.0");
+        let packages = vec!["esbuild".to_string()];
+        let extra = vec!["--silent".to_string()];
+        let result = pm
+            .resolve_approve_builds_command(&ApproveBuildsCommandOptions {
+                packages: &packages,
+                pass_through_args: Some(&extra),
+                ..Default::default()
+            })
+            .expect("resolves")
+            .expect("supported");
+        assert_eq!(result.args, vec!["approve-scripts", "esbuild", "--silent"]);
+    }
+
     #[test]
     fn yarn_berry_warns_and_noop() {
         let pm = create_mock_package_manager(PackageManagerType::Yarn, "4.0.0");

crates/vite_pm_cli/src/cli.rs

@@ -565,11 +565,11 @@ pub enum PmCommands {
     /// Approve dependency lifecycle scripts (install/postinstall) to run
     #[command(name = "approve-builds")]
     ApproveBuilds {
-        /// Packages to approve. Prefix with `!` to deny (pnpm >= 11.0.0 only).
-        /// Omit to launch interactive mode (pnpm only).
+        /// Packages to approve. Prefix with `!` to deny (pnpm >= 11.0.0, npm >= 11.16.0).
+        /// Omit to launch interactive mode (pnpm) or list pending packages (npm >= 11.16.0).
         packages: Vec<String>,
 
-        /// Approve every package currently pending approval (pnpm >= 10.32.0).
+        /// Approve every package currently pending approval (pnpm >= 10.32.0, npm >= 11.16.0).
         /// Mutually exclusive with positional packages.
         #[arg(long, conflicts_with = "packages")]
         all: bool,

packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt

@@ -4,11 +4,11 @@ Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
 Approve dependency lifecycle scripts (install/postinstall) to run
 
 Arguments:
-  [PACKAGES]...           Packages to approve. Prefix with `!` to deny (pnpm >= <semver> only). Omit to launch interactive mode (pnpm only)
+  [PACKAGES]...           Packages to approve. Prefix with `!` to deny (pnpm >= <semver>, npm >= <semver>). Omit to launch interactive mode (pnpm) or list pending packages (npm >= <semver>)
   [PASS_THROUGH_ARGS]...  Additional arguments to pass through to the package manager
 
 Options:
-  --all       Approve every package currently pending approval (pnpm >= <semver>). Mutually exclusive with positional packages
+  --all       Approve every package currently pending approval (pnpm >= <semver>, npm >= <semver>). Mutually exclusive with positional packages
   -h, --help  Print help
 
 Documentation: https://viteplus.dev/guide/install

packages/cli/snap-tests-global/command-pm-approve-builds-npm11/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "command-pm-approve-builds-npm11",
+  "version": "1.0.0",
+  "private": true,
+  "packageManager": "npm@11.16.0"
+}

packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt

@@ -0,0 +1,40 @@
+> vp pm approve-builds --help # should show help with pnpm/npm deny + --all caveats
+Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- <PASS_THROUGH_ARGS>...]
+
+Approve dependency lifecycle scripts (install/postinstall) to run
+
+Arguments:
+  [PACKAGES]...           Packages to approve. Prefix with `!` to deny (pnpm >= <semver>, npm >= <semver>). Omit to launch interactive mode (pnpm) or list pending packages (npm >= <semver>)
+  [PASS_THROUGH_ARGS]...  Additional arguments to pass through to the package manager
+
+Options:
+  --all       Approve every package currently pending approval (pnpm >= <semver>, npm >= <semver>). Mutually exclusive with positional packages
+  -h, --help  Print help
+
+Documentation: https://viteplus.dev/guide/install
+
+
+> vp pm approve-builds # no args -> npm approve-scripts --allow-scripts-pending (lists pending)
+No packages with unreviewed install scripts.
+
+[1]> vp pm approve-builds esbuild # -> npm approve-scripts esbuild (advisory note)
+note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. Enforcement is planned for a future npm release.
+npm error code ENOMATCH
+npm error No installed packages match: esbuild
+npm error A complete log of this run can be found in: <homedir>/.npm/_logs/<timestamp>-debug.log
+
+[1]> vp pm approve-builds '!core-js' # deny-only -> npm deny-scripts core-js (advisory note)
+note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. Enforcement is planned for a future npm release.
+npm error code ENOMATCH
+npm error No installed packages match: core-js
+npm error A complete log of this run can be found in: <homedir>/.npm/_logs/<timestamp>-debug.log
+
+[1]> vp pm approve-builds esbuild '!core-js' # mixed approve+deny -> rejected, exit non-zero
+npm manages approvals and denials separately. Run them as two invocations, e.g. `vp pm approve-builds <approve-pkg>...` then `vp pm approve-builds !<deny-pkg>...`.
+
+[1]> vp pm approve-builds -- esbuild # positional via -- on the pending path -> rejected, exit non-zero
+Pass package names as positionals (`vp pm approve-builds <pkg>...`), not after `--`.
+
+> vp pm approve-builds --all # -> npm approve-scripts --all (advisory note)
+note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. Enforcement is planned for a future npm release.
+No packages with unreviewed install scripts.