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)