From e5944c94e049a135fe9a375cb83fdee8ae82cecf Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 10 Oct 2026 14:33:28 +0200 Subject: [PATCH] Probe data dir writability with a uniquely named temp file can_write_to_data_dir() touched and then deleted data/test.txt, which silently wiped any file of the user's that happened to have that name. It now uses tempfile.NamedTemporaryFile in the data dir, which picks an unused name and removes only the file it created. --- spoolman/env.py | 7 ++++--- tests/test_env.py | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/spoolman/env.py b/spoolman/env.py index 315faaf4..2220d123 100644 --- a/spoolman/env.py +++ b/spoolman/env.py @@ -4,6 +4,7 @@ import logging import os import subprocess import sys +import tempfile from datetime import datetime from enum import Enum from pathlib import Path @@ -458,9 +459,9 @@ def get_build_date() -> datetime | None: def can_write_to_data_dir() -> bool: """Check if the data directory is writable.""" try: - test_file = get_data_dir().joinpath("test.txt") - test_file.touch() - test_file.unlink() + # A uniquely named file, so the check can never clobber one of the user's own. + with tempfile.NamedTemporaryFile(dir=get_data_dir()): + pass except: # noqa: E722 return False return True diff --git a/tests/test_env.py b/tests/test_env.py index 0a8d524b..17c0ba28 100644 --- a/tests/test_env.py +++ b/tests/test_env.py @@ -1,5 +1,8 @@ """Tests for environment variable parsing.""" +import os +from pathlib import Path + import pytest from spoolman import env @@ -46,3 +49,23 @@ def test_get_cors_origin_drops_empty_and_duplicate_entries(monkeypatch: pytest.M def test_get_cors_origin_raw_is_unparsed(monkeypatch: pytest.MonkeyPatch): monkeypatch.setenv("SPOOLMAN_CORS_ORIGIN", " https://a.local, https://b.local ") assert env.get_cors_origin_raw() == " https://a.local, https://b.local " + + +def test_can_write_to_data_dir_leaves_existing_files_alone(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """The write check must not touch a file of the user's, even one named like a probe file.""" + monkeypatch.setenv("SPOOLMAN_DIR_DATA", str(tmp_path)) + (tmp_path / "test.txt").write_text("mine") + assert env.can_write_to_data_dir() is True + assert [p.name for p in tmp_path.iterdir()] == ["test.txt"] + assert (tmp_path / "test.txt").read_text() == "mine" + + +def test_can_write_to_data_dir_read_only(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + if os.name == "nt" or os.geteuid() == 0: + pytest.skip("directory permissions are not enforced here") + monkeypatch.setenv("SPOOLMAN_DIR_DATA", str(tmp_path)) + tmp_path.chmod(0o500) + try: + assert env.can_write_to_data_dir() is False + finally: + tmp_path.chmod(0o700)