mirror of
https://github.com/usestrix/strix.git
synced 2026-08-25 12:22:37 +02:00
fix(safety): do not treat data tools reading script-named files as execution
Trace review of two guarded-mode runs found the unresolved-execution guard firing on ordinary commands: curl fetching a .js asset, rg over a .py file, sed and cat and cp on script-named files — 24 blocks in one run. The guard was meant to fail closed on an unknown interpreter handed a script, but a read, transfer, or text tool takes such a file as data, not as a program to run. Exclude known read commands, HTTP clients, and a set of text/data tools from the script-suffix branch, so only a genuinely unknown executable given a script still fails closed. awk moves from the interpreter set to the data tools: its program is an inline positional argument, not a -c or script file the entrypoint reader can resolve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -34,15 +34,15 @@ _REDIRECT_INPUT_RE = re.compile(
|
||||
r"""(?<![<])\d?<(?![<(])\s*(?P<file>"[^"]+"|'[^']+'|[^\s;|&<>()]+)""",
|
||||
)
|
||||
_SCRIPT_SUFFIXES = (".py", ".sh", ".bash", ".js", ".mjs", ".rb", ".pl")
|
||||
# `awk` is intentionally absent: its program is an inline positional argument, not a
|
||||
# `-c`/script-file the entrypoint reader can resolve, so it belongs with the data tools.
|
||||
_INTERPRETERS = frozenset(
|
||||
{
|
||||
"awk",
|
||||
"bash",
|
||||
"bun",
|
||||
"dash",
|
||||
"deno",
|
||||
"fish",
|
||||
"gawk",
|
||||
"ksh",
|
||||
"lua",
|
||||
"node",
|
||||
@@ -59,6 +59,54 @@ _INTERPRETERS = frozenset(
|
||||
"zsh",
|
||||
}
|
||||
)
|
||||
# Commands that take a file argument as data to read or transform, never as a program to
|
||||
# execute. They must not trip the unresolved-execution guard when handed a script-named
|
||||
# file (a `.js` asset fetched by curl, a `.py` grepped by rg, a file edited by sed).
|
||||
_DATA_COMMANDS = frozenset(
|
||||
{
|
||||
"awk",
|
||||
"base64",
|
||||
"bunzip2",
|
||||
"bzip2",
|
||||
"cmp",
|
||||
"column",
|
||||
"comm",
|
||||
"cp",
|
||||
"csplit",
|
||||
"cut",
|
||||
"dd",
|
||||
"diff",
|
||||
"fmt",
|
||||
"fold",
|
||||
"gawk",
|
||||
"gunzip",
|
||||
"gzip",
|
||||
"jq",
|
||||
"join",
|
||||
"ln",
|
||||
"md5sum",
|
||||
"mv",
|
||||
"nl",
|
||||
"od",
|
||||
"paste",
|
||||
"sed",
|
||||
"sha1sum",
|
||||
"sha256sum",
|
||||
"sort",
|
||||
"split",
|
||||
"strings",
|
||||
"tac",
|
||||
"tar",
|
||||
"tee",
|
||||
"tr",
|
||||
"uniq",
|
||||
"unxz",
|
||||
"xxd",
|
||||
"xz",
|
||||
"yq",
|
||||
"zcat",
|
||||
}
|
||||
)
|
||||
# Versioned names (`python3.12`, `node20`) are the same interpreters. Matching them here
|
||||
# rather than enumerating versions keeps a new point release from silently becoming an
|
||||
# unrecognized executable whose script is never inspected.
|
||||
@@ -613,9 +661,19 @@ def _unresolved_execution(executable: str, args: list[str]) -> str | None:
|
||||
|
||||
Without this, an executable outside the interpreter set produces a packet that is
|
||||
empty but still stamped complete — the exact shape the reviewer is told it may allow.
|
||||
A file argument only counts as unresolved execution for an *unknown* executable:
|
||||
read, transfer, and text tools take a `.py`/`.js`/`.sh` file as data, not as a program
|
||||
to run, so `curl …/app.js`, `rg pat file.py`, or `sed … file.sh` are not execution.
|
||||
"""
|
||||
if _is_interpreter(executable):
|
||||
return f"{executable} was given no script or inline source that can be inspected"
|
||||
if (
|
||||
executable in _KNOWN_READ_COMMANDS
|
||||
or executable in _HTTP_CLIENTS
|
||||
or executable in _DATA_COMMANDS
|
||||
or executable in _DESTRUCTIVE_COMMANDS
|
||||
):
|
||||
return None
|
||||
scripts = [arg for arg in args if arg.endswith(_SCRIPT_SUFFIXES)]
|
||||
if scripts:
|
||||
return (
|
||||
|
||||
@@ -898,3 +898,21 @@ async def test_oversize_input_file_is_attached_truncated() -> None:
|
||||
assert inp["bytes"] <= settings.max_artifact_bytes
|
||||
finally:
|
||||
bundle.cleanup()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"command",
|
||||
[
|
||||
"curl -sS https://example.test/js/app.js -o /dev/null",
|
||||
"wget https://example.test/main.bundle.js -O out.js",
|
||||
"rg -n 'pattern' /workspace/app.py",
|
||||
"sed -n '1,20p' /workspace/probe.py",
|
||||
"cat /workspace/onboarding.py",
|
||||
"cp /workspace/a.sh /workspace/b.sh",
|
||||
"awk '{print $1}' /workspace/hosts.py",
|
||||
],
|
||||
)
|
||||
def test_data_tools_reading_script_named_files_are_not_execution(command: str) -> None:
|
||||
"""A read/transfer/text tool takes a script-named file as data, not as a program to
|
||||
run, so it must not trip the unresolved-execution guard."""
|
||||
assert parse_command(command).parse_error is None
|
||||
|
||||
Reference in New Issue
Block a user