fix: evtrig detection under another extension fmgr_hook

Makefile

@@ -29,6 +29,7 @@ OBJS = $(patsubst $(SRC_DIR)/%.c, $(BUILD_DIR)/%.o, $(SRC))
 PG_VERSION = $(strip $(shell $(PG_CONFIG) --version | $(GREP) -oP '(?<=PostgreSQL )[0-9]+'))
 # 0 is true
 PG_EQ15 = $(shell test $(PG_VERSION) -eq 15; echo $$?)
+PG_NEQ15 = $(shell test $(PG_VERSION) -ne 15; echo $$?)
 PG_GE16 = $(shell test $(PG_VERSION) -ge 16; echo $$?)
 PG_GE14 = $(shell test $(PG_VERSION) -ge 14; echo $$?)
 SYSTEM = $(shell uname -s)
@@ -43,6 +44,10 @@ else
 TESTS := $(filter-out test/sql/lt16_%.sql, $(TESTS))
 endif
 
+ifeq ($(PG_NEQ15), 0)
+TESTS := $(filter-out test/sql/eq15_%.sql, $(TESTS))
+endif
+
 REGRESS = $(patsubst test/sql/%.sql,%,$(TESTS))
 REGRESS_OPTS = --use-existing --inputdir=test
 

README.md

@@ -62,7 +62,7 @@ To protect against privilege escalation, the event triggers created by the privi
 
 - Will be executed for any non-superuser role.
 - Will be skipped for any superuser role.
-- For PostgreSQL < 16: Will also be skipped for [Reserved Roles](#reserved-roles).
+- Will also be skipped for [Reserved Roles](#reserved-roles).
 
 The skipping behavior can be logged by setting the `supautils.log_skipped_evtrigs` config to true, this is false by default.
 
@@ -226,7 +226,7 @@ supautils.drop_trigger_grants = '{ "my_role": ["public.not_my_table", "public.al
 ### Reserved Roles
 
 > [!IMPORTANT]
-> This feature is disabled starting from PostgreSQL 16, from this version onwards the underlying CREATEROLE problem is fixed.
+> The CREATEROLE problem is solved starting from PostgreSQL 16.
 
 Non-superusers with the CREATEROLE privilege can ALTER, DROP or GRANT non-superuser roles without restrictions.
 

nix/pgmq.nix

@@ -0,0 +1,30 @@
+{ lib, stdenv, fetchFromGitHub, postgresql }:
+
+stdenv.mkDerivation rec {
+  pname = "pgmq";
+  version = "1.4.4";
+  buildInputs = [ postgresql ];
+  src = fetchFromGitHub {
+    owner  = "tembo-io";
+    repo   = pname;
+    rev    = "v${version}";
+    hash = "sha256-z+8/BqIlHwlMnuIzMz6eylmYbSmhtsNt7TJf/CxbdVw=";
+  };
+
+  buildPhase = ''
+    cd pgmq-extension
+  '';
+
+  installPhase = ''
+    install -D sql/pgmq.sql "$out/pgmq--${version}.sql"
+    install -D -t $out sql/*.sql
+    install -D -t $out *.control
+  '';
+
+  meta = with lib; {
+    description = "A lightweight message queue. Like AWS SQS and RSMQ but on Postgres.";
+    homepage    = "https://github.com/tembo-io/pgmq";
+    platforms   = postgresql.meta.platforms;
+    license     = licenses.postgresql;
+  };
+}

shell.nix

@@ -10,8 +10,13 @@ mkShell {
       pgsqlcheck15 = callPackage ./nix/plpgsql-check.nix {
         postgresql = xpg.postgresql_15;
       };
+      pgmq15 = callPackage ./nix/pgmq.nix {
+        postgresql = xpg.postgresql_15;
+      };
     in
     [
-      (xpg.xpgWithExtensions { exts15 = [ pgsqlcheck15 ]; })
+      (xpg.xpgWithExtensions {
+        exts15 = [ pgsqlcheck15 pgmq15 ];
+      })
     ];
 }

src/event_triggers.c

@@ -1,5 +1,6 @@
 #include "pg_prelude.h"
 #include "event_triggers.h"
+#include "utils.h"
 
 // this is the underlying function of `select version();`
 extern Datum pgsql_version(PG_FUNCTION_ARGS);
@@ -44,3 +45,6 @@ Oid get_function_owner(func_owner_search search){
   return func_owner;
 }
 
+bool is_event_trigger_function(Oid foid){
+  return get_func_rettype(foid) == SUPAUTILS_EVENT_TRIGGER_OID;
+}

src/event_triggers.h

@@ -18,4 +18,6 @@ extern Oid get_function_owner(func_owner_search search);
 
 extern void force_noop(FmgrInfo *finfo);
 
+extern bool is_event_trigger_function(Oid foid);
+
 #endif

src/supautils.c

@@ -4,7 +4,6 @@
 #include "extensions_parameter_overrides.h"
 #include "policy_grants.h"
 #include "privileged_extensions.h"
-#include "utils.h"
 #include "event_triggers.h"
 
 #define EREPORT_RESERVED_MEMBERSHIP(name)                                   \
@@ -88,10 +87,7 @@ static bool supautils_needs_fmgr_hook(Oid functionId) {
   if (next_needs_fmgr_hook && (*next_needs_fmgr_hook) (functionId))
     return true;
 
-  if (get_func_rettype(functionId) == SUPAUTILS_EVENT_TRIGGER_OID)
-    return true;
-  else
-    return false;
+  return is_event_trigger_function(functionId);
 }
 
 // This function will fire twice: once before execution of the database function (event=FHET_START)
@@ -100,25 +96,27 @@ static void supautils_fmgr_hook(FmgrHookEventType event, FmgrInfo *flinfo, Datum
     switch (event) {
     // we only need to change behavior before the function gets executed
     case FHET_START: {
-        const char *current_role_name = GetUserNameFromId(GetUserId(), false);
-        const bool role_is_super = superuser();
-        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);
-            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))
-                  );
+        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();
+            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);
+                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))
+                      );
+                    }
+                    // we can't skip execution directly inside the fmgr_hook (although we can abort it with ereport)
+                    // so instead we use the workaround of changing the event trigger function to a noop function
+                    force_noop(flinfo);
                 }
-                // we can't skip execution directly inside the fmgr_hook (although we can abort it with ereport)
-                // so instead we use the workaround of changing the event trigger function to a noop function
-                force_noop(flinfo);
             }
         }
 

test/expected/eq15_pgmq.out

@@ -0,0 +1,133 @@
+-- create the extension as usual
+create extension if not exists pgmq;
+set supautils.log_skipped_evtrigs = true;
+\echo
+
+-- downgrade the extension functions owners to non-superuser, to ensure the following function calls are not wrongly skipped by the event trigger mechanism
+do $$
+declare
+  extoid oid := (select oid from pg_extension where extname = 'pgmq');
+  r record;
+  cls pg_class%rowtype;
+begin
+for r in (select * from pg_depend where refobjid = extoid) loop
+    if r.classid = 'pg_proc'::regclass then
+      execute(format('alter function %s(%s) owner to privileged_role;', r.objid::regproc, pg_get_function_identity_arguments(r.objid)));
+    end if;
+end loop;
+end $$;
+\echo
+
+-- Test the standard flow
+select
+  pgmq.create('Foo');
+ create 
+--------
+
+(1 row)
+
+select
+  *
+from
+  pgmq.send(
+    queue_name:='Foo',
+    msg:='{"foo": "bar1"}'
+  );
+ send 
+------
+    1
+(1 row)
+
+-- Test queue is not case sensitive
+select
+  *
+from
+  pgmq.send(
+    queue_name:='foo', -- note: lowercase useage
+    msg:='{"foo": "bar2"}',
+    delay:=5
+  );
+ send 
+------
+    2
+(1 row)
+
+select
+  msg_id,
+  read_ct,
+  message
+from
+  pgmq.read(
+    queue_name:='Foo',
+    vt:=30,
+    qty:=2
+  );
+ msg_id | read_ct |     message     
+--------+---------+-----------------
+      1 |       1 | {"foo": "bar1"}
+(1 row)
+
+select
+  msg_id,
+  read_ct,
+  message
+from
+  pgmq.pop('Foo');
+ msg_id | read_ct | message 
+--------+---------+---------
+(0 rows)
+
+-- Archive message with msg_id=2.
+select
+  pgmq.archive(
+    queue_name:='Foo',
+    msg_id:=2
+  );
+ archive 
+---------
+ t
+(1 row)
+
+select
+  pgmq.create('my_queue');
+ create 
+--------
+
+(1 row)
+
+select
+  pgmq.send_batch(
+  queue_name:='my_queue',
+  msgs:=array['{"foo": "bar3"}','{"foo": "bar4"}','{"foo": "bar5"}']::jsonb[]
+);
+ send_batch 
+------------
+          1
+          2
+          3
+(3 rows)
+
+select
+  pgmq.archive(
+    queue_name:='my_queue',
+    msg_ids:=array[3, 4, 5]
+  );
+ archive 
+---------
+       3
+(1 row)
+
+select
+  pgmq.delete('my_queue', 6);
+ delete 
+--------
+ f
+(1 row)
+
+select
+  pgmq.drop_queue('my_queue');
+ drop_queue 
+------------
+ t
+(1 row)
+

test/sql/eq15_pgmq.sql

@@ -0,0 +1,90 @@
+-- create the extension as usual
+create extension if not exists pgmq;
+set supautils.log_skipped_evtrigs = true;
+\echo
+
+-- downgrade the extension functions owners to non-superuser, to ensure the following function calls are not wrongly skipped by the event trigger mechanism
+do $$
+declare
+  extoid oid := (select oid from pg_extension where extname = 'pgmq');
+  r record;
+  cls pg_class%rowtype;
+begin
+for r in (select * from pg_depend where refobjid = extoid) loop
+    if r.classid = 'pg_proc'::regclass then
+      execute(format('alter function %s(%s) owner to privileged_role;', r.objid::regproc, pg_get_function_identity_arguments(r.objid)));
+    end if;
+end loop;
+end $$;
+\echo
+
+-- Test the standard flow
+select
+  pgmq.create('Foo');
+
+select
+  *
+from
+  pgmq.send(
+    queue_name:='Foo',
+    msg:='{"foo": "bar1"}'
+  );
+
+-- Test queue is not case sensitive
+select
+  *
+from
+  pgmq.send(
+    queue_name:='foo', -- note: lowercase useage
+    msg:='{"foo": "bar2"}',
+    delay:=5
+  );
+
+select
+  msg_id,
+  read_ct,
+  message
+from
+  pgmq.read(
+    queue_name:='Foo',
+    vt:=30,
+    qty:=2
+  );
+
+select
+  msg_id,
+  read_ct,
+  message
+from
+  pgmq.pop('Foo');
+
+
+-- Archive message with msg_id=2.
+select
+  pgmq.archive(
+    queue_name:='Foo',
+    msg_id:=2
+  );
+
+
+select
+  pgmq.create('my_queue');
+
+select
+  pgmq.send_batch(
+  queue_name:='my_queue',
+  msgs:=array['{"foo": "bar3"}','{"foo": "bar4"}','{"foo": "bar5"}']::jsonb[]
+);
+
+select
+  pgmq.archive(
+    queue_name:='my_queue',
+    msg_ids:=array[3, 4, 5]
+  );
+
+select
+  pgmq.delete('my_queue', 6);
+
+
+select
+  pgmq.drop_queue('my_queue');