fix(file-safety): defense-in-depth read-deny on Hermes credential stores (#17659)

agent/file_safety.py

@@ -136,21 +136,121 @@ def is_write_denied(path: str) -> bool:
 
 
 def get_read_block_error(path: str) -> Optional[str]:
-    """Return an error message when a read targets internal Hermes cache files."""
+    """Return an error message when a read targets a denied Hermes path.
+
+    Two categories are blocked:
+
+      * Internal Hermes cache files under ``HERMES_HOME/skills/.hub`` —
+        readable metadata that an attacker could use as a prompt-injection
+        carrier.
+      * Credential / secret stores under HERMES_HOME and the global Hermes
+        root: ``auth.json``, ``auth.lock``, ``.anthropic_oauth.json``,
+        ``.env``, ``webhook_subscriptions.json``, and anything under
+        ``mcp-tokens/``. These hold plaintext provider keys, OAuth tokens,
+        and HMAC secrets that the agent never needs to read directly —
+        provider tools / gateway adapters consume them through internal
+        channels.
+
+    **This is NOT a security boundary.** The terminal tool runs as the
+    same OS user with shell access; the agent can still ``cat auth.json``
+    or ``cat ~/.hermes/.env`` and exfiltrate the file. The read-deny exists
+    as defense-in-depth that:
+
+      * Returns a clear error to models that respect tool denials, which
+        empirically prompts most modern models to stop rather than reach
+        for the shell.
+      * Surfaces a visible audit trail when something tries to read
+        credentials — easier to spot in logs than a generic ``cat``.
+
+    Treat any user-visible framing around this as "may help" rather than
+    "stops attackers." A determined model or malicious instruction can
+    always shell out.
+
+    Callers that resolve relative paths against a non-process cwd
+    (e.g. ``TERMINAL_CWD`` in ``tools/file_tools.py``) MUST pre-resolve
+    and pass the absolute path string.  This function's own ``resolve()``
+    is anchored at the Python process cwd, so a relative input like
+    ``"auth.json"`` would otherwise miss the denylist when the task's
+    terminal cwd differs from the process cwd.
+    """
     resolved = Path(path).expanduser().resolve()
-    hermes_home = _hermes_home_path().resolve()
-    blocked_dirs = [
-        hermes_home / "skills" / ".hub" / "index-cache",
-        hermes_home / "skills" / ".hub",
-    ]
-    for blocked in blocked_dirs:
+
+    # Resolve BOTH the active HERMES_HOME (profile-aware) AND the global
+    # Hermes root so credential stores at <root>/auth.json etc. are also
+    # blocked when running under a profile (HERMES_HOME points at
+    # <root>/profiles/<name> in profile mode). Same shape as the write
+    # deny widening (#15981, #14157).
+    hermes_dirs: list[Path] = []
+    for base in (_hermes_home_path(), _hermes_root_path()):
         try:
-            resolved.relative_to(blocked)
+            real = base.resolve()
+            if real not in hermes_dirs:
+                hermes_dirs.append(real)
+        except Exception:
+            continue
+
+    # Skills .hub: prompt-injection carriers.
+    for hd in hermes_dirs:
+        blocked_dirs = [
+            hd / "skills" / ".hub" / "index-cache",
+            hd / "skills" / ".hub",
+        ]
+        for blocked in blocked_dirs:
+            try:
+                resolved.relative_to(blocked)
+            except ValueError:
+                continue
+            return (
+                f"Access denied: {path} is an internal Hermes cache file "
+                "and cannot be read directly to prevent prompt injection. "
+                "Use the skills_list or skill_view tools instead."
+            )
+
+    # Credential / secret stores. Exact-file matches under either
+    # HERMES_HOME or <root>.
+    credential_file_names = (
+        "auth.json",
+        "auth.lock",
+        ".anthropic_oauth.json",
+        ".env",
+        "webhook_subscriptions.json",
+    )
+    for hd in hermes_dirs:
+        for name in credential_file_names:
+            try:
+                blocked = (hd / name).resolve()
+            except Exception:
+                continue
+            if resolved == blocked:
+                return (
+                    f"Access denied: {path} is a Hermes credential store "
+                    "and cannot be read directly. Provider tools consume "
+                    "these credentials through internal channels. "
+                    "(Defense-in-depth — not a security boundary; the "
+                    "terminal tool can still bypass.)"
+                )
+
+    # mcp-tokens/: directory prefix match — anything inside is OAuth
+    # token material.
+    for hd in hermes_dirs:
+        try:
+            mcp_tokens = (hd / "mcp-tokens").resolve()
+        except Exception:
+            continue
+        if resolved == mcp_tokens:
+            return (
+                f"Access denied: {path} is the Hermes MCP token directory "
+                "and cannot be read directly. (Defense-in-depth — not a "
+                "security boundary; the terminal tool can still bypass.)"
+            )
+        try:
+            resolved.relative_to(mcp_tokens)
         except ValueError:
             continue
         return (
-            f"Access denied: {path} is an internal Hermes cache file "
-            "and cannot be read directly to prevent prompt injection. "
-            "Use the skills_list or skill_view tools instead."
+            f"Access denied: {path} is a Hermes MCP token file "
+            "and cannot be read directly. (Defense-in-depth — not a "
+            "security boundary; the terminal tool can still bypass.)"
         )
+
     return None