fix: clearer warning when a project .npmrc uses env variables in registry/auth settings (#12333)

* fix: clearer warning when a project .npmrc uses env variables in registry/auth settings

The previous warning only said the setting was ignored. It now explains why
(the project .npmrc is committed to the repository and must not expand secrets
into request destinations or credentials) and how to fix it: move the value to a
trusted source such as the user-level ~/.npmrc or via pnpm config set, with a
link to the docs.

The suggested 'pnpm config set' example is only shown when the key has no
${...} placeholder, so the snippet is always safe to copy-paste (a shell would
otherwise expand a placeholder embedded in the key). The wording does not claim
a specific destination file.

* fix: only suggest a pnpm config set command for shell-safe keys

The key embedded in the warning's suggested 'pnpm config set' command comes
from a repository-controlled .npmrc. The previous guard only suppressed the
example for keys containing a ${...} placeholder, but a shell also expands
$(...), backticks and $VAR inside double quotes — so a crafted key could turn
the suggested copy-paste command into command execution. The example is now
emitted only for keys made up entirely of shell-inert characters.

.changeset/clever-warnings-guide.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/config.reader": patch
+"pnpm": patch
+---
+
+Improved the warning printed when a project `.npmrc` uses an environment variable in a registry/proxy URL or in registry credentials. The message now explains why the setting was ignored and how to migrate it to a trusted source — for example by moving the line to the user-level `~/.npmrc` or running `pnpm config set "<key>" <value>` — with a link to https://pnpm.io/npmrc. The `pnpm config set` example is only suggested when the key has no `${...}` placeholder, so the snippet is always safe to copy-paste.

config/reader/src/loadNpmrcFiles.ts

@@ -265,12 +265,36 @@ function hasEnvPlaceholder (value: string): boolean {
   return /\$\{[^}]+\}/.test(value)
 }
 
+const DOCS_URL = 'https://pnpm.io/npmrc'
+
+// 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 SHELL_SAFE_KEY.test(key) ? ` (for example, run: pnpm config set "${key}" <value>)` : ''
+}
+
 function warnIgnoredRequestDestinationEnv (filePath: string, key: string, warnings: string[]): void {
-  warnings.push(`Ignored project-level request destination "${key}" in "${filePath}": environment variables are not expanded in repository-controlled registry or proxy URLs.`)
+  warnings.push(`Ignored project-level request destination "${key}" in "${filePath}": ` +
+    'environment variables are not expanded in registry or proxy URLs that come from a project .npmrc, ' +
+    'because that file is committed to the repository and a malicious value could redirect requests or leak secrets. ' +
+    'Move this setting to a trusted source that pnpm still expands — put it in your user-level ~/.npmrc, ' +
+    `or set it with pnpm config set${configSetExample(key)}. ` +
+    `If the value is not secret, you can also write it literally in the project .npmrc. See ${DOCS_URL}`)
 }
 
 function warnIgnoredAuthValueEnv (filePath: string, key: string, warnings: string[]): void {
-  warnings.push(`Ignored project-level auth setting "${key}" in "${filePath}": environment variables are not expanded in repository-controlled registry credentials.`)
+  warnings.push(`Ignored project-level auth setting "${key}" in "${filePath}": ` +
+    'environment variables are not expanded in registry credentials that come from a project .npmrc, ' +
+    'because that file is committed to the repository and could leak the secret to an attacker-controlled registry. ' +
+    'Move this credential to a trusted source that pnpm still expands — put the line in your user-level ~/.npmrc, ' +
+    `or set it with pnpm config set${configSetExample(key)}. See ${DOCS_URL}`)
 }
 
 // Rewrite any unscoped per-registry keys in `source` to their URL-scoped

config/reader/test/index.ts

@@ -627,6 +627,11 @@ test('project .npmrc does not expand env variables in registry URLs', async () =
   expect(warnings).toEqual(expect.arrayContaining([
     expect.stringContaining('Ignored project-level request destination "registry"'),
   ]))
+  // The warning should guide the user toward a trusted source and the docs.
+  const registryWarning = warnings.find((w) => w.includes('Ignored project-level request destination "registry"')) ?? ''
+  expect(registryWarning).toContain('~/.npmrc')
+  expect(registryWarning).toContain('pnpm config set "registry" <value>')
+  expect(registryWarning).toContain('https://pnpm.io/npmrc')
 })
 
 test('project .npmrc does not expand env variables in scoped registry URLs or URL-scoped keys', async () => {
@@ -653,6 +658,33 @@ test('project .npmrc does not expand env variables in scoped registry URLs or UR
     expect.stringContaining('Ignored project-level request destination "@scope:registry"'),
     expect.stringContaining('Ignored project-level request destination "//registry.example.com/${PNPM_TEST_TOKEN}/:_authToken"'),
   ]))
+  // When the key itself contains a ${...} placeholder, the warning must not
+  // embed it in a runnable `pnpm config set "<key>"` command — a shell would
+  // expand the placeholder on copy-paste.
+  const urlScopedWarning = warnings.find((w) => w.includes('//registry.example.com/${PNPM_TEST_TOKEN}/:_authToken')) ?? ''
+  expect(urlScopedWarning).not.toContain('pnpm config set "')
+  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')
+
+  const { warnings } = await getConfig({
+    cliOptions: {},
+    env: { ...env, PNPM_TEST_TOKEN: 'secret' },
+    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 () => {
@@ -702,6 +734,11 @@ test('project .npmrc does not expand env variables in auth values', async () =>
     expect.stringContaining('Ignored project-level auth setting "cert"'),
     expect.stringContaining('Ignored project-level auth setting "key"'),
   ]))
+  // The warning should tell the user how to migrate the credential.
+  const authWarning = warnings.find((w) => w.includes('Ignored project-level auth setting "//attacker.example/:_authToken"')) ?? ''
+  expect(authWarning).toContain('pnpm config set "//attacker.example/:_authToken" <value>')
+  expect(authWarning).toContain('~/.npmrc')
+  expect(authWarning).toContain('https://pnpm.io/npmrc')
 })
 
 test('project .npmrc does not expand env variables in proxy URLs', async () => {