From 560245879417625e41e416300b8a78d5d727c6f1 Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Sun, 29 Mar 2026 22:33:47 -0700
Subject: [PATCH] security: harden dangerous command detection and add file
 tool path guards (#3872)

Closes gaps that allowed an agent to expose Docker's Remote API to the
internet by writing to /etc/docker/daemon.json.

Terminal tool (approval.py):
- chmod: now catches 666 and symbolic modes (o+w, a+w), not just 777
- cp/mv/install: detected when targeting /etc/
- sed -i/--in-place: detected when targeting /etc/

File tools (file_tools.py):
- write_file and patch now refuse to write to sensitive system paths
  (/etc/, /boot/, /usr/lib/systemd/, docker.sock)
- Directs users to the terminal tool (which has approval prompts) for
  system file modifications
---
 tools/approval.py   |  8 ++++++--
 tools/file_tools.py | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/tools/approval.py b/tools/approval.py
index 8ae52407..e7313b00 100644
--- a/tools/approval.py
+++ b/tools/approval.py
@@ -41,8 +41,8 @@ DANGEROUS_PATTERNS = [
     (r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"),
     (r'\brm\s+-[^\s]*r', "recursive delete"),
     (r'\brm\s+--recursive\b', "recursive delete (long flag)"),
-    (r'\bchmod\s+(-[^\s]*\s+)*777\b', "world-writable permissions"),
-    (r'\bchmod\s+--recursive\b.*777', "recursive world-writable (long flag)"),
+    (r'\bchmod\s+(-[^\s]*\s+)*(777|666|o\+[rwx]*w|a\+[rwx]*w)\b', "world/other-writable permissions"),
+    (r'\bchmod\s+--recursive\b.*(777|666|o\+[rwx]*w|a\+[rwx]*w)', "recursive world/other-writable (long flag)"),
     (r'\bchown\s+(-[^\s]*)?R\s+root', "recursive chown to root"),
     (r'\bchown\s+--recursive\b.*root', "recursive chown to root (long flag)"),
     (r'\bmkfs\b', "format filesystem"),
@@ -71,6 +71,10 @@ DANGEROUS_PATTERNS = [
     (r'\bnohup\b.*gateway\s+run\b', "start gateway outside systemd (use 'systemctl --user restart hermes-gateway')"),
     # Self-termination protection: prevent agent from killing its own process
     (r'\b(pkill|killall)\b.*\b(hermes|gateway|cli\.py)\b', "kill hermes/gateway process (self-termination)"),
+    # File copy/move/edit into sensitive system paths
+    (r'\b(cp|mv|install)\b.*\s/etc/', "copy/move file into /etc/"),
+    (r'\bsed\s+-[^\s]*i.*\s/etc/', "in-place edit of system config"),
+    (r'\bsed\s+--in-place\b.*\s/etc/', "in-place edit of system config (long flag)"),
 ]
 
 
diff --git a/tools/file_tools.py b/tools/file_tools.py
index 7387c4dc..6226e765 100644
--- a/tools/file_tools.py
+++ b/tools/file_tools.py
@@ -4,7 +4,9 @@
 import errno
 import json
 import logging
+import os
 import threading
+from pathlib import Path
 from tools.file_operations import ShellFileOperations
 from agent.redact import redact_sensitive_text
 
@@ -13,6 +15,31 @@ logger = logging.getLogger(__name__)
 
 _EXPECTED_WRITE_ERRNOS = {errno.EACCES, errno.EPERM, errno.EROFS}
 
+# Paths that file tools should refuse to write to without going through the
+# terminal tool's approval system.  These match prefixes after os.path.realpath.
+_SENSITIVE_PATH_PREFIXES = ("/etc/", "/boot/", "/usr/lib/systemd/")
+_SENSITIVE_EXACT_PATHS = {"/var/run/docker.sock", "/run/docker.sock"}
+
+
+def _check_sensitive_path(filepath: str) -> str | None:
+    """Return an error message if the path targets a sensitive system location."""
+    try:
+        resolved = os.path.realpath(os.path.expanduser(filepath))
+    except (OSError, ValueError):
+        resolved = filepath
+    for prefix in _SENSITIVE_PATH_PREFIXES:
+        if resolved.startswith(prefix):
+            return (
+                f"Refusing to write to sensitive system path: {filepath}\n"
+                "Use the terminal tool with sudo if you need to modify system files."
+            )
+    if resolved in _SENSITIVE_EXACT_PATHS:
+        return (
+            f"Refusing to write to sensitive system path: {filepath}\n"
+            "Use the terminal tool with sudo if you need to modify system files."
+        )
+    return None
+
 
 def _is_expected_write_exception(exc: Exception) -> bool:
     """Return True for expected write denials that should not hit error logs."""
@@ -287,6 +314,9 @@ def notify_other_tool_call(task_id: str = "default"):
 
 def write_file_tool(path: str, content: str, task_id: str = "default") -> str:
     """Write content to a file."""
+    sensitive_err = _check_sensitive_path(path)
+    if sensitive_err:
+        return json.dumps({"error": sensitive_err}, ensure_ascii=False)
     try:
         file_ops = _get_file_ops(task_id)
         result = file_ops.write_file(path, content)
@@ -303,6 +333,18 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
                new_string: str = None, replace_all: bool = False, patch: str = None,
                task_id: str = "default") -> str:
     """Patch a file using replace mode or V4A patch format."""
+    # Check sensitive paths for both replace (explicit path) and V4A patch (extract paths)
+    _paths_to_check = []
+    if path:
+        _paths_to_check.append(path)
+    if mode == "patch" and patch:
+        import re as _re
+        for _m in _re.finditer(r'^\*\*\*\s+(?:Update|Add|Delete)\s+File:\s*(.+)$', patch, _re.MULTILINE):
+            _paths_to_check.append(_m.group(1).strip())
+    for _p in _paths_to_check:
+        sensitive_err = _check_sensitive_path(_p)
+        if sensitive_err:
+            return json.dumps({"error": sensitive_err}, ensure_ascii=False)
     try:
         file_ops = _get_file_ops(task_id)