fix: slippage bounds, audit-log IP spoofing, idempotency key cleanup

prisma/migrations/20260901000000_add_idempotency_expires_index/migration.sql

@@ -0,0 +1,5 @@
+-- CreateIndex
+CREATE INDEX "Payment_idempotencyKeyExpiresAt_idx" ON "Payment"("idempotencyKeyExpiresAt");
+
+-- CreateIndex
+CREATE INDEX "Settlement_idempotencyKeyExpiresAt_idx" ON "Settlement"("idempotencyKeyExpiresAt");

prisma/schema.prisma

@@ -76,6 +76,8 @@ model Payment {
   fxQuoteExpiresAt        DateTime?
 
   @@index([merchantId, status, createdAt(sort: Desc)])
+  // #622 — supports the idempotency-key cleanup job's scan for expired keys.
+  @@index([idempotencyKeyExpiresAt])
 }
 
 model Settlement {
@@ -112,6 +114,8 @@ model Settlement {
   @@index([merchantId, initiatedAt(sort: Desc)])
   @@index([merchantId, status, initiatedAt(sort: Desc)])
   @@index([supersededById])
+  // #622 — supports the idempotency-key cleanup job's scan for expired keys.
+  @@index([idempotencyKeyExpiresAt])
 }
 
 model IndexedEvent {

services/api-gateway/src/idempotency-key-cleanup-cron.test.ts

@@ -0,0 +1,146 @@
+import test from 'tape';
+import { reclaimExpiredIdempotencyKeys, getCronIntervalMs } from './idempotency-key-cleanup-cron.js';
+
+interface MockPrisma {
+  payment: {
+    updateMany: (args: { where: any; data: any }) => Promise<{ count: number }>;
+  };
+  settlement: {
+    updateMany: (args: { where: any; data: any }) => Promise<{ count: number }>;
+  };
+}
+
+interface MockLogger {
+  info: (obj: any, msg?: string) => void;
+  error: (obj: any, msg?: string) => void;
+  warn: (obj: any, msg?: string) => void;
+}
+
+let lastPaymentArgs: { where: any; data: any } | null = null;
+let lastSettlementArgs: { where: any; data: any } | null = null;
+
+function createMockPrisma(paymentCount: number, settlementCount: number): MockPrisma {
+  return {
+    payment: {
+      updateMany: async (args) => {
+        lastPaymentArgs = args;
+        return { count: paymentCount };
+      },
+    },
+    settlement: {
+      updateMany: async (args) => {
+        lastSettlementArgs = args;
+        return { count: settlementCount };
+      },
+    },
+  };
+}
+
+function createMockLogger(): MockLogger {
+  return { info: () => {}, error: () => {}, warn: () => {} };
+}
+
+test('reclaimExpiredIdempotencyKeys - nulls out expired keys on both Payment and Settlement', async (t) => {
+  lastPaymentArgs = null;
+  lastSettlementArgs = null;
+  const mockPrisma = createMockPrisma(2, 1);
+  const mockLogger = createMockLogger();
+
+  const total = await reclaimExpiredIdempotencyKeys(mockPrisma as any, mockLogger as any);
+
+  t.equal(total, 3, 'returns the combined reclaimed count across both tables');
+
+  t.ok(lastPaymentArgs, 'Payment.updateMany should have been called');
+  t.deepEqual(lastPaymentArgs?.data, { idempotencyKey: null, idempotencyKeyExpiresAt: null }, 'clears the key and its expiry on Payment');
+  t.ok(lastPaymentArgs?.where.idempotencyKeyExpiresAt.lt instanceof Date, 'Payment filter compares expiresAt against now');
+  t.equal(lastPaymentArgs?.where.idempotencyKey.not, null, 'Payment filter only targets rows that still have a key set');
+
+  t.ok(lastSettlementArgs, 'Settlement.updateMany should have been called');
+  t.deepEqual(lastSettlementArgs?.data, { idempotencyKey: null, idempotencyKeyExpiresAt: null }, 'clears the key and its expiry on Settlement');
+  t.ok(lastSettlementArgs?.where.idempotencyKeyExpiresAt.lt instanceof Date, 'Settlement filter compares expiresAt against now');
+
+  t.end();
+});
+
+test('reclaimExpiredIdempotencyKeys - a row with expiresAt in the future is not targeted (time-mocked)', async (t) => {
+  lastPaymentArgs = null;
+  lastSettlementArgs = null;
+  const mockPrisma = createMockPrisma(0, 0);
+  const mockLogger = createMockLogger();
+
+  const now = Date.now();
+  const futureExpiry = new Date(now + 60 * 60 * 1000); // expires in 1 hour — not yet reclaimable
+
+  const total = await reclaimExpiredIdempotencyKeys(mockPrisma as any, mockLogger as any);
+
+  t.equal(total, 0, 'returns 0 when nothing is expired');
+  const cutoff = (lastPaymentArgs?.where.idempotencyKeyExpiresAt.lt as Date).getTime();
+  t.ok(cutoff <= now + 1000, 'cutoff is "now", so a row expiring an hour from now would not match `lt` cutoff');
+  t.ok(futureExpiry.getTime() > cutoff, 'sanity check: the future-expiring row is after the cutoff used by the query');
+
+  t.end();
+});
+
+test('reclaimExpiredIdempotencyKeys - returns 0 and does not call Prisma when a concurrent run is in progress', async (t) => {
+  lastPaymentArgs = null;
+  lastSettlementArgs = null;
+  const mockPrisma = createMockPrisma(5, 5);
+  const mockLogger = createMockLogger();
+
+  // Kick off a slow first run without awaiting it, then immediately try a
+  // second run — it must see isCronRunning and bail out.
+  const slowPrisma: MockPrisma = {
+    payment: {
+      updateMany: async (args) => {
+        lastPaymentArgs = args;
+        await new Promise((resolve) => setTimeout(resolve, 50));
+        return { count: 1 };
+      },
+    },
+    settlement: {
+      updateMany: async (args) => {
+        lastSettlementArgs = args;
+        return { count: 0 };
+      },
+    },
+  };
+
+  const firstRun = reclaimExpiredIdempotencyKeys(slowPrisma as any, mockLogger as any);
+  const secondRunCount = await reclaimExpiredIdempotencyKeys(mockPrisma as any, mockLogger as any);
+
+  t.equal(secondRunCount, 0, 'concurrent second run returns 0 immediately');
+
+  const firstRunCount = await firstRun;
+  t.equal(firstRunCount, 1, 'first run completes normally');
+
+  t.end();
+});
+
+test('reclaimExpiredIdempotencyKeys - skips execution when Redis lock cannot be acquired', async (t) => {
+  lastPaymentArgs = null;
+  const mockPrisma = createMockPrisma(3, 3);
+  const mockLogger = createMockLogger();
+
+  const mockRedisLocked = { set: async () => null };
+
+  const total = await reclaimExpiredIdempotencyKeys(mockPrisma as any, mockLogger as any, mockRedisLocked);
+
+  t.equal(total, 0, 'returns 0 when lock fails');
+  t.equal(lastPaymentArgs, null, 'Payment.updateMany is not called when lock is unavailable');
+
+  t.end();
+});
+
+test('getCronIntervalMs - reads options or environment variable', (t) => {
+  t.equal(getCronIntervalMs({ intervalMs: 30000 }), 30000, 'reads intervalMs option');
+
+  const originalEnv = process.env.IDEMPOTENCY_KEY_CLEANUP_CRON_INTERVAL_MS;
+  process.env.IDEMPOTENCY_KEY_CLEANUP_CRON_INTERVAL_MS = '120000';
+  t.equal(getCronIntervalMs(), 120000, 'reads env variable');
+
+  delete process.env.IDEMPOTENCY_KEY_CLEANUP_CRON_INTERVAL_MS;
+  t.equal(getCronIntervalMs(), 3600000, 'defaults to 1 hour (3600000ms)');
+
+  process.env.IDEMPOTENCY_KEY_CLEANUP_CRON_INTERVAL_MS = originalEnv;
+  t.end();
+});

services/api-gateway/src/idempotency-key-cleanup-cron.ts

@@ -0,0 +1,141 @@
+/**
+ * idempotency-key-cleanup-cron.ts (issue #622)
+ *
+ * Payment.idempotencyKey and Settlement.idempotencyKey are @unique so a
+ * stale row's key value permanently blocks a client from reusing that same
+ * key once its 24h idempotencyKeyExpiresAt window has passed — the DB
+ * uniqueness constraint doesn't know the key expired, only the application's
+ * `idempotencyKeyExpiresAt: { gt: now }` lookup does. This job periodically
+ * reclaims expired keys by nulling them out, freeing the value for reuse.
+ */
+
+import type { PrismaClient } from '@prisma/client';
+import type { FastifyLoggerInstance } from 'fastify';
+
+let cronInterval: NodeJS.Timeout | null = null;
+let isCronRunning = false;
+
+export interface CronOptions {
+  intervalMs?: number;
+  jitterMs?: number;
+  redis?: any;
+}
+
+export function getCronIntervalMs(opts?: CronOptions): number {
+  if (opts?.intervalMs && opts.intervalMs > 0) return opts.intervalMs;
+  const envVal = process.env.IDEMPOTENCY_KEY_CLEANUP_CRON_INTERVAL_MS;
+  if (envVal) {
+    const parsed = parseInt(envVal, 10);
+    if (!isNaN(parsed) && parsed > 0) return parsed;
+  }
+  return 60 * 60 * 1000;
+}
+
+/**
+ * Lua script for atomic lock release — only deletes the lock if the stored
+ * value matches what we wrote, preventing accidental deletion of a lock
+ * acquired by another gateway instance.
+ */
+const RELEASE_LOCK_SCRIPT = `
+  if redis.call("get", KEYS[1]) == ARGV[1] then
+    return redis.call("del", KEYS[1])
+  else
+    return 0
+  end
+`;
+
+/**
+ * Nulls out idempotencyKey (and its expiry) on Payment and Settlement rows
+ * whose idempotencyKeyExpiresAt has passed, so the @unique constraint no
+ * longer blocks a client from reusing that key value.
+ *
+ * @returns the total number of rows reclaimed across both tables.
+ */
+export async function reclaimExpiredIdempotencyKeys(
+  prisma: PrismaClient,
+  logger: FastifyLoggerInstance,
+  redis?: any,
+): Promise<number> {
+  if (isCronRunning) {
+    logger.info('Idempotency key cleanup cron job already in progress; skipping execution.');
+    return 0;
+  }
+
+  let lockAcquired = false;
+  const lockKey = 'lock:idempotency-key-cleanup-cron';
+  const lockVal = Math.random().toString(36).substring(2);
+  const lockTtlMs = 5 * 60 * 1000;
+
+  if (redis && typeof redis.set === 'function') {
+    try {
+      const res = await redis.set(lockKey, lockVal, 'PX', lockTtlMs, 'NX');
+      if (res !== 'OK' && res !== true && res !== '1') {
+        logger.info('Idempotency key cleanup cron lock held by another instance; skipping run.');
+        return 0;
+      }
+      lockAcquired = true;
+    } catch (err) {
+      logger.warn({ err }, 'Failed to acquire Redis lock for idempotency key cleanup cron');
+    }
+  }
+
+  isCronRunning = true;
+  try {
+    const now = new Date();
+    const where = {
+      idempotencyKey: { not: null },
+      idempotencyKeyExpiresAt: { lt: now },
+    };
+    const data = { idempotencyKey: null, idempotencyKeyExpiresAt: null };
+
+    const [payments, settlements] = await Promise.all([
+      prisma.payment.updateMany({ where, data }),
+      prisma.settlement.updateMany({ where, data }),
+    ]);
+
+    const total = payments.count + settlements.count;
+    if (total > 0) {
+      logger.info(
+        { paymentsReclaimed: payments.count, settlementsReclaimed: settlements.count },
+        'Reclaimed expired idempotency keys.',
+      );
+    }
+    return total;
+  } catch (error) {
+    logger.error({ err: error }, 'Error during idempotency key cleanup cron job.');
+    return 0;
+  } finally {
+    isCronRunning = false;
+    if (lockAcquired && redis && typeof redis.eval === 'function') {
+      await redis.eval(RELEASE_LOCK_SCRIPT, 1, lockKey, lockVal).catch(() => {});
+    }
+  }
+}
+
+export function startIdempotencyKeyCleanupCron(
+  prisma: PrismaClient,
+  logger: FastifyLoggerInstance,
+  opts?: CronOptions,
+) {
+  if (cronInterval) return;
+
+  const intervalMs = getCronIntervalMs(opts);
+
+  const runJob = async () => {
+    const jitter = Math.floor(Math.random() * (opts?.jitterMs ?? 1000));
+    if (jitter > 0) {
+      await new Promise((resolve) => setTimeout(resolve, jitter));
+    }
+    await reclaimExpiredIdempotencyKeys(prisma, logger, opts?.redis).catch((err) =>
+      logger.error({ err }, 'Idempotency key cleanup cron job failed unexpectedly.'),
+    );
+  };
+
+  cronInterval = setInterval(runJob, intervalMs);
+}
+
+export function stopIdempotencyKeyCleanupCron() {
+  if (cronInterval) clearInterval(cronInterval);
+  cronInterval = null;
+  isCronRunning = false;
+}

services/api-gateway/src/index.ts

@@ -115,6 +115,10 @@ import {
   startAbandonedPaymentsCron,
   stopAbandonedPaymentsCron,
 } from "./abandoned-payments-cron.js";
+import {
+  startIdempotencyKeyCleanupCron,
+  stopIdempotencyKeyCleanupCron,
+} from "./idempotency-key-cleanup-cron.js";
 import {
   createWebhookQueue,
   type WebhookJobData,
@@ -3185,6 +3189,7 @@ async function shutdown(signal: string) {
     }
     await getDefaultPrisma().$disconnect();
     stopAbandonedPaymentsCron();
+    stopIdempotencyKeyCleanupCron();
     process.exit(0);
   } catch (err) {
     app.log.error(err, "Error during shutdown");
@@ -3221,6 +3226,7 @@ const start = async () => {
         (env as any).PAYMENT_ABANDONMENT_HOURS ?? 24,
         webhookQueue,
       );
+      startIdempotencyKeyCleanupCron(prisma, app.log, { redis });
     }
     await app.listen({ port: PORT, host: "0.0.0.0" });
   } catch (err) {

services/fx-engine/src/index.ts

@@ -1250,6 +1250,14 @@ const QuoteQuerySchema = z.object({
   slippageBps: z
     .string()
     .regex(/^\d+$/, "slippageBps must be a non-negative integer")
+    // Issue #620: without an upper bound, a merchant could submit an
+    // arbitrarily large slippageBps (e.g. 10000 = 100%) and get silently
+    // clamped to env.MAX_SLIPPAGE_BPS below with no error signal — the
+    // request looked accepted while their actual tolerance was ignored.
+    // Reject out-of-range values outright instead.
+    .refine((val) => parseInt(val, 10) <= 1000, {
+      message: "slippageBps must be between 0 and 1000",
+    })
     .optional(),
 });
 

services/fx-engine/src/quote-slippage-bounds.test.ts

@@ -0,0 +1,41 @@
+process.env.NODE_ENV = 'test';
+process.env.RATES_API_URL = 'http://localhost:1234';
+process.env.REDIS_URL = 'redis://localhost:6379';
+process.env.INTER_SERVICE_SECRET = 'test-secret-that-is-at-least-16-chars';
+process.env.MAX_STALE_SECONDS = '300';
+process.env.LOG_LEVEL = 'silent';
+
+import test from 'tape';
+
+const { fastify } = await import('./index.js');
+
+// Issue #620 — a merchant submitting an out-of-range slippageBps used to be
+// silently clamped to env.MAX_SLIPPAGE_BPS with no error signal. It must now
+// be rejected with 400 instead.
+
+test('GET /api/quote rejects slippageBps=10000 (100%) with 400', async (t) => {
+  await fastify.ready();
+
+  const res = await fastify.inject({
+    method: 'GET',
+    url: '/api/quote?from=USDC&to=NGN&amount=100&slippageBps=10000',
+  });
+
+  t.equal(res.statusCode, 400, 'rejects slippageBps above the allowed 0-1000 range');
+  t.end();
+});
+
+test('GET /api/quote accepts slippageBps within the allowed range and echoes slippageLimit', async (t) => {
+  const res = await fastify.inject({
+    method: 'GET',
+    url: '/api/quote?from=USDC&to=NGN&amount=100&slippageBps=250',
+  });
+
+  t.equal(res.statusCode, 200, 'accepts an in-range slippageBps');
+  const body = JSON.parse(res.body);
+  t.equal(body.slippageBps, 250, 'echoes the validated slippageBps');
+  t.equal(body.slippageLimit, (250 / 10_000).toFixed(4), 'slippageLimit matches slippageBps/10000');
+
+  t.end();
+  process.exit(0);
+});