supabase / cli

Supabase CLI. Manage postgres migrations, run Supabase locally, deploy edge functions. Postgres backups. Generating types from your database schema.
https://supabase.com/docs/reference/cli/about
MIT License
1.07k stars 209 forks source link

`supabase db diff` granting privileges to `anon` and other roles (in a way it never has) #1864

Closed ghost closed 9 months ago

ghost commented 9 months ago

Describe the bug When I create a new table (demo) and then run supabase db diff (the precise command I use for my tailored setup is supabase db diff --schema storage,extensions,public,pg_catalog,auth -f diff I usually get a schema diff for just the created table. However, now I am getting a whole host of grant commands to roles (including anon) which should not be the case.

I double checked, and none of my previous migrations (after creating tables, et cetera) have these in them:

-- ...

grant delete on table "public"."dispatch_boards" to "anon";

grant insert on table "public"."dispatch_boards" to "anon";

grant references on table "public"."dispatch_boards" to "anon";

grant select on table "public"."dispatch_boards" to "anon";

grant trigger on table "public"."dispatch_boards" to "anon";

grant truncate on table "public"."dispatch_boards" to "anon";

grant update on table "public"."dispatch_boards" to "anon";

grant delete on table "public"."dispatch_boards" to "authenticated";

grant insert on table "public"."dispatch_boards" to "authenticated";

grant references on table "public"."dispatch_boards" to "authenticated";

grant select on table "public"."dispatch_boards" to "authenticated";

grant trigger on table "public"."dispatch_boards" to "authenticated";

grant truncate on table "public"."dispatch_boards" to "authenticated";

grant update on table "public"."dispatch_boards" to "authenticated";

grant delete on table "public"."dispatch_boards" to "service_role";

grant insert on table "public"."dispatch_boards" to "service_role";

grant references on table "public"."dispatch_boards" to "service_role";

grant select on table "public"."dispatch_boards" to "service_role";

grant trigger on table "public"."dispatch_boards" to "service_role";

grant truncate on table "public"."dispatch_boards" to "service_role";

grant update on table "public"."dispatch_boards" to "service_role";

To Reproduce See this video: https://drive.google.com/file/d/1w7Z1uhoQETJO-F6OOJr3RZ5QgJGx004T/view?usp=sharing

Expected behavior None of these statements.

Screenshots See the video above.

Desktop (please complete the following information):

ghost commented 9 months ago

I'm only bringing this up because it appears to me to be a major security issue to be granting these privledges to roles like anon (and for the fact that none of my other 30 some-odd tables have ever experience this before the new 1.131.x version of the cli).

Also, my diff command has never changed (it's what I've always used on this project).

sweatybridge commented 9 months ago

This is probably due to an intentional change to always diff the grant statements. It suggests that your remote database has these grants which are not present in your local migrations.

You can verify this by running the query below

SELECT grantee, privilege_type 
FROM information_schema.role_table_grants 
WHERE table_name='dispatch_boards'

If this is not intended, I think it's best for you to manually revoke these grants through the remote dashboard.

ghost commented 9 months ago

@sweatybridge thanks for your reply. I don't have a remote database connected, I'm diff-ing against the local clone. Basically, I'm using the default functionality (which I believe is the same as using the --local flag). Although, based on the PR you linked, it seems like this isn't a security issue? If it isn't, and this is fine, we can probably close thise (I wanted to be check to be safe).

sweatybridge commented 9 months ago

Oh I see. The local database's default privileges should have setup these grants to anon role for every new table created in public schema. ~It's weird that these grants appear in the diff without any manual changes. Perhaps you can try supabase stop --no-backup to clean up local state before diffing?~

The grants on anon are safe as long as you enable RLS policies on those tables. Without these grants, supabase-js won't be able to access the table using anon key.

EDIT: so it finally occurred to me that these grants appear in the diff because the create table statement is not yet captured in supabase/migrations, leading to a chicken and egg problem. Since it's safer to add these grants, despite being a bit verbose, I will close this issue as working as intended.

danielo515 commented 6 months ago

I am also worried about this. Most of my tables, should only be accessible to the admin user, and no one else. This means that I only enable row level security, and don't add any policy. This seems to be enough to prevent anon to insert or read rows from those tables, but I think it is redundant and unnecessary. Why add those grants? It is confusing and noisy.

stefan-girlich commented 3 months ago

+1 for the security concerns. @sweatybridge Would it make sense / is it possible to add a comment to the corresponding lines in the generated migration file? Even though this should be safe, it could be worth encouraging users to remove at least the permissions that they know they definitely won't need (e.g. any write access for anon) and prevent confusion (and the need to find this thread).

Shabinder commented 2 months ago

+1, this behaviour isnt mentioned anywhere, and seeing these grants, without knowning this, just makes alarms go off... and hence the search for same landed me here.

mentioning it in documentation or adding a comment in the generated migration would be helpful.