From 2cd60d92a8760605f9b02452938ea5d0d7bbacbf Mon Sep 17 00:00:00 2001 From: Omar Baradei Date: Wed, 10 Jun 2026 17:11:54 -0700 Subject: [PATCH] fix(approval): recognize wrapper-prefixed kill spellings in self-host guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on upstream PR #43157: `command kill ` and `builtin kill ` execute kill but bypassed _KILL_CMD_RE, which only matched kill as the first word of a command segment. Anchor the kill guard at command position via _CMDPOS and add `command [-p]` / builtin to the shared wrapper inventory, so wrapper chains (sudo, env, exec, nohup, setsid, time, command, builtin) are consumed before the anchor. `command -v/-V` stays unmatched — it resolves a name without executing. tests/tools/test_hardline_blocklist.py: 5 new cases covering the reported bypass probes end-to-end, wrapper chains, foreign-PID allow, and command -v precision. 115 passed; approval cluster 370 passed. Co-Authored-By: Claude Fable 5 --- tests/tools/test_hardline_blocklist.py | 37 ++++++++++++++++++++++++++ tools/approval.py | 18 ++++++++----- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 3ba0084fb926..a93db4fc8420 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -447,3 +447,40 @@ def test_container_bypasses_self_host_kill(clean_session): # container namespace are not the host gateway's. result = check_all_command_guards(f"kill {_os.getpid()}", "docker") assert result["approved"] is True + + +def test_self_host_kill_blocks_command_and_builtin_wrappers(clean_session): + # `command kill` and `builtin kill` are plain shell spellings that + # still execute kill; they must not slip past the command-position + # anchor. + pid = _os.getpid() + assert _check_self_host_kill(f"command kill {pid}")[0] is True + assert _check_self_host_kill(f"builtin kill {pid}")[0] is True + assert _check_self_host_kill("command -p kill $$")[0] is True + assert _check_self_host_kill(f"command builtin kill {pid}")[0] is True + + +def test_self_host_kill_blocks_standard_wrapper_prefixes(clean_session): + pid = _os.getpid() + assert _check_self_host_kill(f"sudo kill {pid}")[0] is True + assert _check_self_host_kill(f"env kill {pid}")[0] is True + assert _check_self_host_kill("exec kill $PPID")[0] is True + assert _check_self_host_kill(f"nohup setsid kill -9 {pid}")[0] is True + + +def test_self_host_kill_allows_foreign_pid_via_wrappers(clean_session): + assert _check_self_host_kill("command kill 999999999")[0] is False + assert _check_self_host_kill("builtin kill -9 999999998")[0] is False + + +def test_self_host_kill_allows_command_v_lookup(clean_session): + # `command -v kill` resolves the name without executing kill, so it + # is not a wrapper; only `command [-p]` executes its operand. + assert _check_self_host_kill(f"command -v kill {_os.getpid()}")[0] is False + + +def test_check_all_command_guards_blocks_wrapped_self_host_kill(clean_session): + for cmd in (f"command kill {_os.getpid()}", f"builtin kill {_os.getpid()}"): + result = check_all_command_guards(cmd, "local") + assert result["approved"] is False, cmd + assert result.get("hardline") is True, cmd diff --git a/tools/approval.py b/tools/approval.py index 61777fec41db..d39ad241050b 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -236,13 +236,15 @@ def _is_gateway_approval_context() -> bool: # patterns so they don't fire on "echo reboot" or "grep 'shutdown' log". # Matches: start of string, after command separators (; && || | newline), # after subshell openers ( `$(` or backtick ), optionally consuming -# leading wrapper commands (sudo, env VAR=VAL, exec, nohup, setsid). +# leading wrapper commands (sudo, env VAR=VAL, exec, nohup, setsid, +# `command [-p]`, builtin). Only `command -p` is treated as a wrapper: +# `command -v`/`-V` resolve a name without executing it. _CMDPOS = ( r'(?:^|[;&|\n`]|\$\()' # start position r'\s*' # optional whitespace r'(?:sudo\s+(?:-[^\s]+\s+)*)?' # optional sudo with flags r'(?:env\s+(?:\w+=\S*\s+)*)?' # optional env with VAR=VAL pairs - r'(?:(?:exec|nohup|setsid|time)\s+)*' # optional wrapper commands + r'(?:(?:exec|nohup|setsid|time|builtin|command(?:\s+-p)?)\s+)*' # optional wrapper commands r'\s*' ) @@ -343,13 +345,17 @@ def detect_hardline_command(command: str) -> tuple: # (desktop app, launchd, `hermes gateway restart`), not the hosted agent. # # Static patterns can't express "our PID", so this is a function guard -# like the sudo-stdin one: it extracts kill/pkill numeric targets and the +# like the sudo-stdin one: it extracts numeric ``kill`` targets and the # shell self-tokens ``$$`` / ``$PPID`` and compares against this process -# and its parent. Process-group kills (negative PIDs) are intentionally -# out of scope here — ``kill -1`` is already hardline-blocked above. +# and its parent. ``kill`` is anchored at command position via _CMDPOS, +# so wrapper spellings that still execute it (``command kill``, +# ``builtin kill``, ``sudo``/``env``/``exec``/``nohup``/``setsid``/ +# ``time`` prefixes, and chains of those) are recognized too. +# Process-group kills (negative PIDs) are intentionally out of scope +# here — ``kill -1`` is already hardline-blocked above. _KILL_CMD_RE = re.compile( - r'(?:^|[;&|`\n]|&&|\|\||\$\()\s*kill\s+(?P[^;&|`\n]*)', + _CMDPOS + r'kill\s+(?P[^;&|`\n]*)', re.IGNORECASE) _KILL_SELF_TOKEN_RE = re.compile(r'\$\$|\$\{?PPID\}?\b')