[object Object]

← back to Costa Rica

costa-rica: fix TOCTOU silent-overwrite race in /host/listings ownership (Cody gate, cycle 9) — TK-10346

002feea58b4b502321b05f47637a1f755835a2b3 · 2026-09-23 20:03:20 -0700 · Steve

Cody's audit of the claim-approval finding surfaced a SHARPER, separate bug in the
same guard block: the guard SELECT and the place_booking upsert are separate
statements with no lock between them. Two hosts racing to list the same
never-before-listed place could BOTH read current_host=NULL, both pass the guard,
and both run the upsert — and the old unconditional
`ON CONFLICT (place_id) DO UPDATE SET host_id=EXCLUDED.host_id` let the second
writer SILENTLY overwrite the first's ownership and STILL return 200. Last commit
wins; the loser is told nothing (a false success), and the wrong host becomes
place_booking.host_id — the attributed payout recipient for every booking on that
place.

This is a pure correctness bug with no legitimate "intended design" reading, so
it's fixed directly (not gated): added
`WHERE place_booking.host_id = EXCLUDED.host_id OR place_booking.host_id IS NULL`
to the ON CONFLICT DO UPDATE. The single statement is now atomic — the update
applies only when the caller already owns the row (re-list) or it is unowned
(host deleted -> NULL); a racing non-owner matches no row -> empty RETURNING ->
the route now returns 409 instead of a false 200. The host-deleted re-list path
(NULL host_id) is preserved.

NOTE: this is the pure-correctness half. Whether admin approval (claim_status=
'approved') should ALSO gate listing is a separate customer-facing onboarding
decision — drafted to pending-approval, NOT changed here.

Tests (+3, suite 148 -> 151):
  - host-listing-race.test.js (NEW, 2, real DB): a non-owner cannot silently
    overwrite the owner (empty RETURNING); ownership stays with the first host;
    the owner can re-list; an unowned (NULL) place can still be claimed.
  - host-listing-approval-gap.test.js: the route turns an empty upsert RETURNING
    into a 409 + the upsert carries the atomic ownership WHERE clause.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic

Files touched

Diff

commit 002feea58b4b502321b05f47637a1f755835a2b3
Author: Steve <steve@designerwallcoverings.com>
Date:   Wed Sep 23 20:03:20 2026 -0700

    costa-rica: fix TOCTOU silent-overwrite race in /host/listings ownership (Cody gate, cycle 9) — TK-10346
    
    Cody's audit of the claim-approval finding surfaced a SHARPER, separate bug in the
    same guard block: the guard SELECT and the place_booking upsert are separate
    statements with no lock between them. Two hosts racing to list the same
    never-before-listed place could BOTH read current_host=NULL, both pass the guard,
    and both run the upsert — and the old unconditional
    `ON CONFLICT (place_id) DO UPDATE SET host_id=EXCLUDED.host_id` let the second
    writer SILENTLY overwrite the first's ownership and STILL return 200. Last commit
    wins; the loser is told nothing (a false success), and the wrong host becomes
    place_booking.host_id — the attributed payout recipient for every booking on that
    place.
    
    This is a pure correctness bug with no legitimate "intended design" reading, so
    it's fixed directly (not gated): added
    `WHERE place_booking.host_id = EXCLUDED.host_id OR place_booking.host_id IS NULL`
    to the ON CONFLICT DO UPDATE. The single statement is now atomic — the update
    applies only when the caller already owns the row (re-list) or it is unowned
    (host deleted -> NULL); a racing non-owner matches no row -> empty RETURNING ->
    the route now returns 409 instead of a false 200. The host-deleted re-list path
    (NULL host_id) is preserved.
    
    NOTE: this is the pure-correctness half. Whether admin approval (claim_status=
    'approved') should ALSO gate listing is a separate customer-facing onboarding
    decision — drafted to pending-approval, NOT changed here.
    
    Tests (+3, suite 148 -> 151):
      - host-listing-race.test.js (NEW, 2, real DB): a non-owner cannot silently
        overwrite the owner (empty RETURNING); ownership stays with the first host;
        the owner can re-list; an unowned (NULL) place can still be claimed.
      - host-listing-approval-gap.test.js: the route turns an empty upsert RETURNING
        into a 409 + the upsert carries the atomic ownership WHERE clause.
    Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
 routes/app.js                          | 13 +++++
 test/host-listing-approval-gap.test.js | 15 ++++++
 test/host-listing-race.test.js         | 90 ++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)

diff --git a/routes/app.js b/routes/app.js
index 6bd4488..dfbf002 100644
--- a/routes/app.js
+++ b/routes/app.js
@@ -462,15 +462,28 @@ router.post('/host/listings', authRequired, async (req, res) => {
             (SELECT host_id FROM place_booking WHERE place_id=$1) AS current_host`, [pl.id, host.id]);
   if (!guard.claimed) return bad(res, 403, 'claim this place first via /host/claim');
   if (guard.current_host && guard.current_host !== host.id) return bad(res, 409, 'listing owned by another host');
+  // ATOMICITY (Cody gate, cycle 9): the guard SELECT above and this upsert are
+  // SEPARATE statements with no lock between them, so two hosts racing to list the
+  // same never-before-listed place could BOTH read current_host=NULL, both pass the
+  // guard, and both run the upsert. The old unconditional `DO UPDATE SET host_id=
+  // EXCLUDED.host_id` let the second writer SILENTLY overwrite the first and still
+  // 200 — "last commit wins, loser never told". The `WHERE` makes the single
+  // statement atomic: the update only applies when the caller already owns the row
+  // (re-list) or it is unowned (host deleted -> NULL). A racing non-owner matches no
+  // row -> empty RETURNING -> 409, instead of a false success. (Approval enforcement
+  // — gating on claim_status='approved' — is a separate, customer-facing decision;
+  // see pending-approval memo. This is the pure-correctness half.)
   const { rows: [pb] } = await pool.query(
     `INSERT INTO place_booking (place_id, host_id, booking_type, currency, base_price, cleaning_fee, max_guests, min_nights, instant_book, is_active)
      VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,TRUE)
      ON CONFLICT (place_id) DO UPDATE SET host_id=EXCLUDED.host_id, booking_type=EXCLUDED.booking_type, currency=EXCLUDED.currency,
         base_price=EXCLUDED.base_price, cleaning_fee=EXCLUDED.cleaning_fee, max_guests=EXCLUDED.max_guests,
         min_nights=EXCLUDED.min_nights, instant_book=EXCLUDED.instant_book, is_active=TRUE, updated_at=NOW()
+     WHERE place_booking.host_id = EXCLUDED.host_id OR place_booking.host_id IS NULL
      RETURNING *`,
     [pl.id, host.id, b.booking_type || 'nightly', b.currency || 'USD', b.base_price | 0,
      b.cleaning_fee | 0, b.max_guests || 1, b.min_nights || 1, b.instant_book !== false]);
+  if (!pb) return bad(res, 409, 'listing owned by another host');
   ok(res, { listing: pb });
 });
 
diff --git a/test/host-listing-approval-gap.test.js b/test/host-listing-approval-gap.test.js
index 996a3c0..2c074f9 100644
--- a/test/host-listing-approval-gap.test.js
+++ b/test/host-listing-approval-gap.test.js
@@ -74,6 +74,21 @@ test('GAP: /host/listings guard query does NOT filter place_hosts by claim_statu
     'CURRENT STATE: the guard does NOT consult claim_status — admin approval is unenforced on the money path (see pending-approval memo)');
 });
 
+test('RACE FIX: when the atomic upsert returns no row (a racing non-owner), the route returns 409 (not a false 200)', async () => {
+  const token = signToken({ sub: 3, role: 'guest' });
+  reset([
+    { rows: [{ id: 9, user_id: 3, legal_name: 'H', country: 'CR' }] },   // requireHost
+    { rows: [{ id: 5 }] },                                               // SELECT place
+    { rows: [{ claimed: 1, current_host: null }] },                      // guard passes (race window: reads NULL)
+    { rows: [] },                                                        // upsert WHERE rejected -> empty RETURNING
+  ]);
+  const r = await post('/api/app/host/listings', { place_slug: 'casa', base_price: 12000 }, token);
+  assert.equal(r.status, 409, 'an empty RETURNING from the atomic upsert becomes a 409, not a silent success');
+  const upsert = calls.find(c => /INSERT INTO place_booking/.test(c.sql));
+  assert.ok(upsert && /WHERE place_booking\.host_id = EXCLUDED\.host_id OR place_booking\.host_id IS NULL/.test(upsert.sql),
+    'the upsert carries the atomic ownership WHERE clause');
+});
+
 test('GAP: a host with NO claim row is still correctly blocked (403) — the existence check itself works', async () => {
   const token = signToken({ sub: 3, role: 'guest' });
   reset([
diff --git a/test/host-listing-race.test.js b/test/host-listing-race.test.js
new file mode 100644
index 0000000..0064664
--- /dev/null
+++ b/test/host-listing-race.test.js
@@ -0,0 +1,90 @@
+'use strict';
+// Race/atomicity fix for /host/listings (Cody gate, cycle 9, TK-10346).
+//
+// The guard SELECT and the place_booking upsert in routes/app.js POST /host/listings
+// are separate statements with no lock between them. The OLD unconditional
+// `ON CONFLICT (place_id) DO UPDATE SET host_id=EXCLUDED.host_id` let a second host
+// SILENTLY overwrite the first's ownership (last commit wins; loser gets a false
+// 200). The fix adds `WHERE place_booking.host_id = EXCLUDED.host_id OR
+// place_booking.host_id IS NULL` so the single statement is atomic: only the owner
+// (or an unowned/NULL row) can win; a racing non-owner matches no row -> 409.
+//
+// This proves the WHERE semantics at the REAL DB layer (the fix's core), against
+// the actual `place_booking` table. Sentinel fixtures + FK-safe self-cleanup, no
+// live creds, no money. Mirrors the upsert in routes/app.js — keep in sync.
+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-race-${Date.now()}`;
+
+// The exact upsert routes/app.js POST /host/listings runs (kept in sync with it).
+const UPSERT = `
+  INSERT INTO place_booking (place_id, host_id, booking_type, currency, base_price, cleaning_fee, max_guests, min_nights, instant_book, is_active)
+  VALUES ($1,$2,'nightly','USD',12000,0,1,1,TRUE,TRUE)
+  ON CONFLICT (place_id) DO UPDATE SET host_id=EXCLUDED.host_id, base_price=EXCLUDED.base_price, updated_at=NOW()
+  WHERE place_booking.host_id = EXCLUDED.host_id OR place_booking.host_id IS NULL
+  RETURNING host_id`;
+
+test('place_booking upsert: a non-owner cannot silently overwrite the owner (atomic ownership)', async () => {
+  const created = { placeBookings: [], places: [], hosts: [], users: [] };
+  try {
+    const mkUser = async (n) => (await pool.query(`INSERT INTO app_users (full_name) VALUES ($1) RETURNING id`, [`${SENT}-${n}`])).rows[0].id;
+    const mkHost = async (uid) => (await pool.query(`INSERT INTO hosts (user_id, legal_name, country, kyc_status) VALUES ($1,$2,'CR','verified') RETURNING id`, [uid, `${SENT} host`])).rows[0].id;
+    const uA = await mkUser('uA'); created.users.push(uA);
+    const uB = await mkUser('uB'); created.users.push(uB);
+    const hostA = await mkHost(uA); created.hosts.push(hostA);
+    const hostB = await mkHost(uB); created.hosts.push(hostB);
+    const { rows: [pl] } = await pool.query(
+      `INSERT INTO places (slug, name, category, vertical) VALUES ($1,$2,'rentals','rentals') RETURNING id`,
+      [`${SENT}-slug`, `${SENT} place`]);
+    created.places.push(pl.id);
+
+    // 1. Host A lists first -> wins, becomes owner.
+    const first = await pool.query(UPSERT, [pl.id, hostA]);
+    created.placeBookings.push(pl.id);
+    assert.equal(first.rows.length, 1, 'first lister gets a row');
+    assert.equal(String(first.rows[0].host_id), String(hostA), 'host A is the owner');
+
+    // 2. Host B tries to list the SAME place -> the WHERE rejects the update:
+    //    no row returned (would have been a SILENT overwrite + false 200 before the fix).
+    const second = await pool.query(UPSERT, [pl.id, hostB]);
+    assert.equal(second.rows.length, 0, 'a racing non-owner gets NO row back (route turns this into a 409)');
+
+    // 3. Ownership is UNCHANGED — B did not overwrite A.
+    const { rows: [pb] } = await pool.query(`SELECT host_id FROM place_booking WHERE place_id=$1`, [pl.id]);
+    assert.equal(String(pb.host_id), String(hostA), 'owner is still host A — no silent overwrite');
+
+    // 4. Host A (the owner) can re-list / update -> allowed.
+    const relist = await pool.query(UPSERT, [pl.id, hostA]);
+    assert.equal(relist.rows.length, 1, 'the owner can update its own listing');
+  } finally {
+    if (created.placeBookings.length) await pool.query(`DELETE FROM place_booking WHERE place_id = ANY($1)`, [created.placeBookings]);
+    if (created.places.length) await pool.query(`DELETE FROM places WHERE id = ANY($1)`, [created.places]);
+    if (created.hosts.length) await pool.query(`DELETE FROM hosts WHERE id = ANY($1)`, [created.hosts]);
+    if (created.users.length) await pool.query(`DELETE FROM app_users WHERE id = ANY($1)`, [created.users]);
+  }
+});
+
+test('place_booking upsert: an unowned (NULL host) place can be claimed (host-deleted re-list path preserved)', async () => {
+  const created = { placeBookings: [], places: [], hosts: [], users: [] };
+  try {
+    const { rows: [u] } = await pool.query(`INSERT INTO app_users (full_name) VALUES ($1) RETURNING id`, [`${SENT}-uN`]); created.users.push(u.id);
+    const { rows: [h] } = await pool.query(`INSERT INTO hosts (user_id, legal_name, country) VALUES ($1,$2,'CR') RETURNING id`, [u.id, `${SENT} host`]); created.hosts.push(h.id);
+    const { rows: [pl] } = await pool.query(`INSERT INTO places (slug, name, category, vertical) VALUES ($1,$2,'rentals','rentals') RETURNING id`, [`${SENT}-slugN`, `${SENT} placeN`]); created.places.push(pl.id);
+    // Seed a place_booking whose host was deleted (host_id NULL).
+    await pool.query(`INSERT INTO place_booking (place_id, host_id, base_price) VALUES ($1, NULL, 5000)`, [pl.id]); created.placeBookings.push(pl.id);
+
+    const claim = await pool.query(UPSERT, [pl.id, h.id]);
+    assert.equal(claim.rows.length, 1, 'an unowned (NULL) place can be claimed');
+    assert.equal(String(claim.rows[0].host_id), String(h.id), 'the claimer becomes the owner');
+  } finally {
+    if (created.placeBookings.length) await pool.query(`DELETE FROM place_booking WHERE place_id = ANY($1)`, [created.placeBookings]);
+    if (created.places.length) await pool.query(`DELETE FROM places WHERE id = ANY($1)`, [created.places]);
+    if (created.hosts.length) await pool.query(`DELETE FROM hosts WHERE id = ANY($1)`, [created.hosts]);
+    if (created.users.length) await pool.query(`DELETE FROM app_users WHERE id = ANY($1)`, [created.users]);
+  }
+});

← e57398f costa-rica: add routes/admin.js coverage (was zero) + charac  ·  back to Costa Rica  ·  cycle 9 docs: YOLO_NOTES ledger — admin.js audit, TOCTOU rac 0e2af37 →