From 26e54f1fcbac19fe0e5cc5ccd63fae3104f4d38e Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Mon, 18 May 2026 18:39:53 -0700 Subject: [PATCH] fix(rls): members_update WITH CHECK mirrors USING (D8) UPDATE policy was USING-only: a privileged caller could mutate a member row into a space they don't administer (no post-image re-check). Migration 043 adds the mirrored WITH CHECK. Self column escalation (space_id/role/status) remains blocked by the trigger (D9). Schema + DB maps updated. --- DB_SCHEMA_MAP.md | 1 + docs/DATABASE_SCHEMA.md | 1 + scripts/043_members_update_with_check.sql | 23 +++++++++++++++++++++++ scripts/schema.sql | 4 ++++ 4 files changed, 29 insertions(+) create mode 100644 scripts/043_members_update_with_check.sql diff --git a/DB_SCHEMA_MAP.md b/DB_SCHEMA_MAP.md index 8bb3c08..1944568 100644 --- a/DB_SCHEMA_MAP.md +++ b/DB_SCHEMA_MAP.md @@ -408,6 +408,7 @@ Default channels created by trigger on space INSERT: `general`, `announcements`, | 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 | +| 043 | `members_update` RLS gains a WITH CHECK mirroring its USING (defense-in-depth: a privileged caller can't mutate a member row into a space they don't administer). No new table/column | --- diff --git a/docs/DATABASE_SCHEMA.md b/docs/DATABASE_SCHEMA.md index 5c6331c..495f470 100644 --- a/docs/DATABASE_SCHEMA.md +++ b/docs/DATABASE_SCHEMA.md @@ -723,6 +723,7 @@ migration is mirrored as a section in it. Tables/columns added: | 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 | +| 043 | `members_update` RLS policy gains a `WITH CHECK` mirroring its `USING` (`user_id = auth.uid()` OR admin/board/treasurer in `space_id`). Defense-in-depth: the post-image is re-validated so a privileged caller cannot move a member row into a space they do not administer. No new table/column | New enum: `comment_entity_type` = `forum_thread | proposal | incident | policy`. diff --git a/scripts/043_members_update_with_check.sql b/scripts/043_members_update_with_check.sql new file mode 100644 index 0000000..4b67420 --- /dev/null +++ b/scripts/043_members_update_with_check.sql @@ -0,0 +1,23 @@ +-- ============================================================================= +-- 043: space_members UPDATE — add WITH CHECK (defense-in-depth) +-- ============================================================================= +-- members_update had only a USING clause: a privileged caller could mutate a +-- member row into a space where they hold no role (cross-space move) because +-- the post-image was never re-checked. Add a WITH CHECK mirroring the USING +-- so the resulting row must still satisfy the same predicate. (Self column +-- escalation -- space_id/role/status -- is separately blocked by the +-- prevent_member_self_role_change trigger; see scripts/044.) +-- +-- Idempotent: DROP POLICY IF EXISTS then CREATE. +-- ============================================================================= + +DROP POLICY IF EXISTS members_update ON public.space_members; +CREATE POLICY members_update ON public.space_members FOR UPDATE + USING ( + user_id = auth.uid() + OR public.user_has_role_in_space(auth.uid(), space_id, ARRAY['admin','board','treasurer']) + ) + WITH CHECK ( + user_id = auth.uid() + OR public.user_has_role_in_space(auth.uid(), space_id, ARRAY['admin','board','treasurer']) + ); diff --git a/scripts/schema.sql b/scripts/schema.sql index 61b26c2..fd0cd0a 100644 --- a/scripts/schema.sql +++ b/scripts/schema.sql @@ -477,6 +477,10 @@ CREATE POLICY members_update ON public.space_members FOR UPDATE USING ( user_id = auth.uid() OR public.user_has_role_in_space(auth.uid(), space_id, ARRAY['admin','board','treasurer']) + ) + WITH CHECK ( + user_id = auth.uid() + OR public.user_has_role_in_space(auth.uid(), space_id, ARRAY['admin','board','treasurer']) ); CREATE POLICY members_delete ON public.space_members FOR DELETE USING (public.user_has_role_in_space(auth.uid(), space_id, ARRAY['admin']));