mirror of
https://github.com/pnpm/pnpm.git
synced 2026-07-14 01:32:40 -04:00
fix: clearer warning when a project .npmrc uses env variables in registry/auth settings (#12331)
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 no longer
claims a specific destination file, since pnpm config set delegates registry
and auth keys to npm.
This commit is contained in:
6
.changeset/wicked-warnings-guide.md
Normal file
6
.changeset/wicked-warnings-guide.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/config": 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 running `pnpm config set "<key>" <value>` to store it in the global config, or by keeping the `${...}` line in the user-level `~/.npmrc` — with a link to https://pnpm.io/npmrc.
|
||||
@@ -88,10 +88,29 @@ export function hasEnvPlaceholder (value: string): boolean {
|
||||
return /\$\{[^}]+\}/.test(value)
|
||||
}
|
||||
|
||||
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.
|
||||
function configSetExample (key: string): string {
|
||||
return hasEnvPlaceholder(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}`)
|
||||
}
|
||||
|
||||
@@ -275,6 +275,11 @@ describe('project .npmrc env expansion trust boundary', () => {
|
||||
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 () => {
|
||||
@@ -304,6 +309,12 @@ describe('project .npmrc env expansion trust boundary', () => {
|
||||
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('project .npmrc does not expand env variables in auth values', async () => {
|
||||
@@ -354,6 +365,11 @@ describe('project .npmrc env expansion trust boundary', () => {
|
||||
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 () => {
|
||||
|
||||
Reference in New Issue
Block a user