fix(security): close shell line-continuation bypass in command detection
`_normalize_command_for_detection` strips backslash-escapes before matching
DANGEROUS_PATTERNS and HARDLINE_PATTERNS, but the strip rule was
`re.sub(r'\\([^\n])', r'\1', ...)` — its `[^\n]` class deliberately skips
newlines. A backslash immediately followed by a newline is a POSIX line
continuation: the shell removes BOTH characters and joins the tokens, so
`rm -rf \<newline>/` executes as `rm -rf /`. With the dangling backslash left
in place, the structured rm/dd/mkfs patterns no longer match because a literal
`\` sits wedged between the tokens they expect to be adjacent.
The worst consequence is on the HARDLINE floor. The dangerous-command layer
still fired here only by accident (the generic `\brm\s+-[^\s]*r` "recursive
delete" rule needs no path), and that layer is bypassed by `--yolo` /
`approvals.mode=off`. The hardline blocklist — the unconditional floor reserved
for catastrophic, unrecoverable commands and meant to hold even under yolo —
anchors the root path directly after the flags, so `rm -rf \<newline>/`,
`rm -r\<newline>f /`, and `rm -rf \<newline>~` all slipped past it entirely.
A yolo session could therefore wipe the root filesystem.
The fix collapses line continuations (`\` + `\n` or `\r\n`) to nothing,
mirroring the shell, before the existing escape strip runs. This was the gap
left by 621bf3a87, which added the escape strip but only for non-newline chars.
## What does this PR do?
Closes a shell line-continuation bypass in the dangerous-command detector.
Before: `rm -rf \<newline>/` normalized to `rm -rf \<newline>/`, so the
hardline root-delete patterns did not match and the command could run under
`--yolo`. After: line continuations are collapsed first, the command
normalizes to `rm -rf /`, and the hardline floor blocks it unconditionally.
## Related Issue
N/A
## Type of Change
- [x] 🔒 Security fix
## Changes Made
- `tools/approval.py`: in `_normalize_command_for_detection`, add
`command = re.sub(r'\\\r?\n', '', command)` ahead of the existing
backslash-escape strip so shell line continuations (`\`+newline, LF or CRLF)
are removed exactly as the shell would, instead of leaving a stray backslash
that breaks the structured patterns.
- `tests/tools/test_hardline_blocklist.py`: add a parametrized
`test_hardline_blocks_line_continuation` covering the root, in-flag, home,
CRLF, and mkfs continuation forms, plus
`test_line_continuation_root_wipe_cannot_bypass_hardline` asserting the
continuation root wipe stays blocked even with `HERMES_YOLO_MODE=1`.
## How to Test
1. Reproduce: stash the `tools/approval.py` change and run
`scripts/run_tests.sh tests/tools/test_hardline_blocklist.py` — the new
line-continuation cases fail (`rm -rf \<newline>/` is not flagged hardline,
and leaks past the floor under yolo).
2. Restore the change and rerun the file — all 106 tests pass.
3. Regression: `scripts/run_tests.sh tests/tools/test_approval.py` (the
existing fullwidth/ANSI/null-byte normalization and multiline cases still
pass).
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits (`fix(scope):`, `feat(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/feature (no unrelated commits)
- [x] I've run `pytest tests/ -q` and all tests pass
- [x] I've added tests for my changes (required for bug fixes, strongly encouraged for features)
- [x] I've tested on my platform: macOS 15 (Darwin 25.5.0)
### 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) — handles both LF and CRLF line endings
- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
# Conflicts:
# tools/approval.py
This commit is contained in:
parent
e00800fc89
commit
17f07aebdc
2 changed files with 56 additions and 0 deletions
|
|
@ -200,6 +200,37 @@ def test_quoted_and_brace_paths_are_hardline_blocked(command):
|
|||
assert desc
|
||||
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
# Shell line-continuation bypass
|
||||
# -------------------------------------------------------------------------
|
||||
#
|
||||
# A backslash immediately followed by a newline is a POSIX line
|
||||
# continuation: the shell removes BOTH characters and joins the tokens, so
|
||||
# `rm -rf \<newline>/` executes as `rm -rf /`. The normalizer used to strip
|
||||
# only backslash-escapes of NON-newline characters (`\\([^\n])`), leaving the
|
||||
# dangling backslash wedged between tokens — which broke the structured
|
||||
# rm/dd/mkfs patterns and let a root wipe slip past the hardline floor.
|
||||
|
||||
# (command_with_continuation, description_substring) — each is the
|
||||
# line-continuation form of a command already in _HARDLINE_BLOCK.
|
||||
_HARDLINE_LINE_CONTINUATION = [
|
||||
("rm -rf \\\n/", "root"), # split before the path
|
||||
("rm -r\\\nf /", "root"), # split inside the flag bundle
|
||||
("rm -rf \\\n~", "home"), # home-directory wipe
|
||||
("rm -rf \\\r\n/", "root"), # CRLF line ending
|
||||
("mkfs.ext4 \\\n/dev/sda1", "mkfs"), # filesystem format
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("command,desc_substr", _HARDLINE_LINE_CONTINUATION)
|
||||
def test_hardline_blocks_line_continuation(command, desc_substr):
|
||||
is_hl, desc = detect_hardline_command(command)
|
||||
assert is_hl, f"line-continuation bypassed hardline detection: {command!r}"
|
||||
assert desc and desc_substr in desc.lower(), (
|
||||
f"unexpected description {desc!r} for {command!r}"
|
||||
)
|
||||
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
# Integration with the approval flow
|
||||
# -------------------------------------------------------------------------
|
||||
|
|
@ -250,6 +281,21 @@ def test_yolo_env_var_cannot_bypass_hardline(clean_session, monkeypatch):
|
|||
assert r2.get("hardline") is True
|
||||
|
||||
|
||||
def test_line_continuation_root_wipe_cannot_bypass_hardline(clean_session, monkeypatch):
|
||||
"""A line-continuation root wipe must stay blocked even under yolo.
|
||||
|
||||
`rm -rf \\<newline>/` runs as `rm -rf /`. Yolo bypasses the regular
|
||||
dangerous-command layer, so the hardline floor is the only thing left to
|
||||
catch it — it must hold.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_YOLO_MODE", "1")
|
||||
|
||||
result = check_all_command_guards("rm -rf \\\n/", "local")
|
||||
assert result["approved"] is False, "yolo leaked a line-continuation root wipe"
|
||||
assert result.get("hardline") is True
|
||||
assert "BLOCKED (hardline)" in result["message"]
|
||||
|
||||
|
||||
def test_session_yolo_cannot_bypass_hardline(clean_session):
|
||||
"""Gateway /yolo (session-scoped) must not bypass the hardline floor."""
|
||||
enable_session_yolo("hardline_test")
|
||||
|
|
|
|||
|
|
@ -681,6 +681,16 @@ def _normalize_command_for_detection(command: str) -> str:
|
|||
command = command.replace('\x00', '')
|
||||
# Normalize Unicode (fullwidth Latin, halfwidth Katakana, etc.)
|
||||
command = unicodedata.normalize('NFKC', command)
|
||||
# Collapse shell line continuations (backslash-newline). The shell removes
|
||||
# BOTH characters and joins the tokens, so `rm -rf \<newline>/` executes as
|
||||
# `rm -rf /`. This must run BEFORE the generic backslash-escape strip below,
|
||||
# whose [^\n] class deliberately skips newlines and would otherwise leave
|
||||
# the dangling backslash wedged between tokens — defeating the structured
|
||||
# rm/mkfs/dd patterns (notably the HARDLINE root-delete floor, which cannot
|
||||
# be bypassed even with yolo). Handles both \n and \r\n line endings. Line
|
||||
# continuations carry no path separator, so this is a no-op on the Windows
|
||||
# home-prefix folds below (which match C:\Users\alice\... — no newline).
|
||||
command = re.sub(r'\\\r?\n', '', command)
|
||||
# Fold absolute home / active-profile-home prefixes into their canonical
|
||||
# ~/ and ~/.hermes/ forms so static user-sensitive patterns catch
|
||||
# /home/alice/.bashrc and C:\Users\alice\.bashrc the same way they catch
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue