From 8156de83ba54799570894c198b4f8871a0436272 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 28 Aug 2026 10:59:10 +0100 Subject: [PATCH] fix(billing): prevent duplicate refunds during provider retries and concurrent submissions --- src/middleware/idempotency.ts | 47 +++++++++++++++++++++++-------- src/routes/billing/refund.test.ts | 37 +++++++++++++++++++++++- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/middleware/idempotency.ts b/src/middleware/idempotency.ts index 66c8d537..74944279 100644 --- a/src/middleware/idempotency.ts +++ b/src/middleware/idempotency.ts @@ -182,13 +182,13 @@ export async function idempotencyMiddleware( } await db.query('DELETE FROM idempotency_store WHERE expires_at < $1', [new Date().toISOString()]); - const result = await db.query( - 'SELECT request_hash, status, response_status, response_body, expires_at FROM idempotency_store WHERE idempotency_key = $1', - [idempotencyKey] - ); - - if (result.rows.length > 0) { - const record = result.rows[0]; + const handleExistingRecord = (record: { + request_hash: string; + status: string; + response_status: number; + response_body: string; + expires_at: string | Date; + }): boolean => { const expiresAt = new Date(record.expires_at); if (expiresAt > new Date()) { @@ -220,7 +220,7 @@ export async function idempotencyMiddleware( incomingFields: incomingKeys, } ); - return; + return true; } if (record.status === 'completed') { @@ -233,7 +233,7 @@ export async function idempotencyMiddleware( }); res.setHeader('Idempotent-Replayed', 'true'); res.status(record.response_status).json(JSON.parse(record.response_body)); - return; + return true; } if (record.status === 'started') { @@ -251,20 +251,43 @@ export async function idempotencyMiddleware( opts?.inProgressErrorCode ?? 'IDEMPOTENCY_IN_PROGRESS', 'Request already in progress' ); - return; + return true; } } + return false; + }; + + const result = await db.query( + 'SELECT request_hash, status, response_status, response_body, expires_at FROM idempotency_store WHERE idempotency_key = $1', + [idempotencyKey] + ); + + if (result.rows.length > 0) { + if (handleExistingRecord(result.rows[0])) { + return; + } } const retentionSeconds = opts?.retentionSeconds ?? config.idempotency.retentionWindowSeconds; const expiresAtDate = new Date(Date.now() + retentionSeconds * 1000); - await db.query( + const insertResult = await db.query( `INSERT INTO idempotency_store (idempotency_key, request_hash, status, expires_at, created_at) - VALUES ($1, $2, $3, $4, NOW()::timestamp)`, + VALUES ($1, $2, $3, $4, NOW()::timestamp) + ON CONFLICT (idempotency_key) DO NOTHING`, [idempotencyKey, requestHash, 'started', expiresAtDate.toISOString()] ); + if (insertResult && insertResult.rowCount === 0) { + const existing = await db.query( + 'SELECT request_hash, status, response_status, response_body, expires_at FROM idempotency_store WHERE idempotency_key = $1', + [idempotencyKey] + ); + if (existing.rows.length > 0 && handleExistingRecord(existing.rows[0])) { + return; + } + } + const originalSend = res.send; const originalJson = res.json; let saved = false; diff --git a/src/routes/billing/refund.test.ts b/src/routes/billing/refund.test.ts index e84ae2fa..4740dbb0 100644 --- a/src/routes/billing/refund.test.ts +++ b/src/routes/billing/refund.test.ts @@ -57,8 +57,11 @@ function makeIdempotencyPool(): Pool { } if (text.includes('INSERT INTO idempotency_store')) { const [key, requestHash, status, expiresAt] = params as [string, string, string, string]; + if (store.has(key)) { + return { rows: [], rowCount: 0 }; + } store.set(key, { request_hash: requestHash, status, response_status: 0, response_body: '', expires_at: expiresAt }); - return { rows: [] }; + return { rows: [], rowCount: 1 }; } if (text.includes('UPDATE idempotency_store')) { const [status, responseStatus, responseBody, key] = params as [string, number, string, string]; @@ -247,4 +250,36 @@ describe('POST /api/billing/refund', () => { expect(second.body.success).toBe(false); expect(grant).toHaveBeenCalledTimes(1); }); + + it('prevents duplicate refunds on concurrent retries with the same Idempotency-Key', async () => { + let grantCallCount = 0; + const grant = jest.fn().mockImplementation(async () => { + grantCallCount++; + // simulate slight async latency + await new Promise(resolve => setTimeout(resolve, 20)); + return makeCredit({ balance_usdc: '15.00' }); + }); + const pool = makeIdempotencyPool(); + const app = buildApp({ pool, creditsRepository: { grant } as unknown as CreditsRepository }); + + const [res1, res2] = await Promise.all([ + request(app) + .post('/api/billing/refund') + .set('x-admin-api-key', ADMIN_KEY) + .set('idempotency-key', 'refund-key-concurrent') + .send(validPayload), + request(app) + .post('/api/billing/refund') + .set('x-admin-api-key', ADMIN_KEY) + .set('idempotency-key', 'refund-key-concurrent') + .send(validPayload), + ]); + + const statuses = [res1.status, res2.status].sort(); + // One request must succeed (200), and the concurrent conflicting one must be rejected (409) or replayed + expect(statuses).toEqual([200, 409]); + expect(grant).toHaveBeenCalledTimes(1); + expect(grantCallCount).toBe(1); + }); }); +