Make WordPress Core

Opened 5 weeks ago

Closed 11 days ago

Last modified 11 days ago

#65864 closed enhancement (fixed)

Build/Test Tools: Avoid npx when running build and test tooling

Reported by: adrianmoldovanwp Owned by: johnbillion
Priority: normal Milestone: 7.2
Component: Build/Test Tools Version:
Severity: normal Keywords: has-patch has-unit-tests fixed-major
Cc: Focuses:

Description

npx downloads a package and its dependencies from the registry when it cannot find that package locally, then runs the install scripts of every one of them.
Each call is a point where a compromised package can run code during a build.

Build and test tooling invokes five packages through npx, all of them already known and pinnable:

File Command
.github/workflows/reusable-end-to-end-tests.yml npx playwright install
.github/workflows/reusable-performance-test-v2.yml npx playwright install
Gruntfile.js, qunit task npx playwright test
Gruntfile.js, wp-packages:update task npx wp-scripts
Gruntfile.js, browserslist:update task npx update-browserslist-db@latest

Replace each call with npm exec --no, which runs an installed binary and fails when the package is missing.

Change History (32)

This ticket was mentioned in PR #13021 on WordPress/wordpress-develop by @adrianmoldovanwp.


5 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Trac ticket: 65864

npx downloads a package and its dependencies from the registry when it cannot find that package locally, then runs the install scripts of everything it downloaded. Each call is a point where a compromised package can run code during a build.

This patch replaces every npx call in the repository with npm exec --no, which runs an installed binary and fails when the package is missing.
update-browserslist-db is now a devDependency at 1.3.1. browserslist already required it as ^1.2.3, so the lockfile hoists a package that was present and adds none.

## Testing instructions

  1. Run npm ci.
  2. Run npm exec --no -- playwright --version. It prints the installed version and downloads nothing.
  3. Delete node_modules/update-browserslist-db and run npm exec --no -- update-browserslist-db. It fails with npx canceled due to missing packages and no YES option instead of fetching the package. Restore the tree with npm ci.
  4. Run grunt qunit and confirm the QUnit tests still run.
  5. Confirm the end-to-end and performance workflows pass on this pull request.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Auditing the npx call sites. I reviewed and verified the result, including the npm exec --no behavior described above.

#2 @johnbillion
4 weeks ago

  • Milestone Awaiting Review7.2
  • Owner set to johnbillion
  • Status newreviewing

#3 @desrosj
4 weeks ago

  • Resolutionfixed
  • Status reviewingclosed

In 63309:

Build/Test Tools: Remove all npx commands.

When an npx command is run, the specified package will be downloaded and installed when it does not exist locally. This will also install all of the package’s direct and transitive dependencies, and any installation scripts present for every installed package are run. This is potentially dangerous because a compromised package would be able to run code on a local machine or within a GitHub Actions workflow runner.

This replaces every npx call with npm exec --no, which runs an installed binary only and will fail when the package is missing. Additionally, update-browserslist-db is now defined as a direct devDependency.

Props adrianmoldovanwp, desrosj, lancewillett, johnbillion.
Fixes #65864.

#4 @desrosj
4 weeks ago

  • Resolution fixed
  • Status closedreopened

#5 @desrosj
4 weeks ago

  • Keywords fixed-major added

Given that this is a Build/Test Tools change with security benefits, I think that it's worth exploring the level of effort required to backport this to old branches.

@desrosj commented on PR #13021:


4 weeks ago
#6

This was committed in r63309. This should have been closed automatically, but seems there is some kind of bug. I opened Core-65892 to look into this.

This ticket was mentioned in PR #13107 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#7

Backports r63309, already in trunk via #13021, to the 7.0 branch. It replaces the last two npx calls in Gruntfile.js with npm exec --no, which runs an installed binary instead of downloading and installing one.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Checking which hunks of r63309 apply to this branch, editing Gruntfile.js, and verifying that both binaries resolve locally.

This ticket was mentioned in PR #13108 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#8

Backport of r63309 to the 7.1 branch, already in trunk via #13021. It replaces every npx call with npm exec --no, which runs an installed binary and fails when the package is missing.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Applying the backport, confirming both files match r63309, and running the QUnit suite. I reviewed and tested the result.

This ticket was mentioned in PR #13109 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#9

Backport of r63309, already in trunk via #13021. It replaces the two remaining npx calls in Gruntfile.js with npm exec --no, which fails on a missing package instead of installing it.

This ticket was mentioned in PR #13110 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#10

Replaces the two remaining npx calls in Gruntfile.js with npm exec --no, and adds update-browserslist-db as a direct dev dependency. Backport of r63309, already in trunk via #13021.

This ticket was mentioned in PR #13111 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#11

Replaces the two remaining npx calls in Gruntfile.js with npm exec --no --, which runs a locally installed binary and fails instead of downloading one. Backport of r63309, already in trunk via #13021.

This ticket was mentioned in PR #13112 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#12

Backport of r63309 to the 6.6 branch, already in trunk via #13021: the wp-packages:update and browserslist:update Grunt tasks now call npm exec --no instead of npx, which installs a missing binary and runs its install scripts. It also adds update-browserslist-db to devDependencies, so browserslist:update does not rely on npm hoisting it out of browserslist.

This ticket was mentioned in PR #13113 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#13

Backport of r63309 to the 6.4 branch, already in trunk via #13021. It replaces the two remaining npx calls with npm exec --no, which runs an installed binary only instead of first downloading and installing a missing package and its dependencies.

This ticket was mentioned in PR #13121 on WordPress/wordpress-develop by @adrianmoldovanwp.


4 weeks ago
#14

Backport of r63309, already in trunk via #13021. It replaces the two remaining npx calls in Gruntfile.js with npm exec --no, which fails on a missing package instead of installing it, and declares update-browserslist-db as a direct devDependency.

#15 @lancewillett
13 days ago

In 63413:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 7.0 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13107

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#16 @lancewillett
13 days ago

In 63414:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 7.1 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13108

Reviewed by: desrosj.
Props adrianmoldovanwp, desrosj.
See #65864.

#19 @lancewillett
11 days ago

In 63428:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.9 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13110

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#21 @lancewillett
11 days ago

In 63429:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.8 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13109

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#23 @lancewillett
11 days ago

In 63430:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.7 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13121

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#25 @lancewillett
11 days ago

In 63431:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.6 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13112

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#27 @lancewillett
11 days ago

In 63432:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.5 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13111

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#29 @lancewillett
11 days ago

In 63433:

Build/Test Tools: Remove all npx commands.

Merges [63309] to the 6.4 branch.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13113

Reviewed by: desrosj.
Props adrianmoldovanwp.
See #65864.

#31 @lancewillett
11 days ago

  • Resolutionfixed
  • Status reopenedclosed

Committed to trunk in [63309].

Merged to the release branches:

All planned branch commits are complete.

Closing as fixed.

#32 @manzoorwani.jk
11 days ago

In Gutenberg, I had cleaned up much of the npx usage in these PRs, but there are still some uses there.

I will try to clean up the remaining usages.

Note: See TracTickets for help on using tickets.