From 2c7c30be69d09b37af5c4317bea74a835cbcbee0 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 17 Mar 2026 02:22:12 -0700 Subject: [PATCH] fix(security): harden terminal safety and sandbox file writes (#1653) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): harden terminal safety and sandbox file writes Two security improvements: 1. Dangerous command detection: expand shell -c pattern to catch combined flags (bash -lc, bash -ic, ksh -c) that were previously undetected. Pattern changed from matching only 'bash -c' to matching any shell invocation with -c anywhere in the flags. 2. File write sandboxing: add HERMES_WRITE_SAFE_ROOT env var that constrains all write_file/patch operations to a configured directory tree. Opt-in — when unset, behavior is unchanged. Useful for gateway/messaging deployments that should only touch a workspace. Based on PR #1085 by ismoilh. * fix: correct "POSIDEON" typo to "POSEIDON" in banner ASCII art The poseidon skin's banner_logo had the E and I letters swapped, spelling "POSIDEON-AGENT" instead of "POSEIDON-AGENT". --------- Co-authored-by: ismoilh Co-authored-by: unmodeled-tyler --- hermes_cli/skin_engine.py | 12 ++-- tests/tools/test_approval.py | 19 ++++++ tests/tools/test_file_write_safety.py | 83 +++++++++++++++++++++++++++ tests/tools/test_yolo_mode.py | 1 + tools/approval.py | 3 +- tools/file_operations.py | 26 +++++++++ 6 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 tests/tools/test_file_write_safety.py diff --git a/hermes_cli/skin_engine.py b/hermes_cli/skin_engine.py index dad666baf..980ed8b1f 100644 --- a/hermes_cli/skin_engine.py +++ b/hermes_cli/skin_engine.py @@ -351,12 +351,12 @@ _BUILTIN_SKINS: Dict[str, Dict[str, Any]] = { "help_header": "(Ψ) Available Commands", }, "tool_prefix": "│", - "banner_logo": """[bold #B8E8FF]██████╗ ██████╗ ███████╗██╗██████╗ ███████╗ ██████╗ ███╗ ██╗ █████╗ ██████╗ ███████╗███╗ ██╗████████╗[/] -[bold #97D6FF]██╔══██╗██╔═══██╗██╔════╝██║██╔══██╗██╔════╝██╔═══██╗████╗ ██║ ██╔══██╗██╔════╝ ██╔════╝████╗ ██║╚══██╔══╝[/] -[#75C1F6]██████╔╝██║ ██║███████╗██║██║ ██║█████╗ ██║ ██║██╔██╗ ██║█████╗███████║██║ ███╗█████╗ ██╔██╗ ██║ ██║[/] -[#4FA2E0]██╔═══╝ ██║ ██║╚════██║██║██║ ██║██╔══╝ ██║ ██║██║╚██╗██║╚════╝██╔══██║██║ ██║██╔══╝ ██║╚██╗██║ ██║[/] -[#2E7CC7]██║ ╚██████╔╝███████║██║██████╔╝███████╗╚██████╔╝██║ ╚████║ ██║ ██║╚██████╔╝███████╗██║ ╚████║ ██║[/] -[#1B4F95]╚═╝ ╚═════╝ ╚══════╝╚═╝╚═════╝ ╚══════╝ ╚═════╝ ╚═╝ ╚═══╝ ╚═╝ ╚═╝ ╚═════╝ ╚══════╝╚═╝ ╚═══╝ ╚═╝[/]""", + "banner_logo": """[bold #B8E8FF]██████╗ ██████╗ ███████╗███████╗██╗██████╗ ██████╗ ███╗ ██╗ █████╗ ██████╗ ███████╗███╗ ██╗████████╗[/] +[bold #97D6FF]██╔══██╗██╔═══██╗██╔════╝██╔════╝██║██╔══██╗██╔═══██╗████╗ ██║ ██╔══██╗██╔════╝ ██╔════╝████╗ ██║╚══██╔══╝[/] +[#75C1F6]██████╔╝██║ ██║███████╗█████╗ ██║██║ ██║██║ ██║██╔██╗ ██║█████╗███████║██║ ███╗█████╗ ██╔██╗ ██║ ██║[/] +[#4FA2E0]██╔═══╝ ██║ ██║╚════██║██╔══╝ ██║██║ ██║██║ ██║██║╚██╗██║╚════╝██╔══██║██║ ██║██╔══╝ ██║╚██╗██║ ██║[/] +[#2E7CC7]██║ ╚██████╔╝███████║███████╗██║██████╔╝╚██████╔╝██║ ╚████║ ██║ ██║╚██████╔╝███████╗██║ ╚████║ ██║[/] +[#1B4F95]╚═╝ ╚═════╝ ╚══════╝╚══════╝╚═╝╚═════╝ ╚═════╝ ╚═╝ ╚═══╝ ╚═╝ ╚═╝ ╚═════╝ ╚══════╝╚═╝ ╚═══╝ ╚═╝[/]""", "banner_hero": """[#2A6FB9]⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣀⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀[/] [#5DB8F5]⠀⠀⠀⠀⠀⠀⠀⠀⠀⣠⣾⣿⣷⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀[/] [#5DB8F5]⠀⠀⠀⠀⠀⠀⠀⢠⣿⠏⠀Ψ⠀⠹⣿⡄⠀⠀⠀⠀⠀⠀⠀[/] diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 5afe31975..4f61dd2ae 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -43,6 +43,25 @@ class TestDetectDangerousSudo: assert key is not None assert "pipe" in desc.lower() or "shell" in desc.lower() + def test_shell_via_lc_flag(self): + """bash -lc should be treated as dangerous just like bash -c.""" + is_dangerous, key, desc = detect_dangerous_command("bash -lc 'echo pwned'") + assert is_dangerous is True + assert key is not None + + def test_shell_via_lc_with_newline(self): + """Multi-line bash -lc invocations must still be detected.""" + cmd = "bash -lc \\\n'echo pwned'" + is_dangerous, key, desc = detect_dangerous_command(cmd) + assert is_dangerous is True + assert key is not None + + def test_ksh_via_c_flag(self): + """ksh -c should be caught by the expanded pattern.""" + is_dangerous, key, desc = detect_dangerous_command("ksh -c 'echo test'") + assert is_dangerous is True + assert key is not None + class TestDetectSqlPatterns: def test_drop_table(self): diff --git a/tests/tools/test_file_write_safety.py b/tests/tools/test_file_write_safety.py new file mode 100644 index 000000000..12bc1ccac --- /dev/null +++ b/tests/tools/test_file_write_safety.py @@ -0,0 +1,83 @@ +"""Tests for file write safety and HERMES_WRITE_SAFE_ROOT sandboxing. + +Based on PR #1085 by ismoilh (salvaged). +""" + +import os +from pathlib import Path + +import pytest + +from tools.file_operations import _is_write_denied + + +class TestStaticDenyList: + """Basic sanity checks for the static write deny list.""" + + def test_temp_file_not_denied_by_default(self, tmp_path: Path): + target = tmp_path / "regular.txt" + assert _is_write_denied(str(target)) is False + + def test_ssh_key_is_denied(self): + assert _is_write_denied(os.path.expanduser("~/.ssh/id_rsa")) is True + + def test_etc_shadow_is_denied(self): + assert _is_write_denied("/etc/shadow") is True + + +class TestSafeWriteRoot: + """HERMES_WRITE_SAFE_ROOT should sandbox writes to a specific subtree.""" + + def test_writes_inside_safe_root_are_allowed(self, tmp_path: Path, monkeypatch): + safe_root = tmp_path / "workspace" + child = safe_root / "subdir" / "file.txt" + os.makedirs(child.parent, exist_ok=True) + + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", str(safe_root)) + assert _is_write_denied(str(child)) is False + + def test_writes_to_safe_root_itself_are_allowed(self, tmp_path: Path, monkeypatch): + safe_root = tmp_path / "workspace" + os.makedirs(safe_root, exist_ok=True) + + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", str(safe_root)) + assert _is_write_denied(str(safe_root)) is False + + def test_writes_outside_safe_root_are_denied(self, tmp_path: Path, monkeypatch): + safe_root = tmp_path / "workspace" + outside = tmp_path / "other" / "file.txt" + os.makedirs(safe_root, exist_ok=True) + os.makedirs(outside.parent, exist_ok=True) + + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", str(safe_root)) + assert _is_write_denied(str(outside)) is True + + def test_safe_root_env_ignores_empty_value(self, tmp_path: Path, monkeypatch): + target = tmp_path / "regular.txt" + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", "") + assert _is_write_denied(str(target)) is False + + def test_safe_root_unset_allows_all(self, tmp_path: Path, monkeypatch): + target = tmp_path / "regular.txt" + monkeypatch.delenv("HERMES_WRITE_SAFE_ROOT", raising=False) + assert _is_write_denied(str(target)) is False + + def test_safe_root_with_tilde_expansion(self, tmp_path: Path, monkeypatch): + """~ in HERMES_WRITE_SAFE_ROOT should be expanded.""" + # Use a real subdirectory of tmp_path so we can test tilde-style paths + safe_root = tmp_path / "workspace" + inside = safe_root / "file.txt" + os.makedirs(safe_root, exist_ok=True) + + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", str(safe_root)) + assert _is_write_denied(str(inside)) is False + + def test_safe_root_does_not_override_static_deny(self, tmp_path: Path, monkeypatch): + """Even if a static-denied path is inside the safe root, it's still denied.""" + # Point safe root at home to include ~/.ssh + monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", os.path.expanduser("~")) + assert _is_write_denied(os.path.expanduser("~/.ssh/id_rsa")) is True + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/tools/test_yolo_mode.py b/tests/tools/test_yolo_mode.py index 91c751e7a..7d30adcc6 100644 --- a/tests/tools/test_yolo_mode.py +++ b/tests/tools/test_yolo_mode.py @@ -63,6 +63,7 @@ class TestYoloMode: dangerous_commands = [ "rm -rf /", "chmod 777 /etc/passwd", + "bash -lc 'echo pwned'", "mkfs.ext4 /dev/sda1", "dd if=/dev/zero of=/dev/sda", "DROP TABLE users", diff --git a/tools/approval.py b/tools/approval.py index 60c6e5fe5..44029a8cc 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -40,7 +40,8 @@ DANGEROUS_PATTERNS = [ (r'\bkill\s+-9\s+-1\b', "kill all processes"), (r'\bpkill\s+-9\b', "force kill processes"), (r':\(\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;\s*:', "fork bomb"), - (r'\b(bash|sh|zsh)\s+-c\s+', "shell command via -c flag"), + # Any shell invocation via -c or combined flags like -lc, -ic, etc. + (r'\b(bash|sh|zsh|ksh)\s+-[^\s]*c(\s+|$)', "shell command via -c/-lc flag"), (r'\b(python[23]?|perl|ruby|node)\s+-[ec]\s+', "script execution via -e/-c flag"), (r'\b(curl|wget)\b.*\|\s*(ba)?sh\b', "pipe remote content to shell"), (r'\b(bash|sh|zsh|ksh)\s+<\s* Optional[str]: + """Return the resolved HERMES_WRITE_SAFE_ROOT path, or None if unset. + + When set, all write_file/patch operations are constrained to this + directory tree. Writes outside it are denied even if the target is + not on the static deny list. Opt-in hardening for gateway/messaging + deployments that should only touch a workspace checkout. + """ + root = os.getenv("HERMES_WRITE_SAFE_ROOT", "") + if not root: + return None + try: + return os.path.realpath(os.path.expanduser(root)) + except Exception: + return None + + def _is_write_denied(path: str) -> bool: """Return True if path is on the write deny list.""" resolved = os.path.realpath(os.path.expanduser(path)) + + # 1) Static deny list if resolved in WRITE_DENIED_PATHS: return True for prefix in WRITE_DENIED_PREFIXES: if resolved.startswith(prefix): return True + + # 2) Optional safe-root sandbox + safe_root = _get_safe_write_root() + if safe_root: + if not (resolved == safe_root or resolved.startswith(safe_root + os.sep)): + return True + return False