From b23adecf859352cbe3a4eaa44832a4f1df1d8638 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:01:47 +0000 Subject: [PATCH] fix: restructure OTP handling with dedicated error types and direct retry Co-authored-by: KSXGitHub <11488886+KSXGitHub@users.noreply.github.com> --- cspell.json | 1 - .../plugin-commands-publishing/src/otp.ts | 83 +++++++++++-------- 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cspell.json b/cspell.json index 62a75972ec..bffcd28c77 100644 --- a/cspell.json +++ b/cspell.json @@ -187,7 +187,6 @@ "onclickoutside", "oomol", "ossl", - "otplease", "outfile", "overrider", "packlist", diff --git a/releasing/plugin-commands-publishing/src/otp.ts b/releasing/plugin-commands-publishing/src/otp.ts index ba7885c116..698688a78b 100644 --- a/releasing/plugin-commands-publishing/src/otp.ts +++ b/releasing/plugin-commands-publishing/src/otp.ts @@ -43,7 +43,6 @@ export interface OtpEnquirerResponse { otp?: string } -// @types/libnpmpublish unfortunately uses an outdated type definition of package.json export type OtpPublishFn = ( manifest: ExportedManifest, tarballData: Buffer, @@ -67,7 +66,6 @@ export interface OtpContext { export interface OtpParams { context?: OtpContext manifest: ExportedManifest - otpRetryAllowed?: boolean publishOptions: PublishOptions tarballData: Buffer } @@ -79,6 +77,7 @@ export const SHARED_CONTEXT: OtpContext = { fetch: (url, options) => fetch(url, options), globalInfo, process, + // @types/libnpmpublish unfortunately uses an outdated type definition of package.json publish: publish as unknown as OtpPublishFn, } @@ -104,12 +103,11 @@ const isOtpError = (error: unknown): error is OtpError => * @throws {@link OtpWebAuthTimeoutError} if the webauth browser flow times out. * @throws the original error if OTP handling is not applicable. * - * @see https://github.com/npm/cli/blob/7d900c46/lib/utils/otplease.js for npm's `otplease()` implementation. + * @see https://github.com/npm/cli/blob/7d900c46/lib/utils/otplease.js for npm's implementation. */ export async function publishWithOtpHandling ({ context = SHARED_CONTEXT, manifest, - otpRetryAllowed = true, publishOptions, tarballData, }: OtpParams): Promise { @@ -117,36 +115,41 @@ export async function publishWithOtpHandling ({ try { response = await context.publish(manifest, tarballData, publishOptions) } catch (error) { - if (otpRetryAllowed && !!(context.process.stdin.isTTY && context.process.stdout.isTTY) && isOtpError(error)) { - const fetchOptions: OtpWebAuthFetchOptions = { - method: 'GET', - retry: { - factor: publishOptions.fetchRetryFactor, - maxTimeout: publishOptions.fetchRetryMaxtimeout, - minTimeout: publishOptions.fetchRetryMintimeout, - retries: publishOptions.fetchRetries, - }, - timeout: publishOptions.timeout, - } - let otp: string | undefined - if (error.body?.authUrl && error.body?.doneUrl) { - otp = await webAuthOtp(error.body.authUrl, error.body.doneUrl, context, fetchOptions) - } else { - const enquirerResponse = await context.enquirer.prompt({ - message: 'This operation requires a one-time password.\nEnter OTP:', - name: 'otp', - type: 'input', - }) - otp = enquirerResponse?.otp || undefined - } - if (otp != null) { - return publishWithOtpHandling({ - context, - manifest, - otpRetryAllowed: false, - tarballData, - publishOptions: { ...publishOptions, otp }, - }) + if (!isOtpError(error)) { + throw error + } + if (!context.process.stdin.isTTY || !context.process.stdout.isTTY) { + throw new OtpNonInteractiveError() + } + const fetchOptions: OtpWebAuthFetchOptions = { + method: 'GET', + retry: { + factor: publishOptions.fetchRetryFactor, + maxTimeout: publishOptions.fetchRetryMaxtimeout, + minTimeout: publishOptions.fetchRetryMintimeout, + retries: publishOptions.fetchRetries, + }, + timeout: publishOptions.timeout, + } + let otp: string | undefined + if (error.body?.authUrl && error.body?.doneUrl) { + otp = await webAuthOtp(error.body.authUrl, error.body.doneUrl, context, fetchOptions) + } else { + const enquirerResponse = await context.enquirer.prompt({ + message: 'This operation requires a one-time password.\nEnter OTP:', + name: 'otp', + type: 'input', + }) + otp = enquirerResponse?.otp || undefined + } + if (otp != null) { + try { + return await context.publish(manifest, tarballData, { ...publishOptions, otp }) + } catch (retryError) { + if (isOtpError(retryError)) { + throw new OtpSecondChallengeError() + } + throw retryError } } throw error @@ -197,3 +200,15 @@ export class OtpWebAuthTimeoutError extends PnpmError { this.timeout = timeout } } + +export class OtpNonInteractiveError extends PnpmError { + constructor () { + super('OTP_NON_INTERACTIVE', 'The registry requires a one-time password (OTP) but pnpm is not running in an interactive terminal. Please set the --otp option.') + } +} + +export class OtpSecondChallengeError extends PnpmError { + constructor () { + super('OTP_SECOND_CHALLENGE', 'The registry requested a one-time password (OTP) a second time after one was already provided. This is unexpected behavior from the registry.') + } +}