feat(pm): support npm approve-scripts/deny-scripts in approve-builds#1733
Merged
Conversation
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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 <pkg...> - --all -> npm approve-scripts --all - no args -> npm approve-scripts --allow-scripts-pending (list pending) - !pkg denies -> npm deny-scripts <pkg...> (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.
b690672 to
4e25692
Compare
- reject a positional passed via `--` on npm's read-only pending path instead of building an invalid `npm approve-scripts --allow-scripts-pending <pkg>` - 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.
Member
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2a34ce3. Configure here.
cpojer
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
npm 11.16.0 (npm/cli#9360, "Phase 1 of
allowScriptsopt-in install-script policy") addsnpm approve-scriptsandnpm deny-scripts, which manage an advisoryallowScriptsfield inpackage.json. This is the npm equivalent ofpnpm approve-builds/bun pm trust.vp pm approve-buildspreviously 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-buildsinvocation<pkg>...(approves)npm approve-scripts <pkg>...--allnpm approve-scripts --allnpm approve-scripts --allow-scripts-pending(read-only list)!<pkg>...(denies,!stripped)npm deny-scripts <pkg>...!deniesNotes
vp pm approve-builds esbuild !core-jsreturns 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.allowScriptsis 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-pendinglisting.version_satisfies/node_semverpattern (npm_supports_allow_scripts=>=11.16.0), matching pnpm's prerelease semantics.--allupdated from "pnpm only" to reflect pnpm + npm support.Tests
approve_builds.rs(approve-by-name,--all, pending-list, deny-only, multi-deny, mixed-rejected, pass-through, below-gate no-op, prerelease no-op). TheOptionreturn type is unchanged, so existing tests are untouched.command-pm-approve-builds-npm11/(npm@11.16.0) exercising the real npm commands end-to-end.Validation
cargo test -p vite_install -p vite_pm_cli(510 passed)just checkcargo clippy -p vite_install -p vite_pm_cli -- -D warningspnpm bootstrap-cli+ local/global approve-builds snap tests regenerated and reviewedNote
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-buildsnow 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-pendingpending list) andnpm deny-scriptswhen only!pkgtokens 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.Reviewed by Cursor Bugbot for commit 2a34ce3. Configure here.