From a3d60a72a0d7128ec540e9a81602a8141b926034 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sat, 12 Sep 2026 09:28:49 +0530 Subject: [PATCH] test: remove Unix-only and developer-specific assumptions from the suite A full run on Windows 11 failed 31 tests, almost all because a fixture assumed POSIX behaviour or read the developer's own Git configuration rather than because Strix misbehaved. That noise hides real regressions. - Resolve git with `shutil.which` instead of `/usr/bin/env git`, and skip the module when no git is present. This single line accounted for 19 of the 31 failures. - Isolate every git subprocess from `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` and supply a commit identity, so a global `core.excludesFile`, `commit.gpgSign`, or `core.hooksPath` cannot change what a test sees. - Create control-character filenames through a fixture that skips where the filesystem rejects them, instead of failing to build the fixture. - Choose the protected system directory for the platform in the mount policy test. `check_mountable_dir` already handles both families, so on Windows this now exercises the real policy instead of failing early. - Assert secret-file permissions through a fixture that keeps the POSIX 0o600 check exact and, on Windows, skips with the reason stated rather than weakening the assertion. The device-identity test is split so its identity contract still runs everywhere. - Pin mypy to `platform = "linux"` so type checking resolves the same APIs as CI and the container target. This removes the six `fcntl.flock`/`os.getuid`/`os.getgid` attribute errors reported on Windows without adding ignores that `warn_unused_ignores` would then flag on Linux. Windows now reports 12 failures, all owned elsewhere: #1258 (7-8, intermittent), #648/#652, #1288, and #1285. Refs #1259 Co-Authored-By: Claude Opus 5 --- pyproject.toml | 1 + tests/conftest.py | 87 +++++++++++++++++++++++++++++++++ tests/test_cloud_cli_runtime.py | 4 +- tests/test_cloud_session.py | 10 +++- tests/test_completions.py | 6 +-- tests/test_local_sources.py | 14 +++++- tests/test_threat_model_tool.py | 9 +++- tests/test_viewer_auth.py | 6 +-- 8 files changed, 125 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b78fb3aa..19672d19 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,6 +104,7 @@ path = "scripts/tui_sidecar_hook.py" [tool.mypy] python_version = "3.12" +platform = "linux" strict = true strict_optional = true warn_redundant_casts = true diff --git a/tests/conftest.py b/tests/conftest.py index 946ab206..08a0ada6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,9 +2,17 @@ from __future__ import annotations +import os +from typing import TYPE_CHECKING + import pytest +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path + + @pytest.fixture(autouse=True) def _isolate_mcp_config( monkeypatch: pytest.MonkeyPatch, tmp_path_factory: pytest.TempPathFactory @@ -51,3 +59,82 @@ def _isolate_wallet_config(monkeypatch: pytest.MonkeyPatch) -> None: """ for name in ("MPPX_ACCOUNT", "MPPX_STRIPE_SECRET_KEY", "MPPX_STRIPE_PAYMENT_METHOD"): monkeypatch.delenv(name, raising=False) + + +@pytest.fixture(autouse=True) +def _isolate_git_config( + monkeypatch: pytest.MonkeyPatch, tmp_path_factory: pytest.TempPathFactory +) -> None: + """Run every `git` subprocess against empty global and system config. + + Several tests build a throwaway repository and assert on what Git reports + about it. Git layers the developer's own configuration under that, so a + global `core.excludesFile` silently removes files from the source + selection tests, and `commit.gpgSign` or a `core.hooksPath` can fail the + commit outright. Point both config layers at a path that does not exist + and supply the identity the commits need, so the repository a test sees is + the one it created. + """ + empty = tmp_path_factory.mktemp("git-isolation") / "absent.gitconfig" + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(empty)) + monkeypatch.setenv("GIT_CONFIG_SYSTEM", str(empty)) + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") + monkeypatch.setenv("GIT_AUTHOR_NAME", "Strix Tests") + monkeypatch.setenv("GIT_AUTHOR_EMAIL", "tests@example.com") + monkeypatch.setenv("GIT_COMMITTER_NAME", "Strix Tests") + monkeypatch.setenv("GIT_COMMITTER_EMAIL", "tests@example.com") + + +@pytest.fixture +def write_control_character_file() -> Callable[[Path, str, str], Path]: + """Create a file whose *name* carries terminal control characters, or skip. + + A few tests prove that Strix never echoes a filename back to the terminal + with its escape sequences intact, so they need a hostile name on disk to + have something to render. Windows rejects those characters in a filename + outright, so the file cannot be created and the injection the test guards + against is not reachable on that platform. Skip with the real `OSError` + rather than assert, so the POSIX contract keeps being exercised where it + applies and Windows does not report a product failure it does not have. + """ + + def _write(directory: Path, name: str, content: str = "{}") -> Path: + path = directory / name + try: + path.write_text(content, encoding="utf-8") + except OSError as exc: # pragma: no cover - platform dependent + pytest.skip(f"this filesystem rejects control characters in filenames: {exc}") + return path + + return _write + + +@pytest.fixture +def assert_secret_file_permissions() -> Callable[[Path], None]: + """Assert a secret file is owner-only, or skip where that cannot hold. + + `write_secret_text` opens secret files with `SECRET_FILE_MODE` (0o600). + Windows does not implement POSIX mode bits: `os.open` maps the mode down to + a single read-only flag, so `stat()` reports 0o666 no matter what was + requested, and confidentiality comes from the ACL on the user profile + instead. Asserting 0o600 there would fail forever without saying anything + about whether the file is actually protected. + + Keep the POSIX assertion exact, and on Windows skip with that reason + spelled out, so the mode check is never quietly downgraded to something + weaker that still looks green. + """ + + def _assert(path: Path) -> None: + assert path.is_file(), f"{path} was not written" + if os.name == "nt": # pragma: no cover - platform dependent + pytest.skip( + "Windows ignores POSIX mode bits, so a secret file always reports " + "0o666; its confidentiality comes from the user-profile ACL, which " + "this test does not assert" + ) + assert path.stat().st_mode & 0o777 == 0o600, ( + f"{path} is {path.stat().st_mode & 0o777:#o}, expected 0o600" + ) + + return _assert diff --git a/tests/test_cloud_cli_runtime.py b/tests/test_cloud_cli_runtime.py index 4e2a5e5c..357bba9b 100644 --- a/tests/test_cloud_cli_runtime.py +++ b/tests/test_cloud_cli_runtime.py @@ -282,11 +282,11 @@ def test_human_rendering_neutralizes_osc_and_csi_control_sequences() -> None: def test_source_prompt_shows_paths_and_literal_confirmation( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, write_control_character_file: Any ) -> None: (tmp_path / "app.py").write_text("print('ok')\n", encoding="utf-8") dangerous_name = "visible\x1b]52;c;copied\x07\x1b[2J.py" - (tmp_path / dangerous_name).write_text("print('safe')\n", encoding="utf-8") + write_control_character_file(tmp_path, dangerous_name, "print('safe')\n") output = io.StringIO() console = Console(file=output, width=100) prompts: list[tuple[str, bool]] = [] diff --git a/tests/test_cloud_session.py b/tests/test_cloud_session.py index 70349c1a..8f53781d 100644 --- a/tests/test_cloud_session.py +++ b/tests/test_cloud_session.py @@ -163,4 +163,12 @@ def test_cli_device_identity_is_stable_and_privacy_safe( second = platform_identity.read_or_create_identity(device_name=" Build laptop ") assert second["client_instance_id"] == first["client_instance_id"] assert second["device_name"] == "Build laptop" - assert path.stat().st_mode & 0o777 == 0o600 + + +def test_cli_identity_file_is_owner_only( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, assert_secret_file_permissions: Any +) -> None: + path = tmp_path / "cli-identity.json" + monkeypatch.setattr(platform_identity, "IDENTITY_PATH", path) + platform_identity.read_or_create_identity() + assert_secret_file_permissions(path) diff --git a/tests/test_completions.py b/tests/test_completions.py index f9f9157e..66ac40e9 100644 --- a/tests/test_completions.py +++ b/tests/test_completions.py @@ -128,12 +128,12 @@ def test_filesystem_completion_for_source_output_and_data(tmp_path: Any, monkeyp def test_filesystem_completion_omits_terminal_control_names( - tmp_path: Any, monkeypatch: Any, capsys: Any + tmp_path: Any, monkeypatch: Any, capsys: Any, write_control_character_file: Any ) -> None: monkeypatch.chdir(tmp_path) (tmp_path / "safe.json").write_text("{}", encoding="utf-8") - (tmp_path / "unsafe\nname.json").write_text("{}", encoding="utf-8") - (tmp_path / "unsafe\x1b]52;c;payload\x07.json").write_text("{}", encoding="utf-8") + write_control_character_file(tmp_path, "unsafe\nname.json") + write_control_character_file(tmp_path, "unsafe\x1b]52;c;payload\x07.json") words = ["cloud", "scans", "start", "--data", "@"] assert completion_candidates(words) == ["@safe.json"] diff --git a/tests/test_local_sources.py b/tests/test_local_sources.py index 9984bef6..f03bed46 100644 --- a/tests/test_local_sources.py +++ b/tests/test_local_sources.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import os from pathlib import Path from typing import Any @@ -177,8 +178,19 @@ def test_check_mountable_dir_accepts_a_project_under_the_home_root( def test_infer_target_type_applies_the_mount_policy() -> None: + # `infer_target_type` only reaches the mount policy for a path that exists, + # 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")] + ) + 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") + with pytest.raises(ValueError, match="Refusing to mount"): - infer_target_type("/etc") + infer_target_type(str(system_dir)) def test_read_target_list_file_strips_blank_lines(tmp_path: Path) -> None: diff --git a/tests/test_threat_model_tool.py b/tests/test_threat_model_tool.py index 5b378b07..300f5e65 100644 --- a/tests/test_threat_model_tool.py +++ b/tests/test_threat_model_tool.py @@ -2,6 +2,7 @@ from __future__ import annotations +import shutil import subprocess from typing import TYPE_CHECKING @@ -46,8 +47,14 @@ self-scoped information leak. Low: verbose errors. """ +_GIT = shutil.which("git") + +pytestmark = pytest.mark.skipif(_GIT is None, reason="these tests need a git executable") + + def _git(repo: Path, *args: str) -> None: - subprocess.run(["/usr/bin/env", "git", *args], cwd=repo, check=True) # noqa: S603 + assert _GIT is not None + subprocess.run([_GIT, *args], cwd=repo, check=True) # noqa: S603 def _make_repo(tmp_path: Path, name: str = "repo") -> Path: diff --git a/tests/test_viewer_auth.py b/tests/test_viewer_auth.py index 662f84f1..7a426b55 100644 --- a/tests/test_viewer_auth.py +++ b/tests/test_viewer_auth.py @@ -2,7 +2,6 @@ from __future__ import annotations -import stat from datetime import UTC, datetime, timedelta from typing import TYPE_CHECKING, Any @@ -86,10 +85,9 @@ def test_is_verified_accepts_epoch_expiry() -> None: assert auth.is_verified() is True -def test_write_auth_is_0600() -> None: +def test_write_auth_is_0600(assert_secret_file_permissions: Any) -> None: auth.write_auth(email="a@b.com", token="t", verified_at="") # nosec B106 - mode = stat.S_IMODE(auth.AUTH_PATH.stat().st_mode) - assert mode == 0o600 + assert_secret_file_permissions(auth.AUTH_PATH) def test_read_auth_rejects_incomplete_record() -> None: