From a513ad0fabbf5b16d73f55dbd4885d786e4825d4 Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Mon, 18 May 2026 18:26:19 -0700 Subject: [PATCH] fix(equipment): DB-enforced reservation no-overlap (D1, P0) reserveEquipment was check-then-insert with no DB guard: two concurrent requests both passed the in-memory overlap check and double-booked a machine. Migration 042 adds btree_gist + a GiST EXCLUDE constraint (equipment_id =, tstzrange && ) WHERE status='reserved', making the DB the arbiter; the action maps the 23P01 violation to the existing friendly overlap message. App-side check kept as a fast pre-check. Schema + DB maps updated. --- DB_SCHEMA_MAP.md | 1 + docs/DATABASE_SCHEMA.md | 1 + lib/actions/equipment.ts | 10 +++++++++- scripts/042_equipment_no_overlap.sql | 23 +++++++++++++++++++++++ scripts/schema.sql | 15 +++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 scripts/042_equipment_no_overlap.sql diff --git a/DB_SCHEMA_MAP.md b/DB_SCHEMA_MAP.md index a902620..8bb3c08 100644 --- a/DB_SCHEMA_MAP.md +++ b/DB_SCHEMA_MAP.md @@ -407,6 +407,7 @@ Default channels created by trigger on space INSERT: `general`, `announcements`, | 039 | DATA backfill only: link `form_submissions.member_id` (NULL) to earliest matching member by `(space_id, lower email = lower submitter_email)`. Idempotent; not in schema.sql | | 040 | Stripe dues P1: `payment_platform` += `stripe`; `member_billing` (member↔Stripe customer/sub/status; SELECT admin/board/treasurer, service-client-only writes); `stripe_webhook_events` (idempotency, PK=event id, service-only). Per-space own keys (vault + integrations.config) | | 041 | Notifications P2: `notifications` outbox (type, recipient, subject, body_html/text, status, attempts, dedupe_key). UNIQUE(space_id,dedupe_key) = idempotent webhook enqueue; partial idx (created_at) WHERE status='pending'. SELECT admin/board/treasurer, service-client-only writes (webhook enqueue + dispatcher cron); member self-view via validated action | +| 042 | Equipment double-booking P0 fix: `btree_gist` extension + `equipment_reservations_no_overlap` GiST EXCLUDE (equipment_id =, tstzrange(starts_at,ends_at,'[)') &&) WHERE status='reserved'. DB is the concurrency arbiter; reserveEquipment maps 23P01 to a friendly message. No new table/column | --- diff --git a/docs/DATABASE_SCHEMA.md b/docs/DATABASE_SCHEMA.md index 52171e1..5c6331c 100644 --- a/docs/DATABASE_SCHEMA.md +++ b/docs/DATABASE_SCHEMA.md @@ -722,6 +722,7 @@ migration is mirrored as a section in it. Tables/columns added: | 039 | DATA backfill only (no structural change): links existing `form_submissions.member_id` (where NULL) to the earliest-joined matching `space_members` by `(space_id, lower(email)=lower(submitter_email))`. Idempotent (NULL-only). Not mirrored in schema.sql (fresh DB has no submissions); ongoing linking is app-side (submitForm + linkSubmissionsByEmail) | | 040 | Stripe recurring dues P1. `payment_platform` += `'stripe'` (ALTER TYPE ADD VALUE; mirrored in the CREATE TYPE list). `member_billing` (member↔Stripe customer/subscription/status/period; SELECT = admin/board/treasurer, NO client write policy — webhook/service-client only). `stripe_webhook_events` (idempotency ledger, PK = Stripe event id; service-client only, no policy). Per-space OWN keys: secrets in the vault, config in integrations.config. No new permission code | | 041 | Transactional notifications P2. `notifications` outbox (space_id, member_id, type, recipient, subject, body_html/text, status pending/sent/failed, attempts, dedupe_key). `UNIQUE (space_id, dedupe_key)` makes the Stripe-webhook enqueue idempotent (event replay -> no-op). SELECT = admin/board/treasurer, NO client write policy (webhook enqueue + dispatcher cron, service-client only); member self-view via a validated action. Partial index on `(created_at) WHERE status='pending'` for the dispatcher drain. No new permission code | +| 042 | Equipment double-booking P0 fix. `CREATE EXTENSION btree_gist` + `equipment_reservations_no_overlap` `EXCLUDE USING gist (equipment_id WITH =, tstzrange(starts_at,ends_at,'[)') WITH &&) WHERE status='reserved'`. Makes the DB the concurrency arbiter (the app-side check-then-insert was a TOCTOU); `reserveEquipment` maps the 23P01 exclusion violation to the friendly overlap message. No new table/column/policy/permission | New enum: `comment_entity_type` = `forum_thread | proposal | incident | policy`. diff --git a/lib/actions/equipment.ts b/lib/actions/equipment.ts index 8fd2437..9670d77 100644 --- a/lib/actions/equipment.ts +++ b/lib/actions/equipment.ts @@ -290,7 +290,15 @@ export async function reserveEquipment(input: unknown) { }) .select('id') .single() - if (error) return { error: error.message } + if (error) { + // The GiST exclusion constraint is the concurrency-safe arbiter: a + // simultaneous request that passed the in-memory pre-check still loses + // here. Surface the same friendly message as the pre-check. + if (/exclusion constraint|equipment_reservations_no_overlap|23P01/i.test(error.message)) { + return { error: 'That time overlaps an existing reservation.' } + } + return { error: error.message } + } await logActivity(supabase, member, 'reserved', 'equipment', r.equipmentId) revalidatePath('/equipment') diff --git a/scripts/042_equipment_no_overlap.sql b/scripts/042_equipment_no_overlap.sql new file mode 100644 index 0000000..f667515 --- /dev/null +++ b/scripts/042_equipment_no_overlap.sql @@ -0,0 +1,23 @@ +-- ============================================================================= +-- 042: Equipment reservation overlap — database-enforced (P0 fix) +-- ============================================================================= +-- reserveEquipment() did a check-then-insert with no DB guard: two concurrent +-- requests both pass the in-memory overlap check and both insert, double- +-- booking a physical machine. A GiST exclusion constraint makes the database +-- the arbiter. Only 'reserved' rows conflict (cancelled/completed do not). +-- btree_gist provides the `uuid WITH =` operator class for the composite. +-- +-- Idempotent: DROP CONSTRAINT IF EXISTS then ADD (ADD CONSTRAINT has no +-- IF NOT EXISTS form). +-- ============================================================================= + +CREATE EXTENSION IF NOT EXISTS btree_gist; + +ALTER TABLE public.equipment_reservations + DROP CONSTRAINT IF EXISTS equipment_reservations_no_overlap; +ALTER TABLE public.equipment_reservations + ADD CONSTRAINT equipment_reservations_no_overlap + EXCLUDE USING gist ( + equipment_id WITH =, + tstzrange(starts_at, ends_at, '[)') WITH && + ) WHERE (status = 'reserved'); diff --git a/scripts/schema.sql b/scripts/schema.sql index 9048a7d..61b26c2 100644 --- a/scripts/schema.sql +++ b/scripts/schema.sql @@ -20,6 +20,9 @@ -- 1. EXTENSIONS -- ----------------------------------------------------------------------------- CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +-- btree_gist: provides `uuid WITH =` for the equipment_reservations overlap +-- exclusion constraint (see section below / scripts/042). +CREATE EXTENSION IF NOT EXISTS btree_gist; -- ----------------------------------------------------------------------------- @@ -2277,6 +2280,18 @@ CREATE INDEX IF NOT EXISTS idx_equipment_res_space ON public.equipment_reser CREATE INDEX IF NOT EXISTS idx_equipment_res_equipment ON public.equipment_reservations (equipment_id, starts_at); CREATE INDEX IF NOT EXISTS idx_equipment_res_member ON public.equipment_reservations (member_id); +-- DB-enforced no-overlap (equivalent to scripts/042). Only 'reserved' rows +-- conflict; the app-side check stays as a fast pre-check, the DB is the +-- arbiter under concurrency. Idempotent: DROP then ADD. +ALTER TABLE public.equipment_reservations + DROP CONSTRAINT IF EXISTS equipment_reservations_no_overlap; +ALTER TABLE public.equipment_reservations + ADD CONSTRAINT equipment_reservations_no_overlap + EXCLUDE USING gist ( + equipment_id WITH =, + tstzrange(starts_at, ends_at, '[)') WITH && + ) WHERE (status = 'reserved'); + ALTER TABLE public.equipment ENABLE ROW LEVEL SECURITY; ALTER TABLE public.equipment_reservations ENABLE ROW LEVEL SECURITY;