diff --git a/packaging/rpm/post.sh b/packaging/rpm/post.sh index 3699e9e..085364e 100755 --- a/packaging/rpm/post.sh +++ b/packaging/rpm/post.sh @@ -44,4 +44,6 @@ print(f'Fenris migration: {n} step(s) applied') if n else None fi rm -f "${OLD_CONTENT}" done + # Re-apply placement modes (store dir group access, issue #54) + systemd-tmpfiles --create || true fi diff --git a/packaging/tmpfiles.d/fenris.conf b/packaging/tmpfiles.d/fenris.conf index b14194f..ae19be6 100644 --- a/packaging/tmpfiles.d/fenris.conf +++ b/packaging/tmpfiles.d/fenris.conf @@ -1,2 +1,2 @@ # Type Path Mode User Group Age Argument -d /var/lib/fenris 2750 root fenris - - +d /var/lib/fenris 2770 root fenris - - diff --git a/src/fenris/status.py b/src/fenris/status.py index 0100899..781a839 100644 --- a/src/fenris/status.py +++ b/src/fenris/status.py @@ -88,7 +88,13 @@ def open_store_readonly(store_path: Path) -> sqlite3.Connection: Raises StoreFault if unreadable, NewerSchema if user_version > SCHEMA_VERSION. """ - if not store_path.exists(): + try: + exists = store_path.exists() + except OSError as e: + # A non-group user stat()ing a 2750 store directory gets + # PermissionError before any StoreFault can be raised (issue #54). + raise StoreFault("observation store not readable: %s" % e) + if not exists: raise StoreFault("observation store not found at %s" % store_path) try: diff --git a/src/fenris/store.py b/src/fenris/store.py index 01396f1..7e3f43e 100644 --- a/src/fenris/store.py +++ b/src/fenris/store.py @@ -37,10 +37,24 @@ def init_store(store_path: Path) -> sqlite3.Connection: Returns a connection to the store. """ conn = sqlite3.connect(str(store_path)) - + # Enable WAL mode for concurrent reads during writes conn.execute("PRAGMA journal_mode=WAL") - + + # Group members (fenris group) read the live store read-only, but SQLite + # in WAL mode needs write access to the db and its -wal/-shm sidecars even + # for readers. Best effort: root-created stores stay group-accessible + # without relying on the creating process's umask (issue #54). + import os as _os + for sidecar in (store_path, + store_path.with_name(store_path.name + "-wal"), + store_path.with_name(store_path.name + "-shm")): + try: + mode = _os.stat(sidecar).st_mode & 0o777 + _os.chmod(sidecar, mode | 0o060) + except OSError: + pass + # Check if this is a new database cursor = conn.execute("PRAGMA user_version") current_version = cursor.fetchone()[0] diff --git a/tests/test_store_group_access.py b/tests/test_store_group_access.py new file mode 100644 index 0000000..184a830 --- /dev/null +++ b/tests/test_store_group_access.py @@ -0,0 +1,79 @@ +"""Group access to the observation store — regression coverage for issue #54. + +Two defects: (1) a non-group user's stat() on the store directory raised +PermissionError straight through open_store_readonly(), crashing status/TUI +instead of degrading to the Store fault view; (2) even group members could +not open the WAL-mode store because root-created sidecars lacked group write +and the store directory lacked group execute-then-write. +""" +import sqlite3 +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "src")) + +from fenris.store import DEFAULT_STORE_PATH, init_store +from fenris.status import StoreFault, open_store_readonly + + +def test_stat_permission_error_becomes_store_fault(monkeypatch, tmp_path): + """stat() denied (non-group user on a 2750 dir) → StoreFault, not crash.""" + store = tmp_path / "observations.db" + store.write_bytes(b"") + + import pathlib + + def denied(self, follow_symlinks=True): + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr(pathlib.Path, "exists", denied) + with pytest.raises(StoreFault): + open_store_readonly(store) + + +def test_connect_failure_becomes_store_fault(tmp_path): + """sqlite failures stay wrapped as StoreFault (existing contract).""" + garbage = tmp_path / "observations.db" + garbage.write_bytes(b"not a database" * 100) + with pytest.raises(StoreFault): + open_store_readonly(garbage) + + +def test_init_store_leaves_files_group_writable(tmp_path): + """Root-created stores must stay readable by WAL readers: db and sidecars + need group write after init_store (issue #54).""" + store = tmp_path / "observations.db" + conn = init_store(store) + try: + assert (store.stat().st_mode & 0o060) == 0o060, "db not group rw" + wal = store.with_name(store.name + "-wal") + shm = store.with_name(store.name + "-shm") + if wal.exists(): + assert (wal.stat().st_mode & 0o060) == 0o060, "wal not group rw" + if shm.exists(): + assert (shm.stat().st_mode & 0o060) == 0o060, "shm not group rw" + finally: + conn.close() + + +def test_readonly_open_works_after_init_store(tmp_path): + """The shipped read path opens a store created by init_store.""" + store = tmp_path / "observations.db" + writer = init_store(store) + writer.execute("INSERT INTO monitoring_periods (started_at) VALUES ('2026-01-01T00:00:00+00:00')") + writer.commit() + conn = open_store_readonly(store) + assert conn is not None + conn.close() + writer.close() + + +def test_packaging_ships_group_access(): + """tmpfiles must create the store dir group-writable; collect unit must + keep the umask loose so root-created sidecars stay group-accessible.""" + repo = Path(__file__).resolve().parent.parent + assert "2770" in (repo / "packaging" / "tmpfiles.d" / "fenris.conf").read_text() + assert "2750" not in (repo / "packaging" / "tmpfiles.d" / "fenris.conf").read_text() + assert "UMask=002" in (repo / "units" / "fenris-collect.service").read_text() diff --git a/units/fenris-collect.service b/units/fenris-collect.service index e2dffb3..970c9ec 100644 --- a/units/fenris-collect.service +++ b/units/fenris-collect.service @@ -7,3 +7,4 @@ After=local-fs.target Type=oneshot ExecStart=/usr/libexec/fenris/fenris-collect TimeoutStartSec=90 +UMask=002