mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(credentials): never mount master credential stores into skill sandboxes
register_credential_file() takes a skill-declared relative path from
required_credential_files frontmatter and bind-mounts it read-only into the
remote sandbox the skill's own code runs in. It validates that the resolved
path stays inside HERMES_HOME — the docstring names the threat directly:
so that a malicious skill cannot declare
required_credential_files: ['../../.ssh/id_rsa'] and exfiltrate
sensitive host files into a container sandbox
Containment is the wrong boundary on its own, because HERMES_HOME is exactly
where the master credential stores live. Traversal is blocked; asking for the
keys by name is not:
skill declares mounted? agent may read it?
.env YES DENIED
auth.json YES DENIED
.anthropic_oauth.json YES DENIED
cache/bws_cache.json YES DENIED
mcp-tokens/srv.json YES DENIED
google_token.json YES allowed
../../.ssh/id_rsa no n/a
Every row marked DENIED is refused by the canonical read guard
(agent.file_safety.get_read_block_error) — the agent cannot read_file them —
yet one line of hub-installed skill frontmatter gets them bind-mounted where
that skill can cat them. .env alone is every provider API key.
Reuse the canonical deny-list as the mount bar: what the agent is forbidden
to read is not mountable either, so the mount surface cannot hand a skill
what the read surface denies it. Fails CLOSED — if the guard can't be
consulted the mount is refused rather than risked.
The module keeps doing its job: a skill still mounts its own service token
(google_token.json, skills/*), and a refused entry is reported back through
register_credential_files' missing list instead of failing the batch.
The three prior PRs here (#3946, #3951, #4316) all hardened traversal; this
closes the half that traversal validation never covered.
This commit is contained in:
parent
e77ffdc28c
commit
c8882c141c
2 changed files with 111 additions and 0 deletions
|
|
@ -530,3 +530,82 @@ class TestIterCacheFiles:
|
|||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
assert iter_cache_files() == []
|
||||
|
||||
|
||||
class TestMasterCredentialStoresAreNeverMountable:
|
||||
"""Containment is not enough — HERMES_HOME *is* where the keys live.
|
||||
|
||||
``required_credential_files`` is skill-declared frontmatter, and skills are
|
||||
installed from the hub. The traversal guard already stops
|
||||
``../../.ssh/id_rsa`` from escaping HERMES_HOME, but every master
|
||||
credential store sits *inside* it: a one-line declaration would otherwise
|
||||
bind-mount ``.env`` (every provider key) or ``auth.json`` (all provider
|
||||
tokens and OAuth grants) read-only into the sandbox the skill's own code
|
||||
runs in.
|
||||
|
||||
The bar is the canonical read deny-list: whatever the agent is forbidden to
|
||||
``read_file`` must not be mountable either, so the mount surface can't
|
||||
grant what the read surface denies.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _home(tmp_path):
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
(home / ".env").write_text("OPENAI_API_KEY=sk-proj-REAL\n")
|
||||
(home / "auth.json").write_text('{"providers":{}}')
|
||||
(home / ".anthropic_oauth.json").write_text('{"refresh_token":"rt"}')
|
||||
(home / "webhook_subscriptions.json").write_text("{}")
|
||||
(home / "cache").mkdir()
|
||||
(home / "cache" / "bws_cache.json").write_text("{}")
|
||||
(home / "mcp-tokens").mkdir()
|
||||
(home / "mcp-tokens" / "srv.json").write_text('{"access_token":"t"}')
|
||||
(home / "google_token.json").write_text("{}")
|
||||
return home
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"rel_path",
|
||||
[
|
||||
".env",
|
||||
"auth.json",
|
||||
".anthropic_oauth.json",
|
||||
"webhook_subscriptions.json",
|
||||
"cache/bws_cache.json",
|
||||
"mcp-tokens/srv.json",
|
||||
],
|
||||
)
|
||||
def test_master_credential_store_is_refused(self, tmp_path, rel_path):
|
||||
home = self._home(tmp_path)
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(home)}):
|
||||
assert register_credential_file(rel_path) is False, (
|
||||
f"{rel_path} would be bind-mounted into the sandbox"
|
||||
)
|
||||
assert get_credential_file_mounts() == []
|
||||
|
||||
def test_per_service_token_still_mounts(self, tmp_path):
|
||||
"""The module's legitimate purpose must keep working."""
|
||||
home = self._home(tmp_path)
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(home)}):
|
||||
assert register_credential_file("google_token.json") is True
|
||||
mounts = get_credential_file_mounts()
|
||||
assert [m["container_path"] for m in mounts] == [
|
||||
"/root/.hermes/google_token.json"
|
||||
]
|
||||
|
||||
def test_refused_entry_does_not_block_the_rest_of_the_batch(self, tmp_path):
|
||||
home = self._home(tmp_path)
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(home)}):
|
||||
missing = register_credential_files([".env", "google_token.json"])
|
||||
mounts = get_credential_file_mounts()
|
||||
|
||||
paths = [m["container_path"] for m in mounts]
|
||||
assert "/root/.hermes/google_token.json" in paths
|
||||
assert "/root/.hermes/.env" not in paths
|
||||
assert ".env" in missing, "a refused store is reported back to the skill"
|
||||
|
||||
def test_traversal_guard_still_applies(self, tmp_path):
|
||||
"""The pre-existing containment check is untouched."""
|
||||
home = self._home(tmp_path)
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(home)}):
|
||||
assert register_credential_file("../../.ssh/id_rsa") is False
|
||||
assert register_credential_file("/etc/passwd") is False
|
||||
|
|
|
|||
|
|
@ -67,6 +67,15 @@ def register_credential_file(
|
|||
The resolved host path must remain inside HERMES_HOME so that a malicious
|
||||
skill cannot declare ``required_credential_files: ['../../.ssh/id_rsa']``
|
||||
and exfiltrate sensitive host files into a container sandbox.
|
||||
|
||||
Containment alone is not sufficient, because HERMES_HOME is exactly where
|
||||
the MASTER credential stores live. A skill legitimately needs its own
|
||||
service token (``google_token.json``); it never needs ``.env`` (every
|
||||
provider key), ``auth.json`` (all provider tokens and OAuth grants),
|
||||
``mcp-tokens/`` or the Bitwarden plaintext cache. Those are refused via
|
||||
the canonical read deny-list (``agent.file_safety.get_read_block_error``)
|
||||
— the same guard that stops the agent reading them with ``read_file``, so
|
||||
the mount surface cannot hand a skill what the read surface denies it.
|
||||
"""
|
||||
hermes_home = _resolve_hermes_home()
|
||||
|
||||
|
|
@ -98,6 +107,29 @@ def register_credential_file(
|
|||
logger.debug("credential_files: skipping %s (not found)", resolved)
|
||||
return False
|
||||
|
||||
# Master credential stores are never mountable, even though they sit
|
||||
# inside HERMES_HOME and therefore pass the containment check above.
|
||||
# Fails CLOSED: if the canonical guard can't be consulted we refuse the
|
||||
# mount rather than risk bind-mounting auth.json into a sandbox.
|
||||
try:
|
||||
from agent.file_safety import get_read_block_error
|
||||
|
||||
denied = get_read_block_error(str(resolved))
|
||||
except Exception as exc: # pragma: no cover - core module
|
||||
logger.warning(
|
||||
"credential_files: refusing %r — read guard unavailable (%s)",
|
||||
relative_path, exc,
|
||||
)
|
||||
return False
|
||||
if denied:
|
||||
logger.warning(
|
||||
"credential_files: refused %r — it is a credential store the agent "
|
||||
"is denied from reading; a skill may mount its own service token, "
|
||||
"not the master key files",
|
||||
relative_path,
|
||||
)
|
||||
return False
|
||||
|
||||
container_path = f"{container_base.rstrip('/')}/{relative_path}"
|
||||
_get_registered()[container_path] = str(resolved)
|
||||
logger.debug("credential_files: registered %s -> %s", resolved, container_path)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue