From 1cc815f5b6d214cc32bf22e17cad4fbbde6f3f4f Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sat, 12 Sep 2026 12:45:17 +0530 Subject: [PATCH] test: narrow the control-character skip and find Windows by SYSTEMROOT Review feedback, both valid. The control-character fixture caught every OSError, so an unwritable temp directory or a full disk would have reported these security tests as skipped instead of failing. It now skips only for errors that mean the name itself is unrepresentable (EINVAL, EILSEQ, ENAMETOOLONG) and re-raises everything else. The mount-policy test derived the protected directory from the checkout's drive, so a repo cloned to D: would look for D:\Windows, miss, and skip the branch the test exists to cover. Ask Windows where it is installed via SYSTEMROOT instead. Co-Authored-By: Claude Opus 5 --- tests/conftest.py | 13 +++++++++++++ tests/test_local_sources.py | 11 ++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 08a0ada6..bebf3967 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ from __future__ import annotations +import errno import os from typing import TYPE_CHECKING @@ -85,6 +86,13 @@ def _isolate_git_config( monkeypatch.setenv("GIT_COMMITTER_EMAIL", "tests@example.com") +# Errors that mean "this name is not representable here", as opposed to a +# filesystem that is full, read-only, or otherwise broken. +_REJECTED_NAME_ERRNOS = frozenset( + {errno.EINVAL, errno.EILSEQ, errno.ENAMETOOLONG}, +) + + @pytest.fixture def write_control_character_file() -> Callable[[Path, str, str], Path]: """Create a file whose *name* carries terminal control characters, or skip. @@ -103,6 +111,11 @@ def write_control_character_file() -> Callable[[Path, str, str], Path]: try: path.write_text(content, encoding="utf-8") except OSError as exc: # pragma: no cover - platform dependent + # Only "the name itself is unacceptable" is a reason to skip. A + # full disk or an unwritable temp directory must still fail these + # security tests rather than quietly reporting them as skipped. + if exc.errno not in _REJECTED_NAME_ERRNOS: + raise pytest.skip(f"this filesystem rejects control characters in filenames: {exc}") return path diff --git a/tests/test_local_sources.py b/tests/test_local_sources.py index f03bed46..be73fa84 100644 --- a/tests/test_local_sources.py +++ b/tests/test_local_sources.py @@ -182,9 +182,14 @@ def test_infer_target_type_applies_the_mount_policy() -> None: # so pick a protected system directory this platform actually has. The # policy itself already knows about both families (`_FORBIDDEN_MOUNT_TREES` # and `_FORBIDDEN_WINDOWS_TREE_NAMES`); only the fixture was Unix-only. - candidates = ( - [Path(Path.cwd().anchor) / "Windows"] if os.name == "nt" else [Path("/etc"), Path("/usr")] - ) + if os.name == "nt": + # Ask Windows where it is installed. Deriving the drive from the + # checkout would look for D:\Windows on a repo cloned to D:, miss, and + # skip the very branch this test exists to cover. + system_root = os.environ.get("SYSTEMROOT") or os.environ.get("WINDIR") + candidates = [Path(system_root)] if system_root else [] + else: + candidates = [Path("/etc"), Path("/usr")] system_dir = next((p for p in candidates if p.is_dir()), None) if system_dir is None: pytest.skip("no protected system directory on this platform")