From e954b0823287fdef556a24619e44f69024ab00df Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Tue, 19 May 2026 14:04:31 -0700 Subject: [PATCH] fix(rls): status-gate user_has_role_in_space + user_has_permission (046) Lifts the deliberately-deferred SECURITY DEFINER status-gate. The deferral's blocking reason ('no way to verify it breaks nothing') is resolved by the integration harness, so the gap is now closed at the RLS layer too (defense-in-depth beyond D2's app layer): an unverified/ inactive member can no longer exercise a role or permission via direct PostgREST. Scoped to the two privilege entrypoints; get_user_space_ids + user_effective_roles deliberately untouched so unverified self-reads/ onboarding still work. Migration 046 + schema.sql mirror. New integration test (5) proves both halves: gap closed AND current/late/approval/unverified-reads all unbroken. 31 integration tests green; default pnpm test 532/26 hermetic; build clean. --- integration/privilege-status-gate.test.ts | 93 +++++++++++++++++++++++ scripts/046_privilege_status_gate.sql | 53 +++++++++++++ scripts/schema.sql | 20 +++-- 3 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 integration/privilege-status-gate.test.ts create mode 100644 scripts/046_privilege_status_gate.sql diff --git a/integration/privilege-status-gate.test.ts b/integration/privilege-status-gate.test.ts new file mode 100644 index 0000000..e060efc --- /dev/null +++ b/integration/privilege-status-gate.test.ts @@ -0,0 +1,93 @@ +// Validates migration 046 (RLS-layer privilege gate, defense-in-depth for +// D2). Proves BOTH halves of the deferral's blocking concern: the gap is +// closed at the RLS layer for unverified/inactive, AND no legitimate flow +// (current/late access, unverified self-reads, the approval flow) breaks. +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { + dbReady, + rows, + execAsUser, + execService, + seedSpace, + seedMember, + dropSpace, +} from './db' + +const d = describe.skipIf(!dbReady) + +d('privilege status gate (migration 046)', () => { + let spaceId: string + let adminCurrent: { memberId: string; userId: string } + let adminUnverified: { memberId: string; userId: string } + let adminLate: { memberId: string; userId: string } + let memberUnverified: { memberId: string; userId: string } + + const hasRole = (uid: string) => + rows<{ ok: boolean }>( + `select public.user_has_role_in_space('${uid}','${spaceId}',ARRAY['admin','board','treasurer']) ok`, + )[0].ok + const hasPerm = (uid: string) => + rows<{ ok: boolean }>( + `select public.user_has_permission('${uid}','${spaceId}','members.manage') ok`, + )[0].ok + + beforeAll(() => { + spaceId = seedSpace().spaceId + adminCurrent = seedMember(spaceId, { role: 'admin', status: 'current' }) + adminUnverified = seedMember(spaceId, { role: 'admin', status: 'unverified' }) + adminLate = seedMember(spaceId, { role: 'admin', status: 'late' }) + memberUnverified = seedMember(spaceId, { role: 'member', status: 'unverified' }) + }) + afterAll(() => dropSpace(spaceId)) + + // ── gap closed ──────────────────────────────────────────────────────── + it('unverified admin is NOT role-privileged at the RLS layer', () => { + expect(hasRole(adminUnverified.userId)).toBe(false) + expect(hasPerm(adminUnverified.userId)).toBe(false) + }) + + it('unverified admin cannot perform a privileged RLS-gated write (defense-in-depth)', () => { + const before = rows<{ display_name: string }>( + `select display_name from space_members where id='${adminCurrent.memberId}'`, + )[0].display_name + // RLS members_update USING relies on user_has_role_in_space -> now false + // for the unverified admin, so the row is not visible to update. + execAsUser( + adminUnverified.userId, + `update space_members set display_name='hacked' where id='${adminCurrent.memberId}'`, + ) + const after = rows<{ display_name: string }>( + `select display_name from space_members where id='${adminCurrent.memberId}'`, + )[0].display_name + expect(after).toBe(before) // unchanged — escalation blocked at RLS + }) + + // ── nothing legitimate broken ───────────────────────────────────────── + it('current + late admins remain fully privileged', () => { + expect(hasRole(adminCurrent.userId)).toBe(true) + expect(hasPerm(adminCurrent.userId)).toBe(true) // admin short-circuit + expect(hasRole(adminLate.userId)).toBe(true) // lapse keeps role + expect(hasPerm(adminLate.userId)).toBe(true) + }) + + it('unverified member still resolves its own space (reads/onboarding intact)', () => { + const n = rows<{ n: number }>( + `select count(*)::int n from public.get_user_space_ids('${memberUnverified.userId}') g where g='${spaceId}'`, + )[0].n + expect(n).toBe(1) // get_user_space_ids deliberately NOT gated + }) + + it('approval flow still works: a current admin can approve an unverified member', () => { + const r = execAsUser( + adminCurrent.userId, + `update space_members set status='current' where id='${memberUnverified.memberId}'`, + ) + expect(r.ok).toBe(true) + const st = rows<{ status: string }>( + `select status from space_members where id='${memberUnverified.memberId}'`, + )[0].status + expect(st).toBe('current') + // and now that member IS privilege-eligible for reads as before + execService(`update space_members set status='unverified' where id='${memberUnverified.memberId}'`) + }) +}) diff --git a/scripts/046_privilege_status_gate.sql b/scripts/046_privilege_status_gate.sql new file mode 100644 index 0000000..ffd365e --- /dev/null +++ b/scripts/046_privilege_status_gate.sql @@ -0,0 +1,53 @@ +-- ============================================================================= +-- 046: RLS-layer privilege gate on member status (defense-in-depth for D2) +-- ============================================================================= +-- D2 closed the require_approval bypass at the APP layer (requireMemberWithRole +-- rejects non-privilege-eligible status). This adds the same guard at the RLS +-- layer so an `unverified`/`inactive` member who somehow bypassed the app +-- (direct PostgREST) still cannot exercise a role or permission. +-- +-- Mirrors lib/permissions.ts PRIVILEGE_STATUSES = ('current','late'). Scope is +-- deliberately the role/permission entrypoints ONLY: +-- * user_has_role_in_space -> write policies + the self-change trigger's +-- privileged-bypass + the admin short-circuit +-- * user_has_permission -> guarded as a whole (closes the custom-role / +-- area-lead permission path WITHOUT touching +-- user_effective_roles) +-- get_user_space_ids and user_effective_roles are intentionally NOT changed: +-- gating SELECT-policy reads would break an unverified member's own /me + +-- onboarding (the legitimate require_approval flow). 'current'/'late' keep +-- full access; approval (a current admin updating ANOTHER member) is +-- unaffected. Idempotent: CREATE OR REPLACE. +-- ============================================================================= + +CREATE OR REPLACE FUNCTION public.user_has_role_in_space(uid uuid, sid uuid, allowed_roles text[]) + RETURNS boolean + LANGUAGE sql STABLE SECURITY DEFINER + SET search_path = public +AS $$ + SELECT EXISTS ( + SELECT 1 FROM public.space_members + WHERE user_id = uid + AND space_id = sid + AND role::text = ANY(allowed_roles) + AND status IN ('current','late') -- privilege-eligible only + ); +$$; + +CREATE OR REPLACE FUNCTION public.user_has_permission(uid uuid, sid uuid, perm text) + RETURNS boolean LANGUAGE sql STABLE SECURITY DEFINER SET search_path = public +AS $$ + SELECT EXISTS ( + SELECT 1 FROM public.space_members + WHERE user_id = uid AND space_id = sid + AND status IN ('current','late') -- no permission unless privilege-eligible + ) + AND ( + public.user_has_role_in_space(uid, sid, ARRAY['admin']) + OR EXISTS ( + SELECT 1 FROM public.space_role_permissions p + WHERE p.space_id = sid AND p.permission = perm + AND p.subject IN (SELECT public.user_effective_roles(uid, sid)) + ) + ); +$$; diff --git a/scripts/schema.sql b/scripts/schema.sql index ae66d3e..96ac61f 100644 --- a/scripts/schema.sql +++ b/scripts/schema.sql @@ -385,6 +385,7 @@ AS $$ WHERE user_id = uid AND space_id = sid AND role::text = ANY(allowed_roles) + AND status IN ('current','late') -- privilege-eligible only (046) ); $$; @@ -1753,12 +1754,19 @@ $$; CREATE OR REPLACE FUNCTION public.user_has_permission(uid uuid, sid uuid, perm text) RETURNS boolean LANGUAGE sql STABLE SECURITY DEFINER SET search_path = public AS $$ - SELECT public.user_has_role_in_space(uid, sid, ARRAY['admin']) - OR EXISTS ( - SELECT 1 FROM public.space_role_permissions p - WHERE p.space_id = sid AND p.permission = perm - AND p.subject IN (SELECT public.user_effective_roles(uid, sid)) - ); + SELECT EXISTS ( + SELECT 1 FROM public.space_members + WHERE user_id = uid AND space_id = sid + AND status IN ('current','late') -- no permission unless privilege-eligible (046) + ) + AND ( + public.user_has_role_in_space(uid, sid, ARRAY['admin']) + OR EXISTS ( + SELECT 1 FROM public.space_role_permissions p + WHERE p.space_id = sid AND p.permission = perm + AND p.subject IN (SELECT public.user_effective_roles(uid, sid)) + ) + ); $$; ALTER TABLE public.space_role_permissions ENABLE ROW LEVEL SECURITY;