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

[PATCH] allow pg_current_logfile() execution under pg_monitor role

From
Pavlo Golub
Date:
Hello,

The patch attached fixes an oversight/inconsistency of disallowing the pg_monitor system role to execute pg_current_logfile([text]).

pgwatch3=# create user joe;
CREATE ROLE
pgwatch3=# set role joe;
SET
pgwatch3=> select pg_current_logfile();
ERROR:  permission denied for function pg_current_logfile
pgwatch3=> reset role;
RESET
pgwatch3=# grant pg_monitor to joe;
GRANT ROLE
pgwatch3=# set role joe;
SET
pgwatch3=> select pg_current_logfile();
ERROR:  permission denied for function pg_current_logfile
pgwatch3=> select * FROM pg_ls_logdir();
               name               |   size   |      modification
----------------------------------+----------+------------------------
 postgresql-2024-02-08_130906.log |      652 | 2024-02-08 13:10:04+01
(5 rows)

Best regards,
Pavlo Golub
Attachment

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

From
Nathan Bossart
Date:
On Fri, Feb 09, 2024 at 04:01:58PM +0100, Pavlo Golub wrote:
> The patch attached fixes an oversight/inconsistency of disallowing the
> pg_monitor system role to execute pg_current_logfile([text]).

I think this is reasonable.  We allow pg_monitor to execute functions like
pg_ls_logdir(), so it doesn't seem like much of a stretch to expect it to
have privileges for pg_current_logfile(), too.  Are there any other
functions that pg_monitor ought to have privileges for?

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



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

From
"Pavlo Golub"
Date:
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.

Kind regards,
Pavlo Golub

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

From
Nathan Bossart
Date:
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



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

From
Nathan Bossart
Date:
On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
> Okay.  I'll plan on committing this in the next few days.

Here is what I have staged for commit.

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

Attachment

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

From
Daniel Gustafsson
Date:
> On 13 Feb 2024, at 22:29, Nathan Bossart <nathandbossart@gmail.com> wrote:
> 
> On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
>> Okay.  I'll plan on committing this in the next few days.
> 
> Here is what I have staged for commit.

LGTM.

--
Daniel Gustafsson




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

From
Nathan Bossart
Date:
On Wed, Feb 14, 2024 at 08:59:06AM +0100, Daniel Gustafsson wrote:
> LGTM.

Committed.

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



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

From
"Pavlo Golub"
Date:

>On Mon, Feb 12, 2024 at 09:49:45AM -0600, Nathan Bossart wrote:
>>  Okay.  I'll plan on committing this in the next few days.
>
>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



"Pavlo Golub" <pavlo.golub@cybertec.at> writes:
> Oh, thanks! I forgot, indeed, to update docs and catalog version! My 
> bad!

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





On Wed, Feb 14, 2024, 19:45 Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Pavlo Golub" <pavlo.golub@cybertec.at> writes:
> Oh, thanks! I forgot, indeed, to update docs and catalog version! My
> bad!

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...)

Thanks for the clarification. 


                        regards, tom lane
On Wed, Feb 14, 2024 at 01:45:31PM -0500, Tom Lane wrote:
> "Pavlo Golub" <pavlo.golub@cybertec.at> writes:
>> Oh, thanks! I forgot, indeed, to update docs and catalog version! My 
>> bad!
> 
> 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...)

+1, I only included it in the patch I posted so that I didn't forget it...

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