mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(agent): strip only the wrapper options that still run the child command
Independent review, extending "Tests pass without running": prefix stripping skipped every dash-token after timeout/nice/nohup/stdbuf, so `nice --help pytest -q` and `timeout --help 300 pytest -q` matched the detected pytest and their exit 0 (help printed, nothing run) recorded a pass. Each wrapper now has its own list of options that still run the child, with the ones taking a value; timeout also needs its duration. Any other option (--help, --version, -h, -V, typos) leaves the wrapper in place, so the run does not match, as on main.
This commit is contained in:
1 parent
8bfa0160f8
commit
fa6b9c81ba
2 files changed
+52
-14
No files matched your search
@@ -271,26 +271,59 @@ def _canonical_tokens(canonical: str) -> list[str]:
|
||||
return []
|
||||
|
||||
|
||||
_STATUS_PRESERVING_WRAPPERS = {"timeout", "nice", "nohup", "stdbuf"}
|
||||
# Wrapper options whose value is the next token (``timeout -s KILL 300``).
|
||||
_WRAPPER_VALUE_FLAGS = {"-s", "--signal", "-k", "--kill-after", "-n", "--adjustment", "-i", "-o", "-e"}
|
||||
# A bare count or a ``timeout`` duration: ``300``, ``5m``, ``1.5h``.
|
||||
_WRAPPER_ARG_RE = re.compile(r"^\d+(?:\.\d+)?[smhd]?$")
|
||||
# Status-preserving wrappers -> (options that still run the child, options taking a value).
|
||||
# Anything else (``--help``, ``--version``) may exit without running the child: no match.
|
||||
_WRAPPER_OPTIONS: dict[str, tuple[frozenset[str], frozenset[str]]] = {
|
||||
"timeout": (frozenset({"-p", "--preserve-status", "-f", "--foreground", "-v", "--verbose"}),
|
||||
frozenset({"-s", "--signal", "-k", "--kill-after"})),
|
||||
"nice": (frozenset(), frozenset({"-n", "--adjustment"})),
|
||||
"nohup": (frozenset(), frozenset()),
|
||||
"stdbuf": (frozenset(), frozenset({"-i", "-o", "-e", "--input", "--output", "--error"})),
|
||||
}
|
||||
# A ``timeout`` duration: ``300``, ``5m``, ``1.5h``.
|
||||
_TIMEOUT_DURATION_RE = re.compile(r"^\d+(?:\.\d+)?[smhd]?$")
|
||||
|
||||
|
||||
def _wrapper_option_width(token: str, flags: frozenset[str], valued: frozenset[str]) -> int:
|
||||
"""Tokens one wrapper option spans (``-n 5`` 2, ``-n5``/``--signal=KILL`` 1), 0 if unknown."""
|
||||
if token in flags:
|
||||
return 1
|
||||
if token in valued:
|
||||
return 2
|
||||
attached = any(token.startswith(f"{v}=") if v.startswith("--") else token.startswith(v) for v in valued)
|
||||
return 1 if attached else 0
|
||||
|
||||
|
||||
def _skip_wrapper(tokens: list[str], i: int) -> Optional[int]:
|
||||
"""Index of the child command after the wrapper at ``tokens[i]``, ``None`` unless the
|
||||
wrapper's options and arguments are ones that run the child."""
|
||||
flags, valued = _WRAPPER_OPTIONS[tokens[i]]
|
||||
timeout = tokens[i] == "timeout"
|
||||
i += 1
|
||||
while i < len(tokens) and tokens[i].startswith("-") and tokens[i] != "--":
|
||||
width = _wrapper_option_width(tokens[i], flags, valued)
|
||||
if not width:
|
||||
return None
|
||||
i += width
|
||||
i += int(i < len(tokens) and tokens[i] == "--")
|
||||
if timeout:
|
||||
if i >= len(tokens) or not _TIMEOUT_DURATION_RE.match(tokens[i]):
|
||||
return None
|
||||
i += 1
|
||||
return i
|
||||
|
||||
|
||||
def _strip_command_prefix(tokens: list[str]) -> list[str]:
|
||||
"""Remove harmless command prefixes: env assignments, command/time/noglob, and
|
||||
status-preserving wrappers with their flags and duration/niceness arguments."""
|
||||
status-preserving wrappers with the options and duration that still run the child."""
|
||||
i = 1 if tokens and tokens[0] == "env" else 0
|
||||
while i < len(tokens) and "=" in tokens[i] and not tokens[i].startswith("-"):
|
||||
i += 1
|
||||
while i < len(tokens):
|
||||
if tokens[i] in {"command", "time", "noglob"}:
|
||||
i += 1
|
||||
elif tokens[i] in _STATUS_PRESERVING_WRAPPERS:
|
||||
i += 1
|
||||
while i < len(tokens) and (tokens[i].startswith("-") or _WRAPPER_ARG_RE.match(tokens[i])):
|
||||
i += 2 if tokens[i] in _WRAPPER_VALUE_FLAGS else 1
|
||||
elif tokens[i] in _WRAPPER_OPTIONS and (child := _skip_wrapper(tokens, i)) is not None:
|
||||
i = child
|
||||
else:
|
||||
break
|
||||
return list(tokens[i:])
|
||||
|
||||
@@ -206,9 +206,13 @@ def test_dot_slash_script_invocation_matches_detected_script(tmp_path, monkeypat
|
||||
assert evidence.canonical_command == "scripts/run_tests.sh"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("command", ["bash -s scripts/run_tests.sh", "sh -s scripts/run_tests.sh"])
|
||||
def test_shell_flag_that_skips_the_script_is_not_evidence(tmp_path, monkeypatch, command):
|
||||
"""``bash -s`` reads commands from stdin: the script named after it never runs."""
|
||||
@pytest.mark.parametrize("command", [
|
||||
"bash -s scripts/run_tests.sh", "sh -s scripts/run_tests.sh",
|
||||
"nice --help scripts/run_tests.sh", "timeout --help 300 scripts/run_tests.sh",
|
||||
"nohup --version scripts/run_tests.sh", "stdbuf --help scripts/run_tests.sh",
|
||||
])
|
||||
def test_flag_that_skips_the_script_is_not_evidence(tmp_path, monkeypatch, command):
|
||||
"""``bash -s`` reads stdin and ``--help`` exits 0: the script named after it never runs."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
_node_project(tmp_path)
|
||||
|
||||
@@ -219,7 +223,8 @@ def test_shell_flag_that_skips_the_script_is_not_evidence(tmp_path, monkeypatch,
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"command",
|
||||
["timeout 300 pytest -q", "nice -n 5 pytest -q", "timeout 5m pytest -q", "timeout -s KILL 300 pytest -q"],
|
||||
["timeout 300 pytest -q", "nice -n 5 pytest -q", "timeout 5m pytest -q", "timeout -s KILL 300 pytest -q",
|
||||
"timeout --signal=KILL -k 10 5m pytest -q", "stdbuf -oL nohup pytest -q"],
|
||||
)
|
||||
def test_status_preserving_wrapper_still_attributes_the_run(tmp_path, monkeypatch, command):
|
||||
"""``timeout``/``nice`` return the child's status, so the run proves the verifier's."""
|
||||
|
||||
Reference in new issue
Block a user