diff --git a/evals/approval_deny_dispatch.py b/evals/approval_deny_dispatch.py new file mode 100644 index 0000000000..1206052e73 --- /dev/null +++ b/evals/approval_deny_dispatch.py @@ -0,0 +1,44 @@ +"""Offline production-dispatch receipt; never invokes a privileged binary. + +Run with the shared Python environment and a source tree argument to compare +base/head. The executable named sudo is an owned temporary marker script. +""" +import json +import os +from pathlib import Path +import shlex +import sys +import tempfile + +source = Path(sys.argv[1]).resolve() +with tempfile.TemporaryDirectory(prefix="hermes-deny-dispatch-") as directory: + home = Path(directory) + os.environ.clear() + # The terminal's own NOPASSWD probe must also resolve to our inert fixture. + 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' + '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) + sys.path.insert(0, str(source)) + from tools import terminal_tool # noqa: F401 + from tools.registry import registry + from tools import approval_context + + binary = shlex.quote(str(executable)) + cases = [binary + " -n id -u", "env -i " + binary + " -n id -u", + "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'"] + rows = [] + for command in cases: + result = registry.dispatch("terminal", {"command": command, "workdir": directory, "timeout": 10}) + if isinstance(result, str): + result = json.loads(result) + rows.append({"command": command, "result": result}) + print(json.dumps({"source": str(source), "config": approval_context._get_approval_config(), + "rows": rows}, indent=2)) diff --git a/tests/tools/test_approval_deny_rules.py b/tests/tools/test_approval_deny_rules.py index 194cf9f643..30656a9a4c 100644 --- a/tests/tools/test_approval_deny_rules.py +++ b/tests/tools/test_approval_deny_rules.py @@ -59,6 +59,66 @@ class TestMatchUserDenyRule: assert mod._match_user_deny_rule('git pu""sh --force origin main') is not None +def test_deny_follows_executable_identity(deny_config, clean_env, monkeypatch): + """Paths, prefixes and shell carriers cannot outrank an explicit deny.""" + commands = [ + "sudo -n id -u", "/usr/bin/sudo -n id -u", "./sudo -n id -u", + '"/usr/bin/su"do -n id -u', r"/usr/bin/sud\o -n id -u", + "FOO=bar /usr/bin/sudo -n id -u", "env sudo -n id -u", + "/usr/bin/env -i FOO=bar /usr/bin/sudo -n id -u", + "env -u FOO -- /usr/bin/sudo -n id -u", + "env -C /tmp /usr/bin/sudo -n id -u", + "command -p /usr/bin/sudo -n id -u", "exec -a label /usr/bin/sudo -n id -u", + "nohup /usr/bin/sudo -n id -u", "nice -n 5 /usr/bin/sudo -n id -u", + "timeout 5 /usr/bin/sudo -n id -u", + "setsid -f /usr/bin/sudo -n id -u", "time -p /usr/bin/sudo -n id -u", + "stdbuf --output L /usr/bin/sudo -n id -u", + "ionice --class 2 /usr/bin/sudo -n id -u", + "chrt --fifo 20 /usr/bin/sudo -n id -u", + "taskset --cpu-list 0 /usr/bin/sudo -n id -u", + "chroot --userspec root:root /srv /usr/bin/sudo -n id -u", + "true && /usr/bin/sudo -n id -u; echo ok", + "echo ok | /usr/bin/sudo -n id -u", "(/usr/bin/sudo -n id -u)", + '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'", + "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) + monkeypatch.setattr(mod, "_YOLO_MODE_FROZEN", yolo) + for command in commands: + for guard in (mod.check_dangerous_command, mod.check_all_command_guards): + result = guard(command, "local") + assert result.get("user_deny") is True, (mode, yolo, command, result) + assert result["approved"] is False + + +def test_deny_projection_preserves_data_and_path_rules(deny_config): + """Project only executable positions; retain spelling-sensitive argument data.""" + deny_config(["/usr/bin/sudo -n id -u"]) + assert approval_floors._match_user_deny_rule("env /usr/bin/sudo -n id -u; true") + assert approval_floors._match_user_deny_rule("/opt/bin/sudo -n id -u") is None + deny_config(["sudo", "sudo *", "git status"]) + for command in ( + 'echo "sudo -n id -u"', "printf 'ok && sudo -n id -u'", + 'printf "%s" "$(printf safe) sudo -n id -u"', + "command -v sudo", "command -V sudo", "command -pv sudo", + "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", + "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"): + 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"']) + assert approval_floors._match_user_deny_rule('env printf "a b"') + assert approval_floors._match_user_deny_rule('env printf "a b"') is None + + class TestDenyBeatsYolo: def test_deny_blocks_under_yolo_env(self, deny_config, clean_env, monkeypatch): deny_config(["git push --force*"]) diff --git a/tools/approval_detection.py b/tools/approval_detection.py index 9790b30a15..bcc7d6faff 100644 --- a/tools/approval_detection.py +++ b/tools/approval_detection.py @@ -499,8 +499,27 @@ _PARAM_REPLACEMENT_RE = re.compile(r"\$\{[^}/\s]+/[^}/]*/(?P[^}]*)\ _PARAM_DEFAULT_RE = re.compile(r"\$\{[^}:}\s]+:-(?P[^}]*)\}") _SIMPLE_SHELL_LITERAL_RE = re.compile(r"^[A-Za-z0-9_./:@%+=,-]+$") _ENV_ASSIGNMENT_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*=.*") -_COMMAND_WRAPPER_WORDS = {"sudo", "env", "exec", "nohup", "setsid", "time", "command", "builtin"} +_COMMAND_WRAPPER_WORDS = {"sudo", "env", "exec", "nohup", "setsid", "time", "command", "builtin", + "nice", "timeout", "stdbuf", "ionice", "chrt", "taskset", "chroot"} _SUDO_OPTIONS_WITH_ARG = {"-c", "--close-from", "-g", "--group", "-h", "--host", "-p", "--prompt", "-u", "--user"} +# Adapted from embwl0x's command-position work in #76063. Option operands are +# data, not executable positions; option spelling remains case-sensitive. +_COMMAND_WRAPPER_OPTIONS_WITH_ARG = { + "chroot": {"--groups", "--userspec"}, + "sudo": _SUDO_OPTIONS_WITH_ARG, "env": {"-C", "--chdir", "-S", "--split-string", "-u", "--unset"}, + "exec": {"-a"}, "nice": {"-n", "--adjustment"}, + "time": {"-f", "--format", "-o", "--output"}, + "timeout": {"-k", "--kill-after", "-s", "--signal"}, + "stdbuf": {"-e", "--error", "-i", "--input", "-o", "--output"}, + "ionice": {"-c", "--class", "-n", "--classdata"}, +} +_COMMAND_WRAPPER_NON_EXECUTING_OPTIONS = { + "command": {"-v", "-V"}, "chrt": {"-p", "--pid"}, + "ionice": {"-p", "--pid", "--pgid", "--uid"}, "taskset": {"-p", "--pid"}, +} +_COMMAND_WRAPPER_POSITIONAL_ARGS = {"chroot": 1, "chrt": 1, "taskset": 1, "timeout": 1} +_SHELL_COMMAND_TRANSITIONS = {"if", "then", "else", "elif", "do", "while", "until", "!"} +_SHELL_REDIRECTION_RE = re.compile(r"(?:[0-9]+)?(?:>>|<<|<>|>&|<&|>\||[<>])") _INTERPRETER_NAME_RES = tuple((family, re.compile(pattern)) for family, pattern in ( ("python", r"py(?:\.exe)?|python[23]?(?:\.\d+)*(?:\.exe)?"), ("node", r"node(?:js)?(?:\.exe)?"), @@ -889,12 +908,14 @@ def _skip_shell_whitespace(command: str, pos: int) -> int: def _scan_shell(text: str, start: int = 0, end: int | None = None, *, subst: str = "", - brace: bool = False, stop_unterminated: bool = False, naive_backtick: bool = False): + brace: bool = False, stop_unterminated: bool = False, naive_backtick: bool = False, + comments: bool = False): """Yield ``(kind, i, j, quote)`` lexical steps over ``text[start:end]`` without expanding. The single quote/escape state machine behind every detection scanner. ``kind`` is ``"char"`` (one char), ``"esc"`` (backslash + the char it escapes; never inside single quotes), ``"quote"`` (an opening/closing quote char) or ``"subst"`` (a ``$(...)`` / backtick / ``${...}`` span); + With *comments*, ``"comment"`` spans are skipped without interpreting their quote syntax. ``quote`` is the state the step was read in (``None``, ``'`` or ``"``). Substitutions are recognized unquoted when ``"u"`` is in *subst*, inside double quotes when ``"q"`` is; *brace* adds unquoted ``${...}``. An unterminated substitution falls through as plain chars unless @@ -907,7 +928,11 @@ def _scan_shell(text: str, start: int = 0, end: int | None = None, *, subst: str while i < n: ch = text[i] kind, j = "char", i + 1 - if quote != "'" and ch == "\\" and i + 1 < n: + if comments and quote is None and _is_shell_comment_start(text, i): + kind, j = "comment", text.find("\n", i, n) + if j < 0: + j = n + elif quote != "'" and ch == "\\" and i + 1 < n: kind, j = "esc", i + 2 elif ch == quote or (quote is None and ch in "'\""): kind = "quote" @@ -952,7 +977,7 @@ def _read_shell_word(command: str, pos: int) -> tuple[int, int, str]: """Read one shell word without executing expansions.""" start = end = _skip_shell_whitespace(command, pos) for kind, i, j, quote in _scan_shell(command, start, subst="u", brace=True): - if kind == "char" and quote is None and (command[i].isspace() or command[i] in ";&|"): + if kind == "char" and quote is None and (command[i].isspace() or command[i] in ";&|<>()"): break end = j return (start, end, command[start:end]) @@ -1017,12 +1042,18 @@ def _deobfuscate_shell_word_for_detection(word: str) -> str: return word +def _is_shell_comment_start(command: str, index: int) -> bool: + return command[index] == "#" and (index == 0 or command[index - 1].isspace() + or command[index - 1] in ";&|()<>") + + def _iter_shell_command_starts(command: str): starts = [0] def scan(start: int, end: int) -> None: skip = -1 - for kind, i, j, quote in _scan_shell(command, start, end, subst="uq", stop_unterminated=True): + for kind, i, j, quote in _scan_shell(command, start, end, subst="uq", stop_unterminated=True, + comments=True): if kind == "subst": # Record a nested $(...)/backtick command start and scan its body. inner = i + (1 if command[i] == "`" else 2) @@ -1037,8 +1068,16 @@ def _iter_shell_command_starts(command: str): starts.append(i + 1 + repeated) scan(0, len(command)) - # First occurrence wins (dict order), so a start is yielded once even when several openers map to it. - yield from (s for s in dict.fromkeys(_skip_shell_whitespace(command, s) for s in starts) if s < len(command)) + seen = set() + for start in starts: + start = _skip_shell_whitespace(command, start) + if start >= len(command) or start in seen or _is_shell_comment_start(command, start): + continue + seen.add(start) + yield start + _, end, word = _read_shell_word(command, start) + if word in _SHELL_COMMAND_TRANSITIONS: + starts.append(end) def _mark_command_starts(command: str) -> str: @@ -1089,27 +1128,111 @@ def _mask_quoted_newlines_span(command: str, start: int, end: int) -> str: def _iter_shell_command_word_spans(command: str): """Yield command-position words that may be executable names.""" for pos in _iter_shell_command_starts(command): - skip_wrapper_options = skip_next_wrapper_arg = False - for _ in range(12): + wrapper, positionals = None, 0 + options, skip_arg = True, False + while pos < len(command): + redirect = _SHELL_REDIRECTION_RE.match(command, _skip_shell_whitespace(command, pos)) + if redirect: + _, pos, _ = _read_shell_word(command, redirect.end()) + continue word_start, word_end, word = _read_shell_word(command, pos) if word_start == word_end: break pos = word_end deobfuscated = _deobfuscate_shell_word_for_detection(word) - lower_word = deobfuscated.lower() - if skip_next_wrapper_arg: - skip_next_wrapper_arg = False + name = os.path.basename(deobfuscated).lower() + if skip_arg: + skip_arg = False continue - if skip_wrapper_options and lower_word.startswith("-"): - skip_next_wrapper_arg = "=" not in lower_word and lower_word in _SUDO_OPTIONS_WITH_ARG + if wrapper and options and deobfuscated == "--": + options = False + continue + if wrapper and options and deobfuscated.startswith("-"): + option = deobfuscated.split("=", 1)[0] + 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"}): + break + skip_arg = "=" not in deobfuscated and option in _COMMAND_WRAPPER_OPTIONS_WITH_ARG.get(wrapper, set()) + continue + if positionals: + positionals -= 1 + continue + if _ENV_ASSIGNMENT_RE.fullmatch(word): continue yield (word_start, word_end, word) - if lower_word in _COMMAND_WRAPPER_WORDS: - skip_wrapper_options = lower_word in {"sudo", "env"} - elif _ENV_ASSIGNMENT_RE.fullmatch(deobfuscated): - skip_wrapper_options = False - else: + if name not in _COMMAND_WRAPPER_WORDS: break + wrapper, options = name, True + positionals = _COMMAND_WRAPPER_POSITIONAL_ARGS.get(name, 0) + + +def _shell_command_segment(command: str, start: int) -> str: + """Bound a candidate to its command, preserving quoted argument bytes.""" + end = len(command) + for kind, i, _, quote in _scan_shell(command, start, subst="uq", brace=True, comments=True): + if kind == "comment" or (kind == "char" and quote is None and command[i] in ";&|\n)`"): + end = i + break + return command[start:end].strip() + + +def _env_split_payload(tokens: list[str]) -> str | None: + index = 1 + while index < len(tokens): + token = tokens[index] + if token == "--" or not token.startswith("-"): + 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) + index += 2 if not equals and option in _COMMAND_WRAPPER_OPTIONS_WITH_ARG["env"] else 1 + return None + + +def _deny_command_variants(command: str): + """Add executable projections without reparsing normalized argument data. + + Whole-input matching is retained for existing globs. New projections parse + the original quote state, preserve path-specific rules, and fold only the + executable basename (never arbitrary argument paths). + """ + yield from _command_detection_variants(command) + pending, seen = [command], set() + while pending: + source = pending.pop() + if source in seen: + continue + seen.add(source) + for start, end, word in _iter_shell_command_word_spans(source): + segment = _shell_command_segment(source, start) + executable = _deobfuscate_shell_word_for_detection(word) + tail = segment[end - start:] + # Collapse only unquoted inter-word whitespace; quoted prose is data. + parts = [] + for kind, i, j, quote in _scan_shell(tail): + if kind == "char" and quote is None and tail[i].isspace(): + if not parts or parts[-1] != " ": + parts.append(" ") + else: + parts.append(tail[i:j]) + tail = "".join(parts) + for name in dict.fromkeys((executable, os.path.basename(executable))): + candidate = name + tail + yield candidate + # Apply the existing text matching semantics only AFTER locating + # executable positions; never parse its rewritten quotes again. + yield _normalize_command_for_detection(candidate) + if os.path.basename(executable) == "env": + tokens = _shell_segment_tokens(segment, 0) + if tokens: + payload = _env_split_payload(tokens) + if payload: + pending.append(payload) + for _, payload in _execution_flag_findings(source): + if payload: + pending.append(payload) def _command_detection_variants(command: str): diff --git a/tools/approval_floors.py b/tools/approval_floors.py index 17f8d4e408..d85c4c60c9 100644 --- a/tools/approval_floors.py +++ b/tools/approval_floors.py @@ -15,7 +15,7 @@ import time import uuid from tools import approval_context as _ctx from tools.approval_detection import ( - _MALFORMED_EXEC_DESCRIPTION, _PARSER_LIMIT_DESCRIPTION, _command_detection_variants) + _MALFORMED_EXEC_DESCRIPTION, _PARSER_LIMIT_DESCRIPTION, _deny_command_variants) logger = logging.getLogger("tools.approval") @@ -34,7 +34,7 @@ def _match_user_deny_rule(command: str) -> str | None: globs = [p.strip() for p in deny_patterns if isinstance(p, str) and p.strip()] if not globs: return None - for command_variant in _command_detection_variants(command): + for command_variant in _deny_command_variants(command): candidate = command_variant.lower().strip() for pattern in globs: if fnmatch.fnmatchcase(candidate, pattern.lower()): diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index c07cbbbfcc..e3f0ae9390 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -154,16 +154,19 @@ approvals: Details: -- Patterns are [fnmatch](https://docs.python.org/3/library/fnmatch.html) globs (`*`, `?`, `[...]`) matched **case-insensitively** against the whole command text. `git push --force*` matches `git push --force origin main` but not `git push origin main`. +- 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. +- 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. -- Deny rules apply to host-reaching backends (local, SSH, host-mounted Docker). Isolated container backends skip the guard stack entirely, as they always have — nothing they run can touch the host. +- User-defined deny rules apply to all terminal backends, including isolated containers, before any backend-specific approval shortcut. - A denied command returns a BLOCKED error to the agent telling it not to retry or rephrase. Nothing runs. Like the rest of the approval config, changes take effect immediately (the config cache is mtime-keyed) — no session restart needed. :::note Threat model -Deny rules are a guardrail against an honest-but-wrong agent, the same threat model as the dangerous-pattern detector. They are not a sandbox against a deliberately adversarial process — for that, use an isolated backend (Docker, Modal) or an egress-restricted environment. +Deny rules are a shell-command policy, not a complete shell interpreter or an OS capability sandbox. Normalization does not resolve arbitrary variables, aliases, functions, renamed binaries, scripts, interpreter programs, or every shell/launcher grammar (for example, case-pattern syntax). Do not use a basename deny rule as a guarantee that a capability cannot be reached by other means. For containment, use OS permissions and an isolated backend with appropriately restricted mounts, credentials, and network access. This matching behavior does not change the configured approval mode or the empty-deny-list default. ::: ### Approval Timeout