fix(security): backport path-traversal and containment fixes to v10 (#12504)

Backports three merged security fixes from main to release/10:

- Contain hoisted dependency aliases so a crafted lockfile alias cannot
  escape node_modules or overwrite pnpm-owned layout. The hoisted graph
  builder now validates each alias at the safeJoinModulesDir sink.
  (GHSA-fr4h-3cph-29xv, #12343)
- Contain pnpm patch-remove deletions to the configured patches
  directory. (#12341)
- Reject path-traversal config dependency names and versions from
  pnpm-workspace.yaml before they are used to build filesystem paths.
  (GHSA-qrv3-253h-g69c, #12470)

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

.changeset/config-deps-path-traversal.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/config.deps-installer": patch
+"pnpm": patch
+---
+
+Security: validate config dependency names and versions before using them to build filesystem paths. A `pnpm-workspace.yaml` with a traversal-shaped `configDependencies` name (such as `../../PWNED`) or version (such as `../../../PWNED`) could previously cause `pnpm install` to create symlinks or write package files outside `node_modules/.pnpm-config` and the store. Names must now be valid npm package names and versions must be exact semver versions. See [GHSA-qrv3-253h-g69c](https://github.com/pnpm/pnpm/security/advisories/GHSA-qrv3-253h-g69c).

.changeset/contain-hoisted-dependency-aliases.md

@@ -0,0 +1,9 @@
+---
+"@pnpm/symlink-dependency": patch
+"@pnpm/headless": patch
+"pnpm": patch
+---
+
+Reject path-traversal and reserved dependency aliases (such as `../../../escape`, `.bin`, `.pnpm`, or `node_modules`) that come from a lockfile rather than a freshly resolved manifest. A crafted lockfile alias could otherwise be joined directly under a hoisted `node_modules` directory, letting package files be written outside the intended install root or overwrite pnpm-owned layout.
+
+The `nodeLinker: hoisted` graph builder now validates each alias at the directory sink (`safeJoinModulesDir`), matching the validation pnpm already performs when resolving aliases from manifests. See [GHSA-fr4h-3cph-29xv](https://github.com/pnpm/pnpm/security/advisories/GHSA-fr4h-3cph-29xv).

.changeset/fix-patch-remove-containment.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-patching": patch
+"pnpm": patch
+---
+
+Prevent `pnpm patch-remove` from removing files outside the configured patches directory.

config/deps-installer/package.json

@@ -47,7 +47,9 @@
     "@pnpm/read-package-json": "workspace:*",
     "@pnpm/types": "workspace:*",
     "@zkochan/rimraf": "catalog:",
-    "get-npm-tarball-url": "catalog:"
+    "get-npm-tarball-url": "catalog:",
+    "semver": "catalog:",
+    "validate-npm-package-name": "catalog:"
   },
   "peerDependencies": {
     "@pnpm/logger": "catalog:"
@@ -57,6 +59,8 @@
     "@pnpm/prepare": "workspace:*",
     "@pnpm/registry-mock": "catalog:",
     "@pnpm/testing.temp-store": "workspace:*",
+    "@types/semver": "catalog:",
+    "@types/validate-npm-package-name": "catalog:",
     "load-json-file": "catalog:",
     "read-yaml-file": "catalog:"
   },

config/deps-installer/src/assertValidConfigDepName.ts

@@ -0,0 +1,20 @@
+import { PnpmError } from '@pnpm/error'
+import validateNpmPackageName from 'validate-npm-package-name'
+
+// A config-dependency name becomes a directory pnpm creates during install
+// (`node_modules/.pnpm-config/<name>`) and a store path segment, so it must be
+// a valid npm package name. A traversal-shaped name (`../../PWNED`), a reserved
+// name (`.bin`, `.pnpm`, `node_modules`), or `__proto__` is rejected before any
+// path is built from it. Matches the `validForOldPackages` check pnpm applies
+// to dependency aliases read from a manifest.
+export function assertValidConfigDepName (name: string): void {
+  if (!validateNpmPackageName(name).validForOldPackages) {
+    throw new PnpmError(
+      'INVALID_DEPENDENCY_NAME',
+      `The configDependencies in pnpm-workspace.yaml contains a dependency with an invalid name: ${JSON.stringify(name)}`,
+      {
+        hint: 'A dependency name must be a valid npm package name — a single `name` or `@scope/name` consisting of URL-friendly characters, with no leading `.` or `_`, and not equal to reserved names such as `node_modules`.',
+      }
+    )
+  }
+}

config/deps-installer/src/assertValidConfigDepVersion.ts

@@ -0,0 +1,15 @@
+import { PnpmError } from '@pnpm/error'
+import semver from 'semver'
+
+// A config-dep version becomes a store path segment (`<name>/<version>/<hash>`),
+// so reject non-semver values to keep a traversal-shaped version from escaping
+// the store root.
+export function assertValidConfigDepVersion (name: string, version: string): void {
+  if (semver.valid(version) == null) {
+    throw new PnpmError(
+      'INVALID_CONFIG_DEP_VERSION',
+      `The config dependency "${name}" has an invalid version "${version}"`,
+      { hint: 'A config dependency version must be an exact semver version.' }
+    )
+  }
+}

config/deps-installer/src/normalizeConfigDeps.ts

@@ -2,6 +2,8 @@ import getNpmTarballUrl from 'get-npm-tarball-url'
 import { PnpmError } from '@pnpm/error'
 import { pickRegistryForPackage } from '@pnpm/pick-registry-for-package'
 import { type ConfigDependencies, type Registries } from '@pnpm/types'
+import { assertValidConfigDepName } from './assertValidConfigDepName.js'
+import { assertValidConfigDepVersion } from './assertValidConfigDepVersion.js'
 
 interface NormalizeConfigDepsOpts {
   registries: Registries
@@ -18,10 +20,14 @@ type NormalizedConfigDeps = Record<string, {
 export function normalizeConfigDeps (configDependencies: ConfigDependencies, opts: NormalizeConfigDepsOpts): NormalizedConfigDeps {
   const deps: NormalizedConfigDeps = {}
   for (const [pkgName, pkgSpec] of Object.entries(configDependencies)) {
+    // Validate before building any path from the name or version: both become
+    // path segments of the directories pnpm creates during install.
+    assertValidConfigDepName(pkgName)
     const registry = pickRegistryForPackage(opts.registries, pkgName)
 
     if (typeof pkgSpec === 'object') {
       const { version, integrity } = parseIntegrity(pkgName, pkgSpec.integrity)
+      assertValidConfigDepVersion(pkgName, version)
       deps[pkgName] = {
         version,
         resolution: {
@@ -34,6 +40,7 @@ export function normalizeConfigDeps (configDependencies: ConfigDependencies, opt
 
     if (typeof pkgSpec === 'string') {
       const { version, integrity } = parseIntegrity(pkgName, pkgSpec)
+      assertValidConfigDepVersion(pkgName, version)
       deps[pkgName] = {
         version,
         resolution: {

config/deps-installer/test/installConfigDeps.ts

@@ -1,4 +1,5 @@
 import fs from 'fs'
+import path from 'path'
 import { prepareEmpty } from '@pnpm/prepare'
 import { getIntegrity, REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
 import { createTempStore } from '@pnpm/testing.temp-store'
@@ -7,6 +8,22 @@ import { sync as loadJsonFile } from 'load-json-file'
 
 const registry = `http://localhost:${REGISTRY_MOCK_PORT}/`
 
+// Recursively search `dir` for an entry named `name`, without following
+// symlinks (so it can't loop through the links a successful install creates).
+function containsEntryNamed (dir: string, name: string): boolean {
+  let entries: fs.Dirent[]
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true })
+  } catch {
+    return false
+  }
+  for (const entry of entries) {
+    if (entry.name === name) return true
+    if (entry.isDirectory() && containsEntryNamed(path.join(dir, entry.name), name)) return true
+  }
+  return false
+}
+
 test('configuration dependency is installed', async () => {
   prepareEmpty()
   const { storeController } = createTempStore()
@@ -102,3 +119,62 @@ test('installation fails if the config dependency does not have a checksum', asy
     store: storeController,
   })).rejects.toThrow("doesn't have an integrity checksum")
 })
+
+test('a config dependency with a path-traversal name is rejected', async () => {
+  prepareEmpty()
+  const { storeController, storeDir } = createTempStore()
+
+  const configDeps: Record<string, string> = {
+    '../../PWNED': `100.0.0+${getIntegrity('@pnpm.e2e/foo', '100.0.0')}`,
+  }
+  await expect(installConfigDeps(configDeps, {
+    registries: {
+      default: registry,
+    },
+    rootDir: process.cwd(),
+    store: storeController,
+  })).rejects.toThrow('invalid name')
+
+  expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false)
+  expect(containsEntryNamed(storeDir, 'PWNED')).toBe(false)
+})
+
+test('a config dependency named __proto__ is rejected', async () => {
+  prepareEmpty()
+  const { storeController, storeDir } = createTempStore()
+
+  // JSON.parse makes `__proto__` an own enumerable key (as on-disk parsing
+  // does); a plain object literal would set the prototype and hide it.
+  const configDeps: Record<string, string> = JSON.parse(
+    `{"__proto__":"100.0.0+${getIntegrity('@pnpm.e2e/foo', '100.0.0')}"}`
+  )
+  await expect(installConfigDeps(configDeps, {
+    registries: {
+      default: registry,
+    },
+    rootDir: process.cwd(),
+    store: storeController,
+  })).rejects.toThrow('invalid name')
+
+  expect(containsEntryNamed(process.cwd(), '__proto__')).toBe(false)
+  expect(containsEntryNamed(storeDir, '__proto__')).toBe(false)
+})
+
+test('a config dependency with a path-traversal version is rejected', async () => {
+  prepareEmpty()
+  const { storeController, storeDir } = createTempStore()
+
+  const configDeps: Record<string, string> = {
+    '@pnpm.e2e/foo': `../../../PWNED+${getIntegrity('@pnpm.e2e/foo', '100.0.0')}`,
+  }
+  await expect(installConfigDeps(configDeps, {
+    registries: {
+      default: registry,
+    },
+    rootDir: process.cwd(),
+    store: storeController,
+  })).rejects.toThrow('invalid version')
+
+  expect(containsEntryNamed(process.cwd(), 'PWNED')).toBe(false)
+  expect(containsEntryNamed(storeDir, 'PWNED')).toBe(false)
+})

fs/symlink-dependency/package.json

@@ -37,7 +37,8 @@
   "dependencies": {
     "@pnpm/core-loggers": "workspace:*",
     "@pnpm/types": "workspace:*",
-    "symlink-dir": "catalog:"
+    "symlink-dir": "catalog:",
+    "validate-npm-package-name": "catalog:"
   },
   "peerDependencies": {
     "@pnpm/logger": "catalog:"
@@ -45,7 +46,8 @@
   "devDependencies": {
     "@pnpm/logger": "workspace:*",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/symlink-dependency": "workspace:*"
+    "@pnpm/symlink-dependency": "workspace:*",
+    "@types/validate-npm-package-name": "catalog:"
   },
   "engines": {
     "node": ">=18.12"

fs/symlink-dependency/src/index.ts

@@ -3,6 +3,7 @@ import symlinkDir from 'symlink-dir'
 
 import { safeJoinModulesDir } from './safeJoinModulesDir.js'
 
+export { safeJoinModulesDir } from './safeJoinModulesDir.js'
 export { symlinkDirectRootDependency } from './symlinkDirectRootDependency.js'
 
 export async function symlinkDependency (

fs/symlink-dependency/src/safeJoinModulesDir.ts

@@ -1,19 +1,37 @@
 import path from 'path'
 
-// `path.join(modulesDir, alias)` paired with a containment check, so a
-// caller can't accidentally use the joined path without verifying that
-// it lives inside `modulesDir`. Earlier passes reject path-traversal
-// aliases at manifest-read time, but this layer also runs for paths
-// reconstructed from lockfiles and snapshots, so the check stays here
-// as a final guarantee.
+import validateNpmPackageName from 'validate-npm-package-name'
+
+// Joins `modulesDir` with a dependency alias and guarantees the result
+// stays a direct child of `modulesDir`. The alias becomes a directory
+// name inside `node_modules`, so it must be a valid npm package name: a
+// single `name` or `@scope/name` of URL-friendly characters with no
+// leading `.` or `_`, and not a reserved name. That rejects
+// path-traversal (`../x`), absolute, and pnpm-owned aliases (`.bin`,
+// `.pnpm`, `node_modules`) before they can escape `modulesDir` or
+// overwrite pnpm's own layout. The containment check is kept as a
+// belt-and-suspenders guard for any platform-specific join behavior the
+// name check might not anticipate.
+//
+// Earlier passes reject such aliases at manifest-read and resolution
+// time, but this layer also runs for paths reconstructed from lockfiles
+// and snapshots, so the check stays here as a final guarantee.
 export function safeJoinModulesDir (modulesDir: string, alias: string): string {
+  if (!validateNpmPackageName(alias).validForOldPackages) {
+    throw invalidDependencyNameError(modulesDir, alias)
+  }
   const link = path.join(modulesDir, alias)
   const resolvedDir = path.resolve(modulesDir)
   const resolvedLink = path.resolve(link)
   if (resolvedLink === resolvedDir || !resolvedLink.startsWith(resolvedDir + path.sep)) {
-    const error = new Error(`Refusing to symlink dependency outside ${modulesDir}: alias ${JSON.stringify(alias)} resolves to ${resolvedLink}`) as Error & { code: string }
-    error.code = 'ERR_PNPM_INVALID_DEPENDENCY_NAME'
-    throw error
+    throw invalidDependencyNameError(modulesDir, alias, resolvedLink)
   }
   return link
 }
+
+function invalidDependencyNameError (modulesDir: string, alias: string, resolvedLink?: string): Error & { code: string } {
+  const detail = resolvedLink ? ` (it resolves to ${resolvedLink})` : ''
+  const error = new Error(`Refusing to place a dependency under ${modulesDir} with the invalid alias ${JSON.stringify(alias)}${detail}`) as Error & { code: string }
+  error.code = 'ERR_PNPM_INVALID_DEPENDENCY_NAME'
+  return error
+}

fs/symlink-dependency/test/safeJoinModulesDir.test.ts

@@ -4,7 +4,18 @@ import path from 'path'
 import { tempDir } from '@pnpm/prepare'
 import { symlinkDependency, symlinkDependencySync, symlinkDirectRootDependency } from '@pnpm/symlink-dependency'
 
-const escapeAliases = ['@x/../../../etc', '../sibling', '', '.']
+const escapeAliases = [
+  '@x/../../../etc',
+  '../sibling',
+  '',
+  '.',
+  // Reserved names that resolve *inside* `node_modules` but would
+  // overwrite pnpm-owned layout, so the containment check alone can't
+  // catch them.
+  '.bin',
+  '.pnpm',
+  'node_modules',
+]
 
 test.each(escapeAliases)('symlinkDependency refuses alias %p', async (alias) => {
   const tmp = tempDir(false)
@@ -33,3 +44,15 @@ test.each(escapeAliases)('symlinkDirectRootDependency refuses alias %p', async (
     prefix: '',
   })).rejects.toThrow(expect.objectContaining({ code: 'ERR_PNPM_INVALID_DEPENDENCY_NAME' }))
 })
+
+const validAliases = ['foo', '@scope/name', 'foo.bar']
+
+test.each(validAliases)('symlinkDependency accepts valid alias %p', async (alias) => {
+  const tmp = tempDir(false)
+  const destModulesDir = path.join(tmp, 'node_modules')
+  fs.mkdirSync(destModulesDir)
+  const dep = path.join(tmp, 'dep')
+  fs.mkdirSync(dep)
+  await expect(symlinkDependency(dep, destModulesDir, alias)).resolves.toBeDefined()
+  expect(fs.existsSync(path.join(destModulesDir, alias))).toBe(true)
+})

patching/plugin-commands-patching/src/isSubdirectory.ts

@@ -0,0 +1,17 @@
+import path from 'path'
+
+interface PathUtils {
+  isAbsolute: (path: string) => boolean
+  relative: (from: string, to: string) => string
+  sep: string
+}
+
+export function isSubdirectory (parentDir: string, childPath: string, pathUtils: PathUtils = path): boolean {
+  const relativePath = pathUtils.relative(parentDir, childPath)
+
+  return relativePath === '' || (
+    relativePath !== '..' &&
+    !relativePath.startsWith(`..${pathUtils.sep}`) &&
+    !pathUtils.isAbsolute(relativePath)
+  )
+}

patching/plugin-commands-patching/src/patchRemove.ts

@@ -1,5 +1,6 @@
-import path from 'path'
+import { type Stats } from 'fs'
 import fs from 'fs/promises'
+import path from 'path'
 import { docsUrl } from '@pnpm/cli-utils'
 import { install } from '@pnpm/plugin-commands-installation'
 import { type Config, types as allTypes } from '@pnpm/config'
@@ -7,6 +8,7 @@ import { PnpmError } from '@pnpm/error'
 import renderHelp from 'render-help'
 import { prompt } from 'enquirer'
 import pick from 'ramda/src/pick'
+import { isSubdirectory } from './isSubdirectory.js'
 import { updatePatchedDependencies } from './updatePatchedDependencies.js'
 
 export function rcOptionsTypes (): Record<string, unknown> {
@@ -31,7 +33,7 @@ export type PatchRemoveCommandOptions = install.InstallCommandOptions & Pick<Con
 
 export async function handler (opts: PatchRemoveCommandOptions, params: string[]): Promise<void> {
   let patchesToRemove = params
-  const patchedDependencies = opts.patchedDependencies ?? {}
+  const patchedDependencies = { ...opts.patchedDependencies }
 
   if (!params.length) {
     const allPatches = Object.keys(patchedDependencies)
@@ -60,16 +62,21 @@ export async function handler (opts: PatchRemoveCommandOptions, params: string[]
     }
   }
 
-  const patchesDirs = new Set<string>()
-  await Promise.all(patchesToRemove.map(async (patch) => {
-    if (Object.hasOwn(patchedDependencies, patch)) {
-      const patchFile = patchedDependencies[patch]
-      patchesDirs.add(path.dirname(patchFile))
-      await fs.rm(patchFile, { force: true })
-      delete patchedDependencies![patch]
+  const patchRemovalContext = await getPatchRemovalContext(opts)
+  const patchesToRemoveTargets = await Promise.all(patchesToRemove.map(async (patch) => {
+    const patchFile = patchedDependencies[patch]
+    if (patchFile == null) {
+      throw new PnpmError('PATCH_NOT_FOUND', `Patch "${patch}" not found in patched dependencies`)
     }
+    return getPatchRemovalTarget(patch, patchFile, patchRemovalContext)
   }))
 
+  await Promise.all(patchesToRemoveTargets.map(unlinkPatchIfExists))
+  for (const { patch } of patchesToRemoveTargets) {
+    delete patchedDependencies[patch]
+  }
+
+  const patchesDirs = new Set(patchesToRemoveTargets.map(({ parentDir }) => parentDir))
   await Promise.all(Array.from(patchesDirs).map(async (dir) => {
     try {
       const files = await fs.readdir(dir)
@@ -83,5 +90,114 @@ export async function handler (opts: PatchRemoveCommandOptions, params: string[]
     workspaceDir: opts.workspaceDir ?? opts.rootProjectManifestDir,
   })
 
-  return install.handler(opts)
+  return install.handler({
+    ...opts,
+    patchedDependencies,
+  })
+}
+
+interface PatchRemovalContext {
+  lockfileDir: string
+  patchesDir: string
+  realPatchesDir?: string
+}
+
+interface PatchRemovalTarget {
+  patch: string
+  patchFile: string
+  parentDir: string
+  targetPath: string
+  targetExists: boolean
+}
+
+async function getPatchRemovalContext (opts: PatchRemoveCommandOptions): Promise<PatchRemovalContext> {
+  const lockfileDir = path.resolve(opts.lockfileDir ?? opts.dir ?? process.cwd())
+  const realLockfileDir = await fs.realpath(lockfileDir)
+  const patchesDirSetting = opts.patchesDir ?? 'patches'
+  const patchesDir = path.join(lockfileDir, path.normalize(patchesDirSetting))
+
+  if (!isSubdirectory(lockfileDir, patchesDir)) {
+    throw new PnpmError('PATCHES_DIR_OUTSIDE_PROJECT', `The configured patches directory is outside the project: ${patchesDirSetting}`)
+  }
+
+  const realPatchesDir = await realpathIfExists(patchesDir)
+  if (realPatchesDir != null && !isSubdirectory(realLockfileDir, realPatchesDir)) {
+    throw new PnpmError('PATCHES_DIR_OUTSIDE_PROJECT', `The configured patches directory is outside the project: ${patchesDirSetting}`)
+  }
+
+  return {
+    lockfileDir,
+    patchesDir,
+    realPatchesDir,
+  }
+}
+
+async function getPatchRemovalTarget (
+  patch: string,
+  patchFile: string,
+  ctx: PatchRemovalContext
+): Promise<PatchRemovalTarget> {
+  const targetPath = path.resolve(ctx.lockfileDir, patchFile)
+  if (
+    targetPath === ctx.patchesDir ||
+    !isSubdirectory(ctx.patchesDir, targetPath)
+  ) {
+    throw new PnpmError('PATCH_FILE_OUTSIDE_PATCHES_DIR', `Patch file "${patchFile}" is outside the configured patches directory`)
+  }
+
+  const parentDir = path.dirname(targetPath)
+  const targetStats = await lstatIfExists(targetPath)
+  const realParentDir = await realpathIfExists(parentDir)
+  const realPatchesDir = ctx.realPatchesDir ?? (await realpathIfExists(ctx.patchesDir))
+  if (
+    realParentDir != null &&
+    realPatchesDir != null &&
+    !isSubdirectory(realPatchesDir, realParentDir)
+  ) {
+    throw new PnpmError('PATCH_FILE_OUTSIDE_PATCHES_DIR', `Patch file "${patchFile}" is outside the configured patches directory`)
+  }
+  if (targetStats?.isDirectory()) {
+    throw new PnpmError('PATCH_FILE_IS_DIRECTORY', `Patch file "${patchFile}" is a directory`)
+  }
+
+  return {
+    patch,
+    patchFile,
+    parentDir,
+    targetPath,
+    targetExists: targetStats != null,
+  }
+}
+
+async function unlinkPatchIfExists ({ targetExists, targetPath }: PatchRemovalTarget): Promise<void> {
+  if (!targetExists) return
+
+  try {
+    await fs.unlink(targetPath)
+  } catch (err: unknown) {
+    if (isErrorWithCode(err, 'ENOENT')) return
+    throw err
+  }
+}
+
+async function lstatIfExists (targetPath: string): Promise<Stats | undefined> {
+  try {
+    return await fs.lstat(targetPath)
+  } catch (err: unknown) {
+    if (isErrorWithCode(err, 'ENOENT')) return undefined
+    throw err
+  }
+}
+
+async function realpathIfExists (targetPath: string): Promise<string | undefined> {
+  try {
+    return await fs.realpath(targetPath)
+  } catch (err: unknown) {
+    if (isErrorWithCode(err, 'ENOENT')) return undefined
+    throw err
+  }
+}
+
+function isErrorWithCode (err: unknown, code: string): err is NodeJS.ErrnoException {
+  return typeof err === 'object' && err !== null && 'code' in err && err.code === code
 }

patching/plugin-commands-patching/test/isSubdirectory.test.ts

@@ -0,0 +1,19 @@
+import path from 'path'
+
+import { isSubdirectory } from '../src/isSubdirectory.js'
+
+test('isSubdirectory() accepts paths inside the parent directory', () => {
+  expect(isSubdirectory('/project/patches', '/project/patches/pkg.patch')).toBe(true)
+  expect(isSubdirectory('/project/patches', '/project/patches/..pkg/pkg.patch')).toBe(true)
+})
+
+test('isSubdirectory() rejects parent traversal and sibling prefixes', () => {
+  expect(isSubdirectory('/project/patches', '/project/pkg.patch')).toBe(false)
+  expect(isSubdirectory('/project/patches', '/project/patches-other/pkg.patch')).toBe(false)
+})
+
+test('isSubdirectory() rejects Windows drive and UNC escapes', () => {
+  expect(isSubdirectory('C:\\project\\patches', 'D:\\pkg.patch', path.win32)).toBe(false)
+  expect(isSubdirectory('C:\\project\\patches', '\\\\server\\share\\pkg.patch', path.win32)).toBe(false)
+  expect(isSubdirectory('C:\\project\\patches', 'C:\\project\\patches\\..pkg\\pkg.patch', path.win32)).toBe(true)
+})

patching/plugin-commands-patching/test/patchRemove.test.ts

@@ -0,0 +1,166 @@
+import fs from 'fs'
+import os from 'os'
+import path from 'path'
+
+import { afterEach, beforeEach, expect, jest, test } from '@jest/globals'
+import { install } from '@pnpm/plugin-commands-installation'
+
+import * as patchRemove from '../src/patchRemove.js'
+import { updatePatchedDependencies } from '../src/updatePatchedDependencies.js'
+import type { PatchRemoveCommandOptions } from '../src/patchRemove.js'
+
+jest.mock('@pnpm/plugin-commands-installation', () => ({
+  install: {
+    handler: jest.fn(),
+  },
+}))
+
+jest.mock('../src/updatePatchedDependencies.js', () => ({
+  updatePatchedDependencies: jest.fn(),
+}))
+
+const installHandler = jest.mocked(install.handler)
+const updatePatchedDependenciesMock = jest.mocked(updatePatchedDependencies)
+const testOnNonWindows = process.platform === 'win32' ? test.skip : test
+
+let tempRoot: string
+
+beforeEach(() => {
+  tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-patch-remove-'))
+  installHandler.mockResolvedValue(undefined)
+  updatePatchedDependenciesMock.mockResolvedValue(undefined)
+})
+
+afterEach(() => {
+  installHandler.mockReset()
+  updatePatchedDependenciesMock.mockReset()
+  fs.rmSync(tempRoot, { force: true, recursive: true })
+})
+
+test('patch-remove rejects traversal outside the patches directory before deleting any patch', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  const outsideFile = path.join(tempRoot, 'outside.patch')
+  const goodPatch = path.join(projectDir, 'patches/good.patch')
+  fs.mkdirSync(path.dirname(goodPatch), { recursive: true })
+  fs.writeFileSync(goodPatch, 'good patch', 'utf8')
+  fs.writeFileSync(outsideFile, 'outside patch', 'utf8')
+
+  await expect(patchRemove.handler(createOptions(projectDir, {
+    good: 'patches/good.patch',
+    bad: '../outside.patch',
+  }), ['good', 'bad'])).rejects.toMatchObject({
+    code: 'ERR_PNPM_PATCH_FILE_OUTSIDE_PATCHES_DIR',
+  })
+
+  expect(fs.existsSync(goodPatch)).toBe(true)
+  expect(fs.existsSync(outsideFile)).toBe(true)
+  expect(updatePatchedDependenciesMock).not.toHaveBeenCalled()
+  expect(installHandler).not.toHaveBeenCalled()
+})
+
+test('patch-remove rejects directory entries before deleting any patch', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  const goodPatch = path.join(projectDir, 'patches/good.patch')
+  const patchDir = path.join(projectDir, 'patches/not-a-file.patch')
+  fs.mkdirSync(patchDir, { recursive: true })
+  fs.writeFileSync(goodPatch, 'good patch', 'utf8')
+
+  await expect(patchRemove.handler(createOptions(projectDir, {
+    good: 'patches/good.patch',
+    bad: 'patches/not-a-file.patch',
+  }), ['good', 'bad'])).rejects.toMatchObject({
+    code: 'ERR_PNPM_PATCH_FILE_IS_DIRECTORY',
+  })
+
+  expect(fs.existsSync(goodPatch)).toBe(true)
+  expect(updatePatchedDependenciesMock).not.toHaveBeenCalled()
+  expect(installHandler).not.toHaveBeenCalled()
+})
+
+testOnNonWindows('patch-remove rejects a nested parent symlink outside the patches directory before unlinking a dangling target', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  const patchesDir = path.join(projectDir, 'patches')
+  const outsideDir = path.join(tempRoot, 'outside')
+  const outsideDanglingLink = path.join(outsideDir, 'dangling.patch')
+  fs.mkdirSync(patchesDir, { recursive: true })
+  fs.mkdirSync(outsideDir, { recursive: true })
+  fs.symlinkSync(outsideDir, path.join(patchesDir, 'linked-dir'), 'dir')
+  fs.symlinkSync(path.join(tempRoot, 'missing-target.patch'), outsideDanglingLink)
+
+  await expect(patchRemove.handler(createOptions(projectDir, {
+    bad: 'patches/linked-dir/dangling.patch',
+  }), ['bad'])).rejects.toMatchObject({
+    code: 'ERR_PNPM_PATCH_FILE_OUTSIDE_PATCHES_DIR',
+  })
+
+  expect(fs.lstatSync(outsideDanglingLink).isSymbolicLink()).toBe(true)
+  expect(updatePatchedDependenciesMock).not.toHaveBeenCalled()
+  expect(installHandler).not.toHaveBeenCalled()
+})
+
+testOnNonWindows('patch-remove unlinks a final symlink inside the patches directory without touching its target', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  const patchesDir = path.join(projectDir, 'patches')
+  const outsideTarget = path.join(tempRoot, 'outside-target.patch')
+  const patchLink = path.join(patchesDir, 'linked.patch')
+  fs.mkdirSync(patchesDir, { recursive: true })
+  fs.writeFileSync(outsideTarget, 'outside target', 'utf8')
+  fs.symlinkSync(outsideTarget, patchLink)
+
+  await patchRemove.handler(createOptions(projectDir, {
+    pkg: 'patches/linked.patch',
+  }), ['pkg'])
+
+  expect(fs.existsSync(patchLink)).toBe(false)
+  expect(fs.readFileSync(outsideTarget, 'utf8')).toBe('outside target')
+  expect(updatePatchedDependenciesMock).toHaveBeenCalledWith({}, expect.any(Object))
+  expect(installHandler).toHaveBeenCalledWith(expect.objectContaining({
+    patchedDependencies: {},
+  }))
+})
+
+test('patch-remove allows a symlinked patches directory that resolves inside the project', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  const realPatchesDir = path.join(projectDir, 'real-patches')
+  const patchFile = path.join(realPatchesDir, 'pkg.patch')
+  fs.mkdirSync(realPatchesDir, { recursive: true })
+  fs.symlinkSync(realPatchesDir, path.join(projectDir, 'patches'), process.platform === 'win32' ? 'junction' : 'dir')
+  fs.writeFileSync(patchFile, 'patch', 'utf8')
+
+  await patchRemove.handler(createOptions(projectDir, {
+    pkg: 'patches/pkg.patch',
+  }), ['pkg'])
+
+  expect(fs.existsSync(patchFile)).toBe(false)
+  expect(updatePatchedDependenciesMock).toHaveBeenCalledWith({}, expect.any(Object))
+  expect(installHandler).toHaveBeenCalledWith(expect.objectContaining({
+    patchedDependencies: {},
+  }))
+})
+
+test('patch-remove keeps missing patch files as no-ops', async () => {
+  const projectDir = path.join(tempRoot, 'project')
+  fs.mkdirSync(path.join(projectDir, 'patches'), { recursive: true })
+
+  await patchRemove.handler(createOptions(projectDir, {
+    pkg: 'patches/missing.patch',
+  }), ['pkg'])
+
+  expect(updatePatchedDependenciesMock).toHaveBeenCalledWith({}, expect.any(Object))
+  expect(installHandler).toHaveBeenCalledWith(expect.objectContaining({
+    patchedDependencies: {},
+  }))
+})
+
+function createOptions (
+  projectDir: string,
+  patchedDependencies: Record<string, string>
+): PatchRemoveCommandOptions {
+  return {
+    dir: projectDir,
+    lockfileDir: projectDir,
+    patchedDependencies,
+    rootProjectManifest: {},
+    rootProjectManifestDir: projectDir,
+  } as PatchRemoveCommandOptions
+}

pkg-manager/headless/src/lockfileToHoistedDepGraph.ts

@@ -15,6 +15,7 @@ import { type IncludedDependencies } from '@pnpm/modules-yaml'
 import { packageIsInstallable } from '@pnpm/package-is-installable'
 import { type PatchGroupRecord, getPatchInfo } from '@pnpm/patching.config'
 import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
+import { safeJoinModulesDir } from '@pnpm/symlink-dependency'
 import { type DepPath, type SupportedArchitectures, type ProjectId, type Registries, type AllowBuild } from '@pnpm/types'
 import {
   type FetchPackageToStoreFunction,
@@ -216,7 +217,7 @@ async function fetchDeps (
       return
     }
 
-    const dir = path.join(modules, dep.name)
+    const dir = safeJoinModulesDir(modules, dep.name)
     const depLocation = path.relative(opts.lockfileDir, dir)
     const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries)
     let fetchResponse!: ReturnType<FetchPackageToStoreFunction>

pkg-manager/headless/test/lockfileToHoistedDepGraph.test.ts

@@ -0,0 +1,86 @@
+/// <reference path="../../../__typings__/index.d.ts" />
+import fs from 'fs'
+import path from 'path'
+
+import { type LockfileObject } from '@pnpm/lockfile.fs'
+import { tempDir } from '@pnpm/prepare'
+
+// Imported from the built output rather than `../src` on purpose: the source
+// pulls in `@yarnpkg/nm`'s `HoisterResult` type, which ts-jest cannot resolve
+// transitively under `moduleResolution: node`. The `.d.ts` is a lib file, so
+// `skipLibCheck` skips it, and the test exercises the shipped graph builder.
+import { lockfileToHoistedDepGraph } from '../lib/lockfileToHoistedDepGraph.js'
+
+// A crafted lockfile whose dependency *alias* (the key pnpm turns into a
+// `node_modules/<alias>` directory) is a path-traversal or reserved name,
+// pointing at an otherwise ordinary package snapshot. The `nodeLinker:
+// hoisted` restore path reads aliases straight from the lockfile, so this
+// is the shape an attacker who can ship a lockfile would use to escape
+// `node_modules` or overwrite pnpm-owned layout (`.bin` / `.pnpm`).
+function craftedLockfile (alias: string): LockfileObject {
+  return {
+    lockfileVersion: '9.0',
+    importers: {
+      '.': {
+        dependencies: { [alias]: '1.0.0' },
+        specifiers: { [alias]: '1.0.0' },
+      },
+    },
+    packages: {
+      [`${alias}@1.0.0`]: {
+        resolution: { integrity: 'sha512-deadbeef' },
+      },
+    },
+  } as unknown as LockfileObject
+}
+
+// `force: true` skips the installability check so the walk reaches the
+// alias sink directly; the store controller throws if touched, proving
+// the alias is rejected before any fetch or filesystem work.
+function hoistedOpts (lockfileDir: string): Parameters<typeof lockfileToHoistedDepGraph>[2] {
+  const unreachable = (name: string) => () => {
+    throw new Error(`${name} must not be reached for a rejected alias`)
+  }
+  return {
+    autoInstallPeers: false,
+    engineStrict: false,
+    force: true,
+    importerIds: ['.'],
+    include: { dependencies: true, devDependencies: true, optionalDependencies: true },
+    ignoreScripts: false,
+    lockfileDir,
+    nodeVersion: process.version,
+    pnpmVersion: '0.0.0',
+    registries: { default: 'http://localhost/' },
+    sideEffectsCacheRead: false,
+    skipped: new Set<string>(),
+    storeController: {
+      fetchPackage: unreachable('fetchPackage'),
+      getFilesIndexFilePath: unreachable('getFilesIndexFilePath'),
+    },
+    storeDir: path.join(lockfileDir, 'store'),
+    virtualStoreDir: path.join(lockfileDir, 'node_modules', '.pnpm'),
+  } as unknown as Parameters<typeof lockfileToHoistedDepGraph>[2]
+}
+
+test.each([
+  '../../../escape',
+  '@scope/../../escape',
+  '.bin',
+  '.pnpm',
+  'node_modules',
+])('lockfileToHoistedDepGraph rejects hoisted alias %p', async (alias) => {
+  const dir = tempDir(false)
+  await expect(
+    lockfileToHoistedDepGraph(craftedLockfile(alias), null, hoistedOpts(dir))
+  ).rejects.toThrow(expect.objectContaining({ code: 'ERR_PNPM_INVALID_DEPENDENCY_NAME' }))
+})
+
+test('lockfileToHoistedDepGraph does not create a file outside node_modules for a traversal alias', async () => {
+  const dir = tempDir(false)
+  const escaped = path.join(dir, 'node_modules', '..', '..', '..', 'escape')
+  await expect(
+    lockfileToHoistedDepGraph(craftedLockfile('../../../escape'), null, hoistedOpts(dir))
+  ).rejects.toThrow(expect.objectContaining({ code: 'ERR_PNPM_INVALID_DEPENDENCY_NAME' }))
+  expect(fs.existsSync(escaped)).toBe(false)
+})

pnpm-lock.yaml

@@ -1823,6 +1823,12 @@ importers:
       get-npm-tarball-url:
         specifier: 'catalog:'
         version: 2.1.0
+      semver:
+        specifier: 'catalog:'
+        version: 7.7.4
+      validate-npm-package-name:
+        specifier: 'catalog:'
+        version: 5.0.0
     devDependencies:
       '@pnpm/config.deps-installer':
         specifier: workspace:*
@@ -1836,6 +1842,12 @@ importers:
       '@pnpm/testing.temp-store':
         specifier: workspace:*
         version: link:../../testing/temp-store
+      '@types/semver':
+        specifier: 'catalog:'
+        version: 7.5.3
+      '@types/validate-npm-package-name':
+        specifier: 'catalog:'
+        version: 4.0.2
       load-json-file:
         specifier: 'catalog:'
         version: 6.2.0
@@ -3498,6 +3510,9 @@ importers:
       symlink-dir:
         specifier: 'catalog:'
         version: 6.0.5
+      validate-npm-package-name:
+        specifier: 'catalog:'
+        version: 5.0.0
     devDependencies:
       '@pnpm/logger':
         specifier: workspace:*
@@ -3508,6 +3523,9 @@ importers:
       '@pnpm/symlink-dependency':
         specifier: workspace:*
         version: 'link:'
+      '@types/validate-npm-package-name':
+        specifier: 'catalog:'
+        version: 4.0.2
 
   hooks/pnpmfile:
     dependencies:
@@ -19237,7 +19255,7 @@ snapshots:
   '@yarnpkg/core@4.2.0(typanion@3.14.0)':
     dependencies:
       '@arcanis/slice-ansi': 1.1.1
-      '@types/semver': 7.5.3
+      '@types/semver': 7.7.1
       '@types/treeify': 1.0.3
       '@yarnpkg/fslib': 3.1.5
       '@yarnpkg/libzip': 3.2.2(@yarnpkg/fslib@3.1.5)