From 3197fb3419fe50802466dbf49c0d43fbdcd51afd Mon Sep 17 00:00:00 2001 From: Kyle Reese Date: Tue, 11 Aug 2026 13:21:09 -0400 Subject: [PATCH 1/3] security improvements for mpp commands --- CLAUDE.md | 2 + .../cli/src/commands/mpp/decode-view.test.tsx | 42 +++++++++++++++++++ packages/cli/src/commands/mpp/decode.test.ts | 37 ++++++++++++++++ packages/cli/src/commands/mpp/decode.ts | 13 +++++- 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/commands/mpp/decode-view.test.tsx diff --git a/CLAUDE.md b/CLAUDE.md index 60563f6..7152641 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,8 +127,10 @@ Key input field notes: Server-returned strings can contain ANSI escape sequences or control characters that spoof the terminal approval UI. Sanitization is handled automatically via `sanitizeDeep()` from `packages/cli/src/utils/sanitize-text.ts`: +- **SDK-resource data** — sanitized automatically at the `sanitizeResource()` proxy boundary in `packages/cli/src/utils/resource-factory.ts`. All server data flowing through SDK resources (spend-request, payment-methods, sources, etc.) is `sanitizeDeep()`'d before reaching components or the incur formatter, in every output format. - **Commands using `useAsyncAction` hook** — sanitized automatically. The hook calls `sanitizeDeep()` on all returned data before it reaches components. - **Commands with manual state management** (e.g. `create.tsx`, `retrieve.tsx`, `request-approval.tsx`, `mpp/pay.tsx`) — must call `sanitizeDeep()` on API responses before calling `setRequest()`/`setState()`. +- **Attacker-controlled data that does NOT flow through an SDK resource** — must be sanitized at its own parse boundary. `mpp pay` sanitizes the HTTP response in `readPayResult()` (`pay.tsx`); `mpp decode` sanitizes the parsed `WWW-Authenticate` challenge in `decodeStripeChallenge()` (`decode.ts`). These bypass the resource factory, so the return value of the parse/fetch helper is the chokepoint — sanitizing there covers both the interactive Ink render and the agent (toon/yaml/md) output at once. JSON output mode (`--format json`) is **not** affected — `JSON.stringify` encodes escape sequences as Unicode literals. ## Environment Variables diff --git a/packages/cli/src/commands/mpp/decode-view.test.tsx b/packages/cli/src/commands/mpp/decode-view.test.tsx new file mode 100644 index 0000000..b39711e --- /dev/null +++ b/packages/cli/src/commands/mpp/decode-view.test.tsx @@ -0,0 +1,42 @@ +import { render } from 'ink-testing-library'; +import { describe, expect, it } from 'vitest'; +import { decodeStripeChallenge } from './decode'; +import { DecodeChallengeView } from './decode-view'; + +const ESCAPE_PAYLOAD = '\x1b[2JEvil\rHidden'; +const CLEAN_TEXT = 'EvilHidden'; + +function encodeRequest(request: Record): string { + return Buffer.from(JSON.stringify(request)).toString('base64'); +} + +describe('DecodeChallengeView', () => { + it('renders no raw ANSI escapes for an attacker-controlled challenge', () => { + // The challenge string is fully attacker-controlled. Sanitization happens + // at the decode.ts boundary, so render the real decoded output rather than + // a hand-built object. + const header = [ + `Payment id="${ESCAPE_PAYLOAD}",`, + `realm="${ESCAPE_PAYLOAD}",`, + 'method="stripe",', + 'intent="charge",', + `request="${encodeRequest({ + amount: '1000', + currency: 'usd', + merchantName: ESCAPE_PAYLOAD, + methodDetails: { + networkId: 'net_001', + paymentMethodTypes: ['card'], + }, + })}"`, + ].join(' '); + + const decoded = decodeStripeChallenge(header); + const { lastFrame } = render(); + + const frame = lastFrame() ?? ''; + expect(frame).toContain(CLEAN_TEXT); + expect(frame).not.toContain('\x1b[2J'); + expect(frame).not.toContain('\r'); + }); +}); diff --git a/packages/cli/src/commands/mpp/decode.test.ts b/packages/cli/src/commands/mpp/decode.test.ts index b9af8a3..ca5a102 100644 --- a/packages/cli/src/commands/mpp/decode.test.ts +++ b/packages/cli/src/commands/mpp/decode.test.ts @@ -126,4 +126,41 @@ describe('decodeStripeChallenge', () => { }, }); }); + + it('strips ANSI escape and control characters from decoded fields', () => { + const payload = '\x1b[2JEvil\rHidden'; + const clean = 'EvilHidden'; + const header = [ + `Payment id="${payload}",`, + `realm="${payload}",`, + 'method="stripe",', + 'intent="charge",', + `description="${payload}",`, + `request="${encodeRequest({ + amount: '1000', + currency: 'usd', + merchantName: payload, + methodDetails: { + networkId: 'net_001', + paymentMethodTypes: ['card'], + }, + })}"`, + ].join(' '); + + const decoded = decodeStripeChallenge(header); + expect(decoded).toMatchObject({ + id: clean, + realm: clean, + description: clean, + network_id: 'net_001', + request_json: { + amount: '1000', + currency: 'usd', + merchantName: clean, + }, + }); + const serialized = JSON.stringify(decoded); + expect(serialized).not.toContain('\x1b[2J'); + expect(serialized).not.toContain('\r'); + }); }); diff --git a/packages/cli/src/commands/mpp/decode.ts b/packages/cli/src/commands/mpp/decode.ts index 4e98db0..2afaa4d 100644 --- a/packages/cli/src/commands/mpp/decode.ts +++ b/packages/cli/src/commands/mpp/decode.ts @@ -1,4 +1,5 @@ import { Challenge } from 'mppx'; +import { sanitizeDeep } from '../../utils/sanitize-text'; type StripeChargeChallenge = Challenge.Challenge< Record, @@ -118,7 +119,15 @@ export function decodeStripeChallenge( Challenge.deserializeList(challengeHeader), ); - return { + // The challenge string is fully attacker-controlled (it is the raw + // WWW-Authenticate header value). Strip ANSI escape sequences and control + // characters so the decoded fields cannot spoof the terminal UI when rendered + // (interactive Ink render or agent non-JSON output). This is the sanitization + // boundary for decode data, which does not pass through the SDK resource + // factory. See CLAUDE.md security note. Stripping only affects strings with + // control bytes, so numeric/logic consumers (amount, currency, network_id) + // are unaffected for normal values. + return sanitizeDeep({ id: challenge.id, realm: challenge.realm, method: 'stripe', @@ -128,5 +137,5 @@ export function decodeStripeChallenge( expires: challenge.expires, network_id: networkId, request_json: request, - }; + }); } From 65c3dfc572f49b96b007861ec2950e5ec377cf99 Mon Sep 17 00:00:00 2001 From: Kyle Reese Date: Tue, 11 Aug 2026 17:15:36 -0400 Subject: [PATCH 2/3] rm comment --- packages/cli/src/commands/mpp/decode.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/cli/src/commands/mpp/decode.ts b/packages/cli/src/commands/mpp/decode.ts index 2afaa4d..55d4c21 100644 --- a/packages/cli/src/commands/mpp/decode.ts +++ b/packages/cli/src/commands/mpp/decode.ts @@ -119,14 +119,6 @@ export function decodeStripeChallenge( Challenge.deserializeList(challengeHeader), ); - // The challenge string is fully attacker-controlled (it is the raw - // WWW-Authenticate header value). Strip ANSI escape sequences and control - // characters so the decoded fields cannot spoof the terminal UI when rendered - // (interactive Ink render or agent non-JSON output). This is the sanitization - // boundary for decode data, which does not pass through the SDK resource - // factory. See CLAUDE.md security note. Stripping only affects strings with - // control bytes, so numeric/logic consumers (amount, currency, network_id) - // are unaffected for normal values. return sanitizeDeep({ id: challenge.id, realm: challenge.realm, From b4a9956169ea015d3db64de2062b703879d71ead Mon Sep 17 00:00:00 2001 From: Kyle Reese Date: Tue, 11 Aug 2026 17:21:32 -0400 Subject: [PATCH 3/3] Add changeset --- .changeset/true-words-warn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/true-words-warn.md diff --git a/.changeset/true-words-warn.md b/.changeset/true-words-warn.md new file mode 100644 index 0000000..b4c8b7c --- /dev/null +++ b/.changeset/true-words-warn.md @@ -0,0 +1,5 @@ +--- +"@stripe/link-cli": patch +--- + +Sanitize output of decode call during mpp flow