# SPDX-License-Identifier: BUSL-1.1 # Copyright (c) 2024-2026 Ali Sadeghi Aghili """Owner-only file permissions for everything this project writes to disk. Why this module exists ----------------------- Finding 18 (2026 audit) started as "nothing this codebase writes restricts its own permissions" -- a repo-wide grep for ``chmod``/``0o600``/``umask`` came back empty, so every file this application ever created landed at whatever the process umask happened to be (``0644`` on a typical Linux host -- world-readable). Round 1 of remediation fixed the two sharpest instances (``logs/logger.py``'s audit trail, ``webapp/app.py``'s Flask session-signing key) by hand, each with its own inline ``if os.name != "nt": os.chmod(path, 0o600)``. That closed the finding's most dangerous example but left it open everywhere else: the application database (API key digests, role grants), the session store (full conversation transcripts), ``webapp/app.db`` (password hashes), and ``exports/*.xlsx`` (query results) were all still created at the ambient umask, because the fix was never factored into something a fifth call site could just call. This module is that shared call. Every creation point in the codebase that writes something sensitive now calls :func:`restrict_file` (or :func:`restrict_sqlite_family` for a SQLite database, which also has to cover the ``-wal``/``-shm`` sidecars -- see that function's docstring) the moment the file exists, instead of re-deriving the same three lines with a slightly different guard or a forgotten sidecar. Design decisions, and why each one is fail-closed rather than fail-loud ------------------------------------------------------------------------ * **Windows is a no-op, not a crash.** File mode bits are a POSIX concept; Windows has no equivalent this module can map onto, and the deployment target this finding describes is a Linux server. The ``os.chmod`` call itself stays present unconditionally in the *source* (guarded only at the call, with ``if os.name != "nt":``) rather than being conditionally imported or wrapped in a platform shim, because ``tests/security_audit/test_secret_file_permissions.py`` greps the source text for exactly this pattern as a backstop that survives on a platform where the live-permission assertion itself has to skip. * **A ``chmod`` failure never propagates.** An exotic filesystem (a FUSE mount, some NFS configurations, a container filesystem with a restrictive mount option) can refuse a ``chmod`` for reasons that have nothing to do with whether the file itself was written successfully. Failing the caller's actual operation -- a query that ran fine, a session that saved fine -- over a permissions call that is defense in depth on top of that write would be a worse outcome than the world-readable file this module exists to prevent. Every failure is therefore caught, logged at ``WARNING`` (loud enough that an operator monitoring logs sees it, quiet enough that it never becomes a user-facing error), and swallowed -- the same "fail open but loud" shape ``observability/audit.py``'s audit-counter fallback uses for the same reason (round 1 remediation). * **Idempotent.** Calling this on a file that already has the right mode (or that another process already restricted) is a no-op in effect -- ``os.chmod`` unconditionally sets the mode rather than reading it back first, which is simpler and no slower than a read-then-maybe-write, and is safe to call on every write rather than only the write that first creates the file. """ from __future__ import annotations import logging import os from pathlib import Path logger = logging.getLogger(__name__) #: Owner read/write only. Applied to every plain file this project writes #: that is not meant to be group- or world-readable -- the application #: database, the session store, the Flask user table, exported workbooks. _FILE_MODE = 0o600 #: Owner read/write/execute only. Applied to directories this project #: creates to hold the files above, so a restrictive file mode is not #: undermined by a world-readable parent directory an attacker could still #: list (even if the file inside cannot be opened, its existence, size and #: mtime are already information a shared host should not hand out). _DIR_MODE = 0o700 #: SQLite sidecar suffixes that can hold data the main file does not (yet). #: See :func:`restrict_sqlite_family` for why this list is exactly these #: two and not, say, ``-journal`` as well. _SQLITE_SIDECAR_SUFFIXES = ("-wal", "-shm") def _chmod_quietly(path: Path, mode: int) -> None: """Apply *mode* to *path*, never letting the attempt fail the caller. A no-op on Windows (see module docstring). On POSIX, any ``OSError`` -- permission denied because the process does not own the file, an unsupported filesystem, the path having vanished between creation and this call -- is logged and swallowed rather than raised, because a permissions tightening step must never be the reason a query, a login, or an export fails for the user in front of it. """ if os.name == "nt": return try: os.chmod(path, mode) except OSError as exc: logger.warning( "Could not restrict permissions on %s to %o: %s. The file may " "be readable by other local accounts.", path, mode, exc, ) def restrict_file(path: str | Path) -> None: """Restrict *path* to owner read/write only (``0o600``). Call this immediately after a sensitive file is first created -- the application database, the session store, ``webapp/app.db``, an exported ``.xlsx`` workbook, or anything else this project writes that is not meant to be readable by another local account. Safe to call on a file that does not exist (a no-op: there is nothing to ``chmod``) so a caller does not need its own existence check first, and safe to call repeatedly on the same file (idempotent -- see module docstring). """ p = Path(path) if not p.exists(): return _chmod_quietly(p, _FILE_MODE) def restrict_directory(path: str | Path) -> None: """Restrict *path* to owner read/write/execute only (``0o700``). Call this on a directory this project creates to hold sensitive files (an export directory, a session-store parent) -- a ``0o600``-restricted file inside a world-readable directory still leaks the file's name, size and modification time to anyone who can list that directory, which is exactly the kind of exposure this finding exists to close. Safe to call on a path that does not exist or is not a directory (a no-op). """ p = Path(path) if not p.is_dir(): return _chmod_quietly(p, _DIR_MODE) def restrict_sqlite_family(db_path: str | Path) -> None: """Restrict a SQLite database file and its ``-wal``/``-shm`` sidecars. Why the sidecars matter as much as the main file -------------------------------------------------- A database opened in write-ahead-logging mode (every SQLite database this project opens is -- see ``session/persistence.py`` and ``appdb/engine.py``) does not write committed pages straight into the main file. They land first in ``-wal``, and stay there, readable by anyone who can open that file, until SQLite next checkpoints them back into the main database. On a live, frequently written database (the session store logging every turn, the key store recording every issued key) the ``-wal`` file is not a transient artefact -- it is routinely where the freshest rows actually live. Restricting only ``db_path`` and leaving ``db_path-wal`` at the ambient umask would protect the stale copy and expose the current one. ``.gitignore`` in this repository carries a comment about this exact lesson, learned the hard way, for the same reason. ``-shm`` (the shared-memory index SQLite uses to coordinate WAL readers across processes) holds no row data of its own, but is restricted alongside the other two anyway -- it is trivial to include and there is no argument for leaving any file this project's own database writes at a looser mode than the database itself. Deliberately not ``-journal``: unlike ``-wal``, a rollback-journal sidecar exists only *during* an in-flight transaction and is removed by SQLite itself the moment that transaction commits or rolls back. ``.gitignore`` still lists ``*.db-journal`` for the era before every SQLite database this project opens was WAL -- both ``session/persistence.py`` and, since the concurrency fix that gave every file-backed application-database connection ``PRAGMA journal_mode=WAL`` (``appdb/engine.py``'s own ``"connect"`` listener), ``appdb`` as well, now use WAL exclusively, so a rollback journal is not a file either database's own connections would ordinarily create at all. The exclusion is kept anyway, for the same two reasons it would still apply even if some future caller opened a non-WAL connection to one of these files: a ``-journal`` sidecar exists only *during* an in-flight transaction and is removed by SQLite itself the moment that transaction commits or rolls back, so it is exactly the file a caller here would race to ``chmod`` between two already-atomic operations it does not control the timing of, not a file it could ever observe sitting at rest with unwanted permissions the way an ``-wal`` file routinely does. Each of the three paths is restricted independently and only if it exists -- a database not yet under active write load may have no ``-wal``/``-shm`` sidecar at all (SQLite creates them lazily, on the first write in WAL mode), and that is a normal state, not an error. """ p = Path(db_path) restrict_file(p) for suffix in _SQLITE_SIDECAR_SUFFIXES: restrict_file(p.with_name(p.name + suffix))