From 4e2569210a694aac59f484565158da352b9971cd Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 1 Jun 2026 13:37:39 +0800 Subject: [PATCH 1/2] feat(pm): support npm approve-scripts/deny-scripts in approve-builds npm 11.16.0 (npm/cli #9360) adds `approve-scripts` and `deny-scripts` commands that manage an advisory `allowScripts` field in package.json. `vp pm approve-builds` previously warned and no-oped on npm; it now forwards to these commands when npm >= 11.16.0: - approves -> npm approve-scripts - --all -> npm approve-scripts --all - no args -> npm approve-scripts --allow-scripts-pending (list pending) - !pkg denies -> npm deny-scripts (the `!` is stripped) Mixed approve+deny in a single invocation is rejected with an actionable message, since npm splits the two operations into separate commands. A one-line note is shown after a write because npm 11.x's allowScripts is advisory (install scripts still run). npm < 11.16.0 keeps the warn + exit-0 no-op, now pointing at the upgrade. --- .../src/commands/approve_builds.rs | 210 +++++++++++++++++- crates/vite_pm_cli/src/cli.rs | 6 +- .../command-pm-approve-builds-bun/snap.txt | 4 +- .../package.json | 6 + .../command-pm-approve-builds-npm11/snap.txt | 37 +++ .../steps.json | 11 + .../command-pm-approve-builds-npm/snap.txt | 10 +- .../command-pm-approve-builds-pnpm10/snap.txt | 4 +- .../command-pm-approve-builds-yarn/snap.txt | 4 +- rfcs/approve-builds-command.md | 21 +- 10 files changed, 279 insertions(+), 34 deletions(-) create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-npm11/package.json create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt create mode 100644 packages/cli/snap-tests-global/command-pm-approve-builds-npm11/steps.json diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index caa0445fba..818a46ad51 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/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,60 @@ 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 `.", - ); - 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 `.", + ); + 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 ...` then \ + `vp pm approve-builds !...`." + .into(), + )); + } + + bin_name = "npm".into(); + if has_denies { + // `deny-scripts ` — 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()), + ); + output::note(NPM_ADVISORY_NOTE); + } else { + args.push("approve-scripts".into()); + if options.all { + args.push("--all".into()); + output::note(NPM_ADVISORY_NOTE); + } else if has_approves { + args.extend(approves.into_iter().cloned()); + output::note(NPM_ADVISORY_NOTE); + } else { + // No args, no --all: npm has no interactive picker — list pending + // (read-only). No advisory note: nothing is written. + args.push("--allow-scripts-pending".into()); + } + } } PackageManagerType::Yarn => { // Yarn 1 (Classic) runs lifecycle scripts by default; Berry (2+) blocks them. @@ -173,6 +227,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 +566,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 +578,132 @@ 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_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"); diff --git a/crates/vite_pm_cli/src/cli.rs b/crates/vite_pm_cli/src/cli.rs index c82537e9ed..ee992e9b61 100644 --- a/crates/vite_pm_cli/src/cli.rs +++ b/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, - /// 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, diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt index 82158c431f..e51dbda9fa 100644 --- a/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-bun/snap.txt @@ -4,11 +4,11 @@ Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] Approve dependency lifecycle scripts (install/postinstall) to run Arguments: - [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= , npm >= ). Omit to launch interactive mode (pnpm) or list pending packages (npm >= ) [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager Options: - --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + --all Approve every package currently pending approval (pnpm >= , npm >= ). Mutually exclusive with positional packages -h, --help Print help Documentation: https://viteplus.dev/guide/install diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/package.json b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/package.json new file mode 100644 index 0000000000..a92e74501c --- /dev/null +++ b/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" +} diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt new file mode 100644 index 0000000000..04f7b15d5a --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt @@ -0,0 +1,37 @@ +> vp pm approve-builds --help # should show help with pnpm/npm deny + --all caveats +Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] + +Approve dependency lifecycle scripts (install/postinstall) to run + +Arguments: + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= , npm >= ). Omit to launch interactive mode (pnpm) or list pending packages (npm >= ) + [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager + +Options: + --all Approve every package currently pending approval (pnpm >= , npm >= ). 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: /.npm/_logs/-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: /.npm/_logs/-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 ...` then `vp pm approve-builds !...`. + +> 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. diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/steps.json b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/steps.json new file mode 100644 index 0000000000..1609046fcd --- /dev/null +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/steps.json @@ -0,0 +1,11 @@ +{ + "ignoredPlatforms": ["win32"], + "commands": [ + "vp pm approve-builds --help # should show help with pnpm/npm deny + --all caveats", + "vp pm approve-builds # no args -> npm approve-scripts --allow-scripts-pending (lists pending)", + "vp pm approve-builds esbuild # -> npm approve-scripts esbuild (advisory note)", + "vp pm approve-builds '!core-js' # deny-only -> npm deny-scripts core-js (advisory note)", + "vp pm approve-builds esbuild '!core-js' # mixed approve+deny -> rejected, exit non-zero", + "vp pm approve-builds --all # -> npm approve-scripts --all (advisory note)" + ] +} diff --git a/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt index d3bdc9fcce..e42a8bfde2 100644 --- a/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt +++ b/packages/cli/snap-tests/command-pm-approve-builds-npm/snap.txt @@ -4,18 +4,18 @@ Approve dependency lifecycle scripts (install/postinstall) to run Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] Arguments: - [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= , npm >= ). Omit to launch interactive mode (pnpm) or list pending packages (npm >= ) [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager Options: - --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + --all Approve every package currently pending approval (pnpm >= , npm >= ). Mutually exclusive with positional packages -h, --help Print help > vp pm approve-builds # warn and exit 0 (no-op on npm) -warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: npm runs lifecycle scripts by default. Upgrade to npm >= for `npm approve-scripts`/`deny-scripts`, or set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. > vp pm approve-builds esbuild # warn and exit 0 (no-op on npm) -warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: npm runs lifecycle scripts by default. Upgrade to npm >= for `npm approve-scripts`/`deny-scripts`, or set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. > vp pm approve-builds --all # warn and exit 0 (no-op on npm) -warn: npm runs lifecycle scripts by default. To restrict them, set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. +warn: npm runs lifecycle scripts by default. Upgrade to npm >= for `npm approve-scripts`/`deny-scripts`, or set `ignore-scripts=true` in .npmrc and rebuild approved packages with `vp pm rebuild `. diff --git a/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt index 5018fba0bd..4a102e90fa 100644 --- a/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt +++ b/packages/cli/snap-tests/command-pm-approve-builds-pnpm10/snap.txt @@ -4,11 +4,11 @@ Approve dependency lifecycle scripts (install/postinstall) to run Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] Arguments: - [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= , npm >= ). Omit to launch interactive mode (pnpm) or list pending packages (npm >= ) [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager Options: - --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + --all Approve every package currently pending approval (pnpm >= , npm >= ). Mutually exclusive with positional packages -h, --help Print help > vp pm approve-builds --all # forwards to pnpm approve-builds --all (nothing to approve) diff --git a/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt index b3f189fbcd..7a40e5dfec 100644 --- a/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt +++ b/packages/cli/snap-tests/command-pm-approve-builds-yarn/snap.txt @@ -4,11 +4,11 @@ Approve dependency lifecycle scripts (install/postinstall) to run Usage: vp pm approve-builds [OPTIONS] [PACKAGES]... [-- ...] Arguments: - [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= only). Omit to launch interactive mode (pnpm only) + [PACKAGES]... Packages to approve. Prefix with `!` to deny (pnpm >= , npm >= ). Omit to launch interactive mode (pnpm) or list pending packages (npm >= ) [PASS_THROUGH_ARGS]... Additional arguments to pass through to the package manager Options: - --all Approve every package currently pending approval (pnpm >= ). Mutually exclusive with positional packages + --all Approve every package currently pending approval (pnpm >= , npm >= ). Mutually exclusive with positional packages -h, --help Print help > vp pm approve-builds # warn and exit 0 (no-op on yarn) diff --git a/rfcs/approve-builds-command.md b/rfcs/approve-builds-command.md index 42ba643632..e631ec73c8 100644 --- a/rfcs/approve-builds-command.md +++ b/rfcs/approve-builds-command.md @@ -177,23 +177,26 @@ Running: bun pm trust --all **npm references:** -- No equivalent command. Closest configuration: [`ignore-scripts`](https://docs.npmjs.com/cli/v11/using-npm/config#ignore-scripts) and [`npm rebuild`](https://docs.npmjs.com/cli/v11/commands/npm-rebuild). +- `npm approve-scripts` / `npm deny-scripts` (npm ≥ 11.16.0, npm/cli #9360) manage an advisory `allowScripts` field in `package.json`. In npm 11.x this is advisory only: install scripts still run; npm just warns about unreviewed packages. +- For npm < 11.16.0: no equivalent command. Closest configuration: [`ignore-scripts`](https://docs.npmjs.com/cli/v11/using-npm/config#ignore-scripts) and [`npm rebuild`](https://docs.npmjs.com/cli/v11/commands/npm-rebuild). **yarn references:** - No equivalent command. yarn@2+ already blocks third-party build scripts by default ([`enableScripts`](https://yarnpkg.com/configuration/yarnrc#enableScripts) defaults to `false`); per-package opt-in is via [`dependenciesMeta..built`](https://yarnpkg.com/configuration/manifest#dependenciesMeta) in `package.json`. -| Vite+ Flag | pnpm | npm | yarn@1 | yarn@2+ | bun | Description | -| ----------------------------- | ---------------------------------------- | ---------- | ---------- | ---------- | --------------------------- | ------------------------------- | -| `vp pm approve-builds` | `pnpm approve-builds` | N/A (warn) | N/A (warn) | N/A (warn) | N/A (note) | Interactive prompt (pnpm only) | -| `vp pm approve-builds ` | `pnpm approve-builds ` | N/A (warn) | N/A (warn) | N/A (warn) | `bun pm trust ` | Approve named packages | -| `vp pm approve-builds !` | `pnpm approve-builds !` | N/A (warn) | N/A (warn) | N/A (warn) | N/A (warn — model mismatch) | Deny named packages (pnpm only) | -| `--all` | `pnpm approve-builds --all` (≥ v10.32.0) | N/A (warn) | N/A (warn) | N/A (warn) | `bun pm trust --all` | Approve every pending package | +| Vite+ Flag | pnpm | npm (≥ 11.16.0) | yarn@1 | yarn@2+ | bun | Description | +| ----------------------------- | ---------------------------------------- | --------------------------------------------- | ---------- | ---------- | --------------------------- | ------------------------------------------- | +| `vp pm approve-builds` | `pnpm approve-builds` | `npm approve-scripts --allow-scripts-pending` | N/A (warn) | N/A (warn) | N/A (note) | pnpm: interactive prompt; npm: list pending | +| `vp pm approve-builds ` | `pnpm approve-builds ` | `npm approve-scripts ` | N/A (warn) | N/A (warn) | `bun pm trust ` | Approve named packages | +| `vp pm approve-builds !` | `pnpm approve-builds !` | `npm deny-scripts ` | N/A (warn) | N/A (warn) | N/A (warn — model mismatch) | Deny named packages (pnpm, npm) | +| `--all` | `pnpm approve-builds --all` (≥ v10.32.0) | `npm approve-scripts --all` | N/A (warn) | N/A (warn) | `bun pm trust --all` | Approve every pending package | **Notes:** -- **`!pkg` deny syntax is pnpm-only.** For bun the deny syntax is rejected with a warning that names the affected positionals (so users notice rather than silently get a partial approval). -- **npm and yarn never have an `approve-builds` command.** Vite+ prints a one-line `warn` and exits 0. For npm (which runs scripts by default) the warn points at `ignore-scripts`. For yarn (which blocks third-party scripts by default) the warn points at `dependenciesMeta..built`. We intentionally exit 0 (not non-zero) so monorepo scripts that run `vp pm approve-builds` opportunistically don't break on heterogeneous environments. +- **`!pkg` deny syntax is supported on pnpm and npm.** pnpm forwards `!core-js` verbatim; npm strips the `!` and routes it to `npm deny-scripts core-js`. For bun the deny syntax is rejected with a warning that names the affected positionals (so users notice rather than silently get a partial approval). +- **npm splits approve vs. deny into two separate commands** (`approve-scripts` / `deny-scripts`). Because `vp pm approve-builds` accepts both in one invocation, a mixed call (`vp pm approve-builds esbuild !core-js`) is **rejected** on npm with an actionable message asking the user to run the two operations separately. pnpm handles the mixed case in a single command. +- **npm < 11.16.0 and yarn never have an `approve-builds` command.** Vite+ prints a one-line `warn` and exits 0. For npm the warn points at upgrading to npm ≥ 11.16.0 (or `ignore-scripts`). For yarn (which blocks third-party scripts by default) the warn points at `dependenciesMeta..built`. We intentionally exit 0 (not non-zero) so monorepo scripts that run `vp pm approve-builds` opportunistically don't break on heterogeneous environments. +- **npm's `allowScripts` is advisory in npm 11.x.** Even after approving, install scripts still run; npm only warns about unreviewed packages at install time. Vite+ surfaces a one-line `note` after an npm approve/deny write to make this clear. Enforcement is planned for a future npm release. - **No-args mode on bun** also exits 0 with a `note` (bun's `bun pm trust` requires package names; there's no interactive picker to forward to). - **Configuration storage differs:** pnpm writes to `pnpm-workspace.yaml` under `allowBuilds:`. bun writes to `package.json` under `trustedDependencies: []`. Vite+ does not normalize the storage location — each PM owns its own state. (See [Design Decision §2](#2-do-not-normalize-storage).) From 2a34ce3d7ca8f16f653a465e4db6d742c5fb6df5 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 1 Jun 2026 15:10:19 +0800 Subject: [PATCH 2/2] refactor(pm): address npm approve-builds review findings - reject a positional passed via `--` on npm's read-only pending path instead of building an invalid `npm approve-scripts --allow-scripts-pending ` - collapse the three duplicated advisory-note calls into a single `writes_policy` gate - fix the now-stale pass-through comment (npm also reaches the shared tail) - update RFC section 4, which no longer applies to npm >= 11.16.0 Adds unit tests for the pending guard (rejects positionals, still forwards flags) and a snap-test step covering the rejection. --- .../src/commands/approve_builds.rs | 63 +++++++++++++++++-- .../command-pm-approve-builds-npm11/snap.txt | 3 + .../steps.json | 1 + rfcs/approve-builds-command.md | 14 +++-- 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/crates/vite_install/src/commands/approve_builds.rs b/crates/vite_install/src/commands/approve_builds.rs index 818a46ad51..c93e22dd3a 100644 --- a/crates/vite_install/src/commands/approve_builds.rs +++ b/crates/vite_install/src/commands/approve_builds.rs @@ -161,27 +161,46 @@ impl PackageManager { } 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 ` — 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()), ); - output::note(NPM_ADVISORY_NOTE); + writes_policy = true; } else { args.push("approve-scripts".into()); if options.all { args.push("--all".into()); - output::note(NPM_ADVISORY_NOTE); + writes_policy = true; } else if has_approves { args.extend(approves.into_iter().cloned()); - output::note(NPM_ADVISORY_NOTE); + 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 ...`), not after `--`." + .into(), + )); } else { - // No args, no --all: npm has no interactive picker — list pending - // (read-only). No advisory note: nothing is written. + // 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. @@ -202,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); } @@ -645,6 +665,37 @@ mod tests { 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"); diff --git a/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt index 04f7b15d5a..bda22ba9e1 100644 --- a/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt +++ b/packages/cli/snap-tests-global/command-pm-approve-builds-npm11/snap.txt @@ -32,6 +32,9 @@ npm error A complete log of this run can be found in: /.npm/_logs/