mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-13 14:02:16 +00:00
fix(security): block subshell/brace-group wrappers at the hardline floor
Wrapping a catastrophic command in a bare subshell or brace group walked straight past the unconditional hardline floor -- even under --yolo, /yolo, approvals.mode=off, and cron approve mode. The command-substitution forms were already caught; the bare paren / brace-group forms were the gap. Rather than add the paren and brace openers to the flat _CMDPOS pattern class (which cannot tell a real subshell opener from one sitting inside a quoted argument, and would false-positive on ordinary prose such as a PR title that merely mentions the trigger word), teach the existing QUOTE-AWARE command-start tokenizer (_iter_shell_command_starts) to treat the paren and brace openers as command starts, then emit a detection variant that marks each real command start with a newline (already a _CMDPOS separator). Openers inside quotes never register as starts, so quoted arguments are left untouched while real subshell/brace bypasses now anchor. One place covers every _CMDPOS rule (shutdown/reboot/init/ systemctl/telinit and the rm root/home/system floor). Tests: subshell/brace bypasses added to the hardline-block, root-wipe, and yolo-bypass sets; a regression set asserts quoted paren/brace prose is NOT blocked (guards our own gh-pr-create workflow).
This commit is contained in:
parent
6d1291f2cc
commit
6a6fd42111
2 changed files with 124 additions and 0 deletions
|
|
@ -118,6 +118,23 @@ _HARDLINE_BLOCK = [
|
|||
"exec shutdown",
|
||||
"nohup reboot",
|
||||
"setsid poweroff",
|
||||
# Bare subshell `(cmd)` and brace-group `{ cmd; }` openers put the trigger
|
||||
# at a real command position, so they must hit the floor just like `$(…)`.
|
||||
# These slipped through before the quote-aware command-start tokenizer
|
||||
# learned to recognize `(` / `{` (issue: (reboot) walked past --yolo).
|
||||
"(reboot)",
|
||||
"( reboot )",
|
||||
"(shutdown -h now)",
|
||||
"(poweroff)",
|
||||
"(halt)",
|
||||
"(init 0)",
|
||||
"(systemctl reboot)",
|
||||
"(sudo reboot)",
|
||||
"{ reboot; }",
|
||||
"{ shutdown -h now; }",
|
||||
"{ poweroff; }",
|
||||
"true && (reboot)",
|
||||
"echo hi; { reboot; }",
|
||||
]
|
||||
|
||||
|
||||
|
|
@ -225,6 +242,18 @@ _DATA_ARG_NOT_A_COMMAND = [
|
|||
'echo "rm -rf /"',
|
||||
'printf "%s" "rm -rf /"',
|
||||
'gh issue comment 1 --body "the fix blocks rm -rf //"',
|
||||
# A `(` or `{` INSIDE a quoted argument is prose, not a subshell/brace
|
||||
# opener — the trigger word after it is data. Naively adding `(` / `{` to
|
||||
# the flat command-position class blocked these (it broke our own
|
||||
# `gh pr create --title "…(reboot)…"` workflow); the quote-aware tokenizer
|
||||
# must leave them alone.
|
||||
'gh pr create --title "block (reboot) spellings"',
|
||||
'git commit -m "(rm -rf /) note"',
|
||||
'echo "(reboot)"',
|
||||
'echo "{ reboot; }"',
|
||||
"echo '(poweroff)'",
|
||||
"echo '{ rm -rf /; }'",
|
||||
'find . -name "*(reboot)*"',
|
||||
]
|
||||
|
||||
|
||||
|
|
@ -249,6 +278,11 @@ _COMMAND_POSITION_ROOT_WIPES = [
|
|||
"$(rm -rf /)",
|
||||
"`rm -rf /`",
|
||||
'echo "$(rm -rf /)"',
|
||||
# Bare subshell / brace-group openers are real command positions too.
|
||||
"(rm -rf /)",
|
||||
"{ rm -rf /; }",
|
||||
"(rm -rf ~)",
|
||||
"(sudo rm -rf /)",
|
||||
]
|
||||
|
||||
|
||||
|
|
@ -372,6 +406,47 @@ def test_root_collapse_pattern_leaves_real_paths_alone(clean_session):
|
|||
assert not is_hl, f"{cmd!r} must not be hardline-blocked (over-match)"
|
||||
|
||||
|
||||
def test_subshell_brace_group_cannot_bypass_hardline(clean_session, monkeypatch):
|
||||
"""Wrapping a catastrophic command in `(…)` or `{ …; }` must not bypass
|
||||
the floor, even under yolo. `(reboot)` / `{ shutdown -h now; }` walked
|
||||
straight past the guard before the command-start tokenizer recognized the
|
||||
subshell and brace-group openers.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_YOLO_MODE", "1")
|
||||
|
||||
for cmd in ["(reboot)", "( reboot )", "(shutdown -h now)", "(poweroff)",
|
||||
"(systemctl reboot)", "(init 0)", "(sudo reboot)",
|
||||
"{ reboot; }", "{ shutdown -h now; }", "{ poweroff; }",
|
||||
"(rm -rf /)", "{ rm -rf /; }", "(rm -rf ~)",
|
||||
"true && (reboot)", "echo hi; { reboot; }"]:
|
||||
r1 = check_dangerous_command(cmd, "local")
|
||||
assert r1["approved"] is False, f"yolo leaked hardline on {cmd!r} (check_dangerous_command)"
|
||||
assert r1.get("hardline") is True
|
||||
|
||||
r2 = check_all_command_guards(cmd, "local")
|
||||
assert r2["approved"] is False, f"yolo leaked hardline on {cmd!r} (check_all_command_guards)"
|
||||
assert r2.get("hardline") is True
|
||||
|
||||
|
||||
def test_quoted_paren_brace_prose_not_blocked_under_yolo(clean_session, monkeypatch):
|
||||
"""A `(` / `{` inside a quoted argument is prose, not a command opener.
|
||||
|
||||
Regression guard: naively adding `(` / `{` to the flat command-position
|
||||
class blocked ordinary quoted arguments — including our own
|
||||
`gh pr create --title "…(reboot)…"` workflow. The quote-aware tokenizer
|
||||
must leave quoted text untouched, so these stay runnable.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_YOLO_MODE", "1")
|
||||
|
||||
for cmd in ['gh pr create --title "block (reboot) spellings"',
|
||||
'git commit -m "(rm -rf /) note"',
|
||||
'echo "(reboot)"', 'echo "{ reboot; }"',
|
||||
"echo '(poweroff)'", 'find . -name "*(reboot)*"']:
|
||||
assert detect_hardline_command(cmd)[0] is False, (
|
||||
f"quoted prose false-positived on the hardline floor: {cmd!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_line_continuation_root_wipe_cannot_bypass_hardline(clean_session, monkeypatch):
|
||||
"""A line-continuation root wipe must stay blocked even under yolo.
|
||||
|
||||
|
|
|
|||
|
|
@ -1158,6 +1158,17 @@ def _iter_shell_command_starts(command: str):
|
|||
starts.append(i + 2)
|
||||
i += 2
|
||||
continue
|
||||
# Bare subshell `(cmd)` and brace group `{ cmd; }` openers begin a new
|
||||
# command context, just like `;` or `$(`. We only reach this branch
|
||||
# OUTSIDE any quote (the quote arms above `continue` first), so a `(`
|
||||
# or `{` sitting inside a quoted argument — `--title "block (reboot)"`,
|
||||
# `echo "{ reboot; }"` — never registers a command start. That is the
|
||||
# whole reason this lives in the quote-aware tokenizer instead of the
|
||||
# flat `_CMDPOS` regex, which cannot tell quoted text from real syntax.
|
||||
if ch in ("(", "{"):
|
||||
starts.append(i + 1)
|
||||
i += 1
|
||||
continue
|
||||
if ch == ";":
|
||||
starts.append(i + 1)
|
||||
i += 1
|
||||
|
|
@ -1190,6 +1201,29 @@ def _iter_shell_command_starts(command: str):
|
|||
yield start
|
||||
|
||||
|
||||
def _mark_command_starts(command: str) -> str:
|
||||
"""Insert a newline before each real (quote-aware) command start.
|
||||
|
||||
``\\n`` is already a ``_CMDPOS`` separator, so this rewrites subshell
|
||||
``(cmd)`` and brace-group ``{ cmd; }`` openers — which the flat pattern
|
||||
class deliberately omits — into a form the anchored hardline/dangerous
|
||||
patterns recognize, WITHOUT the quoted-prose false positives that adding
|
||||
``(`` / ``{`` to ``_CMDPOS`` would cause. Starts inside quotes are never
|
||||
produced by ``_iter_shell_command_starts``, so quoted arguments such as
|
||||
``--title "block (reboot)"`` are left exactly as-is.
|
||||
"""
|
||||
# Collect the (whitespace-skipped) start offsets, drop 0 (already anchored
|
||||
# by ``^``), and splice a newline in front of each — right-to-left so the
|
||||
# earlier offsets stay valid as we mutate.
|
||||
offsets = sorted(o for o in _iter_shell_command_starts(command) if o > 0)
|
||||
if not offsets:
|
||||
return command
|
||||
out = command
|
||||
for offset in reversed(offsets):
|
||||
out = out[:offset] + "\n" + out[offset:]
|
||||
return out
|
||||
|
||||
|
||||
def _iter_shell_command_word_spans(command: str):
|
||||
"""Yield command-position words that may be executable names."""
|
||||
for command_start in _iter_shell_command_starts(command):
|
||||
|
|
@ -1236,6 +1270,21 @@ def _command_detection_variants(command: str):
|
|||
normalized = _normalize_command_for_detection(command)
|
||||
seen = {normalized}
|
||||
yield normalized
|
||||
# Subshell `(cmd)` and brace-group `{ cmd; }` openers put `cmd` at a real
|
||||
# command position, but the flat `_CMDPOS`-anchored patterns can't see it:
|
||||
# their start-position class deliberately omits `(`/`{` because a bare
|
||||
# regex cannot tell `(reboot)` (real subshell) from `--title "(reboot)"`
|
||||
# (quoted prose) — adding them there regresses ordinary quoted arguments.
|
||||
# Instead, reconstruct the command with a newline (already a `_CMDPOS`
|
||||
# separator) inserted at each command start the QUOTE-AWARE tokenizer
|
||||
# found. Openers inside quotes never yield a start, so quoted prose is
|
||||
# untouched, while `(reboot)` / `{ shutdown -h now; }` now anchor. This
|
||||
# covers every `_CMDPOS` rule (shutdown/reboot/init/systemctl/telinit and
|
||||
# the rm root/home/system floor) in one place.
|
||||
marked = _mark_command_starts(normalized)
|
||||
if marked != normalized and marked not in seen:
|
||||
seen.add(marked)
|
||||
yield marked
|
||||
# Shell quoting/escaping can spell a dangerous executable name in pieces
|
||||
# (for example r\m or r''m). Keep that deobfuscation scoped to command
|
||||
# words so similarly shaped arguments do not become false positives.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue