pg_recvlogical: honor source cluster file permissions for output files

First seen: 2026-05-15 14:59:15+00:00 · Messages: 6 · Participants: 2

Latest Update

2026-05-22 · claude-opus-4-6

Incremental Update: Patch Committed

What's New

Fujii Masao has committed the patch. His message "I've pushed the patch. Thanks!" confirms the fix has been applied to the repository. This closes out the thread.

No additional technical discussion, no changes to the patch between the last review and the commit. The split backpatch strategy (code fix to all supported branches, TAP test to master only) was the final agreed-upon approach.

History (2 prior analyses)
2026-05-20 · claude-opus-4-6

Incremental Update: Test Coverage Expansion and Backpatch Strategy

What's New

Fujii Masao reviewed the TAP test proposed by Srinath and identified a gap: the test only verified the group-access-enabled case (mode 0640) but not the default case (mode 0600 when group access is disabled). He revised the patch to test both scenarios, following the established pattern from 010_basebackup.pl which uses chmod_recursive() to toggle group access and verify permissions in both states.

Backpatch Decision

Fujii proposes a split backpatch strategy:

  • Code fix: Backpatch to all supported branches (fixes the actual permission bug)
  • TAP test: Apply only to master (not worth the effort to backport due to differences in test infrastructure across older branches)

Patch Status

Srinath acknowledged the revised patch with a "+1, LGTM" — no further technical objections raised. The patch appears ready for commit.

2026-05-18 · claude-opus-4-6

pg_recvlogical: File Permission Bug Analysis

Core Problem

pg_recvlogical is PostgreSQL's tool for receiving logical replication changes from a source cluster. Its documentation claims it preserves group permissions on output files when group permissions are enabled on the source cluster. However, this behavior is broken — output files are always created with hard-coded S_IRUSR | S_IWUSR (0600) permissions, meaning only the file owner can read/write them, regardless of the source cluster's permission configuration.

Technical Background

PostgreSQL's Permission Model

PostgreSQL has a cluster-wide setting for file permissions. When data_directory_mode includes group permissions (mode 0750 vs 0700), various tools are supposed to honor this by:

  1. Setting umask(pg_mode_mask) — a mask derived from the source cluster's data_directory_mode
  2. Using pg_file_create_mode (typically 0640 when group access is enabled, 0600 otherwise) when creating files

The key variables are:

  • pg_mode_mask: The umask to apply (e.g., 0027 for group-readable, 0077 for owner-only)
  • pg_file_create_mode: The mode to pass to open()/fopen() calls (e.g., 0600 or 0640)

The Bug's Origin

Commit c37b3d08ca6 was intended to make pg_recvlogical preserve group permissions by calling umask(pg_mode_mask) based on the source cluster settings. However, this fix was incomplete. While the umask was correctly set, the actual file creation calls still used a hard-coded mode of S_IRUSR | S_IWUSR (0600).

The umask is a subtractive mechanism — it can only remove permission bits, never add them. So even with umask(0027) (which would allow group-read), if you create a file with mode 0600, the result is still 0600 because there are no group bits to subtract from. The umask setting is effectively a no-op when the base mode doesn't include group bits.

Proposed Fix

The patch makes two changes:

1. Code Fix: Use pg_file_create_mode instead of hard-coded mode

By replacing the hard-coded S_IRUSR | S_IWUSR with pg_file_create_mode, the output files will be created with the correct base permissions (0640 when group access is enabled). Combined with the already-set umask(pg_mode_mask), the resulting file permissions will correctly match the source cluster's configuration.

2. Documentation Fix: "received WAL files" → "output files"

The documentation incorrectly referred to "received WAL files" — but pg_recvlogical doesn't write WAL files. It writes logical decoding output files (containing decoded changesets). This is a distinct tool from pg_receivewal which does write actual WAL files. The documentation correction properly describes what pg_recvlogical actually produces.

Architectural Significance

This bug matters for several reasons:

  1. Security posture: Organizations using group permissions typically do so to allow monitoring or backup processes to read PostgreSQL files without running as the postgres user. If pg_recvlogical output files are incorrectly restricted, these operational workflows break silently.

  2. Consistency: PostgreSQL's file permission model is designed to be cluster-wide and consistent. Having one tool ignore the configured permissions creates an inconsistency that's hard to diagnose.

  3. Backpatch scope: The bug exists in all supported branches since c37b3d08ca6 was committed, meaning it affects production deployments that rely on this documented behavior.

Additional Proposed Enhancement: TAP Test

The reviewer proposes adding a TAP test to verify group permission behavior. This is important because:

  • The original commit c37b3d08ca6 shipped without adequate test coverage for the permission behavior
  • A regression test would prevent future changes from re-introducing this class of bug
  • TAP tests in src/bin/pg_basebackup/ (where pg_recvlogical tests live) can inspect file modes on the filesystem

Assessment

This is a straightforward bug fix with clear correctness implications. The fix is minimal and surgical — replacing a constant with the appropriate variable that already exists and is already initialized correctly. The backpatch request is well-justified since the documented behavior has never actually worked correctly.