← back to Costa Rica
costa-rica: WhatsApp inbound hardening — auto-reply cost guard + marker-release on failure (cycle 30) — TK-10346
1f25a1f704b97d1a394766d2b4a86f04366e9389 · 2026-09-24 07:47:00 -0700 · Steve
Cold Cody audit of the WhatsApp inbound handler. Impersonation is sound (m.from is
Meta-signature-verified + only ever echoed back to itself; no cross-user lookup),
and stored-XSS / SSRF-via-send-link are LATENT (no admin surface renders WA data,
no caller invokes sendImage/sendDocument) — documented as traps in migrate_013.
Two REAL fixes:
- COST AMPLIFICATION (exploitable today, zero preconditions): the keyword auto-reply
fired one Meta-BILLED sendButtons per inbound message with NO rate limit — anyone
who can WhatsApp the number could drive unbounded billed sends. Added a durable
per-contact cooldown via an atomic conditional UPDATE on a new
whatsapp_contacts.last_auto_reply_at column (migrate_013): the UPDATE both
checks+claims the 60s slot (no TOCTOU, survives restarts), rowCount 0 -> skip.
- SILENT INBOUND-MESSAGE LOSS: handleInbound errors were swallowed (warn + 200)
AFTER the idempotency marker was claimed, so Meta never retried -> permanent loss.
Now releases the marker + 500s (handleInbound is idempotent), scoped to the
persistence step only (auto-reply send failures stay best-effort + 200).
Cody's diff-gate then caught a REAL regression in my first pass: the cost-guard
UPDATE sat OUTSIDE any try/catch, in the loop whose comment promises "always 200s".
A DB blip on that UPDATE (after the marker is committed) would escape the loop -> 500
-> Meta's retry dedupes to 200 -> the auto-reply silently lost for this AND every
remaining event in the batch (same bug class as the #4b it fixes, inconsistent since
the flakier network send WAS wrapped). Fixed: the whole per-event body is now one
try/catch, fail-CLOSED on a cooldown error (no billed send), never escaping the loop.
test/wa-webhook-cooldown.test.js: cooldown won->sends / within-window->skips /
non-keyword->no-check / handleInbound-throw->marker-release+500 / cooldown-UPDATE-
throw->still-200-no-send (E) / one event's error doesn't abort the batch (F).
Suite 234 -> 240, serial green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
Files touched
M docs/GO-LIVE.mdM routes/webhooks.jsA scripts/migrate_013_wa_auto_reply_cooldown.sqlA test/wa-webhook-cooldown.test.js
Diff
commit 1f25a1f704b97d1a394766d2b4a86f04366e9389
Author: Steve <steve@designerwallcoverings.com>
Date: Thu Sep 24 07:47:00 2026 -0700
costa-rica: WhatsApp inbound hardening — auto-reply cost guard + marker-release on failure (cycle 30) — TK-10346
Cold Cody audit of the WhatsApp inbound handler. Impersonation is sound (m.from is
Meta-signature-verified + only ever echoed back to itself; no cross-user lookup),
and stored-XSS / SSRF-via-send-link are LATENT (no admin surface renders WA data,
no caller invokes sendImage/sendDocument) — documented as traps in migrate_013.
Two REAL fixes:
- COST AMPLIFICATION (exploitable today, zero preconditions): the keyword auto-reply
fired one Meta-BILLED sendButtons per inbound message with NO rate limit — anyone
who can WhatsApp the number could drive unbounded billed sends. Added a durable
per-contact cooldown via an atomic conditional UPDATE on a new
whatsapp_contacts.last_auto_reply_at column (migrate_013): the UPDATE both
checks+claims the 60s slot (no TOCTOU, survives restarts), rowCount 0 -> skip.
- SILENT INBOUND-MESSAGE LOSS: handleInbound errors were swallowed (warn + 200)
AFTER the idempotency marker was claimed, so Meta never retried -> permanent loss.
Now releases the marker + 500s (handleInbound is idempotent), scoped to the
persistence step only (auto-reply send failures stay best-effort + 200).
Cody's diff-gate then caught a REAL regression in my first pass: the cost-guard
UPDATE sat OUTSIDE any try/catch, in the loop whose comment promises "always 200s".
A DB blip on that UPDATE (after the marker is committed) would escape the loop -> 500
-> Meta's retry dedupes to 200 -> the auto-reply silently lost for this AND every
remaining event in the batch (same bug class as the #4b it fixes, inconsistent since
the flakier network send WAS wrapped). Fixed: the whole per-event body is now one
try/catch, fail-CLOSED on a cooldown error (no billed send), never escaping the loop.
test/wa-webhook-cooldown.test.js: cooldown won->sends / within-window->skips /
non-keyword->no-check / handleInbound-throw->marker-release+500 / cooldown-UPDATE-
throw->still-200-no-send (E) / one event's error doesn't abort the batch (F).
Suite 234 -> 240, serial green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
docs/GO-LIVE.md | 2 +
routes/webhooks.js | 45 ++++++++--
scripts/migrate_013_wa_auto_reply_cooldown.sql | 22 +++++
test/wa-webhook-cooldown.test.js | 117 +++++++++++++++++++++++++
4 files changed, 177 insertions(+), 9 deletions(-)
diff --git a/docs/GO-LIVE.md b/docs/GO-LIVE.md
index 8913135..79789c7 100644
--- a/docs/GO-LIVE.md
+++ b/docs/GO-LIVE.md
@@ -43,6 +43,8 @@ NEVER `--baseline` on prod (marks pending files applied WITHOUT running → woul
**`migrate_011_payments_one_inflight.sql` prerequisite (double-charge backstop):** builds a partial UNIQUE index on the HOT `payments` table. BEFORE applying, run the pre-flight duplicate check (in the migration header) — if any booking already has ≥2 in-flight payments the build fails; remediate (keep newest in-flight, mark the rest `failed`, verify against the processor which actually charged). The file uses `CREATE UNIQUE INDEX CONCURRENTLY` so it won't block checkout writes; an interrupted build leaves an INVALID index → `DROP INDEX CONCURRENTLY IF EXISTS payments_one_inflight_per_booking;` and re-run.
+**`migrate_013_wa_auto_reply_cooldown.sql` prerequisite (WhatsApp cost guard):** adds a nullable `whatsapp_contacts.last_auto_reply_at TIMESTAMPTZ` backing a per-contact auto-reply cooldown (caps Meta-billed sends from an inbound-keyword flood). Additive, `IF NOT EXISTS`, no backfill (NULL = eligible), safe online (brief lock on a small table). No pre-flight needed. The migration header also documents two hostile-data columns to keep in mind: `whatsapp_contacts.profile_name` + `whatsapp_messages.body` are stored RAW from attacker-controlled inbound payloads (escape at any future admin render — see cycle-24 XSS), and the unused `whatsapp_contacts.user_id` FK must not be wired to a bare wa_id/phone match without an OTP/consent step (impersonation).
+
**`migrate_012_payout_method_completeness.sql` prerequisite (payout completeness backstop):** adds two CHECK constraints so a `payout_methods` row carries the identifier its `kind` pays to (`sinpe_movil`→`sinpe_phone`, `cr_iban`→`cr_iban`); without them an incomplete method silently pays a host $0 in LIVE. A plain `ADD CONSTRAINT` fails if any existing row violates, so BEFORE applying run the pre-flight in the migration header — `SELECT id, kind FROM payout_methods WHERE (kind='sinpe_movil' AND sinpe_phone IS NULL) OR (kind='cr_iban' AND cr_iban IS NULL);` — and fix/delete any hits (verify the host's real payout target) first. `payout_methods` is a low-write table (host-initiated registration only, not hot like `payments`), so the brief validation lock is fine; no `CONCURRENTLY`/`NOT VALID` two-step needed at this scale. `plaid_ach` is intentionally unconstrained (its identifier arrives via the `/host/plaid` exchange). NOTE: `cr_iban` is registerable but the IBAN payout rail is NOT wired — a `cr_iban` payout fails loud in LIVE (`lib/payouts.js` guard) until wired; see the pending host-payout-visibility decision memo.
**`migrate_010_search_trgm.sql` prerequisite:** it runs `CREATE EXTENSION pg_trgm`, which needs **SUPERUSER / rds_superuser**. If the app's DB role isn't a superuser on prod, run the extension line once as the superuser BEFORE the migration pass: `psql "$SUPERUSER_URL" -c 'CREATE EXTENSION IF NOT EXISTS pg_trgm;'`. The three `CREATE INDEX CONCURRENTLY` lines then build without blocking writes to `places`; if a build aborts it leaves an INVALID index → `DROP INDEX CONCURRENTLY IF EXISTS idx_places_<col>_trgm;` and re-run.
diff --git a/routes/webhooks.js b/routes/webhooks.js
index 7538771..83897c9 100644
--- a/routes/webhooks.js
+++ b/routes/webhooks.js
@@ -104,7 +104,18 @@ router.post('/whatsapp', raw, async (req, res) => {
if (!(await firstTime('whatsapp', evId, 'inbound', null))) return res.sendStatus(200);
let events = [];
try { events = await wa.handleInbound(body); }
- catch (e) { console.warn('[wa webhook] handleInbound', e.message); }
+ catch (e) {
+ // handleInbound (the PERSISTENCE step) failed AFTER we claimed the idempotency
+ // marker above. Leaving the marker + 200'ing would SILENTLY drop the inbound
+ // message forever — Meta never retries a 200. Release the marker + 500 so Meta's
+ // retry re-processes (handleInbound is idempotent: logMessage ON CONFLICT DO
+ // NOTHING, contactByWaId upsert, status UPDATE). Mirrors the payment webhook's
+ // marker-release. Auto-reply send failures below are separate + best-effort — they
+ // must NOT 500 (that would re-deliver the whole batch). (Cody wa audit, cycle 30.)
+ console.error('[wa webhook] handleInbound failed, releasing marker for retry', e.message);
+ await pool.query(`DELETE FROM webhook_events WHERE source='whatsapp' AND external_id=$1`, [evId]).catch(() => {});
+ return res.sendStatus(500);
+ }
// Auto-reply hook: a simple keyword router (extend as needed). PER-EVENT isolation
// (Cody cycle-13 finding): a payload can carry multiple inbound messages: the old
// code wrapped the WHOLE loop in one try/catch, so a slow/failing sendButtons for
@@ -114,14 +125,30 @@ router.post('/whatsapp', raw, async (req, res) => {
// its siblings. The route always 200s to Meta either way (no retry storm).
for (const ev of events) {
const t = (ev.text || '').toLowerCase();
- if (/^(hola|hi|hello|menu|ayuda|help)/.test(t)) {
- try {
- await wa.sendButtons(ev.contact.wa_id,
- '¡Hola! ¿En qué te ayudamos? / How can we help?',
- [{ id: 'browse', title: 'Ver listados' }, { id: 'mybookings', title: 'Mis reservas' }, { id: 'support', title: 'Soporte' }],
- { header: 'Costa Rica' });
- } catch (e) { console.warn('[wa webhook] auto-reply', ev.contact?.wa_id, e.message); }
- }
+ if (!/^(hola|hi|hello|menu|ayuda|help)/.test(t)) continue;
+ // The COST GUARD + the send are BOTH best-effort and BOTH wrapped: an uncaught
+ // throw here (a DB blip on the cooldown UPDATE, or the send) would escape the
+ // loop and 500 AFTER the idempotency marker is already committed -> Meta's retry
+ // dedupes to a 200 -> the auto-reply is SILENTLY, permanently lost for this AND
+ // every remaining event in the batch (regressing the cycle-13 per-event
+ // isolation). The inbound message is already durably persisted, so a lost reply
+ // is a minor UX miss, not data loss: log + continue. (Cody gate, cycle 30.)
+ try {
+ // COST GUARD: each auto-reply is a Meta-BILLED send. Without a cap, anyone who
+ // can WhatsApp us drives unbounded billed sends by flooding keywords. This
+ // conditional UPDATE atomically checks+claims a per-contact cooldown slot (no
+ // TOCTOU, durable across restarts): rowCount 0 = within the window -> skip the
+ // billed send. A cooldown-UPDATE error is caught below and fails CLOSED (no send).
+ const { rowCount } = await pool.query(
+ `UPDATE whatsapp_contacts SET last_auto_reply_at=NOW()
+ WHERE id=$1 AND (last_auto_reply_at IS NULL OR last_auto_reply_at < NOW() - interval '60 seconds')`,
+ [ev.contact.id]);
+ if (rowCount === 0) continue;
+ await wa.sendButtons(ev.contact.wa_id,
+ '¡Hola! ¿En qué te ayudamos? / How can we help?',
+ [{ id: 'browse', title: 'Ver listados' }, { id: 'mybookings', title: 'Mis reservas' }, { id: 'support', title: 'Soporte' }],
+ { header: 'Costa Rica' });
+ } catch (e) { console.warn('[wa webhook] auto-reply', ev.contact?.wa_id, e.message); }
}
res.sendStatus(200);
});
diff --git a/scripts/migrate_013_wa_auto_reply_cooldown.sql b/scripts/migrate_013_wa_auto_reply_cooldown.sql
new file mode 100644
index 0000000..87ae807
--- /dev/null
+++ b/scripts/migrate_013_wa_auto_reply_cooldown.sql
@@ -0,0 +1,22 @@
+-- migrate_013_wa_auto_reply_cooldown.sql — WhatsApp auto-reply cost guard.
+--
+-- BUG (cold audit, cycle 30): the inbound WhatsApp auto-reply (routes/webhooks.js)
+-- fires one Meta-billed sendButtons per inbound keyword message with NO rate limit —
+-- anyone who can WhatsApp the business number can drive unbounded, real-money outbound
+-- sends. This column backs a per-contact cooldown: the route's auto-reply loop does an
+-- atomic conditional UPDATE (SET last_auto_reply_at=NOW() WHERE it's null or older than
+-- the window) that both checks AND claims the slot — no TOCTOU, durable across restarts
+-- (an in-process Map would reset on deploy + not hold in cluster mode).
+--
+-- SECURITY NOTE (documented per the cycle-30 audit, no code change): whatsapp_contacts
+-- .profile_name and whatsapp_messages.body are persisted RAW from attacker-controlled
+-- inbound payloads (a WhatsApp display name / message text is settable to anything,
+-- incl. `<img src=x onerror=...>`). No admin surface renders them today, but the FIRST
+-- one that does MUST escape (textContent / esc()), or it recreates the cycle-24 stored
+-- XSS. Likewise whatsapp_contacts.user_id is an UNUSED FK — wiring "link my WhatsApp to
+-- my account" by bare wa_id/phone equality (no OTP/consent) would create real
+-- impersonation; require a verification step.
+--
+-- PROD-APPLY: additive nullable column, safe online (brief lock on a small table); no
+-- backfill needed (NULL = never auto-replied = eligible).
+ALTER TABLE whatsapp_contacts ADD COLUMN IF NOT EXISTS last_auto_reply_at TIMESTAMPTZ;
diff --git a/test/wa-webhook-cooldown.test.js b/test/wa-webhook-cooldown.test.js
new file mode 100644
index 0000000..f8ae28d
--- /dev/null
+++ b/test/wa-webhook-cooldown.test.js
@@ -0,0 +1,117 @@
+'use strict';
+// WhatsApp inbound webhook hardening (Cody cold audit, cycle 30):
+// (A) COST GUARD: the keyword auto-reply is a Meta-BILLED send; a per-contact
+// cooldown (atomic conditional UPDATE on whatsapp_contacts.last_auto_reply_at)
+// caps it so a flood of inbound keywords can't drive unbounded billed sends.
+// (B) MARKER RELEASE: if handleInbound (persistence) throws AFTER the idempotency
+// marker is claimed, release the marker + 500 so Meta retries — leaving the
+// marker + 200 would silently drop the inbound message forever.
+// Signed webhook; pool.query mocked (records calls, routes rowCounts); wa.handleInbound
+// + wa.sendButtons stubbed.
+process.env.WHATSAPP_APP_SECRET = 'itest-wa-secret';
+
+const { test, before, after, beforeEach } = require('node:test');
+const assert = require('node:assert');
+const http = require('node:http');
+const crypto = require('crypto');
+const express = require('express');
+
+const db = require('../lib/db');
+const wa = require('../lib/whatsapp');
+const webhooks = require('../routes/webhooks');
+
+let calls = [];
+let cooldownRowCount = 1; // 1 = won the cooldown slot (send); 0 = within window (skip)
+let cooldownThrowRemaining = 0; // >0: the next N cooldown UPDATEs throw (simulate a DB blip)
+let inboundImpl = null; // per-test wa.handleInbound
+let buttonsSent = 0;
+const origQuery = db.pool.query, origInbound = wa.handleInbound, origButtons = wa.sendButtons;
+let server, base;
+before(async () => {
+ db.pool.query = async (sql, args) => {
+ calls.push({ sql, args });
+ if (/INSERT INTO webhook_events/.test(sql)) return { rows: [], rowCount: 1 }; // firstTime: first-seen
+ if (/UPDATE whatsapp_contacts SET last_auto_reply_at/.test(sql)) {
+ if (cooldownThrowRemaining > 0) { cooldownThrowRemaining--; throw new Error('simulated cooldown UPDATE DB blip'); }
+ return { rows: [], rowCount: cooldownRowCount };
+ }
+ return { rows: [], rowCount: 1 };
+ };
+ wa.handleInbound = async (body) => (inboundImpl ? inboundImpl(body) : []);
+ wa.sendButtons = async () => { buttonsSent++; return { messages: [{ id: 'x' }] }; };
+ const app = express();
+ app.use('/webhooks', webhooks);
+ await new Promise(r => { server = app.listen(0, r); });
+ base = `http://127.0.0.1:${server.address().port}`;
+});
+after(() => { db.pool.query = origQuery; wa.handleInbound = origInbound; wa.sendButtons = origButtons; server && server.close(); });
+beforeEach(() => { calls = []; buttonsSent = 0; cooldownRowCount = 1; cooldownThrowRemaining = 0; inboundImpl = null; });
+
+const waSign = (raw) => 'sha256=' + crypto.createHmac('sha256', 'itest-wa-secret').update(raw).digest('hex');
+function postWa(bodyObj) {
+ const body = JSON.stringify(bodyObj);
+ const opts = { method: 'POST', headers: { 'content-type': 'application/json', 'content-length': Buffer.byteLength(body), 'x-hub-signature-256': waSign(body) } };
+ return new Promise((resolve, reject) => {
+ const r = http.request(base + '/webhooks/whatsapp', opts, res => { let b = ''; res.on('data', c => b += c); res.on('end', () => resolve({ status: res.statusCode, body: b })); });
+ r.on('error', reject); r.end(body);
+ });
+}
+const INBOUND = { entry: [{ id: 'e1', changes: [{ value: { messages: [{ id: 'm1' }] } }] }] };
+const oneKeyword = () => [{ contact: { id: 7, wa_id: '50688880000' }, text: 'hola' }];
+
+test('A: an auto-reply fires only when the per-contact cooldown slot is WON (rowCount 1)', async () => {
+ inboundImpl = oneKeyword; cooldownRowCount = 1;
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 200);
+ assert.equal(buttonsSent, 1, 'winning the cooldown slot sends the auto-reply');
+ const upd = calls.find(c => /UPDATE whatsapp_contacts SET last_auto_reply_at/.test(c.sql));
+ assert.ok(upd, 'the cooldown is claimed via a conditional UPDATE');
+ assert.match(upd.sql, /interval '60 seconds'/, 'the UPDATE enforces the cooldown window atomically');
+});
+
+test('A: an auto-reply within the cooldown window (rowCount 0) is SKIPPED — no billed send', async () => {
+ inboundImpl = oneKeyword; cooldownRowCount = 0;
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 200);
+ assert.equal(buttonsSent, 0, 'within the cooldown window, no Meta-billed send fires');
+});
+
+test('A: a non-keyword message never even checks the cooldown / sends', async () => {
+ inboundImpl = () => [{ contact: { id: 7, wa_id: '50688880000' }, text: 'random chatter' }];
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 200);
+ assert.equal(buttonsSent, 0);
+ assert.equal(calls.some(c => /UPDATE whatsapp_contacts SET last_auto_reply_at/.test(c.sql)), false);
+});
+
+test('E: a cooldown-UPDATE DB error does NOT escape the loop — still 200, no 500 (Cody gate)', async () => {
+ // If this threw uncaught, it would 500 AFTER the marker is committed -> Meta's
+ // retry dedupes -> the auto-reply is silently lost. Must stay 200, fail-closed.
+ inboundImpl = oneKeyword; cooldownThrowRemaining = 1;
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 200, 'a cooldown-UPDATE blip must not 500 (which would dedupe-swallow the retry)');
+ assert.equal(buttonsSent, 0, 'fail closed: no billed send when the cooldown slot cannot be claimed');
+});
+
+test('F: a cooldown-UPDATE error on ONE event does not abort the rest of the batch', async () => {
+ // Two keyword events; the FIRST cooldown UPDATE throws. The catch+continue must let
+ // the SECOND event still process (per-event isolation preserved).
+ inboundImpl = () => [
+ { contact: { id: 7, wa_id: '50688880000' }, text: 'hola' },
+ { contact: { id: 8, wa_id: '50688881111' }, text: 'help' },
+ ];
+ cooldownThrowRemaining = 1; // only the 1st cooldown UPDATE throws
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 200);
+ assert.equal(buttonsSent, 1, 'the 2nd event still gets its auto-reply despite the 1st failing');
+});
+
+test('B: handleInbound failure RELEASES the idempotency marker and returns 500 (Meta retries)', async () => {
+ inboundImpl = () => { throw new Error('transient DB error'); };
+ const r = await postWa(INBOUND);
+ assert.equal(r.status, 500, 'a persistence failure must 500 so Meta retries (not silently 200)');
+ const del = calls.find(c => /DELETE FROM webhook_events/.test(c.sql));
+ assert.ok(del, 'the idempotency marker must be released on failure');
+ assert.deepEqual(del.args, ['e1:m1'], 'the released marker matches the claimed evId');
+ assert.equal(buttonsSent, 0);
+});
← d94c613 cycle 29 docs: YOLO_NOTES ledger — payment-webhook refund-pa
·
back to Costa Rica
·
cycle 30 docs: YOLO_NOTES ledger — WhatsApp inbound cost gua cce97e4 →