mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-10 13:31:38 +00:00
fix(security): close hardline rm bypass via quoted paths and ${HOME}
## 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
This commit is contained in:
rrevenanttt
Teknium
parent
32bc36522e
commit
a81b519d41
2 changed files with 89 additions and 5 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue