-
Notifications
You must be signed in to change notification settings - Fork 9
security(db): enable RLS on all public tables #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jon-bell
wants to merge
4
commits into
main
Choose a base branch
from
security/enable-rls
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
2f59d06
security(db): enable RLS on all public tables
jon-bell e19ca18
security(authz): enforce app-layer ownership on all query RPCs
jon-bell c0af58f
style: prettier-format the ownership authz changes
jon-bell 6d5b9aa
security(authz): address review follow-ups (link ownership, RLS, fail…
jon-bell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,211 @@ | ||
| -- Enable Row Level Security on every public application table and define | ||
| -- owner-scoped access policies. | ||
| -- | ||
| -- WHY THIS EXISTS | ||
| -- Supabase's PostgREST auto-exposes every table in the `public` schema over | ||
| -- HTTP, authorized by the *public* anon key (NEXT_PUBLIC_SUPABASE_ANON_KEY, | ||
| -- shipped to every browser). With RLS disabled, anyone holding that key can | ||
| -- read and modify all rows in these tables (workspaces, prompts, documents, | ||
| -- prolific study IDs, ...). Enabling RLS closes that hole. | ||
| -- | ||
| -- WHY IT DOES NOT BREAK THE APP | ||
| -- The workbench never reads/writes these tables through PostgREST. All table | ||
| -- access goes through Drizzle over DATABASE_URL as the `postgres` role, which | ||
| -- has BYPASSRLS; supabase-js is used only for `.auth` and `.storage`. The | ||
| -- service_role key (used by the test suite) also bypasses RLS. We deliberately | ||
| -- do NOT use FORCE ROW LEVEL SECURITY, so the table-owning `postgres` role and | ||
| -- service_role continue to bypass — exactly the roles the app and tests use. | ||
| -- | ||
| -- OWNERSHIP GRAPH | ||
| -- `workspaces.user_id` = `auth.uid()::text` is the root of ownership. Every | ||
| -- other table inherits ownership by walking back to its workspace (directly via | ||
| -- workspace_id, or via chart_id -> charts -> workspace). Policies are scoped | ||
| -- `TO authenticated`; `anon` matches no policy and is therefore denied on all | ||
| -- tables. auth.uid() is wrapped in a scalar sub-select so Postgres caches it as | ||
| -- an initplan (Supabase's recommended RLS performance pattern). | ||
| -- | ||
| -- NOT INCLUDED (by design) | ||
| -- No anon-readable policy for `workspaces.public = true`. That sharing path is | ||
| -- not served over PostgREST today, and a blanket table policy would expose | ||
| -- user_id / prolific columns. Public sharing over the API, if ever needed, | ||
| -- should be a column-limited view, not a table policy. | ||
| -- | ||
| -- ORPHAN TABLES | ||
| -- Some tables exist in the live DB but not in the Drizzle schema (e.g. | ||
| -- `generations`, left over from the removed generation panel). PostgREST | ||
| -- exposes those too. So rather than enable RLS on a hand-listed set, we enable | ||
| -- it on EVERY base table in `public` — this self-heals against current and | ||
| -- future orphans, locking them to default-deny (bypass roles only) until | ||
| -- someone gives them an explicit policy. The owner-scoped policies below then | ||
| -- layer onto the known application tables. | ||
| -- | ||
| -- Idempotent: safe to re-run (enable-rls is a no-op if already on; policies are | ||
| -- dropped-if-exists before creation). | ||
|
|
||
| begin; | ||
|
|
||
| -- ── Enable RLS on every public base table (covers known + orphan tables) ───── | ||
| do $$ | ||
| declare | ||
| t text; | ||
| begin | ||
| for t in | ||
| select tablename from pg_tables where schemaname = 'public' | ||
| loop | ||
| execute format('alter table public.%I enable row level security', t); | ||
| end loop; | ||
| end $$; | ||
|
|
||
| -- ── workspaces : the ownership root ────────────────────────────────────────── | ||
| drop policy if exists workspaces_owner_all on public.workspaces; | ||
| create policy workspaces_owner_all on public.workspaces | ||
| for all | ||
| to authenticated | ||
| using (user_id = (select auth.uid())::text) | ||
| with check (user_id = (select auth.uid())::text); | ||
|
|
||
| -- ── charts : owned via workspace_id ────────────────────────────────────────── | ||
| drop policy if exists charts_owner_all on public.charts; | ||
| create policy charts_owner_all on public.charts | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = charts.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = charts.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
|
||
| -- ── configs : owned via workspace_id ───────────────────────────────────────── | ||
| drop policy if exists configs_owner_all on public.configs; | ||
| create policy configs_owner_all on public.configs | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = configs.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = configs.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
|
||
| -- ── documents : owned via workspace_id ─────────────────────────────────────── | ||
| drop policy if exists documents_owner_all on public.documents; | ||
| create policy documents_owner_all on public.documents | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = documents.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.workspaces w | ||
| where w.id = documents.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
|
||
| -- ── lens_runs : owned via workspace_id, and chart_id must agree ────────────── | ||
| -- Validating workspace_id alone would let an owner insert a run under their | ||
| -- workspace while pointing chart_id at another user's chart; join charts and | ||
| -- require both columns resolve to the same owned workspace. | ||
| drop policy if exists lens_runs_owner_all on public.lens_runs; | ||
| create policy lens_runs_owner_all on public.lens_runs | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| where c.id = lens_runs.chart_id | ||
| and c.workspace_id = lens_runs.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| where c.id = lens_runs.chart_id | ||
| and c.workspace_id = lens_runs.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| -- ── views : owned via chart_id -> charts -> workspace ──────────────────────── | ||
| drop policy if exists views_owner_all on public.views; | ||
| create policy views_owner_all on public.views | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| where c.id = views.chart_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| where c.id = views.chart_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
|
||
| -- ── chart_config_links : chart AND config must share one owned workspace ───── | ||
| -- Validating chart_id alone would let an owner link their chart to another | ||
| -- user's config (cross-tenant disclosure via copyChart); require the config to | ||
| -- live in the same owned workspace as the chart. | ||
| drop policy if exists chart_config_links_owner_all on public.chart_config_links; | ||
| create policy chart_config_links_owner_all on public.chart_config_links | ||
| for all | ||
| to authenticated | ||
| using ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| join public.configs cfg on cfg.id = chart_config_links.config_id | ||
| where c.id = chart_config_links.chart_id | ||
| and cfg.workspace_id = c.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ) | ||
| with check ( | ||
| exists ( | ||
| select 1 from public.charts c | ||
| join public.workspaces w on w.id = c.workspace_id | ||
| join public.configs cfg on cfg.id = chart_config_links.config_id | ||
| where c.id = chart_config_links.chart_id | ||
| and cfg.workspace_id = c.workspace_id | ||
| and w.user_id = (select auth.uid())::text | ||
| ) | ||
| ); | ||
|
|
||
| -- ── workshops : admin-managed metadata, no client access ───────────────────── | ||
| -- Created and read only through server actions (Drizzle `postgres` role) and | ||
| -- the /w/{slug} join flow (also server-side). RLS is already enabled by the | ||
| -- do-loop above; with NO policy defined, anon and authenticated are both fully | ||
| -- denied over PostgREST — only bypass roles reach it. No ALTER needed here. | ||
|
|
||
| commit; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: ndif-team/workbench
Length of output: 17129
Future tables still need explicit RLS. The loop only enables RLS on tables that exist when this migration runs; any later
CREATE TABLE public.*will still start without RLS, so the “future orphans” claim is too broad. Add an event trigger or a CI check that rejects new public tables without RLS.🤖 Prompt for AI Agents