feat(runtime): verify SHASUMS256.txt PGP signature before installing Node#1848
feat(runtime): verify SHASUMS256.txt PGP signature before installing Node#1848fengmk2 wants to merge 7 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 canceled.
|
|
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.
|
@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.
|
@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.
|
@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.
|
@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).
|
@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". |
1 participant
Problem
vite_js_runtimedownloaded the plainSHASUMS256.txtand trusted its checksums unconditionally. An attacker who could tamper with that file could also supply a matching malicious archive, so the checksum gave no real protection.Fix
Download the clearsigned
SHASUMS256.txt.ascand verify its PGP signature against the Node.js release keys before trusting any checksum. The chain is now: download.asc-> verify signature -> parse verified plaintext -> check archive SHA-256 -> extract.pgp_verifymodule (rPGP, pure Rust) with the Node release keyring vendored fromnodejs/release-keys. Each releaser's primary key and subkeys are tried, since releasers sign with subkeys.pgpis added withdefault-features = false(its defaultbzip2feature pulls a C dep that breaks musl/Windows cross-builds; verification needs no compression).SHASUMS256.txt.Tests
Unit tests cover a genuine fixture verifying, tampered content rejected, untrusted/empty keyring rejected, and all vendored keys parsing. The Node download integration test exercises the full verify-then-extract path.
just checkandjust lintpass.Closes #1807