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
This commit is contained in:
@@ -41,8 +41,8 @@ DANGEROUS_PATTERNS = [
|
|||||||
(r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"),
|
(r'\brm\s+(-[^\s]*\s+)*/', "delete in root path"),
|
||||||
(r'\brm\s+-[^\s]*r', "recursive delete"),
|
(r'\brm\s+-[^\s]*r', "recursive delete"),
|
||||||
(r'\brm\s+--recursive\b', "recursive delete (long flag)"),
|
(r'\brm\s+--recursive\b', "recursive delete (long flag)"),
|
||||||
(r'\bchmod\s+(-[^\s]*\s+)*777\b', "world-writable permissions"),
|
(r'\bchmod\s+(-[^\s]*\s+)*(777|666|o\+[rwx]*w|a\+[rwx]*w)\b', "world/other-writable permissions"),
|
||||||
(r'\bchmod\s+--recursive\b.*777', "recursive world-writable (long flag)"),
|
(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+(-[^\s]*)?R\s+root', "recursive chown to root"),
|
||||||
(r'\bchown\s+--recursive\b.*root', "recursive chown to root (long flag)"),
|
(r'\bchown\s+--recursive\b.*root', "recursive chown to root (long flag)"),
|
||||||
(r'\bmkfs\b', "format filesystem"),
|
(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')"),
|
(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
|
# 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)"),
|
(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)"),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -4,7 +4,9 @@
|
|||||||
import errno
|
import errno
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
import threading
|
import threading
|
||||||
|
from pathlib import Path
|
||||||
from tools.file_operations import ShellFileOperations
|
from tools.file_operations import ShellFileOperations
|
||||||
from agent.redact import redact_sensitive_text
|
from agent.redact import redact_sensitive_text
|
||||||
|
|
||||||
@@ -13,6 +15,31 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
_EXPECTED_WRITE_ERRNOS = {errno.EACCES, errno.EPERM, errno.EROFS}
|
_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:
|
def _is_expected_write_exception(exc: Exception) -> bool:
|
||||||
"""Return True for expected write denials that should not hit error logs."""
|
"""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:
|
def write_file_tool(path: str, content: str, task_id: str = "default") -> str:
|
||||||
"""Write content to a file."""
|
"""Write content to a file."""
|
||||||
|
sensitive_err = _check_sensitive_path(path)
|
||||||
|
if sensitive_err:
|
||||||
|
return json.dumps({"error": sensitive_err}, ensure_ascii=False)
|
||||||
try:
|
try:
|
||||||
file_ops = _get_file_ops(task_id)
|
file_ops = _get_file_ops(task_id)
|
||||||
result = file_ops.write_file(path, content)
|
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,
|
new_string: str = None, replace_all: bool = False, patch: str = None,
|
||||||
task_id: str = "default") -> str:
|
task_id: str = "default") -> str:
|
||||||
"""Patch a file using replace mode or V4A patch format."""
|
"""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:
|
try:
|
||||||
file_ops = _get_file_ops(task_id)
|
file_ops = _get_file_ops(task_id)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user