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.02k stars 199 forks source link

supabase db diff migration files does not include view with (security_invoker=on) clause #792

Open simbas opened 1 year ago

simbas commented 1 year ago

Bug report

Describe the bug

the diff tool does not add the with (security_invoker=on) clause for views created with this clause, this omission can cause security issues.

To Reproduce

in the supabase studio sql editor, create a table with RLS enabled, insert some data, and create a view with security invoker on:

create table test (
  name text
);

alter table "test" enable row level security;

insert into test values ('test1');
insert into test values ('test2');

create view view_with_security_invoker_on with (security_invoker=on) as select
name from test

you get the following result when calling the view with an anon key:

[]

then run migration and reset:

supabase db diff -f create_ view_with_security_invoker_on
supabase db reset

insert back the data (deleted during the reset) in the sql editor:

insert into test values ('test1');
insert into test values ('test2');

you get the following result when calling again the view with an anon key:

[{"name":"test1"}, 
 {"name":"test2"}]

the creation of the view in the migration file is done without the with (security_invoker=on) clause:

create table "public"."test" (
    "name" text
);

alter table "public"."test" enable row level security;

create or replace view "public"."view_with_security_invoker_on" as  SELECT test.name
   FROM test;

Expected behavior

migration file should include the with (security_invoker=on) clause for views created with with (security_invoker=on).

System information

gilbert commented 1 year ago

Just ran into this. Glad I checked, this is a subtle security hazard.

SamuraiT commented 7 months ago

The same thing happens to me

Palabola commented 6 months ago

Same here. The update almost got live. Fix would be much appreciated.

daroczig commented 6 months ago

We have also encountered this and accidentally caught the problem while applying the auto-generated migration files to staging. Note that this is not only a bug, but a potentially serious security risk if a view gets into prod without security_invoker=on unintentionally :disappointed:

gabriel-jones commented 4 months ago

We also encountered this. It's annoying as it means we have to replace all our views after migrations run. Please look into this as it's a high priority security issue.

yannis216 commented 1 month ago

Also (still) running into this security risk :-/

tvogel commented 1 month ago

As the supabase linter currently recommends WITH (security_invoker = on) as a remedy for implicit SECURITY DEFINER views, see https://supabase.com/docs/guides/database/database-advisors?lint=0010_security_definer_view#how-to-resolve, I think, supabase db diff should really support this now.

What is the priority / timeline of this?

sweatybridge commented 1 month ago

What is the priority / timeline of this?

It's hard to say... we are not really keen to fork and maintain our own version of migra, as it may end up being abandoned like pgadmin-schema-diff.

If anyone has the time, I'd recommend trying out and contributing to a new diff tool from stripe which is more actively maintained https://github.com/stripe/pg-schema-diff.

It takes a lot of work build a perfect schema diff tool. But if we can focus on one, I think it will be a huge win for the postgres community.

How-Sustainable commented 2 weeks ago

If you have large number of views to invoke the security on, running this script on your db will find all views with the schema 'public' and invoke the security.

DO $$ 
DECLARE 
    view_rec RECORD;
    view_def TEXT;
BEGIN
    -- Loop through all views in the 'public' schema
    FOR view_rec IN 
        SELECT n.nspname as table_schema,
               c.relname as table_name,
               pg_get_viewdef(c.oid) as view_definition
        FROM pg_class c
        JOIN pg_namespace n ON n.oid = c.relnamespace
        JOIN pg_roles r ON r.oid = c.relowner
        WHERE c.relkind = 'v'  -- Only views
          AND n.nspname = 'public'  -- Only in the 'public' schema
          AND r.rolname = current_user  -- Only process views owned by the current user
    LOOP
        -- Recreate the view with security_invoker=on
        EXECUTE format('
            CREATE OR REPLACE VIEW %I.%I WITH (security_invoker=on) AS
            %s', 
            view_rec.table_schema, 
            view_rec.table_name, 
            view_rec.view_definition);
    END LOOP;
END $$;