fix(config): redact userinfo in ignored project .npmrc key warnings (#16633)
A project .npmrc key that pnpm ignores because it would expand an
environment variable is named in a warning. The key was printed raw, and
URL-scoped keys carry their registry's authority, so
`//user:password@registry.example.com/${PATH}/:_authToken` printed
`user:password` to stderr. The generic redaction only strips userinfo
after `scheme://`, not after the scheme-less `//`.
pnpm v12 now redacts the request-destination warning with
redact_npm_auth_key, as the auth-value warning already did. pnpm v11 now
redacts both warnings with redactRegistryUrl, which moves from
registrySettings.ts into its own module so the .npmrc loader can use it.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
cf4dbaeecb
commit
f511b21127
8 files changed
+118
-60
No files matched your search
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/config.reader": patch
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
The warnings about ignored project `.npmrc` registry and auth settings no longer print the username and password of a URL-scoped key such as `//user:password@registry.example.com/:_authToken`.
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pacquet": patch
|
||||
---
|
||||
|
||||
The warning about an ignored project `.npmrc` registry setting no longer prints the username and password of a URL-scoped key such as `//user:password@registry.example.com/${PATH}/:_authToken`.
|
||||
@@ -261,6 +261,7 @@ impl NpmrcAuth {
|
||||
}
|
||||
|
||||
pub(super) fn warn_ignored_request_destination_env(&mut self, key: &str) {
|
||||
let key = redact_npm_auth_key(key);
|
||||
self.warnings.push(format!(
|
||||
"Ignored project-level request destination {key:?}: environment variables are not expanded in repository-controlled registry or proxy URLs.",
|
||||
));
|
||||
|
||||
@@ -756,6 +756,23 @@ fn ignored_auth_warning_redacts_protocol_relative_userinfo() {
|
||||
assert!(!warning.contains("ss@registry"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignored_request_destination_warning_redacts_protocol_relative_userinfo() {
|
||||
static_env!(Env, &[("REGISTRY_PATH", "npm")]);
|
||||
|
||||
let auth = NpmrcAuth::from_project_ini::<Env>(
|
||||
"//user:password@registry.example.com/${REGISTRY_PATH}/:_authToken=abc\n",
|
||||
Path::new(""),
|
||||
);
|
||||
let warning = auth.warnings
|
||||
.iter()
|
||||
.find(|warning| warning.contains("Ignored project-level request destination"))
|
||||
.expect("ignored request destination warning");
|
||||
|
||||
assert!(warning.contains("//registry.example.com/${REGISTRY_PATH}/:_authToken"));
|
||||
assert!(!warning.contains("user:password"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_ini_expands_auth_env_placeholder_without_warning() {
|
||||
static_env!(Env, &[("MY_TOKEN", "secret")]);
|
||||
|
||||
@@ -11,6 +11,7 @@ import { findLocalPrefix } from './findLocalPrefix.js'
|
||||
import { type JsonAuthResult, readGlobalConfigAuth, readJsonAuthEnv } from './jsonAuth.js'
|
||||
import { isNpmrcReadableKey } from './localConfig.js'
|
||||
import { parseCAFileContents } from './parseCAFileContents.js'
|
||||
import { redactRegistryUrl } from './registryUrlUserinfo.js'
|
||||
import { rescopeUnscopedCreds } from './rescopeUnscopedCreds.js'
|
||||
|
||||
export { findLocalPrefix } from './findLocalPrefix.js'
|
||||
@@ -412,14 +413,14 @@ function hasEnvPlaceholder (value: string): boolean {
|
||||
const DOCS_URL = 'https://pnpm.io/npmrc'
|
||||
|
||||
function warnIgnoredRequestDestinationEnv (filePath: string, key: string, warnings: string[]): void {
|
||||
warnings.push(`Ignored project-level request destination "${key}" in "${filePath}": ` +
|
||||
warnings.push(`Ignored project-level request destination "${redactRegistryUrl(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. ' +
|
||||
`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}": ` +
|
||||
warnings.push(`Ignored project-level auth setting "${redactRegistryUrl(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. ' +
|
||||
`See ${DOCS_URL}`)
|
||||
|
||||
@@ -10,6 +10,7 @@ import normalizeRegistryUrl from 'normalize-registry-url'
|
||||
|
||||
import type { OptionsFromRootManifest } from './getOptionsFromRootManifest.js'
|
||||
import { quoteAndJoin } from './quoteAndJoin.js'
|
||||
import { redactRegistryUrl, registryUrlHasUserinfo } from './registryUrlUserinfo.js'
|
||||
import { assertBoolean, assertObjectSetting, assertString, assertStringArray } from './settingAssertions.js'
|
||||
|
||||
const REGISTRY_SERVER_TYPES = new Set(['npm', 'artifactory'])
|
||||
@@ -236,61 +237,3 @@ function assertKnownRegistryFields (declaration: RegistryDeclaration, registry:
|
||||
function looksLikeRegistryUrl (key: string): boolean {
|
||||
return key.includes('://') || key.startsWith('//')
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the authority of `url` carries a `user:pass@` prefix. The authority
|
||||
* ends at the first `/`, `?`, or `#`, so a later `@` in the path is not one.
|
||||
*
|
||||
* Both the full form and the scheme-less `//host/` form count. The latter is
|
||||
* the shape `.npmrc` scopes settings with, so it is the one a user is most
|
||||
* likely to reach for here.
|
||||
*/
|
||||
function registryUrlHasUserinfo (url: string): boolean {
|
||||
return userinfoEnd(url) !== undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* The offset just past the `user:pass@` of `url`, or `undefined` when its
|
||||
* authority carries none. Splitting it out keeps the detection and the
|
||||
* redaction below agreeing on what the authority is.
|
||||
*/
|
||||
function userinfoEnd (url: string): number | undefined {
|
||||
const authorityStart = authorityStartOf(url)
|
||||
if (authorityStart === undefined) return undefined
|
||||
const authority = url.slice(authorityStart)
|
||||
const authorityEnd = authority.search(/[/?#]/)
|
||||
const at = (authorityEnd === -1 ? authority : authority.slice(0, authorityEnd)).lastIndexOf('@')
|
||||
return at === -1 ? undefined : authorityStart + at + 1
|
||||
}
|
||||
|
||||
/**
|
||||
* Where the authority of `url` begins, or `undefined` if it has none.
|
||||
*
|
||||
* The scheme is anchored at the start rather than found by searching for the
|
||||
* first `://`: a `://` inside the path (`//host/a://b`) would otherwise be
|
||||
* taken for the separator, and the real authority — credentials and all —
|
||||
* would go unexamined.
|
||||
*/
|
||||
function authorityStartOf (url: string): number | undefined {
|
||||
const schemeEnd = url.indexOf('://')
|
||||
if (schemeEnd !== -1 && SCHEME.test(url.slice(0, schemeEnd))) return schemeEnd + '://'.length
|
||||
if (url.startsWith('//')) return '//'.length
|
||||
return undefined
|
||||
}
|
||||
|
||||
const SCHEME = /^[a-z][a-z0-9+.-]*$/i
|
||||
|
||||
/**
|
||||
* `url` with any `user:pass@` removed, safe to put in a message.
|
||||
*
|
||||
* {@link redactAndSanitize} only recognizes an authority after a `://`, and
|
||||
* deliberately so: it runs over arbitrary prose, where a bare `//` is more
|
||||
* often a comment or a path than a URL. Here the string is known to be a
|
||||
* registry URL, so the scheme-less `//host/` form can be handled too.
|
||||
*/
|
||||
function redactRegistryUrl (url: string): string {
|
||||
const authorityStart = authorityStartOf(url)
|
||||
const end = userinfoEnd(url)
|
||||
if (authorityStart === undefined || end === undefined) return redactAndSanitize(url)
|
||||
return redactAndSanitize(`${url.slice(0, authorityStart)}${url.slice(end)}`)
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
import { redactAndSanitize } from '@pnpm/error'
|
||||
|
||||
/**
|
||||
* Whether the authority of `url` carries a `user:pass@` prefix. The authority
|
||||
* ends at the first `/`, `?`, or `#`, so a later `@` in the path is not one.
|
||||
*
|
||||
* Both the full form and the scheme-less `//host/` form count. The latter is
|
||||
* the shape `.npmrc` scopes settings with, so it is the one a user is most
|
||||
* likely to reach for here.
|
||||
*/
|
||||
export function registryUrlHasUserinfo (url: string): boolean {
|
||||
return userinfoEnd(url) !== undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* The offset just past the `user:pass@` of `url`, or `undefined` when its
|
||||
* authority carries none. Splitting it out keeps the detection and the
|
||||
* redaction below agreeing on what the authority is.
|
||||
*/
|
||||
function userinfoEnd (url: string): number | undefined {
|
||||
const authorityStart = authorityStartOf(url)
|
||||
if (authorityStart === undefined) return undefined
|
||||
const authority = url.slice(authorityStart)
|
||||
const authorityEnd = authority.search(/[/?#]/)
|
||||
const at = (authorityEnd === -1 ? authority : authority.slice(0, authorityEnd)).lastIndexOf('@')
|
||||
return at === -1 ? undefined : authorityStart + at + 1
|
||||
}
|
||||
|
||||
/**
|
||||
* Where the authority of `url` begins, or `undefined` if it has none.
|
||||
*
|
||||
* The scheme is anchored at the start rather than found by searching for the
|
||||
* first `://`: a `://` inside the path (`//host/a://b`) would otherwise be
|
||||
* taken for the separator, and the real authority — credentials and all —
|
||||
* would go unexamined.
|
||||
*/
|
||||
function authorityStartOf (url: string): number | undefined {
|
||||
const schemeEnd = url.indexOf('://')
|
||||
if (schemeEnd !== -1 && SCHEME.test(url.slice(0, schemeEnd))) return schemeEnd + '://'.length
|
||||
if (url.startsWith('//')) return '//'.length
|
||||
return undefined
|
||||
}
|
||||
|
||||
const SCHEME = /^[a-z][a-z0-9+.-]*$/i
|
||||
|
||||
/**
|
||||
* `url` with any `user:pass@` removed, safe to put in a message.
|
||||
*
|
||||
* {@link redactAndSanitize} only recognizes an authority after a `://`, and
|
||||
* deliberately so: it runs over arbitrary prose, where a bare `//` is more
|
||||
* often a comment or a path than a URL. Here the string is known to be a
|
||||
* registry URL or a URL-scoped `.npmrc` key, so the scheme-less `//host/`
|
||||
* form can be handled too.
|
||||
*/
|
||||
export function redactRegistryUrl (url: string): string {
|
||||
const authorityStart = authorityStartOf(url)
|
||||
const end = userinfoEnd(url)
|
||||
if (authorityStart === undefined || end === undefined) return redactAndSanitize(url)
|
||||
return redactAndSanitize(`${url.slice(0, authorityStart)}${url.slice(end)}`)
|
||||
}
|
||||
@@ -1828,6 +1828,31 @@ test('project .npmrc does not expand env variables in scoped registry URLs or UR
|
||||
expect(urlScopedWarning).toContain('https://pnpm.io/npmrc')
|
||||
})
|
||||
|
||||
test('ignored project .npmrc warnings do not print the userinfo of URL-scoped keys', async () => {
|
||||
prepare()
|
||||
|
||||
fs.writeFileSync('.npmrc', [
|
||||
'//user:password@registry.example.com/${PNPM_TEST_TOKEN}/:_authToken=token',
|
||||
'//user:password@attacker.example/:_authToken=${PNPM_TEST_TOKEN}',
|
||||
'',
|
||||
].join('\n'), 'utf8')
|
||||
|
||||
const { warnings } = await getConfig({
|
||||
cliOptions: {},
|
||||
env: { ...env, PNPM_TEST_TOKEN: 'secret' },
|
||||
packageManager: {
|
||||
name: 'pnpm',
|
||||
version: '1.0.0',
|
||||
},
|
||||
})
|
||||
|
||||
expect(warnings).toEqual(expect.arrayContaining([
|
||||
expect.stringContaining('Ignored project-level request destination "//registry.example.com/${PNPM_TEST_TOKEN}/:_authToken"'),
|
||||
expect.stringContaining('Ignored project-level auth setting "//attacker.example/:_authToken"'),
|
||||
]))
|
||||
expect(warnings.join('\n')).not.toContain('user:password')
|
||||
})
|
||||
|
||||
test('project .npmrc does not expand env variables in auth values', async () => {
|
||||
prepare()
|
||||
|
||||
|
||||
Reference in new issue
Block a user