From 775900d0bf3fe464f8f8407d03d0f5c4b737aac0 Mon Sep 17 00:00:00 2001 From: Ben Sandler Date: Thu, 10 Sep 2026 21:51:37 -0400 Subject: [PATCH 1/5] Bind MPP credentials to the challenged destination - Follow unauthenticated redirects explicitly and bind the authenticated retry to the exact request that returned the 402 challenge, preventing a redirecting origin from receiving and replaying another server's Payment credential. - Preserve Fetch redirect method, body, and header rewrites while stripping sensitive headers across origins, and reject every redirect after credentials are attached. - Pin interactive challenge refreshes and agent continuations to the effective destination, and require HTTPS for remote MPP endpoints while retaining exact loopback HTTP for local development. - Cover five focused redirect/payment unit cases and 17 targeted built-CLI integration cases, including two new cross-origin credential-leak regressions. Committed-By-Agent: codex Co-authored-by: codex --- .changeset/safe-mpp-redirects.md | 8 ++ packages/cli/src/__tests__/cli.test.ts | 98 ++++++++++++- packages/cli/src/commands/mpp/index.tsx | 33 +++-- packages/cli/src/commands/mpp/pay.test.ts | 114 +++++++++++++++ packages/cli/src/commands/mpp/pay.tsx | 93 ++++++++---- packages/cli/src/commands/mpp/request.test.ts | 78 ++++++++++ packages/cli/src/commands/mpp/request.ts | 136 ++++++++++++++++++ 7 files changed, 511 insertions(+), 49 deletions(-) create mode 100644 .changeset/safe-mpp-redirects.md create mode 100644 packages/cli/src/commands/mpp/pay.test.ts create mode 100644 packages/cli/src/commands/mpp/request.test.ts create mode 100644 packages/cli/src/commands/mpp/request.ts diff --git a/.changeset/safe-mpp-redirects.md b/.changeset/safe-mpp-redirects.md new file mode 100644 index 0000000..c172229 --- /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. +Paid requests now reject redirects instead of forwarding credentials, and 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..4e5c510 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)); }); @@ -2607,6 +2616,45 @@ describe('production mode', () => { expect(merchantRequests[1].headers.authorization).toMatch(/^Payment /); }); + it('sends the credential only to a cross-origin challenge destination', async () => { + setNextResponse(200, APPROVED_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, + }); + setMerchantResponse(200, '{"success":true}'); + + const result = await runProdCli( + 'mpp', + 'pay', + `http://127.0.0.1:${serverPort}/merchant-redirect`, + '--spend-request-id', + 'lsrq_spt_001', + '--format', + 'json', + ); + + expect(result.exitCode).toBe(0); + const redirectorRequests = requests.filter( + (request) => request.url === '/merchant-redirect', + ); + expect(redirectorRequests).toHaveLength(1); + expect( + redirectorRequests.every( + (request) => !request.headers.authorization?.startsWith('Payment '), + ), + ).toBe(true); + expect(merchantRequests).toHaveLength(2); + expect(merchantRequests[0].headers.authorization).toBeUndefined(); + expect( + merchantRequests.filter((request) => + request.headers.authorization?.startsWith('Payment '), + ), + ).toHaveLength(1); + }); + it('returns structured response when the paid retry fails', async () => { setNextResponse(200, APPROVED_SPT_REQUEST); setMerchantResponse(402, '{"error":"payment required"}', { @@ -2894,17 +2942,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..2090fa4 --- /dev/null +++ b/packages/cli/src/commands/mpp/pay.test.ts @@ -0,0 +1,114 @@ +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('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..df36c28 100644 --- a/packages/cli/src/commands/mpp/pay.tsx +++ b/packages/cli/src/commands/mpp/pay.tsx @@ -14,6 +14,13 @@ import { decodeStripeChallenge, getStripeChargeChallengeFromResponse, } from './decode'; +import { + type MppProbe, + createMppRequest, + fetchMppRequest, + isRedirectResponse, + probeMppRequest, +} from './request'; export type PayResult = { status: number; @@ -162,30 +169,60 @@ export async function payWithSpt( ): Promise { const httpMethod = method ?? (data !== undefined ? 'POST' : 'GET'); const requestHeaders = buildHeaders(data, headers); + const probe = await probeMppRequest( + createMppRequest(url, httpMethod, data, requestHeaders), + ); + + if (probe.response.status !== 402) return readPayResult(probe.response); + return submitMppPayment(probe, spt); +} - const initialResponse = await fetch(url, { - method: httpMethod, - body: data, - headers: requestHeaders, +async function submitMppPayment( + challenge: MppProbe, + spt: string, +): Promise { + // MPPx clones its Response even though this transport reads only headers. + // A bodyless copy lets us cancel the real response without leaving a tee open. + 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 refreshAndPayWithSpt( + request: MppProbe, + spt: string, +): Promise { + // Approval can take minutes. Refresh the challenge at the pinned destination, + // but do not let that destination move after the user has approved. + 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 +248,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 +264,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 +335,7 @@ export async function runMppPayFullFlow( // 7. Pay onStep?.('submitting'); - return payWithSpt( - url, - withSpt.shared_payment_token.id, - method, - data, - headers, - ); + return refreshAndPayWithSpt(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..93e1107 --- /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 { + // Redirects are handled by probeMppRequest so a later credential can be + // sent to the exact request that returned the challenge. + 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, + }; + } +} From 3bba1f999ed3732a1811c7b395d6367547181699 Mon Sep 17 00:00:00 2001 From: Ben Sandler Date: Thu, 10 Sep 2026 22:27:13 -0400 Subject: [PATCH 2/5] Prevent approved MPP continuations from changing destination - Deferred agent continuations re-probed the pinned challenge URL while following redirects, allowing that destination to move after approval. - Route every approved spend request through one no-redirect challenge refresh and reject redirects before attaching the payment credential. - Add focused regression coverage for the pinned refresh and remove the obsolete expectation that approved requests may follow redirects. Committed-By-Agent: codex Co-authored-by: codex --- .changeset/safe-mpp-redirects.md | 6 ++-- packages/cli/src/__tests__/cli.test.ts | 39 ----------------------- packages/cli/src/commands/mpp/pay.test.ts | 28 ++++++++++++++++ packages/cli/src/commands/mpp/pay.tsx | 13 ++++---- 4 files changed, 37 insertions(+), 49 deletions(-) diff --git a/.changeset/safe-mpp-redirects.md b/.changeset/safe-mpp-redirects.md index c172229..dc551df 100644 --- a/.changeset/safe-mpp-redirects.md +++ b/.changeset/safe-mpp-redirects.md @@ -3,6 +3,6 @@ --- Send MPP payment credentials only to the URL that returned the payment challenge. -Paid requests now reject redirects instead of forwarding credentials, and remote -MPP endpoints must use HTTPS. HTTP remains supported for exact loopback addresses -used in local development. +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 4e5c510..38e8f69 100644 --- a/packages/cli/src/__tests__/cli.test.ts +++ b/packages/cli/src/__tests__/cli.test.ts @@ -2616,45 +2616,6 @@ describe('production mode', () => { expect(merchantRequests[1].headers.authorization).toMatch(/^Payment /); }); - it('sends the credential only to a cross-origin challenge destination', async () => { - setNextResponse(200, APPROVED_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, - }); - setMerchantResponse(200, '{"success":true}'); - - const result = await runProdCli( - 'mpp', - 'pay', - `http://127.0.0.1:${serverPort}/merchant-redirect`, - '--spend-request-id', - 'lsrq_spt_001', - '--format', - 'json', - ); - - expect(result.exitCode).toBe(0); - const redirectorRequests = requests.filter( - (request) => request.url === '/merchant-redirect', - ); - expect(redirectorRequests).toHaveLength(1); - expect( - redirectorRequests.every( - (request) => !request.headers.authorization?.startsWith('Payment '), - ), - ).toBe(true); - expect(merchantRequests).toHaveLength(2); - expect(merchantRequests[0].headers.authorization).toBeUndefined(); - expect( - merchantRequests.filter((request) => - request.headers.authorization?.startsWith('Payment '), - ), - ).toHaveLength(1); - }); - it('returns structured response when the paid retry fails', async () => { setNextResponse(200, APPROVED_SPT_REQUEST); setMerchantResponse(402, '{"error":"payment required"}', { diff --git a/packages/cli/src/commands/mpp/pay.test.ts b/packages/cli/src/commands/mpp/pay.test.ts index 2090fa4..04249f7 100644 --- a/packages/cli/src/commands/mpp/pay.test.ts +++ b/packages/cli/src/commands/mpp/pay.test.ts @@ -27,6 +27,34 @@ afterEach(() => { }); 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() diff --git a/packages/cli/src/commands/mpp/pay.tsx b/packages/cli/src/commands/mpp/pay.tsx index df36c28..11e3673 100644 --- a/packages/cli/src/commands/mpp/pay.tsx +++ b/packages/cli/src/commands/mpp/pay.tsx @@ -16,6 +16,7 @@ import { } from './decode'; import { type MppProbe, + type MppRequest, createMppRequest, fetchMppRequest, isRedirectResponse, @@ -169,12 +170,10 @@ export async function payWithSpt( ): Promise { const httpMethod = method ?? (data !== undefined ? 'POST' : 'GET'); const requestHeaders = buildHeaders(data, headers); - const probe = await probeMppRequest( + return refreshAndPayWithSpt( createMppRequest(url, httpMethod, data, requestHeaders), + spt, ); - - if (probe.response.status !== 402) return readPayResult(probe.response); - return submitMppPayment(probe, spt); } async function submitMppPayment( @@ -208,11 +207,11 @@ async function submitMppPayment( } async function refreshAndPayWithSpt( - request: MppProbe, + request: MppRequest, spt: string, ): Promise { - // Approval can take minutes. Refresh the challenge at the pinned destination, - // but do not let that destination move after the user has approved. + // 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(); From 41b8fa4a1875bae52d4e1732355384f1bc4a791e Mon Sep 17 00:00:00 2001 From: Ben Sandler Date: Thu, 10 Sep 2026 22:40:53 -0400 Subject: [PATCH 3/5] Format approved redirect assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Collapse the approved redirect assertion to Biome’s single-line form. - Leave behavior unchanged; CI build and typechecks passed. Committed-By-Agent: codex Co-authored-by: codex --- packages/cli/src/commands/mpp/pay.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli/src/commands/mpp/pay.test.ts b/packages/cli/src/commands/mpp/pay.test.ts index 04249f7..4052b4d 100644 --- a/packages/cli/src/commands/mpp/pay.test.ts +++ b/packages/cli/src/commands/mpp/pay.test.ts @@ -47,9 +47,7 @@ describe('payWithSpt', () => { ).rejects.toThrow(/redirected with status 307 after approval/); expect(fetcher).toHaveBeenCalledTimes(1); - expect(fetcher.mock.calls[0][0]).toBe( - 'https://merchant.example/challenge', - ); + expect(fetcher.mock.calls[0][0]).toBe('https://merchant.example/challenge'); expect( new Headers(fetcher.mock.calls[0][1]?.headers).has('authorization'), ).toBe(false); From e523dd81c9b868cee26383bf9c48a4b91d63e61d Mon Sep 17 00:00:00 2001 From: Ben Sandler Date: Fri, 11 Sep 2026 08:42:13 -0400 Subject: [PATCH 4/5] Clarify pinned MPP payment flow - Name the post-approval helper around its pinned-destination guarantee. - Explain why redirects stay visible to probe and payment callers. Committed-By-Agent: codex Co-authored-by: codex --- packages/cli/src/commands/mpp/pay.tsx | 6 +++--- packages/cli/src/commands/mpp/request.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/commands/mpp/pay.tsx b/packages/cli/src/commands/mpp/pay.tsx index 11e3673..a03b68c 100644 --- a/packages/cli/src/commands/mpp/pay.tsx +++ b/packages/cli/src/commands/mpp/pay.tsx @@ -170,7 +170,7 @@ export async function payWithSpt( ): Promise { const httpMethod = method ?? (data !== undefined ? 'POST' : 'GET'); const requestHeaders = buildHeaders(data, headers); - return refreshAndPayWithSpt( + return payPinnedChallengeWithSpt( createMppRequest(url, httpMethod, data, requestHeaders), spt, ); @@ -206,7 +206,7 @@ async function submitMppPayment( return readPayResult(response); } -async function refreshAndPayWithSpt( +async function payPinnedChallengeWithSpt( request: MppRequest, spt: string, ): Promise { @@ -334,7 +334,7 @@ export async function runMppPayFullFlow( // 7. Pay onStep?.('submitting'); - return refreshAndPayWithSpt(probe, withSpt.shared_payment_token.id); + return payPinnedChallengeWithSpt(probe, withSpt.shared_payment_token.id); } export type Step = diff --git a/packages/cli/src/commands/mpp/request.ts b/packages/cli/src/commands/mpp/request.ts index 93e1107..08dbcbb 100644 --- a/packages/cli/src/commands/mpp/request.ts +++ b/packages/cli/src/commands/mpp/request.ts @@ -66,8 +66,8 @@ export async function fetchMppRequest( request: MppRequest, fetcher: typeof fetch = fetch, ): Promise { - // Redirects are handled by probeMppRequest so a later credential can be - // sent to the exact request that returned the challenge. + // 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, From ee93269d4f8d736d44df3a8b3912702827ba090a Mon Sep 17 00:00:00 2001 From: Ben Sandler Date: Fri, 11 Sep 2026 08:59:01 -0400 Subject: [PATCH 5/5] Describe MPP challenge handling generically - Explain the status-and-header signing boundary without relying on mppx internals. - Keep response-body isolation and cleanup behavior unchanged. Committed-By-Agent: codex Co-authored-by: codex --- packages/cli/src/commands/mpp/pay.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/mpp/pay.tsx b/packages/cli/src/commands/mpp/pay.tsx index a03b68c..788f4f8 100644 --- a/packages/cli/src/commands/mpp/pay.tsx +++ b/packages/cli/src/commands/mpp/pay.tsx @@ -180,8 +180,8 @@ async function submitMppPayment( challenge: MppProbe, spt: string, ): Promise { - // MPPx clones its Response even though this transport reads only headers. - // A bodyless copy lets us cancel the real response without leaving a tee open. + // 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,