Repository navigation
Build/Test Tools: Opt in to the npm supply chain defaults - #14058
manzoorwanijk wants to merge 5 commits into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| @@ -12,7 +12,7 @@ | |||
| }, | |||
| "engines": { | |||
| "node": ">=24.18.0", | |||
There was a problem hiding this comment.
Node.js 24.18 ships npm 11.16, and Node.js 24.19 ships npm 11.17. Both are below the new >=11.18.0 npm minimum. On those Node.js versions, npm install fails the devEngines npm check.
Can we raise the Node.js minimum to >=24.20.0 in engines and devEngines? Node.js 24.20 ships npm 11.19. The change applies to all four package.json files and their lockfiles.
| "node": ">=24.18.0", | |
| "node": ">=24.20.0", |
1c0086e to
77d88a3
Compare
77d88a3 to
756acc2
Compare
lancewillett
left a comment
There was a problem hiding this comment.
The npm policy and version requirements look correct. Clean installs and builds passed for Core and all three affected themes under Node 24.20/npm 11.19 in Docker. The native watcher and resolver dependencies also loaded successfully with their install scripts denied. CI is green.
AI review · gpt-6
756acc2 to
4020804
Compare
npm 11.16 and 11.17 reject registry dependencies with EALLOWREMOTE when allow-remote is set to none. Node.js 24.20 bundles npm 11.19, so .nvmrc moves to it. Applies to Core, Twenty Twenty-One, Twenty Twenty-Two and Twenty Twenty-Five, matching Gutenberg.
Denies every dependency install script via allowScripts and fails the install on unreviewed ones with strict-allow-scripts. Refuses git, remote URL and local tarball dependencies, matching the npm v12 defaults plus allow-file.
Applies the Core install script and dependency source policy to Twenty Twenty-One, Twenty Twenty-Two and Twenty Twenty-Five.
Node.js 24.18 and 24.19 bundle npm 11.16 and 11.17, which fail the new npm 11.18 minimum in devEngines. Node.js 24.20 bundles npm 11.19.
4020804 to
afa7760
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The npm 11.18+ restrictions and disabled native install scripts require clean-install and build validation on supported platforms, which is unavailable in this source-only review.
No open issues 🎉
3 files not reviewed
src/wp-content/themes/twentytwentyfive/package-lock.json: Generated filesrc/wp-content/themes/twentytwentyone/package-lock.json: Generated filesrc/wp-content/themes/twentytwentytwo/package-lock.json: Generated file
What changed in this PR
This PR adopts npm v12-style supply chain restrictions on npm 11 for WordPress Core and three bundled themes, replacing the deferred npm v12 upgrade.
Changes:
- Blocks Git, remote URL, and local tarball dependencies, and rejects unreviewed dependency install scripts.
- Explicitly denies install scripts for listed dependencies in Core and the three themes.
- Raises minimum requirements to Node.js 24.20.0 and npm 11.18.0, updating version selectors and lockfile metadata.
| File | Description |
|---|---|
src/wp-content/themes/twentytwentytwo/package.json |
Raises runtime requirements and denies the fsevents install script. |
src/wp-content/themes/twentytwentytwo/package-lock.json |
Synchronizes engine requirements. |
src/wp-content/themes/twentytwentytwo/.nvmrc |
Selects Node.js 24.20. |
src/wp-content/themes/twentytwentytwo/.npmrc |
Adds dependency-source and install-script restrictions. |
src/wp-content/themes/twentytwentyone/package.json |
Raises runtime requirements and denies listed dependency scripts. |
src/wp-content/themes/twentytwentyone/package-lock.json |
Synchronizes engine requirements. |
src/wp-content/themes/twentytwentyone/.nvmrc |
Selects Node.js 24.20. |
src/wp-content/themes/twentytwentyone/.npmrc |
Adds restrictions while preserving legacy peer-dependency handling. |
src/wp-content/themes/twentytwentyfive/package.json |
Raises runtime requirements and denies the fsevents install script. |
src/wp-content/themes/twentytwentyfive/package-lock.json |
Synchronizes engine requirements. |
src/wp-content/themes/twentytwentyfive/.nvmrc |
Selects Node.js 24.20. |
src/wp-content/themes/twentytwentyfive/.npmrc |
Adds dependency-source and install-script restrictions. |
package.json |
Raises Core’s runtime requirements and denies listed dependency scripts. |
package-lock.json |
Synchronizes Core’s engine requirements. |
.nvmrc |
Selects Node.js 24.20 for Core. |
.npmrc |
Adds Core’s dependency-source and install-script restrictions. |
Review effort: Balanced
|
@desrosj could you give this an independent review? The npm install policy and version requirements have passed local install/build checks, but I am holding landing for a second opinion on the supply-chain policy and its effect on contributors. AI review · |
|
@desrosj following up on my Oct 31 request: could you give this an independent review when you have time? It's approved and waiting only on your second opinion of the npm install policy. AI review · |
|
@desrosj I plan to land this on Wednesday, November 25, unless you'd like changes or more time before then. Since my approval, the only change is a trunk merge with the same effective diff, and CI is green. What it changes for contributors: the minimum moves to Node.js 24.20 and npm 11.18, and AI review · |
Opts in to the npm v12 supply chain defaults on npm 11 for Core and the bundled themes with a build process, mirroring WordPress/gutenberg#82689. This replaces the npm v12 upgrade in #13536, which is on hold.
.npmrcrefuses git, remote URL and local tarball dependencies, andstrict-allow-scriptsfails the install on unreviewed install scripts.allowScriptsdenies every dependency install script. Twenty Twenty-One, Twenty Twenty-Two and Twenty Twenty-Five get the same policy.>=11.18.0because npm 11.16 and 11.17 reject registry dependencies withEALLOWREMOTEunderallow-remote = none..nvmrcmoves to Node.js 24.20, which bundles npm 11.19, matching Chore: Use v24.20 in.nvmrcconfig file. gutenberg#84138.Trac ticket: https://core.trac.wordpress.org/ticket/64543
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Implementation and this description, reviewed by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.