← back to Costa Rica
costa-rica: fix double-charge race on /pay — DB unique index + reuse-in-flight (cold-audit find, cycle 19) — TK-10346
08a99153d58eef0cbd3383a06dab749afb1d7fcb · 2026-09-24 01:17:34 -0700 · Steve
A fresh cold Cody audit found a VERIFIED, reproduced money-losing race: POST
/bookings/:code/pay did a lock-free check-then-insert (SELECT "any in-flight
payment?" -> INSERT a 'processing' row -> provider.createCharge()), so two truly
concurrent pay requests for one booking (double-tap "Pay" on flaky mobile data, or
a client auto-retry racing a manual retry) BOTH passed the SELECT, BOTH inserted,
and BOTH called the processor -> the traveler's card charged TWICE for one booking.
No adapter sends a provider idempotency key, so the processor doesn't dedupe either.
Reproduced at the DB layer with two interleaved pg connections. The payout leg had
the mirror guard (payouts_one_per_booking_rail); the charge leg was missing it.
Fix:
1. migrate_011_payments_one_inflight.sql — a PARTIAL unique index enforcing at most
one in-flight ('processing'/'requires_action') payment per booking. A booking can
still have many failed/succeeded/refunded payments (a retry after a 'failed'
attempt is fine); only concurrent in-flight charges are blocked.
2. routes/app.js — the pre-charge INSERT is wrapped: on a 23505 from THIS index
(checked by e.constraint) the race-loser REUSES the winner's payment instead of
firing a second charge. Postgres serializes it — the loser's INSERT blocks on the
winner's row, then 23505s against the now-committed winner.
Cody gate (red-teamed its own find) — logic correct (traced PG locking), 3 deploy/
edge fixes applied:
- CONCURRENTLY: migrate_011 was plain CREATE UNIQUE INDEX -> a SHARE lock on the
HOTTEST write table (every checkout). Changed to CREATE UNIQUE INDEX CONCURRENTLY
(apply-migrations.sh runs it outside a txn) — matching migrate_010's own lesson.
- Pre-flight dupe check: prod may ALREADY have >=2 in-flight rows per booking (from
this very bug), which would fail the index build. Added the GROUP BY HAVING check +
a remediation step (mark all-but-newest 'failed', verify against the processor) to
the migration runbook.
- The resolved-winner 500 (real, was untested): if the winner's charge resolves fast
(sandbox/live instant succeed) before the loser's catch runs, an in-flight-ONLY
re-SELECT misses it -> the loser 500s though its sibling's payment SUCCEEDED. Fixed:
the catch re-SELECTs the booking's most-recent payment (NO status filter) and returns
its real status, so the loser always gets a payment_id to poll, never a 500.
PROD-APPLY is gated (rides the go-live migration pass; runbook has the pre-check +
CONCURRENTLY). Dev-applied (reversible: DROP INDEX).
Ticket (pre-existing, NOT this diff): nothing (webhook/poll aside) moves a truly-stuck
'processing' payment off that status — a dropped webhook + abandoned app session =
a permanently-unpayable booking. Needs a reconciler/TTL. The in-flight SELECT gate
already had this exposure; this index hardens it, doesn't introduce it.
Tests (+3, suite 190 -> 193): DB-level index rejects a 2nd in-flight insert (23505)
+ allows a retry-after-failed; route reuses an in-flight winner (0 createCharge);
route handles a RESOLVED winner without a 500.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
Files touched
M routes/app.jsA scripts/migrate_011_payments_one_inflight.sqlM test/pay-failed-charge.test.jsA test/payments-race-index.test.js
Diff
commit 08a99153d58eef0cbd3383a06dab749afb1d7fcb
Author: Steve <steve@designerwallcoverings.com>
Date: Thu Sep 24 01:17:34 2026 -0700
costa-rica: fix double-charge race on /pay — DB unique index + reuse-in-flight (cold-audit find, cycle 19) — TK-10346
A fresh cold Cody audit found a VERIFIED, reproduced money-losing race: POST
/bookings/:code/pay did a lock-free check-then-insert (SELECT "any in-flight
payment?" -> INSERT a 'processing' row -> provider.createCharge()), so two truly
concurrent pay requests for one booking (double-tap "Pay" on flaky mobile data, or
a client auto-retry racing a manual retry) BOTH passed the SELECT, BOTH inserted,
and BOTH called the processor -> the traveler's card charged TWICE for one booking.
No adapter sends a provider idempotency key, so the processor doesn't dedupe either.
Reproduced at the DB layer with two interleaved pg connections. The payout leg had
the mirror guard (payouts_one_per_booking_rail); the charge leg was missing it.
Fix:
1. migrate_011_payments_one_inflight.sql — a PARTIAL unique index enforcing at most
one in-flight ('processing'/'requires_action') payment per booking. A booking can
still have many failed/succeeded/refunded payments (a retry after a 'failed'
attempt is fine); only concurrent in-flight charges are blocked.
2. routes/app.js — the pre-charge INSERT is wrapped: on a 23505 from THIS index
(checked by e.constraint) the race-loser REUSES the winner's payment instead of
firing a second charge. Postgres serializes it — the loser's INSERT blocks on the
winner's row, then 23505s against the now-committed winner.
Cody gate (red-teamed its own find) — logic correct (traced PG locking), 3 deploy/
edge fixes applied:
- CONCURRENTLY: migrate_011 was plain CREATE UNIQUE INDEX -> a SHARE lock on the
HOTTEST write table (every checkout). Changed to CREATE UNIQUE INDEX CONCURRENTLY
(apply-migrations.sh runs it outside a txn) — matching migrate_010's own lesson.
- Pre-flight dupe check: prod may ALREADY have >=2 in-flight rows per booking (from
this very bug), which would fail the index build. Added the GROUP BY HAVING check +
a remediation step (mark all-but-newest 'failed', verify against the processor) to
the migration runbook.
- The resolved-winner 500 (real, was untested): if the winner's charge resolves fast
(sandbox/live instant succeed) before the loser's catch runs, an in-flight-ONLY
re-SELECT misses it -> the loser 500s though its sibling's payment SUCCEEDED. Fixed:
the catch re-SELECTs the booking's most-recent payment (NO status filter) and returns
its real status, so the loser always gets a payment_id to poll, never a 500.
PROD-APPLY is gated (rides the go-live migration pass; runbook has the pre-check +
CONCURRENTLY). Dev-applied (reversible: DROP INDEX).
Ticket (pre-existing, NOT this diff): nothing (webhook/poll aside) moves a truly-stuck
'processing' payment off that status — a dropped webhook + abandoned app session =
a permanently-unpayable booking. Needs a reconciler/TTL. The in-flight SELECT gate
already had this exposure; this index hardens it, doesn't introduce it.
Tests (+3, suite 190 -> 193): DB-level index rejects a 2nd in-flight insert (23505)
+ allows a retry-after-failed; route reuses an in-flight winner (0 createCharge);
route handles a RESOLVED winner without a 500.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
routes/app.js | 40 ++++++++++++++---
scripts/migrate_011_payments_one_inflight.sql | 42 ++++++++++++++++++
test/pay-failed-charge.test.js | 46 ++++++++++++++++++-
test/payments-race-index.test.js | 63 +++++++++++++++++++++++++++
4 files changed, 183 insertions(+), 8 deletions(-)
diff --git a/routes/app.js b/routes/app.js
index 8705be9..7018b24 100644
--- a/routes/app.js
+++ b/routes/app.js
@@ -266,16 +266,42 @@ router.post('/bookings/:code/pay', authRequired, async (req, res) => {
// REORDER (PRE-FLIGHT #6): check for an in-flight payment before calling the
// provider, so a timeout/error leaves a reconcilable row + a retry detects an
- // existing payment instead of double-charging (closes Cody #1/#2).
- const { rows: [existing] } = await pool.query(
+ // existing payment instead of double-charging (closes Cody #1/#2). This SELECT is
+ // the fast path for the SEQUENTIAL retry; the atomic backstop below handles true
+ // CONCURRENCY.
+ const inflight = () => pool.query(
`SELECT id FROM payments WHERE booking_id=$1 AND status IN ('processing','requires_action') LIMIT 1`, [bk.id]);
+ const { rows: [existing] } = await inflight();
if (existing) return ok(res, { payment_id: existing.id, status: 'requires_action', client_action: null, live_mode: provider.liveMode });
- // Pre-charge: write a 'processing' row so a timeout leaves something to reconcile against.
- let { rows: [pay] } = await pool.query(
- `INSERT INTO payments (booking_id, provider, method, currency, amount, status, live_mode)
- VALUES ($1,$2,$3,$4,$5,'processing',$6) RETURNING id`,
- [bk.id, provider.name, method, bk.currency, bk.total, provider.liveMode]);
+ // Pre-charge: write a 'processing' row so a timeout leaves something to reconcile
+ // against. RACE BACKSTOP (Cody cold audit, cycle 19): the SELECT above and this
+ // INSERT are not atomic, so two concurrent /pay requests could both pass the SELECT
+ // and both insert -> both call the processor -> DOUBLE CHARGE. The partial unique
+ // index payments_one_inflight_per_booking (migrate_011) makes the DB reject the
+ // second in-flight insert with 23505; the race-loser then REUSES the winner's
+ // payment instead of firing a second real charge.
+ let pay;
+ try {
+ const ins = await pool.query(
+ `INSERT INTO payments (booking_id, provider, method, currency, amount, status, live_mode)
+ VALUES ($1,$2,$3,$4,$5,'processing',$6) RETURNING id`,
+ [bk.id, provider.name, method, bk.currency, bk.total, provider.liveMode]);
+ pay = ins.rows[0];
+ } catch (e) {
+ // 23505 on payments_one_inflight_per_booking = another concurrent /pay won the
+ // in-flight slot. Return ITS payment instead of firing a second real charge.
+ // Fetch the booking's most-recent payment WITHOUT a status filter: the winner may
+ // already have RESOLVED (a fast sandbox/live succeed flips it off 'processing'
+ // before we get here), so an in-flight-only lookup could miss it and 500 a
+ // traveler whose payment actually went through. (Cody gate, cycle 19.)
+ if (e && e.code === '23505' && e.constraint === 'payments_one_inflight_per_booking') {
+ const { rows: [raced] } = await pool.query(
+ `SELECT id, status FROM payments WHERE booking_id=$1 ORDER BY created_at DESC LIMIT 1`, [bk.id]);
+ if (raced) return ok(res, { payment_id: raced.id, status: raced.status, client_action: null, live_mode: provider.liveMode });
+ }
+ throw e;
+ }
let charge;
try {
diff --git a/scripts/migrate_011_payments_one_inflight.sql b/scripts/migrate_011_payments_one_inflight.sql
new file mode 100644
index 0000000..94601da
--- /dev/null
+++ b/scripts/migrate_011_payments_one_inflight.sql
@@ -0,0 +1,42 @@
+-- migrate_011_payments_one_inflight.sql — backstop for a double-charge race.
+--
+-- BUG (cold audit, cycle 19): POST /bookings/:code/pay does a lock-free
+-- check-then-act — SELECT "any in-flight payment for this booking?" then INSERT a
+-- 'processing' row, then call the processor. Two truly concurrent pay requests for
+-- one booking (double-tap "Pay" on flaky CR mobile data, or a client auto-retry
+-- racing a manual retry) BOTH pass the SELECT (each sees 0 in-flight), BOTH INSERT,
+-- and BOTH call provider.createCharge() -> the traveler's card is charged TWICE for
+-- one booking. Neither adapter sends a provider idempotency key, so the processor
+-- doesn't dedupe it either. Reproduced at the DB layer against two interleaved pg
+-- connections.
+--
+-- The app-level SELECT can't fix a TOCTOU; the DB must enforce it. This partial
+-- unique index allows at most ONE non-terminal (in-flight) payment per booking —
+-- the exact mirror of payouts_one_per_booking_rail (migrate_009) on the payout leg,
+-- which the charge leg was missing. A booking can still have many failed/succeeded/
+-- refunded payments (a retry after a 'failed' attempt is fine — 'failed' isn't
+-- in-flight); only concurrent in-flight charges are blocked. The race-loser's INSERT
+-- raises 23505, which routes/app.js catches and turns into "reuse the winner's
+-- in-flight payment" -> no second charge fires.
+--
+-- ('requires_action' is never actually stored — the payments.status CHECK coerces it
+-- to 'processing' — but the predicate mirrors the route's in-flight check exactly.)
+--
+-- PROD-APPLY (do BOTH, in order):
+-- (1) PRE-FLIGHT DUPLICATE CHECK — `payments` is the hottest write path and the bug
+-- this fixes may ALREADY have created ≥2 in-flight rows for a booking, which would
+-- make the index build FAIL. Check first, and remediate (keep the newest in-flight
+-- row, mark the rest 'failed') before building:
+-- SELECT booking_id, count(*) FROM payments
+-- WHERE status IN ('processing','requires_action') GROUP BY booking_id HAVING count(*) > 1;
+-- -- if any rows: UPDATE payments SET status='failed' WHERE id IN (
+-- -- SELECT id FROM (SELECT id, row_number() OVER (PARTITION BY booking_id ORDER BY created_at DESC) rn
+-- -- FROM payments WHERE status IN ('processing','requires_action')) t WHERE rn > 1);
+-- -- (verify against the processor which of the dupes actually charged before failing the rest.)
+-- (2) BUILD CONCURRENTLY — a plain CREATE UNIQUE INDEX SHARE-locks payments (blocks
+-- every checkout write) for the build; CONCURRENTLY does not. apply-migrations.sh
+-- runs this file outside a transaction, so CONCURRENTLY is safe through the runner.
+-- If a CONCURRENTLY build is interrupted it leaves an INVALID index:
+-- DROP INDEX CONCURRENTLY IF EXISTS payments_one_inflight_per_booking; -- then re-run.
+CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS payments_one_inflight_per_booking
+ ON payments (booking_id) WHERE status IN ('processing','requires_action');
diff --git a/test/pay-failed-charge.test.js b/test/pay-failed-charge.test.js
index 8b165f8..c16d35d 100644
--- a/test/pay-failed-charge.test.js
+++ b/test/pay-failed-charge.test.js
@@ -24,7 +24,7 @@ const origCreateCharge = tilopay.createCharge;
let server, base;
before(async () => {
- db.pool.query = async (sql, args) => { calls.push({ sql, args }); return responses.length ? responses.shift() : { rows: [], rowCount: 0 }; };
+ db.pool.query = async (sql, args) => { calls.push({ sql, args }); if (responses.length) { const r = responses.shift(); if (r instanceof Error) throw r; return r; } return { rows: [], rowCount: 0 }; };
const app = express();
app.use(express.json());
app.use('/api/app', router);
@@ -65,6 +65,50 @@ test('MONEY: a failed createCharge records the payment as failed + returns 402 (
assert.equal(hasSql(/confirmBooking|UPDATE bookings SET status='confirmed'/), false, 'a failed charge never confirms the booking');
});
+function dupErr() {
+ const e = new Error('duplicate key value violates unique constraint "payments_one_inflight_per_booking"');
+ e.code = '23505'; e.constraint = 'payments_one_inflight_per_booking';
+ return e;
+}
+const RACE_BOOKING = { rows: [{ id: 7, code: 'CR-RACE', place_id: 5, host_id: 9, traveler_id: 3, currency: 'USD', subtotal: 10000, fees: 0, platform_fee: 1200, total: 12000, host_payout: 10800, status: 'pending', created_at: new Date().toISOString() }] };
+const RACE_USER = { rows: [{ email: 'a@b.c', full_name: 'A', phone_e164: '+50611112222' }] };
+
+test('RACE: a concurrent /pay whose pre-charge INSERT loses the unique index (23505) REUSES the winner (no 2nd charge)', async () => {
+ const token = signToken({ sub: 3, role: 'guest' });
+ let chargeCalls = 0;
+ tilopay.createCharge = async () => { chargeCalls++; return { providerRef: 'x', status: 'requires_action', raw: {}, clientAction: null }; };
+ reset([
+ RACE_BOOKING, RACE_USER,
+ { rows: [] }, // inflight() fast-path: saw none (raced past the SELECT)
+ dupErr(), // pre-charge INSERT -> 23505 (the other request won the slot)
+ { rows: [{ id: 88, status: 'processing' }] }, // catch re-SELECT: the winner's still-in-flight payment
+ ]);
+ const r = await post('/api/app/bookings/CR-RACE/pay', { method: 'card' }, token);
+ assert.equal(r.status, 200, 'the race-loser gets a clean response, not a 500');
+ assert.equal(r.json.payment_id, 88, 'it reuses the winner payment');
+ assert.equal(r.json.status, 'processing');
+ assert.equal(chargeCalls, 0, 'the race-loser must NOT call createCharge — no second charge on the card');
+});
+
+test('RACE: if the winner ALREADY RESOLVED (fast succeed) before the loser catches, the loser still returns it (not a 500)', async () => {
+ const token = signToken({ sub: 3, role: 'guest' });
+ let chargeCalls = 0;
+ tilopay.createCharge = async () => { chargeCalls++; return { providerRef: 'x', status: 'succeeded', raw: {}, clientAction: null }; };
+ reset([
+ RACE_BOOKING, RACE_USER,
+ { rows: [] }, // inflight() fast-path: none
+ dupErr(), // INSERT -> 23505
+ // catch re-SELECT (no status filter): the winner already flipped to 'succeeded'.
+ // An in-flight-ONLY lookup would miss this and 500 a traveler whose payment DID go through.
+ { rows: [{ id: 88, status: 'succeeded' }] },
+ ]);
+ const r = await post('/api/app/bookings/CR-RACE/pay', { method: 'card' }, token);
+ assert.equal(r.status, 200, 'a resolved winner must NOT 500 the loser');
+ assert.equal(r.json.payment_id, 88);
+ assert.equal(r.json.status, 'succeeded', 'the loser sees the winner\'s real (resolved) status to poll on');
+ assert.equal(chargeCalls, 0, 'still no second charge');
+});
+
test('MONEY: a succeeded createCharge still records + returns 200 (guard did not break the happy path)', async () => {
const token = signToken({ sub: 3, role: 'guest' });
tilopay.createCharge = async () => ({ providerRef: 'pay_ok', status: 'succeeded', raw: { ok: true }, clientAction: null });
diff --git a/test/payments-race-index.test.js b/test/payments-race-index.test.js
new file mode 100644
index 0000000..87d7ba5
--- /dev/null
+++ b/test/payments-race-index.test.js
@@ -0,0 +1,63 @@
+'use strict';
+// Double-charge backstop (Cody cold audit, cycle 19, TK-10346). The partial unique
+// index payments_one_inflight_per_booking (migrate_011) enforces at most ONE
+// in-flight ('processing'/'requires_action') payment per booking, so two concurrent
+// POST /bookings/:code/pay requests can't both insert a pre-charge row and both
+// charge the card. This proves the DB-level guard directly (real dev DB), and that
+// a legitimate retry after a 'failed' attempt is still allowed (that's the whole
+// point of the PARTIAL predicate). Sentinel fixtures + FK-safe self-cleanup.
+require('dotenv').config();
+const { test, after } = require('node:test');
+const assert = require('node:assert');
+const { pool } = require('../lib/db');
+
+after(() => pool.end());
+
+const SENT = `YOLOTEST-payrace-${Date.now()}`;
+const INS = `INSERT INTO payments (booking_id, provider, method, currency, amount, status, live_mode)
+ VALUES ($1,'tilopay','card','USD',12000,$2,false) RETURNING id`;
+
+test('payments_one_inflight_per_booking: a 2nd concurrent in-flight payment is rejected (23505); a retry after failed is allowed', async () => {
+ const created = { users: [], bookings: [], payments: [] };
+ try {
+ const { rows: [u] } = await pool.query(`INSERT INTO app_users (full_name) VALUES ($1) RETURNING id`, [`${SENT}-u`]);
+ created.users.push(u.id);
+ // Far-future, unique date window so this pending booking on place_id=1 can't
+ // collide with other real-DB test files' place_id=1 bookings under the
+ // bookings_no_overlap_stay EXCLUDE constraint (node --test runs files in parallel).
+ const off = 6000 + (Date.now() % 1000);
+ const { rows: [b] } = await pool.query(
+ `INSERT INTO bookings (traveler_id, place_id, code, status, currency, subtotal, total, platform_fee, host_payout, check_in, check_out)
+ VALUES ($1,1,$2,'pending','USD',10000,12000,1200,10800,CURRENT_DATE + $3::int, CURRENT_DATE + ($3::int + 1)) RETURNING id`,
+ [u.id, `${SENT}-bk`, off]);
+ created.bookings.push(b.id);
+
+ // First in-flight payment -> OK.
+ const { rows: [p1] } = await pool.query(INS, [b.id, 'processing']);
+ created.payments.push(p1.id);
+
+ // A second in-flight payment for the SAME booking (the race-loser) -> rejected.
+ await assert.rejects(
+ () => pool.query(INS, [b.id, 'processing']),
+ (e) => { assert.equal(e.code, '23505', 'unique_violation'); return true; },
+ 'a 2nd in-flight payment for one booking must violate payments_one_inflight_per_booking',
+ );
+
+ // Terminal states don't count as in-flight, so a fresh attempt after a FAILED one is allowed.
+ await pool.query(`UPDATE payments SET status='failed' WHERE id=$1`, [p1.id]);
+ const { rows: [p2] } = await pool.query(INS, [b.id, 'processing']);
+ created.payments.push(p2.id);
+ assert.ok(p2.id, 'a retry after a failed attempt creates a fresh in-flight payment (partial index)');
+
+ // And a 'succeeded' payment also doesn't block... actually it shouldn't happen
+ // (booking would be confirmed), but prove the predicate: mark p2 succeeded, insert allowed.
+ await pool.query(`UPDATE payments SET status='succeeded' WHERE id=$1`, [p2.id]);
+ const { rows: [p3] } = await pool.query(INS, [b.id, 'processing']);
+ created.payments.push(p3.id);
+ assert.ok(p3.id, 'a succeeded payment is terminal, not in-flight');
+ } finally {
+ if (created.payments.length) await pool.query(`DELETE FROM payments WHERE id = ANY($1)`, [created.payments]);
+ if (created.bookings.length) await pool.query(`DELETE FROM bookings WHERE id = ANY($1)`, [created.bookings]);
+ if (created.users.length) await pool.query(`DELETE FROM app_users WHERE id = ANY($1)`, [created.users]);
+ }
+});
← f326dd4 cycle 18 docs: YOLO_NOTES ledger — env-independent test suit
·
back to Costa Rica
·
cycle 19 docs: YOLO_NOTES ledger + GO-LIVE migrate_011 prere d0912c8 →