← back to Costa Rica
costa-rica: SAFE/LOCAL security + robustness fixes (C1,C2,R1-R5,M2/R4,M3,L1) — TK-10346
8e875094aac18023159e209360ecf25c03674be7 · 2026-08-07 16:27:30 -0700 · Steve
Route-level validation now rejects bad input with a clean 400 BEFORE the DB
CHECK constraints from migration 008 can fire a constraint-violation 500:
- C1: coerce/validate guests to a positive integer, cap at max_guests, feed the
validated int into the base_price*guests money math (was raw unvalidated body).
- C2: validate check_in/check_out (real ISO, check_out>check_in) for stay mode
and slot_start/slot_end for slot mode → 400 instead of NaN money; nights()
stays defensive (NaN→0) but the route is the real gate.
- R1: /pay wraps confirmBooking in try/catch — payment already succeeded+recorded,
so a confirm failure logs server-side and still returns ok (webhook/poll retries).
- R2: /auth/login pool.query wrapped in try/catch → bad(500,'login failed').
- R3: payment webhook returns 400 on a signed-but-unparseable body (event==null)
and on a signed event with no resolvable id — no phantom pass to firstTime().
- R5: computeSplit asserts subtotal/cleaningFee non-negative integers, bps in
0..10000, and clamps hostPayout to >=0.
- M2/R4: every 500 catch now logs the full error server-side and returns a
GENERIC 'internal error'/'login failed' — no DB/driver text leaked (server.js
serverError() helper, routes/app.js register+apple, routes/webhooks.js whFail).
4xx validation messages kept (intentional + safe).
- M3: verifyToken pins the JWT header alg to HS256 before HMAC verify (rejects
alg:none / a future asymmetric alg-confusion).
- L1: loadSession validates sid=/^[0-9a-f]{16}$/ before path.join (traversal guard).
Local only, no deploy, no live-money behavior change. C3 amount-verification
left untouched (gated for the go-live memo).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Files touched
M lib/auth.jsM lib/money.jsM routes/app.jsM routes/logo-agent.jsM routes/webhooks.jsM server.js
Diff
commit 8e875094aac18023159e209360ecf25c03674be7
Author: Steve <steve@designerwallcoverings.com>
Date: Fri Aug 7 16:27:30 2026 -0700
costa-rica: SAFE/LOCAL security + robustness fixes (C1,C2,R1-R5,M2/R4,M3,L1) — TK-10346
Route-level validation now rejects bad input with a clean 400 BEFORE the DB
CHECK constraints from migration 008 can fire a constraint-violation 500:
- C1: coerce/validate guests to a positive integer, cap at max_guests, feed the
validated int into the base_price*guests money math (was raw unvalidated body).
- C2: validate check_in/check_out (real ISO, check_out>check_in) for stay mode
and slot_start/slot_end for slot mode → 400 instead of NaN money; nights()
stays defensive (NaN→0) but the route is the real gate.
- R1: /pay wraps confirmBooking in try/catch — payment already succeeded+recorded,
so a confirm failure logs server-side and still returns ok (webhook/poll retries).
- R2: /auth/login pool.query wrapped in try/catch → bad(500,'login failed').
- R3: payment webhook returns 400 on a signed-but-unparseable body (event==null)
and on a signed event with no resolvable id — no phantom pass to firstTime().
- R5: computeSplit asserts subtotal/cleaningFee non-negative integers, bps in
0..10000, and clamps hostPayout to >=0.
- M2/R4: every 500 catch now logs the full error server-side and returns a
GENERIC 'internal error'/'login failed' — no DB/driver text leaked (server.js
serverError() helper, routes/app.js register+apple, routes/webhooks.js whFail).
4xx validation messages kept (intentional + safe).
- M3: verifyToken pins the JWT header alg to HS256 before HMAC verify (rejects
alg:none / a future asymmetric alg-confusion).
- L1: loadSession validates sid=/^[0-9a-f]{16}$/ before path.join (traversal guard).
Local only, no deploy, no live-money behavior change. C3 amount-verification
left untouched (gated for the go-live memo).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
lib/auth.js | 4 ++++
lib/money.js | 12 +++++++++--
routes/app.js | 56 +++++++++++++++++++++++++++++++++++++++-------------
routes/logo-agent.js | 10 ++++++++++
routes/webhooks.js | 13 ++++++++++--
server.js | 39 +++++++++++++++++++++---------------
6 files changed, 100 insertions(+), 34 deletions(-)
diff --git a/lib/auth.js b/lib/auth.js
index 533e53f..3639ac4 100644
--- a/lib/auth.js
+++ b/lib/auth.js
@@ -29,6 +29,10 @@ function verifyToken(token) {
const parts = token.split('.');
if (parts.length !== 3) return null;
const [h, p, sig] = parts;
+ // M3 — pin the header alg to HS256 before HMAC-verifying (defense-in-depth
+ // against a future asymmetric refactor / alg-confusion; reject alg:none too).
+ let header; try { header = JSON.parse(Buffer.from(h, 'base64url').toString()); } catch { return null; }
+ if (!header || header.alg !== 'HS256') return null;
const expect = crypto.createHmac('sha256', SECRET).update(`${h}.${p}`).digest('base64url');
try {
if (!crypto.timingSafeEqual(Buffer.from(sig), Buffer.from(expect))) return null;
diff --git a/lib/money.js b/lib/money.js
index 6f011cc..53c33d3 100644
--- a/lib/money.js
+++ b/lib/money.js
@@ -11,11 +11,17 @@ function assertCurrency(c) {
// bps = basis points (100 bps = 1%). platformFeeBps default 1000 = 10%.
function computeSplit({ subtotal, cleaningFee = 0, currency, platformFeeBps = 1000, processorFeeBps = 0 }) {
assertCurrency(currency);
+ // R5 — guard inputs so bad numbers can never yield NaN/negative money that a
+ // DB CHECK (bookings_money_nonneg / bookings_total_reconciles) would 500 on.
+ if (!Number.isInteger(subtotal) || subtotal < 0) throw new Error('subtotal must be a non-negative integer (minor units)');
+ if (!Number.isInteger(cleaningFee) || cleaningFee < 0) throw new Error('cleaningFee must be a non-negative integer (minor units)');
+ if (!(platformFeeBps >= 0 && platformFeeBps <= 10000)) throw new Error('platformFeeBps out of range (0..10000)');
+ if (!(processorFeeBps >= 0 && processorFeeBps <= 10000)) throw new Error('processorFeeBps out of range (0..10000)');
const chargeableFees = cleaningFee; // fees the guest pays on top of subtotal
const platformFee = Math.round((subtotal + chargeableFees) * platformFeeBps / 10000);
const total = subtotal + chargeableFees; // what the traveler is charged
const processorFee = Math.round(total * processorFeeBps / 10000);
- const hostPayout = total - platformFee - processorFee; // what the host receives
+ const hostPayout = Math.max(0, total - platformFee - processorFee); // what the host receives (never negative)
return {
currency,
subtotal,
@@ -31,7 +37,9 @@ function computeSplit({ subtotal, cleaningFee = 0, currency, platformFeeBps = 10
// Nightly booking subtotal from a base nightly price and a date range.
function nights(checkIn, checkOut) {
const a = new Date(checkIn + 'T00:00:00Z'), b = new Date(checkOut + 'T00:00:00Z');
- return Math.max(0, Math.round((b - a) / 86400000));
+ const d = (b - a) / 86400000;
+ if (Number.isNaN(d)) return 0; // defensive: invalid dates → 0 (the route is the real gate, C2)
+ return Math.max(0, Math.round(d));
}
const fmt = (minor, currency) => `${currency} ${(minor / 100).toFixed(2)}`;
diff --git a/routes/app.js b/routes/app.js
index 82085ff..ed44654 100644
--- a/routes/app.js
+++ b/routes/app.js
@@ -41,19 +41,25 @@ router.post('/auth/register', async (req, res) => {
ok(res, { token: signToken({ sub: u.id, role: u.role }), user: u });
} catch (e) {
if (e.code === '23505') return bad(res, 409, 'email or phone already registered');
- bad(res, 500, e.message);
+ console.error('[register]', e.message);
+ bad(res, 500, 'internal error'); // M2/R4 — never leak DB/driver text to the client
}
});
router.post('/auth/login', async (req, res) => {
const { email, password } = req.body || {};
- const { rows } = await pool.query(
- `SELECT id, email, full_name, role, is_host, password_hash FROM app_users WHERE email=$1`,
- [String(email || '').toLowerCase()]);
- const u = rows[0];
- if (!u || !verifyPassword(password || '', u.password_hash)) return bad(res, 401, 'invalid credentials');
- ok(res, { token: signToken({ sub: u.id, role: u.role, host_id: null }),
- user: { id: u.id, email: u.email, full_name: u.full_name, role: u.role, is_host: u.is_host } });
+ try {
+ const { rows } = await pool.query(
+ `SELECT id, email, full_name, role, is_host, password_hash FROM app_users WHERE email=$1`,
+ [String(email || '').toLowerCase()]);
+ const u = rows[0];
+ if (!u || !verifyPassword(password || '', u.password_hash)) return bad(res, 401, 'invalid credentials');
+ ok(res, { token: signToken({ sub: u.id, role: u.role, host_id: null }),
+ user: { id: u.id, email: u.email, full_name: u.full_name, role: u.role, is_host: u.is_host } });
+ } catch (e) {
+ console.error('[login]', e.message);
+ bad(res, 500, 'login failed');
+ }
});
// Sign in with Apple — app sends Apple's identity_token; we verify + upsert.
@@ -85,7 +91,8 @@ router.post('/auth/apple', async (req, res) => {
user: { id: u.id, email: u.email, full_name: u.full_name, role: u.role, is_host: u.is_host } });
} catch (e) {
if (e.code === '23505') return bad(res, 409, 'account conflict');
- bad(res, 500, e.message);
+ console.error('[auth/apple]', e.message);
+ bad(res, 500, 'internal error'); // M2/R4 — never leak DB/driver text to the client
}
});
@@ -152,10 +159,21 @@ router.post('/bookings', authRequired, async (req, res) => {
`SELECT pb.*, p.id AS place_id, p.name FROM place_booking pb
JOIN places p ON p.id = pb.place_id WHERE p.slug=$1 AND pb.is_active`, [place_slug]);
if (!pb) return bad(res, 404, 'listing not bookable');
- if (guests > pb.max_guests) return bad(res, 400, `max ${pb.max_guests} guests`);
+
+ // C1 — validate guests BEFORE it feeds the money math (unvalidated `guests`
+ // reached `base_price * guests` → NaN/negative money + a CHECK-violation 500).
+ const g = Number.parseInt(guests, 10);
+ if (!Number.isInteger(g) || g < 1) return bad(res, 400, 'guests must be a positive integer');
+ if (g > pb.max_guests) return bad(res, 400, `max ${pb.max_guests} guests`);
let subtotal;
if (pb.booking_type === 'nightly') {
+ // C2 — validate dates at the route so bad input is a clean 400, never NaN
+ // money or a DB CHECK-violation 500 (bookings_has_a_date / bookings_stay_order).
+ if (Number.isNaN(Date.parse(check_in)) || Number.isNaN(Date.parse(check_out)))
+ return bad(res, 400, 'check_in and check_out must be valid ISO dates');
+ if (new Date(check_out + 'T00:00:00Z') <= new Date(check_in + 'T00:00:00Z'))
+ return bad(res, 400, 'check_out must be after check_in');
const n = nights(check_in, check_out);
if (n < (pb.min_nights || 1)) return bad(res, 400, `min ${pb.min_nights} nights`);
// Overlap guard (Cody gate, TK-10346 c1). Robust fix = a btree_gist EXCLUDE
@@ -166,7 +184,12 @@ router.post('/bookings', authRequired, async (req, res) => {
if (conflict.length) return bad(res, 409, 'those dates are not available');
subtotal = pb.base_price * n;
} else {
- subtotal = pb.base_price * (guests || 1); // slot/ticket priced per guest
+ // C2 (slot mode) — validate slot bounds symmetrically.
+ if (Number.isNaN(Date.parse(slot_start)) || Number.isNaN(Date.parse(slot_end)))
+ return bad(res, 400, 'slot_start and slot_end must be valid ISO datetimes');
+ if (new Date(slot_end) <= new Date(slot_start))
+ return bad(res, 400, 'slot_end must be after slot_start');
+ subtotal = pb.base_price * g; // slot/ticket priced per guest (validated integer)
}
const split = computeSplit({ subtotal, cleaningFee: pb.cleaning_fee, currency: pb.currency, platformFeeBps: pb.platform_fee_bps });
const code = bookingCode();
@@ -175,7 +198,7 @@ router.post('/bookings', authRequired, async (req, res) => {
guests, currency, subtotal, fees, platform_fee, total, host_payout, status)
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,'pending') RETURNING *`,
[code, pb.place_id, pb.host_id, req.user.sub, check_in || null, check_out || null, slot_start || null, slot_end || null,
- guests, pb.currency, split.subtotal, split.fees, split.platformFee, split.total, split.hostPayout]);
+ g, pb.currency, split.subtotal, split.fees, split.platformFee, split.total, split.hostPayout]);
ok(res, { booking: bk, split });
});
@@ -234,8 +257,13 @@ router.post('/bookings/:code/pay', authRequired, async (req, res) => {
[bk.id, provider.name, charge.providerRef, method, bk.currency, bk.total,
charge.status === 'succeeded' ? 'succeeded' : 'processing', provider.liveMode, JSON.stringify(charge.raw || {})]);
- // If sandbox/instant-succeeded, confirm the booking immediately.
- if (charge.status === 'succeeded') await confirmBooking(bk.id);
+ // If sandbox/instant-succeeded, confirm the booking immediately. R1 — the
+ // payment already succeeded and was recorded; a confirm failure must NOT 500
+ // the caller (the webhook / GET /payments poll retries confirmation).
+ if (charge.status === 'succeeded') {
+ try { await confirmBooking(bk.id); }
+ catch (e) { console.error('[pay] confirmBooking failed (payment ok; will retry via webhook/poll)', bk.id, e.message); }
+ }
ok(res, { payment_id: pay.id, status: charge.status, client_action: charge.clientAction, live_mode: provider.liveMode });
});
diff --git a/routes/logo-agent.js b/routes/logo-agent.js
index 1c8139d..acf8cd7 100644
--- a/routes/logo-agent.js
+++ b/routes/logo-agent.js
@@ -127,6 +127,9 @@ function assemble(genome) {
}
function loadSession(sid) {
+ // L1 — path-traversal guard: sid is a 16-hex token (crypto.randomBytes(8)).
+ // Reject anything else before it reaches path.join (blocks ../ escapes).
+ if (!/^[0-9a-f]{16}$/.test(sid)) return null;
const f = path.join(SESS_DIR, `${sid}.json`);
if (!fs.existsSync(f)) return null;
return JSON.parse(fs.readFileSync(f, 'utf8'));
@@ -243,4 +246,11 @@ router.post('/finalize/:sid', (req, res) => {
res.json({ ok: true, ...out });
});
+// Expose pure internals for the test suite WITHOUT changing the mount contract
+// (Express routers are functions; attaching props is safe, `app.use(router)` still works).
+router._internals = {
+ crossover, buildSvg, assemble, randomGenesFor, motifPath,
+ MOTIFS, CONTAINERS, PALETTES, TYPE, TAGLINES, LAYOUTS, COMPONENTS,
+};
+
module.exports = router;
diff --git a/routes/webhooks.js b/routes/webhooks.js
index aac8662..ba6cff3 100644
--- a/routes/webhooks.js
+++ b/routes/webhooks.js
@@ -27,8 +27,15 @@ async function paymentWebhook(providerName, req, res) {
const provider = getProvider(providerName);
const { ok, event } = provider.verifyWebhook(req.headers, req.body); // req.body is a Buffer (raw)
if (!ok) return res.status(401).send('bad signature');
+ // R3 — a signed-but-unparseable body yields ok:true, event:null. Mirror the
+ // WhatsApp malformed-body 400 and STOP: never fall through to firstTime() with
+ // a null id (which would `return true` and let a phantom event "pass" processing).
+ if (!event) return res.status(400).send('bad body');
const evId = event?.id || event?.paymentId || event?.event_id;
const evType = event?.type || event?.status;
+ // A signed event with no resolvable id is suspicious — do NOT silently pass the
+ // idempotency gate (which best-efforts to firstTime===true on a missing id).
+ if (!evId) return res.status(400).send('missing event id');
if (!(await firstTime(providerName, evId, evType, event))) return res.status(200).send('dup');
// Resolve the charge id the adapter reported to us.
@@ -46,8 +53,10 @@ async function paymentWebhook(providerName, req, res) {
res.status(200).send('ok');
}
-router.post('/tilopay', raw, (req, res) => paymentWebhook('tilopay', req, res).catch(e => res.status(500).send(e.message)));
-router.post('/onvo', raw, (req, res) => paymentWebhook('onvo', req, res).catch(e => res.status(500).send(e.message)));
+// M2/R4 — log the full error server-side; return a generic message (no DB/driver leak).
+const whFail = (res) => (e) => { console.error('[payment webhook]', e && e.message, e && e.stack); res.status(500).send('internal error'); };
+router.post('/tilopay', raw, (req, res) => paymentWebhook('tilopay', req, res).catch(whFail(res)));
+router.post('/onvo', raw, (req, res) => paymentWebhook('onvo', req, res).catch(whFail(res)));
// ---- WhatsApp webhook ----
router.get('/whatsapp', (req, res) => {
diff --git a/server.js b/server.js
index 378c4fb..47e2507 100644
--- a/server.js
+++ b/server.js
@@ -12,6 +12,13 @@ const SITE_DOMAIN = process.env.SITE_DOMAIN || 'costarica.agentabrams.com';
const pool = new Pool({ connectionString: process.env.DATABASE_URL });
+// M2/R4 — log the full error server-side, return a GENERIC message to the client.
+// Never leak DB/driver text (table names, SQL, connection strings) to a caller.
+function serverError(res, e, where) {
+ console.error(`[500]${where ? ' ' + where : ''}`, e && e.message);
+ res.status(500).json({ error: 'internal error' });
+}
+
const app = express();
app.set('trust proxy', true);
@@ -83,7 +90,7 @@ app.get('/api/map', async (req, res) => {
AND ($1::text IS NULL OR p.category=$1)
LIMIT 20000`, [req.query.category || null]);
res.json({ ok: true, count: rows.length, places: rows });
- } catch (e) { res.status(500).json({ ok: false, error: e.message }); }
+ } catch (e) { console.error('[500]', e && e.message); res.status(500).json({ ok: false, error: 'internal error' }); }
});
app.get('/map', (_req, res) => res.sendFile(path.join(__dirname, 'public', 'map.html')));
@@ -129,7 +136,7 @@ app.get('/api/provinces', async (_req, res) => {
GROUP BY pt.name, pt.total_places
ORDER BY pt.total_places DESC, pt.name ASC`);
res.json({ provinces: rows });
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
// Province detail: all cantones + by-vertical totals + 8 sample places
@@ -188,7 +195,7 @@ app.get('/api/provinces/:slug', async (req, res) => {
by_vertical: byVert,
samples,
});
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
// Vertical = e.g. service_retail, tourism_hotel, rentals_realestate.
@@ -214,7 +221,7 @@ app.get('/api/verticals', async (_req, res) => {
FROM counts c LEFT JOIN samples s ON s.vertical = c.vertical
ORDER BY c.n DESC, c.vertical ASC`);
res.json({ verticals: rows.map(r => ({ ...r, slug: vSlug(r.vertical) })) });
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
app.get('/api/verticals/:slug', async (req, res) => {
@@ -258,7 +265,7 @@ app.get('/api/verticals/:slug', async (req, res) => {
by_province: byProv,
samples,
});
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
// Cross-entity search: places + regions + provinces, grouped & ranked
@@ -320,7 +327,7 @@ app.get('/api/search', async (req, res) => {
places, regions, provinces: provMatches,
counts: { places: placesCnt[0]?.total || 0, regions: regionsCnt[0]?.total || 0, provinces: provMatches.length },
});
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
app.get('/api/regions', async (_req, res) => {
@@ -335,7 +342,7 @@ app.get('/api/regions', async (_req, res) => {
`);
res.json({ regions: rows });
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -409,7 +416,7 @@ app.get('/api/places', async (req, res) => {
res.json({ total, limit, offset, places: rows });
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -455,7 +462,7 @@ app.get('/api/places/:slug', async (req, res) => {
res.json(place);
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -478,7 +485,7 @@ app.get('/api/stats', async (_req, res) => {
);
res.json({ total, by_category: byCat, by_vertical: byVert, by_region: byRegion, verticals: VERTICALS });
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -491,7 +498,7 @@ app.post('/api/leads', async (req, res) => {
`, [place_id || null, name, email, phone, message, meta || {}, req.ip, req.get('user-agent') || '']);
res.json({ ok: true, id: rows[0].id });
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -503,7 +510,7 @@ app.get('/api/ingest/runs', async (_req, res) => {
);
res.json({ runs: rows });
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -601,7 +608,7 @@ app.get('/api/places/:slug/jsonld', async (req, res) => {
};
Object.keys(ld).forEach(k => ld[k] === undefined && delete ld[k]);
res.json(ld);
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
// Region landing page — dedicated layout with hero image + region map.
@@ -610,7 +617,7 @@ app.get('/r/:slug', async (req, res) => {
const { rows } = await pool.query('SELECT slug FROM regions WHERE slug = $1', [req.params.slug]);
if (!rows.length) return res.status(404).sendFile(path.join(__dirname, 'public', '404.html'));
res.sendFile(path.join(__dirname, 'public', 'region.html'));
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
// Region details endpoint (one region + count + sibling regions in same province)
@@ -644,7 +651,7 @@ app.get('/api/regions/:slug', async (req, res) => {
region.nearby_in_province = provincial;
res.json(region);
} catch (e) {
- res.status(500).json({ error: e.message });
+ serverError(res, e);
}
});
@@ -659,7 +666,7 @@ app.get('/p/:slug', async (req, res) => {
`, [req.params.slug]);
if (!rows.length) return res.status(404).sendFile(path.join(__dirname, 'public', '404.html'));
res.sendFile(path.join(__dirname, 'public', 'place.html'));
- } catch (e) { res.status(500).json({ error: e.message }); }
+ } catch (e) { serverError(res, e); }
});
app.get('/stats', (_req, res) => res.sendFile(path.join(__dirname, 'public', 'stats.html')));
← 59cc7f3 TK-10346: migration 008 — DB integrity guards (money invaria
·
back to Costa Rica
·
costa-rica: logo-agent test coverage (15 tests) + saveSessio a04f2e1 →