Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,18 @@

from __future__ import annotations

import errno
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
Expand Down Expand Up @@ -51,3 +60,94 @@ 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")


# 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.

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
# 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}")
Comment on lines +113 to +119

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unrelated errors become skips

This catches every OSError, not only the invalid-filename error expected on Windows. If writing fails because the temporary directory is unwritable, storage is exhausted, or another filesystem problem occurs, these security-focused tests will be reported as skipped instead of exposing the failure. Restrict the skip to the expected invalid-name error or explicitly to Windows.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/conftest.py
Line: 105-106

Comment:
**Unrelated errors become skips**

This catches every `OSError`, not only the invalid-filename error expected on Windows. If writing fails because the temporary directory is unwritable, storage is exhausted, or another filesystem problem occurs, these security-focused tests will be reported as skipped instead of exposing the failure. Restrict the skip to the expected invalid-name error or explicitly to Windows.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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
4 changes: 2 additions & 2 deletions tests/test_cloud_cli_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]] = []
Expand Down
10 changes: 9 additions & 1 deletion tests/test_cloud_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 3 additions & 3 deletions tests/test_completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
19 changes: 18 additions & 1 deletion tests/test_local_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import argparse
import os
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -177,8 +178,24 @@ 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.
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")

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:
Expand Down
9 changes: 8 additions & 1 deletion tests/test_threat_model_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import shutil
import subprocess
from typing import TYPE_CHECKING

Expand Down Expand Up @@ -46,8 +47,14 @@
"""


_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:
Expand Down
6 changes: 2 additions & 4 deletions tests/test_viewer_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from __future__ import annotations

import stat
from datetime import UTC, datetime, timedelta
from typing import TYPE_CHECKING, Any

Expand Down Expand Up @@ -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:
Expand Down