public inbox for [email protected]help / color / mirror / Atom feed
Re: pg_recvlogical: honor source cluster file permissions for output files 4+ messages / 2 participants [nested] [flat]
* Re: pg_recvlogical: honor source cluster file permissions for output files @ 2026-05-18 10:47 Fujii Masao <[email protected]> 2026-05-18 10:48 ` Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> 2026-05-18 11:59 ` Re: pg_recvlogical: honor source cluster file permissions for output files Srinath Reddy Sadipiralla <[email protected]> 0 siblings, 2 replies; 4+ messages in thread From: Fujii Masao @ 2026-05-18 10:47 UTC (permalink / raw) To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Sun, May 17, 2026 at 12:22 AM Srinath Reddy Sadipiralla <[email protected]> wrote: > i have reviewed, tested the patch and it LGTM, Thanks for the review! > i think we can add > a TAP test to verify the group permission, attached a diff patch > for the same, Thanks for the patch! The test you added checks that pg_recvlogical creates output files with mode 0640 when the cluster is initialized with group access enabled. However, it does not check the opposite case, i.e., that pg_recvlogical creates output files with mode 0600 when group access is disabled. It seems we should test both cases, similar to what 010_basebackup.pl does? As far as I can tell, 010_basebackup.pl initializes the cluster without group access and checks the backup permissions, then enables group access using chmod_recursive() and verifies that group permissions are also applied to the backup. I updated the TAP test following this approach and attached a revised patch. >> I think this should be backpatched to all supported branches. > > > +1 I'm currently thinking of backpatching the fix itself to all supported branches, but adding the test only to master. Because it does not seem worthwhile to spend much time backporting the test to older branches, where the test code differs much from master. Thought? Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_recvlogical: honor source cluster file permissions for output files 2026-05-18 10:47 Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> @ 2026-05-18 10:48 ` Fujii Masao <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Fujii Masao @ 2026-05-18 10:48 UTC (permalink / raw) To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, May 18, 2026 at 7:47 PM Fujii Masao <[email protected]> wrote: > As far as I can tell, 010_basebackup.pl initializes the cluster without group > access and checks the backup permissions, then enables group access using > chmod_recursive() and verifies that group permissions are also applied to > the backup. I updated the TAP test following this approach and attached > a revised patch. Sorry, I forgot to attach the patch. I've attached it in this email. Regards, -- Fujii Masao Attachments: [application/octet-stream] v2-0001-pg_recvlogical-Honor-source-cluster-file-permissi.patch (3.9K, 2-v2-0001-pg_recvlogical-Honor-source-cluster-file-permissi.patch) download | inline diff: From 19e42b14119297daf8a4bbedd4578733dd85412b Mon Sep 17 00:00:00 2001 From: Fujii Masao <[email protected]> Date: Fri, 15 May 2026 22:47:28 +0900 Subject: [PATCH v2] pg_recvlogical: Honor source cluster file permissions for output files Commit c37b3d08ca6 attempted to preserve group permissions on pg_recvlogical output files when group access was enabled on the source cluster. However, the output files were still created with a fixed S_IRUSR | S_IWUSR mode, preventing group-read permissions from being applied. This commit fixes the issue by creating output files with pg_file_create_mode instead of a hard-coded mode. This allows pg_recvlogical to correctly preserve group permissions from the source cluster. Backpatch to all supported branches. --- doc/src/sgml/ref/pg_recvlogical.sgml | 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 46 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml index 5380d776baf..5f76e424e26 100644 --- a/doc/src/sgml/ref/pg_recvlogical.sgml +++ b/doc/src/sgml/ref/pg_recvlogical.sgml @@ -494,7 +494,7 @@ PostgreSQL documentation <para> <application>pg_recvlogical</application> will preserve group permissions on - the received WAL files if group permissions are enabled on the source + the output files if group permissions are enabled on the source cluster. </para> diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c index be71783b370..2fdf64bcadb 100644 --- a/src/bin/pg_basebackup/pg_recvlogical.c +++ b/src/bin/pg_basebackup/pg_recvlogical.c @@ -342,7 +342,7 @@ StreamLogicalLog(void) outfd = fileno(stdout); else outfd = open(outfile, O_CREAT | O_APPEND | O_WRONLY | PG_BINARY, - S_IRUSR | S_IWUSR); + pg_file_create_mode); if (outfd == -1) { pg_log_error("could not open log file \"%s\": %m", outfile); diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl index 063ad96b9be..945a242bdad 100644 --- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl +++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl @@ -236,6 +236,52 @@ my $count = (() = $outfiledata =~ /INSERT/g); cmp_ok($count, '==', 2, 'pg_recvlogical has received and written two INSERTs'); +# Check that pg_recvlogical derives output file permissions from the source +# cluster. +SKIP: +{ + skip "unix-style permissions not supported on Windows", 2 + if ($Config{osname} eq 'MSWin32' || $Config{osname} eq 'cygwin'); + + # The cluster was initialized without group access, so pg_recvlogical + # should create the output file as 0600 (-rw-------). + my $mode = sprintf('%04o', (stat($outfile))[2] & 07777); + is($mode, '0600', + 'pg_recvlogical output file has no group permissions (0600)'); + + # Enable group access on the source cluster and its files, then restart + # so pg_recvlogical observes the updated source cluster permissions. + $node->stop; + chmod_recursive($node->data_dir, 0750, 0640); + $node->start; + + $outfile = $node->basedir . '/group_access.out'; + @pg_recvlogical_cmd = ( + 'pg_recvlogical', + '--slot' => 'reconnect_test', + '--dbname' => $node->connstr('postgres'), + '--start', + '--file' => $outfile, + '--fsync-interval' => '1'); + + $recv = IPC::Run::start( + [@pg_recvlogical_cmd], + '>' => \$stdout, + '2>' => \$stderr); + + $node->safe_psql('postgres', 'INSERT INTO test_table VALUES (3)'); + wait_for_file($outfile, qr/INSERT/); + + $recv->signal('TERM'); + $recv->finish(); + + # With group access enabled on the source cluster, pg_recvlogical should + # create the output file as 0640 (-rw-r-----). + $mode = sprintf('%04o', (stat($outfile))[2] & 07777); + is($mode, '0640', + 'pg_recvlogical output file respects group permissions (0640)'); +} + $node->command_ok( [ 'pg_recvlogical', -- 2.53.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_recvlogical: honor source cluster file permissions for output files 2026-05-18 10:47 Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> @ 2026-05-18 11:59 ` Srinath Reddy Sadipiralla <[email protected]> 2026-05-20 07:04 ` Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> 1 sibling, 1 reply; 4+ messages in thread From: Srinath Reddy Sadipiralla @ 2026-05-18 11:59 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, May 18, 2026 at 4:17 PM Fujii Masao <[email protected]> wrote: > > > The test you added checks that pg_recvlogical creates output files with > mode 0640 when the cluster is initialized with group access enabled. > However, it does not check the opposite case, i.e., that pg_recvlogical > creates output files with mode 0600 when group access is disabled. > > It seems we should test both cases, similar to what 010_basebackup.pl > does? > > As far as I can tell, 010_basebackup.pl initializes the cluster without > group > access and checks the backup permissions, then enables group access using > chmod_recursive() and verifies that group permissions are also applied to > the backup. I updated the TAP test following this approach and attached > a revised patch. > +1 , I totally missed this case :), patch LGTM. > > I'm currently thinking of backpatching the fix itself to all supported > branches, > but adding the test only to master. Because it does not seem worthwhile to > spend much time backporting the test to older branches, where the test code > differs much from master. Thought? > makes sense. -- Thanks, Srinath Reddy Sadipiralla EDB: https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pg_recvlogical: honor source cluster file permissions for output files 2026-05-18 10:47 Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> 2026-05-18 11:59 ` Re: pg_recvlogical: honor source cluster file permissions for output files Srinath Reddy Sadipiralla <[email protected]> @ 2026-05-20 07:04 ` Fujii Masao <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Fujii Masao @ 2026-05-20 07:04 UTC (permalink / raw) To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Mon, May 18, 2026 at 8:59 PM Srinath Reddy Sadipiralla <[email protected]> wrote: > > +1 , I totally missed this case :), patch LGTM. I've pushed the patch. Thanks! Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-05-20 07:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-05-18 10:47 Re: pg_recvlogical: honor source cluster file permissions for output files Fujii Masao <[email protected]> 2026-05-18 10:48 ` Fujii Masao <[email protected]> 2026-05-18 11:59 ` Srinath Reddy Sadipiralla <[email protected]> 2026-05-20 07:04 ` Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox