Skip to content

Commit a23956e

Browse files
zkochanzkochan
authored
fix(config/reader): pin unscoped per-registry settings to their source's registry at load time (#11953)
* fix(config/reader): drop user-level default auth when workspace overrides registry When a workspace `.npmrc` overrides `registry=` to a different value than the user's `~/.npmrc` or `~/.config/pnpm/auth.ini` would have set, do not bind unscoped/default credentials (`_authToken`, `_auth`, `username`/`_password`) from the user-level config to the workspace-selected registry. The previous behavior leaked user-trusted credentials to whatever registry an untrusted workspace `.npmrc` pointed at. Reported by JUNYI LIU. * chore(cspell): allow JUNYI in changeset and tests * fix(config/reader): also defend when pnpm-workspace.yaml overrides registry Move the rebind defense to after all config layers (CLI, env vars, pnpm-workspace.yaml, .npmrc) have settled. Compare the final resolved default registry against what the user-level config alone would produce, and skip the check entirely if the user requested a registry via CLI/env themselves. * feat(config/reader): deprecate unscoped authentication credentials Emit a per-file warning whenever an .npmrc or auth.ini contains an unscoped auth value (_authToken, _auth, username, _password, tokenHelper). URL-scoped tokens have been npm's recommended pattern since npm@9, and unscoped credentials are slated for removal in a future major. The warning fires independently of whether the rebind defense rejects the credentials, so users see the deprecation even when their setup happens to be safe today. * refactor(config/reader): rescope unscoped credentials at load time instead of detecting rebinds post-merge Each .npmrc / auth.ini / CLI source's unscoped credential keys (_authToken, _auth, username, _password, tokenHelper) are rewritten to their URL-scoped equivalent during load, using the same source's registry= value (or the npmjs default if it declares none). A later layer overriding registry= can no longer rebind a credential to its own registry — the credential is already pinned to the URL its author intended. This removes the post-merge source-tracking defense and replaces it with the simpler per-source normalization. Each rescope emits a deprecation warning so users migrate to writing the URL-scoped form directly. * refactor(network/auth-header): drop empty-string default-registry slot After load-time rescoping, no source can populate configByUri[''] — every credential is either URL-scoped from the start or rewritten to the URL-scoped form during the .npmrc / auth.ini / CLI parse. The runtime fallback that re-keyed configByUri[''] onto the merged default registry, and the publish-side fallback that read it, are both dead code. Removed: - empty-string handling in getAuthHeadersFromCreds, including its defaultRegistry parameter - defaultRegistry parameter from createGetAuthHeaderByURI - the corresponding dedicated unit test - the configByUri['']?.creds fallback in publishPackedPkg.ts - empty-key assertions in config/reader tests Updated all ~16 call sites of createGetAuthHeaderByURI to drop the now unused second argument. * feat(config/reader): extend per-source rescoping to client TLS cert/key The same trust-boundary issue that affected unscoped credentials applies to client TLS settings: an unscoped cert=/key= would be presented to whatever registry the merged config settles on, even if a later layer (workspace .npmrc, pnpm-workspace.yaml, CLI flag) overrode it. The existing rescope helper now also rewrites unscoped `cert` and `key` to their URL-scoped form, pinning them to the registry their author named in the same source. `ca`/`cafile` are intentionally left unscoped: they're trust anchors, not credentials, and corporate MITM-proxy setups depend on them applying to every HTTPS request. The default-registry override can't weaponize an unscoped CA — the attacker would need a cert signed by it. `certfile`/`keyfile` (file-path variants) are not rescoped either: `certfile` isn't read unscoped by pnpm today (asymmetric vs. `keyfile` in NPM_AUTH_SETTINGS), and supporting only one of them would be confusing. Users wanting the path form can write it URL-scoped directly. * chore(config/reader): remove dead unscoped `keyfile` allowlist entry `keyfile` was listed in NPM_AUTH_SETTINGS so unscoped `keyfile=<path>` passed the .npmrc filter and ended up in authConfig — but nothing in the codebase ever read it from there. The dispatcher uses `opts.key` (inline PEM) and `configByUri[host].tls.key` (URL-scoped path/inline content), neither of which is populated from unscoped `keyfile=`. `certfile` was already absent from the allowlist for the same reason, so this also removes the asymmetry between the two file-path variants. URL-scoped `//host/:certfile=...` and `//host/:keyfile=...` continue to work via `tryParseSslKey` and are unaffected. * test(network/auth-header): drop test for removed default-registry slot This test exercised the configByUri[''] re-keying path that was removed in the rescope-at-load refactor. With createGetAuthHeaderByURI no longer accepting a defaultRegistry parameter and unscoped credentials no longer reaching the merged config, the scenario the test described is structurally unreachable. * fix(config/reader): handle empty/invalid registry value in rescope Two CI fixes: 1. When a source's `registry=` resolves to an empty string (e.g. an unresolved `${ENV_VAR}` placeholder), `new URL(...)` inside `nerfDart` throws. Guard the call with try/catch: drop the unscoped per-registry keys (a bare token has nowhere safe to bind) and emit a warning naming the offending source. 2. Update `.npmrc does not load pnpm settings` to expect the rescoped form of unscoped `_authToken`/`username` in `authConfig` — they now appear as `//registry.npmjs.org/:_authToken` etc. since the test's .npmrc declares no `registry=` of its own. * chore(cspell): allow "rescoping" * test(installing/deps-installer): drop "legacy way" auth test This test passed credentials via the configByUri[''] empty-string slot, which the auth-header layer re-keyed to the merged default registry at request time. That slot was removed in the rescope-at-load refactor — credentials are now always URL-scoped before they reach configByUri, so the empty-key entry is unreachable from any code path. The scenario the test covered (basicAuth via username/password) is already exercised by the existing "installing a package that need authentication, using password" test using the URL-scoped form.
1 parent 0c5b66f commit a23956e

30 files changed

Lines changed: 460 additions & 100 deletions

File tree

‎.changeset/credential-rebind-defense.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@pnpm/config.reader": patch
3+
"@pnpm/network.auth-header": major
4+
"pnpm": patch
5+
---
6+
7+
Fix a credential disclosure issue where an unscoped `_authToken` (or `_auth`, or `username` + `_password`, or `tokenHelper`) defined in one source — `~/.npmrc`, `~/.config/pnpm/auth.ini`, a workspace `.npmrc`, CLI flags, etc. — would be sent as an `Authorization` header to whichever registry a different (potentially untrusted) source named. The same fix extends to client TLS credentials (`cert`, `key`) so they aren't presented to a registry their author didn't choose.
8+
9+
pnpm now rewrites each unscoped per-registry setting (`_authToken`, `_auth`, `username`, `_password`, `tokenHelper`, `cert`, `key`) to its URL-scoped form at load time, using the `registry=` value declared in the same source (or the npmjs default registry if the source declares none). A later layer overriding `registry=` therefore cannot pull an unscoped credential along, because it is already pinned to the URL its author intended. `ca`/`cafile` are intentionally not rescoped — they're trust anchors, not credentials, and corporate MITM-proxy setups rely on them applying globally.
10+
11+
Every rescope emits a deprecation warning telling the user where the setting was pinned and how to write it directly. npm has rejected unscoped credentials outright since `npm@9`, and pnpm intends to remove support in a future major release. To target a specific registry, write the setting URL-scoped (e.g. `//registry.example.com/:_authToken=...` or `//registry.example.com/:cert=...`).
12+
13+
`@pnpm/network.auth-header`: removed the `defaultRegistry` parameter from `createGetAuthHeaderByURI` and `getAuthHeadersFromCreds`. Now that credentials are URL-scoped at load time, the merged `configByUri` never contains the empty-string "default registry" placeholder slot, so re-keying it onto the merged default registry is no longer needed.

‎config/reader/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"@pnpm/catalogs.types": "workspace:*",
3838
"@pnpm/config.env-replace": "catalog:",
3939
"@pnpm/config.matcher": "workspace:*",
40+
"@pnpm/config.nerf-dart": "catalog:",
4041
"@pnpm/constants": "workspace:*",
4142
"@pnpm/error": "workspace:*",
4243
"@pnpm/hooks.pnpmfile": "workspace:*",

‎config/reader/src/index.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { isConfigFileKey } from './configFileKey.js'
3838
import { extractAndRemoveDependencyBuildOptions, hasDependencyBuildOptions } from './dependencyBuildOptions.js'
3939
import { getCacheDir, getConfigDir, getDataDir, getStateDir } from './dirs.js'
4040
import { parseEnvVars } from './env.js'
41-
import { getDefaultCreds, getNetworkConfigs } from './getNetworkConfigs.js'
41+
import { getNetworkConfigs } from './getNetworkConfigs.js'
4242
import { getOptionsFromPnpmSettings } from './getOptionsFromRootManifest.js'
4343
import { loadNpmrcConfig } from './loadNpmrcFiles.js'
4444
import { inheritDlxConfig, pickIniConfig } from './localConfig.js'
@@ -321,11 +321,8 @@ export async function getConfig (opts: {
321321
...networkConfigs.registries,
322322
}
323323
pnpmConfig.registries = { ...registriesFromNpmrc }
324-
const defaultCreds = getDefaultCreds(pnpmConfig.authConfig)
325-
pnpmConfig.configByUri = {
326-
...networkConfigs.configByUri,
327-
...defaultCreds ? { '': { creds: defaultCreds } } : {},
328-
}
324+
pnpmConfig.configByUri = { ...networkConfigs.configByUri }
325+
329326
// tokenHelper must only come from user-level config (~/.npmrc or global auth.ini),
330327
// not project-level, to prevent project .npmrc from executing arbitrary commands.
331328
const userConfig = npmrcResult.userConfig as Record<string, string>

‎config/reader/src/loadNpmrcFiles.ts‎

Lines changed: 99 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ import os from 'node:os'
33
import path from 'node:path'
44

55
import { envReplaceLossy } from '@pnpm/config.env-replace'
6+
import { nerfDart } from '@pnpm/config.nerf-dart'
7+
import normalizeRegistryUrl from 'normalize-registry-url'
68
import { readIniFileSync } from 'read-ini-file'
79

810
import { isNpmrcReadableKey } from './localConfig.js'
11+
import { npmDefaults } from './npmDefaults.js'
912

1013
export interface NpmrcConfigResult {
1114
/**
@@ -69,6 +72,11 @@ export function loadNpmrcConfig (opts: LoadNpmrcConfigOpts): NpmrcConfigResult {
6972
env
7073
)
7174

75+
// Apply the same per-source rescope to CLI options so an unscoped
76+
// `--_authToken` follows the same trust rule as one written into an .npmrc.
77+
// We clone first to avoid mutating the caller's cliOptions object.
78+
const cliOptions = rescopeUnscopedCreds({ ...opts.cliOptions }, '<command line>', warnings)
79+
7280
// Read pnpm builtin rc + inline defaults
7381
const pnpmBuiltinConfig: Record<string, unknown> = {
7482
...readAndFilterNpmrc(
@@ -83,7 +91,7 @@ export function loadNpmrcConfig (opts: LoadNpmrcConfigOpts): NpmrcConfigResult {
8391
// Handle cafile: expand to ca certs.
8492
// Priority: CLI > workspace > auth.ini > user > defaults
8593
loadCAFile([
86-
opts.cliOptions,
94+
cliOptions,
8795
workspaceNpmrc,
8896
pnpmAuthConfig,
8997
userConfig,
@@ -93,7 +101,7 @@ export function loadNpmrcConfig (opts: LoadNpmrcConfigOpts): NpmrcConfigResult {
93101
// Merge all sources (lowest to highest priority):
94102
// builtin < defaults < user < auth.ini < workspace < CLI
95103
const mergedConfig: Record<string, unknown> = {}
96-
for (const source of [pnpmBuiltinConfig, opts.defaultOptions, userConfig, pnpmAuthConfig, workspaceNpmrc, opts.cliOptions]) {
104+
for (const source of [pnpmBuiltinConfig, opts.defaultOptions, userConfig, pnpmAuthConfig, workspaceNpmrc, cliOptions]) {
97105
for (const [key, value] of Object.entries(source)) {
98106
if (isNpmrcReadableKey(key)) {
99107
mergedConfig[key] = value
@@ -108,7 +116,7 @@ export function loadNpmrcConfig (opts: LoadNpmrcConfigOpts): NpmrcConfigResult {
108116
...userConfig,
109117
...pnpmAuthConfig,
110118
...workspaceNpmrc,
111-
...opts.cliOptions,
119+
...cliOptions,
112120
}
113121

114122
return {
@@ -121,6 +129,35 @@ export function loadNpmrcConfig (opts: LoadNpmrcConfigOpts): NpmrcConfigResult {
121129
}
122130
}
123131

132+
// Per-registry rc keys that, when written without a `//host/` prefix, fall
133+
// through to whatever default registry the merged config settles on. We
134+
// rewrite each such key to its URL-scoped form at load time, pinning it to
135+
// the `registry=` value declared in the same source. A later layer can
136+
// still override the merged registry, but it cannot pull along a credential
137+
// or client certificate authored for a different host.
138+
//
139+
// Two groups:
140+
// * auth keys — `_authToken` etc. Pinned to prevent credential leaks. npm
141+
// rejects these unscoped since npm@9 (ERR_INVALID_AUTH); pnpm keeps them
142+
// working but warns so users migrate before a future major drops support.
143+
// * client certificate keys — `cert`/`key` (inline PEM). Pinned to prevent
144+
// a client certificate (and the identity it carries) being presented to
145+
// the wrong host. The `certfile`/`keyfile` path variants are not in
146+
// `NPM_AUTH_SETTINGS`, so unscoped forms never reach the merged config
147+
// in the first place — only the URL-scoped `//host/:certfile=...` and
148+
// `//host/:keyfile=...` forms are honored, and those are already pinned
149+
// to their authoring registry by construction.
150+
//
151+
// `ca`/`cafile` are intentionally left unscoped-by-default: they're trust
152+
// anchors, not credentials, and corporate MITM-proxy setups rely on them
153+
// applying globally to every HTTPS request. The default registry override
154+
// can't weaponize an unscoped CA (the attacker would need a cert signed
155+
// by it), so the same pinning isn't warranted.
156+
const UNSCOPED_RESCOPABLE_KEYS = [
157+
'_authToken', '_auth', 'username', '_password', 'tokenHelper',
158+
'cert', 'key',
159+
] as const
160+
124161
function readAndFilterNpmrc (
125162
filePath: string,
126163
warnings: string[],
@@ -157,7 +194,65 @@ function readAndFilterNpmrc (
157194
result[key] = value
158195
}
159196
}
160-
return result
197+
return rescopeUnscopedCreds(result, filePath, warnings)
198+
}
199+
200+
// Rewrite any unscoped per-registry keys in `source` to their URL-scoped
201+
// equivalents (`//host[:port]/path/:<key>=...`) using `source.registry` —
202+
// or the builtin default registry if the source doesn't declare its own.
203+
// This pins each layer's credential, client certificate, or CA setting to
204+
// the registry that layer named (or the implicit npmjs default), so a
205+
// later layer overriding `registry=` cannot pull a setting authored for
206+
// one host along to a different host. A URL-scoped key for the same
207+
// registry already present in `source` wins; we never overwrite an
208+
// explicit scoped value.
209+
//
210+
// Each rewrite triggers a deprecation warning so users migrate to writing
211+
// the URL-scoped form directly. npm has rejected unscoped credentials
212+
// outright since `npm@9` (`ERR_INVALID_AUTH`).
213+
function rescopeUnscopedCreds (
214+
source: Record<string, unknown>,
215+
sourceLabel: string,
216+
warnings: string[]
217+
): Record<string, unknown> {
218+
// Bail early if there's nothing to rescope. This skips the nerfDart call
219+
// when a source like the builtin pnpmrc has only a `registry=` line —
220+
// rescoping there would do nothing anyway.
221+
if (!UNSCOPED_RESCOPABLE_KEYS.some(key => key in source)) {
222+
return source
223+
}
224+
const rawRegistry = typeof source.registry === 'string' && source.registry !== '' ? source.registry : null
225+
const fallbackRegistry = rawRegistry ?? npmDefaults.registry
226+
let nerfedRegistry: string
227+
try {
228+
nerfedRegistry = nerfDart(normalizeRegistryUrl(fallbackRegistry))
229+
} catch {
230+
// `registry=` resolved to something `URL` can't parse — often an
231+
// unresolved `${VAR}` placeholder that left the string empty. Drop the
232+
// unscoped keys (a bare token is unsafe to bind anywhere) and warn.
233+
const dropped = UNSCOPED_RESCOPABLE_KEYS.filter(key => key in source)
234+
for (const key of dropped) delete source[key]
235+
warnings.push(`Unscoped per-registry settings (${dropped.join(', ')}) in "${sourceLabel}" were ignored: ` +
236+
`the source's "registry" value (${JSON.stringify(source.registry)}) is not a parseable URL, so pnpm cannot pin them anywhere safe. ` +
237+
'Write them URL-scoped (e.g. "//registry.example.com/:_authToken=...") to send them to a specific registry.')
238+
return source
239+
}
240+
const rescoped: string[] = []
241+
for (const key of UNSCOPED_RESCOPABLE_KEYS) {
242+
if (!(key in source)) continue
243+
const scopedKey = `${nerfedRegistry}:${key}`
244+
if (!(scopedKey in source)) {
245+
source[scopedKey] = source[key]
246+
}
247+
delete source[key]
248+
rescoped.push(key)
249+
}
250+
if (rescoped.length > 0) {
251+
warnings.push(`Unscoped per-registry settings (${rescoped.join(', ')}) in "${sourceLabel}" are deprecated. ` +
252+
`pnpm pinned them to "${nerfedRegistry}" for this run, but a future release will stop supporting unscoped per-registry settings. ` +
253+
`Write them as "${nerfedRegistry}:${rescoped[0]}=..." instead.`)
254+
}
255+
return source
161256
}
162257

163258
// Use the lossy variant so unresolved `${VAR}` placeholders become '' (each

‎config/reader/src/localConfig.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ const NPM_AUTH_SETTINGS = [
110110
'_authToken',
111111
'_password',
112112
'email',
113-
'keyfile',
114113
'username',
115114
]
116115

0 commit comments

Comments
 (0)