pg_recvlogical: honor source cluster file permissions for output files

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

Latest Update

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:

Patch Status

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

History (1 prior analysis)
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.