Skip to content

feat: add get-pnpm, the installer as an npm package - #51

Merged
zkochan merged 4 commits into
mainfrom
move-get-pnpm
Aug 9, 2026
Merged

zkochan merged 4 commits into
mainfrom
move-get-pnpm

Conversation

@zkochan

@zkochan zkochan commented Aug 8, 2026 •

Copy link
Copy Markdown
Member

Summary

Moves get-pnpm — the installer as an npm package, npx get-pnpm — out of the pnpm monorepo and in here, beside the two installers it duplicates in miniature.

It was built in pnpm/pnpm#13732, which is the wrong home for it: no workspace dependencies, a release cadence unrelated to pnpm's, and it needed a meta-updater exception to stop its version being normalised to the 11xx scheme. Here, the things all three installers must agree on live in one place — the pinned npm signing key, the platform package names, the refusal messages.

npx get-pnpm            # the latest version
npx get-pnpm 12         # a major
npx get-pnpm next-12    # a dist-tag
npx get-pnpm 11.20.0    # an exact version

Shape

  • No dependencies. libc detection is the probe pnpm's own wrappers use rather than detect-libc — every dependency is one more thing npx fetches before pnpm exists.
  • TypeScript compiled to lib/, tests via node --test, so the repo gains a compiler and nothing else.
  • Same verification as install.sh: npm's signature over the checksum, checked against a pinned key, then the download against that checksum.

Keeping the key current

scripts/update-npm-keys.mjs rewrites the pinned key in all three installers and exits non-zero if any has drifted. A weekly workflow runs the check and opens a PR when npm rotates.

This replaces the mirroring I had added to pnpm's own update-npm-signing-keys.mjs, which reached across repositories. Self-contained is better: one job, one repo, and a rotation can't reach two installers and miss the third.

I rehearsed a rotation rather than assuming it works — pointing all three at a stale keyid, then:

✗ Pinned npm signing keys are out of date:
  - npm/src/npmSigningKeys.ts
  - install.sh
  - install.ps1

$ node scripts/update-npm-keys.mjs --update
updated npm/src/npmSigningKeys.ts
updated install.sh
updated install.ps1

The shell and PowerShell installers carry only the current key; the package keeps the union, so packages published before a rotation still verify.

Testing

22 tests. The verifier against a generated key pair (valid, wrong checksum, wrong package, untrusted key, unpinned keyid, unsigned, expired, not-yet-expired), version resolution, platform naming, and an end-to-end suite against a mock registry covering the happy path plus every refusal — tampered tarball, a checksum the signature doesn't cover, unsigned, unpinned keyid — asserting each leaves nothing installed.

Real installs against the live registry: next-12 → 12.0.0-rc.1, 11.20.0 → 11.20.0, with @reflink/reflink present in the v11 install (its absence silently downgrades copy-on-write cloning to hard linking).

Before this can publish

get-pnpm has never been published from CI. The first release needs either a trusted publisher configured for this repo on npmjs.com, or an NPM_TOKEN secret — I didn't add a publish workflow rather than guess which you want. The name is already yours; the placeholder 0.0.0 from 2018 is what's on the registry today.

Follow-ups

  • pnpm/pnpm#13732 should shrink to just the pnpm setup "type": "module" fix, which is a genuine CLI fix and belongs there.
  • pnpm/setup#24 duplicates the verifier; once this publishes, it can depend on a downloadPnpm export instead — verify and place, without the pnpm setup step an action must not run.

Written by an agent (Claude Code, claude-opus-5).

Summary by CodeRabbit

  • New Features
    • Added the get-pnpm command-line tool for installing selected pnpm versions across supported platforms.
    • Supports version selection, custom registries, platform detection, and automated setup.
    • Verifies downloaded packages using checksums and npm signing keys.
  • Documentation
    • Added usage guidance, configuration options, verification details, and alternative installation methods.
  • Tests
    • Added coverage for installation, version resolution, platform compatibility, downloads, and signature validation.
  • Chores
    • Added automated cross-platform testing and scheduled npm signing-key maintenance.

@vercel

vercel Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
get-pnpm-io Ready Ready Preview Aug 9, 2026 12:30pm

Request Review

@netlify

netlify Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for getpnpm ready!

Name Link
🔨 Latest commit c64af8c
🔍 Latest deploy log https://app.netlify.com/projects/getpnpm/deploys/6a7872f9ecd7d600081ff054
😎 Deploy Preview https://deploy-preview-51--getpnpm.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@zkochan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 109751be-6f64-4c69-b31f-2ab435645fbb

📥 Commits

Reviewing files that changed from the base of the PR and between d4c1caf and c64af8c.

⛔ Files ignored due to path filters (1)
  • get-pnpm/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (17)
  • .github/workflows/get-pnpm.yml
  • .github/workflows/npm-keys.yml
  • .gitignore
  • get-pnpm/README.md
  • get-pnpm/bin/get-pnpm.js
  • get-pnpm/package.json
  • get-pnpm/src/extractTarball.ts
  • get-pnpm/src/index.ts
  • get-pnpm/src/npmSigningKeys.ts
  • get-pnpm/src/platformPackageName.ts
  • get-pnpm/src/registry.ts
  • get-pnpm/src/resolveVersion.ts
  • get-pnpm/src/verifySignature.ts
  • get-pnpm/test/install.test.ts
  • get-pnpm/test/unit.test.ts
  • get-pnpm/tsconfig.json
  • scripts/update-npm-keys.mjs
📝 Walkthrough

Walkthrough

Adds a standalone TypeScript get-pnpm package. The package resolves versions and platforms, downloads npm packages, verifies checksums and signatures, assembles pnpm installations, runs tests, and automates signing-key maintenance.

Changes

get-pnpm package

Layer / File(s) Summary
Package contracts and entry points
npm/package.json, npm/tsconfig.json, npm/bin/get-pnpm.js, npm/README.md, .gitignore, .github/workflows/npm-package.yml
Defines the package metadata, build and test commands, CLI binary, documentation, ignored output, and multi-platform test workflow.
Version and platform resolution
npm/src/resolveVersion.ts, npm/src/platformPackageName.ts, npm/test/unit.test.ts
Resolves version selectors and platform package names. Tests cover tags, architectures, operating systems, musl detection, and Intel macOS handling.
Registry integrity and signature verification
npm/src/registry.ts, npm/src/verifySignature.ts, npm/src/npmSigningKeys.ts, npm/test/unit.test.ts
Fetches registry metadata and tarballs. Validates checksums and npm signatures against pinned keys. Unit tests cover valid and invalid verification cases.
CLI installation orchestration
npm/src/index.ts, npm/src/extractTarball.ts, npm/test/install.test.ts
Parses CLI options, downloads and extracts verified packages, creates legacy compatibility metadata, runs pnpm setup --force, cleans temporary files, and validates the flow with integration tests.
Signing-key maintenance automation
scripts/update-npm-keys.mjs, .github/workflows/npm-keys.yml
Synchronizes signing keys across installer files and opens an update pull request when scheduled or manual checks detect changes.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • pnpm/get.pnpm.io#50: Adds related npm registry signature and integrity verification for pnpm downloads.

Poem

A rabbit checks each key in line,
Then hops through tarballs, crisp and fine.
With signatures bright and checksums true,
pnpm finds its home anew.
“Run setup!” sings the bunny crew.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes adding get-pnpm as an npm package, which is the main change in the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch move-get-pnpm

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add get-pnpm npm package installer with signature verification and key rotation automation

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add npx get-pnpm TypeScript package to install standalone pnpm executables via npm registry.
• Verify downloads via pinned npm signing keys and registry signatures before running pnpm setup.
• Add CI and weekly automation to refresh pinned npm signing keys across installers.
Diagram

graph TD
  U["User (npx)"] --> CLI["get-pnpm CLI"] --> REG[("npm registry")] --> VD["Fetch + verify"] --> ASM["Assemble temp install"] --> SETUP["Run pnpm setup"]
  KEYS["Pinned signing keys"] --> VD --> ASM
  WF["Key refresh workflow"] --> KEYS
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use `pacote`/npm libraries for fetching and extraction
  • ➕ Less bespoke HTTP/tar handling; reuse well-tested npm tooling
  • ➕ Better handling of edge cases around registries and tarballs
  • ➖ Adds dependencies, increasing npx cold-start cost before pnpm exists
  • ➖ Brings in transitive supply-chain surface area to the installer itself
2. Pure-JS tar extraction (no `tar` shell-out)
  • ➕ Avoids reliance on host tar presence/behavior
  • ➕ More controllable behavior across platforms
  • ➖ Requires adding a tar library dependency or substantial custom code
  • ➖ Larger download for npx get-pnpm, contradicting the dependency-free goal
3. Delegate verification to npm CLI (`npm view`/`npm pack`)
  • ➕ Leverages npm’s own registry interactions and integrity mechanisms
  • ➕ Reduces custom registry protocol code
  • ➖ Assumes npm behavior/versioning; still requires network and may not expose the same signature semantics
  • ➖ More moving parts and less deterministic verification + error messaging

Recommendation: Keep the current dependency-free approach: it aligns with the stated goal (minimize what npx must fetch before pnpm exists) while still performing strong verification (pinned key + signature over @: + checksum check). The weekly key refresh workflow is the right operational complement to a pinned-key model.

Files changed (18) +1147 / -0

Enhancement (8) +507 / -0
get-pnpm.jsAdd Node.js CLI entrypoint for get-pnpm +9/-0

Add Node.js CLI entrypoint for get-pnpm

• Creates the executable bin script that calls 'runCli()' from the compiled library and maps failures to exit code 1 with a concise error message.

npm/bin/get-pnpm.js

extractTarball.tsImplement tarball extraction via host 'tar' +24/-0

Implement tarball extraction via host 'tar'

• Adds a small extraction utility that shells out to 'tar -xzf' for cross-platform unpacking without adding dependencies. Provides explicit errors for missing 'tar' or extraction failures.

npm/src/extractTarball.ts

index.tsImplement installer flow: resolve, verify, assemble, run setup +193/-0

Implement installer flow: resolve, verify, assemble, run setup

• Implements 'runCli()' argument parsing/help, and 'installPnpm()' orchestration: resolve version, compute platform package name, download and verify tarballs/signatures, assemble the directory expected by 'pnpm setup', and execute 'pnpm setup --force'. Ensures temp directory cleanup on completion and on termination signals.

npm/src/index.ts

npmSigningKeys.tsAdd generated pinned npm registry signing keys module +29/-0

Add generated pinned npm registry signing keys module

• Introduces a generated module containing the pinned npm registry signing keys mirrored from npm’s keys endpoint. Serves as the default trust store for registry signature verification.

npm/src/npmSigningKeys.ts

platformPackageName.tsCompute platform-specific pnpm binary package name +71/-0

Compute platform-specific pnpm binary package name

• Encodes the mapping from host platform/arch/libc to the correct pnpm binary package name, including the v12 naming scheme and legacy v11/v10 scheme. Adds refusal messaging for unsupported targets and the v11 Intel macOS gap, plus a dependency-free musl probe.

npm/src/platformPackageName.ts

registry.tsAdd npm registry client for packument/meta fetch and checksummed downloads +90/-0

Add npm registry client for packument/meta fetch and checksummed downloads

• Implements registry URL selection from environment, fetches packument/version metadata, and downloads tarballs while verifying integrity/shasum during streaming. Provides clearer error messages for network and HTTP failures.

npm/src/registry.ts

resolveVersion.tsResolve user version spec to concrete pnpm version +30/-0

Resolve user version spec to concrete pnpm version

• Adds logic to resolve a dist-tag, exact version (optional leading 'v'), or bare major (preferring stable then prerelease lane). Produces a helpful error listing available tags on mismatch.

npm/src/resolveVersion.ts

verifySignature.tsVerify npm registry signatures using pinned keys +61/-0

Verify npm registry signatures using pinned keys

• Implements signature validation for '<pkg>@<version>:<integrity>' using a pinned set of npm signing keys, including keyid pinning and expiry checks. Refuses unsigned packages, unexpected keyids, expired keys, and invalid signatures.

npm/src/verifySignature.ts

Tests (2) +274 / -0
install.test.tsAdd end-to-end installer tests with mock registry +150/-0

Add end-to-end installer tests with mock registry

• Adds a POSIX-only end-to-end suite using a local HTTP server that simulates registry responses and tarball serving. Covers happy path, cleanup behavior, and refusal cases for tampered tarballs, mismatched signature coverage, unsigned packages, and unpinned keyids.

npm/test/install.test.ts

unit.test.tsAdd unit tests for version resolution, platform naming, and signature checks +124/-0

Add unit tests for version resolution, platform naming, and signature checks

• Adds unit tests for 'resolveVersion', 'platformPackageName' edge cases, and 'verifyRegistrySignature' behavior (wrong checksum/package, untrusted key, missing signature, key expiry). Uses generated key pairs for deterministic signature tests.

npm/test/unit.test.ts

Documentation (1) +49 / -0
README.mdDocument get-pnpm usage and security model +49/-0

Document get-pnpm usage and security model

• Adds package documentation describing 'npx get-pnpm' usage, version spec formats, and the verification steps. Documents key environment variables and positions the package alongside the shell/PowerShell installers.

npm/README.md

Other (7) +317 / -0
npm-keys.ymlAdd scheduled npm signing key refresh workflow +45/-0

Add scheduled npm signing key refresh workflow

• Adds a weekly (and manual) GitHub Actions workflow that checks pinned npm signing keys and, on drift, updates them and opens a PR. Ensures key rotations surface as automated PRs rather than user-facing installer failures.

.github/workflows/npm-keys.yml

npm-package.ymlAdd CI to build and test get-pnpm package on 3 OSes +33/-0

Add CI to build and test get-pnpm package on 3 OSes

• Introduces a workflow that runs 'npm ci'/'npm install' and 'npm test' for the 'npm/' package on Linux, macOS, and Windows. Uses Node 24 and isolates runs to the 'npm' working directory.

.github/workflows/npm-package.yml

.gitignoreIgnore get-pnpm build output and node_modules +2/-0

Ignore get-pnpm build output and node_modules

• Updates '.gitignore' to exclude 'npm/lib' and 'npm/node_modules' from version control. Keeps compiled TypeScript output and dependencies out of diffs.

.gitignore

package-lock.jsonAdd lockfile for TypeScript build/test tooling +57/-0

Add lockfile for TypeScript build/test tooling

• Adds a lockfile capturing dev-only dependencies ('typescript', '@types/node') for deterministic installs in CI and development.

npm/package-lock.json

package.jsonIntroduce get-pnpm package manifest and scripts +43/-0

Introduce get-pnpm package manifest and scripts

• Defines the 'get-pnpm' package as ESM, configures bin/main/exports, and adds build/test/prepublish scripts. Declares only devDependencies (TypeScript + Node types) and sets a Node >=22.13 engine requirement.

npm/package.json

tsconfig.jsonAdd TypeScript config for ESM build output to lib/ +17/-0

Add TypeScript config for ESM build output to lib/

• Configures a strict NodeNext TypeScript build emitting JS and declarations into 'npm/lib'. Targets ES2023 and keeps the compile surface limited to 'src/**/*.ts'.

npm/tsconfig.json

update-npm-keys.mjsAdd script to sync pinned npm signing keys across installers +120/-0

Add script to sync pinned npm signing keys across installers

• Adds a Node script that fetches npm’s current signing keys, compares against pinned copies, and either fails (check mode) or rewrites all pinned-key locations (update mode). Updates the union of keys for the npm package while keeping shell/PowerShell installers pinned to the current key to match their verification goals.

scripts/update-npm-keys.mjs

`npx get-pnpm` installs pnpm the way install.sh does, for people who
have Node.js and would rather not pipe a script into a shell. It was
built in the pnpm monorepo, which is the wrong home: it has no workspace
dependencies, its releases have nothing to do with pnpm's, and it needed
an exception there to keep its version from being normalised to the
`11xx` scheme.

Here it sits beside the two installers it duplicates in miniature, and
the things all three have to agree on stay in one place — the pinned npm
signing key, the platform package names, the refusal messages. Moving it
already caught one drift: this one reported a checksum mismatch in
different words than install.sh does.

The package has no dependencies. libc detection is the probe pnpm's own
wrappers use rather than `detect-libc`, because every dependency is one
more thing `npx` fetches before pnpm exists. TypeScript compiled to
`lib/`, tests through `node --test`, so the repo gains a compiler and
nothing else.

`scripts/update-npm-keys.mjs` refreshes the pinned key in all three
installers at once and fails if any has drifted; a weekly workflow runs
the check and opens a pull request when npm rotates. That replaces the
mirroring that ran from the pnpm monorepo, which would have reached
across repositories to work.

CI installs pnpm with pnpm's own action, and runs the tests on Node 22
as well as 24 — `engines` allows 22, so it is exercised there rather
than only claimed. The test runner itself needs 22.18, where Node runs
TypeScript without a flag; the published package has no such floor.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (5)
.github/workflows/npm-package.yml (2)

3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The trigger runs every pull request twice.

push and pull_request both fire for branches pushed to this repository. Restrict push to the default branch to halve the CI runs.

♻️ Proposed change
 on:
-  - push
-  - pull_request
+  push:
+    branches:
+      - main
+  pull_request:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/npm-package.yml around lines 3 - 5, Update the workflow
trigger configuration so the push event runs only for the repository’s default
branch, while retaining pull_request triggers for all applicable requests.
Adjust the `on` block without changing the workflow jobs or other event
behavior.

32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

npm ci || npm install hides a missing or stale lockfile.

No package-lock.json appears in this PR. npm ci therefore fails on every run and the job silently installs floating ^ ranges instead. A future lockfile drift also passes unnoticed. Commit npm/package-lock.json and use npm ci alone, so dependency resolution stays reproducible and actions/setup-node can cache.

♻️ Proposed change
       - uses: actions/setup-node@v6
         with:
           node-version: 24
-      - run: npm ci || npm install
+          cache: npm
+          cache-dependency-path: npm/package-lock.json
+      - run: npm ci
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/npm-package.yml at line 32, Commit the
npm/package-lock.json lockfile and update the workflow step to run npm ci
without the fallback npm install. Ensure the lockfile is maintained with the
package manifests so missing or stale dependency resolutions fail the job and
setup-node caching remains reproducible.
npm/test/unit.test.ts (1)

67-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the checksum path in npm/src/registry.ts.

The signature tests are thorough. The checksum comparison in downloadTarball and the algorithm selection in checksum carry equal security weight and have no unit tests. Add cases for an integrity value, the shasum fallback, a mismatching digest, and metadata with neither field.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@npm/test/unit.test.ts` around lines 67 - 124, Add unit tests covering the
checksum logic used by downloadTarball and checksum: verify integrity-based
hashing, shasum fallback, rejection of a mismatching digest, and handling
metadata with neither integrity nor shasum. Reuse the existing test fixtures and
assert the expected success or failure behavior for each case.
npm/tsconfig.json (1)

16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test files are never type-checked.

include covers src/**/*.ts only. Node.js strips types at runtime without checking them, so npm/test/*.test.ts receives no type checking at all. Consider adding a second config (for example tsconfig.test.json with noEmit) that includes test/**/*.ts, and running it in the test script.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@npm/tsconfig.json` at line 16, Extend the TypeScript test workflow so test
files are type-checked: add a noEmit test configuration including the test
TypeScript sources, and update the package test script to run that configuration
alongside the existing tests. Preserve the current production tsconfig include
and test execution behavior.
npm/package.json (1)

33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update the Node.js baseline or pass --experimental-strip-types for the test script.

npm/package.json allows Node.js >=22.13, but node --test test/*.test.ts needs TypeScript type stripping unflagged by Node.js: at minimum 22.18 or 23.6. Either raise engines.node/document a higher development baseline, or add --experimental-strip-types conditionally for Node.js versions before that baseline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@npm/package.json` at line 33, Update the test script and Node.js engine
declaration so the TypeScript test files run on the supported baseline: either
raise engines.node to a version with unflagged type stripping, or conditionally
pass --experimental-strip-types for older supported versions. Keep the tsc
compilation and node --test execution behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/npm-keys.yml:
- Around line 36-42: Make the npm key rotation workflow use one stable
automation branch instead of the date-based branch generated near git checkout,
and reuse or update an existing rotation pull request rather than creating
duplicates. Add workflow-level concurrency covering scheduled and manual runs,
while preserving the existing commit and push behavior.

In `@npm/bin/get-pnpm.js`:
- Around line 6-9: Update the catch block in the get-pnpm error-handling flow to
safely handle thrown values that are null, undefined, primitives, or Error
objects. Log a meaningful string representation of the caught value without
directly dereferencing err.message, while preserving process.exitCode = 1.

In `@npm/src/index.ts`:
- Around line 112-117: Update the package-fetch flow around Promise.all so both
fetchPackage tasks settle before the surrounding finally cleanup removes tmpDir.
Preserve propagation of any fetch failure after both tasks have completed, while
retaining the existing conditional wrapper fetch behavior for major versions
below 11.
- Around line 101-106: Update the signal-handler setup around installPnpm to
store each registered handler, then remove all three handlers in a finally block
after installPnpm completes. Preserve the existing cleanup and exit behavior
while ensuring repeated calls do not retain stale process-level listeners.

In `@npm/src/registry.ts`:
- Around line 76-90: Update the registry request flow in request to pass an
AbortSignal to fetch, using a normal timeout for metadata requests and a
substantially longer timeout when downloading the tarball stream. Ensure the
timeout covers the entire response and streamed body, while preserving the
existing error handling and request behavior.
- Around line 60-69: Update checksum to parse only the first space-separated SRI
entry, then split that entry at the first hyphen so the full digest remains
intact; remove the shasum/SHA-1 fallback and retain the existing error when no
integrity checksum is available.

In `@npm/src/verifySignature.ts`:
- Around line 51-53: Update the expiry validation in verifySignature to parse
key.expires once and reject it when the parsed date is invalid (its time is
NaN), while preserving the existing expired-date error and opts.now comparison
for valid values.
- Around line 38-50: Update signature selection in the verification flow to
search all entries in opts.signatures for one whose keyid matches the configured
keys, rather than always using opts.signatures[0]. Preserve the existing
no-signature and unexpected-key errors, and add coverage for an unpinned
signature followed by a pinned signature, which must verify successfully.

In `@scripts/update-npm-keys.mjs`:
- Around line 34-37: Update the merge logic in the pinned-key handling to use
the authoritative npm endpoint key set as the complete trusted set, removing the
union that re-adds absent pinned keys. Ensure keys removed from the endpoint are
not retained or trusted by the generated key data.

---

Nitpick comments:
In @.github/workflows/npm-package.yml:
- Around line 3-5: Update the workflow trigger configuration so the push event
runs only for the repository’s default branch, while retaining pull_request
triggers for all applicable requests. Adjust the `on` block without changing the
workflow jobs or other event behavior.
- Line 32: Commit the npm/package-lock.json lockfile and update the workflow
step to run npm ci without the fallback npm install. Ensure the lockfile is
maintained with the package manifests so missing or stale dependency resolutions
fail the job and setup-node caching remains reproducible.

In `@npm/package.json`:
- Line 33: Update the test script and Node.js engine declaration so the
TypeScript test files run on the supported baseline: either raise engines.node
to a version with unflagged type stripping, or conditionally pass
--experimental-strip-types for older supported versions. Keep the tsc
compilation and node --test execution behavior intact.

In `@npm/test/unit.test.ts`:
- Around line 67-124: Add unit tests covering the checksum logic used by
downloadTarball and checksum: verify integrity-based hashing, shasum fallback,
rejection of a mismatching digest, and handling metadata with neither integrity
nor shasum. Reuse the existing test fixtures and assert the expected success or
failure behavior for each case.

In `@npm/tsconfig.json`:
- Line 16: Extend the TypeScript test workflow so test files are type-checked:
add a noEmit test configuration including the test TypeScript sources, and
update the package test script to run that configuration alongside the existing
tests. Preserve the current production tsconfig include and test execution
behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 57e621ea-888d-44d4-8c3a-eb9b15132532

📥 Commits

Reviewing files that changed from the base of the PR and between 1b57065 and d4c1caf.

⛔ Files ignored due to path filters (1)
  • npm/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • .github/workflows/npm-keys.yml
  • .github/workflows/npm-package.yml
  • .gitignore
  • npm/README.md
  • npm/bin/get-pnpm.js
  • npm/package.json
  • npm/src/extractTarball.ts
  • npm/src/index.ts
  • npm/src/npmSigningKeys.ts
  • npm/src/platformPackageName.ts
  • npm/src/registry.ts
  • npm/src/resolveVersion.ts
  • npm/src/verifySignature.ts
  • npm/test/install.test.ts
  • npm/test/unit.test.ts
  • npm/tsconfig.json
  • scripts/update-npm-keys.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
  • GitHub Check: install (dash, next-12, install.sh)
  • GitHub Check: install (windows-latest, next-11)
  • GitHub Check: install (windows-latest, 10.30.1)
  • GitHub Check: install (ubuntu-latest, next-12)
  • GitHub Check: install (macos-latest, next-12)
  • GitHub Check: test (ubuntu-latest)
  • GitHub Check: install (bash, next-12, install.sh)
  • GitHub Check: test (windows-latest)
  • GitHub Check: install (sh, next-11, install.sh)
  • GitHub Check: install (dash, next-12, install.sh)
  • GitHub Check: install (bash, next-11, install.sh)
  • GitHub Check: install (windows-latest, next-11)
  • GitHub Check: install (windows-latest, next-12)
  • GitHub Check: install (windows-latest, 10.30.1)
  • GitHub Check: install (macos-latest, next-11)
  • GitHub Check: install (sh, 10.30.1, install.sh)
  • GitHub Check: install (ksh, next-11, install.sh)
  • GitHub Check: install (zsh, 10.30.1, install.sh)
🧰 Additional context used
🪛 ast-grep (0.45.0)
npm/src/extractTarball.ts

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

npm/test/install.test.ts

[warning] 46-88: Use https protocol over http
Context: http.createServer((req, res) => {
const url = decodeURIComponent(req.url!)
const json = (body: unknown): void => {
res.writeHead(200, { 'content-type': 'application/json' })
res.end(JSON.stringify(body))
}

  if (url === '/pnpm') {
    json({ 'dist-tags': { latest: VERSION }, versions: { [VERSION]: {} } })
    return
  }
  if (url.endsWith(`/${VERSION}`)) {
    const kind = url === `/pnpm/${VERSION}` ? 'pnpm' : 'platform'
    const name = url.slice(1, url.lastIndexOf('/'))
    const bytes = fs.readFileSync(tarballs[kind])
    const integrity = `sha512-${createHash('sha512').update(bytes).digest('base64')}`
    // Signing a different checksum than the one served is what a tampered
    // checksum looks like: the bytes and the metadata agree, the signature
    // does not cover them.
    const signed = mode === 'bad-signature' ? `sha512-${'A'.repeat(86)}==` : integrity
    json({
      dist: {
        tarball: `${registry}${kind}.tgz`,
        integrity,
        signatures: mode === 'unsigned'
          ? []
          : [{
            keyid: mode === 'wrong-keyid' ? 'SHA256:not-pinned' : KEYS[0]!.keyid,
            sig: createSign('SHA256').update(`${name}@${VERSION}:${signed}`).sign(privateKey, 'base64'),
          }],
      },
    })
    return
  }
  if (url === '/pnpm.tgz' || url === '/platform.tgz') {
    const bytes = fs.readFileSync(tarballs[url === '/pnpm.tgz' ? 'pnpm' : 'platform'])
    res.writeHead(200, { 'content-type': 'application/octet-stream' })
    res.end(mode === 'bad-tarball' ? Buffer.concat([bytes, Buffer.from('tampered')]) : bytes)
    return
  }
  res.writeHead(404)
  res.end()
})

Note: [CWE-319] Cleartext Transmission of Sensitive Information. Security best practice.

(https-protocol-missing-typescript)


[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)


[warning] 60-60: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(tarballs[kind])
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)


[warning] 81-81: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(tarballs[url === '/pnpm.tgz' ? 'pnpm' : 'platform'])
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)


[warning] 103-103: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(setupLog, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)


[warning] 143-143: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(dest, content, { mode: 0o755 })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

npm/src/index.ts

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)


[warning] 150-150: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(path.join(opts.wrapperDir, 'package.json'), 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)


[warning] 152-158: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(opts.tmpDir, 'package.json'), JSON.stringify({
name: '@pnpm/exe',
version: opts.version,
type: 'module',
bin: { pnpm: opts.executable, pn: opts.executable },
dependencies: wrapper.dependencies,
}))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

🪛 Betterleaks (1.7.3)
npm/src/npmSigningKeys.ts

[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 21-21: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 zizmor (1.29.0)
.github/workflows/npm-keys.yml

[warning] 18-18: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🔇 Additional comments (14)
npm/src/extractTarball.ts (1)

12-23: LGTM!

npm/test/install.test.ts (1)

13-149: LGTM!

npm/package.json (1)

1-30: LGTM!

Also applies to: 34-43

npm/tsconfig.json (1)

1-15: LGTM!

npm/README.md (1)

1-49: LGTM!

.gitignore (1)

2-3: LGTM!

npm/src/resolveVersion.ts (2)

1-23: LGTM!

Also applies to: 28-30


24-27: 🎯 Functional Correctness

No change needed for bare-major resolution.

pnpm publishes latest-11, so resolveVersion(packument, '11') resolves the current stable major through the existing tag.

npm/src/platformPackageName.ts (1)

1-71: LGTM!

npm/test/unit.test.ts (1)

1-66: LGTM!

npm/src/registry.ts (1)

1-59: LGTM!

Also applies to: 70-75

.github/workflows/npm-package.yml (1)

26-31: 📐 Maintainability & Code Quality

No change needed for action major versions.

actions/checkout@v7 and actions/setup-node@v6 are valid major-version references.

npm/src/verifySignature.ts (1)

55-61: 🩺 Stability & Availability

No change needed. Node.js accepts these pinned public keys as single-line PEM bodies.

npm/src/npmSigningKeys.ts (1)

8-29: 🔒 Security & Privacy

No issue found.

These entries are npm registry public signing keys, and scripts/update-npm-keys.mjs keeps expired pinned keys for pre-rotation artifact validation.

Comment thread .github/workflows/npm-keys.yml Outdated
Comment thread get-pnpm/bin/get-pnpm.js
Comment thread npm/src/index.ts Outdated
Comment thread npm/src/index.ts Outdated
Comment thread get-pnpm/src/registry.ts
Comment thread npm/src/registry.ts Outdated
Comment thread npm/src/verifySignature.ts Outdated
Comment thread npm/src/verifySignature.ts Outdated
Comment thread scripts/update-npm-keys.mjs Outdated
Signature selection was positional. A package can carry several
signatures in no guaranteed order, and mid-rotation one of them can come
from a key this installer does not pin — which is no reason to refuse a
package that a pinned key also signed. The signature is now chosen by
matching keyid against the pinned set.

An unreadable `expires` compared as `Invalid Date`, which is false
against everything, so a key with a corrupt date read as "never
expires". It is now rejected.

Withdrawn keys are no longer kept. A signature carries no trusted
timestamp, so one made before a withdrawal cannot be told apart from one
forged after it; these installers only ever fetch current releases, so
following npm's advertised set costs nothing and keeps a withdrawal
meaning something.

`integrity` is an SRI field and may hold several space-separated
entries, so splitting on every `-` could yield a corrupt digest and
refuse a good download. The SHA-1 `shasum` fallback is gone: a package
without `integrity` is now refused by name before anything is
downloaded, rather than reaching a weaker digest.

Requests had no timeout, so a registry that accepted the connection and
then stalled would hang the install with no output. Metadata gets 30s;
the tarball gets a budget that survives a slow connection carrying
~150 MB.

`installPnpm` is a library function, and it left three signal handlers
registered per call. It also used `Promise.all`, so a rejected fetch let
the other one keep writing into the directory the cleanup was about to
remove.

The bin script read `.message` off whatever was thrown, which would
print `undefined` — or throw again — for a non-Error.

The rotation workflow opened a new branch and pull request every week
until one merged, and a same-day re-run collided with its own branch.
One long-lived branch, force-pushed, with a pull request opened only
when none is waiting, and a concurrency group so a manual run cannot
race the scheduled one.
`installPnpm` was the only way in, and it ends by running `pnpm setup
--force`: a global install that edits shell rc files and writes GitHub
Actions env files. That is right for a person typing `npx get-pnpm` and
wrong for anything that manages its own directory, so pnpm/setup could
not have used this package without reimplementing the half it needed.

`downloadPnpm` is now that half — resolve, verify, place the executable
beside its `dist/`, touch nothing else — and `installPnpm` is it plus
the setup step.

The directory is `get-pnpm/` rather than `npm/`, after the package
rather than after the registry, and the version starts at 0.0.0.
0.0.0 is taken: it is the placeholder published under this name in 2018,
and npm refuses a re-publish of an existing version.
@zkochan
zkochan merged commit 38e1d0a into main Aug 9, 2026
136 checks passed
@zkochan

zkochan commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Correction to something I claimed in this PR and in its commit message.

I said the registry copy of v11 loses @reflink/reflink and that this "silently downgrades copy-on-write cloning to hard linking", citing a before/after where one install reported hard linked and the other cloned. A controlled A/B says otherwise. Same filesystem, same version, only the manifest suppressed:

v11 install pnpm add is-odd reports
with the dependency manifest Packages are cloned from the content-addressable store
with it suppressed Packages are cloned from the content-addressable store

The GitHub release tarball points the same way: its bundled @reflink is binding.js, index.js and package.json with no native .node anywhere in dist/, and installs made through install.sh clone regardless. Whatever produced the original hard linked reading, it was not this.

So the honest position on the manifest: it makes a registry-sourced install match what npm i -g @pnpm/exe produces, by declaring the dependencies the wrapper declares. That is worth doing on its own terms. It does not fix a demonstrated cloning regression, and I should not have said it did.

This has a consequence worth acting on separately: the reason I kept v11 on the GitHub path in pnpm/setup#24 was exactly this @reflink argument. It does not hold, so v11 there could come from the registry too — which matters, because latest resolves to v11 today, so most users of that action currently get the weaker same-origin digest check rather than the signature.

This branch was successfully deployed

1 active deployment
Preview — c64af8c2 Deployed Aug 9, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant