Subject: Re: [PATCH] allow pg_current_logfile() execution under pg_monitor role

From: Nathan Bossart <[email-masked]>
Date: Mon, 12 Feb 2024 09:49:45 -0600

On Mon, Feb 12, 2024 at 12:27:54PM +0000, Pavlo Golub wrote:
>>  Are there any other
>> functions that pg_monitor ought to have privileges for?
>> 
> Not that I'm aware of at the moment. This one was found by chance.

Okay.  I'll plan on committing this in the next few days.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 13 Feb 2024 15:29:28 -0600

> Okay.  I'll plan on committing this in the next few days.

Here is what I have staged for commit.

>From bfe542c5d7b3c981e75ac6551abb34fbdf646eea Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Tue, 13 Feb 2024 15:12:36 -0600
Subject: [PATCH v2 1/1] Allow pg_monitor to execute pg_current_logfile().

We allow roles with privileges of pg_monitor to execute functions
like pg_ls_logdir(), so it seems natural that such roles would also
be able to execute this function.

Bumps catversion.

Co-authored-by: Pavlo Golub
Discussion: https://postgr.es/m/CAK7ymcLmEYWyQkiCZ64WC-HCzXAB0omM%3DYpj9B3rXe8vUAFMqw%40mail.gmail.com
---
 doc/src/sgml/func.sgml                       |  5 +++++
 src/backend/catalog/system_functions.sql     |  4 ++++
 src/include/catalog/catversion.h             |  2 +-
 src/test/regress/expected/misc_functions.out | 20 ++++++++++++++++++++
 src/test/regress/sql/misc_functions.sql      | 11 +++++++++++
 5 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 11d537b341..c4e5b4967e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23735,6 +23735,11 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
         <xref linkend="guc-log-destination"/>.
         The result reflects the contents of
         the <filename>current_logfiles</filename> file.
+       </para>
+       <para>
+        This function is restricted to superusers and roles with privileges of
+        the <literal>pg_monitor</literal> role by default, but other users can
+        be granted EXECUTE to run the function.
        </para></entry>
       </row>
 
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 346cfb98a0..fe2bb50f46 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -777,6 +777,10 @@ GRANT EXECUTE ON FUNCTION pg_ls_logicalmapdir() TO pg_monitor;
 
 GRANT EXECUTE ON FUNCTION pg_ls_replslotdir(text) TO pg_monitor;
 
+GRANT EXECUTE ON FUNCTION pg_current_logfile() TO pg_monitor;
+
+GRANT EXECUTE ON FUNCTION pg_current_logfile(text) TO pg_monitor;
+
 GRANT pg_read_all_settings TO pg_monitor;
 
 GRANT pg_read_all_stats TO pg_monitor;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9fc8ac9290..80a4c19565 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202401301
+#define CATALOG_VERSION_NO	202402131
 
 #endif
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 7c15477104..d5f61dfad9 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -683,3 +683,23 @@ SELECT gist_stratnum_identity(18::smallint);
                      18
 (1 row)
 
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+  'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege 
+------------------------
+ f
+(1 row)
+
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+  'pg_current_logfile()', 'EXECUTE');
+ has_function_privilege 
+------------------------
+ t
+(1 row)
+
+DROP ROLE regress_current_logfile;
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
index 851dad90f4..928b04db7f 100644
--- a/src/test/regress/sql/misc_functions.sql
+++ b/src/test/regress/sql/misc_functions.sql
@@ -254,3 +254,14 @@ FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size - 1),
 -- test stratnum support functions
 SELECT gist_stratnum_identity(3::smallint);
 SELECT gist_stratnum_identity(18::smallint);
+
+-- pg_current_logfile
+CREATE ROLE regress_current_logfile;
+-- not available by default
+SELECT has_function_privilege('regress_current_logfile',
+  'pg_current_logfile()', 'EXECUTE');
+GRANT pg_monitor TO regress_current_logfile;
+-- role has privileges of pg_monitor and can execute the function
+SELECT has_function_privilege('regress_current_logfile',
+  'pg_current_logfile()', 'EXECUTE');
+DROP ROLE regress_current_logfile;
-- 
2.25.1

---

From: Daniel Gustafsson <[email-masked]>
Date: Wed, 14 Feb 2024 08:59:06 +0100

> On 13 Feb 2024, at 22:29, Nathan Bossart <[email-masked]> wrote:
> 

LGTM.

--
Daniel Gustafsson

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Feb 2024 11:50:23 -0600

> LGTM.

Committed.

---

From: "Pavlo Golub" <[email-masked]>
Date: Wed, 14 Feb 2024 18:05:41 +0000

>Here is what I have staged for commit.
Oh, thanks! I forgot, indeed, to update docs and catalog version! My 
bad!
In my defense, I was trying to find tests but I missed 
regress/sql/misc_functions.sql somehow.
Now I will know. Thanks again!

Best regards,
Pavlo Golub

---

From: Tom Lane <[email-masked]>
Date: Wed, 14 Feb 2024 13:45:31 -0500

> Oh, thanks! I forgot, indeed, to update docs and catalog version! My 

Docs, yes, but don't include catversion bumps in submitted patches.
They'll just lead to merge problems when somebody else changes the
current catversion.  We rely on the committer to remember to do this
(which is an imperfect system, but...)

			regards, tom lane

---

From: Pavlo Golub <[email-masked]>
Date: Wed, 14 Feb 2024 19:47:07 +0100

> (which is an imperfect system, but...)

Thanks for the clarification.