feat(runtime): verify Node.js downloads with PGP signatures#1848
Open
fengmk2 wants to merge 24 commits into
Open
feat(runtime): verify Node.js downloads with PGP signatures#1848fengmk2 wants to merge 24 commits into
fengmk2 wants to merge 24 commits into
Conversation
…Node Download the clearsigned SHASUMS256.txt.asc and verify it against the embedded Node.js release keys before trusting any checksum, so a tampered SHASUMS file cannot pass off a malicious archive whose hash it controls. Falls back to the plain SHASUMS256.txt for musl unofficial builds, which publish no signature. Closes #1807
✅ Deploy Preview for viteplus-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
The base64 armor in the vendored Node release keys trips crate-ci/typos; exclude the .asc assets and fix the real "unparseable" typo in code.
- Scope the ShasumsSignature import to non-musl so musl builds don't emit an unused-import warning (the symbol is only referenced under the same cfg). - Return String from verify_signed_shasums instead of Str so the caller can borrow it directly for parse_shasums, dropping two needless allocations and simplifying the error mapping to map_err.
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0a3fad1dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ature Requiring SHASUMS256.txt.asc on every non-musl source regressed VP_NODE_DIST_MIRROR users whose mirror ships only the archives plus the plain SHASUMS256.txt. Make signature verification mandatory only for the official nodejs.org source; for custom mirrors it is best-effort, falling back to the plain SHASUMS when the .asc is unavailable. A downloaded but invalid signature still fails everywhere.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48df9085b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address two Codex review findings: - pgp_verify: a raw cryptographic match no longer suffices. Reject revoked signing keys, and require the signature's creation time to fall within the signing key/subkey validity window. This mirrors gpgv and stops a compromised, long-expired release key from signing a fresh SHASUMS for a current release, while genuine old signatures made when the key was valid still verify. - node: base whether the signature is required on the resolved host (nodejs.org) rather than merely whether VP_NODE_DIST_MIRROR is set, so a mirror pointed back at the official host still requires verification.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed93d29e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address two Codex review findings on the PGP key-policy checks: - Subkey signing path now rejects non-signing or revoked subkeys: a matched subkey must carry a signing-capable binding signature (with a valid embedded primary-key back-signature) and must not be revoked, so a leaked encryption-only or revoked subkey secret can no longer pass the release- signing check. - Expiry is now evaluated against the self/binding signature effective at the release signature's creation time, not the loosest expiration across all self-signatures, matching gpgv when a key's expiry changes over time. Adds a regression fixture (v18.14.0, signed by a since-expired key) proving a genuine old signature still verifies.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c967de5572
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address Codex review findings: - Primary-key path now only lets the key's own self-issued certifications and direct signatures define its policy (third-party certifications are ignored), and rejects a primary marked certify-only, matching the rigor of the subkey path. - Document the two intentional limitations of the gpgv-style curated-keyring model: expiry is checked against the signature's self-asserted creation time (so it is defense-in-depth, not protection against a leaked key that backdates a forgery; revocation plus the curated keyring is the real boundary), and the vendored keyring is a snapshot that must be refreshed as Node's releaser set changes (a brand-new releaser's release fails closed until then).
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The Round-3 change selected the self-signature "effective at the release signature time" (created on or before it), but a release key is often re-certified later (e.g. to extend its expiry). For such keys the only self-signature in the vendored keyring postdates older release signatures, so no self-signature was found and verification rejected legitimate releases (e.g. node-v20.18.0, whose signing key was re-certified after the release). Use the current (latest) self/binding signature to determine the effective expiry, as gpgv does. Adds a regression fixture for the re-certified-key case.
Enforcing key expiry diverged from the reference (gpgv) and rejected legitimate Node releases: gpgv reports such signatures good (exit 0) even when made after the signing key's expiry (e.g. node-v16.20.0, signed days after its key expired), and when the key was re-certified after the release (node-v20.18.0). The earlier expiry checks broke `vp env exec`/install and the CLI snap tests for those versions. Verification now mirrors gpgv against the curated release keyring: accept a cryptographically valid signature from a trusted, non-revoked key; a subkey signature additionally requires a signing-capable, validly-bound subkey so a leaked encryption-only subkey cannot sign. Expiry is not enforced (it is advisory in gpgv and is defeatable by backdating anyway); revocation and the curated keyring remain the trust boundary. Adds regression fixtures for the expired-at-signing and re-certified-key releases.
Document the design, gpgv-matching verification policy, mirror handling, and the intentional trust-model limitations (expiry not enforced, vendored keyring currency, rsa advisory).
Add a weekly workflow (update-node-release-keys.yml) that regenerates the embedded Node.js release keyring from nodejs/release-keys via a reproducible script and opens a PR for human review. The PR body includes a before/after diff of added/removed/modified keys (by fingerprint and user id). The keyring is never auto-merged, since it is the trust anchor for SHASUMS verification. Also relax the block-count test to be self-consistent with the vendored file (keeping a >=28 floor) so adding keys upstream needs no test edit, document the automation in the RFC, and allow the "fpr" gpg colon-record term in typos.
- update-node-release-keys.sh: decode gpg \xHH-escaped uids in the PR body (so e.g. "http\x3a//" renders as "http://"), clean up the temporary gpg homedirs instead of leaking them, and normalize CRLF/CR to LF before concatenation for deterministic output. - is_official_dist_host: strip userinfo, port, and a trailing dot so an official nodejs.org URL reached through those forms still requires the signature; add look-alike negative tests. - Document that primary_key_revoked honors self-revocations (Node release keys use no designated revokers).
fengmk2
added
test: e2e
- Extract resolve_shasums_content() to flatten the nested match in download_runtime_with_provider: an early return for the no-signature case and a single match (verify / propagate-if-required / warn-and-fallback), with identical behavior. - Remove the embedded_keys_parse test, which is subsumed by every_vendored_key_parses plus the >=28 floor in split_armored_blocks_finds_every_key.
Update HashVerification (signature/ShasumsSignature), the integrity section (now PGP-verifies the clearsigned SHASUMS256.txt.asc against the Node release keys, with mandatory/best-effort/musl-fallback behavior), the download-flow diagram, and the success criteria. Cross-link verify-node-shasums-signature.md.
fengmk2
changed the title
The unofficial musl builds, and custom mirrors that publish only SHASUMS256.txt, have no PGP signature, so verification falls back to the SHA-256 checksum. Emit a user-facing warning on that path so users know the download was not signature-verified. The previous tracing::warn was invisible unless VITE_LOG was set, and the musl (no-signature) path warned nothing at all.
fengmk2
commented
Jun 16, 2026
…p in CI Print the no-signature/checksum-only warning via the shared vite_shared::output::warn helper (consistent styled "warn:" output) instead of a raw eprintln, and suppress it when running in CI where it is just noise.
The no-signature path (unofficial musl builds, or a mirror that publishes only SHASUMS256.txt) is an expected, non-actionable condition for those sources, not an anomaly: on musl it would fire on every install. Record it with tracing::debug! (visible via VITE_LOG) instead of a user-facing warning, which also makes the CI suppression unnecessary.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Add a "Prior art" section to the SHASUMS signature RFC comparing download verification across nvm, fnm, Volta, asdf-nodejs, mise, and nodenv. Most stop at SHA-256 checksums (or, for fnm, HTTPS only); only asdf-nodejs and mise also verify the PGP signature, and both require an external gpg. Documents why a built-in, dependency-free verifier justifies bundling rPGP and the keyring.
Allow skipping PGP signature verification of SHASUMS256.txt via the VP_NODE_SKIP_SIGNATURE_VERIFY env var, so a future keyring or certificate issue can be temporarily bypassed without blocking installs. The SHA-256 checksum is still verified (integrity preserved, only authenticity dropped), and a warning is printed on every skipped install. Mirrors asdf's NODEJS_CHECK_SIGNATURES and mise's signature opt-out; env-var only, no config or CI flag, so the secure path stays the unconditional default.
Contributor
|
✅ Staging deployment successful! Preview: https://viteplus-staging.void.app/ |
…ry fix TEMP verification only: point at fengmk2/security-action@a2a9a4b (head of oxc-project/security-action#22, which ignores RUSTSEC-2023-0071). Revert to the upstream oxc-project pin once #22 is merged and released.
Reverts the temporary fork pin used to verify oxc-project/security-action#22. Re-pin to a released oxc-project version that includes the RUSTSEC-2023-0071 ignore once #22 is merged and tagged.
cpojer
approved these changes
Jun 19, 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
3 participants
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.
When
vpinstalls a managed Node.js version, it now verifies the release's PGP signature (the clearsignedSHASUMS256.txt.asc) against Node's release keys before trusting any checksum. Previously the plainSHASUMS256.txtwas trusted as-is, so anyone able to tamper with it could swap in a matching malicious archive.This is the same guarantee as
gpgvagainst the official keyring, built in (pure Rust, nogpgrequired). The keyring is vendored fromnodejs/release-keysand kept current by a weekly workflow.Example output
A normal install is unchanged (verification is transparent):
A tampered or untrusted SHASUMS is rejected before anything is installed:
The official
nodejs.orgsource always requires a valid signature. The unofficial musl builds, and custom mirrors that publish onlySHASUMS256.txt, have no signature to verify, so they fall back to checksum-only verification. That is an expected, documented condition for those sources, so it is recorded in the debug log (VITE_LOG) rather than warning on every install.Binary size
The
.nodeNAPI binding (vite-plus-cli) is unaffected: no size change.pgpand the vendored keyring are reachable only throughvite_js_runtime, which is linked into the globalvpbinary, not into the binding (vite_install/vite_pm_cli). Only the globalvpbinary grows, by ~1.2 MiB (rPGP plus the RustCrypto verification code and the embedded release keyring).Closes #1807