Strengthen payment security, gateway safety, and correctness

backend/src/integrations/__tests__/gateway-adapter-result.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest';
 import {
-  BaseGatewayAdapter, type GatewayConfig, type GatewayType,
+  BaseGatewayAdapter, GatewayCircuitOpenError, GatewayManager, type GatewayConfig, type GatewayType,
   type PaymentRequest, type PaymentResponse,
 } from '../gateway-adapter.js';
 
@@ -44,3 +44,53 @@ describe('gateway result accounting (#1177)', () => {
     expect(gateway.getHealthStatus().successRate).toBe(0.5);
   });
 });
+
+
+describe('gateway manager safe fallback (#1182)', () => {
+  it('does not replay a payment through another gateway after an ambiguous provider error', async () => {
+    let firstCalls = 0;
+    let fallbackCalls = 0;
+    const first = {
+      getType: () => 'stripe' as const,
+      processPayment: async () => { firstCalls++; throw new Error('provider timeout'); },
+      checkHealth: async () => true,
+      getHealthStatus: () => ({ gatewayType: 'stripe' as const, isHealthy: true, lastCheckAt: new Date().toISOString(), responseTimeMs: 1, errorRate: 0, successRate: 1, consecutiveFailures: 0 }),
+    };
+    const fallback = {
+      getType: () => 'paypal' as const,
+      processPayment: async () => { fallbackCalls++; return { success: true, transactionId: 'tx2', status: 'completed' as const }; },
+      checkHealth: async () => true,
+      getHealthStatus: () => ({ gatewayType: 'paypal' as const, isHealthy: true, lastCheckAt: new Date().toISOString(), responseTimeMs: 1, errorRate: 0, successRate: 1, consecutiveFailures: 0 }),
+    };
+    const manager = new GatewayManager();
+    manager.registerAdapter(first);
+    manager.registerAdapter(fallback);
+
+    await expect(manager.processPayment(request)).rejects.toThrow('provider timeout');
+    expect(firstCalls).toBe(1);
+    expect(fallbackCalls).toBe(0);
+  });
+
+  it('permits fallback only when a gateway rejects before calling its provider due to an open circuit', async () => {
+    let fallbackCalls = 0;
+    const openCircuit = {
+      getType: () => 'stripe' as const,
+      processPayment: async () => { throw new GatewayCircuitOpenError('circuit is open'); },
+      checkHealth: async () => false,
+      getHealthStatus: () => ({ gatewayType: 'stripe' as const, isHealthy: true, lastCheckAt: new Date().toISOString(), responseTimeMs: 1, errorRate: 0, successRate: 1, consecutiveFailures: 0 }),
+    };
+    const fallback = {
+      getType: () => 'paypal' as const,
+      processPayment: async () => { fallbackCalls++; return { success: true, transactionId: 'tx2', status: 'completed' as const }; },
+      checkHealth: async () => true,
+      getHealthStatus: () => ({ gatewayType: 'paypal' as const, isHealthy: true, lastCheckAt: new Date().toISOString(), responseTimeMs: 1, errorRate: 0, successRate: 1, consecutiveFailures: 0 }),
+    };
+    const manager = new GatewayManager();
+    manager.registerAdapter(openCircuit);
+    manager.registerAdapter(fallback);
+
+    const result = await manager.processPayment(request);
+    expect(result.gatewayUsed).toBe('paypal');
+    expect(fallbackCalls).toBe(1);
+  });
+});

backend/src/integrations/gateway-adapter.ts

@@ -112,7 +112,7 @@ export abstract class BaseGatewayAdapter implements IGatewayAdapter {
       if (this.shouldAttemptReset()) {
         this.circuitState = CircuitState.HalfOpen;
       } else {
-        throw new Error(
+        throw new GatewayCircuitOpenError(
           `Gateway ${this.config.gatewayType} circuit is open. Try again later.`,
         );
       }
@@ -268,6 +268,17 @@ export abstract class BaseGatewayAdapter implements IGatewayAdapter {
   }
 }
 
+/**
+ * Indicates that a gateway rejected the request before invoking its provider.
+ * This is the only failure class that permits automatic cross-gateway fallback.
+ */
+export class GatewayCircuitOpenError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = 'GatewayCircuitOpenError';
+  }
+}
+
 /**
  * Gateway manager with failover support
  */
@@ -315,7 +326,15 @@ export class GatewayManager {
         };
       } catch (error) {
         lastError = error instanceof Error ? error : new Error('Unknown error');
-        // Continue to next gateway
+
+        // Provider/transport errors can be ambiguous: the provider may have charged
+        // even when its response was lost. Do not retry on another gateway unless
+        // the failure is known to have happened before a provider call (open circuit).
+        if (!(error instanceof GatewayCircuitOpenError)) {
+          throw lastError;
+        }
+        // An open circuit rejects before executePayment, so trying another adapter
+        // cannot duplicate a payment that this adapter may have processed.
       }
     }
 

backend/src/middleware/__tests__/payout-security-validation.test.ts

@@ -0,0 +1,62 @@
+import { describe, expect, it, vi } from 'vitest';
+import type { Request, Response } from 'express';
+import { payoutSecurityMiddleware, DEFAULT_SECURITY_CONFIG } from '../payout-security.js';
+
+function makeRequest(body: unknown): Request {
+  return {
+    body,
+    headers: {},
+    socket: { remoteAddress: '203.0.113.10' },
+  } as unknown as Request;
+}
+
+function makeResponse() {
+  let statusCode = 200;
+  let responseBody: unknown;
+  const res = {
+    status: vi.fn((code: number) => { statusCode = code; return res; }),
+    json: vi.fn((body: unknown) => { responseBody = body; return res; }),
+  } as unknown as Response;
+  return { res, get statusCode() { return statusCode; }, get responseBody() { return responseBody; } };
+}
+
+describe('payout security input validation', () => {
+  const middleware = payoutSecurityMiddleware({
+    ...DEFAULT_SECURITY_CONFIG,
+    enableGeoChecks: false,
+    enableVelocityChecks: false,
+  });
+
+  it.each([
+    [{ merchantId: 'merchant-a', amount: -1 }],
+    [{ merchantId: 'merchant-a', amount: 0 }],
+    [{ merchantId: 'merchant-a', amount: 'NaN' }],
+    [{ merchantId: 'merchant-a', amount: Infinity }],
+    [{ merchantId: '', amount: 1 }],
+    [{ merchantId: 'x'.repeat(129), amount: 1 }],
+    [{ merchantId: 'merchant-a', amount: 1_000_001 }],
+    [null],
+  ])('rejects malformed payout input before continuing', async (body) => {
+    const req = makeRequest(body);
+    const { res } = makeResponse();
+    const next = vi.fn();
+
+    await middleware(req, res, next);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+      error: expect.objectContaining({ code: 'INVALID_PAYOUT_REQUEST' }),
+    }));
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('allows valid payout input to continue', async () => {
+    const req = makeRequest({ merchantId: 'merchant-valid', amount: 10 });
+    const { res } = makeResponse();
+    const next = vi.fn();
+
+    await middleware(req, res, next);
+
+    expect(next).toHaveBeenCalledOnce();
+  });
+});

backend/src/middleware/payout-security.ts

@@ -100,14 +100,40 @@ const attemptStore = new PayoutAttemptStore();
 export function payoutSecurityMiddleware(config: SecurityConfig = DEFAULT_SECURITY_CONFIG) {
   return async (req: Request, res: Response, next: NextFunction): Promise<void> => {
     try {
-      const { merchantId, amount, asset } = req.body;
-      const ipAddress = (req.headers['x-forwarded-for'] as string) || req.socket.remoteAddress || 'unknown';
+      const { merchantId, amount } = req.body ?? {};
+      const numericAmount = typeof amount === 'number' ? amount : Number(amount);
+
+      // Reject malformed payout inputs before recording attempts or running fraud
+      // checks. In particular, NaN/Infinity/negative values must never enter the
+      // velocity totals, and an untrusted forwarded-for chain is not an IP address.
+      if (
+        typeof merchantId !== 'string' ||
+        merchantId.trim().length === 0 ||
+        merchantId.length > 128 ||
+        amount === null ||
+        amount === undefined ||
+        (typeof amount !== 'number' && typeof amount !== 'string') ||
+        (typeof amount === 'string' && amount.trim() === '') ||
+        !Number.isFinite(numericAmount) ||
+        numericAmount <= 0 ||
+        numericAmount > 1_000_000
+      ) {
+        res.status(400).json({
+          error: { code: 'INVALID_PAYOUT_REQUEST', message: 'A valid merchantId and positive payout amount are required.' },
+        });
+        return;
+      }
+
+      const forwardedFor = req.headers['x-forwarded-for'];
+      const ipAddress = (typeof forwardedFor === 'string' ? forwardedFor.split(',')[0].trim() : undefined)
+        || req.socket.remoteAddress
+        || 'unknown';
       const userAgent = req.headers['user-agent'] || 'unknown';
 
-      // Record attempt
+      // Record only validated attempts so rejected input cannot poison daily limits.
       const attempt: PayoutAttempt = {
         merchantId,
-        amount: Number(amount),
+        amount: numericAmount,
         timestamp: new Date(),
         ipAddress,
         userAgent,

docs/GATEWAY_FAILURE_SEMANTICS.md

@@ -20,12 +20,14 @@ half-open until another threshold is reached.
 
 ## Safe fallback and idempotency
 
-Do not automatically replay a payment against another provider solely because
-the response is a definitive decline/failure: the provider may already have
-processed the request. Failover after ambiguous transport errors requires a
-shared idempotency key or reconciliation that can establish the original
-outcome. Keep the same payment identifier and idempotency key across safe
-retries.
+Do not automatically replay a payment against another provider after a provider
+or transport error: the provider may already have processed the request even if its
+response was lost. The gateway manager only falls back automatically when an adapter
+rejects before calling its provider because its circuit is open. Other errors are
+returned to the caller for reconciliation. If an application adds retry after an
+ambiguous outcome, it must first reconcile the original transaction or use a provider
+contract that guarantees cross-gateway deduplication. Keep the same payment identifier
+and idempotency key across safe retries.
 
 ## Verification
 

docs/security/payment-security.md

@@ -52,6 +52,15 @@ The asset allowlist and the amount ceiling are enforced **in the engine as well
 route**, so a caller that reaches `PayoutEngine` directly cannot pay an unlisted asset or
 an unbounded amount.
 
+## Request validation before fraud accounting
+
+Payout security middleware rejects missing or malformed merchant identifiers and
+amounts before recording an attempt or running velocity checks. Amounts must be finite,
+strictly positive, and no greater than 1,000,000. This prevents invalid values from
+poisoning daily amount totals or bypassing fraud checks. When forwarding headers are
+used, only the first `X-Forwarded-For` address is considered; deployments must only trust
+forwarded headers from their configured reverse proxy.
+
 ## Rate limiting
 
 The payouts router carries its own ceilings **on top of** the global `/api` limiter