From 0c137bbbe0c1ab7dc60407d8dd8f59ea9fd1868e Mon Sep 17 00:00:00 2001 From: Fotis Papadogeorgopoulos Date: Tue, 5 May 2026 15:27:12 +0300 Subject: [PATCH] fix(config): gracefully handle configDependencies errors during config commands (#11362) Closes #10684. Running `pnpm config set` triggers `resolveAndInstallConfigDeps` before the new config (e.g. auth token) is saved to `.npmrc`, causing a 401 crash when the config dependency is hosted in a private registry. This patch adds a `tolerateConfigDependenciesErrors` flag to `installConfigDepsAndLoadHooks`. For `cmd === 'config'`, `'set'` and `'get'`, installation failures are now caught and logged at debug level so the command can proceed and actually write / read the auth token. `pnpm set` and `pnpm get` are separate top-level commands whose handlers delegate to the `config` command internally, so they are explicitly listed alongside `'config'`; users can hit #10684 via any of the three entry points. Plugin pnpmfile paths from `configDependencies` are resolved by checking each file's existence on disk, so previously-installed hooks survive a transient install failure and `requireHooks` won't throw `PNPMFILE_NOT_FOUND` when nothing has been installed yet. Covers the fix with: - Unit tests in `pnpm/test/getConfig.test.ts` exercising the `installConfigDepsAndLoadHooks` contract (success, tolerated error, rethrown error, store creation/close errors propagating) and the on-disk pnpmfile resolution behavior. - Four e2e tests in `pnpm/test/configurationalDependencies.test.ts` that spawn the pnpm CLI against a bogus `configDependency` and assert each entrypoint (`pnpm config set`, `pnpm config get`, `pnpm set`, `pnpm get`) still works. --------- Co-authored-by: Zoltan Kochan --- .changeset/major-hats-press.md | 5 + pnpm/src/getConfig.ts | 28 +++- pnpm/src/main.ts | 8 +- pnpm/test/configurationalDependencies.test.ts | 65 ++++++++ pnpm/test/getConfig.test.ts | 157 +++++++++++++++++- 5 files changed, 257 insertions(+), 6 deletions(-) create mode 100644 .changeset/major-hats-press.md diff --git a/.changeset/major-hats-press.md b/.changeset/major-hats-press.md new file mode 100644 index 0000000000..2bbaaabd39 --- /dev/null +++ b/.changeset/major-hats-press.md @@ -0,0 +1,5 @@ +--- +"pnpm": patch +--- + +Prevent crashes during `pnpm config`, `pnpm set`, and `pnpm get` by tolerating `configDependencies` install failures. For these commands, a failure to install `configDependencies` (for example because the registry auth token has not been written yet) is now logged at debug level and the command proceeds. All other commands still surface the install error [#10684](https://github.com/pnpm/pnpm/issues/10684). diff --git a/pnpm/src/getConfig.ts b/pnpm/src/getConfig.ts index f9fc8c7735..c25823eb1a 100644 --- a/pnpm/src/getConfig.ts +++ b/pnpm/src/getConfig.ts @@ -1,11 +1,13 @@ import fs from 'node:fs' import path from 'node:path' +import util from 'node:util' import { formatWarn } from '@pnpm/cli.default-reporter' import { packageManager } from '@pnpm/cli.meta' import { type CliOptions, type Config, type ConfigContext, getConfig as _getConfig } from '@pnpm/config.reader' import { requireHooks } from '@pnpm/hooks.pnpmfile' import { resolveAndInstallConfigDeps } from '@pnpm/installing.env-installer' +import { logger } from '@pnpm/logger' import { createStoreController } from '@pnpm/store.connection-manager' import type { ConfigDependencies } from '@pnpm/types' import { lexCompare } from '@pnpm/util.lex-comparator' @@ -42,7 +44,10 @@ export async function getConfig ( export async function installConfigDepsAndLoadHooks ( config: Config, - context: ConfigContext + context: ConfigContext, + opts?: { + tolerateConfigDependenciesErrors?: boolean + } ): Promise<{ config: Config, context: ConfigContext }> { if (config.configDependencies) { const store = await createStoreController({ ...config, ...context }) @@ -55,6 +60,15 @@ export async function installConfigDepsAndLoadHooks ( rootDir: config.lockfileDir ?? context.rootProjectManifestDir, frozenLockfile: config.frozenLockfile, }) + } catch (err: unknown) { + if (!opts?.tolerateConfigDependenciesErrors) { + throw err + } + const errorMessage = util.types.isNativeError(err) ? err.message : String(err) + logger.debug({ + message: `Failed to install configDependencies. This is expected if authentication is not yet configured. Proceeding. Error: ${errorMessage}`, + err, + }) } finally { await store.ctrl.close() } @@ -87,12 +101,18 @@ export async function installConfigDepsAndLoadHooks ( export function * calcPnpmfilePathsOfPluginDeps (configModulesDir: string, configDependencies: ConfigDependencies): Generator { for (const configDepName of Object.keys(configDependencies).sort(lexCompare)) { if (isPluginName(configDepName)) { - const mjsPath = path.join(configModulesDir, configDepName, 'pnpmfile.mjs') + const pluginDir = path.join(configModulesDir, configDepName) + // If the plugin directory itself is missing, the install didn't run + // (or hasn't run yet) — skip silently. If the plugin directory exists + // but contains no pnpmfile, fall through to yield the .cjs path so + // requireHooks surfaces PNPMFILE_NOT_FOUND for the misconfigured plugin. + if (!fs.existsSync(pluginDir)) continue + const mjsPath = path.join(pluginDir, 'pnpmfile.mjs') if (fs.existsSync(mjsPath)) { yield mjsPath - } else { - yield path.join(configModulesDir, configDepName, 'pnpmfile.cjs') + continue } + yield path.join(pluginDir, 'pnpmfile.cjs') } } } diff --git a/pnpm/src/main.ts b/pnpm/src/main.ts index 95ee1f9079..2337a4dfd2 100644 --- a/pnpm/src/main.ts +++ b/pnpm/src/main.ts @@ -125,7 +125,13 @@ export async function main (inputArgv: string[]): Promise { } } } - ;({ config, context } = await installConfigDepsAndLoadHooks(config, context) as { config: typeof config, context: ConfigContext }) + // `pnpm set` / `pnpm get` are separate top-level commands whose handlers + // delegate to the `config` command internally. They are not rewritten to + // `cmd === 'config'` at this layer, so list them explicitly — users can + // hit the #10684 crash via any of these three entry points. + ;({ config, context } = await installConfigDepsAndLoadHooks(config, context, { + tolerateConfigDependenciesErrors: cmd === 'config' || cmd === 'set' || cmd === 'get', + }) as { config: typeof config, context: ConfigContext }) if (isDlxOrCreateCommand || cmd === 'sbom' || cmd === 'with') { config.useStderr = true } diff --git a/pnpm/test/configurationalDependencies.test.ts b/pnpm/test/configurationalDependencies.test.ts index b6b7159526..10cb16512c 100644 --- a/pnpm/test/configurationalDependencies.test.ts +++ b/pnpm/test/configurationalDependencies.test.ts @@ -212,3 +212,68 @@ test('installing a new configurational dependency', async () => { getIntegrity('@pnpm.e2e/foo', '100.0.0') ) }) + +// Regression tests for https://github.com/pnpm/pnpm/issues/10684 — if the user +// has a configDependency stored in a registry that needs auth, the config +// commands must not crash when pnpm tries to fetch the configDependency before +// the new setting is written. We reference a non-existent package version so +// the install errors out fast; the real-world scenario is a 401 from the +// private registry. +// +// All four entry points are tested: `pnpm config set`, `pnpm config get`, +// `pnpm set`, and `pnpm get`. The latter two are shortcuts that delegate to +// the config handler internally but are separate top-level commands, so they +// need their own coverage at the main.ts guard level. +function writeFailingConfigDep () { + // Clean specifier for a version that does not exist on the mock registry. + // fetchRetries: 0 keeps the failure fast so the test does not time out. + writeYamlFileSync('pnpm-workspace.yaml', { + configDependencies: { + '@pnpm.e2e/foo': '999.999.999', + }, + fetchRetries: 0, + }) +} + +test('pnpm config set succeeds even when configDependencies fail to install', async () => { + prepare() + writeFailingConfigDep() + + // Use an auth-style key so the setting lands in ./.npmrc (project scope). + const authKey = '//example.com/:_authToken' + await execPnpm(['config', 'set', '--location=project', authKey, 'my-secret-token']) + + const npmrc = fs.readFileSync('.npmrc', 'utf8') + expect(npmrc).toContain(`${authKey}=my-secret-token`) +}) + +test('pnpm config get succeeds even when configDependencies fail to install', async () => { + prepare() + writeFailingConfigDep() + const authKey = '//example.com/:_authToken' + fs.writeFileSync('.npmrc', `${authKey}=my-secret-token\n`, 'utf8') + + const result = execPnpmSync(['config', 'get', '--location=project', authKey], { expectSuccess: true }) + expect(result.stdout.toString()).toContain('my-secret-token') +}) + +test('pnpm set succeeds even when configDependencies fail to install', async () => { + prepare() + writeFailingConfigDep() + + const authKey = '//example.com/:_authToken' + await execPnpm(['set', '--location=project', authKey, 'my-secret-token']) + + const npmrc = fs.readFileSync('.npmrc', 'utf8') + expect(npmrc).toContain(`${authKey}=my-secret-token`) +}) + +test('pnpm get succeeds even when configDependencies fail to install', async () => { + prepare() + writeFailingConfigDep() + const authKey = '//example.com/:_authToken' + fs.writeFileSync('.npmrc', `${authKey}=my-secret-token\n`, 'utf8') + + const result = execPnpmSync(['get', '--location=project', authKey], { expectSuccess: true }) + expect(result.stdout.toString()).toContain('my-secret-token') +}) diff --git a/pnpm/test/getConfig.test.ts b/pnpm/test/getConfig.test.ts index cf36f2a586..0e3e26dee3 100644 --- a/pnpm/test/getConfig.test.ts +++ b/pnpm/test/getConfig.test.ts @@ -3,12 +3,46 @@ import fs from 'node:fs' import path from 'node:path' import { afterEach, beforeEach, describe, expect, jest, test } from '@jest/globals' +import type { Config, ConfigContext } from '@pnpm/config.reader' import { prepare } from '@pnpm/prepare' -import { calcPnpmfilePathsOfPluginDeps, getConfig } from '../src/getConfig.js' +jest.unstable_mockModule('@pnpm/installing.env-installer', () => ({ + resolveAndInstallConfigDeps: jest.fn(), +})) + +jest.unstable_mockModule('@pnpm/store.connection-manager', () => ({ + createStoreController: jest.fn(), +})) + +const loggerMock = { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), +} +jest.unstable_mockModule('@pnpm/logger', () => ({ + logger: Object.assign(jest.fn(() => loggerMock), loggerMock), + globalWarn: jest.fn(), + globalInfo: jest.fn(), + globalError: jest.fn(), +})) + +const { resolveAndInstallConfigDeps } = await import('@pnpm/installing.env-installer') +const { logger } = await import('@pnpm/logger') +const { createStoreController } = await import('@pnpm/store.connection-manager') +const { calcPnpmfilePathsOfPluginDeps, getConfig, installConfigDepsAndLoadHooks } = await import('../src/getConfig.js') + +const storeCloseMock = jest.fn<() => Promise>() beforeEach(() => { jest.spyOn(console, 'warn') + loggerMock.debug.mockClear() + jest.mocked(resolveAndInstallConfigDeps).mockReset() + storeCloseMock.mockReset().mockResolvedValue(undefined) + jest.mocked(createStoreController).mockReset().mockResolvedValue({ + ctrl: { close: storeCloseMock }, + dir: '/tmp/store', + } as unknown as Awaited>) }) afterEach(() => { @@ -59,6 +93,29 @@ describe('calcPnpmfilePathsOfPluginDeps', () => { fs.rmSync(tmpDir, { recursive: true }) } }) + + test('skips plugins whose directory is missing (e.g. config dep install never ran)', () => { + const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-')) + try { + const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })] + expect(paths).toEqual([]) + } finally { + fs.rmSync(tmpDir, { recursive: true }) + } + }) + + test('yields pnpmfile.cjs path even if missing when the plugin directory exists (so requireHooks reports the misconfiguration)', () => { + const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-')) + try { + const pluginDir = path.join(tmpDir, 'pnpm-plugin-foo') + fs.mkdirSync(pluginDir, { recursive: true }) + + const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })] + expect(paths).toEqual([path.join(pluginDir, 'pnpmfile.cjs')]) + } finally { + fs.rmSync(tmpDir, { recursive: true }) + } + }) }) test('hoist: false removes hoistPattern', async () => { @@ -74,3 +131,101 @@ test('hoist: false removes hoistPattern', async () => { expect(config.hoist).toBe(false) expect(config.hoistPattern).toBeUndefined() }) + +describe('installConfigDepsAndLoadHooks', () => { + test('proceeds normally when configDependencies install succeeds', async () => { + prepare() + + jest.mocked(resolveAndInstallConfigDeps).mockResolvedValueOnce(undefined as never) + + const { config, context } = buildBaseConfig() + const result = await installConfigDepsAndLoadHooks(config, context) + + expect(result).toBeDefined() + expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1) + expect(loggerMock.debug).not.toHaveBeenCalled() + }) + + test('does not throw when install fails and tolerateConfigDependenciesErrors is true', async () => { + prepare() + + const simulatedError = new Error('401 Unauthorized: missing auth token') + jest.mocked(resolveAndInstallConfigDeps).mockRejectedValueOnce(simulatedError) + + const { config, context } = buildBaseConfig() + + const result = await installConfigDepsAndLoadHooks(config, context, { + tolerateConfigDependenciesErrors: true, + }) + + expect(result).toBeDefined() + expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1) + expect(logger.debug).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining('Failed to install configDependencies'), + err: simulatedError, + }) + ) + }) + + test('throws when install fails and tolerateConfigDependenciesErrors is not set (default behaviour)', async () => { + prepare() + + const simulatedError = new Error('401 Unauthorized: missing auth token') + jest.mocked(resolveAndInstallConfigDeps).mockRejectedValueOnce(simulatedError) + + const { config, context } = buildBaseConfig() + + await expect( + installConfigDepsAndLoadHooks(config, context) + ).rejects.toThrow('401 Unauthorized: missing auth token') + + expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1) + }) + + test('does not swallow store creation errors even when tolerateConfigDependenciesErrors is true', async () => { + prepare() + + const storeError = new Error('EACCES: permission denied opening store dir') + jest.mocked(createStoreController).mockRejectedValueOnce(storeError) + + const { config, context } = buildBaseConfig() + + await expect( + installConfigDepsAndLoadHooks(config, context, { tolerateConfigDependenciesErrors: true }) + ).rejects.toThrow('EACCES: permission denied opening store dir') + + expect(resolveAndInstallConfigDeps).not.toHaveBeenCalled() + expect(loggerMock.debug).not.toHaveBeenCalled() + }) + + test('does not swallow store close errors even when tolerateConfigDependenciesErrors is true', async () => { + prepare() + + jest.mocked(resolveAndInstallConfigDeps).mockResolvedValueOnce(undefined as never) + storeCloseMock.mockReset().mockRejectedValueOnce(new Error('store close failed')) + + const { config, context } = buildBaseConfig() + + await expect( + installConfigDepsAndLoadHooks(config, context, { tolerateConfigDependenciesErrors: true }) + ).rejects.toThrow('store close failed') + + expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1) + expect(loggerMock.debug).not.toHaveBeenCalled() + }) + + function buildBaseConfig (): { config: Config, context: ConfigContext } { + const dir = process.cwd() + const config = { + ignorePnpmfile: true, + configDependencies: { 'some-helper-pkg': '1.0.0+sha512-abc' }, + dir, + lockfileDir: dir, + } as unknown as Config + const context = { + rootProjectManifestDir: dir, + } as unknown as ConfigContext + return { config, context } + } +})