fix: require reviewed current patches and enforce fix network isolation

This commit is contained in:
Jonathan Singer 2026-09-30 23:21:24 -04:00
parent f75fb5fcf9
commit bba4aa24dc
13 changed files with 311 additions and 30 deletions

View file

@ -17,16 +17,18 @@
- `strix/fix/scan.py`: supplies the finding/worktree, deduplicates requests, preserves turn counts, exports completion, and cleans up. It replays persisted findings on resume. Terminal failures are reported truthfully; an explicit retry starts a fresh attempt using the remaining turn allowance. - `strix/fix/scan.py`: supplies the finding/worktree, deduplicates requests, preserves turn counts, exports completion, and cleans up. It replays persisted findings on resume. Terminal failures are reported truthfully; an explicit retry starts a fresh attempt using the remaining turn allowance.
- `strix/runtime/agent_session.py`: borrows the scan sandbox with a worktree-specific filesystem root and process ownership. All scan agents get a process scope. Use `stop_process` or Ctrl-C on an owned tool session; broad shell kill commands are rejected. This prevents accidental interference, not hostile code escaping an OS security boundary. - `strix/runtime/agent_session.py`: borrows the scan sandbox with a worktree-specific filesystem root and process ownership. All scan agents get a process scope. Use `stop_process` or Ctrl-C on an owned tool session; broad shell kill commands are rejected. This prevents accidental interference, not hostile code escaping an OS security boundary.
- `strix/agents/prompts/fix.jinja`: the single Fix assignment; shared workspace guidance is in `fix_workspace.jinja`. - `strix/agents/prompts/fix.jinja`: the single Fix assignment; shared workspace guidance is in `fix_workspace.jinja`.
- `strix/fix/runtime.py`: uses `build_strix_agent`, `run_agent_loop`, native tools, persisted sessions, and usage hooks. There is no separate reviewer or custom conversation loop. - `strix/fix/runtime.py`: uses `build_strix_agent`, `run_agent_loop`, native tools, persisted sessions, and usage hooks for repair and independent review.
- `strix/fix/prepare.py`: checks source identity and the completed patch, then exports successful artifacts. - `strix/fix/prepare.py`: checks source identity and the completed patch, then exports artifacts only after independent approval of the final source digest. Scan delivery also rechecks that the finding is still current.
- Pro supplies progress/result callbacks. The app registers the inline attempt, stores successful artifacts privately, and creates draft PRs using its existing repository integration. Neither starts another fix sandbox. - Pro supplies progress/result callbacks. The app registers the inline attempt, stores successful artifacts privately, and creates draft PRs using its existing repository integration. Neither starts another fix sandbox.
The `single_agent` result contract exposes the agent's limitations in both `completion.gaps` and top-level `gaps` for compatible readers. It records the Fix agent's completion, command history, final file manifest, and source digest. It does not claim independent verification. Commands include diagnostic failures and superseded attempts; the agent's final summary explains which tests passed and any optional follow-ups. The result exposes the agent's limitations in both `completion.gaps` and top-level `gaps` for compatible readers. Ready results use `agent_review` and record the repair completion, independent review, command history, final file manifest, and approved source digest. Commands include diagnostic failures and superseded attempts; the Markdown result includes both repair and reviewer summaries.
## Standalone OSS command ## Standalone OSS command
`strix fix --finding findings.json --finding-id FINDING_ID --repo /path/to/repo` `strix fix --finding findings.json --finding-id FINDING_ID --repo /path/to/repo`
The standalone command uses the same single-agent implementation in its own sandbox, because no live scan exists to borrow. It preserves the supplied checkout and writes private outputs outside the repository by default. Only successful runs export a patch and archive. `--max-agent-turns` and the legacy `--max-repair-turns` can lower the turn cap; they cannot raise it above 300. Old request fields are accepted for compatibility, but reviewer limits no longer control a second agent. The standalone command uses the same repair and review workflow in its own sandbox, because no live scan exists to borrow. It preserves the supplied checkout and writes private outputs outside the repository by default. Only successful runs export a patch and archive. `--max-agent-turns` and the legacy `--max-repair-turns` can lower the repair turn cap; they cannot raise it above 300.
Standalone fix requests default to `network_allowed=false`, enforced with Docker's `network_mode="none"`. Use `--allow-network` or explicitly set `network_allowed=true` in request JSON to use `STRIX_DOCKER_SANDBOX_NETWORK` (Docker's default network when unset). Offline sandboxes skip host-side proxy bootstrap and cannot reuse a session with a different network policy; unsupported backends reject offline requests. Fixes running inside a live scan inherit that scan's network policy.
Uploaded archives and ambiguous multiple-repository findings cannot currently start an automatic worktree fix: the candidate must identify one Git source and its exact revision. Uploaded archives and ambiguous multiple-repository findings cannot currently start an automatic worktree fix: the candidate must identify one Git source and its exact revision.

View file

@ -290,7 +290,10 @@ async def prepare_fix( # noqa: PLR0911, PLR0912
if completion.source_digest != await workspace_digest(workspace): if completion.source_digest != await workspace_digest(workspace):
return await finish(PreparationState.BLOCKED, "Source changed after completion.") return await finish(PreparationState.BLOCKED, "Source changed after completion.")
if verify is None: if verify is None:
return await finish(PreparationState.READY, completion.summary) return await finish(
PreparationState.BLOCKED,
"Independent verification is required before delivery.",
)
before_review = await workspace_digest(workspace) before_review = await workspace_digest(workspace)
attempt = FixPreparationAttempt( attempt = FixPreparationAttempt(
attempt=1, attempt=1,
@ -304,6 +307,8 @@ async def prepare_fix( # noqa: PLR0911, PLR0912
context.feedback.append(attempt) context.feedback.append(attempt)
verifier = await verify(context, attempt.checks) verifier = await verify(context, attempt.checks)
attempt.verifier = verifier attempt.verifier = verifier
if cancelled():
raise PreparationCancelledError # noqa: TRY301
after_review = await workspace_digest(workspace) after_review = await workspace_digest(workspace)
if after_review != before_review: if after_review != before_review:
return await finish( return await finish(

View file

@ -719,12 +719,15 @@ class ManagedIndependentVerifier(_FixAgent):
async def _create_command_sandbox( async def _create_command_sandbox(
sandbox_id: str, sandbox_id: str,
*,
network_allowed: bool = False,
) -> BaseSandboxSession: ) -> BaseSandboxSession:
settings = load_settings() settings = load_settings()
bundle = await session_manager.create_or_reuse( bundle = await session_manager.create_or_reuse(
sandbox_id, sandbox_id,
image=settings.runtime.image, image=settings.runtime.image,
local_sources=[], local_sources=[],
network_allowed=network_allowed,
) )
return cast("BaseSandboxSession", bundle["session"]) return cast("BaseSandboxSession", bundle["session"])
@ -933,7 +936,9 @@ async def run_isolated_fix_preparation(
f"fix-preparation-{request.finding_id}-" f"fix-preparation-{request.finding_id}-"
f"{request.candidate.digest()[:12]}-{attempt_digest}" f"{request.candidate.digest()[:12]}-{attempt_digest}"
) )
sandbox_session = await _create_command_sandbox(sandbox_id) sandbox_session = await _create_command_sandbox(
sandbox_id, network_allowed=request.network_allowed
)
environment = _RuntimeEnvironment( environment = _RuntimeEnvironment(
workspace=mirror, workspace=mirror,
sandbox_session=sandbox_session, sandbox_session=sandbox_session,

View file

@ -329,6 +329,9 @@ class ScanFixes:
prepared = await finish_native_fix( prepared = await finish_native_fix(
request, environment, hooks, result, session, artifact request, environment, hooks, result, session, artifact
) )
if not self._current(finding_id, digest):
prepared = None
raise ValueError("The finding changed or was withdrawn during Fix.") # noqa: TRY301
prepared.elapsed_seconds = time.monotonic() - started_at prepared.elapsed_seconds = time.monotonic() - started_at
delivered = await self._emit( delivered = await self._emit(
"finished", "finished",

View file

@ -49,6 +49,12 @@ def _parser() -> argparse.ArgumentParser:
parser.add_argument("--max-review-turns", type=int, help=argparse.SUPPRESS) parser.add_argument("--max-review-turns", type=int, help=argparse.SUPPRESS)
parser.add_argument("--max-budget", type=float, help="Combined LLM cost budget in USD.") parser.add_argument("--max-budget", type=float, help="Combined LLM cost budget in USD.")
parser.add_argument("--timeout", type=int, help="Whole-job timeout in seconds.") parser.add_argument("--timeout", type=int, help="Whole-job timeout in seconds.")
parser.add_argument(
"--allow-network",
action="store_true",
default=None,
help="Allow sandbox network access using the operator-configured Docker network.",
)
return parser return parser
@ -80,7 +86,6 @@ def _load_request(args: argparse.Namespace) -> FixPreparationRequestV1:
scan_id=str(finding.get("scan_id") or "local"), scan_id=str(finding.get("scan_id") or "local"),
finding_id=str(finding.get("id") or args.finding_id or uuid.uuid4().hex), finding_id=str(finding.get("id") or args.finding_id or uuid.uuid4().hex),
candidate=candidate, candidate=candidate,
network_allowed=True,
) )
if ( if (
request.candidate.source_identity is None request.candidate.source_identity is None
@ -95,6 +100,7 @@ def _load_request(args: argparse.Namespace) -> FixPreparationRequestV1:
"max_review_turns": args.max_review_turns, "max_review_turns": args.max_review_turns,
"max_budget_usd": args.max_budget, "max_budget_usd": args.max_budget,
"timeout_seconds": args.timeout, "timeout_seconds": args.timeout,
"network_allowed": args.allow_network,
}.items() }.items()
if value is not None if value is not None
} }
@ -116,10 +122,10 @@ def _summary(result: FixPreparationResultV1) -> str:
lines = ["# Fix preparation", "", f"Status: {result.state.value}", "", result.stop_reason] lines = ["# Fix preparation", "", f"Status: {result.state.value}", "", result.stop_reason]
if result.completion: if result.completion:
lines.extend(["", "## Fix", "", result.completion.summary]) lines.extend(["", "## Fix", "", result.completion.summary])
elif result.verifier:
lines.extend(["", "## Review", "", result.verifier.summary])
elif result.attempt_history: elif result.attempt_history:
lines.extend(["", "## Repair", "", result.attempt_history[-1].repair.summary]) lines.extend(["", "## Repair", "", result.attempt_history[-1].repair.summary])
if result.verifier:
lines.extend(["", "## Review", "", result.verifier.summary])
lines.extend(["", "## Recorded commands", "", "Includes diagnostic and superseded attempts."]) lines.extend(["", "## Recorded commands", "", "Includes diagnostic and superseded attempts."])
lines.extend( lines.extend(
f"- {check.name}: {check.status.value}; exit code {check.exit_code}." f"- {check.name}: {check.status.value}; exit code {check.exit_code}."

View file

@ -23,6 +23,7 @@ async def _docker_backend(
manifest: Manifest, manifest: Manifest,
exposed_ports: tuple[int, ...], exposed_ports: tuple[int, ...],
bind_mounts: list[dict[str, Any]] | None = None, bind_mounts: list[dict[str, Any]] | None = None,
network_allowed: bool = True,
) -> tuple[Any, Any]: ) -> tuple[Any, Any]:
"""Bring up a session backed by the local Docker daemon. """Bring up a session backed by the local Docker daemon.
@ -44,6 +45,7 @@ async def _docker_backend(
client = StrixDockerSandboxClient(docker.from_env()) client = StrixDockerSandboxClient(docker.from_env())
client.strix_bind_mounts = bind_mounts or [] client.strix_bind_mounts = bind_mounts or []
client.network_allowed = network_allowed
options = DockerSandboxClientOptions(image=image, exposed_ports=exposed_ports) options = DockerSandboxClientOptions(image=image, exposed_ports=exposed_ports)
session = await client.create(options=options, manifest=manifest) session = await client.create(options=options, manifest=manifest)
await session.start() await session.start()

View file

@ -59,7 +59,11 @@ def _sandbox_network() -> str | None:
return value or None return value or None
def _apply_sandbox_network(create_kwargs: dict[str, Any]) -> None: def _apply_sandbox_network(create_kwargs: dict[str, Any], *, network_allowed: bool = True) -> None:
if not network_allowed:
create_kwargs["network_mode"] = "none"
create_kwargs.pop("ports", None)
return
network = _sandbox_network() network = _sandbox_network()
if network: if network:
create_kwargs["network"] = network create_kwargs["network"] = network
@ -162,6 +166,7 @@ class StrixDockerSandboxSession(DockerSandboxSession):
class StrixDockerSandboxClient(DockerSandboxClient): class StrixDockerSandboxClient(DockerSandboxClient):
network_allowed: bool = True
# Host directories to bind-mount into the container, set by the docker # Host directories to bind-mount into the container, set by the docker
# backend before ``create()``. Each item is ``{source, target, read_only}``. # backend before ``create()``. Each item is ``{source, target, read_only}``.
strix_bind_mounts: list[dict[str, Any]] | None = None strix_bind_mounts: list[dict[str, Any]] | None = None
@ -232,7 +237,7 @@ class StrixDockerSandboxClient(DockerSandboxClient):
extra_hosts = create_kwargs.setdefault("extra_hosts", {}) extra_hosts = create_kwargs.setdefault("extra_hosts", {})
extra_hosts["host.docker.internal"] = "host-gateway" extra_hosts["host.docker.internal"] = "host-gateway"
_apply_sandbox_network(create_kwargs) _apply_sandbox_network(create_kwargs, network_allowed=self.network_allowed)
_apply_resource_limits(create_kwargs) _apply_resource_limits(create_kwargs)
_apply_log_limits(create_kwargs) _apply_log_limits(create_kwargs)
_apply_run_labels(create_kwargs) _apply_run_labels(create_kwargs)
@ -268,7 +273,7 @@ class StrixDockerSandboxClient(DockerSandboxClient):
async def create(self, **kwargs: Any) -> SandboxSession: async def create(self, **kwargs: Any) -> SandboxSession:
session = await super().create(**kwargs) session = await super().create(**kwargs)
network = _sandbox_network() network = _sandbox_network() if self.network_allowed else None
inner = session._inner inner = session._inner
if network and isinstance(inner, DockerSandboxSession): if network and isinstance(inner, DockerSandboxSession):
inner.__class__ = StrixDockerSandboxSession inner.__class__ = StrixDockerSandboxSession

View file

@ -262,6 +262,7 @@ async def create_or_reuse(
local_sources: list[dict[str, Any]], local_sources: list[dict[str, Any]],
extra_files: list[dict[str, Any]] | None = None, extra_files: list[dict[str, Any]] | None = None,
status_sink: StatusSink | None = None, status_sink: StatusSink | None = None,
network_allowed: bool = True,
) -> dict[str, Any]: ) -> dict[str, Any]:
"""Return the existing session bundle for ``scan_id`` or create a new one. """Return the existing session bundle for ``scan_id`` or create a new one.
@ -280,11 +281,15 @@ async def create_or_reuse(
cached = _SESSION_CACHE.get(scan_id) cached = _SESSION_CACHE.get(scan_id)
if cached is not None: if cached is not None:
if cached.get("network_allowed", True) != network_allowed:
raise ValueError("Cannot reuse a sandbox with a different network policy.")
logger.info("Reusing existing sandbox session for scan %s", scan_id) logger.info("Reusing existing sandbox session for scan %s", scan_id)
return cached return cached
backend_name = load_settings().runtime.backend backend_name = load_settings().runtime.backend
backend = get_backend(backend_name) backend = get_backend(backend_name)
if not network_allowed and backend_name != "docker":
raise ValueError("Network isolation is only supported by the Docker backend.")
if backend_supports_bind_mounts(backend_name): if backend_supports_bind_mounts(backend_name):
bind_mounts = build_bind_mounts(local_sources) bind_mounts = build_bind_mounts(local_sources)
@ -327,8 +332,9 @@ async def create_or_reuse(
client, session = await backend( client, session = await backend(
image=image, image=image,
manifest=manifest, manifest=manifest,
exposed_ports=(_CONTAINER_CAIDO_PORT,), exposed_ports=(_CONTAINER_CAIDO_PORT,) if network_allowed else (),
bind_mounts=bind_mounts, bind_mounts=bind_mounts,
**({"network_allowed": False} if not network_allowed else {}),
) )
if extra_file_archive is not None: if extra_file_archive is not None:
@ -339,6 +345,17 @@ async def create_or_reuse(
await _discard_session(client, session) await _discard_session(client, session)
raise raise
if not network_allowed:
# Offline command sandboxes have no reachable proxy endpoint to bootstrap.
bundle = {
"client": client,
"session": session,
"caido_client": None,
"network_allowed": False,
}
_SESSION_CACHE[scan_id] = bundle
return bundle
report("Setting up the proxy") report("Setting up the proxy")
caido_endpoint = await session.resolve_exposed_port(_CONTAINER_CAIDO_PORT) caido_endpoint = await session.resolve_exposed_port(_CONTAINER_CAIDO_PORT)
scheme = "https" if caido_endpoint.tls else "http" scheme = "https" if caido_endpoint.tls else "http"
@ -364,6 +381,7 @@ async def create_or_reuse(
"client": client, "client": client,
"session": session, "session": session,
"caido_client": caido_client, "caido_client": caido_client,
"network_allowed": True,
} }
_SESSION_CACHE[scan_id] = bundle _SESSION_CACHE[scan_id] = bundle
logger.info("Sandbox session for scan %s ready and cached", scan_id) logger.info("Sandbox session for scan %s ready and cached", scan_id)

View file

@ -54,11 +54,48 @@ def test_cli_role_budget_overrides(tmp_path: Path) -> None:
assert (loaded.repair_turn_limit, loaded.review_turn_limit) == (300, 250) assert (loaded.repair_turn_limit, loaded.review_turn_limit) == (300, 250)
def _local_runtime(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, model: ScriptedModel) -> None: @pytest.mark.parametrize("allow_network", [False, True])
@pytest.mark.parametrize("input_kind", ["finding", "request"])
def test_cli_network_access_requires_opt_in(
tmp_path: Path, allow_network: bool, input_kind: str
) -> None:
request = _request("a" * 40)
assert request.network_allowed is False
path = tmp_path / "input.json"
path.write_text(
request.model_dump_json()
if input_kind == "request"
else json.dumps({"fix_candidate": request.candidate.model_dump(mode="json")})
)
argv = [f"--{input_kind}", str(path), "--repo", str(tmp_path)]
if allow_network:
argv.append("--allow-network")
assert (
fix_cli._load_request(fix_cli._parser().parse_args(argv)).network_allowed is allow_network
)
def test_cli_preserves_explicit_request_network_policy(tmp_path: Path) -> None:
request = _request("a" * 40)
request.network_allowed = True
path = tmp_path / "request.json"
path.write_text(request.model_dump_json())
args = fix_cli._parser().parse_args(["--request", str(path), "--repo", str(tmp_path)])
assert fix_cli._load_request(args).network_allowed is True
def _local_runtime(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
model: ScriptedModel,
*,
allow_network: bool = False,
) -> None:
root = tmp_path / "execution" / "source" root = tmp_path / "execution" / "source"
original_environment = fix_runtime._RuntimeEnvironment original_environment = fix_runtime._RuntimeEnvironment
async def sandbox(_sandbox_id: str) -> LocalSandbox: async def sandbox(_sandbox_id: str, *, network_allowed: bool) -> LocalSandbox:
assert network_allowed is allow_network
return LocalSandbox(root.parent) return LocalSandbox(root.parent)
async def noop(*_args: Any) -> None: async def noop(*_args: Any) -> None:
@ -82,8 +119,9 @@ def _local_runtime(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, model: Scrip
@pytest.mark.parametrize("blocked", [False, True]) @pytest.mark.parametrize("blocked", [False, True])
@pytest.mark.parametrize("allow_network", [False, True])
def test_cli_runs_shared_workflow_and_preserves_original_checkout( def test_cli_runs_shared_workflow_and_preserves_original_checkout(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, blocked: bool tmp_path: Path, monkeypatch: pytest.MonkeyPatch, blocked: bool, allow_network: bool
) -> None: ) -> None:
workspace, _ = _workspace(tmp_path) workspace, _ = _workspace(tmp_path)
commit = existing_suite(workspace) commit = existing_suite(workspace)
@ -94,12 +132,13 @@ def test_cli_runs_shared_workflow_and_preserves_original_checkout(
review = ( review = (
[shell("exit 1"), finish("blocked", "Required tests need a customer database.")] [shell("exit 1"), finish("blocked", "Required tests need a customer database.")]
if blocked if blocked
else [*suite_commands(), finish("approved", "Existing and regression tests passed.")] else [*suite_commands(), finish("done", "Existing and regression tests passed.")]
) )
model = ScriptedModel( model = ScriptedModel(
[*patch(), *(review if blocked else [*suite_commands(), finish("done", "tests passed")])] [*patch(), *(review if blocked else [*suite_commands(), finish("done", "tests passed")])],
review=[] if blocked else review,
) )
_local_runtime(monkeypatch, tmp_path, model) _local_runtime(monkeypatch, tmp_path, model, allow_network=allow_network)
output = tmp_path / "result.json" output = tmp_path / "result.json"
code = fix_cli.run_fix( code = fix_cli.run_fix(
@ -112,6 +151,7 @@ def test_cli_runs_shared_workflow_and_preserves_original_checkout(
str(workspace), str(workspace),
"--output", "--output",
str(output), str(output),
*(["--allow-network"] if allow_network else []),
] ]
) )
@ -124,6 +164,10 @@ def test_cli_runs_shared_workflow_and_preserves_original_checkout(
".md" ".md"
).read_text() ).read_text()
if not blocked: if not blocked:
assert (
"## Review\n\nExisting and regression tests passed."
in output.with_suffix(".md").read_text()
)
with zipfile.ZipFile(output.with_suffix(".zip")) as artifact: with zipfile.ZipFile(output.with_suffix(".zip")) as artifact:
assert "files/tests/test_security.py" in artifact.namelist() assert "files/tests/test_security.py" in artifact.namelist()
assert "tool-results.jsonl" in artifact.namelist() assert "tool-results.jsonl" in artifact.namelist()
@ -320,6 +364,7 @@ def test_summary_includes_followups_from_repair_and_reviewer() -> None:
source_identity=request.candidate.source_identity, source_identity=request.candidate.source_identity,
candidate=request.candidate, candidate=request.candidate,
candidate_digest=request.candidate.digest(), candidate_digest=request.candidate.digest(),
completion=attempt.repair,
attempt_history=[attempt], attempt_history=[attempt],
verifier=VerifierResult( verifier=VerifierResult(
decision=VerificationDecision.VERIFIED, decision=VerificationDecision.VERIFIED,
@ -328,6 +373,8 @@ def test_summary_includes_followups_from_repair_and_reviewer() -> None:
), ),
) )
summary = fix_cli._summary(result) summary = fix_cli._summary(result)
assert "## Fix\n\nPatched." in summary
assert "## Review\n\nApproved." in summary
assert "re-run the nightly suite" in summary assert "re-run the nightly suite" in summary
assert "rotate the leaked token" in summary assert "rotate the leaked token" in summary

View file

@ -74,12 +74,15 @@ def suite_commands() -> list[Any]:
class ScriptedModel(Model): class ScriptedModel(Model):
def __init__(self, repair: list[Any], review: list[Any] | None = None) -> None: def __init__(self, repair: list[Any], review: list[Any] | None = None) -> None:
self.responses = {"repair": repair, "review": review or []} self.responses = {
"repair": repair,
"review": review if review is not None else [finish("done", "Independently approved.")],
}
self.inputs: dict[str, list[Any]] = {"repair": [], "review": []} self.inputs: dict[str, list[Any]] = {"repair": [], "review": []}
self.tools: set[str] = set() self.tools: set[str] = set()
async def get_response(self, **kwargs: Any) -> ModelResponse: async def get_response(self, **kwargs: Any) -> ModelResponse:
role = "review" if "Independently review" in kwargs["system_instructions"] else "repair" role = "review" if "Independently verify" in kwargs["system_instructions"] else "repair"
self.inputs[role].append(list(kwargs["input"])) self.inputs[role].append(list(kwargs["input"]))
self.tools.update(t.name for t in kwargs["tools"]) self.tools.update(t.name for t in kwargs["tools"])
assert self.responses[role], f"Unexpected additional {role} turn" assert self.responses[role], f"Unexpected additional {role} turn"
@ -164,13 +167,15 @@ async def scenario(
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_single_agent_implements_runs_both_test_suites_and_exports(tmp_path, monkeypatch): async def test_agent_implements_runs_both_test_suites_and_exports_after_review(
tmp_path, monkeypatch
):
model = ScriptedModel([*patch(), *suite_commands(), finish("done", "Both suites passed")]) model = ScriptedModel([*patch(), *suite_commands(), finish("done", "Both suites passed")])
result, _env = await scenario(tmp_path, monkeypatch, model) result, _env = await scenario(tmp_path, monkeypatch, model)
assert result.state is PreparationState.READY, result.model_dump_json() assert result.state is PreparationState.READY, result.model_dump_json()
assert result.validation_mode == "single_agent" assert result.validation_mode == "agent_review"
assert not model.inputs["review"] assert model.inputs["review"]
assert result.verifier is None assert result.verifier.summary == "Independently approved."
assert result.completion.turns_used == 5 assert result.completion.turns_used == 5
assert all("Ran 1 test" in c.output for c in result.checks[-2:]) assert all("Ran 1 test" in c.output for c in result.checks[-2:])
assert {"exec_command", "apply_patch", "agent_finish"} <= model.tools assert {"exec_command", "apply_patch", "agent_finish"} <= model.tools
@ -189,7 +194,7 @@ async def test_agent_corrects_failed_test_in_same_conversation(tmp_path, monkeyp
assert result.state is PreparationState.READY assert result.state is PreparationState.READY
assert any(c.exit_code != 0 for c in result.checks) assert any(c.exit_code != 0 for c in result.checks)
assert all(c.exit_code == 0 for c in result.checks[-2:]) assert all(c.exit_code == 0 for c in result.checks[-2:])
assert not model.inputs["review"] assert model.inputs["review"]
@pytest.mark.asyncio @pytest.mark.asyncio

96
tests/test_fix_network.py Normal file
View file

@ -0,0 +1,96 @@
"""Fix requests enforce network policy at container creation and session reuse."""
from __future__ import annotations
from types import SimpleNamespace
from typing import TYPE_CHECKING, Any
from unittest.mock import AsyncMock, MagicMock
import pytest
from strix.fix import runtime as fix_runtime
from strix.runtime import session_manager
from strix.runtime.docker_client import StrixDockerSandboxClient
if TYPE_CHECKING:
from agents.sandbox.manifest import Manifest
@pytest.mark.parametrize("network_allowed", [False, True])
@pytest.mark.parametrize("operator_network", ["", "operator-network"])
async def test_fix_network_policy_reaches_docker(
monkeypatch: pytest.MonkeyPatch, network_allowed: bool, operator_network: str
) -> None:
docker = MagicMock()
session = SimpleNamespace(
start=AsyncMock(),
resolve_exposed_port=AsyncMock(
return_value=SimpleNamespace(tls=False, host="localhost", port=48080)
),
)
settings = SimpleNamespace(runtime=SimpleNamespace(backend="docker", image="test-image"))
monkeypatch.setattr(fix_runtime, "load_settings", lambda: settings)
monkeypatch.setattr(session_manager, "load_settings", lambda: settings)
monkeypatch.setattr(session_manager, "_SESSION_CACHE", {})
bootstrap = AsyncMock()
monkeypatch.setattr(session_manager, "bootstrap_caido", bootstrap)
monkeypatch.setenv("STRIX_DOCKER_SANDBOX_NETWORK", operator_network)
monkeypatch.setattr("docker.from_env", lambda: docker)
monkeypatch.setattr(StrixDockerSandboxClient, "image_exists", lambda *_args: True)
async def create(self: StrixDockerSandboxClient, *, options: Any, manifest: Manifest) -> Any:
await self._create_container(
options.image, manifest=manifest, exposed_ports=options.exposed_ports
)
return session
monkeypatch.setattr(StrixDockerSandboxClient, "create", create)
result = await fix_runtime._create_command_sandbox("fix", network_allowed=network_allowed)
assert result is session
session.start.assert_awaited_once()
kwargs = docker.containers.create.call_args.kwargs
if not network_allowed:
assert kwargs["network_mode"] == "none"
assert "network" not in kwargs
assert "ports" not in kwargs
session.resolve_exposed_port.assert_not_awaited()
bootstrap.assert_not_awaited()
else:
assert "network_mode" not in kwargs
if operator_network:
assert kwargs["network"] == operator_network
assert "ports" not in kwargs
else:
assert "network" not in kwargs
assert kwargs["ports"] == {"48080/tcp": ("127.0.0.1", None)}
await session_manager._SESSION_CACHE["fix"]["caido_client"].aclose()
assert (
await fix_runtime._create_command_sandbox("fix", network_allowed=network_allowed) is session
)
with pytest.raises(ValueError, match="different network policy"):
await fix_runtime._create_command_sandbox("fix", network_allowed=not network_allowed)
docker.containers.create.assert_called_once()
async def test_fix_sandbox_defaults_to_no_network(monkeypatch: pytest.MonkeyPatch) -> None:
create = AsyncMock(return_value={"session": object()})
monkeypatch.setattr(session_manager, "create_or_reuse", create)
await fix_runtime._create_command_sandbox("fix")
assert create.call_args.kwargs["network_allowed"] is False
async def test_network_isolation_rejects_unsupported_backend(
monkeypatch: pytest.MonkeyPatch,
) -> None:
settings = SimpleNamespace(runtime=SimpleNamespace(backend="custom", image="test-image"))
backend = AsyncMock()
monkeypatch.setattr(session_manager, "_SESSION_CACHE", {})
monkeypatch.setattr(session_manager, "load_settings", lambda: settings)
monkeypatch.setattr(session_manager, "get_backend", lambda _name: backend)
with pytest.raises(ValueError, match="only supported by the Docker backend"):
await session_manager.create_or_reuse(
"fix", image="test-image", local_sources=[], network_allowed=False
)
backend.assert_not_awaited()

View file

@ -368,10 +368,17 @@ def test_candidate_keeps_full_finding_without_inventing_reproduction() -> None:
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_agent_tests_are_reused_without_controller_execution(tmp_path): async def test_unreviewed_completion_never_exports_a_patch(tmp_path):
workspace, commit = _workspace(tmp_path) workspace, commit = _workspace(tmp_path)
result = await prepare_fix(_request(_candidate(commit)), workspace, repair=_noop_repair) export = AsyncMock()
assert result.state is PreparationState.READY result = await prepare_fix(
_request(_candidate(commit)), workspace, repair=_noop_repair, manifest_builder=export
)
assert result.state is PreparationState.BLOCKED
assert "Independent verification is required" in result.stop_reason
assert not result.final_file_manifest
assert result.artifact_ref is None
export.assert_not_awaited()
assert result.validation_mode == "single_agent" assert result.validation_mode == "single_agent"
assert result.completion.status == RepairStatus.COMPLETE assert result.completion.status == RepairStatus.COMPLETE
assert result.verifier is None assert result.verifier is None
@ -379,6 +386,34 @@ async def test_agent_tests_are_reused_without_controller_execution(tmp_path):
assert "Ran 1 test" in result.checks[0].output assert "Ran 1 test" in result.checks[0].output
@pytest.mark.asyncio
async def test_cancellation_during_review_never_exports_a_patch(tmp_path):
workspace, commit = _workspace(tmp_path)
stopped = False
async def verify(context, _checks):
nonlocal stopped
stopped = True
return VerifierResult(
decision=VerificationDecision.VERIFIED,
summary="Approved.",
source_digest=await workspace_digest(context.workspace),
)
export = AsyncMock()
result = await prepare_fix(
_request(_candidate(commit)),
workspace,
repair=_noop_repair,
verify=verify,
cancelled=lambda: stopped,
manifest_builder=export,
)
assert result.state is PreparationState.FAILED
assert "cancelled" in result.stop_reason
export.assert_not_awaited()
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_ready_requires_independent_verifier_approval(tmp_path): async def test_ready_requires_independent_verifier_approval(tmp_path):
workspace, commit = _workspace(tmp_path) workspace, commit = _workspace(tmp_path)
@ -514,6 +549,13 @@ async def test_completion_limitations_are_preserved_at_top_level(tmp_path):
completion = await _noop_repair(context, checks) completion = await _noop_repair(context, checks)
return completion.model_copy(update={"gaps": ["External integration was not exercised."]}) return completion.model_copy(update={"gaps": ["External integration was not exercised."]})
result = await prepare_fix(_request(_candidate(commit)), workspace, repair=agent) async def verify(context, _checks):
return VerifierResult(
decision=VerificationDecision.VERIFIED,
summary="Approved with the recorded limitation.",
source_digest=await workspace_digest(context.workspace),
)
result = await prepare_fix(_request(_candidate(commit)), workspace, repair=agent, verify=verify)
assert result.state is PreparationState.READY assert result.state is PreparationState.READY
assert result.gaps == result.completion.gaps == ["External integration was not exercised."] assert result.gaps == result.completion.gaps == ["External integration was not exercised."]

View file

@ -188,6 +188,51 @@ async def test_finding_revision_invalidates_active_completion(tmp_path):
assert not fixes._current("finding", digest) assert not fixes._current("finding", digest)
@pytest.mark.parametrize("change", ["revised", "withdrawn", "unconfirmed"])
async def test_finding_changed_before_delivery_discards_reviewed_patch(
tmp_path, monkeypatch, change
):
fixes, report, _, _, reports, context, _ = setup(tmp_path)
callback = None
async def spawn(**kwargs):
nonlocal callback
callback = kwargs["on_complete"]
fixes.coordinator.runtimes["fix"] = SimpleNamespace(
task=asyncio.create_task(asyncio.sleep(0))
)
return {"success": True, "agent_id": "fix"}
async def finish_preparation(request, _env, _hooks, _result, _session, artifact):
artifact.write_bytes(b"reviewed patch")
if change == "revised":
report["fix_candidate"]["security_invariant"] = "Revised attack"
elif change == "withdrawn":
reports.clear()
else:
report["validation_status"] = "unconfirmed"
return scan_module.FixPreparationResultV1(
state="ready",
stop_reason="Approved.",
source_identity=request.candidate.source_identity,
candidate=request.candidate,
candidate_digest=request.candidate.digest(),
artifact_ref=str(artifact),
)
monkeypatch.setattr(scan_module, "finish_native_fix", finish_preparation)
fixes.sink = AsyncMock(return_value=True)
await fixes.spawn(
"finding", spawn, parent_ctx=context.context, name="Fix", task="Repair", skills=[]
)
with pytest.raises(ValueError, match="changed or was withdrawn"):
await callback(None, None)
assert fixes.records["finding"]["status"] == "stopped"
assert "artifact" not in fixes.records["finding"]
assert not list((tmp_path / "state/fixes").glob("*/prepared-fix.zip"))
assert all(call.args[2:] == (None, None) for call in fixes.sink.await_args_list)
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_worktree_process_cleanup_never_terminates_parent_sessions(tmp_path): async def test_worktree_process_cleanup_never_terminates_parent_sessions(tmp_path):
parent = LocalSandbox(tmp_path) parent = LocalSandbox(tmp_path)