diff --git a/.changeset/shell-safe-config-set.md b/.changeset/shell-safe-config-set.md new file mode 100644 index 0000000000..fd7d2f671f --- /dev/null +++ b/.changeset/shell-safe-config-set.md @@ -0,0 +1,6 @@ +--- +"@pnpm/config": patch +"pnpm": patch +--- + +Hardened the warning printed when a project `.npmrc` uses environment variables in registry/auth settings: the suggested `pnpm config set` command is now only included for keys made up of shell-inert characters. Because the key comes from a repository-controlled `.npmrc` and a shell expands `$(...)`, backticks, and `$VAR` even inside double quotes, a crafted key could otherwise have turned the suggested copy-paste command into command execution. diff --git a/config/config/src/dropUntrustedEnvExpansions.ts b/config/config/src/dropUntrustedEnvExpansions.ts index f72ab3c1fb..30b5b21e2b 100644 --- a/config/config/src/dropUntrustedEnvExpansions.ts +++ b/config/config/src/dropUntrustedEnvExpansions.ts @@ -90,12 +90,17 @@ export function hasEnvPlaceholder (value: string): boolean { const DOCS_URL = 'https://pnpm.io/npmrc' -// A runnable `pnpm config set` example is only safe to suggest when the key has -// no `${...}` placeholder — embedding such a key in a shell command would let -// the shell expand it on copy-paste, producing a different key and possibly -// leaking an env value into shell history. +// The key embedded in the suggested `pnpm config set` command comes from a +// repository-controlled .npmrc. A shell expands `$(...)`, backticks and `$VAR` +// even inside double quotes, so suggesting a runnable command built from an +// arbitrary key would turn this warning into a copy-paste command-injection +// vector. Only emit the runnable example for keys made up entirely of +// shell-inert characters — which covers every real registry/auth key +// (`//host/:_authToken`, `@scope:registry`, `registry`, `https-proxy`, …). +const SHELL_SAFE_KEY = /^[\w@.:/-]+$/ + function configSetExample (key: string): string { - return hasEnvPlaceholder(key) ? '' : ` (for example, run: pnpm config set "${key}" )` + return SHELL_SAFE_KEY.test(key) ? ` (for example, run: pnpm config set "${key}" )` : '' } function warnIgnoredRequestDestinationEnv (filePath: string, key: string, warnings: string[]): void { diff --git a/config/config/test/index.ts b/config/config/test/index.ts index 91c59e3398..1dfa17611e 100644 --- a/config/config/test/index.ts +++ b/config/config/test/index.ts @@ -317,6 +317,30 @@ describe('project .npmrc env expansion trust boundary', () => { expect(urlScopedWarning).toContain('~/.npmrc') }) + test('the warning never embeds a shell-unsafe key in a runnable pnpm config set command', async () => { + prepare() + + // A malicious repository could craft a key with shell metacharacters; the + // suggested copy-paste command must not become a command-injection vector. + fs.writeFileSync('.npmrc', '//$(touch pwned)`id`/:_authToken=${PNPM_TEST_TOKEN}\n', 'utf8') + fs.mkdirSync('user-home') + fs.writeFileSync(path.resolve('user-home', '.npmrc'), '', 'utf8') + + const { warnings } = await getConfig({ + cliOptions: { + userconfig: path.resolve('user-home', '.npmrc'), + }, + packageManager: { + name: 'pnpm', + version: '1.0.0', + }, + }) + + const unsafeWarning = warnings.find((w) => w.includes('$(touch pwned)')) ?? '' + expect(unsafeWarning).not.toBe('') + expect(unsafeWarning).not.toContain('pnpm config set "') + }) + test('project .npmrc does not expand env variables in auth values', async () => { prepare()