Repository navigation
fix(release): strip unused npm and corepack from bundled node - #238
Merged
Merged
Conversation
The runner only executes externals/node*/bin/node. The npm and corepack shipped with Node 20 and 24 are never used, but they pin vulnerable dependencies (tar, brace-expansion, minimatch, undici, ...) that account for all High/Critical secscan findings in the release tarballs (ISD-6336). Strip them after layout in the fork-only release workflow, so no upstream source file changes and upstream merges stay conflict-free.
There was a problem hiding this comment.
🔵 Needs a closer look
The stated all-architecture CI verification does not run for pull requests.
0 open findings
What changed in this PR
Removes unused bundled npm and corepack files from Node release packages to eliminate vulnerable dependencies.
Changes:
- Deletes npm, npx, and corepack from bundled Node directories.
- Validates Node presence, removal, and runtime execution.
| File | Summary |
|---|---|
.github/workflows/release.yml |
Adds release-layout cleanup and validation; the check is not currently run in PR CI. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
release.yml only runs on release branches, so the strip and its checks never ran for pull requests. Move the logic to .github/scripts/strip-bundled-npm.sh and call it from both build.yml (PR CI, all 5 architectures) and release.yml.
find | grep -q under pipefail can fail with SIGPIPE after a match and skip the error. Use find -print -quit instead, and also check the npm/npx/corepack launchers in bin/.
The version check globbed bin/node and silently skipped Node directories without one. Require bin/node in every bundled Node, including alpine and linux-arm where it cannot be executed on the build host.
cbartz
marked this pull request as ready for review
October 9, 2026 13:53
minulo
approved these changes
Oct 9, 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
Removes the npm and corepack that come bundled with Node 20 and 24 in
externals/from the release packages. This clears all High/Critical secscan findings in the runner tarballs (ISD-6336, SEC0025).The runner only runs
externals/node*/bin/node, so these copies are never used. They pin vulnerable dependencies such as tar, brace-expansion, minimatch, undici and ip-address. Upstream v2.338.0 doesn't fix them: Node 20 is unchanged, and Node 24.21.0 still ships vulnerable tar, brace-expansion and undici.Changes
.github/scripts/strip-bundled-npm.sh: deleteslib/node_modules/{npm,corepack}andbin/{npm,npx,corepack}from everyexternals/node*. Bothbuild.yml(PR CI) andrelease.ymlrun it right after layout, so shipped packages and CI artifacts match.node --versionfor each bundled Node on hosts that can run them.src/changes, so upstream merges and the arch patches are unaffected.Verification
node24still runs JavaScript, and the step fails on an empty layout.build.yml): runs the script and the L0 tests on the stripped layout for all 5 architectures.Risk
A workflow that calls the runner's bundled npm by its full path would break. The bundled npm is not on the PATH of job steps, and
setup-nodeinstalls its own.AI-generated message.