From ba3c450914d32a502b4a95f50202f60ec0f07234 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Wed, 6 May 2026 22:41:14 +0800 Subject: [PATCH] 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 --- agent/file_safety.py | 36 ++++- tests/agent/test_file_safety.py | 150 ++++++++++++++++++++ tests/agent/test_file_safety_credentials.py | 22 +-- 3 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 tests/agent/test_file_safety.py diff --git a/agent/file_safety.py b/agent/file_safety.py index e8c80f2b6d0..22b190c3a6c 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -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 diff --git a/tests/agent/test_file_safety.py b/tests/agent/test_file_safety.py new file mode 100644 index 00000000000..a7ff019d438 --- /dev/null +++ b/tests/agent/test_file_safety.py @@ -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 diff --git a/tests/agent/test_file_safety_credentials.py b/tests/agent/test_file_safety_credentials.py index 30199892c84..d0fbb80f123 100644 --- a/tests/agent/test_file_safety_credentials.py +++ b/tests/agent/test_file_safety_credentials.py @@ -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()