From a3d60a72a0d7128ec540e9a81602a8141b926034 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sat, 12 Sep 2026 09:28:49 +0530 Subject: [PATCH 1/3] 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: From 1cc815f5b6d214cc32bf22e17cad4fbbde6f3f4f Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sat, 12 Sep 2026 12:45:17 +0530 Subject: [PATCH 2/3] 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") From 6bb668ec2a31c62fce59d1e22336e9c66d251986 Mon Sep 17 00:00:00 2001 From: itzzdev09 Date: Sat, 12 Sep 2026 13:26:46 +0530 Subject: [PATCH 3/3] test: drop the mypy platform pin in favour of a real fix `platform = "linux"` was a workaround for six Windows-only attribute errors on `fcntl` and `os.getuid`. Review pushed back, correctly: it makes local runs deterministic but checks no platform-specific branch under its own platform. #1301 fixes those six at the source and adds a workflow running mypy under --platform linux, darwin and win32, so the pin is no longer needed. Removing it here leaves this PR scoped to test assumptions and lets the two merge in either order. Co-Authored-By: Claude Opus 5 --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 19672d19..b78fb3aa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,7 +104,6 @@ path = "scripts/tui_sidecar_hook.py" [tool.mypy] python_version = "3.12" -platform = "linux" strict = true strict_optional = true warn_redundant_casts = true