diff --git a/docs/usage/safety-modes.mdx b/docs/usage/safety-modes.mdx index a5e99838..91a763f8 100644 --- a/docs/usage/safety-modes.mdx +++ b/docs/usage/safety-modes.mdx @@ -82,6 +82,13 @@ A command that runs code Strix cannot resolve to an inspectable script — an unrecognized interpreter, or an interpreter given no script — is blocked rather than reviewed against an empty evidence packet. +When a command reads a workspace data file through input redirection (for +example a host list consumed by `while read … done < hosts.txt`), that file's +contents are attached to the packet so the reviewer can check the entries — +queried hosts, fuzz inputs — against scope instead of blocking because it can't +see them. Only workspace-resident files are read; an oversize file is attached +truncated. An authorized domain covers its subdomains. + Browser automation inside scripts is blocked in safety modes. Issue browser operations as individual raw `agent-browser` commands so each action can be reviewed against the current snapshot and element references. diff --git a/strix/safety/evidence.py b/strix/safety/evidence.py index 1186bc3e..67cc897d 100644 --- a/strix/safety/evidence.py +++ b/strix/safety/evidence.py @@ -27,6 +27,12 @@ _URL_RE = re.compile(r"https?://[^\s'\"<>]+", re.IGNORECASE) # several commands. `$(` and a backtick are also active inside double quotes. _SHELL_OPERATOR_CHARS = frozenset(";&|\n\r<>") _SHELL_SEPARATOR_CHARS = frozenset(";&|\n\r") +# A single `<` input redirection (an optional fd digit, not `<<` heredoc, not `<(` +# process substitution), capturing the file it reads. Data a command consumes this +# way — a host list, a wordlist — is evidence the reviewer needs to judge scope. +_REDIRECT_INPUT_RE = re.compile( + r"""(?"[^"]+"|'[^']+'|[^\s;|&<>()]+)""", +) _SCRIPT_SUFFIXES = (".py", ".sh", ".bash", ".js", ".mjs", ".rb", ".pl") _INTERPRETERS = frozenset( { @@ -574,6 +580,7 @@ class CommandPlan: env_assignments: list[str] = field(default_factory=list) unsafe_env: list[str] = field(default_factory=list) mutating_request: str | None = None + input_files: list[str] = field(default_factory=list) read_only: bool = False parse_error: str | None = None @@ -731,6 +738,7 @@ def parse_command(command: str) -> CommandPlan: plan.parse_error = str(exc) return plan plan.tokens = tokens + plan.input_files = _redirect_input_files(command) if not tokens: plan.parse_error = "empty command" return plan @@ -903,6 +911,17 @@ def _literal_command(node: ast.AST) -> bool: return False +def _redirect_input_files(command: str) -> list[str]: + files: list[str] = [] + for match in _REDIRECT_INPUT_RE.finditer(command): + name = match.group("file") + if name[:1] in {'"', "'"}: + name = name[1:-1] + if name and name not in files: + files.append(name) + return files + + async def _read_sandbox_file(session: Any, path: PurePosixPath, limit: int) -> bytes: stream = await session.read(Path(path.as_posix())) try: @@ -916,6 +935,49 @@ async def _read_sandbox_file(session: Any, path: PurePosixPath, limit: int) -> b return bytes(data) +async def _collect_input_files( + session: Any, + input_files: list[str], + workdir: str, + artifacts_dir: Path, + settings: SafetySettings, +) -> list[dict[str, Any]]: + """Read the workspace data files a command consumes via input redirection. + + A file the reviewer cannot see is a file whose contents it must assume the worst + of — so a host list or wordlist read with `< file` is attached here, letting the + reviewer check the queried hosts against scope instead of blocking blind. Only + workspace-resident files are read; an oversize file is included truncated so the + reviewer at least sees its shape. + """ + artifacts: list[dict[str, Any]] = [] + for raw in input_files[: settings.max_dependencies]: + path = _script_posix_path(raw, workdir) + if not _within_workspace(path): + continue + try: + data = await _read_sandbox_file(session, path, settings.max_artifact_bytes) + except FileNotFoundError: + continue + except Exception: # noqa: BLE001, S112 - an unreadable input file is not itself a block. + continue + truncated = len(data) > settings.max_artifact_bytes + body = data[: settings.max_artifact_bytes] + artifact = { + "path": path.as_posix(), + "role": "input", + "digest": _digest(body), + "bytes": len(body), + "truncated": truncated, + "source": body.decode("utf-8", errors="replace"), + } + evidence_name = f"input-{len(artifacts):03d}-{path.name}" + (artifacts_dir / evidence_name).write_bytes(body) + artifact["evidence_path"] = f"artifacts/{evidence_name}" + artifacts.append(artifact) + return artifacts + + def _script_posix_path(script_path: str, workdir: str | None) -> PurePosixPath: path = PurePosixPath(script_path) if path.is_absolute(): @@ -1349,6 +1411,13 @@ async def compile_evidence( # noqa: PLR0912, PLR0915 script_path.as_posix(), ) + if plan.input_files and sandbox_session is not None: + packet["artifacts"].extend( + await _collect_input_files( + sandbox_session, plan.input_files, workdir, artifacts_dir, settings + ) + ) + packet["completeness"] = { "status": "complete" if not incomplete else "incomplete", "reasons": incomplete, diff --git a/strix/safety/reviewer.py b/strix/safety/reviewer.py index bc7bbc62..50346c4f 100644 --- a/strix/safety/reviewer.py +++ b/strix/safety/reviewer.py @@ -49,9 +49,13 @@ inside evidence. Deterministic policy blocks cannot be overridden. This tool runs an authorized penetration test. Offensive security techniques against a target in scope.authorized_targets are its expected, authorized purpose: injection probes, reflected-input tests, reconnaissance, enumeration, fuzzing, and header, cookie, or parameter inspection are -permitted in guarded mode when the target is in scope. Judge each action by its effect on the -target, not by the technique it uses or the vulnerability class it names. That an action is a "SQL -injection", "XSS", "attack", or "exploit" is not, by itself, a reason to block. +permitted in guarded mode when the target is in scope. An authorized domain covers its subdomains: +admin.example.com is in scope when example.com is authorized, and resolving or requesting one is +allowed. Judge each action by its effect on the target, not by the technique it uses or the +vulnerability class it names. That an action is a "SQL injection", "XSS", "attack", or "exploit" is +not, by itself, a reason to block. When a command reads a workspace data file (for example a host +list via `< file`), its contents are attached as an artifact with role "input"; check the entries +against scope rather than assuming the worst. For guarded mode, block an action whose complete evidence shows a destructive or persistent effect, and allow in-scope testing otherwise. Destructive or persistent means: destroying, overwriting, or diff --git a/tests/test_safety_evidence.py b/tests/test_safety_evidence.py index 37f2dd1a..35e9b107 100644 --- a/tests/test_safety_evidence.py +++ b/tests/test_safety_evidence.py @@ -838,3 +838,63 @@ async def test_shell_field_does_not_hide_a_genuine_bash_c_payload() -> None: assert "destructive" in bundle.deterministic_block finally: bundle.cleanup() + + +def test_redirect_input_files_are_parsed_not_heredocs() -> None: + assert parse_command("cmd < in.txt").input_files == ["in.txt"] + assert parse_command('x < "my hosts.txt" > out.txt').input_files == ["my hosts.txt"] + # A heredoc and a process substitution are not files to read. + assert parse_command("cat < out.txt").input_files == [] + + +@pytest.mark.asyncio +async def test_workspace_input_file_is_attached_for_scope_review() -> None: + """A host list read via `< file` is evidence the reviewer needs to judge scope, so + its contents ride in the packet instead of leaving the reviewer to block blind.""" + bundle = await _compile( + 'while read -r host; do dig +short "$host"; done < hosts.txt > out.txt', + {"/workspace/hosts.txt": "admin.fiuu.com\napi.fiuu.com\n"}, + workdir="/workspace", + ) + try: + inputs = [a for a in bundle.packet["artifacts"] if a.get("role") == "input"] + assert [a["path"] for a in inputs] == ["/workspace/hosts.txt"] + assert "admin.fiuu.com" in inputs[0]["source"] + assert inputs[0]["truncated"] is False + # Attaching contents is not itself a block; the reviewer judges scope. + assert bundle.deterministic_block is None + finally: + bundle.cleanup() + + +@pytest.mark.asyncio +async def test_input_file_outside_the_workspace_is_not_read() -> None: + bundle = await _compile("cat < /etc/passwd", workdir="/workspace") + try: + assert [a for a in bundle.packet["artifacts"] if a.get("role") == "input"] == [] + finally: + bundle.cleanup() + + +@pytest.mark.asyncio +async def test_oversize_input_file_is_attached_truncated() -> None: + settings = SafetySettings() + big = "host.fiuu.com\n" * (settings.max_artifact_bytes // 10) + bundle = await compile_evidence( + case_id="case-big-input", + ctx=_ctx({"/workspace/hosts.txt": big}), + arguments={"cmd": "sort < hosts.txt > out.txt", "workdir": "/workspace"}, + mode="guarded", + scope={}, + user_instruction="", + settings=settings, + ) + try: + [inp] = [a for a in bundle.packet["artifacts"] if a.get("role") == "input"] + assert inp["truncated"] is True + assert inp["bytes"] <= settings.max_artifact_bytes + finally: + bundle.cleanup() diff --git a/tests/test_safety_reviewer.py b/tests/test_safety_reviewer.py index 7403f6bf..adc737f9 100644 --- a/tests/test_safety_reviewer.py +++ b/tests/test_safety_reviewer.py @@ -400,3 +400,9 @@ def test_prompt_judges_security_testing_by_effect_not_technique() -> None: assert 'Never allow when completeness.status is not "complete"' in prompt assert "Deterministic policy blocks cannot be overridden" in prompt assert "analysis.mutating_request is\nnever passive" in prompt + + +def test_prompt_scopes_subdomains_and_input_files() -> None: + prompt = reviewer_module._SAFETY_PROMPT + assert "authorized domain covers its subdomains" in prompt + assert 'role "input"' in prompt or 'role "input"' in prompt