mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(file-safety): widen read-deny to .env, mcp-tokens/, webhook secrets, root
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 = <root>/profiles/<name>) also block <root>/{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 <zqiao@microsoft.com>
This commit is contained in:
parent
567ea61298
commit
97e975edd2
2 changed files with 222 additions and 29 deletions
|
|
@ -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 <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.)"
|
||||
)
|
||||
|
||||
# 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
|
||||
|
|
|
|||
|
|
@ -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 = <root>/profiles/<name>, but
|
||||
<root>/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 "")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue