diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 7e6f918b8ca..e084e61fe8f 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -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. diff --git a/tools/approval.py b/tools/approval.py index e7c1cecba66..2797f4a7ad5 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -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.