From a81b519d41147cf347ad90e9e4be169dbdf9d852 Mon Sep 17 00:00:00 2001 From: rrevenanttt <290873280+rrevenanttt@users.noreply.github.com> Date: Sat, 6 Jun 2026 23:44:12 +0300 Subject: [PATCH] fix(security): close hardline rm bypass via quoted paths and ${HOME} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What does this PR do? Closes a critical hole in the hardline command floor. HARDLINE_PATTERNS is the unconditional last line of defense: detect_hardline_command runs BEFORE every yolo / approvals.mode=off / cron approve-mode bypass, so it is the only gate standing between the agent (or a prompt-injected instruction) and an irrecoverable disk wipe. The three rm rules anchored on a bare path token, and _normalize_command_for_detection never strips shell quotes — so the ordinary, recommended shell idioms slipped straight through: rm -rf "/" rm -rf '/' rm -rf "/etc" rm -rf "$HOME" rm -rf ${HOME} rm -rf "${HOME}" All of these returned NO hardline match. A leading quote pushes the path out of reach of the flag group, a trailing quote breaks the `(\s|$)` terminator, and the `${HOME}` brace form was never listed at all. Under --yolo, approvals.mode=off, or cron approve-mode the dangerous-command layer is also skipped, so these commands reached execution with zero gate — exactly the unrecoverable data loss the floor is documented to make impossible. Because quoting paths and `${HOME}` are normal shell usage, not exotic obfuscation, this is a high-severity, easily-triggered bypass. The fix makes the rm path matcher quote- and brace-tolerant while staying conservative: a path is matched when it is either fully wrapped in its own matching quote pair (`"/"`) or bare with a whitespace/end terminator. The matching-quote requirement is deliberate so the change adds no new false positives — a dangerous-looking string that is merely an argument to another command (e.g. `git commit -m "rm -rf /"`) has a closing quote but no opening quote of its own around the path, so neither branch fires. ## Related Issue N/A ## Type of Change - [x] 🔒 Security fix ## Changes Made - `tools/approval.py`: added `_hardline_rm_path()` (matches a destructive path either fully quoted or bare-with-terminator), factored the protected system-dir list into `_HARDLINE_SYSTEM_DIRS` and the rm flag prefix into `_RM_FLAG_PREFIX`, and rebuilt the three rm `HARDLINE_PATTERNS` on top of them, adding the `${HOME}` brace form. Kept as plain concatenation so regex backslashes never land inside an f-string field (Python 3.11 floor). - `tests/tools/test_hardline_blocklist.py`: added quoted (`"/"`, `'/'`, `"/etc"`, `"$HOME"`, ...) and brace (`${HOME}`, `"${HOME}"`) cases to the must-block set, a dedicated `_QUOTED_BRACE_BYPASS` regression parametrization, no-false-positive guards (`git commit -m "rm -rf /"`), and extended the yolo-cannot-bypass integration test to cover the quoted/brace forms. ## How to Test 1. Reproduce the bypass on `main`: `detect_hardline_command('rm -rf "/"')` returns `(False, None)` — the floor lets it through. 2. With this change it returns `(True, "recursive delete of root filesystem")`; the same holds for `'/'`, `"/etc"`, `"$HOME"`, `${HOME}`, `"${HOME}"`. 3. Run the suite: `scripts/run_tests.sh tests/tools/test_hardline_blocklist.py` — 125 passed, including the new bypass and no-false-positive cases. ## Checklist ### Code - [x] I've read the Contributing Guide - [x] My commit messages follow Conventional Commits (`fix(scope):`, etc.) - [x] I searched for existing PRs to make sure this isn't a duplicate - [x] My PR contains **only** changes related to this fix (no unrelated commits) - [x] I've run the relevant tests and they pass - [x] I've added tests for my changes (required for bug fixes) - [x] I've tested on my platform: macOS 15 (Darwin 25.5) ### Documentation & Housekeeping - [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A - [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A - [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A - [x] I've considered cross-platform impact (Windows, macOS) — pattern-only change, ruff + footgun gate pass - [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A --- tests/tools/test_hardline_blocklist.py | 53 +++++++++++++++++++++++++- tools/approval.py | 41 ++++++++++++++++++-- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_hardline_blocklist.py b/tests/tools/test_hardline_blocklist.py index 8d8062139b8..960b4e7c20c 100644 --- a/tests/tools/test_hardline_blocklist.py +++ b/tests/tools/test_hardline_blocklist.py @@ -45,6 +45,27 @@ _HARDLINE_BLOCK = [ "rm -rf ~/", "rm -rf ~/*", "rm -rf $HOME", + # Quoted path idioms — the recommended shell form for paths with special + # chars. These previously slipped past the floor because the surrounding + # quote broke both the flag group and the (\s|$) terminator (regression + # guard: catastrophic disk/home wipe under --yolo / approvals.mode=off). + 'rm -rf "/"', + "rm -rf '/'", + 'rm -rf "/*"', + 'rm -rf "/etc"', + "rm -rf '/etc'", + 'rm -rf "/home"', + 'rm -rf "/usr"', + 'rm -rf "$HOME"', + "rm -rf '$HOME'", + 'rm -rf "$HOME/"', + 'rm -rf "~"', + 'sudo rm -rf "/"', + 'rm -rf "/" && echo done', + # ${HOME} brace form (universally common, previously unmatched). + "rm -rf ${HOME}", + 'rm -rf "${HOME}"', + "rm -fr ${HOME}", # Filesystem format "mkfs.ext4 /dev/sda1", "mkfs /dev/sdb", @@ -100,6 +121,12 @@ _HARDLINE_ALLOW = [ "rm -rf $HOME/tmp", "rm foo.txt", "rm -rf some/path", + # A dangerous-looking command embedded as a quoted *argument* to another + # command must not trip the floor: the path is immediately followed by a + # closing quote with no matching opening quote of its own, so the + # quote-tolerant matcher must still ignore it (no new false positives). + 'git commit -m "rm -rf /"', + 'git commit -m "wipe with rm -rf /etc"', # dd to regular files "dd if=/dev/zero of=./image.bin", "dd if=./data of=./backup.bin", @@ -150,6 +177,29 @@ def test_hardline_detection_allows(command): assert desc is None +# Commands written with the ordinary quoting / brace shell idioms that +# previously slipped past the floor. Kept as an explicit regression set so +# the intent (quoting `rm -rf "/"` must not be a disk-wipe bypass) survives +# any future refactor of the rm patterns. +_QUOTED_BRACE_BYPASS = [ + 'rm -rf "/"', + "rm -rf '/'", + 'rm -rf "/etc"', + 'rm -rf "/home"', + 'rm -rf "$HOME"', + "rm -rf ${HOME}", + 'rm -rf "${HOME}"', +] + + +@pytest.mark.parametrize("command", _QUOTED_BRACE_BYPASS) +def test_quoted_and_brace_paths_are_hardline_blocked(command): + """Quoted paths and ${HOME} must hit the floor (was a silent bypass).""" + is_hl, desc = detect_hardline_command(command) + assert is_hl, f"quoting/brace bypass leaked through hardline floor: {command!r}" + assert desc + + # ------------------------------------------------------------------------- # Integration with the approval flow # ------------------------------------------------------------------------- @@ -189,7 +239,8 @@ def test_yolo_env_var_cannot_bypass_hardline(clean_session, monkeypatch): """HERMES_YOLO_MODE=1 must not bypass the hardline floor.""" monkeypatch.setenv("HERMES_YOLO_MODE", "1") - for cmd in ["rm -rf /", "shutdown -h now", "mkfs.ext4 /dev/sda", "reboot"]: + for cmd in ['rm -rf /', 'rm -rf "/"', 'rm -rf "$HOME"', "rm -rf ${HOME}", + "shutdown -h now", "mkfs.ext4 /dev/sda", "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 diff --git a/tools/approval.py b/tools/approval.py index e5cb744420c..bc1a607a32b 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -301,11 +301,44 @@ _CMDPOS = ( r'\s*' ) +# Destructive-path argument matcher for the rm hardline rules. +# +# The path token in `rm -rf /` is almost always written quoted in real +# shells — `rm -rf "/"`, `rm -rf "$HOME"` — and `${HOME}` is the universal +# brace form. A bare-token anchor (`(/...)(\s|$)`) silently misses all of +# these: the surrounding quote breaks both the leading position (the flag +# group can't consume `"`) and the trailing `(\s|$)` terminator, letting +# `rm -rf "/"` slip past the unconditional floor entirely. +# +# Accept the path either fully wrapped in a matching quote pair OR bare with +# a whitespace/end terminator. The matching-quote requirement is deliberate: +# it catches `rm -rf "/"` (path quoted on its own) while NOT firing on a +# dangerous-looking string that is merely an argument to another command — +# e.g. `git commit -m "rm -rf /"` — where the closing quote follows the path +# but no opening quote precedes it, so neither branch applies. +def _hardline_rm_path(path_alt: str, tail: str = r'(?:\s|$)') -> str: + return rf'(?:["\'](?:{path_alt})["\']|(?:{path_alt}){tail})' + + +# Protected system roots whose recursive deletion has no recovery path. +_HARDLINE_SYSTEM_DIRS = ( + r'/home|/home/\*|/root|/root/\*|/etc|/etc/\*|/usr|/usr/\*|' + r'/var|/var/\*|/bin|/bin/\*|/sbin|/sbin/\*|/boot|/boot/\*|/lib|/lib/\*' +) + +# `rm` plus its flag group, shared by the three rm hardline rules. Kept as a +# plain concatenation (not an f-string) so the regex backslashes never live +# inside an f-string replacement field — unsupported on the Python 3.11 floor. +_RM_FLAG_PREFIX = r'\brm\s+(-[^\s]*\s+)*' + HARDLINE_PATTERNS = [ - # rm recursive targeting the root filesystem or protected roots - (r'\brm\s+(-[^\s]*\s+)*(/|/\*|/ \*)(\s|$)', "recursive delete of root filesystem"), - (r'\brm\s+(-[^\s]*\s+)*(/home|/home/\*|/root|/root/\*|/etc|/etc/\*|/usr|/usr/\*|/var|/var/\*|/bin|/bin/\*|/sbin|/sbin/\*|/boot|/boot/\*|/lib|/lib/\*)(\s|$)', "recursive delete of system directory"), - (r'\brm\s+(-[^\s]*\s+)*(~|\$HOME)(/?|/\*)?(\s|$)', "recursive delete of home directory"), + # rm recursive targeting the root filesystem or protected roots. + # `${HOME}` brace form and quoted paths (`rm -rf "/"`, `rm -rf "$HOME"`) + # are handled via _hardline_rm_path so the floor cannot be bypassed with + # the ordinary quoting/brace shell idioms. + (_RM_FLAG_PREFIX + _hardline_rm_path(r'/|/\*|/ \*'), "recursive delete of root filesystem"), + (_RM_FLAG_PREFIX + _hardline_rm_path(_HARDLINE_SYSTEM_DIRS), "recursive delete of system directory"), + (_RM_FLAG_PREFIX + _hardline_rm_path(r'(?:~|\$\{?HOME\}?)(?:/?|/\*)?'), "recursive delete of home directory"), # Filesystem format (r'\bmkfs(\.[a-z0-9]+)?\b', "format filesystem (mkfs)"), # Raw block device overwrites (dd + redirection)