fix(bins.resolver): reject reserved manifest bin names (GHSA-4gxm-v5v7-fqc4) by zkochan · Pull Request #12289 · pnpm/pnpm

zkochan · 2026-06-09T16:43:26Z

Summary

Manifest bin object keys such as "", ".", "..", and scoped forms like @scope/.. (which strips to ..) passed pnpm's bin-name guard. The guard at bins/resolver/src/index.ts only required binName === encodeURIComponent(binName), and encodeURIComponent leaves . unchanged — so these names were accepted.

When a package carrying such a key is installed globally, later global remove / update / add-replacement flows re-derive the name from the installed manifest (scanGlobalPackages → getInstalledBinNames → getBinsFromPackageManifest) and pass path.join(globalBinDir, binName) to removeBin (a recursive rimraf):

"." → resolves to the global bin directory itself
".." → resolves to its parent

So a malicious package could cause pnpm to recursively delete the global bin directory or its parent.

Fix

bins/resolver/src/index.ts rejects empty, ., and .. bin names after scope stripping, before the URL-safe check.
pacquet/crates/cmd-shim/src/bin_resolver.rs mirrors the same rejection (is_safe_bin_name already rejected empty; ./.. slipped through because . is in the URL-safe byte set). Per the monorepo sync rule, this is the shared resolver/linker boundary pacquet's dependency-management commands also use.

Tests

bins/resolver/test/index.ts — covers "", ., .., and @scope/...
global/commands/test/globalRemove.test.ts — drives the real resolver/scan path against a temp global install and asserts the deletion sink only ever receives path.join(globalBinDir, "good") (without the fix it would call removeBin four times).
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs — parity coverage for ., .., and @scope/...

Notes

This addresses advisory GHSA-4gxm-v5v7-fqc4 / CAND-PNPM-085. Opened as a draft so disclosure timing can be decided before this is made visible — note that a draft PR on the public repo is still publicly visible.

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

Summary by CodeRabbit

Bug Fixes
- Fixed a security vulnerability in bin resolution that could enable unintended deletion of the bin directory and parent directories during global package operations. Reserved bin names (empty string, ., .., and scoped variants) are now properly rejected.
Tests
- Added test coverage to verify reserved bin names are correctly filtered during package bin resolution.

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 in both the TypeScript resolver and the pacquet cmd-shim bin resolver.

coderabbitai · 2026-06-09T16:43:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 56556622-2150-4d78-a0dc-0d97e8fad1ab

📥 Commits

Reviewing files that changed from the base of the PR and between d188316 and c06312c.

📒 Files selected for processing (6)

.changeset/strange-bin-segments.md
bins/resolver/src/index.ts
bins/resolver/test/index.ts
global/commands/test/globalRemove.test.ts
pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: windows-latest / Node.js 22.13.0 / Test

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use Standard Style with trailing commas, prefer functions over classes, declare functions after they are used (relying on hoisting), limit functions to no more than two or three arguments, and use a single options object for functions needing more parameters
Follow import order: standard libraries first, then external dependencies (sorted alphabetically), then relative imports
Do not write comments that restate what the code already says; rename variables, split helpers, or move checks to more obvious places instead
Do not repeat documentation at call sites that already lives on the callee; update the JSDoc once and let every call site benefit
Use JSDoc for the function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the function body
Do not record past implementation shape, refactor history, or removed code in comments; use git log and git blame for that information instead
Write comments only when the reason for code is non-obvious, a hidden invariant exists, a workaround for a known bug is needed, or an exception to surrounding pattern is deliberate

Files:

bins/resolver/test/index.ts
bins/resolver/src/index.ts
global/commands/test/globalRemove.test.ts

pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not use instanceof Error for checking if a caught error is an Error object in Jest tests; use util.types.isNativeError() instead to work across realms

Files:

global/commands/test/globalRemove.test.ts

🧠 Learnings (8)

📚 Learning: 2026-05-26T21:01:06.666Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.

Applied to files:

.changeset/strange-bin-segments.md

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

bins/resolver/test/index.ts
bins/resolver/src/index.ts
global/commands/test/globalRemove.test.ts

📚 Learning: 2026-06-05T13:47:05.929Z

Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/test/install/frozenStore.ts:2-17
Timestamp: 2026-06-05T13:47:05.929Z
Learning: In the pnpm/pnpm repository, the shared Jest preset keeps `injectGlobals` at its default (`true`), so `test` and `expect` are available as Jest globals. Therefore, reviewers should not flag (or treat as TypeScript/compilation errors) missing `import { test, expect } from 'jest/globals'` when a test file uses `test`/`expect` without importing them. Importing from `jest/globals` may still be used for consistency with sibling files, but it is not required for execution in this repo unless a Jest preset is explicitly configured with `injectGlobals: false`.

Applied to files:

bins/resolver/test/index.ts
global/commands/test/globalRemove.test.ts

📚 Learning: 2026-06-05T13:47:26.046Z

Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/src/install/index.ts:2337-2343
Timestamp: 2026-06-05T13:47:26.046Z
Learning: In the pnpm/pnpm codebase, `PnpmError` automatically prefixes `err.code` with `ERR_PNPM_` when you pass a code that does not already start with `ERR_PNPM_` (it normalizes `this.code` via `code.startsWith('ERR_PNPM_') ? code : `ERR_PNPM_${code}``). Therefore, during code review you should NOT flag `new PnpmError(...)` call sites for passing a bare error code (e.g., `new PnpmError('FROZEN_STORE_INCOMPATIBLE_WITH_PNPR', ...)`); the resulting `err.code` will still be `ERR_PNPM_FROZEN_STORE_INCOMPATIBLE_WITH_PNPR`.

Applied to files:

bins/resolver/test/index.ts
bins/resolver/src/index.ts
global/commands/test/globalRemove.test.ts

📚 Learning: 2026-05-20T19:40:55.051Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

📚 Learning: 2026-05-22T00:08:44.646Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

📚 Learning: 2026-05-20T23:07:58.444Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.

Applied to files:

pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

📚 Learning: 2026-06-06T18:58:37.156Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 12243
File: pacquet/crates/package-manager/src/install_package_by_snapshot.rs:319-322
Timestamp: 2026-06-06T18:58:37.156Z
Learning: When reviewing Rust code, do not assume `matches!(expr, Pattern(_))` will move out of `expr` if `Pattern(_)` contains no by-value bindings. `matches!` desugars to a `match` that auto-borrows the scrutinee for discrimination, so even if `expr` is a non-`Copy` value behind a shared reference (e.g., `&T`), the macro should not move-out of the borrowed data purely due to `matches!`. Treat `matches!(&expr, Pattern(_))` as a readability/clarity improvement, not a correctness requirement. Only flag potential move-out-of-borrow risks when the pattern includes by-value bindings that would require moving the matched value.

Applied to files:

pacquet/crates/cmd-shim/src/bin_resolver.rs
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

🔇 Additional comments (6)

.changeset/strange-bin-segments.md (1)

1-6: LGTM!

bins/resolver/src/index.ts (1)

69-75: LGTM!

bins/resolver/test/index.ts (1)

150-168: LGTM!

global/commands/test/globalRemove.test.ts (1)

1-47: LGTM!

pacquet/crates/cmd-shim/src/bin_resolver.rs (1)

174-177: LGTM!

Also applies to: 182-184

pacquet/crates/cmd-shim/src/bin_resolver/tests.rs (1)

365-385: LGTM!

📝 Walkthrough

Walkthrough

This PR hardens bin name validation across pnpm's TypeScript and Pacquet's Rust implementations by explicitly rejecting reserved names (empty string, ".", "..", and scoped variants like "@scope/..") that could resolve to unsafe global bin directories or parent paths during package management operations.

Changes

Reserved Bin Name Validation

Layer / File(s)	Summary
Release documentation `.changeset/strange-bin-segments.md`	Changeset documents patch version updates for `@pnpm/bins.resolver` and `pnpm`, describing the rejection of reserved bin names and the security risk they pose during global remove/update/add operations.
TypeScript bin resolver implementation `bins/resolver/src/index.ts`, `bins/resolver/test/index.ts`	`commandsFromBin` validates binName values and skips empty strings, `.`, and `..` before URL-safe character validation. Unit test verifies reserved names are excluded from bin resolution.
Global remove integration test `global/commands/test/globalRemove.test.ts`	Integration test constructs a malicious package manifest with reserved bin entries and verifies `handleGlobalRemove` only removes bins with safe names from the global bin directory.
Rust bin resolver implementation `pacquet/crates/cmd-shim/src/bin_resolver.rs`, `pacquet/crates/cmd-shim/src/bin_resolver/tests.rs`	`is_safe_bin_name` validation explicitly rejects `.` and `..` with updated documentation. Unit test verifies reserved relative path names are rejected while safe names are accepted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Reserved names cause quite a fright,
Dot and double-dot blocked from sight!
TypeScript and Rust, both standing guard,
Against the paths both wild and hard.
Safety wins—no more risky flight! 🛡️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: rejecting reserved bin names in the manifest resolver to fix a security vulnerability.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 vuln-085

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

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 and usage tips.}

github-actions · 2026-06-09T16:47:49Z

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.02      8.0±0.36ms   542.8 KB/sec    1.00      7.8±0.08ms   556.0 KB/sec

codecov-commenter · 2026-06-09T16:51:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.55%. Comparing base (d188316) to head (c06312c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12289      +/-   ##
==========================================
- Coverage   87.55%   87.55%   -0.01%     
==========================================
  Files         280      280              
  Lines       33664    33664              
==========================================
- Hits        29476    29475       -1     
- Misses       4188     4189       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2026-06-09T17:27:41Z

Integrated-Benchmark Report (Linux)

Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD.

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	10.073 ± 0.179	9.813	10.344	1.86 ± 0.05
`pacquet@main`	9.963 ± 0.151	9.854	10.356	1.84 ± 0.05
`pnpr@HEAD`	5.544 ± 0.194	5.333	5.812	1.02 ± 0.04
`pnpr@main`	5.428 ± 0.113	5.290	5.700	1.00

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 10.073020473879998,
      "stddev": 0.17902215112356604,
      "median": 10.04645889388,
      "user": 3.0712956,
      "system": 3.3818287399999996,
      "min": 9.812548441879999,
      "max": 10.34389486888,
      "times": [
        10.17697854888,
        10.05173880488,
        10.246723913879999,
        10.25517035888,
        10.34389486888,
        9.970926854879998,
        10.04117898288,
        9.812548441879999,
        9.839490527879999,
        9.991553435879998
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 9.96342116688,
      "stddev": 0.15141876067928436,
      "median": 9.90568507438,
      "user": 3.1226301999999997,
      "system": 3.3774285399999995,
      "min": 9.854196088879998,
      "max": 10.356324943879999,
      "times": [
        9.98271949288,
        9.88426246888,
        9.90778033088,
        9.854196088879998,
        10.356324943879999,
        10.06100860888,
        9.903589817879999,
        9.859587531879999,
        9.887732564879999,
        9.93700981988
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 5.54396334048,
      "stddev": 0.19432017902606835,
      "median": 5.5259802668799995,
      "user": 2.5204372,
      "system": 3.1006234399999992,
      "min": 5.33302628088,
      "max": 5.81222740188,
      "times": [
        5.81222740188,
        5.77603118588,
        5.34927942188,
        5.46120585988,
        5.34908134288,
        5.36902955888,
        5.64647058388,
        5.33302628088,
        5.7525270948800005,
        5.59075467388
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 5.42774198258,
      "stddev": 0.11267187271534403,
      "median": 5.39906848388,
      "user": 2.5059061000000002,
      "system": 3.04499574,
      "min": 5.28991596988,
      "max": 5.70048667288,
      "times": [
        5.41713771588,
        5.38559072688,
        5.38650802988,
        5.34883700388,
        5.51848220488,
        5.37868854088,
        5.28991596988,
        5.41162893788,
        5.44014402288,
        5.70048667288
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`pacquet@HEAD`	676.1 ± 24.1	652.8	736.4	1.01 ± 0.04
`pacquet@main`	672.1 ± 17.6	647.5	695.8	1.00
`pnpr@HEAD`	794.0 ± 21.0	775.2	847.0	1.18 ± 0.04
`pnpr@main`	781.2 ± 11.7	765.9	801.0	1.16 ± 0.04

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.6760562938000001,
      "stddev": 0.02406778888053719,
      "median": 0.6723477217,
      "user": 0.41266484,
      "system": 1.34639554,
      "min": 0.6528006877,
      "max": 0.7364346957,
      "times": [
        0.6685591537000001,
        0.7364346957,
        0.6620001027,
        0.6576069687,
        0.6761362897000001,
        0.6860775247,
        0.6528006877,
        0.6802667267,
        0.6591759847,
        0.6815048037
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6720998880000002,
      "stddev": 0.017595576604555452,
      "median": 0.6677405172,
      "user": 0.38578393999999994,
      "system": 1.34870784,
      "min": 0.6475053537000001,
      "max": 0.6957529317000001,
      "times": [
        0.6648504507,
        0.6786269417,
        0.6475053537000001,
        0.6953448597,
        0.6584807257,
        0.6612414597,
        0.6926065387,
        0.6957529317000001,
        0.6706305837000001,
        0.6559590347
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.7939785491000001,
      "stddev": 0.020995298403113885,
      "median": 0.7894879232,
      "user": 0.4242800399999999,
      "system": 1.3532114399999997,
      "min": 0.7752261267,
      "max": 0.8470015177,
      "times": [
        0.7766600537,
        0.7808659137,
        0.8013633097,
        0.7992820307,
        0.7996600617,
        0.7807506307000001,
        0.7752261267,
        0.8470015177,
        0.7884848027,
        0.7904910437
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.7811970431,
      "stddev": 0.011712369207795132,
      "median": 0.7813466372,
      "user": 0.4059736399999999,
      "system": 1.35704564,
      "min": 0.7658697367,
      "max": 0.8009877607,
      "times": [
        0.7658697367,
        0.7949200477,
        0.7880972557,
        0.7722265177000001,
        0.7853560127,
        0.8009877607,
        0.7664508767,
        0.7753689487000001,
        0.7779661417,
        0.7847271327
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	9.280 ± 0.043	9.240	9.365	1.81 ± 0.03
`pacquet@main`	9.270 ± 0.049	9.207	9.394	1.81 ± 0.03
`pnpr@HEAD`	5.225 ± 0.165	5.085	5.537	1.02 ± 0.04
`pnpr@main`	5.118 ± 0.093	5.029	5.363	1.00

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 9.280275675900002,
      "stddev": 0.042749743942392074,
      "median": 9.259713476200002,
      "user": 3.59862018,
      "system": 3.3646721399999997,
      "min": 9.2404626012,
      "max": 9.3646793832,
      "times": [
        9.2433770852,
        9.316839014200001,
        9.2468116352,
        9.3271053942,
        9.3646793832,
        9.2581832122,
        9.261243740200001,
        9.2527765142,
        9.2404626012,
        9.2912781792
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 9.270196352100001,
      "stddev": 0.04889992263115843,
      "median": 9.2624348367,
      "user": 3.65010988,
      "system": 3.36397894,
      "min": 9.2072889492,
      "max": 9.3937985372,
      "times": [
        9.2072889492,
        9.2429401682,
        9.242852091200001,
        9.262682359200001,
        9.2647101222,
        9.2581968852,
        9.2753322092,
        9.2621873142,
        9.2919748852,
        9.3937985372
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 5.2249431465,
      "stddev": 0.1646972543171939,
      "median": 5.1616143797,
      "user": 2.3045213799999997,
      "system": 2.92482144,
      "min": 5.0849053512,
      "max": 5.536631926199999,
      "times": [
        5.1239066831999995,
        5.536631926199999,
        5.1226162162,
        5.1895853642,
        5.235995658199999,
        5.1056267832,
        5.511876226199999,
        5.0849053512,
        5.1336433952,
        5.204643861199999
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 5.117951507699999,
      "stddev": 0.09316934815122235,
      "median": 5.096371918199999,
      "user": 2.31961228,
      "system": 2.9210641399999995,
      "min": 5.029110989199999,
      "max": 5.3634936172,
      "times": [
        5.0820807452,
        5.029110989199999,
        5.1531420061999995,
        5.0782771991999995,
        5.3634936172,
        5.0592635182,
        5.1204018782,
        5.110663091199999,
        5.0698198912,
        5.1132621412
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	1.474 ± 0.024	1.425	1.504	2.16 ± 0.04
`pacquet@main`	1.472 ± 0.026	1.433	1.513	2.16 ± 0.05
`pnpr@HEAD`	0.683 ± 0.009	0.675	0.704	1.00
`pnpr@main`	0.696 ± 0.054	0.658	0.840	1.02 ± 0.08

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.47408537596,
      "stddev": 0.023576142778185618,
      "median": 1.4718588131600001,
      "user": 1.6610549200000002,
      "system": 1.8129208799999996,
      "min": 1.42541265116,
      "max": 1.50400437216,
      "times": [
        1.4666955371600001,
        1.45660970016,
        1.46205136516,
        1.4930476001600002,
        1.49438979216,
        1.4748133491600002,
        1.50400437216,
        1.49492511516,
        1.42541265116,
        1.46890427716
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.47187158226,
      "stddev": 0.025768872486845888,
      "median": 1.46664610316,
      "user": 1.62586712,
      "system": 1.8285236799999995,
      "min": 1.43297886316,
      "max": 1.5130370381600002,
      "times": [
        1.50238138816,
        1.4533521921600001,
        1.49328306816,
        1.5130370381600002,
        1.44768499916,
        1.4698700551600001,
        1.43297886316,
        1.48506214316,
        1.46342215116,
        1.45764392416
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.6829086137600002,
      "stddev": 0.008961775094844896,
      "median": 0.6803843171600001,
      "user": 0.36957052,
      "system": 1.27898948,
      "min": 0.67496090216,
      "max": 0.7039744351600001,
      "times": [
        0.68695301616,
        0.6764732481600001,
        0.67658192916,
        0.67496090216,
        0.6837140821600001,
        0.6770545521600001,
        0.6842115891600001,
        0.68895523016,
        0.6762071531600001,
        0.7039744351600001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.6961520939600001,
      "stddev": 0.05382633473323299,
      "median": 0.6806721491600001,
      "user": 0.33954522,
      "system": 1.29079238,
      "min": 0.6583179891600001,
      "max": 0.8402676601600001,
      "times": [
        0.71206124416,
        0.6732148221600001,
        0.6583179891600001,
        0.8402676601600001,
        0.7077829841600001,
        0.6782714601600001,
        0.66516839916,
        0.6830728381600001,
        0.6841695461600001,
        0.6591939961600001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + hot store

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	5.028 ± 0.016	4.999	5.058	7.18 ± 0.31
`pacquet@main`	5.030 ± 0.019	5.001	5.066	7.18 ± 0.31
`pnpr@HEAD`	0.700 ± 0.030	0.673	0.775	1.00
`pnpr@main`	0.712 ± 0.079	0.668	0.930	1.02 ± 0.12

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 5.02767873856,
      "stddev": 0.01640791125215337,
      "median": 5.02528426686,
      "user": 1.8042253600000002,
      "system": 1.93746662,
      "min": 4.9992254888600005,
      "max": 5.05772884786,
      "times": [
        5.02350404386,
        4.9992254888600005,
        5.02280302086,
        5.01998803786,
        5.05772884786,
        5.01357111186,
        5.02706448986,
        5.03281493186,
        5.03378015586,
        5.04630725686
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 5.02998587656,
      "stddev": 0.01938587797568597,
      "median": 5.032875773360001,
      "user": 1.8024876600000002,
      "system": 1.9407335200000002,
      "min": 5.00126745386,
      "max": 5.06628138686,
      "times": [
        5.06628138686,
        5.02326514886,
        5.00317376086,
        5.00126745386,
        5.04153089286,
        5.01887091986,
        5.04172156786,
        5.03799608786,
        5.03347501386,
        5.03227653286
      ]
    },
    {
      "command": "pnpr@HEAD",
      "mean": 0.7002511339599999,
      "stddev": 0.029940061794671274,
      "median": 0.6984788043600001,
      "user": 0.37003706,
      "system": 1.28489692,
      "min": 0.67335844186,
      "max": 0.7754529838600001,
      "times": [
        0.7754529838600001,
        0.69841011086,
        0.67335844186,
        0.69854749786,
        0.71455265086,
        0.67343981586,
        0.7070719318600001,
        0.69884598886,
        0.6805694008600001,
        0.6822625168600001
      ]
    },
    {
      "command": "pnpr@main",
      "mean": 0.71166140146,
      "stddev": 0.07863559454606472,
      "median": 0.68330103436,
      "user": 0.34460576,
      "system": 1.28956672,
      "min": 0.66807694986,
      "max": 0.93030945886,
      "times": [
        0.72804682386,
        0.69775000286,
        0.68144756886,
        0.67265731786,
        0.67923569486,
        0.93030945886,
        0.68081786986,
        0.66807694986,
        0.69311782786,
        0.68515449986
      ]
    }
  ]
}

github-actions · 2026-06-09T17:27:43Z

Bencher Report

Branch	pr/12289
Testbed	pacquet

🚨 1 Alert

Benchmark	Measure Units	View	Benchmark Result (Result Δ%)	Upper Boundary (Limit %)
isolated-linker.fresh-install.cold-cache.cold-store	Latency seconds (s)	📈 plot 🚷 threshold 🚨 alert (🔔)	9.28 s (+38.78%) Baseline: 6.69 s	8.02 s (115.65%)

Click to view all benchmark results

Benchmark	Latency	Benchmark Result milliseconds (ms) (Result Δ%)	Upper Boundary milliseconds (ms) (Limit %)
isolated-linker.fresh-install.cold-cache.cold-store	📈 view plot 🚷 view threshold 🚨 view alert (🔔)	9,280.28 ms (+38.78%) Baseline: 6,686.85 ms	8,024.22 ms (115.65%)
isolated-linker.fresh-install.cold-cache.hot-store	📈 view plot 🚷 view threshold	5,027.68 ms (+0.27%) Baseline: 5,014.36 ms	6,017.23 ms (83.55%)
isolated-linker.fresh-install.hot-cache.hot-store	📈 view plot 🚷 view threshold	1,474.09 ms (+5.00%) Baseline: 1,403.86 ms	1,684.63 ms (87.50%)
isolated-linker.fresh-restore.cold-cache.cold-store	📈 view plot 🚷 view threshold	10,073.02 ms (+14.52%) Baseline: 8,795.58 ms	10,554.69 ms (95.44%)
isolated-linker.fresh-restore.hot-cache.hot-store	📈 view plot 🚷 view threshold	676.06 ms (+3.79%) Baseline: 651.39 ms	781.67 ms (86.49%)

🐰 View full continuous benchmarking report in Bencher

qodo-free-for-open-source-projects · 2026-06-09T18:02:19Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

qodo-free-for-open-source-projects · 2026-06-09T18:03:04Z

PR Summary by Qodo

Fix: reject reserved manifest bin names to prevent global bin directory deletion
🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Walkthroughs

Description

• Reject empty, ".", and ".." manifest bin keys (including scoped forms) during bin resolution.
• Mirror the same reserved-name guard in pacquet’s Rust bin resolver for parity.
• Add unit + integration coverage to ensure global remove only deletes safe shims.

Diagram

graph TD
  M["package.json bin keys"] --> R["bins.resolver (TS)"] --> G["global remove flow"] --> D["removeBin (rimraf)"] --> FS["global bin dir"]
  M --> PR["cmd-shim resolver (Rust)"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Add sink-side path safety check before rimraf

➕ Defense-in-depth even if a future resolver regression reintroduces bad names
➕ Can reject any path that resolves outside/equals globalBinDir (e.g., via realpath/normalize + prefix check)
➖ More platform-specific path semantics to get right (symlinks, case-folding)
➖ May require refactoring bins.remover/global code to pass structured inputs

2. Strict allowlist regex for bin names

➕ Simple rule to reason about (e.g., alphanumerics + limited punctuation)
➕ Avoids edge cases in URL-encoding equivalence checks
➖ Potential breaking change for existing packages with currently-accepted characters
➖ Would need careful compatibility assessment and migration guidance

Recommendation: The PR’s approach (explicitly rejecting "", ".", and ".." after scope stripping, in both TS and Rust) is the right minimal fix for the reported vulnerability and keeps compatibility with existing URL-safe names. Consider a follow-up defense-in-depth change in the deletion sink (or just before calling it) to normalize/realpath and ensure the computed target is strictly within the intended global bin directory.

File Changes

Bug fix (2)

index.ts Reject empty/relative (. ..) bin names before URL-safe validation +7/-1
Reject empty/relative (. ..) bin names before URL-safe validation
• Adds an explicit guard to skip resolved bin names that are empty, ".", or ".." (including scoped keys after scope stripping). Keeps the existing encodeURIComponent-based URL-safe check for other names.
bins/resolver/src/index.ts

bin_resolver.rs Mirror reserved-name rejection in Rust is_safe_bin_name +5/-1
Mirror reserved-name rejection in Rust is_safe_bin_name
• Updates the Rust bin-name safety predicate to reject "." and ".." (in addition to empty), aligning behavior with the TypeScript resolver and documenting the rationale.
pacquet/crates/cmd-shim/src/bin_resolver.rs

Tests (3)

index.ts Add unit coverage for reserved manifest bin keys +20/-0
Add unit coverage for reserved manifest bin keys
• Adds a test ensuring manifest bin keys "", ".", "..", and "@scope/.." are ignored while a normal key is preserved.
bins/resolver/test/index.ts

globalRemove.test.ts Integration test: global remove must not delete bin dir via reserved names +47/-0
Integration test: global remove must not delete bin dir via reserved names
• Mocks the deletion sink and builds a temporary global install layout to verify handleGlobalRemove only attempts to remove the safe shim name, preventing accidental deletion of the bin directory or its parent.
global/commands/test/globalRemove.test.ts

tests.rs Add Rust parity tests for reserved relative bin names +22/-0
Add Rust parity tests for reserved relative bin names
• Adds a test asserting that ".", "..", and "@scope/.." bin keys are rejected and only a safe bin entry is returned.
pacquet/crates/cmd-shim/src/bin_resolver/tests.rs

Other (1)

strange-bin-segments.md Add patch changeset documenting reserved bin-name rejection +6/-0
Add patch changeset documenting reserved bin-name rejection
• Introduces a changeset bumping @pnpm/bins.resolver and pnpm with a security-oriented note explaining why reserved bin keys are now rejected.
.changeset/strange-bin-segments.md

* fix(package-bins): reject reserved manifest bin names 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. * fix: block untrusted request destination env expansion 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. * fix(security): verify npm registry signature before spawning a package-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. * fix: harden package-manager bootstrap metadata 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. * fix(security): verify Node.js runtime SHASUMS OpenPGP signature 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). * test(env): sign the SHASUMS fixture for Node.js download tests 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. * fix(self-updater): redact registry credentials from engine identity errors 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. * fix: update vulnerable transitive dependencies 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.

zkochan · 2026-06-10T12:07:27Z

🚢 v11.5.3

zkochan marked this pull request as ready for review June 9, 2026 18:02

coderabbitai Bot approved these changes Jun 9, 2026

View reviewed changes

zkochan merged commit 230df57 into main Jun 9, 2026
26 of 28 checks passed

zkochan deleted the vuln-085 branch June 9, 2026 18:20

zkochan mentioned this pull request Jun 9, 2026

fix(security): port the latest security fixes to v10 #12300

Merged

coderabbitai Bot mentioned this pull request Jun 15, 2026

chore: update lockfile, Node.js, pnpm, and pacquet versions #12421

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(bins.resolver): reject reserved manifest bin names (GHSA-4gxm-v5v7-fqc4)#12289

fix(bins.resolver): reject reserved manifest bin names (GHSA-4gxm-v5v7-fqc4)#12289
zkochan merged 1 commit into
mainfrom
vuln-085

zkochan commented Jun 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

codecov-commenter commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026

Uh oh!

Uh oh!

zkochan commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zkochan commented Jun 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Tests

Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented Jun 9, 2026

Micro-Benchmark Results

Linux

Uh oh!

codecov-commenter commented Jun 9, 2026

Codecov Report

Uh oh!

github-actions Bot commented Jun 9, 2026

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Scenario: Isolated linker: fresh restore, hot cache + hot store

Scenario: Isolated linker: fresh install, cold cache + cold store

Scenario: Isolated linker: fresh install, hot cache + hot store

Scenario: Isolated linker: fresh install, cold cache + hot store

Uh oh!

github-actions Bot commented Jun 9, 2026

Bencher Report

🚨 1 Alert

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Great, no issues found!

Uh oh!

qodo-free-for-open-source-projects Bot commented Jun 9, 2026

PR Summary by Qodo

Walkthroughs

File Changes

Uh oh!

Uh oh!

zkochan commented Jun 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented Jun 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 9, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jun 9, 2026 •

edited

Loading