From 6801a0b45ea10fedcaf2d4c3d2359ad70b402474 Mon Sep 17 00:00:00 2001
From: pax <paxxe@protonmail.com>
Date: Sat, 11 Apr 2026 16:14:30 -0500
Subject: [PATCH] =?UTF-8?q?security:=20fix=20#4=20=E2=80=94=20chmod=20data?=
 =?UTF-8?q?=5Fdir=20to=200o700=20on=20POSIX?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The data directory holds the SQLite database whose `sites` table
stores api_key and api_user in plaintext. Previous behavior used
the inherited umask (typically 0o755), which leaves the dir
world-traversable on shared workstations and on networked home
dirs whose home is 0o755. Tighten to 0o700 unconditionally on
every data_dir() call so the fix is applied even when an older
version (or external tooling) left the directory loose.

Failures from filesystems that don't support chmod (some FUSE
mounts) are swallowed — better to keep working than refuse to
start. Windows: no-op, NTFS ACLs handle this separately.

behavior change from v0.2.5: ~/.local/share/booru-viewer is now
0o700 even if it was previously 0o755.

Audit-Ref: SECURITY_AUDIT.md finding #4
Severity: Medium
---
 booru_viewer/core/config.py | 17 ++++++++++++++++-
 tests/core/test_config.py   | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/booru_viewer/core/config.py b/booru_viewer/core/config.py
index 7f6e840..a668c09 100644
--- a/booru_viewer/core/config.py
+++ b/booru_viewer/core/config.py
@@ -44,7 +44,15 @@ def popout_aspect_lock_enabled() -> bool:
 
 
 def data_dir() -> Path:
-    """Return the platform-appropriate data/cache directory."""
+    """Return the platform-appropriate data/cache directory.
+
+    On POSIX, the directory is chmod'd to 0o700 after creation so the
+    SQLite DB inside (and the api_key/api_user columns it stores) are
+    not exposed to other local users on shared workstations or
+    networked home dirs with permissive umasks. On Windows the chmod
+    is a no-op — NTFS ACLs handle access control separately and the
+    OS already restricts AppData\\Roaming\\<app> to the owning user.
+    """
     if IS_WINDOWS:
         base = Path.home() / "AppData" / "Roaming"
     else:
@@ -55,6 +63,13 @@ def data_dir() -> Path:
         )
     path = base / APPNAME
     path.mkdir(parents=True, exist_ok=True)
+    if not IS_WINDOWS:
+        try:
+            os.chmod(path, 0o700)
+        except OSError:
+            # Filesystem may not support chmod (e.g. some FUSE mounts).
+            # Better to keep working than refuse to start.
+            pass
     return path
 
 
diff --git a/tests/core/test_config.py b/tests/core/test_config.py
index b5ef43c..f807fdd 100644
--- a/tests/core/test_config.py
+++ b/tests/core/test_config.py
@@ -6,10 +6,14 @@ Locks in:
   depth alongside `_validate_folder_name`)
 - `find_library_files` matching exactly the root + 1-level subdirectory
   layout that the library uses, with the right MEDIA_EXTENSIONS filter
+- `data_dir` chmods its directory to 0o700 on POSIX (audit #4)
 """
 
 from __future__ import annotations
 
+import os
+import sys
+
 import pytest
 
 from booru_viewer.core import config
@@ -55,3 +59,33 @@ def test_find_library_files_walks_root_and_one_level(tmp_library):
     match_names = {p.name for p in matches}
 
     assert match_names == {"123.jpg", "123.png"}
+
+
+# -- data_dir permissions (audit finding #4) --
+
+@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only chmod check")
+def test_data_dir_chmod_700(tmp_path, monkeypatch):
+    """`data_dir()` chmods the platform data dir to 0o700 on POSIX so the
+    SQLite DB and api_key columns inside aren't readable by other local
+    users on shared machines or networked home dirs."""
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    path = config.data_dir()
+    mode = os.stat(path).st_mode & 0o777
+    assert mode == 0o700, f"expected 0o700, got {oct(mode)}"
+    # Idempotent: a second call leaves the mode at 0o700.
+    config.data_dir()
+    mode2 = os.stat(path).st_mode & 0o777
+    assert mode2 == 0o700
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only chmod check")
+def test_data_dir_tightens_loose_existing_perms(tmp_path, monkeypatch):
+    """If a previous version (or external tooling) left the dir at 0o755,
+    the next data_dir() call must tighten it back to 0o700."""
+    monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
+    pre = tmp_path / config.APPNAME
+    pre.mkdir()
+    os.chmod(pre, 0o755)
+    config.data_dir()
+    mode = os.stat(pre).st_mode & 0o777
+    assert mode == 0o700