mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-29 06:31:32 +00:00
fix(security): block read_file on project-local .env files
get_read_block_error() only blocked internal Hermes cache files but allowed reading project-local secret-bearing environment files (.env, .env.production, .env.local, etc.) through both read_file and ACP fs/read_text_file paths. Add a basename deny set for common secret-bearing .env variants. .env.example remains readable as documentation. Fixes #20734
This commit is contained in:
parent
51c913caf7
commit
ba3c450914
3 changed files with 197 additions and 11 deletions
|
|
@ -148,10 +148,24 @@ def is_write_denied(path: str) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
# Common secret-bearing project-local environment file basenames.
|
||||
# These are blocked because .env files routinely contain API keys,
|
||||
# database passwords, and other credentials.
|
||||
_BLOCKED_PROJECT_ENV_BASENAMES: set[str] = {
|
||||
".env",
|
||||
".env.local",
|
||||
".env.development",
|
||||
".env.production",
|
||||
".env.test",
|
||||
".env.staging",
|
||||
".envrc",
|
||||
}
|
||||
|
||||
|
||||
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:
|
||||
Three categories are blocked:
|
||||
|
||||
* Internal Hermes cache files under ``HERMES_HOME/skills/.hub`` —
|
||||
readable metadata that an attacker could use as a prompt-injection
|
||||
|
|
@ -163,6 +177,13 @@ def get_read_block_error(path: str) -> Optional[str]:
|
|||
OAuth tokens, and HMAC secrets that the agent never needs to read
|
||||
directly — provider tools / gateway adapters consume them through
|
||||
internal channels.
|
||||
* Project-local environment files anywhere on disk: ``.env``,
|
||||
``.env.local``, ``.env.development``, ``.env.production``,
|
||||
``.env.test``, ``.env.staging``, ``.envrc``. These routinely hold
|
||||
API keys, database passwords, and other credentials for the user's
|
||||
own projects. The agent helping debug a project shouldn't normally
|
||||
need to read these — ``.env.example`` is the documented-shape
|
||||
substitute.
|
||||
|
||||
**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``
|
||||
|
|
@ -267,6 +288,19 @@ def get_read_block_error(path: str) -> Optional[str]:
|
|||
"security boundary; the terminal tool can still bypass.)"
|
||||
)
|
||||
|
||||
# Block common secret-bearing project-local .env files anywhere on disk.
|
||||
# The agent helping a user with their project rarely needs to read raw
|
||||
# .env contents — .env.example is the documented-shape substitute. The
|
||||
# terminal tool can still ``cat .env``; this is defense-in-depth, not a
|
||||
# boundary (see module docstring).
|
||||
if resolved.name in _BLOCKED_PROJECT_ENV_BASENAMES:
|
||||
return (
|
||||
f"Access denied: {path} is a secret-bearing environment file "
|
||||
"and cannot be read to prevent credential leakage. "
|
||||
"If you need to check the file structure, read .env.example instead. "
|
||||
"(Defense-in-depth — not a security boundary; the terminal tool can still bypass.)"
|
||||
)
|
||||
|
||||
return None
|
||||
|
||||
|
||||
|
|
|
|||
150
tests/agent/test_file_safety.py
Normal file
150
tests/agent/test_file_safety.py
Normal file
|
|
@ -0,0 +1,150 @@
|
|||
"""Tests for agent/file_safety.py read guards — env file blocking.
|
||||
|
||||
Run with: python -m pytest tests/agent/test_file_safety.py -v
|
||||
"""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.file_safety import (
|
||||
_BLOCKED_PROJECT_ENV_BASENAMES,
|
||||
get_read_block_error,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Project-local .env file blocking (issue #20734)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestEnvFileReadBlocking:
|
||||
"""Secret-bearing .env files must be blocked by get_read_block_error."""
|
||||
|
||||
@pytest.mark.parametrize("basename", [
|
||||
".env",
|
||||
".env.local",
|
||||
".env.development",
|
||||
".env.production",
|
||||
".env.test",
|
||||
".env.staging",
|
||||
".envrc",
|
||||
])
|
||||
def test_blocked_env_basenames(self, basename):
|
||||
"""All secret-bearing .env basenames are blocked regardless of directory."""
|
||||
path = f"/tmp/project/{basename}"
|
||||
error = get_read_block_error(path)
|
||||
assert error is not None, f"{basename} should be blocked"
|
||||
assert "Access denied" in error
|
||||
assert "secret-bearing" in error.lower() or "environment file" in error.lower()
|
||||
|
||||
def test_blocked_env_in_subdirectory(self):
|
||||
"""Nested .env files are also blocked."""
|
||||
error = get_read_block_error("/home/user/app/services/api/.env.production")
|
||||
assert error is not None
|
||||
|
||||
def test_blocked_env_absolute_path(self):
|
||||
"""Absolute paths to .env files are blocked."""
|
||||
error = get_read_block_error("/opt/myapp/.env")
|
||||
assert error is not None
|
||||
|
||||
def test_allowed_env_example(self):
|
||||
""""The .env.example file is explicitly allowed — it's documentation, not a secret."""
|
||||
error = get_read_block_error("/tmp/project/.env.example")
|
||||
assert error is None
|
||||
|
||||
def test_allowed_env_sample(self):
|
||||
"""Other .env variants like .env.sample are allowed."""
|
||||
error = get_read_block_error("/tmp/project/.env.sample")
|
||||
assert error is None
|
||||
|
||||
def test_allowed_non_env_files(self):
|
||||
"""Regular files are not affected by the env guard."""
|
||||
for path in ["/tmp/project/config.yaml", "/tmp/project/main.py",
|
||||
"/tmp/project/README.md", "/tmp/project/.gitignore"]:
|
||||
error = get_read_block_error(path)
|
||||
assert error is None, f"{path} should be allowed"
|
||||
|
||||
def test_allowed_hermes_env(self):
|
||||
"""Hermes' own .env inside HERMES_HOME is NOT blocked by this rule
|
||||
(it's handled by other mechanisms). Only project-local .env is blocked."""
|
||||
# Note: hermes internal .env is in ~/.hermes/.env which is NOT a project-local
|
||||
# path, but the basename check applies to ANY .env. This is intentional —
|
||||
# even ~/.hermes/.env should not be readable via read_file.
|
||||
error = get_read_block_error(os.path.expanduser("~/.hermes/.env"))
|
||||
assert error is not None
|
||||
|
||||
def test_blocked_set_is_lowercase(self):
|
||||
"""All entries in the blocked set are lowercase for case-insensitive matching."""
|
||||
for name in _BLOCKED_PROJECT_ENV_BASENAMES:
|
||||
assert name == name.lower(), f"{name} should be lowercase"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Existing cache-file blocking (regression — must still work)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCacheFileReadBlocking:
|
||||
"""Internal Hermes cache files must remain blocked."""
|
||||
|
||||
def test_hub_index_cache_blocked(self, tmp_path):
|
||||
"""Hub index-cache reads are blocked."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
cache = hermes_home / "skills" / ".hub" / "index-cache" / "data.json"
|
||||
cache.parent.mkdir(parents=True)
|
||||
cache.write_text("{}")
|
||||
|
||||
with patch("agent.file_safety._hermes_home_path", return_value=hermes_home):
|
||||
error = get_read_block_error(str(cache))
|
||||
assert error is not None
|
||||
assert "internal Hermes cache" in error
|
||||
|
||||
def test_hub_directory_blocked(self, tmp_path):
|
||||
"""Hub directory reads are blocked."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hub = hermes_home / "skills" / ".hub" / "metadata.json"
|
||||
hub.parent.mkdir(parents=True)
|
||||
hub.write_text("{}")
|
||||
|
||||
with patch("agent.file_safety._hermes_home_path", return_value=hermes_home):
|
||||
error = get_read_block_error(str(hub))
|
||||
assert error is not None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Combined: env guard + cache guard don't interfere
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCombinedGuards:
|
||||
"""Both guards should work independently without interference."""
|
||||
|
||||
def test_env_guard_works_regardless_of_hermes_home(self, tmp_path):
|
||||
"""The env basename guard does not depend on HERMES_HOME resolution."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
||||
with patch("agent.file_safety._hermes_home_path", return_value=hermes_home):
|
||||
# Regular project .env should still be blocked
|
||||
error = get_read_block_error("/workspace/.env")
|
||||
assert error is not None
|
||||
|
||||
# .env.example should still be allowed
|
||||
error = get_read_block_error("/workspace/.env.example")
|
||||
assert error is None
|
||||
|
||||
def test_cache_guard_still_works_with_env_guard(self, tmp_path):
|
||||
"""Cache file blocking still works when env guard is active."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
cache = hermes_home / "skills" / ".hub" / "index-cache" / "x"
|
||||
cache.parent.mkdir(parents=True)
|
||||
cache.write_text("")
|
||||
|
||||
with patch("agent.file_safety._hermes_home_path", return_value=hermes_home):
|
||||
error = get_read_block_error(str(cache))
|
||||
assert error is not None
|
||||
assert "internal Hermes cache" in error
|
||||
|
|
@ -246,22 +246,24 @@ def test_mcp_tokens_dir_itself_blocked(fake_home):
|
|||
assert "MCP token" in err
|
||||
|
||||
|
||||
def test_identically_named_files_outside_hermes_home_not_blocked(
|
||||
def test_identically_named_hermes_files_outside_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."""
|
||||
"""Hermes-specific filenames (``auth.json``, ``mcp-tokens/``, ``google_oauth.json``)
|
||||
outside HERMES_HOME must remain readable — the gate is per-location for
|
||||
those, not per-filename. ``.env`` is the exception: it's blocked anywhere
|
||||
on disk (see test_project_local_env_blocked) because the basename always
|
||||
means \"secret-bearing environment file\" regardless of directory."""
|
||||
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"
|
||||
)
|
||||
# auth.json outside HERMES_HOME — readable (per-location gate).
|
||||
p = project / "auth.json"
|
||||
p.write_text("not secret here", encoding="utf-8")
|
||||
assert get_read_block_error(str(p)) is None, (
|
||||
"auth.json outside HERMES_HOME should NOT be blocked"
|
||||
)
|
||||
|
||||
google_oauth = project / "auth" / "google_oauth.json"
|
||||
google_oauth.parent.mkdir()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue