From a57ba4edee072c54ae2ba3324cd533218c1600fb Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Fri, 6 Feb 2026 23:04:19 +0800 Subject: [PATCH] fix: warn when directory contains PATH delimiter character (#10487) * fix: warn when directory contains PATH delimiter character Add a warning when the current directory contains the PATH delimiter character (colon on macOS/Linux, semicolon on Windows). On macOS, folder names containing forward slashes (/) appear as colons (:) at the Unix layer. Since colons are PATH separators in POSIX systems, this breaks PATH injection for node_modules/.bin. close #10457 * test: add tests for PATH delimiter warning - Test warning is emitted when directory contains delimiter - Test no warning for normal directories --- .changeset/warn-path-delimiter.md | 8 ++ config/config/src/index.ts | 6 ++ config/config/test/index.ts | 134 ++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 .changeset/warn-path-delimiter.md diff --git a/.changeset/warn-path-delimiter.md b/.changeset/warn-path-delimiter.md new file mode 100644 index 0000000000..32c1f44b00 --- /dev/null +++ b/.changeset/warn-path-delimiter.md @@ -0,0 +1,8 @@ +--- +"@pnpm/config": patch +"pnpm": patch +--- + +Add a warning when the current directory contains the PATH delimiter character. On macOS, folder names containing forward slashes (/) appear as colons (:) at the Unix layer. Since colons are PATH separators in POSIX systems, this breaks PATH injection for `node_modules/.bin`, causing binaries to not be found when running commands like `pnpm exec`. + +Closes [#10457](https://github.com/pnpm/pnpm/issues/10457). diff --git a/config/config/src/index.ts b/config/config/src/index.ts index 266019d84f..12649a22ee 100644 --- a/config/config/src/index.ts +++ b/config/config/src/index.ts @@ -247,6 +247,12 @@ export async function getConfig (opts: { // This prevents potential inconsistencies in the future, especially when processing or mapping subdirectories. const cwd = fs.realpathSync(betterPathResolve(cliOptions.dir ?? npmConfig.localPrefix)) + // Unfortunately, there is no way to escape the PATH delimiter, + // so directories added to PATH should not contain it. + if (cwd.includes(path.delimiter)) { + warnings.push(`Directory "${cwd}" contains the path delimiter character (${path.delimiter}), so binaries from node_modules/.bin will not be accessible via PATH. Consider renaming the directory.`) + } + pnpmConfig.maxSockets = npmConfig.maxsockets // @ts-expect-error delete pnpmConfig['maxsockets'] diff --git a/config/config/test/index.ts b/config/config/test/index.ts index 640957aeb5..41617cbb5e 100644 --- a/config/config/test/index.ts +++ b/config/config/test/index.ts @@ -1,5 +1,6 @@ /// import fs from 'fs' +import os from 'os' import path from 'path' import PATH from 'path-name' import { getCurrentBranch } from '@pnpm/git-utils' @@ -7,6 +8,7 @@ import { getConfig } from '@pnpm/config' import loadNpmConf from '@pnpm/npm-conf' import { prepare, prepareEmpty } from '@pnpm/prepare' import { fixtures } from '@pnpm/test-fixtures' +import writeYamlFile from 'write-yaml-file' import { jest } from '@jest/globals' import symlinkDir from 'symlink-dir' @@ -1176,3 +1178,135 @@ test('when dangerouslyAllowAllBuilds is set to true and neverBuiltDependencies n expect(config.neverBuiltDependencies).toStrictEqual([]) expect(warnings).toStrictEqual(['You have set dangerouslyAllowAllBuilds to true. The dependencies listed in neverBuiltDependencies will run their scripts.']) }) + +test('getConfig() should read pnpm_config_* environment variables', async () => { + prepareEmpty() + + async function getConfigValue (env: NodeJS.ProcessEnv): Promise { + const { config } = await getConfig({ + cliOptions: {}, + env, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + workspaceDir: process.cwd(), + }) + return config.fetchRetries + } + + expect(await getConfigValue({})).toBe(5) + expect(await getConfigValue({ + pnpm_config_fetch_retries: '10', + })).toBe(10) +}) + +test('CLI should override environment variable pnpm_config_*', async () => { + prepareEmpty() + + async function getConfigValue (cliOptions: Record): Promise { + const { config } = await getConfig({ + cliOptions, + env: { + pnpm_config_fetch_retries: '5', + }, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + workspaceDir: process.cwd(), + }) + return config.fetchRetries + } + + expect(await getConfigValue({})).toBe(5) + expect(await getConfigValue({ + fetchRetries: 10, + })).toBe(10) + expect(await getConfigValue({ + 'fetch-retries': 10, + })).toBe(10) +}) + +test('warn when directory contains PATH delimiter character', async () => { + const tempDir = path.join(os.tmpdir(), `pnpm-test${path.delimiter}project-${Date.now()}`) + fs.mkdirSync(tempDir, { recursive: true }) + + try { + const { warnings } = await getConfig({ + cliOptions: { dir: tempDir }, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + }) + + expect(warnings).toContainEqual( + expect.stringContaining('path delimiter character') + ) + } finally { + fs.rmSync(tempDir, { recursive: true }) + } +}) + +test('no warning when directory does not contain PATH delimiter character', async () => { + const tempDir = path.join(os.tmpdir(), `pnpm-test-normal-${Date.now()}`) + fs.mkdirSync(tempDir, { recursive: true }) + + try { + const { warnings } = await getConfig({ + cliOptions: { dir: tempDir }, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + }) + + expect(warnings).not.toContainEqual( + expect.stringContaining('path delimiter character') + ) + } finally { + fs.rmSync(tempDir, { recursive: true }) + } +}) + +describe('global config.yaml', () => { + let XDG_CONFIG_HOME: string | undefined + + beforeEach(() => { + XDG_CONFIG_HOME = process.env.XDG_CONFIG_HOME + }) + + afterEach(() => { + process.env.XDG_CONFIG_HOME = XDG_CONFIG_HOME + }) + + test('reads config from global config.yaml', async () => { + prepareEmpty() + + fs.mkdirSync('.config/pnpm', { recursive: true }) + writeYamlFile('.config/pnpm/config.yaml', { + dangerouslyAllowAllBuilds: true, + }) + + // TODO: `getConfigDir`, `getHomeDir`, etc. (from dirs.ts) should allow customizing env or process. + // TODO: after that, remove this `describe` wrapper. + process.env.XDG_CONFIG_HOME = path.resolve('.config') + + const { config } = await getConfig({ + cliOptions: {}, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + workspaceDir: process.cwd(), + }) + + expect(config.dangerouslyAllowAllBuilds).toBe(true) + + // NOTE: the field may appear kebab-case here, but only internally, + // `pnpm config list` would convert them to camelCase. + // TODO: switch to camelCase entirely later. + expect(config.rawConfig).toHaveProperty(['dangerously-allow-all-builds']) + }) +})