diff --git a/docs/changelog/753.bugfix.rst b/docs/changelog/753.bugfix.rst new file mode 100644 index 00000000..23200ca8 --- /dev/null +++ b/docs/changelog/753.bugfix.rst @@ -0,0 +1,2 @@ +Skip access-denial checks when the process can read mode-``0o000`` files. Keep mode-bit checks enabled for privileged +processes on filesystems that support POSIX permissions. diff --git a/tasks/capabilities.py b/tasks/capabilities.py index 19bb53ec..0fe353fc 100644 --- a/tasks/capabilities.py +++ b/tasks/capabilities.py @@ -214,16 +214,32 @@ def _reports_object_tracebacks() -> bool: return True -def _enforces_file_mode() -> bool: - # Without POSIX permission bits a chmod does not read back. +def _supports_file_mode() -> bool: + with _unreadable_file() as probe: + return probe.stat().st_mode & 0o777 == 0 + + +def _enforces_file_permissions() -> bool: + # Privileged processes can retain mode bits while bypassing access checks. + with _unreadable_file() as probe: + try: + probe.read_bytes() + except PermissionError: + return True + return False + + +@contextmanager +def _unreadable_file() -> Iterator[Path]: with tempfile.TemporaryDirectory() as directory: - probe = Path(directory, "probe") + probe: Final[Path] = Path(directory, "probe") probe.touch() - probe.chmod(_OWNER_READ_WRITE) - return probe.stat().st_mode & 0o777 == _OWNER_READ_WRITE - + probe.chmod(0) + try: + yield probe + finally: + probe.chmod(0o600) -_OWNER_READ_WRITE: Final[int] = 0o600 #: Capability -> whether this runtime provides it. Tests gate their skipif on this same mapping. CAPABILITIES: Final[dict[str, bool]] = { @@ -239,7 +255,8 @@ def _enforces_file_mode() -> bool: "fcntl": find_spec("fcntl") is not None, "unlink-open-file": _supports_unlinking_an_open_file(), "posix-signals": hasattr(signal, "SIGKILL"), - "file-mode": _enforces_file_mode(), + "file-mode": _supports_file_mode(), + "file-permissions": _enforces_file_permissions(), "prompt-finalization": _finalizes_on_last_reference(), "collected-finalization": _finalizes_on_collection(), "class-collection": _collects_classes(), diff --git a/tests/capability_marks.py b/tests/capability_marks.py index 5fcf8746..daf632ba 100644 --- a/tests/capability_marks.py +++ b/tests/capability_marks.py @@ -54,7 +54,13 @@ ) NEEDS_FILE_MODE: Final[pytest.MarkDecorator] = pytest.mark.skipif( - not CAPABILITIES["file-mode"], reason="making a folder or a claim unreadable needs POSIX permission bits" + not CAPABILITIES["file-mode"], + reason="requires a filesystem that preserves POSIX mode bits", +) + +NEEDS_FILE_PERMISSIONS: Final[pytest.MarkDecorator] = pytest.mark.skipif( + not CAPABILITIES["file-permissions"], + reason="requires enforced file permissions", ) NEEDS_UNLINK_OPEN_FILE: Final[pytest.MarkDecorator] = pytest.mark.skipif( @@ -105,6 +111,7 @@ "NEEDS_COLLECTED_FINALIZATION", "NEEDS_FCNTL", "NEEDS_FILE_MODE", + "NEEDS_FILE_PERMISSIONS", "NEEDS_FORK", "NEEDS_FORK1", "NEEDS_GENERATOR_EXCEPTION_CONTEXT", diff --git a/tests/test_filelock.py b/tests/test_filelock.py index 125ef0cd..1a680647 100644 --- a/tests/test_filelock.py +++ b/tests/test_filelock.py @@ -40,7 +40,7 @@ from filelock._api import _append_exception_context, _raise_grouped_errors, _registry from tests.capability_marks import ( NEEDS_FCNTL, - NEEDS_FILE_MODE, + NEEDS_FILE_PERMISSIONS, NEEDS_PARENT_SYMLINK_COLLAPSE, NEEDS_PROMPT_FINALIZATION, NEEDS_SYMLINK, @@ -98,17 +98,13 @@ def make_ro(path: Path) -> Iterator[None]: @pytest.fixture -def tmp_path_ro(tmp_path: Path) -> Iterator[Path]: # pragma: needs file-mode +def tmp_path_ro(tmp_path: Path) -> Iterator[Path]: # pragma: needs file-permissions with make_ro(tmp_path): yield tmp_path @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock]) -@NEEDS_FILE_MODE # pragma: needs file-mode -@pytest.mark.skipif( - sys.platform != "win32" and os.geteuid() == 0, - reason="Cannot make a read only file (that the current user: root can't read)", -) +@NEEDS_FILE_PERMISSIONS # pragma: needs file-permissions def test_ro_folder(lock_type: type[BaseFileLock], tmp_path_ro: Path) -> None: lock = lock_type(str(tmp_path_ro / "a")) with pytest.raises(PermissionError, match="Permission denied"): diff --git a/tests/test_strict_soft.py b/tests/test_strict_soft.py index 6719a425..7df2fb3e 100644 --- a/tests/test_strict_soft.py +++ b/tests/test_strict_soft.py @@ -11,7 +11,7 @@ import pytest -from tests.capability_marks import NEEDS_FILE_MODE +from tests.capability_marks import NEEDS_FILE_PERMISSIONS if sys.version_info >= (3, 11): from builtins import ExceptionGroup # pragma: >=3.11 cover @@ -285,7 +285,7 @@ def test_strict_soft_rejects_non_directory_coordination_path(tmp_path: Path) -> StrictSoftFileLock(lock_path).acquire() -@NEEDS_FILE_MODE # pragma: needs file-mode +@NEEDS_FILE_PERMISSIONS # pragma: needs file-permissions def test_strict_soft_unreadable_claim_fails_closed(tmp_path: Path) -> None: lock_path = tmp_path / "resource.lock" lock = StrictSoftFileLock(lock_path) diff --git a/tests/test_util.py b/tests/test_util.py index e289c314..bb354220 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,11 +1,13 @@ from __future__ import annotations import os +import runpy import socket import stat import sys import time from contextlib import suppress +from errno import EACCES, EIO, ENOENT, EPERM from pathlib import Path from typing import TYPE_CHECKING, Final @@ -237,3 +239,35 @@ def test_touch_does_not_follow_symlink(tmp_path: Path) -> None: # pragma: needs assert victim.stat().st_mtime == pytest.approx(past, abs=1) # a timestamp round-trip need not be bit-exact assert link.lstat().st_mtime > victim.stat().st_mtime + + +@pytest.mark.parametrize( + ("error", "expected"), + [ + pytest.param(None, False, id="privileged"), + pytest.param(PermissionError(EACCES, "denied"), True, id="access-denied"), + pytest.param(PermissionError(EPERM, "denied"), True, id="operation-denied"), + ], +) +@pytest.mark.usefixtures("_read_failure") +def test_file_permissions_capability(*, expected: bool) -> None: + assert runpy.run_module("capabilities")["CAPABILITIES"]["file-permissions"] is expected + + +@pytest.mark.parametrize( + "error", + [ + pytest.param(OSError(EIO, "probe failed"), id="io-error"), + pytest.param(OSError(ENOENT, "probe failed"), id="missing-file"), + ], +) +@pytest.mark.usefixtures("_read_failure") +def test_file_permissions_probe_propagates_io_errors(error: OSError) -> None: + with pytest.raises(OSError, match="probe failed") as raised: + runpy.run_module("capabilities") + assert raised.value is error + + +@pytest.fixture +def _read_failure(mocker: MockerFixture, error: OSError | None) -> None: + mocker.patch.object(Path, "read_bytes", autospec=True, side_effect=error, return_value=b"")