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
This commit is contained in:
Brandon Cheng authored and GitHub committed 2024-10-25 14:06:17 +02:00
1 parent 74b6f73ffa
commit 2aed76dbaf
4 files changed
+68 -2

No files matched your search

+5
View File
@@ -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.
+5
View File
@@ -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).
+32 -2
View File
@@ -42,14 +42,44 @@ export async function switchCliVersion (config: Config): Promise<void> {
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
}
}
}
+26
View File
@@ -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')
})