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)