diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index e65a73333434..98bf9e55659b 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -586,6 +586,83 @@ def test_tee_local_file_safe(self): assert key is None +class TestHermesApprovalStateWriteProtection: + @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 diff --git a/tests/tools/test_file_write_safety.py b/tests/tools/test_file_write_safety.py index ae766a7a723e..0e647f2dd754 100644 --- a/tests/tools/test_file_write_safety.py +++ b/tests/tools/test_file_write_safety.py @@ -3,6 +3,7 @@ Based on PR #1085 by ismoilh (salvaged). """ +import json import os from pathlib import Path @@ -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 diff --git a/tools/approval.py b/tools/approval.py index c94eb8603b30..c44a1f061879 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -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 = ( @@ -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}|' rf'{_SHELL_RC_FILES}|' rf'{_CREDENTIAL_FILES})' ) @@ -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"), # 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 ... @@ -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)"), # 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. @@ -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"), diff --git a/tools/file_tools.py b/tools/file_tools.py index c077d21cfa23..fbbb3195a051 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -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." + ) return None