From 10a54ccc2c5d07959b099336eef392aca7033754 Mon Sep 17 00:00:00 2001 From: mrparker0980 <290881485+mrparker0980@users.noreply.github.com> Date: Mon, 8 Jun 2026 13:51:09 +0300 Subject: [PATCH] fix(security): anchor @file context refs to canonical read deny-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@file` / `@folder` context-reference expansion enforced its own narrow deny-list (`_ensure_reference_path_allowed` in `agent/context_references.py`) that only covered `~/.ssh` keys, a handful of shell dotfiles, `~/.hermes/.env`, and `skills/.hub`. It never blocked the credential stores that the canonical read guard (`agent/file_safety.get_read_block_error`) protects: provider API keys (`~/.hermes/auth.json`), Anthropic OAuth tokens (`~/.hermes/.anthropic_oauth.json`), MCP OAuth material (`~/.hermes/mcp-tokens/`), webhook HMAC secrets, and project-local `.env` files. This matters because the messaging gateway feeds **untrusted** remote text straight into reference expansion: `gateway/run.py` calls `preprocess_context_references_async(..., allowed_root=_msg_cwd)` where `_msg_cwd` defaults to the operator's HOME when `TERMINAL_CWD` is unset. A chat peer (Telegram/Discord/Slack/...) could send `@file:~/.hermes/auth.json`, pass the `allowed_root` check (it resolves under HOME), slip past the narrow list, and have the operator's live keys read into the agent's context — where the model would typically echo or act on them. Rather than duplicate and re-sync a second secret list, this routes the guard through the existing single source of truth. A reviewer might ask "why not just add `auth.json` to the local list?" — because the local list has already drifted once (a prior commit had to add `.config/gh`); anchoring to `get_read_block_error` means every future addition there protects this path too. The narrow checks are kept as a fallback since they also cover dirs that guard does not (`.aws`, `.gnupg`, `.kube`, etc.), and the canonical lookup is wrapped so it can never crash reference expansion. N/A - [x] 🔒 Security fix - `agent/context_references.py`: `_ensure_reference_path_allowed` now also consults `agent.file_safety.get_read_block_error` after its existing checks and refuses the reference when that canonical guard flags the resolved path. The lookup is wrapped so guard-resolution failures fall back to the explicit checks instead of breaking expansion. - `tests/agent/test_context_references.py`: added `test_blocks_canonical_read_denylist_credential_stores`, asserting that `@file` attaches for `auth.json`, `.anthropic_oauth.json`, `mcp-tokens/*`, and a project-local `.env` are all refused and their secret bodies never reach the expanded message. - `scripts/release.py`: added the contributor email to `AUTHOR_MAP` (release gate). 1. `scripts/run_tests.sh tests/agent/test_context_references.py` — all 15 tests pass, including the new credential-store case. 2. Regression proof: stash `agent/context_references.py`, run the suite with `-- -k canonical`, and confirm the new test fails (secrets leak into the message) without the fix; restore and confirm it passes. 3. `ruff check agent/context_references.py tests/agent/test_context_references.py` and `python scripts/check-windows-footguns.py agent/context_references.py tests/agent/test_context_references.py` both pass. - [x] I've read the Contributing Guide - [x] My commit messages follow Conventional Commits (`fix(scope):`, etc.) - [x] I searched for existing PRs to make sure this isn't a duplicate - [x] My PR contains **only** changes related to this fix (plus the AUTHOR_MAP release gate) - [x] I've run the test suite for the touched area and all tests pass - [x] I've added tests for my changes (required for bug fixes) - [x] I've tested on my platform: macOS 15 (Darwin 25.5) - [x] I've updated relevant documentation (README, `docs/`, docstrings) — or N/A - [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A - [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A - [x] I've considered cross-platform impact (Windows, macOS) — or N/A - [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A --- agent/context_references.py | 31 +++++++++ scripts/release.py | 1 + tests/agent/test_context_references.py | 92 ++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/agent/context_references.py b/agent/context_references.py index fe63190e2c0..eea16ae52b4 100644 --- a/agent/context_references.py +++ b/agent/context_references.py @@ -381,6 +381,37 @@ def _ensure_reference_path_allowed(path: Path) -> None: continue raise ValueError("path is a sensitive credential or internal Hermes path and cannot be attached") + # Anchor to the canonical read deny-list (agent/file_safety.get_read_block_error), + # the single source of truth used by the file/terminal read path. The narrow + # list above predates that guard and never caught the real credential stores: + # provider keys (auth.json), Anthropic OAuth tokens (.anthropic_oauth.json), + # MCP OAuth material (mcp-tokens/), webhook HMAC secrets, and project-local + # .env files. That gap matters because the gateway feeds UNTRUSTED remote + # message text into reference expansion, so `@file:~/.hermes/auth.json` from a + # chat peer would otherwise read the operator's keys straight into context. + # Routing through the canonical guard closes the gap today and keeps this path + # protected automatically whenever that deny-list grows. + try: + from agent.file_safety import get_read_block_error + + if get_read_block_error(str(path)) is not None: + raise ValueError( + "path is a sensitive credential or internal Hermes path and cannot be attached" + ) + except ValueError: + raise + except Exception: + # Fail CLOSED on the security path. This guard exists specifically to + # cover credential stores the narrow list above misses (auth.json, + # .anthropic_oauth.json, mcp-tokens/, ...). If the canonical lookup + # ever fails, silently falling through would re-open that exact hole — + # the gateway feeds untrusted remote text here, so a probe could then + # attach the operator's keys. Refuse instead: a spurious block on a + # legitimate file is a recoverable annoyance; a leaked credential is not. + raise ValueError( + "path could not be verified against the credential deny-list and cannot be attached" + ) + def _strip_trailing_punctuation(value: str) -> str: stripped = value.rstrip(TRAILING_PUNCTUATION) diff --git a/scripts/release.py b/scripts/release.py index feec9a8a71d..feaa0ef9efa 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -1792,6 +1792,7 @@ AUTHOR_MAP = { "steveonjava@gmail.com": "steveonjava", # PR #29669 (redact secrets in kanban tool payloads) "afnlegion01@gmail.com": "Afnath-max", # PR #49129 salvage (opencode-zen catalog refresh + uncapped/live-first picker) "sharma.priyanshu96@gmail.com": "ipriyaaanshu", # PR #51488 salvage (clear stale base_url on gateway model switches; #25107) + "290881485+mrparker0980@users.noreply.github.com": "mrparker0980", # @file context-ref expansion anchored to canonical read deny-list } diff --git a/tests/agent/test_context_references.py b/tests/agent/test_context_references.py index 1afd5ee20b2..1fab3ef1b74 100644 --- a/tests/agent/test_context_references.py +++ b/tests/agent/test_context_references.py @@ -353,3 +353,95 @@ async def test_blocks_sensitive_home_and_hermes_paths(tmp_path: Path, monkeypatc assert "API_KEY=super-secret" not in result.message assert "PRIVATE-KEY" not in result.message assert any("sensitive credential" in warning for warning in result.warnings) + + +@pytest.mark.asyncio +async def test_blocks_canonical_read_denylist_credential_stores(tmp_path: Path, monkeypatch): + """@file expansion must honour the canonical read deny-list. + + The narrow in-module list historically missed the real credential stores + (provider keys, OAuth tokens, MCP tokens, project-local .env). Because the + gateway routes untrusted remote message text through reference expansion, + a chat peer could otherwise attach `@file:~/.hermes/auth.json` and read the + operator's keys into context. These must all be refused, with their secret + bodies kept out of the expanded message. + """ + from agent.context_references import preprocess_context_references_async + + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + + hermes_home = tmp_path / ".hermes" + (hermes_home).mkdir(parents=True) + + auth_json = hermes_home / "auth.json" + auth_json.write_text('{"openai": "sk-AUTHJSON-SECRET"}\n', encoding="utf-8") + + oauth = hermes_home / ".anthropic_oauth.json" + oauth.write_text('{"access_token": "OAUTH-SECRET"}\n', encoding="utf-8") + + mcp_token = hermes_home / "mcp-tokens" / "github.json" + mcp_token.parent.mkdir(parents=True) + mcp_token.write_text('{"token": "MCP-TOKEN-SECRET"}\n', encoding="utf-8") + + project_env = tmp_path / "project" / ".env" + project_env.parent.mkdir(parents=True) + project_env.write_text("DB_PASSWORD=ENV-SECRET\n", encoding="utf-8") + + result = await preprocess_context_references_async( + "inspect @file:.hermes/auth.json and @file:.hermes/.anthropic_oauth.json " + "and @file:.hermes/mcp-tokens/github.json and @file:project/.env", + cwd=tmp_path, + allowed_root=tmp_path, + context_length=100_000, + ) + + assert result.expanded + for secret in ( + "sk-AUTHJSON-SECRET", + "OAUTH-SECRET", + "MCP-TOKEN-SECRET", + "ENV-SECRET", + ): + assert secret not in result.message + assert sum("sensitive credential" in warning for warning in result.warnings) == 4 + + +@pytest.mark.asyncio +async def test_canonical_guard_fails_closed_when_lookup_raises(tmp_path: Path, monkeypatch): + """If the canonical read guard raises, the reference must fail CLOSED. + + The guard exists specifically to cover credential stores the narrow local + list misses (auth.json, ...). If get_read_block_error ever raised, silently + falling through to the local list would re-open that exact hole — and the + gateway feeds untrusted remote text here, so a chat peer could then attach + auth.json. The reference must be refused and the secret kept out of the + expanded message. + """ + from agent.context_references import preprocess_context_references_async + + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir(parents=True) + auth_json = hermes_home / "auth.json" + auth_json.write_text('{"openai": "sk-AUTHJSON-SECRET"}\n', encoding="utf-8") + + def _boom(_path): + raise RuntimeError("guard resolution failed") + + monkeypatch.setattr("agent.file_safety.get_read_block_error", _boom) + + result = await preprocess_context_references_async( + "inspect @file:.hermes/auth.json", + cwd=tmp_path, + allowed_root=tmp_path, + context_length=100_000, + ) + + assert "sk-AUTHJSON-SECRET" not in result.message + assert any( + "credential deny-list" in warning or "sensitive credential" in warning + for warning in result.warnings + )