fix(approvals): preserve env argv and shell comment boundaries

evals/approval_deny_dispatch.py

@@ -18,11 +18,14 @@ with tempfile.TemporaryDirectory(prefix="hermes-deny-dispatch-") as directory:
     os.environ.update(PATH=directory + ":/usr/bin:/bin", HOME=directory, HERMES_HOME=directory,
                       HERMES_INTERACTIVE="1", TERMINAL_ENV="local")
     (home / "config.yaml").write_text(
-        'approvals:\n  mode: "off"\n  deny: ["sudo *"]\n'
+        'approvals:\n  mode: "off"\n  deny: ["sudo *", "printf SAFE", "git status"]\n'
         'security:\n  tirith_enabled: false\n', encoding="utf-8")
     executable = home / "sudo"
     executable.write_text('#!/bin/sh\nprintf "OWNED_EXECUTABLE_REACHED\\n"\n', encoding="utf-8")
     executable.chmod(0o700)
+    # Prefer GNU env when the host's default env implements different -S syntax.
+    if Path("/usr/bin/gnuenv").exists():
+        (home / "env").symlink_to("/usr/bin/gnuenv")
     sys.path.insert(0, str(source))
     from tools import terminal_tool  # noqa: F401
     from tools.registry import registry
@@ -33,7 +36,12 @@ with tempfile.TemporaryDirectory(prefix="hermes-deny-dispatch-") as directory:
              "command " + binary + " -n id -u",
              "true && " + binary + " -n id -u",
              "bash -c " + shlex.quote(binary + " -n id -u"),
-             "printf '%s\\n' 'sudo -n id -u'"]
+             "printf '%s\\n' 'sudo -n id -u'",
+             "env -S 'printf %s; sudo -n id'", "env -S printf SAFE",
+             "env -S git status", "echo ok # ; bash -c " + shlex.quote(binary + " -n id"),
+             "env -S 'printf %s' 'sudo -n id'", "env -S printf BENIGN",
+             "env -S " + shlex.quote("bash -c " + shlex.quote(binary + " -n id")),
+             "echo ok # ignored\n bash -c " + shlex.quote(binary + " -n id")]
     rows = []
     for command in cases:
         result = registry.dispatch("terminal", {"command": command, "workdir": directory, "timeout": 10})

tests/tools/test_approval_deny_rules.py

@@ -82,11 +82,17 @@ def test_deny_follows_executable_identity(deny_config, clean_env, monkeypatch):
         'echo "$(/usr/bin/sudo -n id -u)"',
         "bash -lc 'env -i /usr/bin/sudo -n id -u'",
         "env -S '/usr/bin/sudo -n id -u'",
+        "env -S /usr/bin/sudo -n id -u",
+        "env --split-string=/usr/bin/sudo -n id -u",
+        "env -S \"bash -c '/usr/bin/sudo -n id -u'\"",
+        "echo ok # ignored\n bash -c '/usr/bin/sudo -n id -u'",
+        "printf SAFE", "env -S printf SAFE", "env -Sprintf SAFE",
+        "env --split-string=printf SAFE", "env -S 'printf' SAFE",
         "2>/tmp/log FOO=bar /usr/bin/sudo -n id -u",
         "if true; then /usr/bin/sudo -n id -u; fi",
     ]
     for mode, yolo in (("manual", False), ("off", False), ("manual", True)):
-        deny_config(["sudo *"], mode=mode)
+        deny_config(["sudo *", "printf SAFE"], mode=mode)
         monkeypatch.setattr(mod, "_YOLO_MODE_FROZEN", yolo)
         for command in commands:
             for guard in (mod.check_dangerous_command, mod.check_all_command_guards):
@@ -108,10 +114,18 @@ def test_deny_projection_preserves_data_and_path_rules(deny_config):
         "env -u sudo printf ok", "exec -a sudo printf ok",
         "ionice --pid sudo", "chrt --pid sudo", "taskset --pid 1 sudo",
         "echo 'first\nsudo -n id -u'", "echo ok # ; sudo -n id -u",
+        "env -S 'printf %s; sudo -n id'",
+        "env -S 'printf %s' 'sudo -n id'",
+        "env -S 'printf %s' '$(sudo -n id)'",
+        "echo ok # ; bash -c '/usr/bin/sudo -n id'",
+        "echo ok # unmatched '\n printf ok",
+        "printf '%s' '# ; bash -c sudo'",
         "git log --grep='git status'", r'printf "%s" "a\"; sudo -n id -u"',
     ):
         assert approval_floors._match_user_deny_rule(command) is None, command
-    for command in ("env git status; echo ok", "(git status)", "git\tstatus # comment"):
+    for command in ("env git status; echo ok", "(git status)", "git\tstatus # comment",
+                    "env -S git status", "env -S 'git' status", "env -Sgit status",
+                    "env --split-string=git status"):
         assert approval_floors._match_user_deny_rule(command) == "git status", command
     assert approval_floors._match_user_deny_rule('env git st""atus') == "git status"
     deny_config(['printf "a  b"'])

tools/approval_detection.py

@@ -772,11 +772,11 @@ def _shell_segment_tokens(segment: str, start: int) -> list[str] | None:
 def _iter_top_level_shell_segments(command: str):
     """Yield top-level command segments in one left-to-right pass."""
     start = 0
-    for kind, i, _, quote in _scan_shell(command):
-        if kind == "char" and quote is None and command[i] in ";&|\n":
+    for kind, i, j, quote in _scan_shell(command, comments=True):
+        if kind == "comment" or (kind == "char" and quote is None and command[i] in ";&|\n"):
             if start < i:
                 yield command[start:i]
-            start = i + 1
+            start = j
     if start < len(command):
         yield command[start:]
 
@@ -1149,6 +1149,10 @@ def _iter_shell_command_word_spans(command: str):
                 continue
             if wrapper and options and deobfuscated.startswith("-"):
                 option = deobfuscated.split("=", 1)[0]
+                if wrapper == "env" and (option == "--split-string" or deobfuscated.startswith("-S")):
+                    # The split string and remaining argv form ONE command, handled
+                    # by _env_split_payload; the suffix is not a new executable.
+                    break
                 queries = _COMMAND_WRAPPER_NON_EXECUTING_OPTIONS.get(wrapper, set())
                 if option in queries or (wrapper == "command" and not option.startswith("--")
                                          and set(option[1:]) & {"v", "V"}):
@@ -1185,8 +1189,17 @@ def _env_split_payload(tokens: list[str]) -> str | None:
             return None
         option, equals, value = token.partition("=")
         if option == "--split-string" or token.startswith("-S"):
-            payload = value if equals else token[2:] if token.startswith("-S") else ""
-            return payload or (tokens[index + 1] if index + 1 < len(tokens) else None)
+            attached = equals if option == "--split-string" else len(token) > 2
+            if not attached:
+                index += 1
+            payload = (value if option == "--split-string" else token[2:]) if attached else (
+                tokens[index] if index < len(tokens) else "")
+            try:
+                # env splits argv, not shell syntax: protect literal separators and
+                # expansions when feeding the existing command-position scanner.
+                return shlex.join(shlex.split(payload) + tokens[index + 1:])
+            except ValueError:
+                return None
         index += 2 if not equals and option in _COMMAND_WRAPPER_OPTIONS_WITH_ARG["env"] else 1
     return None
 

website/docs/user-guide/security.md

@@ -157,7 +157,7 @@ Details:
 - Patterns are [fnmatch](https://docs.python.org/3/library/fnmatch.html) globs (`*`, `?`, `[...]`) matched **case-insensitively** against the whole command text and individual executable-command candidates. `git push --force*` matches `git push --force origin main` but not `git push origin main`.
 - Matching runs over the same normalized/deobfuscated command variants the dangerous-pattern detector uses, so simple quoting tricks (`git pu""sh --force`) don't slip past a rule.
 - Executable candidates retain the literal path and also match its basename: `sudo *` covers `/usr/bin/sudo -n id` and `./sudo -n id`. A path-specific rule such as `/usr/bin/sudo *` does **not** become a rule for every binary named `sudo`.
-- Quote-aware parsing exposes commands after assignments, leading redirections, `;`, `&&`, `||`, pipelines, groups, command substitutions, and ordinary `if`/`then`/`else`/`do` transitions. Supported launchers include `sudo`, `env`, `command`, `exec`, `nohup`, `setsid`, `time`, `nice`, `timeout`, `stdbuf`, `ionice`, `chrt`, `taskset`, and `chroot`. Known option operands are skipped; `command -v`/`-V` lookups are not executions. Shell `-c` payloads and literal `env -S` / `--split-string` payloads are inspected recursively.
+- Quote-aware parsing exposes commands after assignments, leading redirections, `;`, `&&`, `||`, pipelines, groups, command substitutions, and ordinary `if`/`then`/`else`/`do` transitions. Supported launchers include `sudo`, `env`, `command`, `exec`, `nohup`, `setsid`, `time`, `nice`, `timeout`, `stdbuf`, `ionice`, `chrt`, `taskset`, and `chroot`. Known option operands are skipped; `command -v`/`-V` lookups are not executions. Shell `-c` payloads are inspected recursively. Literal `env -S` / `--split-string` strings are split into arguments with the remaining command arguments appended; shell punctuation inside those arguments stays data unless an actual shell `-c` consumes it. Shell comments do not introduce executable candidates.
 - In the additional executable candidates, whitespace **between** words is collapsed, but quoted argument content and argument paths are retained. An exact rule such as `git status` therefore also matches `env git\tstatus; echo done` (where `\t` represents a tab). Quoted mentions such as `echo 'sudo -n id'` are not promoted to commands. Existing whole-input globs such as `*sudo*` still intentionally match mentions anywhere.
 - **YAML quoting:** always quote patterns. A bare leading `*` is a YAML alias and fails to parse; `{`, `!`, and `: ` have their own YAML meanings. Single quotes are safest for shell-ish content.
 - User-defined deny rules apply to all terminal backends, including isolated containers, before any backend-specific approval shortcut.