From 97e975edd2cd666b09412cca5ee22d3e5ad431de Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 22 May 2026 20:11:16 -0700 Subject: [PATCH] fix(file-safety): widen read-deny to .env, mcp-tokens/, webhook secrets, root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends @briandevans's PR #17659 from {auth.json, auth.lock, .anthropic_oauth.json} to also cover: - HERMES_HOME/.env (provider API keys) - HERMES_HOME/webhook_subscriptions.json (per-route HMAC secrets) - HERMES_HOME/mcp-tokens/ (OAuth token directory; dir + everything inside) …AND iterates over both _hermes_home_path() AND _hermes_root_path() so profile-mode runs (HERMES_HOME = /profiles/) also block /{auth.json, .env, mcp-tokens/, ...}. Same widening shape as the write-deny side already does (#15981, #14157). Explicitly NOT a security boundary. Per the personal-assistant trust model, the terminal tool runs as the same OS user and can `cat auth.json` directly. This read-deny exists as defense-in-depth: - Models that respect tool denials empirically tend to stop rather than reach for the shell. - The denial surfaces an audit trail when something tries to read credentials — easier to spot in logs than a generic `cat`. Docstring + error message both flag this as defense-in-depth so future contributors don't mistake it for a real security boundary and don't re-decline reports that propose the same fix shape. Absorbs the .env and mcp-tokens/ coverage from @tomqiaozc's parallel PR #8055 (closed-as-duplicate, credited). Co-authored-by: Tom Qiao --- agent/file_safety.py | 125 ++++++++++++++----- tests/agent/test_file_safety_credentials.py | 126 ++++++++++++++++++++ 2 files changed, 222 insertions(+), 29 deletions(-) diff --git a/agent/file_safety.py b/agent/file_safety.py index e0d7876ae5d..c5e132bdde1 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -139,12 +139,32 @@ def get_read_block_error(path: str) -> Optional[str]: """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 stores at the top of ``HERMES_HOME`` (``auth.json``, - ``auth.lock``, ``.anthropic_oauth.json``) — plaintext provider - keys / OAuth tokens that the agent never needs to read directly. + * 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 @@ -154,36 +174,83 @@ def get_read_block_error(path: str) -> Optional[str]: 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 /auth.json etc. are also + # blocked when running under a profile (HERMES_HOME points at + # /profiles/ 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 . + 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.)" ) - # Credential stores under HERMES_HOME hold plaintext provider keys - # and OAuth tokens. The agent never needs to read these directly — - # auxiliary_client / credential_pool consume them through process - # env / OAuth flows that bypass read_file. Block read access so a - # prompt-injection reaching read_file can't exfiltrate them. - blocked_credential_files = { - hermes_home / "auth.json", - hermes_home / "auth.lock", - hermes_home / ".anthropic_oauth.json", - } - if resolved in blocked_credential_files: - return ( - f"Access denied: {path} is a Hermes credential store " - "and cannot be read directly. Provider tools consume these " - "credentials through internal channels." - ) return None diff --git a/tests/agent/test_file_safety_credentials.py b/tests/agent/test_file_safety_credentials.py index f362249278d..94cf82f2ccd 100644 --- a/tests/agent/test_file_safety_credentials.py +++ b/tests/agent/test_file_safety_credentials.py @@ -147,3 +147,129 @@ def test_read_file_tool_blocks_relative_path_under_terminal_cwd( out = json.loads(ft.read_file_tool("auth.json")) assert "error" in out assert "credential store" in out["error"] + + +# --------------------------------------------------------------------------- +# Widening: .env, webhook_subscriptions.json, mcp-tokens/ +# --------------------------------------------------------------------------- + + +def test_dotenv_blocked(fake_home): + """.env in HERMES_HOME holds API keys — blocked.""" + from agent.file_safety import get_read_block_error + + env = _create(fake_home, ".env") + err = get_read_block_error(str(env)) + assert err is not None + assert "credential store" in err + + +def test_webhook_subscriptions_blocked(fake_home): + """webhook_subscriptions.json holds per-route HMAC secrets — blocked.""" + from agent.file_safety import get_read_block_error + + subs = _create(fake_home, "webhook_subscriptions.json") + err = get_read_block_error(str(subs)) + assert err is not None + assert "credential store" in err + + +def test_mcp_tokens_file_blocked(fake_home): + """Files under mcp-tokens/ hold OAuth tokens — blocked.""" + from agent.file_safety import get_read_block_error + + tok = _create(fake_home, Path("mcp-tokens") / "github.json") + err = get_read_block_error(str(tok)) + assert err is not None + assert "MCP token" in err + + +def test_mcp_tokens_nested_blocked(fake_home): + """Nested files inside mcp-tokens/ are also blocked.""" + from agent.file_safety import get_read_block_error + + tok = _create(fake_home, Path("mcp-tokens") / "providers" / "azure.json") + err = get_read_block_error(str(tok)) + assert err is not None + assert "MCP token" in err + + +def test_mcp_tokens_dir_itself_blocked(fake_home): + """The mcp-tokens directory itself is blocked (listing is exfiltrating).""" + from agent.file_safety import get_read_block_error + + tokens_dir = fake_home / "mcp-tokens" + tokens_dir.mkdir(parents=True, exist_ok=True) + err = get_read_block_error(str(tokens_dir)) + assert err is not None + assert "MCP token" in err + + +def test_identically_named_files_outside_hermes_home_not_blocked( + fake_home, tmp_path +): + """A project's ``.env``, ``auth.json``, or ``mcp-tokens/`` outside + HERMES_HOME must remain readable — the gate is per-location, not + per-filename.""" + from agent.file_safety import get_read_block_error + + project = tmp_path / "myproject" + project.mkdir() + for rel in (".env", "auth.json"): + p = project / rel + p.write_text("not secret here", encoding="utf-8") + assert get_read_block_error(str(p)) is None, ( + f"{rel} outside HERMES_HOME should NOT be blocked" + ) + + tokens = project / "mcp-tokens" + tokens.mkdir() + tok_file = tokens / "token.json" + tok_file.write_text("not really a token", encoding="utf-8") + assert get_read_block_error(str(tok_file)) is None + + +def test_config_yaml_not_blocked(fake_home): + """config.yaml is NOT a credential file — agent should still be + able to read it for debugging. (Writes are denied separately by + is_write_denied; reads stay allowed.)""" + from agent.file_safety import get_read_block_error + + cfg = _create(fake_home, "config.yaml") + assert get_read_block_error(str(cfg)) is None + + +def test_profile_mode_blocks_root_credentials(tmp_path, monkeypatch): + """Under a profile, HERMES_HOME = /profiles/, but + /auth.json must ALSO be blocked — credentials at root are + inherited by every profile.""" + import agent.file_safety as fs + + root = tmp_path / "hermes" + profile = root / "profiles" / "coder" + profile.mkdir(parents=True) + monkeypatch.setattr(fs, "_hermes_home_path", lambda: profile) + monkeypatch.setattr(fs, "_hermes_root_path", lambda: root) + + from agent.file_safety import get_read_block_error + + # Profile-local credential store: blocked + profile_auth = profile / "auth.json" + profile_auth.write_text("x") + assert "credential store" in (get_read_block_error(str(profile_auth)) or "") + + # Root-level credential store: ALSO blocked (this is the widening) + root_auth = root / "auth.json" + root_auth.write_text("x") + assert "credential store" in (get_read_block_error(str(root_auth)) or "") + + # Root-level .env: blocked too + root_env = root / ".env" + root_env.write_text("x") + assert "credential store" in (get_read_block_error(str(root_env)) or "") + + # Root-level mcp-tokens: blocked + root_tok = root / "mcp-tokens" / "gh.json" + root_tok.parent.mkdir(parents=True, exist_ok=True) + root_tok.write_text("x") + assert "MCP token" in (get_read_block_error(str(root_tok)) or "")