diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 04be6981a..b314f5e1c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,3 +33,24 @@ Frontend: ```bash npm run build --prefix frontend ``` + +## Database Migrations + +The schema lives in two places: + +- `backend/schema.sql` — used for fresh installs of a new Supabase database. +- `backend/migrations/YYYYMMDD_.sql` — incremental scripts applied to existing deployments. Each migration should ship with a paired `.down.sql` rollback. + +### Row Level Security + +Every new table in the `public` schema must have RLS enabled with a deny-all policy for the `anon` and `authenticated` roles. An event trigger (`enforce_rls_on_public_tables`) installed by `20260516_enable_rls_deny_all.sql` does this automatically for any `CREATE TABLE` in `public`, so in normal flow you do not need to repeat the policy. If you disable the trigger temporarily, restore it before merging. + +The service role bypasses RLS, so the backend (`createServerSupabase()` via `SUPABASE_SECRET_KEY`) is unaffected; only direct PostgREST access by `anon` / `authenticated` is blocked. Do not grant direct table privileges to those roles — all application data access goes through the backend. + +After a migration touching the schema, verify: + +```bash +psql -v ON_ERROR_STOP=1 -f backend/scripts/verify-rls.sql "$DATABASE_URL" +``` + +This exits non-zero if any `public` base table is missing RLS or a deny-all policy. diff --git a/backend/migrations/20260516_enable_rls_deny_all.down.sql b/backend/migrations/20260516_enable_rls_deny_all.down.sql new file mode 100644 index 000000000..d6f15f023 --- /dev/null +++ b/backend/migrations/20260516_enable_rls_deny_all.down.sql @@ -0,0 +1,28 @@ +-- Rollback for 20260516_enable_rls_deny_all.sql (issue #144). +-- +-- Drops the deny_client_access_ policy and disables RLS on every public +-- base table. The REVOKE statements in schema.sql remain in force so client +-- roles still cannot reach these tables — this rollback only removes the +-- second wall, not the first. +-- +-- Idempotent — safe to re-run. + +drop event trigger if exists enforce_rls_on_public_tables; +drop function if exists public.enforce_rls_on_public_tables(); + +do $$ +declare + tbl text; + policy_name text; +begin + for tbl in + select table_name + from information_schema.tables + where table_schema = 'public' + and table_type = 'BASE TABLE' + loop + policy_name := 'deny_client_access_' || tbl; + execute format('drop policy if exists %I on public.%I', policy_name, tbl); + execute format('alter table public.%I disable row level security', tbl); + end loop; +end$$; diff --git a/backend/migrations/20260516_enable_rls_deny_all.sql b/backend/migrations/20260516_enable_rls_deny_all.sql new file mode 100644 index 000000000..632383669 --- /dev/null +++ b/backend/migrations/20260516_enable_rls_deny_all.sql @@ -0,0 +1,87 @@ +-- Migration: enable RLS + deny-all policy on every public base table, and +-- install an event trigger that does the same for any future public table. +-- Issue: #144 — defense-in-depth second wall against accidental GRANTs. +-- +-- The service role bypasses RLS, so the backend (createServerSupabase) +-- continues to function unchanged. Direct PostgREST access by anon and +-- authenticated roles is blocked by both the existing REVOKE statements +-- in schema.sql and the policy created below. +-- +-- Idempotent — safe to re-run. A matching DOWN script lives at +-- 20260516_enable_rls_deny_all.down.sql. + +do $$ +declare + tbl text; + policy_name text; +begin + for tbl in + select table_name + from information_schema.tables + where table_schema = 'public' + and table_type = 'BASE TABLE' + loop + execute format('alter table public.%I enable row level security', tbl); + policy_name := 'deny_client_access_' || tbl; + if not exists ( + select 1 from pg_policies + where schemaname = 'public' + and tablename = tbl + and policyname = policy_name + ) then + execute format( + 'create policy %I on public.%I for all to anon, authenticated using (false) with check (false)', + policy_name, tbl + ); + end if; + end loop; +end$$; + +-- Auto-enforcement for future tables. See schema.sql for the longer +-- explanation; in short: any new public table automatically receives the +-- same deny-all treatment, eliminating the "developer forgot to add RLS" +-- foot-gun. + +create or replace function public.enforce_rls_on_public_tables() +returns event_trigger +language plpgsql +security definer +set search_path = public +as $$ +declare + obj record; + tbl_name text; + policy_name text; +begin + for obj in + select objid, schema_name + from pg_event_trigger_ddl_commands() + where command_tag = 'CREATE TABLE' and schema_name = 'public' + loop + select c.relname into tbl_name + from pg_class c + where c.oid = obj.objid and c.relkind in ('r', 'p'); + if tbl_name is null then + continue; + end if; + execute format('alter table public.%I enable row level security', tbl_name); + policy_name := 'deny_client_access_' || tbl_name; + if not exists ( + select 1 from pg_policies + where schemaname = 'public' + and tablename = tbl_name + and policyname = policy_name + ) then + execute format( + 'create policy %I on public.%I for all to anon, authenticated using (false) with check (false)', + policy_name, tbl_name + ); + end if; + end loop; +end$$; + +drop event trigger if exists enforce_rls_on_public_tables; +create event trigger enforce_rls_on_public_tables + on ddl_command_end + when tag in ('CREATE TABLE') + execute procedure public.enforce_rls_on_public_tables(); diff --git a/backend/schema.sql b/backend/schema.sql index cc9b9cef9..56552a939 100644 --- a/backend/schema.sql +++ b/backend/schema.sql @@ -365,3 +365,105 @@ revoke all on public.tabular_cells from anon, authenticated; revoke all on public.tabular_review_chats from anon, authenticated; revoke all on public.tabular_review_chat_messages from anon, authenticated; revoke all on public.user_api_keys from anon, authenticated; + +-- --------------------------------------------------------------------------- +-- Row Level Security (defense-in-depth second wall) +-- --------------------------------------------------------------------------- +-- +-- The REVOKE block above is the first wall. RLS with a deny-all policy is +-- the second: if any future migration accidentally GRANTs a privilege to +-- anon or authenticated (a common AI-agent "fix" for empty-result queries), +-- this policy still blocks the row. +-- +-- The service role bypasses RLS, so the backend (which uses +-- SUPABASE_SECRET_KEY via createServerSupabase) is unaffected. +-- +-- Idempotent — safe to re-run on existing deployments. + +do $$ +declare + tbl text; + policy_name text; +begin + for tbl in + select table_name + from information_schema.tables + where table_schema = 'public' + and table_type = 'BASE TABLE' + loop + execute format('alter table public.%I enable row level security', tbl); + policy_name := 'deny_client_access_' || tbl; + if not exists ( + select 1 from pg_policies + where schemaname = 'public' + and tablename = tbl + and policyname = policy_name + ) then + execute format( + 'create policy %I on public.%I for all to anon, authenticated using (false) with check (false)', + policy_name, tbl + ); + end if; + end loop; +end$$; + +-- --------------------------------------------------------------------------- +-- Auto-enforce RLS + deny-all on any future public table +-- --------------------------------------------------------------------------- +-- +-- Event trigger that fires after CREATE TABLE statements. If the new table +-- lives in the public schema, it gets the same deny-all treatment as the +-- existing tables above. This makes it impossible to add a public table +-- without RLS — eliminating the foot-gun where a future migration adds a +-- table and forgets the security boundary. +-- +-- Filters on schema_name = 'public' so DDL in auth/storage/realtime schemas +-- is unaffected. The function is SECURITY DEFINER so it runs as its owner +-- (postgres) — required because the event trigger fires inside the calling +-- transaction and needs privileges to ALTER + CREATE POLICY on the new table. + +create or replace function public.enforce_rls_on_public_tables() +returns event_trigger +language plpgsql +security definer +set search_path = public +as $$ +declare + obj record; + tbl_name text; + policy_name text; +begin + for obj in + select objid, schema_name + from pg_event_trigger_ddl_commands() + where command_tag = 'CREATE TABLE' and schema_name = 'public' + loop + -- pg_class.relname is the unquoted internal identifier; safe to use + -- as the suffix of the policy name and to %I-quote when re-emitting. + select c.relname into tbl_name + from pg_class c + where c.oid = obj.objid and c.relkind in ('r', 'p'); + if tbl_name is null then + continue; + end if; + execute format('alter table public.%I enable row level security', tbl_name); + policy_name := 'deny_client_access_' || tbl_name; + if not exists ( + select 1 from pg_policies + where schemaname = 'public' + and tablename = tbl_name + and policyname = policy_name + ) then + execute format( + 'create policy %I on public.%I for all to anon, authenticated using (false) with check (false)', + policy_name, tbl_name + ); + end if; + end loop; +end$$; + +drop event trigger if exists enforce_rls_on_public_tables; +create event trigger enforce_rls_on_public_tables + on ddl_command_end + when tag in ('CREATE TABLE') + execute procedure public.enforce_rls_on_public_tables(); diff --git a/backend/scripts/verify-rls.sql b/backend/scripts/verify-rls.sql new file mode 100644 index 000000000..ee1be9513 --- /dev/null +++ b/backend/scripts/verify-rls.sql @@ -0,0 +1,67 @@ +-- Verification script for issue #144. +-- Asserts every public base table has Row Level Security enabled AND a +-- deny-all policy for the anon and authenticated roles (both read and write +-- walls). Run with: +-- psql -v ON_ERROR_STOP=1 -f backend/scripts/verify-rls.sql $DATABASE_URL +-- The script exits non-zero on any assertion failure. + +do $$ +declare + missing_rls record; + missing_policy record; + failure_count int := 0; +begin + -- 1. Every public base table must have RLS enabled (relrowsecurity = true). + -- Join through pg_namespace so we never accidentally match a same-named + -- table in a different schema. + for missing_rls in + select t.table_name + from information_schema.tables t + join pg_namespace n on n.nspname = t.table_schema + join pg_class c on c.relname = t.table_name and c.relnamespace = n.oid + where t.table_schema = 'public' + and t.table_type = 'BASE TABLE' + and c.relrowsecurity is false + order by t.table_name + loop + raise warning 'RLS not enabled on public.%', missing_rls.table_name; + failure_count := failure_count + 1; + end loop; + + -- 2. Every public base table must have at least one policy that: + -- - applies to both anon AND authenticated + -- - denies reads (qual = false) + -- - denies writes (with_check = false) + -- Postgres has historically rendered USING (false) as the text 'false'; + -- accept '(false)' as well to be forward-compatible with any future + -- rendering change. with_check is nullable in pg_policies (older policies + -- may not have set it explicitly) — treat null as a failure so the + -- write-wall is always explicit. + for missing_policy in + select t.table_name + from information_schema.tables t + where t.table_schema = 'public' + and t.table_type = 'BASE TABLE' + and not exists ( + select 1 + from pg_policies p + where p.schemaname = 'public' + and p.tablename = t.table_name + and 'anon' = any(p.roles) + and 'authenticated' = any(p.roles) + and p.qual in ('false', '(false)') + and p.with_check in ('false', '(false)') + ) + order by t.table_name + loop + raise warning 'No deny-all policy (read+write) for anon+authenticated on public.%', + missing_policy.table_name; + failure_count := failure_count + 1; + end loop; + + if failure_count > 0 then + raise exception 'verify-rls: % assertion(s) failed', failure_count; + end if; + + raise notice 'verify-rls: all public base tables have RLS enabled and a deny-all read+write policy.'; +end$$;