fix(security): port the latest security fixes to v10#12300
Merged
Conversation
Manifest bin keys "", ".", "..", and scoped forms such as "@scope/.." passed the bin-name guard because encodeURIComponent leaves them unchanged. When joined to the global bin directory during global remove/update/add operations, "." resolves to the bin directory itself and ".." to its parent, which removeBin then recursively deletes. Reject empty, ".", and ".." bin names after scope stripping. Backport of #12289 to v10.
Makes environment expansion trust-aware for registry/auth config and
request destinations:
- Stops project and workspace .npmrc files from expanding ${...}
placeholders in registry/proxy request destinations, URL-scoped keys,
and registry credential values.
- Stops repository-controlled pnpm-workspace.yaml from expanding
${...} placeholders in the registry setting.
- Preserves env expansion for trusted user/global/CLI/env config so
existing token and registry setup flows continue to work.
Backport of #12291 (CAND-PNPM-122 / GHSA-3qhv-2rgh-x77r) to v10.
…e-manager binary The packageManager field (and pnpm self-update) makes pnpm download and run a specific pnpm version. The staged install's bytes were trusted based on lockfile integrity alone, which proves nothing when the inputs are repository-controlled. pnpm now verifies the npm registry signature of the engine it is about to spawn, over the installed integrity, against npm's public signing keys embedded in the pnpm CLI (exactly as corepack does): - verifyPnpmEngineIdentity() checks pnpm/@pnpm/exe and the materialized platform binaries of the staged install before it is linked into the tools directory. - Fails closed: any verification failure, including an unreachable registry, refuses the version switch rather than running an unverified binary. Runs only on a tools-directory cache miss (an actual download). - The embedded keys live in a generated file kept in sync with npm's keys endpoint by scripts/update-npm-signing-keys.mjs; the release workflow runs the check as a gate so a key rotation cannot silently break verification. Backport of #12292 (CAND-PNPM-097) to v10.
Resolve package-manager bootstrap traffic through trusted user/CLI registries and trusted network config, defaulting to the public npm registry instead of project/workspace registry settings: - getConfig() now computes packageManagerRegistries and packageManagerNetworkConfig from trusted config sources only (CLI options, env config, user and global .npmrc) — never the repository's project/workspace .npmrc or pnpm-workspace.yaml. - switchCliVersion() applies that bootstrap config when installing and verifying the wanted pnpm version, so repository .npmrc proxy/TLS/registry values cannot steer package-manager bootstrap traffic. Backport of #12296 to v10. The v11 env-lockfile validation parts do not apply: v10 bootstraps the wanted version through a staged child install instead of an env lockfile.
When a repository requests a Node.js runtime (useNodeVersion or an execution env), pnpm downloads and then executes a Node binary. The download mirror is repository-configurable via node-mirror:<channel> in project .npmrc, and the integrity came from SHASUMS256.txt fetched from that same mirror — a circular check a malicious mirror can satisfy with a tampered binary and matching hashes. pnpm now fetches SHASUMS256.txt.sig and verifies its detached OpenPGP signature against the Node.js release team's public keys, embedded in the pnpm CLI, before trusting the hashes: - @pnpm/crypto.shasums-file: new fetchVerifiedNodeShasums / fetchVerifiedNodeShasumsFile verify the signature via openpgp against the embedded keys (generated src/nodeReleaseKeys.ts, mirrored from the canonical nodejs/release-keys list). - @pnpm/node.fetcher verifies the configurable-mirror SHASUMS for the release channel; pre-release channels (rc, nightly, ...) are unsigned by Node and remain unverified. - scripts/update-node-release-keys.mjs keeps the keys current (pnpm run check:node-release-keys / update:node-release-keys), and the release workflow runs the check as a gate. Backport of #12295 to v10 (without the pacquet Rust port, which does not exist on this branch).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review by Qodo
1.
|
PR Summary by QodoBackport v10 security hardening for config trust boundaries and binary verification WalkthroughsDescription• Block ${...} env expansion from repo-controlled config in request URLs and credentials.
• Verify signatures before executing downloaded pnpm engines and Node.js runtimes.
• Harden package-manager bootstrap to use only trusted registry/proxy/TLS settings.
Diagramgraph TD
CLI["pnpm CLI"] --> CFG["getConfig()"] --> SW["switchCliVersion"] --> UPD["self-updater"] --> VER["verify engine"] --> NPM{{"npm registry"}}
CFG --> DROP["drop untrusted env"]
CFG --> NODE["node.fetcher"] --> SHAS["verify SHASUMS"] --> MIR{{"Node mirror"}}
KEYS[("Embedded keys")] --> VER --> NPM
KEYS --> SHAS --> MIR
subgraph Legend
direction LR
_mod["Module"] ~~~ _data[("Embedded data")] ~~~ _ext{{"External"}}
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Centralize verification in a shared workspace package
2. Soft-fail on unreachable registries (warn and continue)
3. Defer to external tools (e.g., Corepack) for engine verification
Recommendation: Keep the PR’s current approach: enforce trust boundaries at config load time and verify binaries at the v10 self-updater staging chokepoint with fail-closed semantics. This minimizes execution of repo-influenced unverified code while keeping performance acceptable (verification only on cache misses) and adds CI gates to prevent key drift. File ChangesEnhancement (8)
Bug fix (6)
Refactor (4)
Tests (6)
Documentation (5)
Other (10)
|
The Node.js download tests exercise the release channel, whose SHASUMS256.txt is now signature-verified. Sign the fixture with a generated OpenPGP key and trust it through the new trustedNodeReleaseKeys test seam (threaded from plugin-commands-env via @pnpm/node.fetcher to fetchVerifiedNodeShasums), so the tests keep exercising the verification path instead of bypassing it.
|
Code review by qodo was updated up to the latest commit d7ec2cd |
Member
Author
|
CI failed in Fixed in d7ec2cd by signing the SHASUMS fixture with a generated OpenPGP key and trusting it through a new Written by an agent (Claude Code, claude-fable-5). |
…rrors Registry URLs may legally embed basic-auth credentials (https://user:pass@host/). verifyPnpmEngineIdentity() interpolated the packument URL and registry URL into PnpmError messages, and the unreachable-registry path surfaced fetch-layer error messages that embed the request URL — all of which land in terminal output and CI logs. Strip URL credentials from every error message and truncate the non-200 response body.
|
Code review by qodo was updated up to the latest commit 99e5517 |
Override shell-quote to >=1.8.4 (GHSA-w7jw-789q-3m8p, critical, pulled in via concurrently) so the audit workflow passes again. The advisory was published after the last release/10 audit run; it is unrelated to the security backports on this branch.
|
Code review by qodo was updated up to the latest commit 2239a3a |
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
1 participant
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.
Ports the following security fixes from
maintorelease/10:fix(package-bins): reject reserved manifest bin namesfix: block untrusted request destination env expansionfix(security): verify npm registry signature before spawning a package-manager binaryfix: harden package-manager bootstrap metadatafix(security): verify Node.js runtime SHASUMS OpenPGP signatureThe v10 codebase diverges substantially from
main, so these are ports, not cherry-picks. Notable adaptations:Reject reserved manifest bin names (#12289)
bins/resolverdoes not exist on v10; the same guard lands inpkg-manager/package-bins(the code is otherwise identical).global/commandse2e test from main has no v10 home; the resolver-level test covers the guard.Block untrusted request destination env expansion (#12291, CAND-PNPM-122 / GHSA-3qhv-2rgh-x77r)
.npmrcthrough@pnpm/npm-conf, which expands${...}during parsing, so the trust boundary is enforced by post-processing theprojectandworkspacenpm-conf source layers: the raw file is re-read without expansion and any setting whose raw form uses a placeholder in a request destination (registry/proxy URLs, URL-scoped keys) or in a credential value is dropped from that layer with a warning (dropUntrustedEnvExpansions.ts).pnpm-workspace.yamlside only needs theregistrykey on v10 (registries/namedRegistries/pnprServerare v11 features), and v10 has no trusted-yaml caller, so noexpandRequestDestinationEnvflag is threaded — expansion ofregistryvalues is simply blocked.Verify npm registry signature before spawning a package-manager binary (#12292, CAND-PNPM-097)
@pnpm/deps.security.signaturespackage and no env-lockfile machinery; the verification lives in@pnpm/tools.plugin-commands-self-updater, which is the single chokepoint for both the automaticpackageManagerversion switch andpnpm self-update.pnpm addinstall is verified before it is linked into the tools directory:pnpm/@pnpm/exeplus every platform binary materialized on disk must carry a valid npm registry signature over the integrity recorded in the staged lockfile, validated against npm's signing keys embedded in the CLI (corepack-style). Fails closed, including on an unreachable registry; runs only on a tools-directory cache miss.tools/plugin-commands-self-updater/scripts/update-npm-signing-keys.mjs(pnpm run check:npm-signing-keys); v10 has no create-release-pr workflow, so the check gatesrelease.ymlinstead.Harden package-manager bootstrap metadata (#12296)
getConfig()now computespackageManagerRegistries/packageManagerNetworkConfigfrom trusted config layers only (CLI, env, user and global.npmrc— never the repository's project/workspace.npmrcorpnpm-workspace.yaml), defaulting to the public npm registry.switchCliVersion()applies them to the version-switch install + verification.packageManagerLockfile.ts,syncEnvLockfile, peer-suffix handling) do not apply: v10 bootstraps through a staged child install, which already runs outside the repository's config context.Verify Node.js runtime SHASUMS OpenPGP signature (#12295)
@pnpm/crypto.shasums-file(fetchVerifiedNodeShasums, embeddednodeReleaseKeys.ts, update script, release gate —pnpm run check:node-release-keys).@pnpm/node.fetcher(main'sengine/runtime/node-resolverdoesn't exist here):releaseChannelis threaded from@pnpm/plugin-commands-envand verification is gated on thereleasechannel, matching main (pre-release channels are unsigned by Node).Testing
pkg-manager/package-bins,config/config(npmrc + workspace-yaml trust boundaries, bootstrap registries),tools/plugin-commands-self-updater(9 signature-verification tests), andcrypto/shasums-file(4 OpenPGP tests, ported from main).check:npm-signing-keys,check:node-release-keysboth pass).Written by an agent (Claude Code, claude-fable-5).