Skip to content

Commit c7ce80e

Browse files
committed
Create the history, log and config files readable by their owner only
They are created with whatever the process umask happens to be, which is 0644 on most systems and 0664 on a desktop with a per-user group. The history records every statement typed, including "alter role ... password", the log carries the same at DEBUG level, and the config can hold passwords in [alias_dsn] entries. psql keeps its readline history at 0600 and libpq refuses to use a .pgpass that is not, so pgcli is the odd one out here. ensure_private_file() creates the file with 0600 and tightens an existing one that still has group or other bits. It opens with O_NONBLOCK and works on the descriptor it holds: only a regular file owned by the current user is chmod-ed, so "history_file = /dev/null" (the usual way to record nothing) is left alone rather than being made 0600 for the whole machine when pgcli runs as root, a FIFO cannot hang startup, and there is no window between the stat and the chmod. Any OSError is ignored, so a read-only or exotic filesystem never keeps pgcli from starting. The log is secured before logging.FileHandler creates it, so it never exists with the umask mode.
1 parent 101e523 commit c7ce80e

4 files changed

Lines changed: 120 additions & 1 deletion

File tree

‎changelog.rst‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
Upcoming
22
========
33

4+
Bug fixes:
5+
----------
6+
* Create the history, log and config files readable by their owner only
7+
(0600). They were left to the process umask, which is 0644 or 0664 on a
8+
typical desktop, while the history records every statement typed,
9+
``alter role ... password`` included, the log carries the same at DEBUG
10+
level and the config can hold passwords in ``[alias_dsn]``. psql keeps its
11+
history at 0600 and libpq requires it for ``.pgpass``. An existing history
12+
or log with wider permissions is tightened on the next run; device nodes
13+
such as ``/dev/null``, FIFOs and files owned by someone else are left
14+
alone.
15+
416

517
4.7.1 (2026-09-20)
618
==================

‎pgcli/config.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import shutil
22
import os
33
import platform
4+
import stat
45
from os.path import expanduser, exists, dirname
56
import re
67
from typing import TextIO
@@ -34,6 +35,44 @@ def ensure_dir_exists(path):
3435
os.makedirs(parent_dir, exist_ok=True)
3536

3637

38+
def ensure_private_file(path):
39+
"""Create ``path`` if missing and keep it readable by its owner only.
40+
41+
The history records every statement typed, ``alter role ... password``
42+
included, the log can carry the same at DEBUG level and the config can
43+
hold passwords in ``[alias_dsn]`` entries. psql keeps its history at 0600
44+
and libpq refuses a ``.pgpass`` that is not, so pgcli does the same
45+
instead of leaving these to the process umask.
46+
47+
Only a regular file owned by the current user is touched: a history_file
48+
pointing at ``/dev/null`` (the usual way to record nothing), a FIFO or a
49+
directory is left alone, and any OSError (read-only media, exotic
50+
filesystems) is ignored so a permissions problem never keeps pgcli from
51+
starting.
52+
"""
53+
path = expanduser(path)
54+
# O_NONBLOCK: opening a FIFO with no reader must fail, not hang pgcli.
55+
flags = os.O_WRONLY | os.O_CREAT | os.O_APPEND | getattr(os, "O_NONBLOCK", 0) | getattr(os, "O_NOCTTY", 0)
56+
try:
57+
fd = os.open(path, flags, 0o600)
58+
except OSError:
59+
return
60+
try:
61+
st = os.fstat(fd)
62+
owner = getattr(os, "getuid", lambda: st.st_uid)()
63+
if stat.S_ISREG(st.st_mode) and st.st_uid == owner and stat.S_IMODE(st.st_mode) & 0o077:
64+
# fchmod on the descriptor we already hold: no path lookup
65+
# between the stat and the chmod.
66+
if hasattr(os, "fchmod"):
67+
os.fchmod(fd, 0o600)
68+
else:
69+
os.chmod(path, 0o600)
70+
except OSError:
71+
pass
72+
finally:
73+
os.close(fd)
74+
75+
3776
def write_default_config(source, destination, overwrite=False):
3877
destination = expanduser(destination)
3978
if not overwrite and exists(destination):
@@ -42,6 +81,7 @@ def write_default_config(source, destination, overwrite=False):
4281
ensure_dir_exists(destination)
4382

4483
shutil.copyfile(source, destination)
84+
ensure_private_file(destination)
4585

4686

4787
def upgrade_config(config, def_config):

‎pgcli/main.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
load_config,
6767
config_location,
6868
ensure_dir_exists,
69+
ensure_private_file,
6970
get_config,
7071
get_config_filename,
7172
)
@@ -660,6 +661,9 @@ def initialize_logging(self):
660661
if log_level.upper() == "NONE":
661662
handler = logging.NullHandler()
662663
else:
664+
# Before the handler creates it, so the file never exists with
665+
# the umask mode.
666+
ensure_private_file(log_file)
663667
handler = logging.FileHandler(os.path.expanduser(log_file))
664668

665669
level_map = {
@@ -1110,6 +1114,7 @@ def run_cli(self):
11101114
history_file = self.config["main"]["history_file"]
11111115
if history_file == "default":
11121116
history_file = config_location() + "history"
1117+
ensure_private_file(history_file)
11131118
history = FileHistory(os.path.expanduser(history_file))
11141119
self.refresh_completions(history=history, persist_priorities="none")
11151120

‎tests/test_config.py‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44

55
import pytest
66

7-
from pgcli.config import ensure_dir_exists, skip_initial_comment
7+
from pgcli.config import (
8+
ensure_dir_exists,
9+
ensure_private_file,
10+
skip_initial_comment,
11+
write_default_config,
12+
)
813

914

1015
def test_ensure_file_parent(tmpdir):
@@ -41,3 +46,60 @@ def test_ensure_other_create_error(tmpdir):
4146
)
4247
def test_skip_initial_comment(text, skipped_lines):
4348
assert skip_initial_comment(io.StringIO(text)) == skipped_lines
49+
50+
51+
posix_only = pytest.mark.skipif(os.name != "posix", reason="POSIX file modes")
52+
53+
54+
@posix_only
55+
def test_ensure_private_file_creates_owner_only(tmp_path):
56+
path = tmp_path / "history"
57+
ensure_private_file(str(path))
58+
assert path.exists() and path.read_bytes() == b""
59+
assert stat.S_IMODE(path.stat().st_mode) == 0o600
60+
61+
62+
@posix_only
63+
def test_ensure_private_file_tightens_existing_and_keeps_content(tmp_path):
64+
path = tmp_path / "history"
65+
path.write_text("select 1\n")
66+
path.chmod(0o664)
67+
ensure_private_file(str(path))
68+
assert stat.S_IMODE(path.stat().st_mode) == 0o600
69+
assert path.read_text() == "select 1\n"
70+
71+
72+
@posix_only
73+
def test_ensure_private_file_leaves_device_nodes_alone():
74+
# history_file = /dev/null is the usual way to record nothing; running as
75+
# root, a chmod here would change it for every user on the machine.
76+
before = os.stat("/dev/null").st_mode
77+
ensure_private_file("/dev/null")
78+
assert os.stat("/dev/null").st_mode == before
79+
80+
81+
@posix_only
82+
def test_ensure_private_file_does_not_block_on_a_fifo(tmp_path):
83+
import threading
84+
85+
fifo = tmp_path / "history"
86+
os.mkfifo(fifo)
87+
worker = threading.Thread(target=ensure_private_file, args=(str(fifo),), daemon=True)
88+
worker.start()
89+
worker.join(timeout=5)
90+
assert not worker.is_alive(), "open() on a reader-less FIFO must not hang pgcli"
91+
92+
93+
@posix_only
94+
def test_ensure_private_file_ignores_a_directory(tmp_path):
95+
ensure_private_file(str(tmp_path))
96+
assert tmp_path.is_dir()
97+
98+
99+
@posix_only
100+
def test_write_default_config_is_owner_only(tmp_path):
101+
source = tmp_path / "pgclirc"
102+
source.write_text("[main]\n")
103+
destination = tmp_path / "config"
104+
write_default_config(str(source), str(destination))
105+
assert stat.S_IMODE(destination.stat().st_mode) == 0o600

0 commit comments

Comments
 (0)