fix: evtrigs firing SD functions

Author: steve-chavez

src/event_triggers.c

@@ -19,7 +19,7 @@ force_noop(FmgrInfo *finfo)
     finfo->fn_expr   = NULL;                 /* no parse tree */
 }
 
-Oid get_function_owner(func_owner_search search){
+func_attrs get_function_attrs(func_search search){
   // Lookup function name OID. Note that for event trigger functions, there's no arguments.
   Oid func_oid = InvalidOid;
 
@@ -39,10 +39,11 @@ Oid get_function_owner(func_owner_search search){
 
   Form_pg_proc procForm = (Form_pg_proc) GETSTRUCT(proc_tup);
   Oid func_owner = procForm->proowner;
+  bool is_secdef = procForm->prosecdef;
 
   ReleaseSysCache(proc_tup);
 
-  return func_owner;
+  return (func_attrs){func_owner, is_secdef};
 }
 
 bool is_event_trigger_function(Oid foid){

src/event_triggers.h

@@ -12,9 +12,14 @@ typedef struct {
         List     *funcname;
         FmgrInfo *finfo;
     } val;
-} func_owner_search;
+} func_search;
 
-extern Oid get_function_owner(func_owner_search search);
+typedef struct {
+  Oid owner;
+  bool is_security_definer;
+} func_attrs;
+
+extern func_attrs get_function_attrs(func_search search);
 
 extern void force_noop(FmgrInfo *finfo);
 

src/supautils.c

@@ -97,20 +97,26 @@ static void supautils_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum
     // we only need to change behavior before the function gets executed
     case FHET_START: {
         if (is_event_trigger_function(flinfo->fn_oid)){ // recheck the function is an event trigger in case another extension need_fmgr_hook passed our supautils_needs_fmgr_hook
-            const char *current_role_name = GetUserNameFromId(GetUserId(), false);
-            const bool role_is_super = superuser();
+            func_attrs fattrs = get_function_attrs((func_search){ .as = FO_SEARCH_FINFO, .val.finfo = flinfo });
+            const Oid current_role_oid =
+                fattrs.is_security_definer?
+                // when the function is security definer, we need to get the session user id otherwise it will fire for superusers or reserved roles.
+                // See https://github.com/supabase/supautils/issues/140.
+                GetOuterUserId():
+                GetUserId();
+            const char *current_role_name = GetUserNameFromId(current_role_oid, false);
+            const bool role_is_super = superuser_arg(current_role_oid);
             const bool role_is_reserved = is_reserved_role(current_role_name, false);
             if (role_is_super || role_is_reserved) {
-                Oid func_owner = get_function_owner((func_owner_search){ .as = FO_SEARCH_FINFO, .val.finfo = flinfo });
-                bool function_is_owned_by_super = superuser_arg(func_owner);
+                bool function_is_owned_by_super = superuser_arg(fattrs.owner);
                 if (!function_is_owned_by_super){
                     if (log_skipped_evtrigs){
                       char *func_name = get_func_name(flinfo->fn_oid);
                       ereport(
                         NOTICE,
                         errmsg("Skipping event trigger function \"%s\" for user \"%s\"", func_name, current_role_name),
                         errdetail("\"%s\" %s and the function \"%s\" is not superuser-owned, it's owned by \"%s\"",
-                            current_role_name, role_is_super?"is a superuser":"is a reserved role", func_name, GetUserNameFromId(func_owner, false))
+                            current_role_name, role_is_super?"is a superuser":"is a reserved role", func_name, GetUserNameFromId(fattrs.owner, false))
                       );
                     }
                     // we can't skip execution directly inside the fmgr_hook (although we can abort it with ereport)
@@ -814,8 +820,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) {
           const char *current_role_name = GetUserNameFromId(current_user_id, false);
 
           bool current_user_is_super = superuser_arg(current_user_id);
-          Oid  function_owner = get_function_owner((func_owner_search){FO_SEARCH_NAME, {stmt->funcname}});
-          bool function_is_owned_by_super = superuser_arg(function_owner);
+          func_attrs fattrs = get_function_attrs((func_search){FO_SEARCH_NAME, {stmt->funcname}});
+          bool function_is_owned_by_super = superuser_arg(fattrs.owner);
 
           if(!current_user_is_super && function_is_owned_by_super){
             ereport(ERROR, (

test/expected/event_triggers.out.in

@@ -183,3 +183,45 @@ ERROR:  must be owner of event trigger event_trigger_1
 set role privileged_role;
 drop event trigger event_trigger_1;
 drop event trigger event_trigger_2;
+\echo
+
+-- security definer function case
+set role privileged_role;
+create function secdef_show_current_user()
+    returns event_trigger
+    language plpgsql
+    security definer as
+$$
+begin
+    raise notice 'the event trigger is executed for %', current_user;
+end;
+$$;
+create event trigger event_trigger_3 on ddl_command_end
+execute procedure secdef_show_current_user();
+\echo
+
+-- secdef won't be executed for superuser
+set role postgres;
+create table super_foo();
+\echo
+
+-- secdef won't be executed for reserved roles
+set role supabase_storage_admin;
+create table storage_foo();
+\echo
+
+-- secdef will be executed for other roles
+set role rolecreator;
+create table rolecreator_foo();
+NOTICE:  the event trigger is executed for privileged_role
+\echo
+
+-- secdef will be executed for privileged_role
+set role privileged_role;
+create table privileged_role_foo();
+NOTICE:  the event trigger is executed for privileged_role
+\echo
+
+-- cleanup
+set role privileged_role;
+drop event trigger event_trigger_3;

test/sql/event_triggers.sql

@@ -144,3 +144,43 @@ drop event trigger event_trigger_1;
 set role privileged_role;
 drop event trigger event_trigger_1;
 drop event trigger event_trigger_2;
+\echo
+
+-- security definer function case
+set role privileged_role;
+create function secdef_show_current_user()
+    returns event_trigger
+    language plpgsql
+    security definer as
+$$
+begin
+    raise notice 'the event trigger is executed for %', current_user;
+end;
+$$;
+create event trigger event_trigger_3 on ddl_command_end
+execute procedure secdef_show_current_user();
+\echo
+
+-- secdef won't be executed for superuser
+set role postgres;
+create table super_foo();
+\echo
+
+-- secdef won't be executed for reserved roles
+set role supabase_storage_admin;
+create table storage_foo();
+\echo
+
+-- secdef will be executed for other roles
+set role rolecreator;
+create table rolecreator_foo();
+\echo
+
+-- secdef will be executed for privileged_role
+set role privileged_role;
+create table privileged_role_foo();
+\echo
+
+-- cleanup
+set role privileged_role;
+drop event trigger event_trigger_3;