fix(safety): close three evidence bypasses and make workspace staging idempotent

Grouped browser verbs were classified by their verb alone. `tab` and `session`
sit in the passive set, so `tab new <url>` — documented as navigating — and
`session clear` earned a deterministic allow and executed unreviewed in guarded
mode and unblocked in observe, while `open <url>`, the same navigation, was
reviewed. Passivity is now decided from verb plus subcommand, and the packet
carries the result so observe mode stops maintaining a second copy of the rule
that could drift more permissive than guarded. The blocked-action list still
matches on the bare verb, so `auth login` keeps matching `auth`.

Interpreters were a seven-name allowlist, so `python3.12`, `/usr/bin/python3`,
`php`, and `deno` set no script path and produced a packet with no artifacts
that was still stamped complete — the exact shape the reviewer is told it may
allow. Recognize versioned and common interpreters so their sources are
actually collected, and fail closed when a command runs code that cannot be
resolved to an inspectable script.

`from pkg import payload` collected only the package initializer, because an
imported name was treated as an attribute and never as a submodule. Effectful
code in `pkg/payload.py` executed without appearing in the evidence.

Workspace staging runs twice per run and was not idempotent: the second pass
read the origin from `source_path`, which the first pass had already rewritten
to the copy. With the completion marker absent it cleared the destination and
then copied from that same emptied directory, silently handing the agent an
empty workspace. The origin is now read back from `original_source_path`.

Each fix is covered by a test that fails when the fix is reverted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

tests/test_safety_evidence.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import ast
 import io
 from types import SimpleNamespace
 from typing import TYPE_CHECKING, Any
@@ -9,7 +10,7 @@ from typing import TYPE_CHECKING, Any
 import pytest
 
 from strix.config.settings import SafetySettings
-from strix.safety.evidence import compile_evidence, parse_command
+from strix.safety.evidence import _PythonFacts, compile_evidence, parse_command
 
 
 if TYPE_CHECKING:
@@ -33,6 +34,12 @@ class _Sandbox:
         return io.BytesIO(self.files[key].encode())
 
 
+def _facts(source: str) -> _PythonFacts:
+    facts = _PythonFacts()
+    facts.visit(ast.parse(source))
+    return facts
+
+
 def _ctx(files: dict[str, str], *, turn_input: list[Any] | None = None) -> Any:
     return SimpleNamespace(
         context={"agent_id": "agent-1", "sandbox_session": _Sandbox(files)},
@@ -624,3 +631,169 @@ async def test_oversized_dependency_closure_makes_evidence_incomplete() -> None:
         assert any("total byte limit" in reason for reason in bundle.incomplete_reasons)
     finally:
         bundle.cleanup()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    ("command", "action", "subcommand"),
+    [
+        ("agent-browser tab new https://example.test/admin", "tab", "new"),
+        ("agent-browser tab close 2", "tab", "close"),
+        ("agent-browser session clear", "session", "clear"),
+    ],
+)
+async def test_grouped_browser_verbs_are_not_passive(
+    command: str,
+    action: str,
+    subcommand: str,
+) -> None:
+    """`tab new <url>` navigates and `tab close` destroys page state, so the bare verb
+    must not be enough to earn the observation fast path."""
+    plan = parse_command(command)
+    assert plan.browser_action == action
+    assert plan.browser_subcommand == subcommand
+    assert plan.read_only is False
+
+    bundle = await _compile(command)
+    try:
+        assert bundle.deterministic_allow is None
+        assert bundle.packet["browser"]["passive"] is False
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("command", ["agent-browser tab", "agent-browser snapshot -i"])
+async def test_bare_listing_verbs_keep_the_observation_fast_path(command: str) -> None:
+    bundle = await _compile(command)
+    try:
+        assert bundle.deterministic_allow is not None
+        assert bundle.packet["browser"]["passive"] is True
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+async def test_grouped_blocked_verbs_still_match_on_the_verb() -> None:
+    """The blocked list keys off the bare verb, so qualifying the action must not stop
+    `auth login` from matching `auth`."""
+    bundle = await _compile("agent-browser auth login my-app")
+    try:
+        assert bundle.deterministic_block is not None
+        assert "auth" in bundle.deterministic_block
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "command",
+    [
+        "python3.12 /workspace/run.py",
+        "/usr/bin/python3.12 /workspace/run.py",
+        "pypy3 /workspace/run.py",
+    ],
+)
+async def test_versioned_interpreters_are_inspected(command: str) -> None:
+    bundle = await _compile(
+        command,
+        {"/workspace/run.py": "import helper\n", "/workspace/helper.py": "value = 1\n"},
+    )
+    try:
+        paths = {item["path"] for item in bundle.packet["artifacts"]}
+        assert paths == {"/workspace/run.py", "/workspace/helper.py"}
+        assert bundle.complete is True
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+async def test_non_python_interpreter_source_is_inspected() -> None:
+    bundle = await _compile(
+        "php /workspace/app.php",
+        {"/workspace/app.php": "<?php unlink('/workspace/data'); ?>\n"},
+    )
+    try:
+        [artifact] = bundle.packet["artifacts"]
+        assert artifact["path"] == "/workspace/app.php"
+        assert "unlink" in artifact["source"]
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "command",
+    [
+        "python3.12",
+        "node",
+        "mystery-runner /workspace/run.py",
+        "./vendored-tool /workspace/run.sh",
+    ],
+)
+async def test_unresolvable_code_execution_fails_closed(command: str) -> None:
+    """A packet with no artifacts must never be stamped complete just because the
+    executable fell outside the interpreter set."""
+    bundle = await _compile(command, {"/workspace/run.py": "import os\n"})
+    try:
+        assert bundle.complete is False
+        assert bundle.packet["artifacts"] == []
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("command", ["nmap -sV example.test", "ls /workspace", "whoami"])
+async def test_commands_without_a_script_are_not_forced_incomplete(command: str) -> None:
+    """Fail-closed on unresolved script execution must not swallow ordinary tools."""
+    bundle = await _compile(command)
+    try:
+        assert bundle.complete is True
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+async def test_absolute_submodule_import_is_collected() -> None:
+    """`from pkg import payload` may name a submodule, not an attribute of the package."""
+    bundle = await _compile(
+        "python /workspace/main.py",
+        {
+            "/workspace/main.py": "from pkg import payload\npayload.go()\n",
+            "/workspace/pkg/__init__.py": "",
+            "/workspace/pkg/payload.py": "import shutil\n\n\ndef go():\n    shutil.rmtree('/x')\n",
+        },
+    )
+    try:
+        paths = {item["path"] for item in bundle.packet["artifacts"]}
+        assert "/workspace/pkg/payload.py" in paths
+    finally:
+        bundle.cleanup()
+
+
+@pytest.mark.asyncio
+async def test_relative_submodule_import_is_collected() -> None:
+    bundle = await _compile(
+        "python /workspace/main.py",
+        {
+            "/workspace/main.py": "import pkg.mod\n",
+            "/workspace/pkg/__init__.py": "",
+            "/workspace/pkg/mod.py": "from .inner import payload\n",
+            "/workspace/pkg/inner/__init__.py": "",
+            "/workspace/pkg/inner/payload.py": "import shutil\nshutil.rmtree('/x')\n",
+        },
+    )
+    try:
+        paths = {item["path"] for item in bundle.packet["artifacts"]}
+        assert "/workspace/pkg/inner/payload.py" in paths
+    finally:
+        bundle.cleanup()
+
+
+def test_imported_attributes_do_not_pollute_the_reported_imports() -> None:
+    """Submodule candidates are resolution-only; the packet still shows the statements
+    as the author wrote them."""
+    facts = _facts("from os import path\nfrom mypkg import CONSTANT\n")
+
+    assert facts.imports == {"os", "mypkg"}
+    assert facts.submodule_imports == {"os.path", "mypkg.CONSTANT"}

tests/test_safety_runtime.py

@@ -477,3 +477,71 @@ async def test_a_read_only_command_leaves_the_epoch_alone(tmp_path: Path) -> Non
     )
 
     assert runtime._workspace_epoch == before
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "command",
+    [
+        "agent-browser tab new https://example.test/admin",
+        "agent-browser tab close 2",
+        "agent-browser session clear",
+    ],
+)
+async def test_observe_mode_blocks_grouped_browser_verbs(tmp_path: Path, command: str) -> None:
+    """The bare verb sits in the passive set, so these are the commands that would slip
+    through if passivity were decided on the verb alone."""
+    runtime = _runtime(tmp_path, "observe")
+    reviewer = _StubReviewer()
+    runtime._reviewer = reviewer
+    invoked = False
+
+    async def invoke(_ctx: Any, _raw_input: str) -> str:
+        nonlocal invoked
+        invoked = True
+        return "bad"
+
+    result = await runtime.invoke_exec(
+        ctx=_ctx(),
+        arguments={"cmd": command},
+        invoke_tool=invoke,
+    )
+
+    payload = json.loads(result)
+    assert payload["status"] == "blocked"
+    assert payload["safety"]["categories"] == ["target_mutation"]
+    assert reviewer.calls == 0
+    assert invoked is False
+
+
+@pytest.mark.asyncio
+async def test_guarded_grouped_browser_verb_reaches_the_reviewer(tmp_path: Path) -> None:
+    """In guarded mode it loses only the fast path; the reviewer still gets to decide."""
+    runtime = _runtime(tmp_path, "guarded")
+    reviewer = _StubReviewer()
+    runtime._reviewer = reviewer
+
+    result = await runtime.invoke_exec(
+        ctx=_ctx(),
+        arguments={"cmd": "agent-browser tab new https://example.test/admin"},
+        invoke_tool=_noop_invoke,
+    )
+
+    assert result == "patched"
+    assert reviewer.calls == 1
+
+
+@pytest.mark.asyncio
+async def test_bare_tab_listing_keeps_the_fast_path(tmp_path: Path) -> None:
+    runtime = _runtime(tmp_path, "observe")
+    reviewer = _StubReviewer()
+    runtime._reviewer = reviewer
+
+    result = await runtime.invoke_exec(
+        ctx=_ctx(),
+        arguments={"cmd": "agent-browser tab"},
+        invoke_tool=_noop_invoke,
+    )
+
+    assert result == "patched"
+    assert reviewer.calls == 0

tests/test_workspace_isolation.py

@@ -77,3 +77,40 @@ def test_isolated_copy_drops_out_of_tree_symlink(tmp_path: Path) -> None:
     )
 
     assert not (Path(staged["source_path"]) / "escape").exists()
+
+
+def test_repeated_materialization_preserves_the_true_origin(tmp_path: Path) -> None:
+    source = tmp_path / "source"
+    source.mkdir()
+    (source / "app.py").write_text("code\n", encoding="utf-8")
+    run_dir = tmp_path / "runs" / "scan"
+    sources = [{"source_path": str(source), "workspace_subdir": "source", "protect_metadata": True}]
+
+    # Staging runs in prepare_run and again in run_strix_scan on the same entries.
+    staged = materialize_isolated_sources(sources, run_dir=run_dir)
+    [restaged] = materialize_isolated_sources(staged, run_dir=run_dir)
+
+    assert restaged["original_source_path"] == str(source.resolve())
+    assert restaged["source_path"] == staged[0]["source_path"]
+    assert restaged["source_path"] != restaged["original_source_path"]
+
+
+def test_restaging_without_a_completion_marker_recopies_the_source(tmp_path: Path) -> None:
+    """A second pass that treats the copy as its own origin clears the destination and
+    then reads it back empty, silently handing the agent an empty workspace."""
+    source = tmp_path / "source"
+    source.mkdir()
+    (source / "app.py").write_text("code\n", encoding="utf-8")
+    run_dir = tmp_path / "runs" / "scan"
+
+    [staged] = materialize_isolated_sources(
+        [{"source_path": str(source), "workspace_subdir": "source", "protect_metadata": True}],
+        run_dir=run_dir,
+    )
+    destination = Path(staged["source_path"])
+    (destination.parent / f".{destination.name}.complete").unlink()
+
+    [restaged] = materialize_isolated_sources([staged], run_dir=run_dir)
+
+    assert (Path(restaged["source_path"]) / "app.py").read_text(encoding="utf-8") == "code\n"
+    assert restaged["original_source_path"] == str(source.resolve())