From 1a15f3b88aa73ca2eca3de7134d60de4993077cf Mon Sep 17 00:00:00 2001 From: Godsmiracle001 Date: Mon, 19 Oct 2026 11:37:38 +0100 Subject: [PATCH] all --- .../__tests__/payout-security.test.ts | 23 ++++++++++++++ backend/src/middleware/payout-security.ts | 20 ++++++++---- .../payouts/__tests__/payout-batcher.test.ts | 31 ++++++++++++++++++- .../src/services/payouts/payout-batcher.ts | 6 ++++ docs/PAYOUT_RELIABILITY.md | 7 ++++- docs/security/payment-security.md | 10 +++++- 6 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 backend/src/middleware/__tests__/payout-security.test.ts diff --git a/backend/src/middleware/__tests__/payout-security.test.ts b/backend/src/middleware/__tests__/payout-security.test.ts new file mode 100644 index 00000000..b99d9e8a --- /dev/null +++ b/backend/src/middleware/__tests__/payout-security.test.ts @@ -0,0 +1,23 @@ +import { createHmac } from 'node:crypto'; +import { describe, expect, it } from 'vitest'; +import { verifyRequestSignature } from '../payout-security.js'; + +describe('payout request signatures', () => { + const payload = JSON.stringify({ merchantId: 'm1', amount: 25 }); + const secret = 'test-secret'; + + it('accepts an HMAC-SHA256 signature, including the sha256= prefix', () => { + const digest = createHmac('sha256', secret).update(payload).digest('hex'); + + expect(verifyRequestSignature(payload, digest, secret)).toBe(true); + expect(verifyRequestSignature(payload, `sha256=${digest}`, secret)).toBe(true); + }); + + it('rejects tampered payloads, secrets, and malformed signatures', () => { + const digest = createHmac('sha256', secret).update(payload).digest('hex'); + + expect(verifyRequestSignature(`${payload}.tampered`, digest, secret)).toBe(false); + expect(verifyRequestSignature(payload, digest, 'wrong-secret')).toBe(false); + expect(verifyRequestSignature(payload, 'not-a-signature', secret)).toBe(false); + }); +}); diff --git a/backend/src/middleware/payout-security.ts b/backend/src/middleware/payout-security.ts index d12ef50f..949e48e4 100644 --- a/backend/src/middleware/payout-security.ts +++ b/backend/src/middleware/payout-security.ts @@ -6,7 +6,7 @@ */ import type { Request, Response, NextFunction } from 'express'; -import { createHash, randomBytes } from 'node:crypto'; +import { createHash, createHmac, randomBytes, timingSafeEqual } from 'node:crypto'; export interface SecurityConfig { /** Maximum payout amount before additional verification */ @@ -262,11 +262,19 @@ export function verifyRequestSignature( signature: string, secret: string ): boolean { - const expectedSignature = createHash('sha256') - .update(payload + secret) - .digest('hex'); - - return signature === expectedSignature; + if (typeof payload !== 'string' || typeof signature !== 'string' || typeof secret !== 'string' || secret.length === 0) { + return false; + } + + // HMAC binds the secret as a key instead of concatenating it into the message. + // Compare decoded bytes in constant time and reject malformed lengths before + // calling timingSafeEqual (which otherwise throws on length mismatch). + const expected = createHmac('sha256', secret).update(payload, 'utf8').digest(); + const provided = signature.replace(/^sha256=/i, ''); + if (!/^[0-9a-f]{64}$/i.test(provided)) return false; + + const providedBytes = Buffer.from(provided, 'hex'); + return providedBytes.length === expected.length && timingSafeEqual(providedBytes, expected); } /** diff --git a/backend/src/services/payouts/__tests__/payout-batcher.test.ts b/backend/src/services/payouts/__tests__/payout-batcher.test.ts index 3d5d0c03..1080b1bf 100644 --- a/backend/src/services/payouts/__tests__/payout-batcher.test.ts +++ b/backend/src/services/payouts/__tests__/payout-batcher.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { PayoutBatcher, type ProviderBatchRequest, type PayoutBatchProvider } from '../payout-batcher.js'; +import { + PayoutBatcher, + startPayoutBatchWorker, + type ProviderBatchRequest, + type PayoutBatchProvider, +} from '../payout-batcher.js'; import { MoneyMovementAuditLog } from '../../../audit/money-movement-audit.js'; import { PayoutAnomalyDetector } from '../payout-anomaly-detector.js'; @@ -290,3 +295,27 @@ describe('PayoutBatcher — Issue #1008', () => { }); }); }); + +describe('startPayoutBatchWorker', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('skips an interval while the previous batch pass is still running', async () => { + vi.useFakeTimers(); + let release!: () => void; + const runDue = vi.fn(() => new Promise((resolve) => { + release = () => resolve(undefined); + })); + const worker = startPayoutBatchWorker({ runDue }, 1_000); + + await vi.advanceTimersByTimeAsync(3_000); + expect(runDue).toHaveBeenCalledTimes(1); + + release(); + await vi.advanceTimersByTimeAsync(1_000); + expect(runDue).toHaveBeenCalledTimes(2); + + worker.stop(); + }); +}); diff --git a/backend/src/services/payouts/payout-batcher.ts b/backend/src/services/payouts/payout-batcher.ts index 3efad972..49c837c5 100644 --- a/backend/src/services/payouts/payout-batcher.ts +++ b/backend/src/services/payouts/payout-batcher.ts @@ -616,12 +616,18 @@ export function startPayoutBatchWorker( batcher: Pick = payoutBatcher, intervalMs: number = DEFAULT_PAYOUT_BATCH_INTERVAL_MS, ): PayoutBatchWorker { + let running = false; + const tick = async (): Promise => { + if (running) return undefined; + running = true; try { return await batcher.runDue(); } catch (error) { console.error('[payouts] Batch run failed', error); return undefined; + } finally { + running = false; } }; diff --git a/docs/PAYOUT_RELIABILITY.md b/docs/PAYOUT_RELIABILITY.md index e962f212..37f1b05e 100644 --- a/docs/PAYOUT_RELIABILITY.md +++ b/docs/PAYOUT_RELIABILITY.md @@ -150,6 +150,11 @@ Amounts must be positive, finite, and at most `1_000_000`. Assets are restricted 4. Alert on `payout.delayed` bursts: a rail that is timing out rather than rejecting will keep every batch in retry for minutes. +The scheduled payout worker also has an in-flight guard. A slow provider call cannot +overlap the next timer tick, and a rejected pass is logged while the worker remains +available for the following tick. This keeps a transient rail slowdown from creating a +second batch pass over the same due payouts. + ## Audit trail Every decision records `(batchId, action, actorId, timestamp, details)` with actions @@ -164,4 +169,4 @@ does not affect engine state. - [Payout approvals](PAYOUT_APPROVALS.md) — the approval thresholds and roles - [Payment correctness](PAYMENT_CORRECTNESS.md) — money arithmetic and idempotency rules -- [Payment security](security/payment-security.md) — authentication and authorization \ No newline at end of file +- [Payment security](security/payment-security.md) — authentication and authorization diff --git a/docs/security/payment-security.md b/docs/security/payment-security.md index d90d054f..6da186c8 100644 --- a/docs/security/payment-security.md +++ b/docs/security/payment-security.md @@ -97,6 +97,14 @@ signal; see [payout reliability](../PAYOUT_RELIABILITY.md#operating-the-queue). - Exhausted retries keep the reservation instead of re-queueing it, which prevents an unattended retry loop against a failing rail. +## Signed payout requests + +The reusable payout security helper verifies `sha256=` (or the equivalent +bare hex digest) as an HMAC-SHA256 over the exact request payload. The secret is used as +the MAC key, and the decoded digests are compared in constant time. Plain hash +concatenation and string equality are not accepted because they do not provide the same +authentication or comparison guarantees. + ## Rate locks and conversions - A rate lock is owned by its creator and can be spent exactly once. @@ -110,4 +118,4 @@ signal; see [payout reliability](../PAYOUT_RELIABILITY.md#operating-the-queue). - [Webhook signature verification](webhook-signatures.md) - [Withdrawal approval security](../WITHDRAWAL_SECURITY.md) -- [Security policy](../security-policy.md) \ No newline at end of file +- [Security policy](../security-policy.md)