diff --git a/docs/payments.md b/docs/payments.md index 3f7e844c1..9506396a9 100644 --- a/docs/payments.md +++ b/docs/payments.md @@ -126,6 +126,26 @@ For details on IAM role separation (ManagementRole vs ProcessPaymentRole), see A payment connector links a credential provider (wallet credentials) to a payment manager. Each manager needs at least one connector before it can process payments. +### Supplying secrets securely (`--credentials-file`) + +Passing secrets as literal flags (`--api-key-secret`, `--wallet-secret`, `--app-secret`, `--authorization-private-key`) +exposes them to your shell history and the process table. For non-interactive use, put the provider secrets in a JSON +file and pass `--credentials-file` instead (or pipe the JSON via `--credentials-file -`). The CLI prints a warning if +you use the literal secret flags. + +```bash +# CoinbaseCDP — cdp-creds.json (chmod 600; .env.local-style secrets, gitignored) +{ "apiKeyId": "...", "apiKeySecret": "...", "walletSecret": "..." } + +agentcore add payment-connector --manager MyManager --name MyCDPConnector \ + --provider CoinbaseCDP --credentials-file ./cdp-creds.json + +# StripePrivy keys: appId, appSecret, authorizationPrivateKey, authorizationId +# Or pipe from a secret manager without a temp file: +get-secret cdp-creds | agentcore add payment-connector --manager MyManager \ + --name MyCDPConnector --provider CoinbaseCDP --credentials-file - +``` + ### CoinbaseCDP Provider ```bash @@ -138,15 +158,16 @@ agentcore add payment-connector \ --wallet-secret your-wallet-secret ``` -| Flag | Description | -| --------------------------- | ---------------------------------------- | -| `--manager ` | Parent payment manager (required) | -| `--name ` | Connector name (required) | -| `--provider ` | `CoinbaseCDP` (default) or `StripePrivy` | -| `--api-key-id ` | Coinbase CDP API Key ID | -| `--api-key-secret ` | Coinbase CDP API Key Secret | -| `--wallet-secret ` | Coinbase CDP Wallet Secret (ECDSA P-256) | -| `--json` | Output result as JSON | +| Flag | Description | +| --------------------------- | ------------------------------------------------------------------------------------------- | +| `--manager ` | Parent payment manager (required) | +| `--name ` | Connector name (required) | +| `--provider ` | `CoinbaseCDP` (default) or `StripePrivy` | +| `--api-key-id ` | Coinbase CDP API Key ID | +| `--api-key-secret ` | Coinbase CDP API Key Secret | +| `--wallet-secret ` | Coinbase CDP Wallet Secret (ECDSA P-256) | +| `--credentials-file ` | JSON file (or `-` for stdin) with the provider secrets; preferred over literal secret flags | +| `--json` | Output result as JSON | ### StripePrivy Provider @@ -161,16 +182,17 @@ agentcore add payment-connector \ --authorization-id your-authorization-key-id ``` -| Flag | Description | -| ----------------------------------- | ----------------------------------- | -| `--manager ` | Parent payment manager (required) | -| `--name ` | Connector name (required) | -| `--provider ` | Must be `StripePrivy` | -| `--app-id ` | Privy App ID | -| `--app-secret ` | Privy App Secret | -| `--authorization-private-key ` | ECDSA P-256 private key for signing | -| `--authorization-id ` | Authorization key identifier | -| `--json` | Output result as JSON | +| Flag | Description | +| ----------------------------------- | ------------------------------------------------------------------------------------------- | +| `--manager ` | Parent payment manager (required) | +| `--name ` | Connector name (required) | +| `--provider ` | Must be `StripePrivy` | +| `--app-id ` | Privy App ID | +| `--app-secret ` | Privy App Secret | +| `--authorization-private-key ` | ECDSA P-256 private key for signing | +| `--authorization-id ` | Authorization key identifier | +| `--credentials-file ` | JSON file (or `-` for stdin) with the provider secrets; preferred over literal secret flags | +| `--json` | Output result as JSON | ### Credential Storage diff --git a/src/cli/aws/__tests__/agentcore-payments.test.ts b/src/cli/aws/__tests__/agentcore-payments.test.ts new file mode 100644 index 000000000..0a1706314 --- /dev/null +++ b/src/cli/aws/__tests__/agentcore-payments.test.ts @@ -0,0 +1,179 @@ +import { isQuotaExceededError } from '../../errors'; +import { buildPaymentApiError, createPaymentCredentialProvider, getPaymentManager } from '../agentcore-payments'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +describe('buildPaymentApiError', () => { + it('never echoes the response body, even secret-bearing nested/snake_case fields', () => { + const body = JSON.stringify({ + message: 'validation failed', + coinbaseCdpConfiguration: { api_key_secret: 'SUPER_SECRET', walletSecret: 'WALLET_SECRET' }, + stripePrivyConfiguration: { authorization_private_key: 'PRIV_KEY' }, + }); + const err = buildPaymentApiError(400, body); + expect(err.message).not.toContain('SUPER_SECRET'); + expect(err.message).not.toContain('WALLET_SECRET'); + expect(err.message).not.toContain('PRIV_KEY'); + expect(err.message).not.toContain('validation failed'); + expect(err.message).toContain('(400)'); + }); + + it('surfaces the parsed code from `code`', () => { + const err = buildPaymentApiError(409, JSON.stringify({ code: 'ConflictException', appSecret: 'leak' })); + expect(err.code).toBe('ConflictException'); + expect(err.message).toBe('Payment API error (409): ConflictException'); + expect(err.message).not.toContain('leak'); + }); + + it('falls back to `__type` when `code` is absent', () => { + const err = buildPaymentApiError(404, JSON.stringify({ __type: 'ResourceNotFoundException' })); + expect(err.code).toBe('ResourceNotFoundException'); + expect(err.message).toContain('ResourceNotFoundException'); + }); + + it('uses a static message when the body is unparseable, with no body content', () => { + const err = buildPaymentApiError(500, 'secret-token-xyz'); + expect(err.code).toBeUndefined(); + expect(err.message).toBe('Payment API error (500): request failed'); + expect(err.message).not.toContain('secret-token-xyz'); + }); + + it('uses the data-plane label when opts.dataPlane is set', () => { + const err = buildPaymentApiError(403, JSON.stringify({ code: 'AccessDenied' }), { dataPlane: true }); + expect(err.message).toBe('Payment data plane API error (403): AccessDenied'); + }); + + describe('safe server message surfacing', () => { + it('surfaces a server message when the body has no secret fields', () => { + const err = buildPaymentApiError( + 400, + JSON.stringify({ code: 'ValidationException', message: 'paymentManagerId must be <= 64 chars' }) + ); + expect(err.code).toBe('ValidationException'); + expect(err.message).toBe('Payment API error (400): ValidationException: paymentManagerId must be <= 64 chars'); + }); + + it('surfaces a message even without a code', () => { + const err = buildPaymentApiError(400, JSON.stringify({ message: 'invalid network preference' })); + expect(err.message).toBe('Payment API error (400): invalid network preference'); + }); + + it('suppresses the message when a secret field appears anywhere in the body', () => { + const err = buildPaymentApiError( + 400, + JSON.stringify({ message: 'bad config', coinbaseCdpConfiguration: { walletSecret: 'LEAK' } }) + ); + expect(err.message).toBe('Payment API error (400): request failed'); + expect(err.message).not.toContain('bad config'); + expect(err.message).not.toContain('LEAK'); + }); + + it('suppresses the message when the message itself names a secret field', () => { + const err = buildPaymentApiError( + 400, + JSON.stringify({ code: 'ValidationException', message: 'apiKeySecret is malformed: BADVALUE' }) + ); + expect(err.message).toBe('Payment API error (400): ValidationException'); + expect(err.message).not.toContain('BADVALUE'); + expect(err.message).not.toContain('apiKeySecret'); + }); + + it('truncates an overlong server message', () => { + const long = 'x'.repeat(500); + const err = buildPaymentApiError(400, JSON.stringify({ message: long })); + expect(err.message.length).toBeLessThan(260); + expect(err.message).toContain('Payment API error (400):'); + }); + }); +}); + +vi.mock('@smithy/signature-v4', () => ({ + SignatureV4: class { + sign = vi.fn().mockResolvedValue({ headers: {} }); + }, +})); +vi.mock('@aws-crypto/sha256-js', () => ({ Sha256: class {} })); +vi.mock('@smithy/protocol-http', () => ({ + HttpRequest: class { + constructor(public options: unknown) {} + }, +})); +vi.mock('@aws-sdk/credential-provider-node', () => ({ defaultProvider: () => vi.fn() })); +vi.mock('../account', () => ({ getCredentialProvider: () => undefined })); +vi.mock('../stage-endpoint', () => ({ + controlPlaneEndpoint: () => 'https://cp.example.com', + dataPlaneEndpoint: () => 'https://dp.example.com', +})); + +const mockFetch = vi.fn(); + +describe('getPaymentManager 404 handling', () => { + beforeEach(() => vi.stubGlobal('fetch', mockFetch)); + afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); + }); + + it('returns null on a 404 with a parsed code and never leaks the body', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 404, + text: () => Promise.resolve(JSON.stringify({ __type: 'ResourceNotFoundException', appSecret: 'leak' })), + }); + const result = await getPaymentManager({ region: 'us-east-1', paymentManagerId: 'pm-1' }); + expect(result).toBeNull(); + }); + + it('rethrows non-404 errors without echoing the body', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 400, + text: () => Promise.resolve(JSON.stringify({ code: 'ValidationException', walletSecret: 'leak' })), + }); + await expect(getPaymentManager({ region: 'us-east-1', paymentManagerId: 'pm-1' })).rejects.toThrow( + /Failed to get payment manager .* ValidationException/ + ); + await expect(getPaymentManager({ region: 'us-east-1', paymentManagerId: 'pm-1' })).rejects.not.toThrow(/leak/); + }); +}); + +describe('createPaymentCredentialProvider quota error propagation', () => { + beforeEach(() => vi.stubGlobal('fetch', mockFetch)); + afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); + }); + + it('preserves .code so isQuotaExceededError detects quota errors without leaking secrets', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 402, + text: () => + Promise.resolve( + JSON.stringify({ + code: 'ServiceQuotaExceededException', + message: 'too many credential providers', + apiKeySecret: 'leak', + }) + ), + }); + + let error: unknown; + try { + await createPaymentCredentialProvider({ + region: 'us-east-1', + name: 'cp1', + vendor: 'CoinbaseCDP', + apiKeyId: 'a', + apiKeySecret: 'b', + walletSecret: 'c', + }); + } catch (err) { + error = err; + } + + expect(error).toBeDefined(); + expect(isQuotaExceededError(error)).toBe(true); + expect((error as { code?: string }).code).toBe('ServiceQuotaExceededException'); + expect(String((error as Error).message)).not.toContain('leak'); + }); +}); diff --git a/src/cli/aws/agentcore-payments.ts b/src/cli/aws/agentcore-payments.ts index bbd9d6272..fe371d79d 100644 --- a/src/cli/aws/agentcore-payments.ts +++ b/src/cli/aws/agentcore-payments.ts @@ -78,6 +78,88 @@ interface PaymentManagerDetail { roleArn?: string; } +// ============================================================================ +// Error helpers +// ============================================================================ + +// Provider secret field names, normalized (lowercased, underscores stripped) +// so camelCase and snake_case variants collapse to one token. Used to decide +// whether a response body is secret-bearing — NOT to redact it. +const SECRET_FIELD_TOKENS = new Set([ + 'apikeyid', + 'apikeysecret', + 'walletsecret', + 'appid', + 'appsecret', + 'authorizationprivatekey', + 'authorizationid', +]); + +const normalizeKey = (key: string): string => key.toLowerCase().replace(/_/g, ''); + +/** + * True if any key anywhere in the value (recursively) names a known provider + * secret field. Used as a structural gate: if a secret field appears anywhere + * in the body, we treat the whole body as untrusted and surface no excerpt. + */ +function containsSecretField(value: unknown): boolean { + if (Array.isArray(value)) { + return value.some(containsSecretField); + } + if (value && typeof value === 'object') { + return Object.entries(value as Record).some( + ([key, child]) => SECRET_FIELD_TOKENS.has(normalizeKey(key)) || containsSecretField(child) + ); + } + return false; +} + +const MAX_MESSAGE_LEN = 200; + +/** + * Build an error for a non-2xx payment API response using an allowlist: + * the HTTP status and the parsed error code/__type are always surfaced. The + * raw response body is never interpolated — payment provisioning bodies echo + * request fields (provider secrets), so a denylist that must "remember every + * key" is unsafe by construction. + * + * A server-provided `message` (e.g. a validation hint) is surfaced ONLY when it + * is provably safe: the parsed body contains no secret field name at any depth, + * and the message text itself names no secret field. Anything else collapses to + * status + code. error.code is preserved for downstream ResourceNotFoundException + * / quota detection. + */ +export function buildPaymentApiError( + status: number, + rawBody: string, + opts?: { dataPlane?: boolean } +): Error & { code?: string } { + let code: string | undefined; + let safeMessage: string | undefined; + try { + const parsed = JSON.parse(rawBody) as Record; + const parsedCode = parsed.code ?? parsed.__type; + if (typeof parsedCode === 'string') code = parsedCode; + + const message = parsed.message; + if (typeof message === 'string' && message.trim() && !containsSecretField(parsed)) { + const normalizedMessage = normalizeKey(message); + const messageNamesSecret = [...SECRET_FIELD_TOKENS].some(token => normalizedMessage.includes(token)); + if (!messageNamesSecret) { + safeMessage = message.trim().slice(0, MAX_MESSAGE_LEN); + } + } + } catch { + /* body is not JSON — surface status only */ + } + + const label = opts?.dataPlane ? 'Payment data plane API error' : 'Payment API error'; + const detail = [code, safeMessage].filter(Boolean).join(': ') || 'request failed'; + const error = new Error(`${label} (${status}): ${detail}`) as Error & { code?: string }; + if (code) error.code = code; + return error; +} + // ============================================================================ // HTTP signing helper // ============================================================================ @@ -139,24 +221,8 @@ async function signedRequest(options: { } if (!response.ok) { - const errorBody = await response.text(); - // Sanitize error body -- API validation errors may echo request fields containing secrets - const sanitized = errorBody - .replace( - /("apiKeySecret"|"walletSecret"|"apiKeyId"|"appId"|"appSecret"|"authorizationPrivateKey"|"authorizationId")\s*:\s*"[^"]*"/g, - '$1:"[REDACTED]"' - ) - .slice(0, 500); - - const error = new Error(`Payment API error (${response.status}): ${sanitized}`) as Error & { code?: string }; - try { - const parsed = JSON.parse(errorBody) as Record; - const code = parsed.code ?? parsed.__type; - if (typeof code === 'string') error.code = code; - } catch (_err) { - /* ignore parse failures */ - } - throw error; + const errorBody = await response.text().catch(() => ''); + throw buildPaymentApiError(response.status, errorBody); } if (response.status === 204) return {}; @@ -219,9 +285,13 @@ export async function createPaymentCredentialProvider( status: data.status, }; } catch (err) { - throw new Error( + // preserve API error code for downstream quota/404 detection + const wrapped = new Error( `Failed to create payment credential provider "${options.name}": ${err instanceof Error ? err.message : String(err)}` - ); + ) as Error & { code?: string }; + const code = (err as { code?: string }).code; + if (code) wrapped.code = code; + throw wrapped; } } @@ -248,9 +318,12 @@ export async function updatePaymentCredentialProvider( status: data.status, }; } catch (err) { - throw new Error( + const wrapped = new Error( `Failed to update payment credential provider "${options.name}": ${err instanceof Error ? err.message : String(err)}` - ); + ) as Error & { code?: string }; + const code = (err as { code?: string }).code; + if (code) wrapped.code = code; + throw wrapped; } } @@ -267,9 +340,14 @@ export async function getPaymentCredentialProvider( return data; } catch (err) { + const code = (err as { code?: string }).code; const msg = err instanceof Error ? err.message : String(err); - if (msg.includes('(404)') || msg.includes('ResourceNotFoundException')) return null; - throw new Error(`Failed to get payment credential provider "${options.name}": ${msg}`); + if (code === 'ResourceNotFoundException' || msg.includes('(404)')) return null; + const wrapped = new Error(`Failed to get payment credential provider "${options.name}": ${msg}`) as Error & { + code?: string; + }; + if (code) wrapped.code = code; + throw wrapped; } } @@ -282,9 +360,12 @@ export async function deletePaymentCredentialProvider(options: { region: string; body: JSON.stringify({ name: options.name }), }); } catch (err) { - throw new Error( + const wrapped = new Error( `Failed to delete payment credential provider "${options.name}": ${err instanceof Error ? err.message : String(err)}` - ); + ) as Error & { code?: string }; + const code = (err as { code?: string }).code; + if (code) wrapped.code = code; + throw wrapped; } } @@ -300,9 +381,14 @@ export async function getPaymentManager(options: GetPaymentManagerOptions): Prom path: `/payments/managers/${encodeURIComponent(options.paymentManagerId)}`, })) as PaymentManagerDetail; } catch (err) { + const code = (err as { code?: string }).code; const msg = err instanceof Error ? err.message : String(err); - if (msg.includes('(404)') || msg.includes('ResourceNotFoundException')) return null; - throw new Error(`Failed to get payment manager "${options.paymentManagerId}": ${msg}`); + if (code === 'ResourceNotFoundException' || msg.includes('(404)')) return null; + const wrapped = new Error(`Failed to get payment manager "${options.paymentManagerId}": ${msg}`) as Error & { + code?: string; + }; + if (code) wrapped.code = code; + throw wrapped; } } @@ -370,23 +456,7 @@ async function signedDataPlaneRequest(options: { if (!response.ok) { const errorBody = await response.text().catch(() => ''); - const sanitized = errorBody - .replace( - /("apiKeySecret"|"walletSecret"|"apiKeyId"|"appId"|"appSecret"|"authorizationPrivateKey"|"authorizationId")\s*:\s*"[^"]*"/g, - '$1:"[REDACTED]"' - ) - .slice(0, 500); - const error = new Error(`Payment data plane API error (${response.status}): ${sanitized}`) as Error & { - code?: string; - }; - try { - const parsed = JSON.parse(errorBody) as Record; - const code = parsed.code ?? parsed.__type; - if (typeof code === 'string') error.code = code; - } catch (_err) { - /* ignore parse failures */ - } - throw error; + throw buildPaymentApiError(response.status, errorBody, { dataPlane: true }); } if (response.status === 204) return {}; diff --git a/src/cli/commands/deploy/actions.ts b/src/cli/commands/deploy/actions.ts index 6b2a5b30b..75f00f534 100644 --- a/src/cli/commands/deploy/actions.ts +++ b/src/cli/commands/deploy/actions.ts @@ -762,6 +762,19 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise = { + apiKeyId: '--api-key-id', + apiKeySecret: '--api-key-secret', + walletSecret: '--wallet-secret', + appId: '--app-id', + appSecret: '--app-secret', + authorizationPrivateKey: '--authorization-private-key', + authorizationId: '--authorization-id', +}; + +/** + * Merge secrets supplied as literal flags with those read from a credentials + * file. Flag values win only when the file does not also set that field; a + * field set by BOTH is an ambiguity error (so a stale flag can't silently + * shadow the file, or vice versa). + */ +export function resolvePaymentSecrets( + flagValues: PaymentSecretValues, + fileValues: PaymentSecretValues +): PaymentSecretValues { + const merged: PaymentSecretValues = { ...fileValues }; + for (const key of Object.keys(flagValues) as (keyof PaymentSecretValues)[]) { + const flagVal = flagValues[key]; + if (flagVal === undefined) continue; + if (fileValues[key] !== undefined) { + throw new Error( + `Conflicting secret for ${key}: both --credentials-file and ${SECRET_FLAG_NAMES[key]} were provided.` + ); + } + merged[key] = flagVal; + } + return merged; +} + /** * Removable connector resource with parent manager context. */ @@ -325,6 +362,10 @@ export class PaymentConnectorPrimitive extends BasePrimitive', 'Privy App Secret (StripePrivy) [non-interactive]') .option('--authorization-private-key ', 'ECDSA P-256 private key (StripePrivy) [non-interactive]') .option('--authorization-id ', 'Authorization key identifier (StripePrivy) [non-interactive]') + .option( + '--credentials-file ', + 'Path to a JSON file (or "-" for stdin) with the provider secrets, keeping them out of shell history [non-interactive]' + ) .option('--json', 'Output as JSON [non-interactive]') .action( async (cliOptions: { @@ -338,6 +379,7 @@ export class PaymentConnectorPrimitive extends BasePrimitive { try { @@ -357,6 +399,7 @@ export class PaymentConnectorPrimitive extends BasePrimitive v !== undefined); + if (usedLiteralSecretFlag) { + process.stderr.write( + `${ANSI.yellow}Warning: passing secrets as CLI flags exposes them to shell history and the ` + + `process table. Use --credentials-file (or "-" for stdin) instead.${ANSI.reset}\n` + ); + } + + let secrets: PaymentSecretValues; + try { + const fileSecrets = cliOptions.credentialsFile ? readCredentialsFile(cliOptions.credentialsFile) : {}; + secrets = resolvePaymentSecrets(flagSecrets, fileSecrets); + } catch (err) { + const error = err instanceof Error ? err.message : String(err); + if (cliOptions.json) { + console.log(JSON.stringify({ success: false, error })); + } else { + console.error(error); + } + process.exit(1); + } + const missing: string[] = []; if (!cliOptions.manager) missing.push('--manager'); if (!cliOptions.name) missing.push('--name'); if (provider === 'StripePrivy') { - if (!cliOptions.appId?.trim()) missing.push('--app-id'); - if (!cliOptions.appSecret?.trim()) missing.push('--app-secret'); - if (!cliOptions.authorizationPrivateKey?.trim()) missing.push('--authorization-private-key'); - if (!cliOptions.authorizationId?.trim()) missing.push('--authorization-id'); + if (!secrets.appId?.trim()) missing.push('--app-id'); + if (!secrets.appSecret?.trim()) missing.push('--app-secret'); + if (!secrets.authorizationPrivateKey?.trim()) missing.push('--authorization-private-key'); + if (!secrets.authorizationId?.trim()) missing.push('--authorization-id'); } else { - if (!cliOptions.apiKeyId?.trim()) missing.push('--api-key-id'); - if (!cliOptions.apiKeySecret?.trim()) missing.push('--api-key-secret'); - if (!cliOptions.walletSecret?.trim()) missing.push('--wallet-secret'); + if (!secrets.apiKeyId?.trim()) missing.push('--api-key-id'); + if (!secrets.apiKeySecret?.trim()) missing.push('--api-key-secret'); + if (!secrets.walletSecret?.trim()) missing.push('--wallet-secret'); } if (missing.length > 0) { @@ -421,10 +496,10 @@ export class PaymentConnectorPrimitive extends BasePrimitive> { + ): Promise> { try { const project = await this.readProjectSpec(); // payments is optional in the schema (absent on projects with no payment @@ -189,7 +190,20 @@ export class PaymentManagerPrimitive extends BasePrimitive 0) { console.warn( `\nWarning: payment capability auto-wiring skipped for non-Strands runtime(s): ${result.skippedRuntimes.join(', ')}.` diff --git a/src/cli/primitives/__tests__/PaymentConnectorPrimitive.test.ts b/src/cli/primitives/__tests__/PaymentConnectorPrimitive.test.ts index 48189aa6f..3d6a8f00b 100644 --- a/src/cli/primitives/__tests__/PaymentConnectorPrimitive.test.ts +++ b/src/cli/primitives/__tests__/PaymentConnectorPrimitive.test.ts @@ -1,5 +1,5 @@ import type { AgentCoreProjectSpec } from '../../../schema'; -import { PaymentConnectorPrimitive } from '../PaymentConnectorPrimitive'; +import { PaymentConnectorPrimitive, resolvePaymentSecrets } from '../PaymentConnectorPrimitive'; import { beforeEach, describe, expect, it, vi } from 'vitest'; // ── Hoisted mocks ──────────────────────────────────────────────────────────── @@ -481,3 +481,23 @@ describe('PaymentConnectorPrimitive', () => { }); }); }); + +describe('resolvePaymentSecrets', () => { + it('fills missing secrets from the file values', () => { + const merged = resolvePaymentSecrets( + { apiKeyId: 'flag-id' }, + { apiKeySecret: 'file-sec', walletSecret: 'file-wal' } + ); + expect(merged).toEqual({ apiKeyId: 'flag-id', apiKeySecret: 'file-sec', walletSecret: 'file-wal' }); + }); + + it('throws when a literal flag and file set the same field', () => { + expect(() => resolvePaymentSecrets({ apiKeySecret: 'flag-sec' }, { apiKeySecret: 'file-sec' })).toThrow( + /both --credentials-file and --api-key-secret/i + ); + }); + + it('returns flag values unchanged when no file is given', () => { + expect(resolvePaymentSecrets({ appSecret: 'x' }, {})).toEqual({ appSecret: 'x' }); + }); +}); diff --git a/src/cli/primitives/__tests__/PaymentManagerPrimitive.test.ts b/src/cli/primitives/__tests__/PaymentManagerPrimitive.test.ts index 7bc1d47c5..c98497e82 100644 --- a/src/cli/primitives/__tests__/PaymentManagerPrimitive.test.ts +++ b/src/cli/primitives/__tests__/PaymentManagerPrimitive.test.ts @@ -207,6 +207,27 @@ describe('PaymentManagerPrimitive', () => { expect(result.error.message).toBe('disk read failure'); } }); + + it('returns an auto-payment warning when enabled (default)', async () => { + mockReadProjectSpec.mockResolvedValue(makeProject({ runtimes: [] })); + + const result = await primitive.add({ name: 'mgr1', authorizerType: 'AWS_IAM' }); + + expect(result.success).toBe(true); + if (!result.success) throw new Error('expected success'); + expect(result.autoPaymentWarning).toMatch(/auto-payment is enabled/i); + expect(result.autoPaymentWarning).toContain('--auto-payment false'); + }); + + it('returns no auto-payment warning when explicitly disabled', async () => { + mockReadProjectSpec.mockResolvedValue(makeProject({ runtimes: [] })); + + const result = await primitive.add({ name: 'mgr2', authorizerType: 'AWS_IAM', autoPayment: false }); + + expect(result.success).toBe(true); + if (!result.success) throw new Error('expected success'); + expect(result.autoPaymentWarning).toBeUndefined(); + }); }); describe('remove()', () => { diff --git a/src/cli/primitives/__tests__/credential-file.test.ts b/src/cli/primitives/__tests__/credential-file.test.ts new file mode 100644 index 000000000..a883211cc --- /dev/null +++ b/src/cli/primitives/__tests__/credential-file.test.ts @@ -0,0 +1,68 @@ +import { readCredentialsFile } from '../credential-file'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +describe('readCredentialsFile', () => { + let dir: string; + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'cred-file-')); + }); + afterEach(() => rmSync(dir, { recursive: true, force: true })); + + it('reads recognized secret fields from a JSON file', () => { + const p = join(dir, 'cdp.json'); + writeFileSync(p, JSON.stringify({ apiKeyId: 'id', apiKeySecret: 'sec', walletSecret: 'w', bogus: 'x' })); + const result = readCredentialsFile(p); + expect(result).toEqual({ apiKeyId: 'id', apiKeySecret: 'sec', walletSecret: 'w' }); + }); + + it('reads from stdin when path is "-"', () => { + const result = readCredentialsFile('-', () => JSON.stringify({ appId: 'a', appSecret: 'b' })); + expect(result).toEqual({ appId: 'a', appSecret: 'b' }); + }); + + it('reads real stdin (fd 0) by default via a piped child process', () => { + const json = JSON.stringify({ apiKeyId: 'fd0-id', apiKeySecret: 'fd0-sec' }); + // Exercise the DEFAULT readStdin (readFileSync(0)) against a real pipe — the + // injected-stub test above can't prove fd 0 is used. Run the helper in a + // child node process with JSON on stdin and read back what it parsed. + const script = `import('${pathToFileURL(join(process.cwd(), 'src/cli/primitives/credential-file.ts')).href}').then(m => process.stdout.write(JSON.stringify(m.readCredentialsFile('-'))));`; + const out = execFileSync(process.execPath, ['--import', 'tsx', '--input-type=module', '-e', script], { + input: json, + encoding: 'utf-8', + }); + expect(JSON.parse(out)).toEqual({ apiKeyId: 'fd0-id', apiKeySecret: 'fd0-sec' }); + }); + + it('throws when the file does not exist', () => { + expect(() => readCredentialsFile(join(dir, 'missing.json'))).toThrow(/not found|no such file/i); + }); + + it('throws on empty content', () => { + const p = join(dir, 'empty.json'); + writeFileSync(p, ' '); + expect(() => readCredentialsFile(p)).toThrow(/empty/i); + }); + + it('throws on malformed JSON', () => { + const p = join(dir, 'bad.json'); + writeFileSync(p, '{not json'); + expect(() => readCredentialsFile(p)).toThrow(/valid JSON|parse/i); + }); + + it('throws when JSON is not an object', () => { + const p = join(dir, 'arr.json'); + writeFileSync(p, '["a"]'); + expect(() => readCredentialsFile(p)).toThrow(/JSON object/i); + }); + + it('ignores non-string values', () => { + const p = join(dir, 'num.json'); + writeFileSync(p, JSON.stringify({ apiKeyId: 123, apiKeySecret: 'ok' })); + expect(readCredentialsFile(p)).toEqual({ apiKeySecret: 'ok' }); + }); +}); diff --git a/src/cli/primitives/__tests__/payment-validation.test.ts b/src/cli/primitives/__tests__/payment-validation.test.ts index df4f56eac..e3dcb4b91 100644 --- a/src/cli/primitives/__tests__/payment-validation.test.ts +++ b/src/cli/primitives/__tests__/payment-validation.test.ts @@ -89,46 +89,3 @@ describe('base64 key validation', () => { expect(validateBase64Key(key).valid).toBe(true); }); }); - -describe('credential sanitization regex', () => { - const REGEX = - /("apiKeySecret"|"walletSecret"|"apiKeyId"|"appId"|"appSecret"|"authorizationPrivateKey"|"authorizationId")\s*:\s*"[^"]*"/g; - - function sanitize(body: string): string { - return body.replace(REGEX, '$1:"[REDACTED]"').slice(0, 500); - } - - it('redacts all 7 credential field names', () => { - const body = JSON.stringify({ - apiKeyId: 'key-123', - apiKeySecret: 'secret-456', - walletSecret: 'wallet-789', - appId: 'app-abc', - appSecret: 'app-secret-def', - authorizationPrivateKey: 'priv-key-ghi', - authorizationId: 'auth-jkl', - }); - const result = sanitize(body); - expect(result).not.toContain('key-123'); - expect(result).not.toContain('secret-456'); - expect(result).not.toContain('wallet-789'); - expect(result).not.toContain('app-abc'); - expect(result).not.toContain('app-secret-def'); - expect(result).not.toContain('priv-key-ghi'); - expect(result).not.toContain('auth-jkl'); - expect(result).toContain('[REDACTED]'); - }); - - it('preserves non-credential fields', () => { - const body = JSON.stringify({ message: 'Not found', code: 'ResourceNotFoundException', apiKeySecret: 'leaked' }); - const result = sanitize(body); - expect(result).toContain('Not found'); - expect(result).toContain('ResourceNotFoundException'); - expect(result).not.toContain('leaked'); - }); - - it('truncates to 500 characters', () => { - const longBody = '{"apiKeyId":"x"}'.repeat(100); - expect(sanitize(longBody).length).toBeLessThanOrEqual(500); - }); -}); diff --git a/src/cli/primitives/credential-file.ts b/src/cli/primitives/credential-file.ts new file mode 100644 index 000000000..7265611ce --- /dev/null +++ b/src/cli/primitives/credential-file.ts @@ -0,0 +1,68 @@ +import { existsSync, readFileSync } from 'node:fs'; + +/** Recognized payment provider secret fields (CoinbaseCDP + StripePrivy). */ +export interface PaymentSecretValues { + apiKeyId?: string; + apiKeySecret?: string; + walletSecret?: string; + appId?: string; + appSecret?: string; + authorizationPrivateKey?: string; + authorizationId?: string; +} + +const SECRET_FIELDS: (keyof PaymentSecretValues)[] = [ + 'apiKeyId', + 'apiKeySecret', + 'walletSecret', + 'appId', + 'appSecret', + 'authorizationPrivateKey', + 'authorizationId', +]; + +/** + * Read payment provider secrets from a JSON file, or from stdin when + * `pathOrDash` is "-". Returns only recognized string-valued secret fields so + * unrelated keys never propagate. Keeping secrets in a file (or piped via + * stdin) keeps them out of argv, shell history, and the process table. + */ +export function readCredentialsFile( + pathOrDash: string, + // Read stdin by file descriptor (0) rather than '/dev/stdin', which does not + // exist on Windows; fd 0 works on Linux, macOS, and Windows. + readStdin: () => string = () => readFileSync(0, 'utf-8') +): PaymentSecretValues { + let raw: string; + if (pathOrDash === '-') { + raw = readStdin(); + } else { + if (!existsSync(pathOrDash)) { + throw new Error(`Credentials file not found: ${pathOrDash}`); + } + raw = readFileSync(pathOrDash, 'utf-8'); + } + + if (!raw.trim()) { + throw new Error('Credentials file is empty.'); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + throw new Error('Credentials file must contain valid JSON.'); + } + + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + throw new Error('Credentials file must contain a JSON object of secret fields.'); + } + + const source = parsed as Record; + const result: PaymentSecretValues = {}; + for (const field of SECRET_FIELDS) { + const value = source[field]; + if (typeof value === 'string') result[field] = value; + } + return result; +} diff --git a/src/cli/tui/screens/payment/AddPaymentFlow.tsx b/src/cli/tui/screens/payment/AddPaymentFlow.tsx index cea6f19d0..e2c15f574 100644 --- a/src/cli/tui/screens/payment/AddPaymentFlow.tsx +++ b/src/cli/tui/screens/payment/AddPaymentFlow.tsx @@ -348,9 +348,19 @@ export function AddPaymentFlow({ ] : []; - const warningFields = !flow.connectorConfig - ? [{ label: '⚠ Warning', value: 'No connector — deploy will fail until you add one' }] - : []; + const warningFields = [ + ...(flow.managerConfig.autoPayment + ? [ + { + label: '⚠ Warning', + value: `Auto-payment ENABLED — agent settles 402s automatically up to $${flow.managerConfig.defaultSpendLimit}/session with no human approval`, + }, + ] + : []), + ...(!flow.connectorConfig + ? [{ label: '⚠ Warning', value: 'No connector — deploy will fail until you add one' }] + : []), + ]; const allFields = [...managerFields, ...connectorFields, ...warningFields];