security: fix #4 — chmod data_dir to 0o700 on POSIX
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
This commit is contained in:
parent
19a22be59c
commit
6801a0b45e
@ -44,7 +44,15 @@ def popout_aspect_lock_enabled() -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def data_dir() -> Path:
|
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:
|
if IS_WINDOWS:
|
||||||
base = Path.home() / "AppData" / "Roaming"
|
base = Path.home() / "AppData" / "Roaming"
|
||||||
else:
|
else:
|
||||||
@ -55,6 +63,13 @@ def data_dir() -> Path:
|
|||||||
)
|
)
|
||||||
path = base / APPNAME
|
path = base / APPNAME
|
||||||
path.mkdir(parents=True, exist_ok=True)
|
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
|
return path
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@ -6,10 +6,14 @@ Locks in:
|
|||||||
depth alongside `_validate_folder_name`)
|
depth alongside `_validate_folder_name`)
|
||||||
- `find_library_files` matching exactly the root + 1-level subdirectory
|
- `find_library_files` matching exactly the root + 1-level subdirectory
|
||||||
layout that the library uses, with the right MEDIA_EXTENSIONS filter
|
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
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from booru_viewer.core import config
|
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}
|
match_names = {p.name for p in matches}
|
||||||
|
|
||||||
assert match_names == {"123.jpg", "123.png"}
|
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
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user