diff --git a/.changeset/safe-mpp-redirects.md b/.changeset/safe-mpp-redirects.md new file mode 100644 index 0000000..dc551df --- /dev/null +++ b/.changeset/safe-mpp-redirects.md @@ -0,0 +1,8 @@ +--- +'@stripe/link-cli': patch +--- + +Send MPP payment credentials only to the URL that returned the payment challenge. +Approved spend requests and paid retries now reject redirects instead of moving +credentials to a new destination. Remote MPP endpoints must use HTTPS; HTTP +remains supported for exact loopback addresses used in local development. diff --git a/packages/cli/src/__tests__/cli.test.ts b/packages/cli/src/__tests__/cli.test.ts index dc2abd8..38e8f69 100644 --- a/packages/cli/src/__tests__/cli.test.ts +++ b/packages/cli/src/__tests__/cli.test.ts @@ -75,7 +75,10 @@ let serverPort: number; let lastRequest: RequestLog; let requests: RequestLog[]; let nextResponse: { status: number; body: unknown }; -let responsesByUrl: Record = {}; +let responsesByUrl: Record< + string, + { status: number; body: unknown; headers?: Record } +> = {}; // ─── Second mock server for merchant endpoints ───────────────────────────── let merchantServer: http.Server; @@ -99,8 +102,13 @@ function setNextResponse(status: number, body: unknown) { nextResponse = { status, body }; } -function setResponseForUrl(url: string, status: number, body: unknown) { - responsesByUrl[url] = { status, body }; +function setResponseForUrl( + url: string, + status: number, + body: unknown, + headers?: Record, +) { + responsesByUrl[url] = { status, body, headers }; } async function runProdCli(...args: string[]): Promise { @@ -195,6 +203,7 @@ describe('production mode', () => { const response = urlOverride ?? nextResponse; res.writeHead(response.status, { 'Content-Type': 'application/json', + ...response.headers, }); res.end(JSON.stringify(response.body)); }); @@ -2894,17 +2903,57 @@ describe('production mode', () => { it('carries the raw URL in pay_argv and a quoted URL in pay_command', async () => { const marker = `${os.tmpdir()}/link-cli-injection-argv-${process.pid}`; const url = payloadUrl(marker); + const effectiveUrl = new URL(url).href; const next = await runFullFlow(url); expect(next.pay_argv.command).toBe('mpp'); expect(next.pay_argv.args[0]).toBe('pay'); - expect(next.pay_argv.args[1]).toBe(url); + expect(next.pay_argv.args[1]).toBe(effectiveUrl); expect(next.pay_argv.args).toContain('--spend-request-id'); expect(next.pay_argv.args).toContain('lsrq_spt_002'); expect(next.pay_command).not.toContain('pay $(touch'); - expect(next.pay_command).toContain(`'${url}'`); + expect(next.pay_command).toContain(`'${effectiveUrl}'`); + }); + + it('continues from the effective redirected request', async () => { + setNextResponse(200, PENDING_SPT_REQUEST); + setResponseForUrl('/merchant-redirect', 302, null, { + Location: `http://127.0.0.1:${merchantPort}/api/charge`, + }); + setMerchantResponse(402, '{"error":"payment required"}', { + 'www-authenticate': WWW_AUTHENTICATE_STRIPE, + }); + + const result = await runProdCli( + 'mpp', + 'pay', + `http://127.0.0.1:${serverPort}/merchant-redirect`, + '--context', + VALID_CONTEXT, + '--payment-method-id', + 'pd_prod_test', + '--data', + '{"item":"book"}', + '--header', + 'Authorization: Bearer caller-value', + '--format', + 'json', + ); + + expect(result.exitCode).toBe(0); + const output = parseJson(result.stdout) as Array<{ + _next: { pay_argv: { command: string; args: string[] } }; + }>; + const args = output[0]._next.pay_argv.args; + expect(args[1]).toBe(`http://127.0.0.1:${merchantPort}/api/charge`); + expect(args.slice(args.indexOf('-X'), args.indexOf('-X') + 2)).toEqual([ + '-X', + 'GET', + ]); + expect(args).not.toContain('-d'); + expect(args.join(' ')).not.toMatch(/authorization|content-type/i); }); it('does not execute the payload when pay_command is run through bash', async () => { diff --git a/packages/cli/src/commands/mpp/index.tsx b/packages/cli/src/commands/mpp/index.tsx index 39ab16f..c990a94 100644 --- a/packages/cli/src/commands/mpp/index.tsx +++ b/packages/cli/src/commands/mpp/index.tsx @@ -18,6 +18,7 @@ import { runMppPayFullFlow, runMppPayWithSpendRequest, } from './pay'; +import { createMppRequest, probeMppRequest } from './request'; import { decodeOptions, payOptions } from './schema'; export function createMppCli( @@ -91,11 +92,10 @@ export function createMppCli( const httpMethod = method ?? (data !== undefined ? 'POST' : 'GET'); const requestHeaders = buildHeaders(data, headers); - const probeResponse = await fetch(url, { - method: httpMethod, - body: data, - headers: requestHeaders, - }); + const probe = await probeMppRequest( + createMppRequest(url, httpMethod, data, requestHeaders), + ); + const probeResponse = probe.response; if (probeResponse.status !== 402) { yield await readPayResult(probeResponse); @@ -111,6 +111,7 @@ export function createMppCli( } const decoded = decodeStripeChallenge(wwwAuth); + await probeResponse.body?.cancel(); const networkId = decoded.network_id; const challengeAmount = decoded.request_json.amount ? Number(decoded.request_json.amount) @@ -160,14 +161,20 @@ export function createMppCli( test: opts.test || undefined, }); - // Build the mpp pay continuation for _next with the spend request ID. - // `url`, `data` and `header` carry merchant-controlled text, so the - // argv form is authoritative and `pay_command` must stay shell-quoted. - const nextArgs = ['pay', url, '--spend-request-id', spendRequest.id]; - if (method) nextArgs.push('-X', method); - if (data) nextArgs.push('-d', data); - if (headers) { - for (const h of headers) nextArgs.push('-H', h); + // Continue from the request that actually returned the challenge. Redirects + // may have changed its URL, method, body, or safe-to-forward headers. + // Merchant-controlled values stay shell-quoted in the display command. + const nextArgs = [ + 'pay', + probe.url, + '--spend-request-id', + spendRequest.id, + '-X', + probe.method, + ]; + if (probe.body !== undefined) nextArgs.push('-d', probe.body); + for (const [name, value] of probe.headers) { + nextArgs.push('-H', `${name}: ${value}`); } const nextCommand = `mpp ${shellCommand(nextArgs)}`; const pollCommand = `spend-request retrieve ${shellQuote(spendRequest.id)} --interval 2 --max-attempts 300`; diff --git a/packages/cli/src/commands/mpp/pay.test.ts b/packages/cli/src/commands/mpp/pay.test.ts new file mode 100644 index 0000000..4052b4d --- /dev/null +++ b/packages/cli/src/commands/mpp/pay.test.ts @@ -0,0 +1,140 @@ +import type { ISpendRequestResource } from '@stripe/link-sdk'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { payWithSpt, runMppPayFullFlow } from './pay'; + +const WWW_AUTHENTICATE_STRIPE = [ + 'Payment id="ch_001",', + 'realm="merchant.example",', + 'method="stripe",', + 'intent="charge",', + `request="${Buffer.from(JSON.stringify({ networkId: 'net_001', amount: '1000', currency: 'usd', decimals: 2, paymentMethodTypes: ['card'] })).toString('base64')}",`, + 'expires="2099-01-01T00:00:00Z"', +].join(' '); + +function challengeResponse(): Response { + return new Response('{"error":"payment required"}', { + status: 402, + headers: { 'www-authenticate': WWW_AUTHENTICATE_STRIPE }, + }); +} + +beforeEach(() => { + vi.stubGlobal('__CLI_VERSION__', 'test'); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('payWithSpt', () => { + it('rejects a redirect before using an approved credential', async () => { + const fetcher = vi.fn().mockResolvedValue( + new Response(null, { + status: 307, + headers: { location: 'https://other.example/challenge' }, + }), + ); + vi.stubGlobal('fetch', fetcher); + + await expect( + payWithSpt( + 'https://merchant.example/challenge', + 'spt_test_123', + undefined, + undefined, + undefined, + ), + ).rejects.toThrow(/redirected with status 307 after approval/); + + expect(fetcher).toHaveBeenCalledTimes(1); + expect(fetcher.mock.calls[0][0]).toBe('https://merchant.example/challenge'); + expect( + new Headers(fetcher.mock.calls[0][1]?.headers).has('authorization'), + ).toBe(false); + }); + + it('replaces caller authorization and refuses a redirect after payment', async () => { + const fetcher = vi + .fn() + .mockResolvedValueOnce(challengeResponse()) + .mockResolvedValueOnce( + new Response(null, { + status: 307, + headers: { location: 'https://other.example/payment' }, + }), + ); + vi.stubGlobal('fetch', fetcher); + + await expect( + payWithSpt( + 'https://merchant.example/challenge', + 'spt_test_123', + 'POST', + '{"item":"book"}', + ['authorization: Bearer caller-value'], + ), + ).rejects.toThrow('redirect 307'); + + const paidHeaders = new Headers(fetcher.mock.calls[1][1]?.headers); + expect( + [...paidHeaders].filter(([name]) => name === 'authorization'), + ).toEqual([['authorization', expect.stringMatching(/^Payment /)]]); + expect(fetcher).toHaveBeenCalledTimes(2); + expect(fetcher.mock.calls[1][1]?.redirect).toBe('manual'); + expect(fetcher.mock.calls[1][1]?.body).toBe('{"item":"book"}'); + }); + + it('refreshes an approved challenge at the pinned destination without following redirects', async () => { + const repository = { + create: vi.fn().mockResolvedValue({ + id: 'lsrq_123', + status: 'pending_approval', + }), + retrieve: vi + .fn() + .mockResolvedValueOnce({ id: 'lsrq_123', status: 'approved' }) + .mockResolvedValueOnce({ + id: 'lsrq_123', + status: 'approved', + shared_payment_token: { id: 'spt_test_123' }, + }), + } as unknown as ISpendRequestResource; + const fetcher = vi + .fn() + .mockResolvedValueOnce( + new Response(null, { + status: 302, + headers: { location: 'https://merchant.example/challenge' }, + }), + ) + .mockResolvedValueOnce(challengeResponse()) + .mockResolvedValueOnce( + new Response(null, { + status: 307, + headers: { location: 'https://other.example/challenge' }, + }), + ); + vi.stubGlobal('fetch', fetcher); + + await expect( + runMppPayFullFlow({ + url: 'https://redirector.example/start', + method: 'GET', + data: undefined, + headers: undefined, + context: + 'Buy a test item from the merchant after explicit Link approval for this machine payment request.', + amountOverride: 1000, + paymentMethodId: 'pd_test_123', + test: true, + repository, + paymentMethodsFactory: vi.fn(), + }), + ).rejects.toThrow(/redirected with status 307 after approval/); + expect(fetcher.mock.calls.map(([url]) => url)).toEqual([ + 'https://redirector.example/start', + 'https://merchant.example/challenge', + 'https://merchant.example/challenge', + ]); + }); +}); diff --git a/packages/cli/src/commands/mpp/pay.tsx b/packages/cli/src/commands/mpp/pay.tsx index 137dace..788f4f8 100644 --- a/packages/cli/src/commands/mpp/pay.tsx +++ b/packages/cli/src/commands/mpp/pay.tsx @@ -14,6 +14,14 @@ import { decodeStripeChallenge, getStripeChargeChallengeFromResponse, } from './decode'; +import { + type MppProbe, + type MppRequest, + createMppRequest, + fetchMppRequest, + isRedirectResponse, + probeMppRequest, +} from './request'; export type PayResult = { status: number; @@ -162,30 +170,58 @@ export async function payWithSpt( ): Promise { const httpMethod = method ?? (data !== undefined ? 'POST' : 'GET'); const requestHeaders = buildHeaders(data, headers); + return payPinnedChallengeWithSpt( + createMppRequest(url, httpMethod, data, requestHeaders), + spt, + ); +} - const initialResponse = await fetch(url, { - method: httpMethod, - body: data, - headers: requestHeaders, +async function submitMppPayment( + challenge: MppProbe, + spt: string, +): Promise { + // Credential creation needs only the challenge status and headers. Keep the + // untrusted response body out of signing and release its stream separately. + const credentialResponse = new Response(null, { + status: challenge.response.status, + statusText: challenge.response.statusText, + headers: challenge.response.headers, }); + const authHeader = + await createStripePaymentClient(spt).createCredential(credentialResponse); + await challenge.response.body?.cancel(); - if (initialResponse.status !== 402) { - return readPayResult(initialResponse); + const paidRequest = { + ...challenge, + headers: new Headers(challenge.headers), + }; + paidRequest.headers.set('Authorization', authHeader); + const response = await fetchMppRequest(paidRequest); + if (isRedirectResponse(response)) { + await response.body?.cancel(); + throw new Error( + `Paid MPP request returned redirect ${response.status}; refusing to forward the payment credential`, + ); } + return readPayResult(response); +} - const authHeader = - await createStripePaymentClient(spt).createCredential(initialResponse); - - const retryResponse = await fetch(url, { - method: httpMethod, - body: data, - headers: { - ...requestHeaders, - Authorization: authHeader, - }, - }); - - return readPayResult(retryResponse); +async function payPinnedChallengeWithSpt( + request: MppRequest, + spt: string, +): Promise { + // Approved credentials may be used minutes later. Refresh the challenge at + // the pinned destination, but never let that destination move afterward. + const response = await fetchMppRequest(request); + if (isRedirectResponse(response)) { + await response.body?.cancel(); + throw new Error( + `MPP challenge destination redirected with status ${response.status} after approval`, + ); + } + const refreshed = { ...request, response }; + if (response.status !== 402) return readPayResult(response); + return submitMppPayment(refreshed, spt); } export async function runMppPayFullFlow( @@ -211,11 +247,10 @@ export async function runMppPayFullFlow( // 1. Probe URL onStep?.('probing'); - const probeResponse = await fetch(url, { - method: httpMethod, - body: data, - headers: requestHeaders, - }); + const probe = await probeMppRequest( + createMppRequest(url, httpMethod, data, requestHeaders), + ); + const probeResponse = probe.response; if (probeResponse.status !== 402) { return readPayResult(probeResponse); @@ -228,6 +263,7 @@ export async function runMppPayFullFlow( } const decoded = decodeStripeChallenge(wwwAuth); + await probeResponse.body?.cancel(); const networkId = decoded.network_id; const challengeAmount = decoded.request_json.amount ? Number(decoded.request_json.amount) @@ -298,13 +334,7 @@ export async function runMppPayFullFlow( // 7. Pay onStep?.('submitting'); - return payWithSpt( - url, - withSpt.shared_payment_token.id, - method, - data, - headers, - ); + return payPinnedChallengeWithSpt(probe, withSpt.shared_payment_token.id); } export type Step = diff --git a/packages/cli/src/commands/mpp/request.test.ts b/packages/cli/src/commands/mpp/request.test.ts new file mode 100644 index 0000000..843c31d --- /dev/null +++ b/packages/cli/src/commands/mpp/request.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createMppRequest, probeMppRequest } from './request'; + +function response(status: number, location?: string): Response { + return new Response('response body', { + status, + headers: location ? { location } : undefined, + }); +} + +describe('probeMppRequest', () => { + it('preserves method and body across 307 while stripping cross-origin credentials', async () => { + const fetcher = vi + .fn() + .mockResolvedValueOnce(response(307, 'https://merchant.example/pay')) + .mockResolvedValueOnce(response(402)); + const request = createMppRequest( + 'https://redirector.example/start', + 'PUT', + 'payload', + { + Authorization: 'Bearer secret', + 'Content-Type': 'text/plain', + }, + ); + + const result = await probeMppRequest(request, fetcher); + + expect(result.method).toBe('PUT'); + expect(result.body).toBe('payload'); + expect(result.headers.get('authorization')).toBeNull(); + expect(result.headers.get('content-type')).toBe('text/plain'); + }); + + it('turns PUT into GET on a same-origin 303 and drops body headers', async () => { + const fetcher = vi + .fn() + .mockResolvedValueOnce(response(303, '/challenge')) + .mockResolvedValueOnce(response(402)); + const request = createMppRequest( + 'https://merchant.example/start', + 'PUT', + 'payload', + { + Authorization: 'Bearer caller-value', + 'Content-Type': 'text/plain', + }, + ); + + const result = await probeMppRequest(request, fetcher); + + expect(result.url).toBe('https://merchant.example/challenge'); + expect(result.method).toBe('GET'); + expect(result.body).toBeUndefined(); + expect(result.headers.get('content-type')).toBeNull(); + expect(result.headers.get('authorization')).toBe('Bearer caller-value'); + }); + + it('rejects remote HTTP and HTTPS downgrade redirects', async () => { + expect(() => + createMppRequest('http://merchant.example/pay', 'GET', undefined, {}), + ).toThrow(/require HTTPS/); + + const redirected = response(302, 'http://127.0.0.1:8080/pay'); + const fetcher = vi.fn().mockResolvedValue(redirected); + const request = createMppRequest( + 'https://merchant.example/start', + 'GET', + undefined, + {}, + ); + + await expect(probeMppRequest(request, fetcher)).rejects.toThrow( + /HTTPS downgrade/, + ); + expect(redirected.bodyUsed).toBe(true); + }); +}); diff --git a/packages/cli/src/commands/mpp/request.ts b/packages/cli/src/commands/mpp/request.ts new file mode 100644 index 0000000..08dbcbb --- /dev/null +++ b/packages/cli/src/commands/mpp/request.ts @@ -0,0 +1,136 @@ +const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); +const BODY_HEADERS = [ + 'content-encoding', + 'content-language', + 'content-length', + 'content-location', + 'content-type', + 'transfer-encoding', +]; +const CROSS_ORIGIN_HEADERS = [ + 'authorization', + 'cookie', + 'cookie2', + 'host', + 'proxy-authorization', +]; + +export interface MppRequest { + url: string; + method: string; + headers: Headers; + body: string | undefined; +} + +export interface MppProbe extends MppRequest { + response: Response; +} + +function isHttpLoopback(url: URL): boolean { + return ( + url.protocol === 'http:' && + (url.hostname === '127.0.0.1' || + url.hostname === 'localhost' || + url.hostname === '[::1]') + ); +} + +function assertSafeMppUrl(url: URL): void { + if (url.protocol === 'https:' || isHttpLoopback(url)) return; + throw new Error( + `MPP requests require HTTPS (HTTP is allowed only for localhost development): ${url.href}`, + ); +} + +export function createMppRequest( + url: string, + method: string, + body: string | undefined, + headers: HeadersInit, +): MppRequest { + const parsed = new URL(url); + assertSafeMppUrl(parsed); + return { + url: parsed.href, + method: method.toUpperCase(), + headers: new Headers(headers), + body, + }; +} + +export function isRedirectResponse(response: Response): boolean { + return response.status >= 300 && response.status < 400; +} + +export async function fetchMppRequest( + request: MppRequest, + fetcher: typeof fetch = fetch, +): Promise { + // Keep redirects visible so probing can follow them safely and approved + // payment flows can reject them. + return fetcher(request.url, { + method: request.method, + headers: request.headers, + body: request.body, + redirect: 'manual', + }); +} + +export async function probeMppRequest( + initial: MppRequest, + fetcher: typeof fetch = fetch, + maxRedirects = 10, +): Promise { + let request = initial; + + for (let redirectCount = 0; ; redirectCount++) { + const response = await fetchMppRequest(request, fetcher); + if (!REDIRECT_STATUSES.has(response.status)) { + return { ...request, response }; + } + + const location = response.headers.get('location'); + if (!location) return { ...request, response }; + if (redirectCount >= maxRedirects) { + await response.body?.cancel(); + throw new Error(`MPP request exceeded ${maxRedirects} redirects`); + } + + // Release this connection before validating or following the next hop. + await response.body?.cancel(); + const currentUrl = new URL(request.url); + const nextUrl = new URL(location, currentUrl); + assertSafeMppUrl(nextUrl); + if (currentUrl.protocol === 'https:' && nextUrl.protocol !== 'https:') { + throw new Error( + `MPP request refused HTTPS downgrade redirect to ${nextUrl.href}`, + ); + } + + const headers = new Headers(request.headers); + let method = request.method; + let body = request.body; + const switchesToGet = + ((response.status === 301 || response.status === 302) && + method === 'POST') || + (response.status === 303 && method !== 'GET' && method !== 'HEAD'); + if (switchesToGet) { + // Match Fetch redirect behavior: GET has no body or body-specific headers. + method = 'GET'; + body = undefined; + for (const header of BODY_HEADERS) headers.delete(header); + } + + if (currentUrl.origin !== nextUrl.origin) { + // Fetch does not forward credentials or a caller-supplied Host to another origin. + for (const header of CROSS_ORIGIN_HEADERS) headers.delete(header); + } + + request = { + url: nextUrl.href, + method, + headers, + body, + }; + } +}