From 20e7affbe27baf841301883fe56588026f45e0cc Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 11 May 2026 16:22:25 +0200 Subject: [PATCH] fix: publish web-auth honors proxy when polling doneUrl (#11581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `pnpm publish` failed to complete the web-based authentication flow when an HTTP/HTTPS proxy was configured. `libnpmpublish` (used for the initial publish request) routes through the proxy, but the subsequent `doneUrl` polling went through `@pnpm/network.fetch` without forwarding any proxy/TLS settings. The registry rejected the poll with `403` because the source IP differed from the initial request, so publish hung on the QR-code prompt forever. - Adds `createDispatchedFetch(opts)` to `@pnpm/network.fetch` — a curried `fetchWithDispatcher` that pre-binds proxy / TLS / local-address / `configByUri`-derived client certificates. `publishPackedPkg` uses it to build an `OtpContext` whose `fetch` honors the same network configuration as the publish request. - `extractTlsConfigs` is now performed automatically inside `createDispatchedFetch` (and hoisted out of the per-request loop in `createFetchFromRegistry`), so callers only have to pass `configByUri` once. Fixes #11561. --- .../fix-11561-publish-web-auth-proxy.md | 7 ++ network/fetch/src/fetchFromRegistry.ts | 25 ++++++- network/fetch/src/index.ts | 2 +- network/fetch/test/fetchFromRegistry.test.ts | 19 ++++- releasing/commands/src/publish/otp.ts | 5 ++ .../commands/src/publish/publishPackedPkg.ts | 36 +++++++++- .../test/publish/createPublishContext.test.ts | 69 +++++++++++++++++++ 7 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 .changeset/fix-11561-publish-web-auth-proxy.md create mode 100644 releasing/commands/test/publish/createPublishContext.test.ts diff --git a/.changeset/fix-11561-publish-web-auth-proxy.md b/.changeset/fix-11561-publish-web-auth-proxy.md new file mode 100644 index 0000000000..3f4eae4dcf --- /dev/null +++ b/.changeset/fix-11561-publish-web-auth-proxy.md @@ -0,0 +1,7 @@ +--- +"@pnpm/network.fetch": patch +"@pnpm/releasing.commands": patch +"pnpm": patch +--- + +`pnpm publish` now honors the configured HTTP/HTTPS proxy (including `https_proxy`/`http_proxy`/`no_proxy` environment variables) when polling the registry's `doneUrl` during the web-based authentication flow. Previously the poll bypassed the proxy, causing the registry to respond `403` from a different source IP and the login to never complete [#11561](https://github.com/pnpm/pnpm/issues/11561). diff --git a/network/fetch/src/fetchFromRegistry.ts b/network/fetch/src/fetchFromRegistry.ts index bdd12a8d62..f7dec438f3 100644 --- a/network/fetch/src/fetchFromRegistry.ts +++ b/network/fetch/src/fetchFromRegistry.ts @@ -31,6 +31,28 @@ export function fetchWithDispatcher (url: string | URL, opts: FetchWithDispatche }) } +export interface CreateDispatchedFetchOptions extends DispatcherOptions { + /** + * Per-registry config (TLS, auth, etc.). When set, the matching TLS entries + * are automatically extracted into `clientCertificates` so callers don't + * have to do it themselves. + */ + configByUri?: Record +} + +/** + * Returns a {@link fetch} pre-bound to the given dispatcher options, so callers + * that need a fetch function (rather than a one-shot call) can route their + * requests through the configured proxy / TLS / local-address settings. + */ +export function createDispatchedFetch (opts: CreateDispatchedFetchOptions): (url: string | URL, opts?: RequestInit) => Promise { + const dispatcherOptions: DispatcherOptions = { + ...opts, + clientCertificates: opts.clientCertificates ?? extractTlsConfigs(opts.configByUri), + } + return (url, fetchOpts) => fetchWithDispatcher(url, { ...fetchOpts, dispatcherOptions }) +} + export type { DispatcherOptions } export interface CreateFetchFromRegistryOptions extends DispatcherOptions { @@ -39,6 +61,7 @@ export interface CreateFetchFromRegistryOptions extends DispatcherOptions { } export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOptions): FetchFromRegistry { + const clientCertificates = extractTlsConfigs(defaultOpts.configByUri) return async (url, opts): Promise => { const headers: Record = { 'user-agent': USER_AGENT, @@ -73,7 +96,7 @@ export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOpt ...defaultOpts, ...opts, strictSsl: defaultOpts.strictSsl ?? true, - clientCertificates: extractTlsConfigs(defaultOpts.configByUri), + clientCertificates, } const response = await fetchWithDispatcher(urlObject, { diff --git a/network/fetch/src/index.ts b/network/fetch/src/index.ts index 91c95e5369..58a483bcc7 100644 --- a/network/fetch/src/index.ts +++ b/network/fetch/src/index.ts @@ -1,4 +1,4 @@ export { clearDispatcherCache, getDispatcher } from './dispatcher.js' export { fetch, isRedirect, type RetryTimeoutOptions } from './fetch.js' -export { createFetchFromRegistry, type CreateFetchFromRegistryOptions, type DispatcherOptions, fetchWithDispatcher } from './fetchFromRegistry.js' +export { createDispatchedFetch, type CreateDispatchedFetchOptions, createFetchFromRegistry, type CreateFetchFromRegistryOptions, type DispatcherOptions, fetchWithDispatcher } from './fetchFromRegistry.js' export type { FetchFromRegistry } from '@pnpm/fetching.types' diff --git a/network/fetch/test/fetchFromRegistry.test.ts b/network/fetch/test/fetchFromRegistry.test.ts index 67cda1b46c..f5c09f6afa 100644 --- a/network/fetch/test/fetchFromRegistry.test.ts +++ b/network/fetch/test/fetchFromRegistry.test.ts @@ -3,7 +3,7 @@ import fs from 'node:fs' import path from 'node:path' import { expect, test } from '@jest/globals' -import { clearDispatcherCache, createFetchFromRegistry } from '@pnpm/network.fetch' +import { clearDispatcherCache, createDispatchedFetch, createFetchFromRegistry } from '@pnpm/network.fetch' import { ProxyServer } from 'https-proxy-server-express' import { type Dispatcher, getGlobalDispatcher, MockAgent, setGlobalDispatcher } from 'undici' @@ -292,3 +292,20 @@ test('redirect without location header throws error', async () => { await teardownMockAgent() } }) + +test('createDispatchedFetch returns a fetch bound to the given dispatcher options', async () => { + setupMockAgent() + try { + const mockPool = getMockAgent().get('http://registry.pnpm.io') + mockPool.intercept({ path: '/ping', method: 'GET' }).reply(200, 'pong') + + // MockAgent intercepts the global dispatcher; passing empty dispatcher + // options means getDispatcher returns undefined and fetch falls back to it. + const dispatchedFetch = createDispatchedFetch({}) + const res = await dispatchedFetch('http://registry.pnpm.io/ping') + expect(res.status).toBe(200) + await expect(res.text()).resolves.toBe('pong') + } finally { + await teardownMockAgent() + } +}) diff --git a/releasing/commands/src/publish/otp.ts b/releasing/commands/src/publish/otp.ts index 88dce5258c..07c21e22d5 100644 --- a/releasing/commands/src/publish/otp.ts +++ b/releasing/commands/src/publish/otp.ts @@ -37,6 +37,11 @@ export interface OtpParams { * - Web based authentication flow (authUrl/doneUrl in error body with doneUrl polling) * - Classic OTP prompt (manual code entry) * + * The caller is responsible for supplying a {@link OtpContext.fetch} that + * honors the desired network configuration (proxy, TLS, etc.); see + * https://github.com/pnpm/pnpm/issues/11561 for why this matters during the + * web-based authentication flow. + * * @see https://github.com/npm/cli/blob/7d900c46/lib/utils/otplease.js for npm's implementation. * @see https://github.com/npm/npm-profile/blob/main/lib/index.js for the webauth polling flow. */ diff --git a/releasing/commands/src/publish/publishPackedPkg.ts b/releasing/commands/src/publish/publishPackedPkg.ts index e1a7550305..dd000508ed 100644 --- a/releasing/commands/src/publish/publishPackedPkg.ts +++ b/releasing/commands/src/publish/publishPackedPkg.ts @@ -5,6 +5,7 @@ import path from 'node:path' import type { Config } from '@pnpm/config.reader' import { PnpmError } from '@pnpm/error' import { globalInfo, globalWarn } from '@pnpm/logger' +import { createDispatchedFetch } from '@pnpm/network.fetch' import type { ExportedManifest } from '@pnpm/releasing.exportable-manifest' import type { Creds, RegistryConfig } from '@pnpm/types' import type { PublishOptions } from 'libnpmpublish' @@ -15,9 +16,10 @@ import { createFailedToPublishError } from './FailedToPublishError.js' import { AuthTokenError, fetchAuthToken } from './oidc/authToken.js' import { getIdToken, IdTokenError } from './oidc/idToken.js' import { determineProvenance, ProvenanceError } from './oidc/provenance.js' -import { publishWithOtpHandling } from './otp.js' +import { type OtpContext, publishWithOtpHandling } from './otp.js' import type { PackResult } from './pack.js' import { allRegistryConfigKeys, type NormalizedRegistryUrl, parseSupportedRegistryUrl } from './registryConfigKeys.js' +import { SHARED_CONTEXT } from './utils/shared-context.js' export type PublishPackedPkgOptions = Pick & { +> & Partial> & { access?: 'public' | 'restricted' ci?: boolean otp?: string // NOTE: There is no existing test for the One-time Password feature @@ -94,7 +105,12 @@ export async function publishPackedPkg ( globalWarn(`Skip publishing ${name}@${version} (dry run)`) return summary } - const response = await publishWithOtpHandling({ manifest: publishedManifest, tarballData, publishOptions }) + const response = await publishWithOtpHandling({ + context: createPublishContext(opts), + manifest: publishedManifest, + publishOptions, + tarballData, + }) if (response.ok) { globalInfo(`✅ Published package ${name}@${version}`) return summary @@ -102,6 +118,20 @@ export async function publishPackedPkg ( throw await createFailedToPublishError(packResult, response) } +/** + * Builds the {@link OtpContext} used to drive the publish. The default fetch + * is replaced by one that respects proxy / TLS / local-address settings, so + * the `doneUrl` polling in the web-based authentication flow goes through + * the same network configuration as the initial publish request (see + * https://github.com/pnpm/pnpm/issues/11561). + */ +export function createPublishContext (opts: PublishPackedPkgOptions): OtpContext { + return { + ...SHARED_CONTEXT, + fetch: createDispatchedFetch({ ...opts, timeout: opts.fetchTimeout }), + } +} + /** * npm accepts both `bundledDependencies` and `bundleDependencies` in package.json and normalizes * to a list of dependency names. We mirror that normalization so consumers see a consistent array. diff --git a/releasing/commands/test/publish/createPublishContext.test.ts b/releasing/commands/test/publish/createPublishContext.test.ts new file mode 100644 index 0000000000..e495de0d40 --- /dev/null +++ b/releasing/commands/test/publish/createPublishContext.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, jest } from '@jest/globals' + +// Mock `@pnpm/network.fetch` so we can spy on the options that +// `createPublishContext` forwards to `createDispatchedFetch`. This is the +// wiring that fixes https://github.com/pnpm/pnpm/issues/11561. +const createDispatchedFetchMock = jest.fn<(opts: Record) => () => Promise>(() => () => Promise.resolve(new Response())) +const realNetworkFetch = await import('@pnpm/network.fetch') +jest.unstable_mockModule('@pnpm/network.fetch', () => ({ + ...realNetworkFetch, + createDispatchedFetch: createDispatchedFetchMock, +})) + +const { createPublishContext } = await import('../../src/publish/publishPackedPkg.js') + +function baseOpts (): Parameters[0] { + return { + configByUri: {}, + fetchTimeout: 60_000, + registries: { default: 'https://registry.npmjs.org/' }, + } as Parameters[0] +} + +describe('createPublishContext', () => { + it('forwards proxy / TLS / local-address settings to createDispatchedFetch', () => { + createDispatchedFetchMock.mockClear() + createPublishContext({ + ...baseOpts(), + httpProxy: 'http://proxy.example:8080', + httpsProxy: 'http://proxy.example:1234', + noProxy: 'localhost,127.0.0.1', + localAddress: '10.0.0.1', + strictSsl: false, + ca: 'ca-pem', + cert: 'cert-pem', + key: 'key-pem', + }) + expect(createDispatchedFetchMock).toHaveBeenCalledTimes(1) + expect(createDispatchedFetchMock.mock.calls[0][0]).toMatchObject({ + httpProxy: 'http://proxy.example:8080', + httpsProxy: 'http://proxy.example:1234', + noProxy: 'localhost,127.0.0.1', + localAddress: '10.0.0.1', + strictSsl: false, + ca: 'ca-pem', + cert: 'cert-pem', + key: 'key-pem', + }) + }) + + it('translates fetchTimeout to timeout', () => { + createDispatchedFetchMock.mockClear() + createPublishContext({ ...baseOpts(), fetchTimeout: 12_345 }) + expect(createDispatchedFetchMock.mock.calls[0][0]).toMatchObject({ timeout: 12_345 }) + }) + + it('forwards configByUri so registry-scoped TLS settings reach the dispatcher', () => { + createDispatchedFetchMock.mockClear() + const configByUri = { '//my-registry.example/': { tls: { cert: 'c', key: 'k' } } } + createPublishContext({ ...baseOpts(), configByUri }) + expect(createDispatchedFetchMock.mock.calls[0][0]).toMatchObject({ configByUri }) + }) + + it('produces a context.fetch that delegates to the dispatched fetch', () => { + const dispatchedFetch = jest.fn<() => Promise>(() => Promise.resolve(new Response())) + createDispatchedFetchMock.mockReturnValueOnce(dispatchedFetch as ReturnType) + const ctx = createPublishContext(baseOpts()) + expect(ctx.fetch).toBe(dispatchedFetch) + }) +})