Skip to content

feat: Phase 1 of allowScripts opt-in install-script policy#9360

Merged
owlstronaut merged 11 commits into
npm:latestfrom
JamieMagee:jamiemagee/install-scripts-phase-1
May 27, 2026
Merged

feat: Phase 1 of allowScripts opt-in install-script policy#9360
owlstronaut merged 11 commits into
npm:latestfrom
JamieMagee:jamiemagee/install-scripts-phase-1

Conversation

@JamieMagee
Copy link
Copy Markdown
Contributor

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 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.

What landed

  • 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

What's deliberately deferred

  • 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

@JamieMagee JamieMagee requested review from a team as code owners May 15, 2026 03:38
@JamieMagee JamieMagee force-pushed the jamiemagee/install-scripts-phase-1 branch 6 times, most recently from 3bb254c to e4270f0 Compare May 15, 2026 22:13
@bakkot
Copy link
Copy Markdown
Contributor

bakkot commented May 15, 2026

I think it would be bad to support an allowScripts field which can contain "foo": false but still allow foo's scripts to run. Similarly I think it would be bad to include a no-op strict-script-builds. That looks like it provides protection, but doesn't.

Also, it's weird to print this advisory information without it being actionable (except by silencing the warning).

I think a better phasing would be to implement the full RFC except that the default for packages not explicitly listed is to allow with warning, rather than to deny with warning, and then a later release can make the default to deny with warning and make no other changes.

If you really don't want to implement blocking in the first phase, it would be better for it to be a hard error to have "foo": false in allowScripts.

Three new configs: allow-scripts, strict-script-builds, dangerously-allow-all-scripts.

Why is the --allow-scripts CLI flag necessary? I feel like that's just going to encourage people to say "install this with npm install foo --allow-scripts=foo" and then it will work on their machine but not for the next person who tries to build the package.

Also, sidebar, the naming of strict-script-builds and dangerously-allow-all-scripts is weird. I would expect those to be named similarly, or possibly be the same option (--enforce-allow-scripts=on vs --enforce-allow-scripts=dangerously-off, maybe).

@JamieMagee
Copy link
Copy Markdown
Contributor Author

@bakkot Yeah, all of that sounds good. I'll switch to your proposed phasing.

Concretely:

  • Actual enforcement goes in. true allows, false blocks, both do what the user expects.
  • Phase 1 default for unlisted packages stays "allow with warning", which keeps the RFC's promise that nothing changes for anyone who hasn't opted in. Phase 2 is then the default flip.
  • Renaming --strict-script-builds to --strict-allow-scripts. --dangerously-allow-all-scripts keeps its name. I'd rather have a long, ugly, obviously discouraged flag than collapse it into a ternary.
  • Going to tighten the --allow-scripts description too, calling out that it's for one-off / npx use and that team-wide policy belongs in package.json or .npmrc. Doesn't remove the footgun but at least doesn't lean into it.

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.

@bakkot
Copy link
Copy Markdown
Contributor

bakkot commented May 17, 2026

SGTM.

Renaming --strict-script-builds to --strict-allow-scripts. --dangerously-allow-all-scripts keeps its name. I'd rather have a long, ugly, obviously discouraged flag than collapse it into a ternary.

I don't feel strongly about this point, but fwiw --enforce-allow-scripts=dangerously-off is in fact longer and more ugly than --dangerously-allow-all-scripts, and is at least as obviously discouraged to my eye.

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 --enforce-allow-scripts=dangerously-allow-missing and --enforce-allow-scripts=dangerously-off respectively.

Comment thread lib/utils/allow-scripts-cmd.js
Comment thread workspaces/config/lib/definitions/definitions.js
Comment thread workspaces/config/lib/definitions/definitions.js
Comment thread workspaces/arborist/lib/install-scripts.js Outdated
Comment thread lib/utils/allow-scripts-writer.js Outdated
Comment thread workspaces/arborist/lib/script-allowed.js
Comment thread lib/utils/reify-output.js
Comment thread lib/utils/allow-scripts-writer.js
Comment thread lib/commands/approve-scripts.js Outdated
Comment thread lib/utils/reify-output.js Outdated
Comment thread lib/utils/resolve-allow-scripts.js Outdated
Comment thread workspaces/arborist/lib/script-allowed.js
Comment thread workspaces/arborist/lib/script-allowed.js
…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
// Returns `{ name, version }` or `null` if no trusted identity exists.
const getTrustedRegistryIdentity = (node) => {
if (node.resolved && typeof node.resolved === 'string') {
const parsed = versionFromTgz('', node.resolved)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm in the minority but I use a custom registry that doesn't host tarballs at the 'standard' tarball URL (/@foo/bar/-/bar-1.2.3.tgz or /foo/-/foo-1.2.3.tgz). Those paths aren't really a requirement of npm registry, since the model is to read from dist.tarball. This function doesn't return the correct version for URLs my mirror uses, which means all name@version patterns don't match.

Is there a way we could implement without parsing the URL?

  • There's the arborist edges, that define 'why' it was fetched
  • There could be some meta-resolved 'source' field derived from the resolved version?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a fair concern. So your name only version will work, but the pinned version doesn't? I think for this PR that will be scope creep and it is in a good and useful state now, but if you'd like to follow up with a pr that addresses pinned approvals for non-standard registry URLs, I'd happily review it. the constraint is that whatever new trust anchor we pick has to keep holding under loadActual() so a forged on-disk package.json can't override it.

@owlstronaut owlstronaut merged commit 7068d42 into npm:latest May 27, 2026
64 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Backport to release/v11 failed.

This usually means the cherry-pick had conflicts. Please create a manual backport:

git fetch origin release/v11
git checkout -b backport/v11/9360 origin/release/v11
git cherry-pick -x 7068d4286eb446fdb0ded08d15d7b5c3883d80f5
# resolve any conflicts, then:
git push origin backport/v11/9360
Error details
Command failed: git cherry-pick -x 7068d4286eb446fdb0ded08d15d7b5c3883d80f5
error: could not apply 7068d4286... feat: Phase 1 of `allowScripts` opt-in install-script policy (#9360)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

owlstronaut pushed a commit that referenced this pull request May 27, 2026
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)
owlstronaut added a commit that referenced this pull request May 27, 2026
…9415)

Backports #9360 to v11.

Co-authored-by: Jamie Magee <jamie.magee@gmail.com>
@github-actions github-actions Bot mentioned this pull request May 26, 2026
@JamieMagee JamieMagee deleted the jamiemagee/install-scripts-phase-1 branch May 28, 2026 06:15
jdalton added a commit to SocketDev/socket-lib that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-packageurl-js that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-sdk-js that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-addon that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-btm that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-mcp that referenced this pull request May 28, 2026
jdalton added a commit to SocketDev/socket-cli that referenced this pull request May 28, 2026
fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Jun 2, 2026
…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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants