feat: Phase 1 of allowScripts opt-in install-script policy#9360
Conversation
3bb254c to
e4270f0
Compare
|
@bakkot Yeah, all of that sounds good. I'll switch to your proposed phasing. Concretely:
The reason I shipped advisory-only originally was to keep the "install behaviour unchanged" promise in the RFC. Your phasing keeps that for everyone who doesn't write a false entry, and anyone who does is opting in deliberately. Better than what I had. Going to amend the RFC for the new phasing and update the PR description here. |
|
SGTM.
I don't feel strongly about this point, but fwiw That said, it's true that there are two dangerous things (the [new] phase 1 default, and disabled entirely), and naming both would be a little awkward. Best I've got is |
…low-all-scripts configs
Three new configs to support the install-script opt-in policy. None
of them affect install behaviour yet; they're read by approve-scripts,
deny-scripts, and the install-time walker in later commits.
- allow-scripts: comma-separated package list. Used as a fallback
when the root package.json has no allowScripts field. Flattens
to flatOptions.allowScripts.
- strict-script-builds: boolean. Reserved for a future release that
will turn blocked-script warnings into errors. No-op for now.
- dangerously-allow-all-scripts: boolean escape hatch for that same
future release. No-op for now.
Refs: npm/rfcs#868
…I configs
A precedence resolver reads the install-time allowScripts policy from
the layered sources and threads it through install/ci into arborist.
- lib/utils/resolve-allow-scripts.js: pure resolver. Reads from
npm.prefix so workspace sub-installs still pick up the project
root. Returns { policy, source }. Strict fallback: package.json
wins over flat config; lower layers are silently ignored, with
one warn when a lower setting is being suppressed.
- install.js / ci.js: await the resolver before constructing
arborist opts, then pass policy through opts.allowScripts. Add
the three new params to each command's static params list.
- workspaces/arborist/lib/arborist/index.js: accept
options.allowScripts and store it on this.options. No enforcement
yet; read in later commits.
Also tightened the flatten function for the new allow-scripts config:
nopt wraps single comma-separated strings in arrays for [String, Array]
types, so each array entry needs splitting on commas before use.
Refs: npm/rfcs#868
Implements Phase 1 of [npm/rfcs#868](npm/rfcs#868), which makes dependency install scripts opt-in. **Install behaviour is unchanged.** Scripts still run as they always have. The only Phase 1 user-visible change is one advisory block at the end of `npm install` listing packages whose install scripts haven't been reviewed via the new `allowScripts` field in `package.json`. A future release will turn that advisory into an actual block. - `allowScripts` field in `package.json`, read at install time - Three new configs: `allow-scripts`, `strict-script-builds`, `dangerously-allow-all-scripts`. The latter two are no-ops in this release. They're registered so projects can pin them in tooling ahead of the release that flips the default. - `npm approve-scripts` and `npm deny-scripts` commands, with the RFC's asymmetric pin rule (approves can pin, denies are always name-only) - Advisory warning during `npm install`, `ci`, `update`, and `rebuild`. `npm exec` / `npx` consult only the user/global `.npmrc` layer per the RFC, with the policy threaded through libnpmexec for Phase 2 enforcement. - Identity matcher in `@npmcli/arborist` covering registry, git, file, and remote tarballs. Registry identity is derived from the lockfile's resolved URL (via `versionFromTgz`), never from `node.packageName` or `node.version`. Those getters read the installed tarball's `package.json` and can be forged. - Aliases match against the underlying registered package, not the alias name. `trusted@npm:naughty@1.0.0` is approved by writing `naughty`, not `trusted`. Holds even under `omitLockfileRegistryResolved`, where the install location alone (`node_modules/trusted`) would be misleading. The underlying name is derived from the incoming edge's alias `subSpec`. - Bundled deps with install scripts are flagged as unreviewed and filtered out of `npm approve-scripts --all` and positional matches. Per RFC they cannot be allowlisted in Phase 1. - Warning when a non-root workspace declares its own `allowScripts` - Actual blocking. The matcher exists and the policy is threaded through to arborist, but `arb.rebuild()`'s build set still runs everything. Phase 2 will gate `#addToBuildSet` on the matcher. - A safe allowlist syntax for bundled deps. The RFC notes a candidate `parent@1.2.3 > bundled-name` form for a follow-up. Refs: npm/rfcs#868 (cherry picked from commit 7068d42)
…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 -->

Implements Phase 1 of npm/rfcs#868, which makes dependency install scripts opt-in.
Install behaviour is unchanged. Scripts still run as they always have. The only Phase 1 user-visible change is one advisory block at the end of
npm installlisting packages whose install scripts haven't been reviewed via the newallowScriptsfield inpackage.json. A future release will turn that advisory into an actual block.What landed
allowScriptsfield inpackage.json, read at install timeallow-scripts,strict-script-builds,dangerously-allow-all-scripts. The latter two are no-ops in this release. They're registered so projects can pin them in tooling ahead of the release that flips the default.npm approve-scriptsandnpm deny-scriptscommands, with the RFC's asymmetric pin rule (approves can pin, denies are always name-only)npm install,ci,update, andrebuild.npm exec/npxconsult only the user/global.npmrclayer per the RFC, with the policy threaded through libnpmexec for Phase 2 enforcement.@npmcli/arboristcovering registry, git, file, and remote tarballs. Registry identity is derived from the lockfile's resolved URL (viaversionFromTgz), never fromnode.packageNameornode.version. Those getters read the installed tarball'spackage.jsonand can be forged.trusted@npm:naughty@1.0.0is approved by writingnaughty, nottrusted. Holds even underomitLockfileRegistryResolved, where the install location alone (node_modules/trusted) would be misleading. The underlying name is derived from the incoming edge's aliassubSpec.npm approve-scripts --alland positional matches. Per RFC they cannot be allowlisted in Phase 1.allowScriptsWhat's deliberately deferred
arb.rebuild()'s build set still runs everything. Phase 2 will gate#addToBuildSeton the matcher.parent@1.2.3 > bundled-nameform for a follow-up.Refs: npm/rfcs#868