From dc5810489123e67f20c9806eaeb10e003d84ddb4 Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Mon, 31 Aug 2026 13:31:31 -0400 Subject: [PATCH 1/5] Harden the store auth handoff endpoint Answer 404 for every rejection so a local prober cannot distinguish a wrong nonce from a spent handoff, or either from a port with no auth in flight. Restrict the endpoint to GET, and decline a browser's speculative fetch so a prefetch or prerender cannot spend the single-use handoff before the navigation it is speculating about arrives. Co-Authored-By: Claude Opus 5 (1M context) Assisted-By: devx/39092f35-5041-4a88-ab8d-808c98322495 --- .../cli/services/store/auth/callback.test.ts | 94 +++++++++++++++++++ .../src/cli/services/store/auth/callback.ts | 54 +++++++---- 2 files changed, 128 insertions(+), 20 deletions(-) diff --git a/packages/store/src/cli/services/store/auth/callback.test.ts b/packages/store/src/cli/services/store/auth/callback.test.ts index 133caca5102..3e357d27c5e 100644 --- a/packages/store/src/cli/services/store/auth/callback.test.ts +++ b/packages/store/src/cli/services/store/auth/callback.test.ts @@ -92,6 +92,100 @@ describe('store auth callback server', () => { ).resolves.toBe('abc123') }) + test('waitForStoreAuthCode answers 404 for a wrong nonce, a replay, and a non-GET handoff request', async () => { + const port = await getAvailablePort() + const params = callbackParams() + const authorizationUrl = 'https://shop.myshopify.com/admin/oauth/authorize?signup=signed.signup.jwt' + const handoffUrl = `http://127.0.0.1:${port}/auth/handoff?nonce=nonce-123` + const statuses: Record = {} + const bodies: string[] = [] + + const onListening = async () => { + const wrongNonce = await globalThis.fetch(`http://127.0.0.1:${port}/auth/handoff?nonce=wrong`, { + redirect: 'manual', + }) + statuses.wrongNonce = wrongNonce.status + bodies.push(await wrongNonce.text()) + + const missingNonce = await globalThis.fetch(`http://127.0.0.1:${port}/auth/handoff`, {redirect: 'manual'}) + statuses.missingNonce = missingNonce.status + bodies.push(await missingNonce.text()) + + const notGet = await globalThis.fetch(handoffUrl, {method: 'POST', redirect: 'manual'}) + statuses.notGet = notGet.status + bodies.push(await notGet.text()) + + const served = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + statuses.served = served.status + await served.text() + + const replay = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + statuses.replay = replay.status + bodies.push(await replay.text()) + + const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`) + await callbackResponse.text() + } + + await expect( + waitForStoreAuthCode({ + store: 'shop.myshopify.com', + state: 'state-123', + port, + timeoutMs: 1000, + authorizationRedirect: {nonce: 'nonce-123', authorizationUrl}, + onListening, + }), + ).resolves.toBe('abc123') + + expect(statuses).toEqual({wrongNonce: 404, missingNonce: 404, notGet: 404, served: 302, replay: 404}) + expect(bodies.join('')).not.toContain('signed.signup.jwt') + }) + + test('waitForStoreAuthCode does not spend the handoff on a speculative browser fetch', async () => { + const port = await getAvailablePort() + const params = callbackParams() + const authorizationUrl = 'https://shop.myshopify.com/admin/oauth/authorize?signup=signed.signup.jwt' + const handoffUrl = `http://127.0.0.1:${port}/auth/handoff?nonce=nonce-123` + let prefetchStatus = 0 + let prefetchBody = '' + let navigationStatus = 0 + let navigationLocation: string | null = null + + const onListening = async () => { + const prefetch = await globalThis.fetch(handoffUrl, { + headers: {'Sec-Purpose': 'prefetch;prerender'}, + redirect: 'manual', + }) + prefetchStatus = prefetch.status + prefetchBody = await prefetch.text() + + const navigation = await globalThis.fetch(handoffUrl, {redirect: 'manual'}) + navigationStatus = navigation.status + navigationLocation = navigation.headers.get('Location') + await navigation.text() + + const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`) + await callbackResponse.text() + } + + await expect( + waitForStoreAuthCode({ + store: 'shop.myshopify.com', + state: 'state-123', + port, + timeoutMs: 1000, + authorizationRedirect: {nonce: 'nonce-123', authorizationUrl}, + onListening, + }), + ).resolves.toBe('abc123') + + expect(prefetchStatus).toBe(404) + expect(prefetchBody).not.toContain('signed.signup.jwt') + expect(navigationStatus).toBe(302) + expect(navigationLocation).toBe(authorizationUrl) + }) + test('waitForStoreAuthCode rejects when callback state does not match', async () => { const port = await getAvailablePort() const params = callbackParams({state: 'wrong-state'}) diff --git a/packages/store/src/cli/services/store/auth/callback.ts b/packages/store/src/cli/services/store/auth/callback.ts index eacf4b95de0..3331a08149b 100644 --- a/packages/store/src/cli/services/store/auth/callback.ts +++ b/packages/store/src/cli/services/store/auth/callback.ts @@ -5,6 +5,12 @@ import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {timingSafeEqual} from 'crypto' import {createServer} from 'http' +import type {IncomingHttpHeaders} from 'http' + +export interface AuthorizationRedirect { + nonce: string + authorizationUrl: string +} export interface WaitForAuthCodeOptions { store: string @@ -12,10 +18,14 @@ export interface WaitForAuthCodeOptions { port: number timeoutMs?: number onListening?: () => void | Promise - authorizationRedirect?: { - nonce: string - authorizationUrl: string - } + authorizationRedirect?: AuthorizationRedirect +} + +// Browsers announce a speculative fetch so servers can decline side effects. Serving one would spend +// the single-use handoff before the navigation it is speculating about ever arrives. +function isSpeculativeRequest(headers: IncomingHttpHeaders): boolean { + const purpose = [headers['sec-purpose'], headers.purpose, headers['x-moz']].flat().join(' ') + return purpose.includes('prefetch') || purpose.includes('prerender') } function renderAuthCallbackPage(title: string, message: string): string { @@ -119,21 +129,26 @@ export async function waitForStoreAuthCode({ const server = createServer((req, res) => { const requestUrl = new URL(req.url ?? '/', `http://127.0.0.1:${port}`) - if (requestUrl.pathname === STORE_AUTH_HANDOFF_PATH && authorizationRedirect) { - const returnedNonce = requestUrl.searchParams.get('nonce') - if (!returnedNonce || !constantTimeEqual(returnedNonce, authorizationRedirect.nonce)) { - res.statusCode = 403 - res.setHeader('Cache-Control', 'no-store') - res.setHeader('Connection', 'close') - res.end('Forbidden') - return - } + const notFound = () => { + res.statusCode = 404 + res.setHeader('Cache-Control', 'no-store') + res.setHeader('Connection', 'close') + res.end('Not found') + } - if (authorizationRedirectUsed) { - res.statusCode = 410 - res.setHeader('Cache-Control', 'no-store') - res.setHeader('Connection', 'close') - res.end('Authorization handoff already used') + if (requestUrl.pathname === STORE_AUTH_HANDOFF_PATH) { + const returnedNonce = requestUrl.searchParams.get('nonce') + // Every rejection answers 404 so a local prober cannot tell a wrong nonce from a spent + // handoff, or either from a port with no store auth in flight. + const servable = + authorizationRedirect !== undefined && + !authorizationRedirectUsed && + req.method === 'GET' && + returnedNonce !== null && + constantTimeEqual(returnedNonce, authorizationRedirect.nonce) + + if (!servable || isSpeculativeRequest(req.headers)) { + notFound() return } @@ -148,8 +163,7 @@ export async function waitForStoreAuthCode({ } if (requestUrl.pathname !== STORE_AUTH_CALLBACK_PATH) { - res.statusCode = 404 - res.end('Not found') + notFound() return } From db219b2c8492c4d6fde856fd967d489f09b3cd58 Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Mon, 31 Aug 2026 13:31:31 -0400 Subject: [PATCH 2/5] Keep the signup credential out of buildStoreAuthUrl buildStoreAuthUrl no longer accepts or serializes the credential; the URL that carries it is derived only for the loopback redirect. Also drops the unused signup field from the authorization context, which returned the credential to every caller of createPkceBootstrap. Co-Authored-By: Claude Opus 5 (1M context) Assisted-By: devx/39092f35-5041-4a88-ab8d-808c98322495 --- .../src/cli/services/store/auth/pkce.test.ts | 33 ++++++++++++------- .../store/src/cli/services/store/auth/pkce.ts | 28 ++++++++-------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/store/src/cli/services/store/auth/pkce.test.ts b/packages/store/src/cli/services/store/auth/pkce.test.ts index bb126f25d59..405e1c763ff 100644 --- a/packages/store/src/cli/services/store/auth/pkce.test.ts +++ b/packages/store/src/cli/services/store/auth/pkce.test.ts @@ -20,19 +20,28 @@ describe('store auth PKCE helpers', () => { expect(computeCodeChallenge(verifier)).toBe(expected) }) - test('buildStoreAuthUrl includes signup JWT when provided', () => { - const url = new URL( - buildStoreAuthUrl({ - store: 'shop.myshopify.com', - scopes: ['read_products'], - state: 'state-123', - redirectUri: 'http://127.0.0.1:13387/auth/callback', - codeChallenge: 'test-challenge-value', - signup: 'signed.signup.jwt', - }), - ) + test('buildStoreAuthUrl builds a store authorization URL that carries no signup credential', () => { + const url = buildStoreAuthUrl({ + store: 'shop.myshopify.com', + scopes: ['read_products'], + state: 'state-123', + redirectUri: 'http://127.0.0.1:13387/auth/callback', + codeChallenge: 'test-challenge-value', + }) + + expect(new URL(url).searchParams.has('signup')).toBe(false) + expect(url).not.toContain('signup') + }) + + test('createPkceBootstrap keeps the signup credential off the authorization context', () => { + const bootstrap = createPkceBootstrap({ + store: 'shop.myshopify.com', + scopes: ['read_products'], + signup: 'signed.signup.jwt', + exchangeCodeForToken: async () => ({access_token: 'token', scope: 'read_products'}), + }) - expect(url.searchParams.get('signup')).toBe('signed.signup.jwt') + expect(JSON.stringify(bootstrap.authorization)).not.toContain('signed.signup.jwt') }) test('createPkceBootstrap uses a loopback handoff URL when signup is provided', () => { diff --git a/packages/store/src/cli/services/store/auth/pkce.ts b/packages/store/src/cli/services/store/auth/pkce.ts index be97728fdf7..f1fd55372a4 100644 --- a/packages/store/src/cli/services/store/auth/pkce.ts +++ b/packages/store/src/cli/services/store/auth/pkce.ts @@ -3,7 +3,7 @@ import {randomUUID} from '@shopify/cli-kit/node/crypto' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {createHash, randomBytes} from 'crypto' import type {StoreTokenResponse} from './token-client.js' -import type {WaitForAuthCodeOptions} from './callback.js' +import type {AuthorizationRedirect, WaitForAuthCodeOptions} from './callback.js' interface StoreAuthorizationContext { store: string @@ -13,7 +13,6 @@ interface StoreAuthorizationContext { redirectUri: string authorizationUrl: string codeVerifier: string - signup?: string } interface StoreAuthBootstrap { @@ -36,7 +35,6 @@ export function buildStoreAuthUrl(options: { state: string redirectUri: string codeChallenge: string - signup?: string }): string { const params = new URLSearchParams() params.set('client_id', STORE_AUTH_APP_CLIENT_ID) @@ -46,11 +44,17 @@ export function buildStoreAuthUrl(options: { params.set('response_type', 'code') params.set('code_challenge', options.codeChallenge) params.set('code_challenge_method', 'S256') - if (options.signup) params.set('signup', options.signup) return `https://${options.store}/admin/oauth/authorize?${params.toString()}` } +function buildAuthorizationRedirect(storeAuthorizationUrl: string, signup: string): AuthorizationRedirect { + const authorizationUrl = new URL(storeAuthorizationUrl) + authorizationUrl.searchParams.set('signup', signup) + + return {nonce: randomBytes(32).toString('base64url'), authorizationUrl: authorizationUrl.toString()} +} + export function createPkceBootstrap(options: { store: string scopes: string[] @@ -68,9 +72,11 @@ export function createPkceBootstrap(options: { const redirectUri = storeAuthRedirectUri(port) const codeVerifier = generateCodeVerifier() const codeChallenge = computeCodeChallenge(codeVerifier) - const sensitiveAuthorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri, codeChallenge, signup}) - const handoffNonce = signup ? randomBytes(32).toString('base64url') : undefined - const authorizationUrl = handoffNonce ? storeAuthHandoffUri(port, handoffNonce) : sensitiveAuthorizationUrl + const storeAuthorizationUrl = buildStoreAuthUrl({store, scopes, state, redirectUri, codeChallenge}) + const authorizationRedirect = signup ? buildAuthorizationRedirect(storeAuthorizationUrl, signup) : undefined + const authorizationUrl = authorizationRedirect + ? storeAuthHandoffUri(port, authorizationRedirect.nonce) + : storeAuthorizationUrl outputDebug( outputContent`Starting PKCE auth for ${outputToken.raw(store)} with scopes ${outputToken.raw(scopes.join(','))} (redirect_uri=${outputToken.raw(redirectUri)})`, @@ -85,18 +91,12 @@ export function createPkceBootstrap(options: { redirectUri, authorizationUrl, codeVerifier, - signup, }, waitForAuthCodeOptions: { store, state, port, - authorizationRedirect: handoffNonce - ? { - nonce: handoffNonce, - authorizationUrl: sensitiveAuthorizationUrl, - } - : undefined, + authorizationRedirect, }, exchangeCodeForToken: (code: string) => exchangeCodeForToken({store, code, codeVerifier, redirectUri}), } From e386785d0f0227a45433d05cdc2e75858dccb7e2 Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Mon, 31 Aug 2026 13:31:31 -0400 Subject: [PATCH 3/5] Withhold manual auth URLs that carry a signup credential The presenter refused to print a URL only when a caller marked it sensitive, so a caller that forgot leaked it. Key the refusal on the URL itself as well, and fail closed on a URL that cannot be parsed. The call site no longer passes a hardcoded sensitive: false. Co-Authored-By: Claude Opus 5 (1M context) Assisted-By: devx/39092f35-5041-4a88-ab8d-808c98322495 --- .../src/cli/services/store/auth/index.test.ts | 2 -- .../src/cli/services/store/auth/index.ts | 2 +- .../cli/services/store/auth/result.test.ts | 35 +++++++++++++++++++ .../src/cli/services/store/auth/result.ts | 9 ++++- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/store/src/cli/services/store/auth/index.test.ts b/packages/store/src/cli/services/store/auth/index.test.ts index 63dad5722a1..beff7842c23 100644 --- a/packages/store/src/cli/services/store/auth/index.test.ts +++ b/packages/store/src/cli/services/store/auth/index.test.ts @@ -309,7 +309,6 @@ describe('store auth service', () => { expect(presenter.openingBrowser).toHaveBeenCalledOnce() expect(presenter.manualAuthUrl).toHaveBeenCalledWith( expect.stringContaining('https://shop.myshopify.com/admin/oauth/authorize?'), - {sensitive: false}, ) expect(presenter.success).toHaveBeenCalledWith(result) }) @@ -347,7 +346,6 @@ describe('store auth service', () => { expect(presenter.manualAuthUrl).toHaveBeenCalledWith( expect.stringContaining('http://127.0.0.1:13387/auth/handoff?nonce='), - {sensitive: false}, ) expect(presenter.manualAuthUrl.mock.calls[0]![0]).not.toContain('signed.signup.jwt') }) diff --git a/packages/store/src/cli/services/store/auth/index.ts b/packages/store/src/cli/services/store/auth/index.ts index dbf6443f345..9a1dc4c9f8e 100644 --- a/packages/store/src/cli/services/store/auth/index.ts +++ b/packages/store/src/cli/services/store/auth/index.ts @@ -76,7 +76,7 @@ export async function authenticateStoreWithApp( ...bootstrap.waitForAuthCodeOptions, onListening: async () => { const opened = await resolvedDependencies.openURL(authorizationUrl) - if (!opened) resolvedDependencies.presenter.manualAuthUrl(authorizationUrl, {sensitive: false}) + if (!opened) resolvedDependencies.presenter.manualAuthUrl(authorizationUrl) }, }) const tokenResponse = await bootstrap.exchangeCodeForToken(code) diff --git a/packages/store/src/cli/services/store/auth/result.test.ts b/packages/store/src/cli/services/store/auth/result.test.ts index 0e820ea1fcb..57ea6791de5 100644 --- a/packages/store/src/cli/services/store/auth/result.test.ts +++ b/packages/store/src/cli/services/store/auth/result.test.ts @@ -121,4 +121,39 @@ describe('store auth presenter', () => { expect(output.info()).not.toContain('secret=sensitive') expect(output.info()).not.toContain('https://shop.myshopify.com/admin/oauth/authorize') }) + + test('withholds a manual auth URL carrying a signup credential even when the caller does not mark it sensitive', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + presenter.manualAuthUrl('https://shop.myshopify.com/admin/oauth/authorize?client_id=test&signup=signed.signup.jwt') + + expect(output.info()).toContain( + 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', + ) + expect(output.info()).not.toContain('signed.signup.jwt') + expect(output.info()).not.toContain('signup=') + }) + + test('withholds a manual auth URL it cannot parse', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + presenter.manualAuthUrl('not-a-url?signup=signed.signup.jwt') + + expect(output.info()).toContain( + 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', + ) + expect(output.info()).not.toContain('signed.signup.jwt') + }) + + test('prints a loopback handoff URL that carries no credential', () => { + const output = mockAndCaptureOutput() + const presenter = createStoreAuthPresenter('text') + + presenter.manualAuthUrl('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + + expect(output.info()).toContain('Browser did not open automatically. Open this URL manually:') + expect(output.info()).toContain('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + }) }) diff --git a/packages/store/src/cli/services/store/auth/result.ts b/packages/store/src/cli/services/store/auth/result.ts index db83df4a277..33b71cec243 100644 --- a/packages/store/src/cli/services/store/auth/result.ts +++ b/packages/store/src/cli/services/store/auth/result.ts @@ -51,8 +51,15 @@ function displayStoreAuthOpeningBrowser(): void { outputInfo('') } +// Callers mark a URL sensitive when they know why it is; this catches the signup credential even when +// they forget, and fails closed on anything it cannot parse well enough to clear. +function carriesSignupCredential(authorizationUrl: string): boolean { + if (!URL.canParse(authorizationUrl)) return true + return new URL(authorizationUrl).searchParams.has('signup') +} + function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): void { - if (options.sensitive) { + if (options.sensitive || carriesSignupCredential(authorizationUrl)) { outputInfo( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', ) From 65cdb02105dbbdcf733733d10a0ef080e37d484b Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Mon, 31 Aug 2026 13:31:31 -0400 Subject: [PATCH 4/5] Don't wait on an interactive stdin for the signup JWT An interactive stdin never ends, so omitting --signup hung the command instead of reporting the missing credential. Report it immediately when stdin is not piped, cap what is read, and treat a blank --signup as not supplied rather than authorizing without a credential. Co-Authored-By: Claude Opus 5 (1M context) Assisted-By: devx/39092f35-5041-4a88-ab8d-808c98322495 --- .../cli/commands/store/stripe-auth.test.ts | 26 ++++++++++++++ .../src/cli/commands/store/stripe-auth.ts | 35 ++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/packages/store/src/cli/commands/store/stripe-auth.test.ts b/packages/store/src/cli/commands/store/stripe-auth.test.ts index aef7e95485a..75512b1eb2c 100644 --- a/packages/store/src/cli/commands/store/stripe-auth.test.ts +++ b/packages/store/src/cli/commands/store/stripe-auth.test.ts @@ -1,6 +1,7 @@ import StoreStripeAuth, {readSignupJwtFromStdin} from './stripe-auth.js' import {authenticateStoreWithApp} from '../../services/store/auth/index.js' import {createStoreAuthPresenter} from '../../services/store/auth/result.js' +import {isStdinPiped} from '@shopify/cli-kit/node/system' import {describe, expect, test, vi} from 'vitest' import {Readable} from 'stream' @@ -9,6 +10,10 @@ vi.mock('../../services/store/attribution.js') vi.mock('../../services/store/auth/result.js', () => ({ createStoreAuthPresenter: vi.fn((format: 'text' | 'json') => ({format})), })) +vi.mock('@shopify/cli-kit/node/system', async (importOriginal) => ({ + ...(await importOriginal()), + isStdinPiped: vi.fn(), +})) describe('store stripe-auth command', () => { test('passes signup JWT through to the auth service', async () => { @@ -71,4 +76,25 @@ describe('store stripe-auth command', () => { test('rejects blank stdin signup JWTs', async () => { await expect(readSignupJwtFromStdin(Readable.from(['\n']))).rejects.toThrow('Missing signup JWT') }) + + test('reports the missing credential instead of waiting when stdin is an interactive terminal', async () => { + vi.mocked(isStdinPiped).mockReturnValue(false) + + await expect(readSignupJwtFromStdin()).rejects.toThrow('Missing signup JWT') + }) + + test('rejects a stdin signup JWT larger than the accepted size', async () => { + const oversized = 'a'.repeat(8 * 1024 + 1) + + await expect(readSignupJwtFromStdin(Readable.from([oversized]))).rejects.toThrow('too large') + }) + + test('does not authenticate when the signup flag is empty and no JWT is piped', async () => { + vi.mocked(isStdinPiped).mockReturnValue(false) + + await expect( + StoreStripeAuth.run(['--store', 'shop.myshopify.com', '--scopes', 'read_products', '--signup', '']), + ).rejects.toThrow() + expect(authenticateStoreWithApp).not.toHaveBeenCalled() + }) }) diff --git a/packages/store/src/cli/commands/store/stripe-auth.ts b/packages/store/src/cli/commands/store/stripe-auth.ts index cf33523c17f..1ce18e20e33 100644 --- a/packages/store/src/cli/commands/store/stripe-auth.ts +++ b/packages/store/src/cli/commands/store/stripe-auth.ts @@ -4,6 +4,7 @@ import StoreCommand from '../../utilities/store-command.js' import {storeFlags} from '../../flags.js' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' import {AbortError} from '@shopify/cli-kit/node/error' +import {isStdinPiped} from '@shopify/cli-kit/node/system' import {Flags} from '@oclif/core' export default class StoreStripeAuth extends StoreCommand { @@ -39,7 +40,7 @@ export default class StoreStripeAuth extends StoreCommand { public async run(): Promise { const {flags} = await this.parse(StoreStripeAuth) - const signup = flags.signup ?? (await readSignupJwtFromStdin()) + const signup = signupFlagValue(flags.signup) ?? (await readSignupJwtFromStdin()) await authenticateStoreWithApp( { @@ -54,21 +55,39 @@ export default class StoreStripeAuth extends StoreCommand { } } +const MAX_SIGNUP_JWT_BYTES = 8 * 1024 +const MISSING_SIGNUP_JWT = 'Missing signup JWT.' +const MISSING_SIGNUP_JWT_GUIDANCE = 'Pass --signup , set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.' + +// A blank --signup is a credential that was never supplied rather than an empty one, so it falls +// through to stdin instead of starting an authorization without it. +function signupFlagValue(signup: string | undefined): string | undefined { + const trimmed = signup?.trim() + return trimmed === '' ? undefined : trimmed +} + export async function readSignupJwtFromStdin( stdin: NodeJS.ReadableStream & AsyncIterable = process.stdin, ): Promise { + // An interactive stdin never ends, so reading it would hang the command instead of reporting the + // credential that was never supplied. + if (stdin === process.stdin && !isStdinPiped()) { + throw new AbortError(MISSING_SIGNUP_JWT, MISSING_SIGNUP_JWT_GUIDANCE) + } + const chunks: Buffer[] = [] + let byteLength = 0 for await (const chunk of stdin) { - chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk) + byteLength += buffer.length + if (byteLength > MAX_SIGNUP_JWT_BYTES) { + throw new AbortError('The signup JWT piped to stdin is too large.', 'Pipe only the JWT.') + } + chunks.push(buffer) } const signup = Buffer.concat(chunks).toString('utf8').trim() - if (!signup) { - throw new AbortError( - 'Missing signup JWT.', - 'Pass --signup , set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.', - ) - } + if (!signup) throw new AbortError(MISSING_SIGNUP_JWT, MISSING_SIGNUP_JWT_GUIDANCE) return signup } From d12057e7a5f01032295a45b68e0a0f4d2989eb6c Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Tue, 1 Sep 2026 11:09:30 -0400 Subject: [PATCH 5/5] Abort when no authorization URL reached the browser The presenter can withhold a URL for more than one reason, and the caller had no way to know it had. Waiting afterwards idles until the five-minute callback timeout, because nothing can reach the local server without a URL for the browser to visit. manualAuthUrl now reports whether it surfaced a usable URL, and the caller aborts when it did not. This restores the fail-fast behaviour added for the withheld-URL case, keyed on what actually happened rather than on whether a signup credential was supplied. Co-Authored-By: Claude Opus 5 (1M context) Assisted-By: devx/39092f35-5041-4a88-ab8d-808c98322495 --- .../src/cli/services/store/auth/index.test.ts | 37 ++++++++++++++++++- .../src/cli/services/store/auth/index.ts | 7 +++- .../cli/services/store/auth/result.test.ts | 23 +++++++++--- .../src/cli/services/store/auth/result.ts | 8 ++-- 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/packages/store/src/cli/services/store/auth/index.test.ts b/packages/store/src/cli/services/store/auth/index.test.ts index beff7842c23..24f15316b5d 100644 --- a/packages/store/src/cli/services/store/auth/index.test.ts +++ b/packages/store/src/cli/services/store/auth/index.test.ts @@ -280,7 +280,7 @@ describe('store auth service', () => { const openURL = vi.fn().mockResolvedValue(false) const presenter = { openingBrowser: vi.fn(), - manualAuthUrl: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(true), success: vi.fn(), } const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { @@ -317,7 +317,7 @@ describe('store auth service', () => { const openURL = vi.fn().mockResolvedValue(false) const presenter = { openingBrowser: vi.fn(), - manualAuthUrl: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(true), success: vi.fn(), } const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { @@ -350,6 +350,39 @@ describe('store auth service', () => { expect(presenter.manualAuthUrl.mock.calls[0]![0]).not.toContain('signed.signup.jwt') }) + test('authenticateStoreWithApp fails immediately when the presenter withholds the authorization URL', async () => { + const openURL = vi.fn().mockResolvedValue(false) + const presenter = { + openingBrowser: vi.fn(), + manualAuthUrl: vi.fn().mockReturnValue(false), + success: vi.fn(), + } + const exchangeStoreAuthCodeForToken = vi.fn() + const waitForStoreAuthCodeMock = vi.fn().mockImplementation(async (options) => { + await options.onListening?.() + return 'abc123' + }) + + await expect( + authenticateStoreWithApp( + { + store: 'shop.myshopify.com', + scopes: 'read_products', + signup: 'signed.signup.jwt', + }, + { + openURL, + waitForStoreAuthCode: waitForStoreAuthCodeMock, + exchangeStoreAuthCodeForToken, + presenter, + }, + ), + ).rejects.toThrow("Authentication can't continue without a browser.") + + expect(exchangeStoreAuthCodeForToken).not.toHaveBeenCalled() + expect(presenter.success).not.toHaveBeenCalled() + }) + test('authenticateStoreWithApp records fqdn metadata before resolving existing scopes', async () => { await expect( authenticateStoreWithApp( diff --git a/packages/store/src/cli/services/store/auth/index.ts b/packages/store/src/cli/services/store/auth/index.ts index 9a1dc4c9f8e..ea7e5962293 100644 --- a/packages/store/src/cli/services/store/auth/index.ts +++ b/packages/store/src/cli/services/store/auth/index.ts @@ -76,7 +76,12 @@ export async function authenticateStoreWithApp( ...bootstrap.waitForAuthCodeOptions, onListening: async () => { const opened = await resolvedDependencies.openURL(authorizationUrl) - if (!opened) resolvedDependencies.presenter.manualAuthUrl(authorizationUrl) + if (opened) return + + // The callback server can only be reached by a browser that was given a URL, so waiting after a + // withheld one would idle until the timeout instead of reporting that authentication cannot proceed. + const surfaced = resolvedDependencies.presenter.manualAuthUrl(authorizationUrl) + if (!surfaced) throw new AbortError("Authentication can't continue without a browser.") }, }) const tokenResponse = await bootstrap.exchangeCodeForToken(code) diff --git a/packages/store/src/cli/services/store/auth/result.test.ts b/packages/store/src/cli/services/store/auth/result.test.ts index 57ea6791de5..92a98d2081f 100644 --- a/packages/store/src/cli/services/store/auth/result.test.ts +++ b/packages/store/src/cli/services/store/auth/result.test.ts @@ -108,9 +108,12 @@ describe('store auth presenter', () => { const output = mockAndCaptureOutput() const presenter = createStoreAuthPresenter('text') - presenter.manualAuthUrl('https://shop.myshopify.com/admin/oauth/authorize?client_id=test&secret=sensitive', { - sensitive: true, - }) + const surfaced = presenter.manualAuthUrl( + 'https://shop.myshopify.com/admin/oauth/authorize?client_id=test&secret=sensitive', + {sensitive: true}, + ) + + expect(surfaced).toBe(false) expect(output.info()).toContain( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', @@ -126,7 +129,11 @@ describe('store auth presenter', () => { const output = mockAndCaptureOutput() const presenter = createStoreAuthPresenter('text') - presenter.manualAuthUrl('https://shop.myshopify.com/admin/oauth/authorize?client_id=test&signup=signed.signup.jwt') + const surfaced = presenter.manualAuthUrl( + 'https://shop.myshopify.com/admin/oauth/authorize?client_id=test&signup=signed.signup.jwt', + ) + + expect(surfaced).toBe(false) expect(output.info()).toContain( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', @@ -139,7 +146,9 @@ describe('store auth presenter', () => { const output = mockAndCaptureOutput() const presenter = createStoreAuthPresenter('text') - presenter.manualAuthUrl('not-a-url?signup=signed.signup.jwt') + const surfaced = presenter.manualAuthUrl('not-a-url?signup=signed.signup.jwt') + + expect(surfaced).toBe(false) expect(output.info()).toContain( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', @@ -151,7 +160,9 @@ describe('store auth presenter', () => { const output = mockAndCaptureOutput() const presenter = createStoreAuthPresenter('text') - presenter.manualAuthUrl('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + const surfaced = presenter.manualAuthUrl('http://127.0.0.1:13387/auth/handoff?nonce=abc123') + + expect(surfaced).toBe(true) expect(output.info()).toContain('Browser did not open automatically. Open this URL manually:') expect(output.info()).toContain('http://127.0.0.1:13387/auth/handoff?nonce=abc123') diff --git a/packages/store/src/cli/services/store/auth/result.ts b/packages/store/src/cli/services/store/auth/result.ts index 33b71cec243..59d20acb1d8 100644 --- a/packages/store/src/cli/services/store/auth/result.ts +++ b/packages/store/src/cli/services/store/auth/result.ts @@ -25,7 +25,7 @@ interface ManualAuthUrlOptions { export interface StoreAuthPresenter { openingBrowser: () => void - manualAuthUrl: (authorizationUrl: string, options?: ManualAuthUrlOptions) => void + manualAuthUrl: (authorizationUrl: string, options?: ManualAuthUrlOptions) => boolean success: (result: StoreAuthResult) => void } @@ -58,19 +58,21 @@ function carriesSignupCredential(authorizationUrl: string): boolean { return new URL(authorizationUrl).searchParams.has('signup') } -function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): void { +function displayStoreAuthManualAuthUrl(authorizationUrl: string, options: ManualAuthUrlOptions = {}): boolean { if (options.sensitive || carriesSignupCredential(authorizationUrl)) { outputInfo( 'Browser did not open automatically. The manual authorization URL contains sensitive credentials and was not printed.', ) outputInfo('Run this command again in an environment where Shopify CLI can open a browser automatically.') outputInfo('') - return + return false } outputInfo('Browser did not open automatically. Open this URL manually:') outputInfo(outputContent`${outputToken.link(authorizationUrl)}`) outputInfo('') + + return true } function displayStoreAuthResult(result: StoreAuthResult, format: StoreAuthOutputFormat = 'text'): void {