mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(security): reject path traversal in credential file registration
This commit is contained in:
parent
0f2ea2062b
commit
a97641b9f2
2 changed files with 142 additions and 4 deletions
|
|
@ -197,3 +197,110 @@ class TestIterSkillsFiles:
|
|||
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
|
||||
assert iter_skills_files() == []
|
||||
|
||||
class TestPathTraversalSecurity:
|
||||
"""Path traversal and absolute path rejection.
|
||||
|
||||
A malicious skill could declare::
|
||||
|
||||
required_credential_files:
|
||||
- path: '../../.ssh/id_rsa'
|
||||
|
||||
Without containment checks, this would mount the host's SSH private key
|
||||
into the container sandbox, leaking it to the skill's execution environment.
|
||||
"""
|
||||
|
||||
def test_dotdot_traversal_rejected(self, tmp_path, monkeypatch):
|
||||
"""'../sensitive' must not escape HERMES_HOME."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
(tmp_path / ".hermes").mkdir()
|
||||
|
||||
# Create a sensitive file one level above hermes_home
|
||||
sensitive = tmp_path / "sensitive.json"
|
||||
sensitive.write_text('{"secret": "value"}')
|
||||
|
||||
result = register_credential_file("../sensitive.json")
|
||||
|
||||
assert result is False
|
||||
assert get_credential_file_mounts() == []
|
||||
|
||||
def test_deep_traversal_rejected(self, tmp_path, monkeypatch):
|
||||
"""'../../etc/passwd' style traversal must be rejected."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
# Create a fake sensitive file outside hermes_home
|
||||
ssh_dir = tmp_path / ".ssh"
|
||||
ssh_dir.mkdir()
|
||||
(ssh_dir / "id_rsa").write_text("PRIVATE KEY")
|
||||
|
||||
result = register_credential_file("../../.ssh/id_rsa")
|
||||
|
||||
assert result is False
|
||||
assert get_credential_file_mounts() == []
|
||||
|
||||
def test_absolute_path_rejected(self, tmp_path, monkeypatch):
|
||||
"""Absolute paths must be rejected regardless of whether they exist."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
# Create a file at an absolute path
|
||||
sensitive = tmp_path / "absolute.json"
|
||||
sensitive.write_text("{}")
|
||||
|
||||
result = register_credential_file(str(sensitive))
|
||||
|
||||
assert result is False
|
||||
assert get_credential_file_mounts() == []
|
||||
|
||||
def test_legitimate_file_still_works(self, tmp_path, monkeypatch):
|
||||
"""Normal files inside HERMES_HOME must still be registered."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
(hermes_home / "token.json").write_text('{"token": "abc"}')
|
||||
|
||||
result = register_credential_file("token.json")
|
||||
|
||||
assert result is True
|
||||
mounts = get_credential_file_mounts()
|
||||
assert len(mounts) == 1
|
||||
assert "token.json" in mounts[0]["container_path"]
|
||||
|
||||
def test_nested_subdir_inside_hermes_home_allowed(self, tmp_path, monkeypatch):
|
||||
"""Files in subdirectories of HERMES_HOME must be allowed."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
subdir = hermes_home / "creds"
|
||||
subdir.mkdir()
|
||||
(subdir / "oauth.json").write_text("{}")
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
result = register_credential_file("creds/oauth.json")
|
||||
|
||||
assert result is True
|
||||
|
||||
def test_symlink_traversal_rejected(self, tmp_path, monkeypatch):
|
||||
"""A symlink inside HERMES_HOME pointing outside must be rejected."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
# Create a sensitive file outside hermes_home
|
||||
sensitive = tmp_path / "sensitive.json"
|
||||
sensitive.write_text('{"secret": "value"}')
|
||||
|
||||
# Create a symlink inside hermes_home pointing outside
|
||||
symlink = hermes_home / "evil_link.json"
|
||||
try:
|
||||
symlink.symlink_to(sensitive)
|
||||
except (OSError, NotImplementedError):
|
||||
pytest.skip("Symlinks not supported on this platform")
|
||||
|
||||
result = register_credential_file("evil_link.json")
|
||||
|
||||
# The resolved path escapes HERMES_HOME — must be rejected
|
||||
assert result is False
|
||||
assert get_credential_file_mounts() == []
|
||||
|
|
|
|||
|
|
@ -55,16 +55,47 @@ def register_credential_file(
|
|||
|
||||
*relative_path* is relative to ``HERMES_HOME`` (e.g. ``google_token.json``).
|
||||
Returns True if the file exists on the host and was registered.
|
||||
|
||||
Security: rejects absolute paths and path traversal sequences (``..``).
|
||||
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.
|
||||
"""
|
||||
hermes_home = _resolve_hermes_home()
|
||||
|
||||
# Reject absolute paths — they bypass the HERMES_HOME sandbox entirely.
|
||||
if os.path.isabs(relative_path):
|
||||
logger.warning(
|
||||
"credential_files: rejected absolute path %r (must be relative to HERMES_HOME)",
|
||||
relative_path,
|
||||
)
|
||||
return False
|
||||
|
||||
host_path = hermes_home / relative_path
|
||||
if not host_path.is_file():
|
||||
logger.debug("credential_files: skipping %s (not found)", host_path)
|
||||
|
||||
# Resolve symlinks and normalise ``..`` before the containment check so
|
||||
# that traversal like ``../. ssh/id_rsa`` cannot escape HERMES_HOME.
|
||||
try:
|
||||
resolved = host_path.resolve()
|
||||
hermes_home_resolved = hermes_home.resolve()
|
||||
resolved.relative_to(hermes_home_resolved) # raises ValueError if outside
|
||||
except ValueError:
|
||||
logger.warning(
|
||||
"credential_files: rejected path traversal %r "
|
||||
"(resolves to %s, outside HERMES_HOME %s)",
|
||||
relative_path,
|
||||
resolved,
|
||||
hermes_home_resolved,
|
||||
)
|
||||
return False
|
||||
|
||||
if not resolved.is_file():
|
||||
logger.debug("credential_files: skipping %s (not found)", resolved)
|
||||
return False
|
||||
|
||||
container_path = f"{container_base.rstrip('/')}/{relative_path}"
|
||||
_registered_files[container_path] = str(host_path)
|
||||
logger.debug("credential_files: registered %s -> %s", host_path, container_path)
|
||||
_registered_files[container_path] = str(resolved)
|
||||
logger.debug("credential_files: registered %s -> %s", resolved, container_path)
|
||||
return True
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue