Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions tests/tools/test_approval.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,83 @@ def test_tee_local_file_safe(self):
assert key is None


class TestHermesApprovalStateWriteProtection:
Comment thread
9thLevelSoftware marked this conversation as resolved.
@staticmethod
def _assert_dangerous(command):
dangerous, key, desc = detect_dangerous_command(command)
assert dangerous is True, command
assert key is not None
assert desc

def test_redirect_overwrite(self):
self._assert_dangerous(
"echo '{\"requests\":[]}' > ~/.hermes/approval_requests.json"
)

def test_append(self):
self._assert_dangerous(
"echo forged >> ~/.hermes/approval_requests.json"
)

def test_tee(self):
self._assert_dangerous(
"echo x | tee ~/.hermes/approval_requests.json"
)

def test_cp_mv_and_install(self):
for command in ("cp", "mv", "install"):
self._assert_dangerous(
f"{command} /tmp/forged.json ~/.hermes/approval_requests.json"
)

def test_symlink(self):
self._assert_dangerous(
"ln -sf /tmp/forged.json ~/.hermes/approval_requests.json"
)
self._assert_dangerous(
"ln -s ~/.hermes/approval_requests.json /tmp/approval-alias && "
"echo forged > /tmp/approval-alias"
)
for option in ("-s", "-l", "--symbolic-link", "--link"):
self._assert_dangerous(
f"cp {option} ~/.hermes/approval_requests.json /tmp/approval-alias"
)

def test_sed_in_place_forms(self):
self._assert_dangerous(
"sed -i 's/pending/resolved/' ~/.hermes/approval_requests.json"
)
self._assert_dangerous(
"sed --in-place 's/pending/resolved/' ~/.hermes/approval_requests.json"
)

def test_perl_and_ruby_in_place(self):
self._assert_dangerous(
"perl -i -pe 's/pending/resolved/' ~/.hermes/approval_requests.json"
)
self._assert_dangerous(
"ruby -i -pe 'gsub(/pending/, \"resolved\")' ~/.hermes/approval_requests.json"
)

def test_profile_qualified_and_custom_home(self):
self._assert_dangerous(
"echo forged > ~/.hermes/profiles/work/approval_requests.json"
)
self._assert_dangerous(
"echo forged > ~/.hermes/./approval_requests.json"
)
self._assert_dangerous(
"echo forged > ~/.hermes/profiles/work/../work/approval_requests.json"
)
self._assert_dangerous(
"echo x | tee $HERMES_HOME/approval_requests.json"
)

def test_absolute_active_home_write_requires_approval(self):
state_path = get_hermes_home() / "approval_requests.json"
self._assert_dangerous(f"echo forged > {state_path}")


class TestHermesConfigWriteProtection:
"""Terminal-side pairing for the file_tools write_file/patch deny on
~/.hermes/config.yaml (#14639). config.yaml IS the security policy
Expand Down
24 changes: 24 additions & 0 deletions tests/tools/test_file_write_safety.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Based on PR #1085 by ismoilh (salvaged).
"""

import json
import os
from pathlib import Path

Expand Down Expand Up @@ -191,6 +192,29 @@ def test_boot_still_blocked(self):
from tools.file_tools import _check_sensitive_path
assert _check_sensitive_path("/boot/grub/grub.cfg") is not None

def test_approval_state_file_blocked(self):
from hermes_constants import get_hermes_home
from tools.file_tools import _check_sensitive_path, write_file_tool

path = get_hermes_home() / "approval_requests.json"
assert _check_sensitive_path(str(path)) is not None
result = json.loads(write_file_tool(str(path), '{"requests": []}'))
assert "error" in result
assert "approval state" in result["error"].lower()

def test_container_and_profile_approval_state_paths_blocked(self):
from tools.file_tools import _check_sensitive_path

assert _check_sensitive_path(
"/root/.hermes/approval_requests.json"
) is not None
assert _check_sensitive_path(
"/root/.hermes/profiles/work/approval_requests.json"
) is not None
assert _check_sensitive_path(
r"C:\Users\agent\.hermes\profiles\work\approval_requests.json"
) is not None

def test_safe_path_allowed(self):
from tools.file_tools import _check_sensitive_path
assert _check_sensitive_path("/tmp/safe_file.txt") is None
Expand Down
17 changes: 13 additions & 4 deletions tools/approval.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ def _is_gateway_approval_context() -> bool:
r'(?:\$hermes_home|\$\{hermes_home\})/)'
r'config\.yaml\b'
)
_HERMES_APPROVAL_STATE_PATH = (
r'(?:~\/\.hermes/|'
r'(?:\$home|\$\{home\})/\.hermes/|'
r'(?:\$hermes_home|\$\{hermes_home\})/)'
r'(?:[^/\s"\'`]+/)*approval_requests\.json\b'
)
_PROJECT_ENV_PATH = r'(?:(?:/|\.{1,2}/)?(?:[^\s/"\'`]+/)*\.env(?:\.[^/\s"\'`]+)*)'
_PROJECT_CONFIG_PATH = r'(?:(?:/|\.{1,2}/)?(?:[^\s/"\'`]+/)*config\.yaml)'
_SHELL_RC_FILES = (
Expand All @@ -255,6 +261,7 @@ def _is_gateway_approval_context() -> bool:
rf'{_SSH_SENSITIVE_PATH}|'
rf'{_HERMES_ENV_PATH}|'
rf'{_HERMES_CONFIG_PATH}|'
rf'{_HERMES_APPROVAL_STATE_PATH}|'
Comment thread
9thLevelSoftware marked this conversation as resolved.
rf'{_SHELL_RC_FILES}|'
rf'{_CREDENTIAL_FILES})'
)
Expand Down Expand Up @@ -672,7 +679,9 @@ def _sudo_stdin_block_result(description: str) -> dict:
# but reading OUT of a sensitive path (`cp ~/.ssh/config /tmp/x`) stays safe.
# The trailing `[^\s"\']*` consumes the rest of the destination filename
# (e.g. `authorized_keys` after the `~/.ssh/` fragment).
(rf'\b(cp|mv|install)\b.*\s["\']?{_SENSITIVE_WRITE_TARGET}[^\s"\']*["\']?{_COMMAND_TAIL}', "copy/move file into sensitive credential/SSH/shell-rc path"),
(rf'\bcp\b(?=[^\n]*(?:\s-[^\s]*[sl][^\s]*|\s--(?:symbolic-link|link)\b)).*{_SENSITIVE_WRITE_TARGET}', "link sensitive credential/SSH/shell-rc/Hermes path via cp"),
(rf'\b(cp|mv|install)\b.*\s["\']?{_SENSITIVE_WRITE_TARGET}[^\s"\']*["\']?{_COMMAND_TAIL}', "copy/move file into sensitive credential/SSH/shell-rc/Hermes path"),
(rf'\bln\b.*{_SENSITIVE_WRITE_TARGET}', "link sensitive credential/SSH/shell-rc/Hermes path"),
Comment thread
9thLevelSoftware marked this conversation as resolved.
# In-place edits mutate the target file directly, bypassing redirection,
# tee, and copy/move/install coverage. Gate the same user-controlled
# startup/credential files so `sed -i ... ~/.bashrc` and `perl -i ...
Expand All @@ -686,8 +695,8 @@ def _sudo_stdin_block_result(description: str) -> dict:
# .env). sed -i bypasses the redirection/tee patterns above because it
# mutates the file directly. Pairs the file_tools write_file/patch deny so
# the terminal side is not an open door. See #14639.
(rf'\bsed\s+-[^\s]*i.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH})', "in-place edit of Hermes config/env"),
(rf'\bsed\s+--in-place\b.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH})', "in-place edit of Hermes config/env (long flag)"),
(rf'\bsed\s+-[^\s]*i.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH}|{_HERMES_APPROVAL_STATE_PATH})', "in-place edit of Hermes security state"),
(rf'\bsed\s+--in-place\b.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH}|{_HERMES_APPROVAL_STATE_PATH})', "in-place edit of Hermes security state (long flag)"),
Comment thread
9thLevelSoftware marked this conversation as resolved.
# perl -i and ruby -i perform the same in-place mutation as sed -i but are
# not caught by the -e/-c script-execution pattern above (which targets code
# evaluation, not file mutation). Pairs the sed -i coverage from #14639.
Expand All @@ -696,7 +705,7 @@ def _sudo_stdin_block_result(description: str) -> dict:
# backup suffix (`perl -i.bak`). Match any flag token containing `i`
# anywhere in the args, not just the first token — `perl -e '...'` (code
# eval, no -i) does not trip because it has no `-...i` flag token.
(rf'\b(?:perl|ruby)\b.*(?:^|\s)-[^\s]*i\b.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH})', "in-place edit of Hermes config/env (perl/ruby)"),
(rf'\b(?:perl|ruby)\b.*(?:^|\s)-[^\s]*i\b.*(?:{_HERMES_CONFIG_PATH}|{_HERMES_ENV_PATH}|{_HERMES_APPROVAL_STATE_PATH})', "in-place edit of Hermes security state (perl/ruby)"),
# Script execution via heredoc — bypasses the -e/-c flag patterns above.
# `python3 << 'EOF'` feeds arbitrary code via stdin without -c/-e flags.
(r'\b(python[23]?|perl|ruby|node)\s+<<', "script execution via heredoc"),
Expand Down
29 changes: 29 additions & 0 deletions tools/file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,35 @@ def _check_sensitive_path(filepath: str, task_id: str = "default") -> str | None
"Agent cannot modify security-sensitive configuration. "
"Edit ~/.hermes/config.yaml directly or use 'hermes config' instead."
)
# Persisted approval outcomes are security policy state. If an agent could
# write this file directly it could forge a resolved request and bypass the
# terminal approval gate after the next lazy load.
try:
from hermes_constants import get_hermes_home

approval_state = str(
(get_hermes_home() / "approval_requests.json").resolve()
)
except Exception:
approval_state = ""
candidates = [resolved, normalized]
is_approval_state = bool(
approval_state and any(path == approval_state for path in candidates)
)
for candidate in candidates:
clean = candidate.replace("\\", "/").rstrip("/")
if clean.endswith("/.hermes/approval_requests.json"):
is_approval_state = True
marker = "/.hermes/profiles/"
if marker in clean:
profile_tail = clean.split(marker, 1)[1].split("/")
if len(profile_tail) == 2 and profile_tail[1] == "approval_requests.json":
is_approval_state = True
if is_approval_state:
return (
f"Refusing to write to Hermes approval state: {filepath}\n"
"Agent cannot modify persisted security decisions."
)
Comment thread
9thLevelSoftware marked this conversation as resolved.
return None


Expand Down