diff --git a/ENV_VARS.md b/ENV_VARS.md index 8348bf0e..5e3841e5 100644 --- a/ENV_VARS.md +++ b/ENV_VARS.md @@ -14,6 +14,7 @@ | VAPID_PRIVATE_KEY | VAPID private key for Web Push API | auto-generated | No | | WS_ENABLED | Enable/disable WebSocket support | true | No | | WS_PORT | WebSocket port | 3001 | No | +| PAYOUT_APPROVAL_THRESHOLD | Payouts at or above this amount require approval | 10000 | No | ## Frontend diff --git a/backend/src/services/__tests__/fiat-payments.test.ts b/backend/src/services/__tests__/fiat-payments.test.ts index 54f7466f..c95bb3bc 100644 --- a/backend/src/services/__tests__/fiat-payments.test.ts +++ b/backend/src/services/__tests__/fiat-payments.test.ts @@ -143,4 +143,48 @@ describe('fiatPaymentsService', () => { expect(report.totals.fees).toBeGreaterThan(0); expect(report.byMethod.ach.count).toBe(1); }); -}); \ No newline at end of file +}); +describe('fiatPaymentsService payment correctness', () => { + beforeEach(() => { + fiatPaymentsService.resetForTests(); + }); + + const createVerifiedAccount = () => + fiatPaymentsService.createBankAccount({ + accountHolderName: 'Correctness Co', + bankName: 'Chase', + accountNumberMasked: '****2222', + routingNumber: '021000021', + verificationMethod: 'plaid', + countryCode: 'US', + }); + + const paymentInput = (bankAccountId: string, amount: number) => ({ + method: 'ach' as const, + bankAccountId, + recipient: { + name: 'Vendor LLC', + accountNumberMasked: '****4444', + routingNumber: '021000021', + bankName: 'Wells Fargo', + countryCode: 'US', + }, + amount, + currency: 'USD', + isInternational: false, + description: 'Invoice payout', + }); + + it.each([0, -50, Number.NaN, Number.POSITIVE_INFINITY])('rejects invalid amount %s', (amount) => { + const account = createVerifiedAccount(); + expect(() => fiatPaymentsService.createPayment(paymentInput(account.id, amount))).toThrow('INVALID_PAYMENT_AMOUNT'); + }); + + it('keeps no payments from a batch when one of its payments is invalid', () => { + const account = createVerifiedAccount(); + const inputs = [paymentInput(account.id, 100), paymentInput(account.id, 0)]; + + expect(() => fiatPaymentsService.createBatch('batch-ref', inputs)).toThrow('INVALID_PAYMENT_AMOUNT'); + expect(fiatPaymentsService.listPayments()).toHaveLength(0); + }); +}); diff --git a/backend/src/services/__tests__/issues-456-459.test.ts b/backend/src/services/__tests__/issues-456-459.test.ts index 74e2a9aa..4af063a6 100644 --- a/backend/src/services/__tests__/issues-456-459.test.ts +++ b/backend/src/services/__tests__/issues-456-459.test.ts @@ -79,7 +79,7 @@ describe('Payout Engine (Issue 457)', () => { expect(batches).toHaveLength(1); expect(batches[0].status).toBe('pending'); - const approved = await engine.approveManualPayout(batches[0].id); + const approved = await engine.approveManualPayout(batches[0].id, { id: 'fin_1', role: 'finance_approver' }); expect(approved).toBe(true); expect(batches[0].status).toBe('completed'); }); diff --git a/backend/src/services/__tests__/stripe-connect.test.ts b/backend/src/services/__tests__/stripe-connect.test.ts new file mode 100644 index 00000000..847ff971 --- /dev/null +++ b/backend/src/services/__tests__/stripe-connect.test.ts @@ -0,0 +1,71 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const stripeMock = vi.hoisted(() => ({ + accounts: { create: vi.fn() }, + accountLinks: { create: vi.fn() }, + paymentIntents: { create: vi.fn() }, + transfers: { create: vi.fn() }, +})); + +vi.mock('../stripe.js', () => ({ getStripe: () => stripeMock })); +vi.mock('../auditService.js', () => ({ auditService: { logAction: vi.fn().mockResolvedValue(undefined) } })); +vi.mock('../../config/env.js', () => ({ config: () => ({ APP_URL: 'https://app.example.com' }) })); + +import { + connectedAccounts, + createConnectedAccount, + createPaymentIntentWithConnect, + createTransfer, + handleConnectWebhook, +} from '../stripe-connect.js'; + +describe('stripe-connect', () => { + beforeEach(() => { + vi.clearAllMocks(); + connectedAccounts.clear(); + }); + + it('does not track a merchant when the onboarding link fails', async () => { + stripeMock.accounts.create.mockResolvedValue({ id: 'acct_1' }); + stripeMock.accountLinks.create.mockRejectedValue(new Error('stripe down')); + + await expect(createConnectedAccount('merch_1', 'owner@example.com')).rejects.toThrow('stripe down'); + expect(connectedAccounts.has('merch_1')).toBe(false); + }); + + it('tracks the merchant once onboarding succeeds', async () => { + stripeMock.accounts.create.mockResolvedValue({ id: 'acct_2' }); + stripeMock.accountLinks.create.mockResolvedValue({ url: 'https://connect.stripe.com/setup' }); + + const result = await createConnectedAccount('merch_2', 'owner@example.com'); + expect(result).toEqual({ accountId: 'acct_2', onboardingUrl: 'https://connect.stripe.com/setup' }); + expect(connectedAccounts.get('merch_2')?.onboardingStatus).toBe('onboarding'); + }); + + it('ignores account.updated webhooks for unknown accounts', async () => { + await expect( + handleConnectWebhook({ type: 'account.updated', data: { object: { id: 'acct_unknown' } } } as never) + ).resolves.toBeUndefined(); + }); + + it.each([0, -100, 10.5, Number.NaN])('rejects non-minor-unit amount %s for payment intents', async (amount) => { + await expect(createPaymentIntentWithConnect(amount, 'usd', 'acct_1')).rejects.toThrow( + 'Amount must be a positive integer in minor units' + ); + expect(stripeMock.paymentIntents.create).not.toHaveBeenCalled(); + }); + + it('rejects non-minor-unit amounts for transfers', async () => { + await expect(createTransfer(0, 'usd', 'acct_1')).rejects.toThrow('Amount must be a positive integer in minor units'); + expect(stripeMock.transfers.create).not.toHaveBeenCalled(); + }); + + it('creates a payment intent with a 0.5% application fee for valid amounts', async () => { + stripeMock.paymentIntents.create.mockResolvedValue({ id: 'pi_1' }); + + await createPaymentIntentWithConnect(10_000, 'USD', 'acct_1'); + expect(stripeMock.paymentIntents.create).toHaveBeenCalledWith( + expect.objectContaining({ amount: 10_000, currency: 'usd', application_fee_amount: 50 }) + ); + }); +}); diff --git a/backend/src/services/fiat-payments.ts b/backend/src/services/fiat-payments.ts index 125495bf..1b17cc2a 100644 --- a/backend/src/services/fiat-payments.ts +++ b/backend/src/services/fiat-payments.ts @@ -199,6 +199,9 @@ class FiatPaymentsService { } createPayment(input: CreatePaymentInput): FiatPaymentRecord { + if (!Number.isFinite(input.amount) || input.amount <= 0) { + throw new Error('INVALID_PAYMENT_AMOUNT'); + } const bankAccount = this.bankAccounts.get(input.bankAccountId); if (!bankAccount) { throw new Error('BANK_ACCOUNT_NOT_FOUND'); @@ -283,7 +286,19 @@ class FiatPaymentsService { } createBatch(reference: string, inputs: CreatePaymentInput[]): { batchId: string; reference: string; payments: FiatPaymentRecord[] } { - const payments = inputs.map((input) => this.createPayment(input)); + // All-or-nothing: if any payment in the batch is rejected, none of the batch is kept. + const paymentIdsBefore = new Set(this.payments.keys()); + const eventCountBefore = this.events.length; + let payments: FiatPaymentRecord[]; + try { + payments = inputs.map((input) => this.createPayment(input)); + } catch (error) { + for (const id of [...this.payments.keys()]) { + if (!paymentIdsBefore.has(id)) this.payments.delete(id); + } + this.events.length = eventCountBefore; + throw error; + } return { batchId: randomUUID(), reference, diff --git a/backend/src/services/payouts/__tests__/payout-engine.test.ts b/backend/src/services/payouts/__tests__/payout-engine.test.ts new file mode 100644 index 00000000..edf4d7dc --- /dev/null +++ b/backend/src/services/payouts/__tests__/payout-engine.test.ts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { DEFAULT_PAYOUT_APPROVAL_THRESHOLD, PayoutApprover, PayoutEngine } from '../payout-engine.js'; + +const finance: PayoutApprover = { id: 'fin_1', role: 'finance_approver' }; +const outsider = { id: 'usr_9', role: 'merchant' } as unknown as PayoutApprover; + +function engineWithSchedule(options?: { approvalThreshold?: number; autoApproveLimit?: number }) { + const engine = new PayoutEngine({ approvalThreshold: options?.approvalThreshold }); + engine.configureSchedule({ + id: 'sch_1', + merchantId: 'merch_1', + scheduleType: 'threshold', + thresholdAmount: 100, + preferredAsset: 'USDC', + autoApproveLimit: options?.autoApproveLimit ?? 500, + }); + return engine; +} + +describe('PayoutEngine approval workflow', () => { + afterEach(() => { + delete process.env.PAYOUT_APPROVAL_THRESHOLD; + }); + + it('uses the default approval threshold when none is configured', () => { + expect(new PayoutEngine().approvalThreshold).toBe(DEFAULT_PAYOUT_APPROVAL_THRESHOLD); + }); + + it('reads the approval threshold from PAYOUT_APPROVAL_THRESHOLD', () => { + process.env.PAYOUT_APPROVAL_THRESHOLD = '2500'; + expect(new PayoutEngine().approvalThreshold).toBe(2500); + }); + + it('rejects an invalid approval threshold', () => { + expect(() => new PayoutEngine({ approvalThreshold: 0 })).toThrow('INVALID_PAYOUT_APPROVAL_THRESHOLD'); + }); + + it('holds payouts at or above the configured threshold for review', async () => { + const engine = engineWithSchedule({ approvalThreshold: 1000, autoApproveLimit: 5000 }); + engine.setPendingAmount('merch_1', 1000); + + const [batch] = await engine.evaluatePayouts(); + expect(batch.status).toBe('pending'); + }); + + it('does not create duplicate batches while a payout awaits approval', async () => { + const engine = engineWithSchedule({ autoApproveLimit: 500 }); + engine.setPendingAmount('merch_1', 600); + + await engine.evaluatePayouts(); + const second = await engine.evaluatePayouts(); + + expect(second).toHaveLength(0); + expect(engine.getPayoutHistory()).toHaveLength(1); + }); + + it('rejects approval from callers without an approver role', async () => { + const engine = engineWithSchedule({ autoApproveLimit: 500 }); + engine.setPendingAmount('merch_1', 600); + const [batch] = await engine.evaluatePayouts(); + + await expect(engine.approveManualPayout(batch.id, outsider)).rejects.toThrow('PAYOUT_APPROVER_FORBIDDEN'); + expect(batch.status).toBe('pending'); + }); + + it('returns reserved funds to the merchant when a payout is rejected', async () => { + const engine = engineWithSchedule({ autoApproveLimit: 500 }); + engine.setPendingAmount('merch_1', 600); + const [batch] = await engine.evaluatePayouts(); + + expect(await engine.rejectManualPayout(batch.id, finance)).toBe(true); + expect(engine.getPendingAmount('merch_1')).toBe(600); + }); + + it('does not allow a rejected payout to be retried around review', async () => { + const engine = engineWithSchedule({ autoApproveLimit: 500 }); + engine.setPendingAmount('merch_1', 600); + const [batch] = await engine.evaluatePayouts(); + await engine.rejectManualPayout(batch.id, finance); + + expect(await engine.retryFailedBatch(batch.id, finance)).toBe(false); + expect(batch.status).toBe('failed'); + }); + + it('records an audit entry for every approval decision', async () => { + const engine = engineWithSchedule({ autoApproveLimit: 500 }); + engine.setPendingAmount('merch_1', 600); + const [batch] = await engine.evaluatePayouts(); + + await engine.approveManualPayout(batch.id, finance); + + const trail = engine.getAuditTrail(batch.id); + expect(trail.map((entry) => entry.action)).toEqual(['created', 'approved']); + expect(trail[1].actorId).toBe('fin_1'); + }); +}); diff --git a/backend/src/services/payouts/payout-engine.ts b/backend/src/services/payouts/payout-engine.ts index eb1a2b1e..297fe9df 100644 --- a/backend/src/services/payouts/payout-engine.ts +++ b/backend/src/services/payouts/payout-engine.ts @@ -1,3 +1,14 @@ +import { randomUUID } from 'node:crypto'; + +export const DEFAULT_PAYOUT_APPROVAL_THRESHOLD = 10_000; + +export type PayoutApproverRole = 'finance_approver' | 'admin'; + +export interface PayoutApprover { + id: string; + role: PayoutApproverRole; +} + export interface PayoutSchedule { id: string; merchantId: string; @@ -9,18 +20,50 @@ export interface PayoutSchedule { export interface PayoutBatch { id: string; + merchantId: string; status: 'pending' | 'executing' | 'completed' | 'failed'; amount: number; asset: string; transactionsCount: number; errorMessage?: string; + rejectedBy?: string; createdAt: Date; } +export interface PayoutAuditEntry { + batchId: string; + action: 'created' | 'approved' | 'rejected' | 'retried'; + actorId: string; + timestamp: Date; + details?: string; +} + +export interface PayoutEngineOptions { + /** Payouts at or above this amount always need an approver. Defaults to PAYOUT_APPROVAL_THRESHOLD or 10,000. */ + approvalThreshold?: number; +} + +const APPROVER_ROLES: readonly PayoutApproverRole[] = ['finance_approver', 'admin']; + +function resolveApprovalThreshold(explicit?: number): number { + const fromEnv = Number(process.env.PAYOUT_APPROVAL_THRESHOLD); + const threshold = explicit ?? (fromEnv > 0 ? fromEnv : DEFAULT_PAYOUT_APPROVAL_THRESHOLD); + if (!Number.isFinite(threshold) || threshold <= 0) { + throw new Error('INVALID_PAYOUT_APPROVAL_THRESHOLD'); + } + return threshold; +} + export class PayoutEngine { private schedules: PayoutSchedule[] = []; private batches: PayoutBatch[] = []; + private auditTrail: PayoutAuditEntry[] = []; private pendingAmounts: Map = new Map(); // merchantId -> amount + readonly approvalThreshold: number; + + constructor(options: PayoutEngineOptions = {}) { + this.approvalThreshold = resolveApprovalThreshold(options.approvalThreshold); + } setPendingAmount(merchantId: string, amount: number) { this.pendingAmounts.set(merchantId, amount); @@ -49,21 +92,21 @@ export class PayoutEngine { } if (triggerPayout) { - const batchId = Math.random().toString(36).substring(7); + const requiresApproval = pending > schedule.autoApproveLimit || pending >= this.approvalThreshold; const batch: PayoutBatch = { - id: batchId, - status: pending > schedule.autoApproveLimit ? 'pending' : 'completed', + id: randomUUID(), + merchantId: schedule.merchantId, + status: requiresApproval ? 'pending' : 'completed', amount: pending, asset: schedule.preferredAsset, transactionsCount: 1, createdAt: new Date(), }; - if (batch.status === 'completed') { - this.pendingAmounts.set(schedule.merchantId, 0); - } - + // Reserve the funds for every batch so the next evaluation cannot create a duplicate payout. + this.pendingAmounts.set(schedule.merchantId, 0); this.batches.push(batch); + this.record(batch.id, 'created', 'system', requiresApproval ? 'Awaiting approval' : 'Auto-approved'); executedBatches.push(batch); } } @@ -71,30 +114,40 @@ export class PayoutEngine { return executedBatches; } - async approveManualPayout(batchId: string): Promise { + async approveManualPayout(batchId: string, approver: PayoutApprover): Promise { + this.assertApprover(approver); const batch = this.batches.find(b => b.id === batchId); if (batch && batch.status === 'pending') { batch.status = 'completed'; + this.record(batchId, 'approved', approver.id); return true; } return false; } - async rejectManualPayout(batchId: string): Promise { + async rejectManualPayout(batchId: string, approver: PayoutApprover): Promise { + this.assertApprover(approver); const batch = this.batches.find(b => b.id === batchId); if (batch && batch.status === 'pending') { batch.status = 'failed'; batch.errorMessage = 'Rejected by manual review'; + batch.rejectedBy = approver.id; + // Return the reserved funds to the merchant's pending balance. + this.pendingAmounts.set(batch.merchantId, this.getPendingAmount(batch.merchantId) + batch.amount); + this.record(batchId, 'rejected', approver.id, batch.errorMessage); return true; } return false; } - async retryFailedBatch(batchId: string): Promise { + async retryFailedBatch(batchId: string, approver: PayoutApprover): Promise { + this.assertApprover(approver); const batch = this.batches.find(b => b.id === batchId); - if (batch && batch.status === 'failed') { - batch.status = 'completed'; + // Rejected payouts are final; a retry would bypass the review that rejected them. + if (batch && batch.status === 'failed' && !batch.rejectedBy) { + batch.status = 'pending'; delete batch.errorMessage; + this.record(batchId, 'retried', approver.id, 'Returned to approval queue'); return true; } return false; @@ -103,4 +156,18 @@ export class PayoutEngine { getPayoutHistory(): PayoutBatch[] { return this.batches; } + + getAuditTrail(batchId?: string): PayoutAuditEntry[] { + return batchId ? this.auditTrail.filter(entry => entry.batchId === batchId) : [...this.auditTrail]; + } + + private assertApprover(approver: PayoutApprover): void { + if (!approver?.id || !APPROVER_ROLES.includes(approver.role)) { + throw new Error('PAYOUT_APPROVER_FORBIDDEN'); + } + } + + private record(batchId: string, action: PayoutAuditEntry['action'], actorId: string, details?: string): void { + this.auditTrail.push({ batchId, action, actorId, timestamp: new Date(), details }); + } } diff --git a/backend/src/services/stripe-connect.ts b/backend/src/services/stripe-connect.ts index 8a2af8c3..dc4188e5 100644 --- a/backend/src/services/stripe-connect.ts +++ b/backend/src/services/stripe-connect.ts @@ -27,6 +27,9 @@ export async function createConnectedAccount(merchantId: string, email: string): capabilities: { card_payments: { requested: true }, transfers: { requested: true } }, }); + // Only track the merchant once onboarding has a link; otherwise a failed link leaves an orphaned record. + const onboardingUrl = await createOnboardingLink(account.id); + const record: ConnectedAccount = { stripeAccountId: account.id, merchantId, @@ -37,8 +40,6 @@ export async function createConnectedAccount(merchantId: string, email: string): }; connectedAccounts.set(merchantId, record); - const onboardingUrl = await createOnboardingLink(account.id); - await auditService.logAction({ action: 'stripe_connect.account_created', resource: 'stripe_connect', resourceId: account.id, details: { merchantId } }); return { accountId: account.id, onboardingUrl }; } @@ -66,8 +67,8 @@ export async function handleConnectWebhook(event: Stripe.Event): Promise { case 'account.updated': { const account = event.data.object as Stripe.Account; const merchantId = findMerchantByAccountId(account.id); - if (merchantId) { - const record = connectedAccounts.get(merchantId)!; + const record = merchantId ? connectedAccounts.get(merchantId) : undefined; + if (merchantId && record) { record.chargesEnabled = account.charges_enabled; record.payoutsEnabled = account.payouts_enabled; if (account.charges_enabled && account.payouts_enabled && record.onboardingStatus !== 'completed') { @@ -90,7 +91,15 @@ function findMerchantByAccountId(stripeAccountId: string): string | undefined { return undefined; } +// Stripe amounts are integers in the currency's smallest unit (e.g. cents). +function assertMinorUnitAmount(amount: number): void { + if (!Number.isSafeInteger(amount) || amount <= 0) { + throw new AppError(400, 'Amount must be a positive integer in minor units', 'VALIDATION_ERROR'); + } +} + export async function createPaymentIntentWithConnect(amount: number, currency: string, merchantStripeAccountId: string): Promise { + assertMinorUnitAmount(amount); const stripe = getStripe(); return stripe.paymentIntents.create({ amount, @@ -102,6 +111,7 @@ export async function createPaymentIntentWithConnect(amount: number, currency: s } export async function createTransfer(amount: number, currency: string, destination: string): Promise { + assertMinorUnitAmount(amount); const stripe = getStripe(); return stripe.transfers.create({ amount, currency: currency.toLowerCase(), destination }); } diff --git a/docs/PAYOUT_APPROVALS.md b/docs/PAYOUT_APPROVALS.md new file mode 100644 index 00000000..61b875ea --- /dev/null +++ b/docs/PAYOUT_APPROVALS.md @@ -0,0 +1,49 @@ +# Payout Approval Workflow + +Large payouts are held for human review instead of executing automatically. + +## When approval is required + +A payout batch is created with status `pending` when either condition is true: + +- the pending amount is greater than the schedule's `autoApproveLimit`, or +- the pending amount is greater than or equal to the approval threshold. + +Otherwise the batch is auto-approved and marked `completed`. + +## Configuring the threshold + +Set `PAYOUT_APPROVAL_THRESHOLD` (see [ENV_VARS.md](../ENV_VARS.md)). If it is unset, the default is `10000`. +The threshold must be a positive number; invalid values cause the engine to fail at startup. + +## Approver roles + +Only these roles may approve, reject, or retry a payout batch: + +| Role | Can approve | Can reject | Can retry | +| ------------------ | ----------- | ---------- | --------- | +| `finance_approver` | Yes | Yes | Yes | +| `admin` | Yes | Yes | Yes | + +Any other caller receives `PAYOUT_APPROVER_FORBIDDEN`. + +## Lifecycle + +```text +created -> pending -> completed (approved) + -> failed (rejected; funds return to the merchant's pending balance) +failed -> pending (retried, back into the approval queue) +``` + +A rejected batch cannot be retried. Create a new payout from the merchant's pending balance instead, +so the rejection cannot be bypassed. + +## Audit trail + +Every decision is recorded with the batch ID, action (`created`, `approved`, `rejected`, `retried`), +actor ID, timestamp, and optional details. Query it with `PayoutEngine.getAuditTrail(batchId?)`. + +## Reserved funds + +When a batch is created, its amount is moved out of the merchant's pending balance, so repeated +evaluations never create a second batch for the same funds.