Skip to content

Commit 6ae7ec7

Browse files
authored
feat(pm): handle npm 12 blocked install scripts in create and approve-builds (#2336)
npm 12 (now `latest`) skips dependency install scripts that the `allowScripts` field in package.json does not cover; the install succeeds with a warning. Approval only records the allowlist: a reinstall short-circuits on the up-to-date tree, and only `npm rebuild` executes previously skipped scripts. npm 12 also stops resolving git and remote tarball dependencies by default (`EALLOWGIT` / `EALLOWREMOTE`). vp treated npm as "runs scripts by default", so `vp create` with npm 12 left native direct dependencies silently unbuilt. This PR: - parses the npm 12 blocked-scripts install warning (`parseNpmBlockedScripts`) and surfaces gated direct dependencies in `vp create` like pnpm/bun/yarn; approval runs `vp pm approve-builds <pkg>` then `vp pm rebuild <pkg>`, and retry hints point at rebuild instead of reinstall - version-gates the `vp pm approve-builds` npm note: npm >= 12 explains the approve-then-rebuild two-step, npm 11.16 - 11.x keeps the advisory wording (allowlist recorded, scripts still run), denials get no note - adds the `command_pm_approve_builds_npm12` PTY fixture recorded against real npm 12.0.2 and re-records the npm11 fixture for the reworded note - updates the create and install guides: npm joins the gated-scripts package managers, new "Dependency build scripts (npm v12+)" section, and a note on the `allow-git` / `allow-remote` defaults The npm 11.x wording ("not yet covered") is deliberately not parsed: those versions still run scripts, so there is nothing to fix up. Verified end-to-end against npm 12.0.2: install of a project with a gated direct dep detects it, auto-approval writes `allowScripts` and the rebuild runs its postinstall. 54 TS unit tests and 34 Rust tests pass; all three npm approve-builds snapshot fixtures pass. Audited but left as follow-ups: `vp install -g` has no `--allow-scripts` plumbing (vp's own global installs have no scripts), `vp migrate` preserves git/remote specs that now fail under npm 12, and migrate installs do not surface gated builds (pre-existing, also true for pnpm). Closes #1823
1 parent 2c8c72d commit 6ae7ec7

11 files changed

Lines changed: 357 additions & 35 deletions

File tree

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_pm_approve_builds_npm11/snapshots/command_pm_approve_builds_npm11.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ No packages with unreviewed install scripts.
3737
**Exit code:** 1
3838

3939
```
40-
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.
40+
note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. npm 12 enforces the policy.
4141
npm error code ENOMATCH
4242
npm error No installed packages match: esbuild
4343
npm error A complete log of this run can be found in: <home>/.npm/_logs/<timestamp>-debug-0.log
@@ -50,7 +50,7 @@ deny-only -> npm deny-scripts core-js (advisory note)
5050
**Exit code:** 1
5151

5252
```
53-
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.
53+
note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. npm 12 enforces the policy.
5454
npm error code ENOMATCH
5555
npm error No installed packages match: core-js
5656
npm error A complete log of this run can be found in: <home>/.npm/_logs/<timestamp>-debug-0.log
@@ -81,6 +81,6 @@ Pass package names as positionals (`vp pm approve-builds <pkg>...`), not after `
8181
-> npm approve-scripts --all (advisory note)
8282

8383
```
84-
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.
84+
note: npm's allowScripts policy is advisory in npm 11.x: install scripts still run; npm only warns about unreviewed packages at install time. npm 12 enforces the policy.
8585
No packages with unreviewed install scripts.
8686
```
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "command-pm-approve-builds-npm12",
3+
"version": "1.0.0",
4+
"private": true,
5+
"packageManager": "npm@12.0.2"
6+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[[case]]
2+
name = "command_pm_approve_builds_npm12"
3+
vp = "global"
4+
skip-platforms = ["windows"]
5+
steps = [
6+
{ argv = ["vp", "pm", "approve-builds"], comment = "no args -> npm approve-scripts --allow-scripts-pending (lists pending)", continue-on-failure = true },
7+
{ argv = ["vp", "pm", "approve-builds", "esbuild"], comment = "-> npm approve-scripts esbuild (npm 12 enforces allowScripts, so vp points at vp pm rebuild)", continue-on-failure = true },
8+
{ argv = ["vp", "pm", "approve-builds", "!core-js"], comment = "deny-only -> npm deny-scripts core-js (denial keeps the enforced default, no note)", continue-on-failure = true },
9+
{ argv = ["vp", "pm", "approve-builds", "esbuild", "!core-js"], comment = "mixed approve+deny -> rejected, exit non-zero", continue-on-failure = true },
10+
{ argv = ["vp", "pm", "approve-builds", "--all"], comment = "-> npm approve-scripts --all (rebuild note)", continue-on-failure = true },
11+
]
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# command_pm_approve_builds_npm12
2+
3+
## `vp pm approve-builds`
4+
5+
no args -> npm approve-scripts --allow-scripts-pending (lists pending)
6+
7+
```
8+
No packages with unreviewed install scripts.
9+
```
10+
11+
## `vp pm approve-builds esbuild`
12+
13+
-> npm approve-scripts esbuild (npm 12 enforces allowScripts, so vp points at vp pm rebuild)
14+
15+
**Exit code:** 1
16+
17+
```
18+
note: npm records the approval in the `allowScripts` field of package.json but does not run scripts a previous install skipped. Run `vp pm rebuild <package>` to execute them.
19+
npm error code ENOMATCH
20+
npm error No installed packages match: esbuild
21+
npm error A complete log of this run can be found in: <home>/.npm/_logs/<timestamp>-debug-0.log
22+
```
23+
24+
## `vp pm approve-builds !core-js`
25+
26+
deny-only -> npm deny-scripts core-js (denial keeps the enforced default, no note)
27+
28+
**Exit code:** 1
29+
30+
```
31+
npm error code ENOMATCH
32+
npm error No installed packages match: core-js
33+
npm error A complete log of this run can be found in: <home>/.npm/_logs/<timestamp>-debug-0.log
34+
```
35+
36+
## `vp pm approve-builds esbuild !core-js`
37+
38+
mixed approve+deny -> rejected, exit non-zero
39+
40+
**Exit code:** 1
41+
42+
```
43+
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>...`.
44+
```
45+
46+
## `vp pm approve-builds --all`
47+
48+
-> npm approve-scripts --all (rebuild note)
49+
50+
```
51+
note: npm records the approval in the `allowScripts` field of package.json but does not run scripts a previous install skipped. Run `vp pm rebuild <package>` to execute them.
52+
No packages with unreviewed install scripts.
53+
```

crates/vp_pm_cli/src/resolution/commands/approve_builds.rs

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ use crate::resolution::{
55
PackageManagerDialect, Pnpm, Resolve, Yarn,
66
};
77

8-
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.";
8+
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. npm 12 enforces the policy.";
9+
10+
const NPM_ENFORCED_NOTE: &str = "npm records the approval in the `allowScripts` field of package.json but does not run scripts a previous install skipped. Run `vp pm rebuild <package>` to execute them.";
911

1012
#[pm_args]
1113
#[derive(clap::Args, Clone, Debug, Default, PartialEq, Eq)]
@@ -150,7 +152,18 @@ impl Resolve<ApproveBuildsArgs> for Npm {
150152
}
151153
}
152154
if writes_policy {
153-
diag.note(DiagnosticKind::BehaviorChange, NPM_ADVISORY_NOTE);
155+
// npm 12 enforces allowScripts (skipped scripts stay skipped until a
156+
// rebuild); 11.16 - 11.x only warn. An unknown version is treated as
157+
// current, matching the version-gate default above.
158+
if self.version().is_none_or(|version| version_satisfies(version, ">=12.0.0")) {
159+
// An approval takes effect on the next rebuild; a denial keeps
160+
// the enforced default and needs no follow-up.
161+
if !has_denies {
162+
diag.note(DiagnosticKind::BehaviorChange, NPM_ENFORCED_NOTE);
163+
}
164+
} else {
165+
diag.note(DiagnosticKind::BehaviorChange, NPM_ADVISORY_NOTE);
166+
}
154167
}
155168
cmd.extend(args.pass_through_args.iter());
156169
cmd.into()
@@ -574,6 +587,56 @@ mod tests {
574587
assert_eq!(resolution.diagnostics[0].message, NPM_ADVISORY_NOTE);
575588
}
576589

590+
#[test]
591+
fn npm_v12_approve_notes_rebuild() {
592+
let resolution = resolve(
593+
&npm("12.0.2"),
594+
ApproveBuildsArgs { packages: vec!["esbuild".to_string()], ..Default::default() },
595+
);
596+
let CommandResolution::Run(command) = resolution.outcome else {
597+
panic!("expected command resolution");
598+
};
599+
600+
assert_eq!(command.args, vec!["approve-scripts", "esbuild"]);
601+
assert_eq!(resolution.diagnostics[0].message, NPM_ENFORCED_NOTE);
602+
}
603+
604+
#[test]
605+
fn npm_v12_all_notes_rebuild() {
606+
let resolution =
607+
resolve(&npm("12.0.2"), ApproveBuildsArgs { all: true, ..Default::default() });
608+
let CommandResolution::Run(command) = resolution.outcome else {
609+
panic!("expected command resolution");
610+
};
611+
612+
assert_eq!(command.args, vec!["approve-scripts", "--all"]);
613+
assert_eq!(resolution.diagnostics[0].message, NPM_ENFORCED_NOTE);
614+
}
615+
616+
#[test]
617+
fn npm_v12_deny_has_no_note() {
618+
let resolution = resolve(
619+
&npm("12.0.2"),
620+
ApproveBuildsArgs { packages: vec!["!core-js".to_string()], ..Default::default() },
621+
);
622+
let CommandResolution::Run(command) = resolution.outcome else {
623+
panic!("expected command resolution");
624+
};
625+
626+
assert_eq!(command.args, vec!["deny-scripts", "core-js"]);
627+
assert!(resolution.diagnostics.is_empty());
628+
}
629+
630+
#[test]
631+
fn npm_unknown_version_notes_rebuild() {
632+
let resolution = resolve(
633+
&Npm::unknown_version(),
634+
ApproveBuildsArgs { packages: vec!["esbuild".to_string()], ..Default::default() },
635+
);
636+
637+
assert_eq!(resolution.diagnostics[0].message, NPM_ENFORCED_NOTE);
638+
}
639+
577640
#[test]
578641
fn npm_v11_16_mixed_rejected() {
579642
let resolution = resolve(

docs/guide/create.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,13 @@ See the [Commit hooks guide](/guide/commit-hooks).
6262

6363
### Dependency build scripts
6464

65-
For security, pnpm, bun, and yarn (Berry) do not run a dependency's build scripts (`install` / `postinstall`, e.g. native builds like `better-sqlite3`) until you approve them. When a template adds such a dependency directly, `vp create` surfaces it after installing instead of leaving the project in a half-built state:
65+
For security, pnpm, bun, yarn (Berry), and npm (v12+) do not run a dependency's build scripts (`install` / `postinstall`, e.g. native builds like `better-sqlite3`) until you approve them. When a template adds such a dependency directly, `vp create` surfaces it after installing instead of leaving the project in a half-built state:
6666

6767
- Interactive: you are asked which of those dependencies to approve and build (nothing is selected by default).
6868
- Non-interactive: a note lists them and points at `vp pm approve-builds`.
6969
- `--approve-builds`: approves and builds them automatically, so non-interactive runs (CI) can produce a ready-to-use project.
7070

71-
Approval is recorded the way each package manager expects: pnpm's `allowBuilds`, bun's `trustedDependencies`, or yarn's `dependenciesMeta.<pkg>.built` (in the workspace root manifest). Transitive build scripts you did not choose (e.g. `esbuild` pulled in by Vite) are left at the package manager's defaults and are not surfaced. npm runs build scripts by default, so there is nothing to approve there.
71+
Approval is recorded the way each package manager expects: pnpm's `allowBuilds`, bun's `trustedDependencies`, npm's `allowScripts`, or yarn's `dependenciesMeta.<pkg>.built` (in the workspace root manifest). Transitive build scripts you did not choose (e.g. `esbuild` pulled in by Vite) are left at the package manager's defaults and are not surfaced. npm 11 and older run build scripts during install, so there is nothing to approve there.
7272

7373
## Template Options
7474

docs/guide/install.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,19 @@ vp rebuild -- --update-binary
167167

168168
With pnpm v10+, bare `vp rebuild` only rebuilds packages whose build scripts are listed in `onlyBuiltDependencies` (or approved via `pnpm approve-builds`); name the package explicitly to force a rebuild that bypasses the approval gate.
169169

170+
#### Dependency build scripts (npm v12+)
171+
172+
npm v12 skips dependency install scripts (`preinstall` / `install` / `postinstall`, including implicit `node-gyp` builds) unless the `allowScripts` field in package.json covers them; the install succeeds and npm warns about what it skipped. `vp pm approve-builds` manages that allowlist:
173+
174+
- `vp pm approve-builds <pkg...>` approves the named packages (`npm approve-scripts`)
175+
- `vp pm approve-builds !<pkg...>` denies them (`npm deny-scripts`)
176+
- `vp pm approve-builds --all` approves everything currently pending
177+
- `vp pm approve-builds` lists the packages whose scripts are not yet covered
178+
179+
Approval only records the allowlist: scripts an earlier install skipped do not run until you run `vp rebuild <pkg>`. With npm 11.16 - 11.x the same commands work, but npm treats the allowlist as advisory and still runs scripts.
180+
181+
npm v12 also stops resolving git dependencies (`github:`, `git+https:`) and remote tarball URLs by default; such installs fail with `EALLOWGIT` / `EALLOWREMOTE`. Opt back in per project with npm's `allow-git` / `allow-remote` config.
182+
170183
#### Advanced
171184

172185
Use these when you need lower-level package-manager behavior.

packages/cli/src/create/bin.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,8 +922,8 @@ Use \`vp create --list\` to list all available templates, or run \`vp create --h
922922
};
923923

924924
// After a successful install, surface gated build scripts (native builds like
925-
// better-sqlite3 the template added as a direct dependency) that pnpm or bun
926-
// blocked, and let the user approve them. `projectPath` is the created package
925+
// better-sqlite3 the template added as a direct dependency) that the package
926+
// manager blocked, and let the user approve them. `projectPath` is the created package
927927
// whose direct deps decide what is worth prompting for; `installCwd` is where
928928
// the package manager (and `node_modules`) lives.
929929
// Gated builds reported by the ESLint/Prettier migration pre-install. yarn

packages/cli/src/utils/__tests__/approve-builds.spec.ts

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
parseBunUntrusted,
1414
parseIgnoredBuilds,
1515
parseInstallGatedBuilds,
16+
parseNpmBlockedScripts,
1617
parseYarnDisabledBuilds,
1718
pnpmSupportsPositionalApprove,
1819
resolveApproveBuildTargets,
@@ -179,6 +180,62 @@ describe('parseYarnDisabledBuilds', () => {
179180
});
180181
});
181182

183+
describe('parseNpmBlockedScripts', () => {
184+
it('parses the npm 12 blocked-scripts warning from real install output', () => {
185+
// Captured from `npm install esbuild core-js` with npm 12.0.2.
186+
const output = [
187+
'',
188+
'added 3 packages in 466ms',
189+
'npm warn install-scripts 2 packages had install scripts blocked because they are not covered by allowScripts:',
190+
'npm warn install-scripts core-js@3.49.0 (postinstall: node -e "try{require(\'./postinstall\')}catch(e){}")',
191+
'npm warn install-scripts esbuild@0.28.1 (postinstall: node install.js)',
192+
'npm warn install-scripts',
193+
'npm warn install-scripts Run `npm install-scripts ls` to review, or `npm install-scripts approve <pkg>` to allow.',
194+
].join('\n');
195+
expect(parseNpmBlockedScripts(output)).toEqual(['core-js', 'esbuild']);
196+
});
197+
198+
it('parses the singular one-package form', () => {
199+
const output = [
200+
'npm warn install-scripts 1 package had install scripts blocked because they are not covered by allowScripts:',
201+
'npm warn install-scripts fsevents@2.3.3 (install: node-gyp rebuild)',
202+
].join('\n');
203+
expect(parseNpmBlockedScripts(output)).toEqual(['fsevents']);
204+
});
205+
206+
it('parses scoped packages and dedupes multiple versions', () => {
207+
const output = [
208+
'npm warn install-scripts 3 packages had install scripts blocked because they are not covered by allowScripts:',
209+
'npm warn install-scripts @scope/native@1.0.0 (preinstall: node setup.js)',
210+
'npm warn install-scripts esbuild@0.25.0 (postinstall: node install.js)',
211+
'npm warn install-scripts esbuild@0.28.1 (postinstall: node install.js)',
212+
].join('\n');
213+
expect(parseNpmBlockedScripts(output)).toEqual(['@scope/native', 'esbuild']);
214+
});
215+
216+
it('returns [] for the npm 11.x advisory warning (scripts still ran)', () => {
217+
// npm 11.16 - 11.x warn about unreviewed packages but run their scripts.
218+
const output = [
219+
'npm warn install-scripts 1 package has install scripts not yet covered by allowScripts:',
220+
'npm warn install-scripts esbuild@0.28.1 (postinstall: node install.js)',
221+
].join('\n');
222+
expect(parseNpmBlockedScripts(output)).toEqual([]);
223+
});
224+
225+
it('returns [] for the npm rebuild summary warning (no package list)', () => {
226+
expect(
227+
parseNpmBlockedScripts(
228+
'npm warn rebuild 1 package had install scripts blocked because they are not covered by allowScripts. Run `npm install-scripts ls` to review.',
229+
),
230+
).toEqual([]);
231+
});
232+
233+
it('returns [] for a clean install log', () => {
234+
expect(parseNpmBlockedScripts('added 87 packages in 39s')).toEqual([]);
235+
expect(parseNpmBlockedScripts('')).toEqual([]);
236+
});
237+
});
238+
182239
describe('parseInstallGatedBuilds', () => {
183240
it('dispatches to the pnpm parser for pnpm', () => {
184241
expect(
@@ -198,9 +255,18 @@ describe('parseInstallGatedBuilds', () => {
198255
).toEqual(['core-js']);
199256
});
200257

201-
it('returns [] for bun/npm (not parsed from install output)', () => {
258+
it('dispatches to the npm parser for npm', () => {
259+
expect(
260+
parseInstallGatedBuilds(
261+
'npm warn install-scripts 1 package had install scripts blocked because they are not covered by allowScripts:\n' +
262+
'npm warn install-scripts better-sqlite3@11.0.0 (install: node-gyp rebuild)',
263+
PackageManager.npm,
264+
),
265+
).toEqual(['better-sqlite3']);
266+
});
267+
268+
it('returns [] for bun (not parsed from install output)', () => {
202269
expect(parseInstallGatedBuilds('whatever', PackageManager.bun)).toEqual([]);
203-
expect(parseInstallGatedBuilds('whatever', PackageManager.npm)).toEqual([]);
204270
expect(parseInstallGatedBuilds('whatever', undefined)).toEqual([]);
205271
});
206272
});
@@ -299,9 +365,16 @@ describe('resolveApproveBuildTargets', () => {
299365
]);
300366
});
301367

302-
it('returns [] for package managers that do not gate builds', () => {
368+
it('returns direct-dep build targets for npm', () => {
369+
writePkg({ dependencies: { 'better-sqlite3': '^11.0.0' } });
370+
expect(
371+
resolveApproveBuildTargets(dir, ['better-sqlite3', 'esbuild'], PackageManager.npm),
372+
).toEqual(['better-sqlite3']);
373+
});
374+
375+
it('returns [] when the package manager is unknown', () => {
303376
writePkg({ dependencies: { 'better-sqlite3': '^11.0.0' } });
304-
expect(resolveApproveBuildTargets(dir, ['better-sqlite3'], PackageManager.npm)).toEqual([]);
377+
expect(resolveApproveBuildTargets(dir, ['better-sqlite3'], undefined)).toEqual([]);
305378
});
306379

307380
it('returns [] when there are no pending builds', () => {

0 commit comments

Comments
 (0)