diff --git a/backend/docs/ISSUE_1174_WEBHOOK_CORRECTNESS.md b/backend/docs/ISSUE_1174_WEBHOOK_CORRECTNESS.md new file mode 100644 index 00000000..c481d48b --- /dev/null +++ b/backend/docs/ISSUE_1174_WEBHOOK_CORRECTNESS.md @@ -0,0 +1,16 @@ +# Payment webhook correctness (#1174) + +This increment protects fiat payment state against duplicate and out-of-order bank webhook delivery. + +- Duplicate state notifications have no timestamp or event-history side effects. +- Only `processing -> settled/failed/returned` and `settled -> returned` are accepted. +- Provider events cannot bypass manual approval or compliance holds. +- Invalid transitions raise `INVALID_PAYMENT_STATUS_TRANSITION`. + +Run regression tests from `backend`: + +```sh +npx vitest run src/services/__tests__/fiat-webhook-correctness.test.ts +``` + +Operational note: The webhook HTTP handler should translate invalid transitions to a suitable permanent response or acknowledgement to avoid repeated provider redelivery. diff --git a/backend/docs/ISSUE_1177_GATEWAY_ACCOUNTING.md b/backend/docs/ISSUE_1177_GATEWAY_ACCOUNTING.md new file mode 100644 index 00000000..b176f532 --- /dev/null +++ b/backend/docs/ISSUE_1177_GATEWAY_ACCOUNTING.md @@ -0,0 +1,16 @@ +# Gateway response accounting (#1177) + +This increment corrects the health metrics and circuit breaker behavior in `BaseGatewayAdapter`. + +- Returned payment responses with `success: false` or `status: failed` count as failures, even when the provider call does not throw. +- Repeated failed responses open the circuit at the configured threshold. +- A subsequent successful response resets the consecutive-failure count. +- The adapter retains its existing response contract rather than throwing for returned payment failures. + +Run regression tests from `backend`: + +```sh +npx vitest run src/integrations/__tests__/gateway-adapter-result.test.ts +``` + +The `GatewayManager` does not fail over for returned failed response objects; adjusting its retry/failover policy requires separate safeguards against ambiguous outcomes and duplicate charges. diff --git a/backend/src/integrations/__tests__/gateway-adapter-result.test.ts b/backend/src/integrations/__tests__/gateway-adapter-result.test.ts new file mode 100644 index 00000000..16f7a341 --- /dev/null +++ b/backend/src/integrations/__tests__/gateway-adapter-result.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest'; +import { + BaseGatewayAdapter, type GatewayConfig, type GatewayType, + type PaymentRequest, type PaymentResponse, +} from '../gateway-adapter.js'; + +const config: GatewayConfig = { + gatewayType: 'stripe', enabled: true, priority: 1, maxRetries: 0, + timeoutMs: 1000, healthCheckIntervalMs: 60000, + circuitBreakerThreshold: 2, credentials: {}, +}; + +class StubGateway extends BaseGatewayAdapter { + response: PaymentResponse = { success: false, transactionId: 'tx1', status: 'failed' }; + constructor() { super(config); } + getType(): GatewayType { return 'stripe'; } + protected async executePayment(_request: PaymentRequest): Promise { + return this.response; + } + protected async performHealthCheck(): Promise {} +} + +const request: PaymentRequest = { + amount: '1.00', currency: 'USD', recipient: 'test', idempotencyKey: 'key', +}; + +describe('gateway result accounting (#1177)', () => { + it('records returned failures and opens circuit at threshold', async () => { + const gateway = new StubGateway(); + await gateway.processPayment(request); + expect(gateway.getHealthStatus().consecutiveFailures).toBe(1); + expect(gateway.getHealthStatus().successRate).toBe(0); + await gateway.processPayment(request); + expect(gateway.getHealthStatus().isHealthy).toBe(false); + await expect(gateway.processPayment(request)).rejects.toThrow('circuit is open'); + }); + + it('resets failure streak after successful response', async () => { + const gateway = new StubGateway(); + await gateway.processPayment(request); + gateway.response = { success: true, transactionId: 'tx2', status: 'completed' }; + await gateway.processPayment(request); + expect(gateway.getHealthStatus().consecutiveFailures).toBe(0); + expect(gateway.getHealthStatus().successRate).toBe(0.5); + }); +}); diff --git a/backend/src/integrations/gateway-adapter.ts b/backend/src/integrations/gateway-adapter.ts index ac76cdb2..db2b0197 100644 --- a/backend/src/integrations/gateway-adapter.ts +++ b/backend/src/integrations/gateway-adapter.ts @@ -124,6 +124,19 @@ export abstract class BaseGatewayAdapter implements IGatewayAdapter { const response = await this.executePayment(request); const processingTime = Date.now() - startTime; + // A gateway may resolve successfully at the transport layer while + // returning a failed payment. Such responses must count as failures. + if (!response.success || response.status === 'failed') { + this.recordFailure(processingTime); + if ( + this.health.consecutiveFailures >= this.config.circuitBreakerThreshold || + this.circuitState === CircuitState.HalfOpen + ) { + this.openCircuit(); + } + return response; + } + this.recordSuccess(processingTime); // Close circuit if half-open diff --git a/backend/src/services/__tests__/fiat-webhook-correctness.test.ts b/backend/src/services/__tests__/fiat-webhook-correctness.test.ts new file mode 100644 index 00000000..da3288df --- /dev/null +++ b/backend/src/services/__tests__/fiat-webhook-correctness.test.ts @@ -0,0 +1,49 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { fiatPaymentsService } from '../fiat-payments.js'; + +describe('fiat payment webhook correctness (#1174)', () => { + beforeEach(() => fiatPaymentsService.resetForTests()); + + function makePayment(amount = 100) { + const account = fiatPaymentsService.createBankAccount({ + accountHolderName: 'Test', bankName: 'Test Bank', + accountNumberMasked: '****1111', routingNumber: '021000021', + verificationMethod: 'plaid', countryCode: 'US', + }); + return fiatPaymentsService.createPayment({ + method: 'ach', bankAccountId: account.id, + recipient: { name: 'Payee', accountNumberMasked: '****2222', + routingNumber: '021000021', bankName: 'Bank', countryCode: 'US' }, + amount, currency: 'USD', isInternational: false, + }); + } + + it('prevents settlement from bypassing approval', () => { + const payment = makePayment(20000); + expect(payment.status).toBe('pending_approval'); + expect(() => fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' })) + .toThrow('INVALID_PAYMENT_STATUS_TRANSITION'); + expect(fiatPaymentsService.getPayment(payment.id)?.status).toBe('pending_approval'); + }); + + it('deduplicates settlement and rejects stale processing notifications', () => { + const payment = makePayment(); + fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' }); + const count = fiatPaymentsService.getPaymentEvents(payment.id).length; + fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' }); + expect(fiatPaymentsService.getPaymentEvents(payment.id)).toHaveLength(count); + expect(() => fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'processing' })) + .toThrow('INVALID_PAYMENT_STATUS_TRANSITION'); + expect(fiatPaymentsService.getPayment(payment.id)?.status).toBe('settled'); + }); + + it('allows a bank return after settlement', () => { + const payment = makePayment(); + fiatPaymentsService.handleWebhook({ paymentId: payment.id, eventType: 'settled' }); + const returned = fiatPaymentsService.handleWebhook({ + paymentId: payment.id, eventType: 'returned', reason: 'Bank return', + }); + expect(returned?.status).toBe('returned'); + expect(returned?.returnReason).toBe('Bank return'); + }); +}); diff --git a/backend/src/services/fiat-payments.ts b/backend/src/services/fiat-payments.ts index 1b17cc2a..7f629c9f 100644 --- a/backend/src/services/fiat-payments.ts +++ b/backend/src/services/fiat-payments.ts @@ -352,6 +352,19 @@ class FiatPaymentsService { return undefined; } + // Bank webhook deliveries can be duplicated or arrive out of order. + if (payment.status === input.eventType) return payment; + + // Do not allow provider notifications to bypass approval/compliance holds + // or regress terminal payment states. A bank return after settlement is valid. + const allowedTransitions: Partial>> = { + processing: ['settled', 'failed', 'returned'], + settled: ['returned'], + }; + if (!allowedTransitions[payment.status]?.includes(input.eventType)) { + throw new Error('INVALID_PAYMENT_STATUS_TRANSITION'); + } + payment.status = input.eventType; payment.updatedAt = this.nowIso();