fix: enable RLS with deny-all policy on all public tables

[bmersereau]

Summary

Adds Row Level Security as a defense-in-depth second wall against accidental GRANT statements on application tables.

• Appends a DO $$ ... END$$ block to backend/schema.sql that, for every public base table, runs ALTER TABLE ... ENABLE ROW LEVEL SECURITY and creates a deny_client_access_<tbl> policy with USING (false) WITH CHECK (false) for anon, authenticated.
• Ships the same block as backend/migrations/20260516_enable_rls_deny_all.sql so existing deployments can apply it incrementally.
• Installs an event trigger (enforce_rls_on_public_tables) so any future CREATE TABLE in the public schema automatically gets the same deny-all treatment.
• Adds backend/scripts/verify-rls.sql — a psql-runnable assertion script that fails non-zero if any public table is missing RLS or the deny-all policy.

Closes #144

Context: existing RLS on main

main already enables RLS (with no policies) on 3 tables:

Table	RLS on main
user_api_keys	enabled, no policy
courtlistener_citation_index	enabled, no policy
courtlistener_opinion_cluster_index	enabled, no policy

No other tables have RLS at all. This PR extends RLS + deny-all to all public tables, filling that gap.

Relation to PR #130

PR #130 ("Disclose database migrations") proposes a different RLS strategy — fine-grained per-user policies for authenticated (owner-only mutations, share-aware SELECTs via SECURITY DEFINER helpers). These two approaches are philosophically incompatible:

	This PR (#145)	PR #130
Philosophy	Backend-only; deny all client roles	Fine-grained per-user RLS
authenticated access	Blocked	Owner + share-aware SELECT/mutate
Frontend direct queries	Not supported	Supported

In Postgres, permissive policies combine with OR, so if both land, PR #130's allow-policies would silently nullify this PR's deny-all for authenticated users. A decision is needed on which model to adopt before both are merged.

The current codebase uses no frontend direct queries — all data access goes through the backend via service role — so this PR's deny-all model fits the existing architecture.

Changes

File	Change
backend/schema.sql	Append RLS + deny-all policy block + event trigger
backend/migrations/20260516_enable_rls_deny_all.sql	New incremental migration
backend/migrations/20260516_enable_rls_deny_all.down.sql	Rollback script

Why this matters

The current authorization model is a single REVOKE ALL ... FROM anon, authenticated block. One accidental GRANT SELECT ON public.documents TO authenticated in a future migration, hotfix, or Supabase dashboard click undoes it for that table with zero pushback. RLS with a using (false) deny-all policy is a second wall: even if a grant lands, the policy still blocks the row. The event trigger makes it impossible to add a new public table without the same protection.

The service role bypasses RLS, so the backend (createServerSupabase() uses SUPABASE_SECRET_KEY) is unaffected.

Test plan

• Backend tsc build passes locally.
• Schema block is idempotent (re-runs are no-ops via if not exists guard and idempotent enable row level security).
• Rebased cleanly onto main (conflict in schema.sql resolved: kept new REVOKE lines for courtlistener_* tables from main alongside the RLS block from this branch).
• Reviewer to apply the migration on a Supabase staging instance and confirm:
  • The verify script exits clean.
  • The backend continues to serve normal API traffic (service role bypasses RLS).
  • A direct PostgREST call from the browser anon client to e.g. from('documents').select('*') returns no rows — confirming the second wall.

Notes for reviewers

• The frontend eslint and next build failures on this branch reproduce identically on origin/main — pre-existing tooling debt, not regressions from this PR.
• npm audit reports unrelated high/moderate transitive CVEs — out of scope here.

[bmersereau]

Updated based on self-review (commit ea8a44b). Five changes:

verify-rls.sql — tightened assertions

• Now also asserts pg_policies.with_check (the write wall — previous script only checked qual, so a policy with USING (false) WITH CHECK (true) would have passed while permitting inserts).
• Accepts both 'false' and '(false)' for the qual/with_check text comparison, so a future Postgres expression-rendering change doesn't produce false negatives.
• Cleaner pg_class join via pg_namespace so a same-named table in another schema can never accidentally match.

Rollback migration — backend/migrations/20260516_enable_rls_deny_all.down.sql

• Drops the deny-all policies, the event-trigger function, and the event trigger; disables RLS on every public base table. Idempotent.
• The REVOKE statements in schema.sql remain in force, so the rollback removes the second wall only — clients still cannot reach these tables via PostgREST.

Event trigger — enforce_rls_on_public_tables

• Fires on ddl_command_end when tag in ('CREATE TABLE'). For any new table in the public schema, it auto-runs ENABLE ROW LEVEL SECURITY and creates the matching deny_client_access_<tbl> policy.
• SECURITY DEFINER + set search_path = public so it runs as its owner (postgres) and has the privileges to ALTER/CREATE POLICY on the new table inside the same transaction.
• Covers relkind in ('r', 'p') — regular and partitioned tables. Temp tables (different schema) and views/materialized views (different relkind) are correctly excluded.
• Eliminates the "developer forgot to add RLS" foot-gun raised in the review.

CONTRIBUTING.md — new "Database Migrations" section

• Documents the schema.sql / migrations directory split, the paired .down.sql rollback expectation, and the RLS-by-default convention.
• Notes that the event trigger handles this automatically in normal flow, and points to the verify-rls.sql smoke command for post-migration checks.

Suggested staging-side verification step added to the rollout list

• CREATE TABLE public.test_evt_trigger (id int); then assert pg_class.relrowsecurity = true and one row in pg_policies for test_evt_trigger. Drop the table after. Confirms the event trigger fires.

Pre-existing items not addressed in this PR (intentional, out of scope):

• handle_new_user trigger functional test under RLS — staging-only verification, no code change needed (function is SECURITY DEFINER; loadProfile repairs missing rows via service role as a fallback).
• DO-block duplication between schema.sql and the migration — preserved per the existing convention in the schema.sql header comment.

[bmersereau]

PR Review: fix: enable RLS with deny-all policy on all public tables

Summary

Adds RLS as a deny-all second wall over the existing REVOKE block, installs an event trigger to auto-protect future tables, ships paired up/down migrations, and adds a verify script. The approach is sound and well-documented. One migration correctness issue and a few nits.

Risk Assessment

• Risk Level: Low
• Blast Radius: SQL-only; backend is unaffected (service role bypasses RLS). No application code changes.
• Rollback Plan: psql -f backend/migrations/20260516_enable_rls_deny_all.down.sql — paired down script is included.

Branch Health

• ✅ Fresh — rebased on current origin/main tip (01dfcfe)
• ✅ 3 commits, all scoped to issue #144
• ✅ Conventional commit format

---

Review by Category

Security

• [severity:praise] Dynamic SQL uses format('%I', tbl_name) throughout — correctly prevents SQL injection via identifier quoting.
• [severity:praise] SECURITY DEFINER sets set search_path = public — prevents search-path hijacking.
• [severity:praise] for all to anon, authenticated using (false) with check (false) — blocks both reads and writes. Explicit with check (false) correctly closes the write wall.

Correctness

• [severity:major] Down migration over-reverts pre-existing RLS

  main already has ALTER TABLE public.user_api_keys ENABLE ROW LEVEL SECURITY (and likewise for both CourtListener tables) before this PR. The down script iterates all public base tables and calls disable row level security on each — so rolling back this migration leaves those three tables worse than their pre-migration state (RLS disabled, REVOKE still in place).

  The comment says "this rollback only removes the second wall, not the first" — but for those three tables it also removes the RLS wall that existed before this migration ran.

  Fix options:

  1. Document it explicitly: "tables that had RLS enabled before this migration are not restored to enabled state on rollback; re-enable manually."
  2. Skip those tables in the down script by recording pre-existing state.

• [severity:minor] verify-rls.sql doesn't validate the event trigger

  The verify script confirms existing tables are protected but doesn't check that enforce_rls_on_public_tables is installed. A CREATE TABLE public._rls_probe_<random> → check policies → DROP TABLE round-trip would make it a complete harness.

• [severity:nit] relrowsecurity is false

  IS FALSE is the NULL-safe form for nullable booleans. pg_class.relrowsecurity is NOT NULL, so = false or NOT c.relrowsecurity is more idiomatic.

PostgreSQL Checklist

• ✅ Idempotent up migration
• ✅ Paired down migration
• ✅ No user input in dynamic SQL — identifiers sourced from information_schema/pg_class
• ✅ search_path locked on SECURITY DEFINER function
• ℹ️ Migration uses information_schema.tables; event trigger uses pg_class directly — minor inconsistency, no functional impact at migration-time scale.

Documentation

• [severity:nit] CONTRIBUTING.md documents the event trigger as installed by the migration file, but doesn't mention it's also embedded in schema.sql for fresh installs.

Test Coverage

• verify-rls.sql is a solid manual harness. No CI step runs it automatically — worth adding.

Issue Lifecycle

• ✅ Closes #144 present

---

Questions

1. Confirmed architecture decision: deny-all for authenticated is intentional and PR #130's fine-grained per-user policies won't be merged alongside this?
2. Any plans to wire verify-rls.sql into CI?

---

Verdict

• Request changes — the major issue (down migration disables pre-existing RLS on user_api_keys and the two CourtListener tables) should be addressed or explicitly documented before merge. All other items are minor/nit.

[bmersereau]

Re-review: All previous issues addressed ✅

All items from the first review have been resolved. Quick pass on the updated diff:

Fixed

• ✅ Major — down.sql pre_existing_rls array correctly skips disabling RLS on user_api_keys, courtlistener_citation_index, courtlistener_opinion_cluster_index. Rollback now removes only what the migration added.
• ✅ Minor — verify-rls.sql check 3 added: asserts event trigger is installed and evtenabled = 'O'.
• ✅ Nit — not c.relrowsecurity (was IS FALSE).
• ✅ Nit — CONTRIBUTING.md now references both install paths (migration + schema.sql).

Remaining (nits, no re-review needed)

• The pre_existing_rls list in down.sql is a point-in-time snapshot — if another table gets RLS on main before this migration is applied, the down script would incorrectly disable it. Acceptable limitation; a one-line comment flagging this would be sufficient.
• CONTRIBUTING.md verify-script blurb still says "missing RLS or a deny-all policy" — the script now has a third check (event trigger). Suggest: "missing RLS, a deny-all policy, or if the event trigger is not active."

Verdict

Approve with nits — ready to merge. Both remaining items are one-liners and don't need another round.

[bmersereau]

Any timeline on wiring verify-rls.sql into CI? Even a simple step against a local Supabase instance would catch regressions automatically.