From 2aed76dbaffcd9302f7c16d2642a16a03fc8c528 Mon Sep 17 00:00:00 2001 From: Brandon Cheng Date: Fri, 25 Oct 2024 08:06:17 -0400 Subject: [PATCH] fix: fork bomb when pnpm self-manages its version (#8679) * fix: prevent fork bombing if .tools dir had an incomplete download * refactor: factor out reusable `VersionSwitchFail` class * fix: avoid swallowing errors from spawn.sync() * test: fix --- .changeset/great-onions-matter.md | 5 +++++ .changeset/swift-panthers-matter.md | 5 +++++ pnpm/src/switchCliVersion.ts | 34 +++++++++++++++++++++++++++-- pnpm/test/switchingVersions.test.ts | 26 ++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 .changeset/great-onions-matter.md create mode 100644 .changeset/swift-panthers-matter.md diff --git a/.changeset/great-onions-matter.md b/.changeset/great-onions-matter.md new file mode 100644 index 0000000000..b208cf7878 --- /dev/null +++ b/.changeset/great-onions-matter.md @@ -0,0 +1,5 @@ +--- +"pnpm": patch +--- + +Fix a bug causing pnpm to infinitely spawn itself when `manage-package-manager-versions=true` is set and the `.tools` directory is corrupt. diff --git a/.changeset/swift-panthers-matter.md b/.changeset/swift-panthers-matter.md new file mode 100644 index 0000000000..eea618593c --- /dev/null +++ b/.changeset/swift-panthers-matter.md @@ -0,0 +1,5 @@ +--- +"pnpm": patch +--- + +When `manage-package-manager-versions` is set to `true`, errors spawning a self-managed version of `pnpm` will now be shown (instead of being silent). diff --git a/pnpm/src/switchCliVersion.ts b/pnpm/src/switchCliVersion.ts index 05403849cd..4ab8042a4e 100644 --- a/pnpm/src/switchCliVersion.ts +++ b/pnpm/src/switchCliVersion.ts @@ -42,14 +42,44 @@ export async function switchCliVersion (config: Config): Promise { const pnpmEnv = prependDirsToPath([wantedPnpmBinDir]) if (!pnpmEnv.updated) { // We throw this error to prevent an infinite recursive call of the same pnpm version. - throw new PnpmError('VERSION_SWITCH_FAIL', `Failed to switch pnpm to v${pm.version}. Looks like pnpm CLI is missing at "${wantedPnpmBinDir}" or is incorrect`) + throw new VersionSwitchFail(pm.version, wantedPnpmBinDir) } - const { status } = spawn.sync('pnpm', process.argv.slice(2), { + + // Specify the exact pnpm file path that's expected to execute to spawn.sync() + // + // It's not safe spawn 'pnpm' (without specifying an absolute path) and expect + // it to resolve to the same file path computed above due to the $PATH + // environment variable. While that does happen in most cases, there's a + // scenario where the wanted pnpm bin dir exists, but no pnpm binary is + // present within that directory. If that's the case, a different pnpm bin can + // get executed, causing infinite spawn and fork bombing the user. See details + // at https://github.com/pnpm/pnpm/pull/8679. + const pnpmBinPath = path.join(wantedPnpmBinDir, 'pnpm') + + const { status, error } = spawn.sync(pnpmBinPath, process.argv.slice(2), { stdio: 'inherit', env: { ...process.env, [pnpmEnv.name]: pnpmEnv.value, }, }) + + if (error) { + throw new VersionSwitchFail(pm.version, wantedPnpmBinDir, error) + } + process.exit(status ?? 0) } + +class VersionSwitchFail extends PnpmError { + constructor (version: string, wantedPnpmBinDir: string, cause?: unknown) { + super( + 'VERSION_SWITCH_FAIL', + `Failed to switch pnpm to v${version}. Looks like pnpm CLI is missing at "${wantedPnpmBinDir}" or is incorrect`, + { hint: cause instanceof Error ? cause?.message : undefined }) + + if (cause != null) { + this.cause = cause + } + } +} diff --git a/pnpm/test/switchingVersions.test.ts b/pnpm/test/switchingVersions.test.ts index 783562d3ba..f4b7cef915 100644 --- a/pnpm/test/switchingVersions.test.ts +++ b/pnpm/test/switchingVersions.test.ts @@ -1,8 +1,10 @@ import path from 'path' import fs from 'fs' import { prepare } from '@pnpm/prepare' +import { getToolDirPath } from '@pnpm/tools.path' import { sync as writeJsonFile } from 'write-json-file' import { execPnpmSync } from './utils' +import isWindows from 'is-windows' test('switch to the pnpm version specified in the packageManager field of package.json, when manager-package-manager=versions is true', async () => { prepare() @@ -58,3 +60,27 @@ test('do not switch to pnpm version when a range is specified', async () => { expect(stdout.toString()).toContain('Cannot switch to pnpm@^9.3.0') }) + +test('throws error if pnpm tools dir is corrupt', () => { + prepare() + const pnpmHome = path.resolve('pnpm') + const env = { PNPM_HOME: pnpmHome } + const version = '9.3.0' + fs.writeFileSync('.npmrc', 'manage-package-manager-versions=true') + writeJsonFile('package.json', { + packageManager: `pnpm@${version}`, + }) + + // Run pnpm once to ensure the tools dir is created. + execPnpmSync(['help'], { env }) + + // Intentionally corrupt the tool dir. + const toolDir = getToolDirPath({ pnpmHomeDir: pnpmHome, tool: { name: 'pnpm', version } }) + fs.rmSync(path.join(toolDir, 'bin/pnpm')) + if (isWindows()) { + fs.rmSync(path.join(toolDir, 'bin/pnpm.cmd')) + } + + const { stderr } = execPnpmSync(['help'], { env }) + expect(stderr.toString()).toContain('Failed to switch pnpm to v9.3.0. Looks like pnpm CLI is missing') +})