diff --git a/CLAUDE.md b/CLAUDE.md index eb6d40c..2b01a5d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,14 @@ Key input field notes: |------|--------| | `--auth ` | Store auth credentials in a specific file instead of the default platform config location. `auth login` writes to this file; all other commands read from it. Parsed from `process.argv` and stripped before incur processes flags. | +## Security: Terminal Output Sanitization + +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`: + +- **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()`. + +JSON output mode (`--format json`) is **not** affected — `JSON.stringify` encodes escape sequences as Unicode literals. ## Environment Variables | Variable | Effect | diff --git a/packages/cli/package.json b/packages/cli/package.json index 5caee8a..73a0c35 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -32,10 +32,11 @@ "ink": "^5.2.1", "ink-spinner": "^5.0.0", "mppx": "0.6.16", - "viem": "^2.47.5", "qrcode": "^1.5.4", "react": "^18.3.1", + "strip-ansi": "^7.2.0", "update-notifier": "^7.3.1", + "viem": "^2.47.5", "zod": "^4.3.6" }, "devDependencies": { @@ -45,6 +46,7 @@ "@types/qrcode": "^1.5.5", "@types/react": "^18.3.28", "@types/update-notifier": "^6.0.8", + "ink-testing-library": "^4.0.0", "tsup": "^8.5.1", "tsx": "^4.21.0", "typescript": "^5.9.3", diff --git a/packages/cli/src/commands/payment-methods/__tests__/payment-methods.test.tsx b/packages/cli/src/commands/payment-methods/__tests__/payment-methods.test.tsx new file mode 100644 index 0000000..1cb3105 --- /dev/null +++ b/packages/cli/src/commands/payment-methods/__tests__/payment-methods.test.tsx @@ -0,0 +1,37 @@ +import type { IPaymentMethodsResource } from '@stripe/link-sdk'; +import { render } from 'ink-testing-library'; +import { describe, expect, it, vi } from 'vitest'; +import { sanitizeResource } from '../../../utils/resource-factory'; +import { PaymentMethodsList } from '../list'; + +const ESCAPE_PAYLOAD = '\x1b[2JEvil\rHidden'; +const CLEAN_TEXT = 'EvilHidden'; + +describe('payment-methods', () => { + describe('sanitization', () => { + it('sanitizes brand and nickname in payment method list', async () => { + const resource = sanitizeResource({ + listPaymentMethods: vi.fn(async () => [ + { + id: 'pm_1', + card_details: { brand: ESCAPE_PAYLOAD, last4: '4242' }, + bank_account_details: null, + nickname: ESCAPE_PAYLOAD, + is_default: false, + }, + ]), + } as unknown as IPaymentMethodsResource); + + const { lastFrame } = render( + {}} />, + ); + + await vi.waitFor(() => { + 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/payment-methods/list.tsx b/packages/cli/src/commands/payment-methods/list.tsx index 4994d76..bca989d 100644 --- a/packages/cli/src/commands/payment-methods/list.tsx +++ b/packages/cli/src/commands/payment-methods/list.tsx @@ -55,9 +55,7 @@ export const PaymentMethodsList: React.FC = ({ 'Bank account'; const last4 = pm.card_details?.last4 ?? pm.bank_account_details?.last4; - const suffix = [pm.nickname ? `(${pm.nickname})` : ''] - .filter(Boolean) - .join(' '); + const suffix = pm.nickname ? `(${pm.nickname})` : ''; return ( diff --git a/packages/cli/src/commands/spend-request/__tests__/spend-request.test.tsx b/packages/cli/src/commands/spend-request/__tests__/spend-request.test.tsx new file mode 100644 index 0000000..c8a0c9e --- /dev/null +++ b/packages/cli/src/commands/spend-request/__tests__/spend-request.test.tsx @@ -0,0 +1,135 @@ +import type { ISpendRequestResource, SpendRequest } from '@stripe/link-sdk'; +import { render } from 'ink-testing-library'; +import { describe, expect, it, vi } from 'vitest'; +import { sanitizeResource } from '../../../utils/resource-factory'; +import { CreateSpendRequest } from '../create'; +import { RetrieveSpendRequest } from '../retrieve'; +import { UpdateSpendRequest } from '../update'; + +const ESCAPE_PAYLOAD = '\x1b[2JEvil\rHidden'; +const CLEAN_TEXT = 'EvilHidden'; + +function makeSpendRequest(overrides: Partial = {}): SpendRequest { + return { + id: 'sr_test', + status: 'approved', + amount: 1000, + currency: 'usd', + merchant_name: ESCAPE_PAYLOAD, + merchant_url: 'https://example.com', + context: 'x'.repeat(100), + credential_type: 'card', + payment_details: 'pm_1', + line_items: [{ name: ESCAPE_PAYLOAD }], + totals: [{ type: 'total', amount: 1000, display_text: 'Total' }], + created_at: '2025-01-01T00:00:00Z', + updated_at: '2025-01-01T00:00:00Z', + approval_url: '', + card: undefined, + shared_payment_token: undefined, + ...overrides, + } as SpendRequest; +} + +function makeMockRepo(result: SpendRequest) { + return sanitizeResource({ + createSpendRequest: vi.fn(async () => result), + getSpendRequest: vi.fn(async () => result), + updateSpendRequest: vi.fn(async () => result), + requestApproval: vi.fn(async () => result), + cancelSpendRequest: vi.fn(async () => result), + } as unknown as ISpendRequestResource); +} + +describe('spend-request', () => { + describe('sanitization', () => { + it('CreateSpendRequest sanitizes merchant_name and line_items', async () => { + const request = makeSpendRequest(); + const repo = makeMockRepo(request); + + const { lastFrame } = render( + {}} + />, + ); + + await vi.waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Merchant'); + expect(frame).toContain(CLEAN_TEXT); + expect(frame).not.toContain('\x1b[2J'); + expect(frame).not.toContain('\r'); + }); + }); + + it('UpdateSpendRequest sanitizes merchant_name and line_items', async () => { + const request = makeSpendRequest(); + const repo = makeMockRepo(request); + + const { lastFrame } = render( + {}} + />, + ); + + await vi.waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Merchant'); + expect(frame).toContain(CLEAN_TEXT); + expect(frame).not.toContain('\x1b[2J'); + expect(frame).not.toContain('\r'); + }); + }); + + it('RetrieveSpendRequest sanitizes merchant_name, line_items, and billing_address', async () => { + const request = makeSpendRequest({ + card: { + id: 'card_1', + number: '4242424242424242', + brand: 'visa', + exp_month: 12, + exp_year: 2030, + cvc: '123', + valid_until: '2025-12-31', + billing_address: { + name: ESCAPE_PAYLOAD, + line1: ESCAPE_PAYLOAD, + city: 'Test City', + state: 'TS', + postal_code: '12345', + country: 'US', + }, + }, + }); + const repo = makeMockRepo(request); + + const { lastFrame } = render( + {}} + />, + ); + + await vi.waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Billing Address'); + expect(frame).toContain(CLEAN_TEXT); + expect(frame).not.toContain('\x1b[2J'); + expect(frame).not.toContain('\r'); + }); + }); + }); +}); diff --git a/packages/cli/src/utils/__tests__/sanitize-text.test.ts b/packages/cli/src/utils/__tests__/sanitize-text.test.ts new file mode 100644 index 0000000..84b1e93 --- /dev/null +++ b/packages/cli/src/utils/__tests__/sanitize-text.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from 'vitest'; +import { sanitizeDeep, sanitizeText } from '../sanitize-text'; + +describe('sanitizeText', () => { + it('returns empty string for null/undefined', () => { + expect(sanitizeText(null)).toBe(''); + expect(sanitizeText(undefined)).toBe(''); + expect(sanitizeText('')).toBe(''); + }); + + it('passes through normal text unchanged', () => { + expect(sanitizeText('Hello World')).toBe('Hello World'); + expect(sanitizeText('Café & Résumé')).toBe('Café & Résumé'); + }); + + it('strips CSI sequences (colors, cursor movement)', () => { + expect(sanitizeText('\x1b[2JScreen Cleared')).toBe('Screen Cleared'); + expect(sanitizeText('\x1b[1A\x1b[2KOverwrite')).toBe('Overwrite'); + expect(sanitizeText('\x1b[31mRed Text\x1b[0m')).toBe('Red Text'); + }); + + it('strips OSC sequences (window title, hyperlinks)', () => { + expect(sanitizeText('\x1b]0;PWNED\x07Normal Store')).toBe('Normal Store'); + expect(sanitizeText('\x1b]8;;https://evil.com\x07Click\x1b]8;;\x07')).toBe( + 'Click', + ); + }); + + it('strips carriage returns', () => { + expect(sanitizeText('Legit Store\rEvil Store')).toBe( + 'Legit StoreEvil Store', + ); + }); + + it('strips other control characters', () => { + expect(sanitizeText('Hello\x00World')).toBe('HelloWorld'); + expect(sanitizeText('Tab\x09is fine but \x01 is not')).toBe( + 'Tab\tis fine but is not', + ); + }); + + it('preserves newlines and tabs', () => { + expect(sanitizeText('Line1\nLine2')).toBe('Line1\nLine2'); + expect(sanitizeText('Col1\tCol2')).toBe('Col1\tCol2'); + }); + + it('handles compound attack payloads', () => { + const payload = '\x1b[2J\x1b[1;1H\x1b]0;Hijacked\x07\rAmount: $0.01'; + expect(sanitizeText(payload)).toBe('Amount: $0.01'); + }); +}); + +describe('sanitizeDeep', () => { + it('sanitizes strings in nested objects', () => { + const input = { + merchant_name: '\x1b[2JEvil', + amount: 1000, + line_items: [{ name: '\rHidden' }], + card: { billing_address: { name: '\x1b]0;X\x07Test' } }, + }; + const result = sanitizeDeep(input); + expect(result.merchant_name).toBe('Evil'); + expect(result.amount).toBe(1000); + expect(result.line_items[0].name).toBe('Hidden'); + expect(result.card.billing_address.name).toBe('Test'); + }); + + it('passes through null, undefined, and primitives', () => { + expect(sanitizeDeep(null)).toBe(null); + expect(sanitizeDeep(undefined)).toBe(undefined); + expect(sanitizeDeep(42)).toBe(42); + expect(sanitizeDeep(true)).toBe(true); + }); + + it('sanitizes arrays of strings', () => { + const input = ['\x1b[2JA', 'B', '\rC']; + expect(sanitizeDeep(input)).toEqual(['A', 'B', 'C']); + }); +}); diff --git a/packages/cli/src/utils/resource-factory.ts b/packages/cli/src/utils/resource-factory.ts index d494263..e77f165 100644 --- a/packages/cli/src/utils/resource-factory.ts +++ b/packages/cli/src/utils/resource-factory.ts @@ -12,6 +12,44 @@ import { import { LinkAuthResource } from '../auth/auth-resource'; import { createAccessTokenProvider } from '../auth/session'; import type { IAuthResource } from '../auth/types'; +import { sanitizeDeep } from './sanitize-text'; + +/** + * Wraps an SDK resource with a Proxy that strips ANSI escape sequences and + * control characters from all string values in async method return values. + * + * This is the single sanitization boundary for all server data entering the CLI. + * It protects against terminal escape injection regardless of output format + * (interactive Ink rendering, toon, yaml, md, JSON) because sanitization happens + * before data reaches either the React components or the incur formatter. + * + * Non-function properties and synchronous return values pass through unchanged. + * Only Promise-returning methods (i.e. all SDK API calls) have their resolved + * values recursively sanitized via sanitizeDeep. + */ +export function sanitizeResource(resource: T): T { + return new Proxy(resource, { + get(target, prop, receiver) { + const value = Reflect.get(target, prop, receiver); + + // Non-function properties (e.g. config fields) pass through as-is. + if (typeof value !== 'function') { + return value; + } + + // Wrap each method call. If the method returns a Promise (all SDK API + // methods do), pipe its resolved value through sanitizeDeep to strip + // escape sequences from every string field in the response object. + return (...args: unknown[]) => { + const result = value.apply(target, args); + if (result && typeof result.then === 'function') { + return result.then(sanitizeDeep); + } + return result; + }; + }, + }); +} interface ResourceFactoryOptions { verbose?: boolean; @@ -71,11 +109,13 @@ export class ResourceFactory { } const getAccessToken = this.createSdkAccessTokenProvider(); - this.spendRequestResource = new SpendRequestResource({ - verbose: this.verbose, - defaultHeaders: this.defaultHeaders, - getAccessToken, - }); + this.spendRequestResource = sanitizeResource( + new SpendRequestResource({ + verbose: this.verbose, + defaultHeaders: this.defaultHeaders, + getAccessToken, + }), + ); return this.spendRequestResource; } @@ -86,11 +126,13 @@ export class ResourceFactory { } const getAccessToken = this.createSdkAccessTokenProvider(); - this.paymentMethodsResource = new PaymentMethodsResource({ - verbose: this.verbose, - defaultHeaders: this.defaultHeaders, - getAccessToken, - }); + this.paymentMethodsResource = sanitizeResource( + new PaymentMethodsResource({ + verbose: this.verbose, + defaultHeaders: this.defaultHeaders, + getAccessToken, + }), + ); return this.paymentMethodsResource; } @@ -101,11 +143,13 @@ export class ResourceFactory { } const getAccessToken = this.createSdkAccessTokenProvider(); - this.shippingAddressResource = new ShippingAddressResource({ - verbose: this.verbose, - defaultHeaders: this.defaultHeaders, - getAccessToken, - }); + this.shippingAddressResource = sanitizeResource( + new ShippingAddressResource({ + verbose: this.verbose, + defaultHeaders: this.defaultHeaders, + getAccessToken, + }), + ); return this.shippingAddressResource; } @@ -116,11 +160,13 @@ export class ResourceFactory { } const getAccessToken = this.createSdkAccessTokenProvider(); - this.userInfoResource = new UserInfoResource({ - verbose: this.verbose, - defaultHeaders: this.defaultHeaders, - getAccessToken, - }); + this.userInfoResource = sanitizeResource( + new UserInfoResource({ + verbose: this.verbose, + defaultHeaders: this.defaultHeaders, + getAccessToken, + }), + ); return this.userInfoResource; } diff --git a/packages/cli/src/utils/sanitize-text.ts b/packages/cli/src/utils/sanitize-text.ts new file mode 100644 index 0000000..eb0af9c --- /dev/null +++ b/packages/cli/src/utils/sanitize-text.ts @@ -0,0 +1,47 @@ +import stripAnsi from 'strip-ansi'; + +// biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally matching control characters for sanitization +const CONTROL_CHAR_RE = /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\r]/g; + +// biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally matching control characters for detection +const NEEDS_SANITIZE_RE = /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\r\x1b]/; + +export function sanitizeText(input: string | undefined | null): string { + if (!input) { + return ''; + } + + if (!NEEDS_SANITIZE_RE.test(input)) { + return input; + } + + return stripAnsi(input).replace(CONTROL_CHAR_RE, ''); +} + +export function sanitizeDeep(value: T): T { + if (value === null || value === undefined) { + return value; + } + + if (typeof value === 'string') { + return sanitizeText(value) as T; + } + + if (Array.isArray(value)) { + return value.map(sanitizeDeep) as T; + } + + if (typeof value === 'object') { + const result: Record = {}; + const keys = Object.keys(value); + + for (let i = 0; i < keys.length; i++) { + const k = keys[i]; + result[k] = sanitizeDeep((value as Record)[k]); + } + + return result as T; + } + + return value; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5555050..dedb338 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -41,6 +41,9 @@ importers: react: specifier: ^18.3.1 version: 18.3.1 + strip-ansi: + specifier: ^7.2.0 + version: 7.2.0 update-notifier: specifier: ^7.3.1 version: 7.3.1 @@ -69,6 +72,9 @@ importers: '@types/update-notifier': specifier: ^6.0.8 version: 6.0.8 + ink-testing-library: + specifier: ^4.0.0 + version: 4.0.0(@types/react@18.3.28) tsup: specifier: ^8.5.1 version: 8.5.1(postcss@8.5.14)(tsx@4.21.0)(typescript@5.9.3)(yaml@2.8.3) @@ -1392,6 +1398,15 @@ packages: ink: '>=4.0.0' react: '>=18.0.0' + ink-testing-library@4.0.0: + resolution: {integrity: sha512-yF92kj3pmBvk7oKbSq5vEALO//o7Z9Ck/OaLNlkzXNeYdwfpxMQkSowGTFUCS5MSu9bWfSZMewGpp7bFc66D7Q==} + engines: {node: '>=18'} + peerDependencies: + '@types/react': '>=18.0.0' + peerDependenciesMeta: + '@types/react': + optional: true + ink@5.2.1: resolution: {integrity: sha512-BqcUyWrG9zq5HIwW6JcfFHsIYebJkWWb4fczNah1goUO0vv5vneIlfwuS85twyJ5hYR/y18FlAYUxrO9ChIWVg==} engines: {node: '>=18'} @@ -3627,6 +3642,10 @@ snapshots: ink: 5.2.1(@types/react@18.3.28)(react@18.3.1) react: 18.3.1 + ink-testing-library@4.0.0(@types/react@18.3.28): + optionalDependencies: + '@types/react': 18.3.28 + ink@5.2.1(@types/react@18.3.28)(react@18.3.1): dependencies: '@alcalzone/ansi-tokenize': 0.1.3