Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 251 additions & 12 deletions crates/vite_install/src/commands/approve_builds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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<'_>,
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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");
Expand Down
6 changes: 3 additions & 3 deletions crates/vite_pm_cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "command-pm-approve-builds-npm11",
"version": "1.0.0",
"private": true,
"packageManager": "npm@11.16.0"
}
Original file line number Diff line number Diff line change
@@ -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.
Loading
Loading