diff --git a/.changeset/credential-rebind-defense.md b/.changeset/credential-rebind-defense.md new file mode 100644 index 0000000000..0f4417bf6b --- /dev/null +++ b/.changeset/credential-rebind-defense.md @@ -0,0 +1,7 @@ +--- +"@pnpm/config": patch +"@pnpm/network.auth-header": major +"pnpm": patch +--- + +Pin unscoped per-registry settings (`_authToken`, `_auth`, `username`/`_password`, `tokenHelper`, inline `cert`/`key`) to the registry declared in the same config source at load time, so a later layer overriding `registry=` (workspace `.npmrc`, `pnpm-workspace.yaml`, CLI `--registry`) cannot redirect a credential or client certificate authored for a different host. A deprecation warning is emitted whenever an unscoped per-registry setting is encountered, naming the source and the URL it was pinned to. Reported by JUNYI LIU. diff --git a/config/config/package.json b/config/config/package.json index 93f3704bfb..7e8487a832 100644 --- a/config/config/package.json +++ b/config/config/package.json @@ -37,6 +37,7 @@ "@pnpm/catalogs.config": "workspace:*", "@pnpm/catalogs.types": "workspace:*", "@pnpm/config.env-replace": "catalog:", + "@pnpm/config.nerf-dart": "catalog:", "@pnpm/constants": "workspace:*", "@pnpm/error": "workspace:*", "@pnpm/git-utils": "workspace:*", diff --git a/config/config/src/index.ts b/config/config/src/index.ts index 45ec2dd628..c566b9e003 100644 --- a/config/config/src/index.ts +++ b/config/config/src/index.ts @@ -20,6 +20,7 @@ import pathAbsolute from 'path-absolute' import which from 'which' import { inheritAuthConfig } from './auth.js' import { checkGlobalBinDir } from './checkGlobalBinDir.js' +import { rescopeUnscopedCreds } from './rescopeUnscopedCreds.js' import { hasDependencyBuildOptions, extractAndRemoveDependencyBuildOptions } from './dependencyBuildOptions.js' import { getNetworkConfigs } from './getNetworkConfigs.js' import { transformPathKeys } from './transformPath.js' @@ -211,7 +212,17 @@ export async function getConfig (opts: { 'peers-suffix-max-length': 1000, } - const { config: npmConfig, warnings, failedToLoadBuiltInConfig } = loadNpmConf(cliOptions, rcOptionsTypes, defaultOptions) + // Pin any unscoped per-registry credentials/cert keys passed on the CLI + // (e.g. `--_authToken=...`) to whichever registry the CLI itself names — + // or to the npmjs default if the CLI didn't name one. A later workspace + // .npmrc or pnpm-workspace.yaml setting `registry=` to a different host + // can no longer pull the credential along. We mutate in place since the + // surrounding code already mutates cliOptions (dir, prefix). + const warnings: string[] = [] + rescopeUnscopedCreds(cliOptions, '', warnings) + + const { config: npmConfig, warnings: npmConfWarnings, failedToLoadBuiltInConfig } = loadNpmConf(cliOptions, rcOptionsTypes, defaultOptions) + warnings.push(...npmConfWarnings) const configDir = getConfigDir(process) { @@ -227,6 +238,20 @@ export async function getConfig (opts: { if (warn) warnings.push(warn) } + // After every source (cli, env, project, workspace, user, global, + // pnpm-global, pnpm-builtin, npm-builtin) has been loaded, rewrite each + // source's unscoped per-registry credential/cert keys to their URL-scoped + // form using that source's own `registry=` (or the npmjs default). The + // merged rawConfig built below picks up the URL-scoped form, so a layer + // overriding the default `registry=` can no longer rebind a credential or + // cert that was authored elsewhere. + for (const [name, sourceEntry] of Object.entries(npmConfig.sources)) { + if (name === 'cli') continue // already rescoped above + const data = (sourceEntry as { data?: Record }).data + if (data == null) continue + rescopeUnscopedCreds(data, sourceLabel(name, sourceEntry as { path?: string }), warnings) + } + delete cliOptions.prefix process.execPath = originalExecPath @@ -563,6 +588,12 @@ function getProcessEnv (env: string): string | undefined { process.env[env.toLowerCase()] } +function sourceLabel (name: string, sourceEntry: { path?: string }): string { + if (typeof sourceEntry.path === 'string' && sourceEntry.path !== '') return sourceEntry.path + if (name === 'env') return 'npm_config_* environment variables' + return name +} + function parsePackageManager (packageManager: string): { name: string, version: string | undefined } { if (!packageManager.includes('@')) return { name: packageManager, version: undefined } const [name, pmReference] = packageManager.split('@') diff --git a/config/config/src/rescopeUnscopedCreds.ts b/config/config/src/rescopeUnscopedCreds.ts new file mode 100644 index 0000000000..8b5c2020d5 --- /dev/null +++ b/config/config/src/rescopeUnscopedCreds.ts @@ -0,0 +1,102 @@ +import loadNpmConf from '@pnpm/npm-conf' +import { nerfDart } from '@pnpm/config.nerf-dart' +import normalizeRegistryUrl from 'normalize-registry-url' + +// Per-registry rc keys that, when written without a `//host/` prefix, fall +// through to whatever default registry the merged config settles on. We +// rewrite each such key to its URL-scoped form at load time, pinning it to +// the `registry=` value declared in the same source. A later layer can +// still override the merged registry, but it cannot pull along a credential +// or client certificate authored for a different host. +// +// Two groups: +// * auth keys — `_authToken` etc. Pinned to prevent credential leaks. npm +// rejects these unscoped since npm@9 (ERR_INVALID_AUTH); pnpm keeps them +// working but warns so users migrate before a future major drops support. +// * client certificate keys — `cert`/`key` (inline PEM). Pinned to prevent +// a client certificate (and the identity it carries) being presented to +// the wrong host. The `certfile`/`keyfile` path variants are not in +// `NPM_AUTH_SETTINGS`, so unscoped forms never reach the merged config +// in the first place — only the URL-scoped `//host/:certfile=...` and +// `//host/:keyfile=...` forms are honored, and those are already pinned +// to their authoring registry by construction. +// +// `ca`/`cafile` are intentionally left unscoped-by-default: they're trust +// anchors, not credentials, and corporate MITM-proxy setups rely on them +// applying globally to every HTTPS request. The default registry override +// can't weaponize an unscoped CA (the attacker would need a cert signed +// by it), so the same pinning isn't warranted. +const UNSCOPED_RESCOPABLE_KEYS = [ + '_authToken', '_auth', 'username', '_password', 'tokenHelper', + 'cert', 'key', +] as const + +const npmDefaults = loadNpmConf.defaults + +// Rewrite any unscoped per-registry keys in `source` to their URL-scoped +// equivalents (`//host[:port]/path/:=...`) using `source.registry` — +// or the builtin default registry if the source doesn't declare its own. +// This pins each layer's credential or client certificate to the registry +// that layer named (or the implicit npmjs default), so a later layer +// overriding `registry=` cannot pull a setting authored for one host +// along to a different host. A URL-scoped key for the same registry +// already present in `source` wins; we never overwrite an explicit scoped +// value. +// +// Each rewrite triggers a deprecation warning so users migrate to writing +// the URL-scoped form directly. npm has rejected unscoped credentials +// outright since `npm@9` (`ERR_INVALID_AUTH`). +export function rescopeUnscopedCreds ( + source: Record, + sourceLabel: string, + warnings: string[] +): Record { + // npm-conf is built on config-chain/proto-list, which chains each source's + // data object via prototype: list[0].__proto__ === list[1], and so on. So + // `key in source` walks UP into other sources, and `source[key]` reads an + // inherited value. We only ever want to rescope keys this source actually + // declared — hence `Object.hasOwn` everywhere instead of `in`/dot access. + // + // `null`/`undefined` values come from npm-conf's defaults layer (it pre- + // fills `cert: null`, `key: null`, etc.). They're not real settings and + // must be skipped, otherwise we'd create URL-scoped null entries that + // downstream consumers would choke on. + if (!UNSCOPED_RESCOPABLE_KEYS.some(key => Object.hasOwn(source, key) && source[key] != null)) { + return source + } + // Read the source's OWN registry — `source.registry` would walk the + // prototype chain into a different source and pin our credentials there. + const ownRegistry = Object.hasOwn(source, 'registry') ? source.registry : undefined + const rawRegistry = typeof ownRegistry === 'string' && ownRegistry !== '' ? ownRegistry : null + const fallbackRegistry = rawRegistry ?? npmDefaults.registry + let nerfedRegistry: string + try { + nerfedRegistry = nerfDart(normalizeRegistryUrl(fallbackRegistry)) + } catch { + // `registry=` resolved to something `URL` can't parse — often an + // unresolved `${VAR}` placeholder that left the string empty. Drop the + // unscoped keys (a bare token is unsafe to bind anywhere) and warn. + const dropped = UNSCOPED_RESCOPABLE_KEYS.filter(key => Object.hasOwn(source, key) && source[key] != null) + for (const key of dropped) delete source[key] + warnings.push(`Unscoped per-registry settings (${dropped.join(', ')}) in "${sourceLabel}" were ignored: ` + + `the source's "registry" value (${JSON.stringify(ownRegistry)}) is not a parseable URL, so pnpm cannot pin them anywhere safe. ` + + 'Write them URL-scoped (e.g. "//registry.example.com/:_authToken=...") to send them to a specific registry.') + return source + } + const rescoped: string[] = [] + for (const key of UNSCOPED_RESCOPABLE_KEYS) { + if (!Object.hasOwn(source, key) || source[key] == null) continue + const scopedKey = `${nerfedRegistry}:${key}` + if (!Object.hasOwn(source, scopedKey)) { + source[scopedKey] = source[key] + } + delete source[key] + rescoped.push(key) + } + if (rescoped.length > 0) { + warnings.push(`Unscoped per-registry settings (${rescoped.join(', ')}) in "${sourceLabel}" are deprecated. ` + + `pnpm pinned them to "${nerfedRegistry}" for this run, but a future release will stop supporting unscoped per-registry settings. ` + + `Write them as "${nerfedRegistry}:${rescoped[0]}=..." instead.`) + } + return source +} diff --git a/config/config/test/index.ts b/config/config/test/index.ts index d30a9c136b..ea63c1bac6 100644 --- a/config/config/test/index.ts +++ b/config/config/test/index.ts @@ -1235,3 +1235,178 @@ test('no warning when directory does not contain PATH delimiter character', asyn fs.rmSync(tempDir, { recursive: true }) } }) + +describe('rescoping unscoped per-registry credentials at load time', () => { + // Reported by JUNYI LIU: a workspace .npmrc that overrides `registry=` to a + // different host than the user's ~/.npmrc would have set must not pull the + // user's unscoped credentials along. Each source's unscoped per-registry + // credentials are pinned to the source's own `registry=` (or npmjs default) + // at load time, so a later layer cannot rebind them. + + // Isolate the maintainer's real ~/.config/pnpm/{rc,auth.ini} (which would + // otherwise be loaded by the pnpm-global addFile and leak its + // `//registry.npmjs.org/:_authToken` into our URL-scoped assertions). + let savedXdgConfigHome: string | undefined + beforeEach(() => { + savedXdgConfigHome = process.env.XDG_CONFIG_HOME + const xdg = path.resolve(os.tmpdir(), `pnpm-test-xdg-${Date.now()}-${Math.random().toString(36).slice(2)}`) + fs.mkdirSync(path.join(xdg, 'pnpm'), { recursive: true }) + process.env.XDG_CONFIG_HOME = xdg + }) + afterEach(() => { + if (savedXdgConfigHome === undefined) { + delete process.env.XDG_CONFIG_HOME + } else { + process.env.XDG_CONFIG_HOME = savedXdgConfigHome + } + }) + + function writeUserConfig (contents: string): string { + const userConfigPath = path.resolve('user-home', '.npmrc') + fs.mkdirSync(path.dirname(userConfigPath), { recursive: true }) + fs.writeFileSync(userConfigPath, contents, 'utf8') + return userConfigPath + } + + test('workspace .npmrc registry override cannot rebind user-level unscoped _authToken', async () => { + prepare() + const userconfig = writeUserConfig([ + 'registry=https://trusted.example.test/', + '_authToken=USER_TRUSTED_TOKEN', + ].join('\n')) + fs.writeFileSync('.npmrc', 'registry=https://attacker.example.test/', 'utf8') + + const { config } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + // The user-level token is pinned to its source's registry (trusted), not + // rebound to the workspace's attacker registry. + expect(config.rawConfig['//trusted.example.test/:_authToken']).toBe('USER_TRUSTED_TOKEN') + expect(config.rawConfig['//attacker.example.test/:_authToken']).toBeUndefined() + expect(config.rawConfig['_authToken']).toBeUndefined() + // The merged default registry still reflects the workspace override. + expect(config.rawConfig.registry).toBe('https://attacker.example.test/') + }) + + test('user-level unscoped _authToken with no source-level registry pins to npmjs default', async () => { + prepare() + const userconfig = writeUserConfig('_authToken=USER_AMBIENT_TOKEN') + fs.writeFileSync('.npmrc', 'registry=https://attacker.example.test/', 'utf8') + + const { config } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(config.rawConfig['//registry.npmjs.org/:_authToken']).toBe('USER_AMBIENT_TOKEN') + expect(config.rawConfig['//attacker.example.test/:_authToken']).toBeUndefined() + expect(config.rawConfig['_authToken']).toBeUndefined() + }) + + test('cli --registry cannot pull along an unscoped user-level _authToken', async () => { + prepare() + const userconfig = writeUserConfig('_authToken=USER_AMBIENT_TOKEN') + + const { config } = await getConfig({ + cliOptions: { + userconfig, + registry: 'https://attacker.example.test/', + }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(config.rawConfig['//registry.npmjs.org/:_authToken']).toBe('USER_AMBIENT_TOKEN') + expect(config.rawConfig['//attacker.example.test/:_authToken']).toBeUndefined() + expect(config.rawConfig['_authToken']).toBeUndefined() + }) + + test('url-scoped credentials pass through unchanged with no deprecation warning', async () => { + prepare() + const userconfig = writeUserConfig('//trusted.example.test/:_authToken=URL_SCOPED') + fs.writeFileSync('.npmrc', 'registry=https://attacker.example.test/', 'utf8') + + const { config, warnings } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(config.rawConfig['//trusted.example.test/:_authToken']).toBe('URL_SCOPED') + expect(warnings).not.toContainEqual(expect.stringContaining('Unscoped per-registry settings')) + }) + + test('an explicit deprecation warning names the source and pinned registry', async () => { + prepare() + const userconfig = writeUserConfig([ + 'registry=https://trusted.example.test/', + '_authToken=USER_TRUSTED_TOKEN', + ].join('\n')) + + const { warnings } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(warnings).toContainEqual(expect.stringMatching( + /Unscoped per-registry settings \(_authToken\).*are deprecated.*pinned them to "\/\/trusted\.example\.test\/"/s + )) + }) + + test('explicit url-scoped key in the same source wins over the unscoped rescope', async () => { + prepare() + const userconfig = writeUserConfig([ + 'registry=https://trusted.example.test/', + '_authToken=UNSCOPED', + '//trusted.example.test/:_authToken=EXPLICIT', + ].join('\n')) + + const { config } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(config.rawConfig['//trusted.example.test/:_authToken']).toBe('EXPLICIT') + expect(config.rawConfig['_authToken']).toBeUndefined() + }) + + test('inline cert and key are pinned to the source registry', async () => { + prepare() + const userconfig = writeUserConfig([ + 'registry=https://trusted.example.test/', + 'cert="-----BEGIN CERTIFICATE-----\\nFAKE\\n-----END CERTIFICATE-----"', + 'key="-----BEGIN PRIVATE KEY-----\\nFAKE\\n-----END PRIVATE KEY-----"', + ].join('\n')) + fs.writeFileSync('.npmrc', 'registry=https://attacker.example.test/', 'utf8') + + const { config } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + expect(config.rawConfig['//trusted.example.test/:cert']).toBeDefined() + expect(config.rawConfig['//trusted.example.test/:key']).toBeDefined() + expect(config.rawConfig['//attacker.example.test/:cert']).toBeUndefined() + expect(config.rawConfig['//attacker.example.test/:key']).toBeUndefined() + expect(config.rawConfig['cert']).toBeUndefined() + expect(config.rawConfig['key']).toBeUndefined() + }) + + test('ca/cafile are intentionally not rescoped', async () => { + prepare() + const userconfig = writeUserConfig([ + 'registry=https://trusted.example.test/', + 'cafile=/path/to/corp-ca.pem', + ].join('\n')) + + const { config, warnings } = await getConfig({ + cliOptions: { userconfig }, + packageManager: { name: 'pnpm', version: '1.0.0' }, + }) + + // cafile should still be present globally — corporate MITM proxies + // depend on it applying to every HTTPS request. + expect(config.rawConfig['cafile']).toBeDefined() + expect(warnings).not.toContainEqual(expect.stringContaining('cafile')) + }) +}) diff --git a/cspell.json b/cspell.json index 91b2160db7..c3652ec06e 100644 --- a/cspell.json +++ b/cspell.json @@ -38,6 +38,7 @@ "corepack", "corge", "cowsay", + "Creds", "cves", "cwsay", "deburr", @@ -119,6 +120,7 @@ "jega", "jhcg", "jnbpamcxayl", + "JUNYI", "kebabcase", "kevva", "keyfile", @@ -238,6 +240,10 @@ "renderable", "replit", "reqheaders", + "rescopable", + "rescope", + "rescoped", + "rescoping", "rmgr", "rpmdevtools", "rpmlint", diff --git a/network/auth-header/src/getAuthHeadersFromConfig.ts b/network/auth-header/src/getAuthHeadersFromConfig.ts index e6ba957532..aae625213f 100644 --- a/network/auth-header/src/getAuthHeadersFromConfig.ts +++ b/network/auth-header/src/getAuthHeadersFromConfig.ts @@ -1,4 +1,3 @@ -import { nerfDart } from '@pnpm/config.nerf-dart' import { PnpmError } from '@pnpm/error' import { spawnSync } from 'child_process' import fs from 'fs' @@ -36,16 +35,6 @@ export function getAuthHeadersFromConfig ( authHeaderValueByURI[uri] = loadToken(value, key) } } - const registry = allSettings['registry'] ? nerfDart(allSettings['registry']) : '//registry.npmjs.org/' - if (userSettings['tokenHelper']) { - authHeaderValueByURI[registry] = loadToken(userSettings['tokenHelper'], 'tokenHelper') - } else if (allSettings['_authToken']) { - authHeaderValueByURI[registry] = `Bearer ${allSettings['_authToken']}` - } else if (allSettings['_auth']) { - authHeaderValueByURI[registry] = `Basic ${allSettings['_auth']}` - } else if (allSettings['_password'] && allSettings['username']) { - authHeaderValueByURI[registry] = `Basic ${Buffer.from(`${allSettings['username']}:${allSettings['_password']}`).toString('base64')}` - } return authHeaderValueByURI } diff --git a/network/auth-header/test/getAuthHeadersFromConfig.test.ts b/network/auth-header/test/getAuthHeadersFromConfig.test.ts index a8cb8fe3a9..786ae49dd8 100644 --- a/network/auth-header/test/getAuthHeadersFromConfig.test.ts +++ b/network/auth-header/test/getAuthHeadersFromConfig.test.ts @@ -33,45 +33,6 @@ describe('getAuthHeadersFromConfig()', () => { '//localhost:3000/': 'Basic foobar', }) }) - describe('should get settings for the default registry', () => { - it('_auth', () => { - const allSettings = { - registry: 'https://reg.com/', - _auth: 'foobar', - } - expect(getAuthHeadersFromConfig({ allSettings, userSettings: {} })).toStrictEqual({ - '//reg.com/': 'Basic foobar', - }) - }) - it('username/_password', () => { - const allSettings = { - registry: 'https://reg.com/', - username: 'foo', - _password: 'bar', - } - expect(getAuthHeadersFromConfig({ allSettings, userSettings: {} })).toStrictEqual({ - '//reg.com/': `Basic ${encodeBase64('foo:bar')}`, - }) - }) - it('tokenHelper', () => { - const allSettings = { - registry: 'https://reg.com/', - } - const userSettings = { - tokenHelper: osTokenHelper[osFamily], - } - expect(getAuthHeadersFromConfig({ allSettings, userSettings })).toStrictEqual({ - '//reg.com/': 'Bearer token-from-spawn', - }) - }) - it('only read token helper from user config', () => { - const allSettings = { - registry: 'https://reg.com/', - tokenHelper: osTokenHelper[osFamily], - } - expect(getAuthHeadersFromConfig({ allSettings, userSettings: {} })).toStrictEqual({}) - }) - }) it('should get tokenHelper', () => { const userSettings = { '//registry.foobar.eu/:tokenHelper': osTokenHelper[osFamily], diff --git a/pkg-manager/core/test/install/auth.ts b/pkg-manager/core/test/install/auth.ts index b346bd05da..885c9cefdc 100644 --- a/pkg-manager/core/test/install/auth.ts +++ b/pkg-manager/core/test/install/auth.ts @@ -71,28 +71,6 @@ test('installing a package that need authentication, using password', async () = project.has('@pnpm.e2e/needs-auth') }) -test('a package that need authentication, legacy way', async () => { - const project = prepareEmpty() - - await addUser({ - email: 'foo@bar.com', - password: 'bar', - username: 'foo', - }) - - const authConfig = { - _auth: 'Zm9vOmJhcg==', // base64 encoded foo:bar - registry: `http://localhost:${REGISTRY_MOCK_PORT}`, - } - await addDependenciesToPackage({}, ['@pnpm.e2e/needs-auth'], testDefaults({}, { - authConfig, - }, { - authConfig, - })) - - project.has('@pnpm.e2e/needs-auth') -}) - test('a scoped package that need authentication specific to scope', async () => { const project = prepareEmpty() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9942151867..9023568e5e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1636,6 +1636,9 @@ importers: '@pnpm/config.env-replace': specifier: 'catalog:' version: 3.0.2 + '@pnpm/config.nerf-dart': + specifier: 'catalog:' + version: 1.0.1 '@pnpm/constants': specifier: workspace:* version: link:../../packages/constants