mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(security): anchor @file context refs to canonical read deny-list
`@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
This commit is contained in:
parent
53b017f03e
commit
10a54ccc2c
3 changed files with 124 additions and 0 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue