diff --git a/booru_viewer/core/db.py b/booru_viewer/core/db.py index 39963b9..45440f2 100644 --- a/booru_viewer/core/db.py +++ b/booru_viewer/core/db.py @@ -11,7 +11,7 @@ from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path -from .config import db_path +from .config import IS_WINDOWS, db_path def _validate_folder_name(name: str) -> str: @@ -210,8 +210,30 @@ class Database: self._conn.execute("PRAGMA foreign_keys=ON") self._conn.executescript(_SCHEMA) self._migrate() + self._restrict_perms() return self._conn + def _restrict_perms(self) -> None: + """Tighten the DB file (and WAL/SHM sidecars) to 0o600 on POSIX. + + The sites table stores api_key + api_user in plaintext, so the + file must not be readable by other local users. Sidecars only + exist after the first WAL checkpoint, so we tolerate + FileNotFoundError. Windows: NTFS ACLs handle this; chmod is a + no-op there. Filesystem-level chmod failures are swallowed — + better to keep working than refuse to start. + """ + if IS_WINDOWS: + return + for suffix in ("", "-wal", "-shm"): + target = Path(str(self._path) + suffix) if suffix else self._path + try: + os.chmod(target, 0o600) + except FileNotFoundError: + pass + except OSError: + pass + @contextmanager def _write(self): """Context manager for write methods. diff --git a/tests/core/test_db.py b/tests/core/test_db.py index 6413d25..cf36c06 100644 --- a/tests/core/test_db.py +++ b/tests/core/test_db.py @@ -13,6 +13,9 @@ These tests lock in the `54ccc40` security/correctness fixes: from __future__ import annotations +import os +import sys + import pytest from booru_viewer.core.db import _validate_folder_name @@ -42,6 +45,34 @@ def test_validate_folder_name_rejects_traversal(): _validate_folder_name("~user") # leading tilde +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only chmod check") +def test_db_file_chmod_600(tmp_db): + """Audit finding #4: the SQLite file must be 0o600 on POSIX so the + plaintext api_key/api_user columns aren't readable by other local + users on shared workstations.""" + # The conn property triggers _restrict_perms() the first time it's + # accessed; tmp_db calls it via add_site/etc., but a defensive + # access here makes the assertion order-independent. + _ = tmp_db.conn + mode = os.stat(tmp_db._path).st_mode & 0o777 + assert mode == 0o600, f"expected 0o600, got {oct(mode)}" + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only chmod check") +def test_db_wal_sidecar_chmod_600(tmp_db): + """The -wal sidecar created by PRAGMA journal_mode=WAL must also + be 0o600. It carries in-flight transactions including the most + recent api_key writes — same exposure as the main DB file.""" + # Force a write so the WAL file actually exists. + tmp_db.add_site("test", "http://example.test", "danbooru") + # Re-trigger the chmod pass now that the sidecar exists. + tmp_db._restrict_perms() + wal = type(tmp_db._path)(str(tmp_db._path) + "-wal") + if wal.exists(): + mode = os.stat(wal).st_mode & 0o777 + assert mode == 0o600, f"expected 0o600 on WAL sidecar, got {oct(mode)}" + + def test_validate_folder_name_accepts_unicode_and_punctuation(): """Common real-world folder names must pass through unchanged. The guard is meant to block escape shapes, not normal naming."""