Review fixes for the store signup JWT handoff

packages/app/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts

@@ -15,7 +15,7 @@ export type Scalars = {
   Int: { input: number; output: number; }
   Float: { input: number; output: number; }
   AccessRoleAssignee: { input: any; output: any; }
-  /** The ID for a AccessRole. */
+  /** The ID for an AccessRole. */
   AccessRoleID: { input: any; output: any; }
   AccessRoleRecordId: { input: any; output: any; }
   /** The ID for a ActionAudit. */

packages/organizations/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts

@@ -15,7 +15,7 @@ export type Scalars = {
   Int: { input: number; output: number; }
   Float: { input: number; output: number; }
   AccessRoleAssignee: { input: any; output: any; }
-  /** The ID for a AccessRole. */
+  /** The ID for an AccessRole. */
   AccessRoleID: { input: any; output: any; }
   AccessRoleRecordId: { input: any; output: any; }
   /** The ID for a ActionAudit. */

packages/store/src/cli/api/graphql/business-platform-organizations/generated/types.d.ts

@@ -15,7 +15,7 @@ export type Scalars = {
   Int: { input: number; output: number; }
   Float: { input: number; output: number; }
   AccessRoleAssignee: { input: any; output: any; }
-  /** The ID for a AccessRole. */
+  /** The ID for an AccessRole. */
   AccessRoleID: { input: any; output: any; }
   AccessRoleRecordId: { input: any; output: any; }
   /** The ID for a ActionAudit. */

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<typeof import('@shopify/cli-kit/node/system')>()),
+  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()
+  })
 })

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,8 @@ export default class StoreStripeAuth extends StoreCommand {
 
   public async run(): Promise<void> {
     const {flags} = await this.parse(StoreStripeAuth)
-    const signup = flags.signup ?? (await readSignupJwtFromStdin())
+    // A blank --signup counts as not supplied, so the command falls back to reading the JWT from stdin.
+    const signup = signupFlagValue(flags.signup) ?? (await readSignupJwtFromStdin())
 
     await authenticateStoreWithApp(
       {
@@ -54,21 +56,35 @@ 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 <jwt>, set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.'
+
+function signupFlagValue(signup: string | undefined): string | undefined {
+  const trimmed = signup?.trim()
+  return trimmed === '' ? undefined : trimmed
+}
+
 export async function readSignupJwtFromStdin(
   stdin: NodeJS.ReadableStream & AsyncIterable<Buffer | string> = process.stdin,
 ): Promise<string> {
+  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 input piped to stdin is too large to be a signup JWT.', 'Pipe only the signup JWT.')
+    }
+    chunks.push(buffer)
   }
 
   const signup = Buffer.concat(chunks).toString('utf8').trim()
-  if (!signup) {
-    throw new AbortError(
-      'Missing signup JWT.',
-      'Pass --signup <jwt>, set SHOPIFY_FLAG_SIGNUP, or pipe the JWT to stdin.',
-    )
-  }
+  if (!signup) throw new AbortError(MISSING_SIGNUP_JWT, MISSING_SIGNUP_JWT_GUIDANCE)
 
   return signup
 }

packages/store/src/cli/services/store/auth/callback.test.ts

@@ -38,6 +38,46 @@ function callbackParams(options?: {code?: string; shop?: string; state?: string;
   return params
 }
 
+const handoffNonce = 'nonce-123'
+const handoffSignupJwt = 'signed.signup.jwt'
+const handoffAuthorizationUrl = `https://shop.myshopify.com/admin/oauth/authorize?signup=${handoffSignupJwt}`
+
+interface HandoffResponse {
+  status: number
+  body: string
+  headers: Headers
+}
+
+async function fetchHandoff(url: string, init?: Parameters<typeof globalThis.fetch>[1]): Promise<HandoffResponse> {
+  const response = await globalThis.fetch(url, {redirect: 'manual', ...init})
+  return {status: response.status, headers: response.headers, body: await response.text()}
+}
+
+// Runs `probe` against a server holding a pending authorization handoff, then completes
+// authentication through the callback to prove the probe did not settle or break the auth flow.
+async function withPendingHandoff(probe: (handoff: {url: string; port: number}) => Promise<void>): Promise<void> {
+  const port = await getAvailablePort()
+  const params = callbackParams()
+
+  const onListening = async () => {
+    await probe({url: `http://127.0.0.1:${port}/auth/handoff?nonce=${handoffNonce}`, port})
+    const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`)
+    expect(callbackResponse.status).toBe(200)
+    await callbackResponse.text()
+  }
+
+  await expect(
+    waitForStoreAuthCode({
+      store: 'shop.myshopify.com',
+      state: 'state-123',
+      port,
+      timeoutMs: 1000,
+      authorizationRedirect: {nonce: handoffNonce, authorizationUrl: handoffAuthorizationUrl},
+      onListening,
+    }),
+  ).resolves.toBe('abc123')
+}
+
 describe('store auth callback server', () => {
   test('waitForStoreAuthCode resolves after a valid callback', async () => {
     const port = await getAvailablePort()
@@ -60,36 +100,60 @@ describe('store auth callback server', () => {
   })
 
   test('waitForStoreAuthCode redirects a valid authorization handoff without settling auth', async () => {
-    const port = await getAvailablePort()
-    const params = callbackParams()
-    const authorizationUrl = 'https://shop.myshopify.com/admin/oauth/authorize?signup=signed.signup.jwt'
-    const onListening = async () => {
-      const handoffResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/handoff?nonce=nonce-123`, {
-        redirect: 'manual',
-      })
-      expect(handoffResponse.status).toBe(302)
-      expect(handoffResponse.headers.get('Location')).toBe(authorizationUrl)
-      expect(handoffResponse.headers.get('Cache-Control')).toBe('no-store')
-      expect(handoffResponse.headers.get('Referrer-Policy')).toBe('no-referrer')
-
-      const callbackResponse = await globalThis.fetch(`http://127.0.0.1:${port}/auth/callback?${params.toString()}`)
-      expect(callbackResponse.status).toBe(200)
-      await callbackResponse.text()
-    }
+    await withPendingHandoff(async ({url}) => {
+      const handoff = await fetchHandoff(url)
+      expect(handoff.status).toBe(302)
+      expect(handoff.headers.get('Location')).toBe(handoffAuthorizationUrl)
+      expect(handoff.headers.get('Cache-Control')).toBe('no-store')
+      expect(handoff.headers.get('Referrer-Policy')).toBe('no-referrer')
+    })
+  })
 
-    await expect(
-      waitForStoreAuthCode({
-        store: 'shop.myshopify.com',
-        state: 'state-123',
-        port,
-        timeoutMs: 1000,
-        authorizationRedirect: {
-          nonce: 'nonce-123',
-          authorizationUrl,
-        },
-        onListening,
-      }),
-    ).resolves.toBe('abc123')
+  test('waitForStoreAuthCode answers 404 to a handoff request with a wrong nonce', async () => {
+    await withPendingHandoff(async ({port}) => {
+      const handoff = await fetchHandoff(`http://127.0.0.1:${port}/auth/handoff?nonce=wrong`)
+      expect(handoff.status).toBe(404)
+      expect(handoff.body).not.toContain(handoffSignupJwt)
+    })
+  })
+
+  test('waitForStoreAuthCode answers 404 to a handoff request with no nonce', async () => {
+    await withPendingHandoff(async ({port}) => {
+      const handoff = await fetchHandoff(`http://127.0.0.1:${port}/auth/handoff`)
+      expect(handoff.status).toBe(404)
+      expect(handoff.body).not.toContain(handoffSignupJwt)
+    })
+  })
+
+  test('waitForStoreAuthCode answers 404 to a non-GET handoff request', async () => {
+    await withPendingHandoff(async ({url}) => {
+      const handoff = await fetchHandoff(url, {method: 'POST'})
+      expect(handoff.status).toBe(404)
+      expect(handoff.body).not.toContain(handoffSignupJwt)
+    })
+  })
+
+  test('waitForStoreAuthCode answers 404 to a replayed handoff request', async () => {
+    await withPendingHandoff(async ({url}) => {
+      const served = await fetchHandoff(url)
+      expect(served.status).toBe(302)
+
+      const replay = await fetchHandoff(url)
+      expect(replay.status).toBe(404)
+      expect(replay.body).not.toContain(handoffSignupJwt)
+    })
+  })
+
+  test('waitForStoreAuthCode does not spend the handoff on a speculative browser fetch', async () => {
+    await withPendingHandoff(async ({url}) => {
+      const prefetch = await fetchHandoff(url, {headers: {'Sec-Purpose': 'prefetch;prerender'}})
+      expect(prefetch.status).toBe(404)
+      expect(prefetch.body).not.toContain(handoffSignupJwt)
+
+      const navigation = await fetchHandoff(url)
+      expect(navigation.status).toBe(302)
+      expect(navigation.headers.get('Location')).toBe(handoffAuthorizationUrl)
+    })
   })
 
   test('waitForStoreAuthCode rejects when callback state does not match', async () => {

packages/store/src/cli/services/store/auth/callback.ts

@@ -5,17 +5,27 @@ 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
   state: string
   port: number
   timeoutMs?: number
   onListening?: () => void | Promise<void>
-  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
       }