GHSA-72R4-9C5J-MJ57: Root-Cause Fix for pnpm patch-remove Path Traversal Deletion

Vulnerability

GHSA-72R4-9C5J-MJ57 describes an arbitrary file deletion issue in pnpm's patch-remove flow. In the vulnerable implementation, patchedDependencies[patch] was consumed as a filesystem path and passed directly to fs.rm(..., { force: true }) after only collecting its parent directory. There was no containment validation ensuring that the configured patch file stayed under the intended patches directory, so traversal-shaped entries such as ../outside.patch could target files outside the project-controlled patch area. The advisory summary and the patch changeset both characterize the impact as deletion outside the configured patches directory.

The pre-fix code path shown in the commit deleted whatever path was referenced by configuration:

const patchFile = patchedDependencies[patch]
patchesDirs.add(path.dirname(patchFile))
await fs.rm(patchFile, { force: true })
delete patchedDependencies![patch]

This is the core flaw: untrusted path material from project configuration reached a destructive filesystem sink without normalization, containment enforcement, or symlink-aware validation. Because the sink was fs.rm, the blast radius also included directory removal semantics rather than only unlinking a patch file. Source references: commit 612a2e6, GitHub advisory, CVE Reports summary.

Patch

The fix in commit 612a2e6 introduces a dedicated containment model for patch removal. First, it computes a trusted context from the lockfile directory and configured patches directory. It resolves the project root, normalizes the configured patches directory, and rejects a patches directory that escapes the project. It then optionally resolves the real path of the patches directory and rejects symlinked configurations that point outside the project.

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}`)
}

Second, each patch removal target is validated before any deletion occurs. The code resolves the configured patch file against the lockfile directory, rejects the exact patches directory itself, rejects any target outside the configured patches subtree, checks the real parent directory against the real patches directory to catch nested symlink escapes, and rejects directory targets.

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`)
}

Third, the destructive sink is narrowed from fs.rm to fs.unlink, which better matches the intended object type and avoids recursive or directory deletion behavior. The implementation also stages all validations up front and only performs unlink operations after all targets have been accepted, preventing partial deletion when one entry is malicious.

The helper isSubdirectory() added in the same commit uses path.relative() and rejects parent traversal, sibling-prefix confusion, and absolute-path escapes, with explicit Windows coverage for drive-letter and UNC cases. The associated tests in patchRemove.test.ts cover traversal outside the patches directory, directory entries, nested parent symlink escapes, safe unlinking of a final symlink inside the patches directory, and symlinked patches directories that still resolve inside the project. Source references: commit 612a2e6 and the broader hardening follow-up in commit 352ae48.

Review

Pros

• Fixes the actual trust-boundary failure by validating filesystem containment before invoking the deletion sink, rather than merely filtering obvious ../ strings.
• Uses both lexical containment checks and realpath-based checks, which is important because lexical normalization alone does not stop symlink-mediated escapes.
• Stages validation for all requested removals before deleting any file, preventing mixed valid/invalid input from causing partial state mutation.
• Replaces fs.rm with fs.unlink, reducing the impact surface and aligning the operation with the expected patch-file abstraction.
• Explicitly rejects directory targets and the patches directory itself, closing off accidental or malicious misuse of broader removal semantics.
• Adds platform-aware tests, including Windows path behavior in isSubdirectory() and non-Windows symlink scenarios in patch removal tests.

Cons

• The realpath containment check is performed on the parent directory rather than the final target path. This is sufficient for preventing deletion outside the patches tree while still allowing unlink of a symlink inside the tree, but it is a subtle design that requires careful maintenance.
• Validation depends on the correctness of isSubdirectory() across path edge cases. The current tests are good, but this helper becomes security-critical and should remain centrally reused and heavily regression-tested.
• The broader hardening commit 352ae48 shows multiple related path-sink issues elsewhere in pnpm, which suggests the project historically lacked a uniform path-safety abstraction.

Verdict

Root-cause.

This patch addresses the root cause for patch-remove: untrusted configuration-derived paths reaching a destructive filesystem operation without containment enforcement. The new flow constrains both the configured patches directory and each patch target, accounts for symlink escapes, narrows deletion semantics to unlink-only behavior, and validates all targets before mutating state. Based on the supplied sources, this is not a superficial filter or a narrow special-case block; it is a structural correction to how patch removal paths are derived and authorized.

For engineers reviewing rollout risk, the change is security-positive and behaviorally appropriate. The main compatibility impact is that previously tolerated but unsafe configurations now fail with explicit pnpm errors such as PATCHES_DIR_OUTSIDE_PROJECT, PATCH_FILE_OUTSIDE_PATCHES_DIR, and PATCH_FILE_IS_DIRECTORY. That is the correct tradeoff for a command that deletes files.