From c63493753a2baf7cf0e23aa959997bb699ba54f3 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 17 Jun 2026 22:47:03 +0000 Subject: [PATCH] fix(payment-connector): validate secret key formats client-side at add time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding a payment connector with a wrong-format secret (e.g. a non-Ed25519 CoinbaseCDP apiKeySecret) succeeded at `add` — the TUI even showed a green tick — and only failed later at deploy with an opaque server-side error. Add a shared validator module (src/cli/primitives/payment-validation.ts) following the validateBYOMountPath pattern: each validator returns `true | string` so it plugs into both the CLI add path and the TUI SecretInput `customValidation` prop, validating identically. - apiKeySecret (CoinbaseCDP): base64-encoded Ed25519 private key - walletSecret (CoinbaseCDP): base64-encoded EC P-256 private key - authorizationPrivateKey (StripePrivy): base64-encoded EC P-256, accepting the optional wallet-auth: prefix (now via the shared stripWalletAuthPrefix) Opaque identifier fields (apiKeyId, appId, appSecret, authorizationId) keep their non-empty checks since they have no documented format. Ed25519 and P-256 keys differ in size, so each gets its own decoded-byte band rather than reusing one range. Integration-test fixtures now use real keys. --- integ-tests/add-remove-payment.test.ts | 36 ++++--- .../primitives/PaymentConnectorPrimitive.ts | 52 +++++----- .../__tests__/payment-validation.test.ts | 98 ++++++++++++++----- src/cli/primitives/payment-validation.ts | 97 ++++++++++++++++++ .../payment/AddPaymentConnectorScreen.tsx | 11 ++- .../screens/payment/useAddPaymentWizard.ts | 5 +- 6 files changed, 228 insertions(+), 71 deletions(-) create mode 100644 src/cli/primitives/payment-validation.ts diff --git a/integ-tests/add-remove-payment.test.ts b/integ-tests/add-remove-payment.test.ts index 665c58037..21d80bb21 100644 --- a/integ-tests/add-remove-payment.test.ts +++ b/integ-tests/add-remove-payment.test.ts @@ -1,9 +1,21 @@ import { createTestProject, readProjectConfig, runCLI } from '../src/test-utils/index.js'; import type { TestProject } from '../src/test-utils/index.js'; +import { generateKeyPairSync } from 'node:crypto'; import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +// The CLI now validates CDP secret formats at add time: apiKeySecret must be a +// base64-encoded Ed25519 private key and walletSecret a base64-encoded EC P-256 +// private key. Use real keys so these lifecycle tests exercise add/remove rather +// than tripping the format check. +const CDP_API_KEY_SECRET = generateKeyPairSync('ed25519') + .privateKey.export({ type: 'pkcs8', format: 'der' }) + .toString('base64'); +const CDP_WALLET_SECRET = generateKeyPairSync('ec', { namedCurve: 'P-256' }) + .privateKey.export({ type: 'pkcs8', format: 'der' }) + .toString('base64'); + describe('integration: add and remove payment managers and connectors', () => { let project: TestProject; @@ -154,9 +166,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'test-key-id', '--api-key-secret', - 'test-key-secret', + CDP_API_KEY_SECRET, '--wallet-secret', - 'test-wallet-secret', + CDP_WALLET_SECRET, '--json', ], project.projectPath @@ -202,9 +214,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'test-key-id-2', '--api-key-secret', - 'test-key-secret-2', + CDP_API_KEY_SECRET, '--wallet-secret', - 'test-wallet-secret-2', + CDP_WALLET_SECRET, '--json', ], project.projectPath @@ -232,9 +244,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'x', '--api-key-secret', - 'y', + CDP_API_KEY_SECRET, '--wallet-secret', - 'z', + CDP_WALLET_SECRET, '--json', ], project.projectPath @@ -260,9 +272,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'x', '--api-key-secret', - 'y', + CDP_API_KEY_SECRET, '--wallet-secret', - 'z', + CDP_WALLET_SECRET, '--json', ], project.projectPath @@ -464,9 +476,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'x', '--api-key-secret', - 'y', + CDP_API_KEY_SECRET, '--wallet-secret', - 'z', + CDP_WALLET_SECRET, '--json', ], project.projectPath @@ -486,9 +498,9 @@ describe('integration: add and remove payment managers and connectors', () => { '--api-key-id', 'x', '--api-key-secret', - 'y', + CDP_API_KEY_SECRET, '--wallet-secret', - 'z', + CDP_WALLET_SECRET, ], project.projectPath ); diff --git a/src/cli/primitives/PaymentConnectorPrimitive.ts b/src/cli/primitives/PaymentConnectorPrimitive.ts index 4fa90a68a..0dc308b45 100644 --- a/src/cli/primitives/PaymentConnectorPrimitive.ts +++ b/src/cli/primitives/PaymentConnectorPrimitive.ts @@ -8,6 +8,12 @@ import { requireTTY } from '../tui/guards/tty'; import { BasePrimitive } from './BasePrimitive'; import { SOURCE_CODE_NOTE } from './constants'; import { computePaymentCredentialEnvVarNames, computeStripePrivyCredentialEnvVarNames } from './credential-utils'; +import { + stripWalletAuthPrefix, + validateApiKeySecret, + validateAuthorizationPrivateKey, + validateWalletSecret, +} from './payment-validation'; import type { AddResult, AddScreenComponent, RemovableResource } from './types'; import type { Command } from '@commander-js/extra-typings'; @@ -418,35 +424,27 @@ export class PaymentConnectorPrimitive extends BasePrimitive { + if (cliOptions.json) { + console.log(JSON.stringify({ success: false, error })); + } else { + console.error(error); + } + process.exit(1); + }; + if (provider === 'StripePrivy') { // AWS docs ship the key with a `wallet-auth:` prefix — strip it transparently. - let trimmedKey = cliOptions.authorizationPrivateKey!.trim(); - if (trimmedKey.startsWith('wallet-auth:')) { - trimmedKey = trimmedKey.slice('wallet-auth:'.length); - cliOptions.authorizationPrivateKey = trimmedKey; - } - const BASE64_REGEX = /^[A-Za-z0-9+/]+=*$/; - if (!BASE64_REGEX.test(trimmedKey)) { - const error = 'authorizationPrivateKey must be base64-encoded'; - if (cliOptions.json) { - console.log(JSON.stringify({ success: false, error })); - } else { - console.error(error); - } - process.exit(1); - } - const decoded = Buffer.from(trimmedKey, 'base64'); - if (decoded.length < 100 || decoded.length > 200) { - const error = - 'authorizationPrivateKey must be a base64-encoded EC P-256 private key (unexpected length)'; - if (cliOptions.json) { - console.log(JSON.stringify({ success: false, error })); - } else { - console.error(error); - } - process.exit(1); - } + cliOptions.authorizationPrivateKey = stripWalletAuthPrefix(cliOptions.authorizationPrivateKey!); + const keyResult = validateAuthorizationPrivateKey(cliOptions.authorizationPrivateKey); + if (keyResult !== true) failValidation(keyResult); + } else { + const apiKeySecretResult = validateApiKeySecret(cliOptions.apiKeySecret!); + if (apiKeySecretResult !== true) failValidation(apiKeySecretResult); + const walletSecretResult = validateWalletSecret(cliOptions.walletSecret!); + if (walletSecretResult !== true) failValidation(walletSecretResult); } let result: Awaited>; diff --git a/src/cli/primitives/__tests__/payment-validation.test.ts b/src/cli/primitives/__tests__/payment-validation.test.ts index df4f56eac..d65ec0c89 100644 --- a/src/cli/primitives/__tests__/payment-validation.test.ts +++ b/src/cli/primitives/__tests__/payment-validation.test.ts @@ -1,5 +1,22 @@ +import { + stripWalletAuthPrefix, + validateApiKeySecret, + validateAuthorizationPrivateKey, + validateWalletSecret, +} from '../payment-validation'; +import { generateKeyPairSync } from 'node:crypto'; import { describe, expect, it } from 'vitest'; +// Realistic base64-encoded private keys for each curve, matching what the +// payment APIs expect. Computed once so the byte-length bands are exercised +// against real key sizes rather than arbitrary buffers. +const ed25519Pkcs8 = generateKeyPairSync('ed25519') + .privateKey.export({ type: 'pkcs8', format: 'der' }) + .toString('base64'); // ~48 bytes +const p256Pkcs8 = generateKeyPairSync('ec', { namedCurve: 'P-256' }) + .privateKey.export({ type: 'pkcs8', format: 'der' }) + .toString('base64'); // ~138 bytes + describe('autoPayment CLI parsing', () => { function parseAutoPayment(value: string | boolean | undefined): boolean | undefined { if (value === undefined) return undefined; @@ -49,44 +66,73 @@ describe('defaultSpendLimit validation', () => { it('accepts empty string as 0 (Number("") === 0)', () => expect(validateSpendLimit('')).toEqual({ valid: true })); }); -describe('base64 key validation', () => { - const BASE64_REGEX = /^[A-Za-z0-9+/]+=*$/; +describe('validateApiKeySecret (CoinbaseCDP — Ed25519)', () => { + it('accepts a base64-encoded Ed25519 PKCS8 key (~48 bytes)', () => { + expect(validateApiKeySecret(ed25519Pkcs8)).toBe(true); + }); - function validateBase64Key(key: string): { valid: boolean; error?: string } { - const trimmed = key.trim(); - if (!BASE64_REGEX.test(trimmed)) return { valid: false, error: 'not base64' }; - const decoded = Buffer.from(trimmed, 'base64'); - if (decoded.length < 100 || decoded.length > 200) return { valid: false, error: 'unexpected length' }; - return { valid: true }; - } + it('accepts a 64-byte Coinbase seed+pubkey secret', () => { + expect(validateApiKeySecret(Buffer.alloc(64, 0x41).toString('base64'))).toBe(true); + }); + + it('accepts a raw 32-byte Ed25519 seed', () => { + expect(validateApiKeySecret(Buffer.alloc(32, 0x41).toString('base64'))).toBe(true); + }); + + it('rejects non-base64 input', () => { + expect(validateApiKeySecret('not base64!')).toContain('Ed25519'); + }); + + it('rejects a P-256 key (wrong curve — too long for Ed25519)', () => { + const result = validateApiKeySecret(p256Pkcs8); + expect(result).not.toBe(true); + expect(result).toContain('length'); + }); - it('rejects non-base64 characters', () => { - expect(validateBase64Key('not-base64!').valid).toBe(false); + it('rejects a too-short key', () => { + expect(validateApiKeySecret(Buffer.alloc(16, 0x41).toString('base64'))).not.toBe(true); + }); +}); + +describe('validateWalletSecret (CoinbaseCDP — EC P-256)', () => { + it('accepts a base64-encoded P-256 PKCS8 key (~138 bytes)', () => { + expect(validateWalletSecret(p256Pkcs8)).toBe(true); }); - it('rejects too-short decoded key (< 100 bytes)', () => { - expect(validateBase64Key('dGVzdA==').valid).toBe(false); + it('rejects non-base64 input', () => { + expect(validateWalletSecret('nope!')).toContain('P-256'); }); - it('rejects too-long decoded key (> 200 bytes)', () => { - const buf = Buffer.alloc(201, 0x42); - expect(validateBase64Key(buf.toString('base64')).valid).toBe(false); + it('rejects an Ed25519 key (wrong curve — too short for P-256)', () => { + expect(validateWalletSecret(ed25519Pkcs8)).not.toBe(true); + }); +}); + +describe('validateAuthorizationPrivateKey (StripePrivy — EC P-256)', () => { + it('accepts a base64-encoded P-256 PKCS8 key', () => { + expect(validateAuthorizationPrivateKey(p256Pkcs8)).toBe(true); }); - it('accepts decoded key of exactly 100 bytes', () => { - const buf = Buffer.alloc(100, 0x41); - expect(validateBase64Key(buf.toString('base64')).valid).toBe(true); + it('accepts a key with the wallet-auth: prefix', () => { + expect(validateAuthorizationPrivateKey(`wallet-auth:${p256Pkcs8}`)).toBe(true); }); - it('accepts decoded key of exactly 200 bytes', () => { - const buf = Buffer.alloc(200, 0x41); - expect(validateBase64Key(buf.toString('base64')).valid).toBe(true); + it('rejects non-base64 input', () => { + expect(validateAuthorizationPrivateKey('not-base64!')).toContain('base64'); + }); + + it('rejects a key of the wrong length', () => { + expect(validateAuthorizationPrivateKey('dGVzdA==')).toContain('length'); + }); +}); + +describe('stripWalletAuthPrefix', () => { + it('strips the wallet-auth: prefix', () => { + expect(stripWalletAuthPrefix('wallet-auth:ABC')).toBe('ABC'); }); - it('accepts a valid ~138 byte key', () => { - const key = - 'RkFLRV9TVFJJUEVfUFJJVllfVEVTVF9LRVlfQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQQ=='; - expect(validateBase64Key(key).valid).toBe(true); + it('trims and leaves an unprefixed value', () => { + expect(stripWalletAuthPrefix(' ABC ')).toBe('ABC'); }); }); diff --git a/src/cli/primitives/payment-validation.ts b/src/cli/primitives/payment-validation.ts new file mode 100644 index 000000000..b8c430733 --- /dev/null +++ b/src/cli/primitives/payment-validation.ts @@ -0,0 +1,97 @@ +/** + * Client-side format validation for payment connector secrets. + * + * Shared by the non-interactive CLI add path (PaymentConnectorPrimitive + * registerCommands) and the interactive TUI (AddPaymentConnectorScreen + * SecretInput). Each validator returns `true` when valid or a human-readable + * error string when not — the same `(value) => true | string` contract used by + * validateBYOMountPath and the SecretInput `customValidation` prop — so both + * surfaces reject identically instead of failing only at deploy time. + * + * Only fields with a server-enforced cryptographic format are validated here. + * The opaque identifier fields (apiKeyId, appId, appSecret, authorizationId) + * have no documented format, so they keep a non-empty check at the call site. + */ + +/** Characters allowed in standard base64 (with optional `=` padding). */ +const BASE64_REGEX = /^[A-Za-z0-9+/]+=*$/; + +/** + * Decoded-byte size bands for the private-key formats the payment APIs accept. + * + * Ed25519 (CoinbaseCDP apiKeySecret): raw seed is 32 bytes, PKCS8-wrapped ~48 + * bytes, and Coinbase's dashboard ships a 64-byte seed+public-key secret — + * all well under the P-256 range, so Ed25519 needs its own band. + * + * EC P-256 (CoinbaseCDP walletSecret, StripePrivy authorizationPrivateKey): + * PKCS8 is 138 bytes and SEC1 ~121 bytes; the 100–200 band covers both with + * headroom and matches the previously shipped authorizationPrivateKey check. + */ +const ED25519_MIN_BYTES = 32; +const ED25519_MAX_BYTES = 64; +const P256_MIN_BYTES = 100; +const P256_MAX_BYTES = 200; + +/** AWS docs ship the StripePrivy authorization key with this prefix. */ +export const WALLET_AUTH_PREFIX = 'wallet-auth:'; + +function decodeBase64(value: string): Buffer | null { + if (!BASE64_REGEX.test(value)) return null; + return Buffer.from(value, 'base64'); +} + +/** + * Validate the CoinbaseCDP API key secret: a base64-encoded Ed25519 private key. + */ +export function validateApiKeySecret(value: string): true | string { + const trimmed = value.trim(); + const decoded = decodeBase64(trimmed); + if (!decoded) { + return 'apiKeySecret must be a base64-encoded Ed25519 private key'; + } + if (decoded.length < ED25519_MIN_BYTES || decoded.length > ED25519_MAX_BYTES) { + return 'apiKeySecret must be a base64-encoded Ed25519 private key (unexpected length)'; + } + return true; +} + +/** + * Validate the CoinbaseCDP wallet secret: a base64-encoded EC P-256 private key. + */ +export function validateWalletSecret(value: string): true | string { + const trimmed = value.trim(); + const decoded = decodeBase64(trimmed); + if (!decoded) { + return 'walletSecret must be a base64-encoded EC P-256 private key'; + } + if (decoded.length < P256_MIN_BYTES || decoded.length > P256_MAX_BYTES) { + return 'walletSecret must be a base64-encoded EC P-256 private key (unexpected length)'; + } + return true; +} + +/** + * Strip the optional `wallet-auth:` prefix from a StripePrivy authorization + * private key. Both the CLI and TUI normalize with this before validating and + * persisting so the stored value is the bare base64 key. + */ +export function stripWalletAuthPrefix(value: string): string { + const trimmed = value.trim(); + return trimmed.startsWith(WALLET_AUTH_PREFIX) ? trimmed.slice(WALLET_AUTH_PREFIX.length) : trimmed; +} + +/** + * Validate the StripePrivy authorization private key: a base64-encoded EC P-256 + * private key, accepting the optional `wallet-auth:` prefix. + */ +export function validateAuthorizationPrivateKey(value: string): true | string { + const key = stripWalletAuthPrefix(value); + const decoded = decodeBase64(key); + if (!decoded) { + return 'authorizationPrivateKey must be base64-encoded'; + } + if (decoded.length < P256_MIN_BYTES || decoded.length > P256_MAX_BYTES) { + return 'authorizationPrivateKey must be a base64-encoded EC P-256 private key (unexpected length)'; + } + return true; +} diff --git a/src/cli/tui/screens/payment/AddPaymentConnectorScreen.tsx b/src/cli/tui/screens/payment/AddPaymentConnectorScreen.tsx index a238b3c70..12a81f5ab 100644 --- a/src/cli/tui/screens/payment/AddPaymentConnectorScreen.tsx +++ b/src/cli/tui/screens/payment/AddPaymentConnectorScreen.tsx @@ -1,5 +1,10 @@ import type { PaymentProvider } from '../../../../schema'; import { PaymentConnectorNameSchema } from '../../../../schema'; +import { + validateApiKeySecret, + validateAuthorizationPrivateKey, + validateWalletSecret, +} from '../../../primitives/payment-validation'; import { ConfirmReview, Panel, Screen, SecretInput, StepIndicator, TextInput, WizardSelect } from '../../components'; import type { SelectableItem } from '../../components'; import { HELP_TEXT } from '../../constants'; @@ -149,7 +154,7 @@ export function AddPaymentConnectorScreen({ prompt="CDP API Key Secret" onSubmit={wizard.setApiKeySecret} onCancel={goBackOrExit} - customValidation={value => value.trim().length > 0 || 'API Key Secret is required'} + customValidation={validateApiKeySecret} revealChars={4} /> )} @@ -160,7 +165,7 @@ export function AddPaymentConnectorScreen({ prompt="CDP Wallet Secret" onSubmit={wizard.setWalletSecret} onCancel={goBackOrExit} - customValidation={value => value.trim().length > 0 || 'Wallet Secret is required'} + customValidation={validateWalletSecret} revealChars={4} /> )} @@ -193,7 +198,7 @@ export function AddPaymentConnectorScreen({ prompt="Authorization Private Key (ECDSA P-256)" onSubmit={wizard.setAuthorizationPrivateKey} onCancel={goBackOrExit} - customValidation={value => value.trim().length > 0 || 'Authorization Private Key is required'} + customValidation={validateAuthorizationPrivateKey} revealChars={4} /> )} diff --git a/src/cli/tui/screens/payment/useAddPaymentWizard.ts b/src/cli/tui/screens/payment/useAddPaymentWizard.ts index 589acbd8e..7665d8018 100644 --- a/src/cli/tui/screens/payment/useAddPaymentWizard.ts +++ b/src/cli/tui/screens/payment/useAddPaymentWizard.ts @@ -1,4 +1,5 @@ import type { PaymentAuthorizerType, PaymentProvider } from '../../../../schema'; +import { stripWalletAuthPrefix } from '../../../primitives/payment-validation'; import type { AddPaymentConnectorConfig, AddPaymentConnectorStep, @@ -286,9 +287,7 @@ export function useAddPaymentConnectorWizard(preSelectedManager?: string) { const setAuthorizationPrivateKey = useCallback( (authorizationPrivateKey: string) => { // AWS docs ship the key with a `wallet-auth:` prefix — strip it transparently. - const cleaned = authorizationPrivateKey.startsWith('wallet-auth:') - ? authorizationPrivateKey.slice('wallet-auth:'.length) - : authorizationPrivateKey; + const cleaned = stripWalletAuthPrefix(authorizationPrivateKey); setConfig(c => ({ ...c, authorizationPrivateKey: cleaned })); advanceFrom('authorization-private-key'); },