fix: apply same path traversal checks to config-based credential files
_load_config_files() had the same hermes_home / item pattern without containment checks. While config.yaml is user-controlled (lower threat than skill frontmatter), defense in depth prevents exploitation via config injection or copy-paste mistakes.
This commit is contained in:
@@ -304,3 +304,57 @@ class TestPathTraversalSecurity:
|
||||
# The resolved path escapes HERMES_HOME — must be rejected
|
||||
assert result is False
|
||||
assert get_credential_file_mounts() == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config-based credential files — same containment checks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestConfigPathTraversal:
|
||||
"""terminal.credential_files in config.yaml must also reject traversal."""
|
||||
|
||||
def _write_config(self, hermes_home: Path, cred_files: list):
|
||||
import yaml
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(yaml.dump({"terminal": {"credential_files": cred_files}}))
|
||||
|
||||
def test_config_traversal_rejected(self, tmp_path, monkeypatch):
|
||||
"""'../secret' in config.yaml must not escape HERMES_HOME."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
sensitive = tmp_path / "secret.json"
|
||||
sensitive.write_text("{}")
|
||||
self._write_config(hermes_home, ["../secret.json"])
|
||||
|
||||
mounts = get_credential_file_mounts()
|
||||
host_paths = [m["host_path"] for m in mounts]
|
||||
assert str(sensitive) not in host_paths
|
||||
assert str(sensitive.resolve()) not in host_paths
|
||||
|
||||
def test_config_absolute_path_rejected(self, tmp_path, monkeypatch):
|
||||
"""Absolute paths in config.yaml must be rejected."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
sensitive = tmp_path / "abs.json"
|
||||
sensitive.write_text("{}")
|
||||
self._write_config(hermes_home, [str(sensitive)])
|
||||
|
||||
mounts = get_credential_file_mounts()
|
||||
assert mounts == []
|
||||
|
||||
def test_config_legitimate_file_works(self, tmp_path, monkeypatch):
|
||||
"""Normal files inside HERMES_HOME via config must still mount."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
|
||||
(hermes_home / "oauth.json").write_text("{}")
|
||||
self._write_config(hermes_home, ["oauth.json"])
|
||||
|
||||
mounts = get_credential_file_mounts()
|
||||
assert len(mounts) == 1
|
||||
assert "oauth.json" in mounts[0]["container_path"]
|
||||
|
||||
@@ -141,11 +141,27 @@ def _load_config_files() -> List[Dict[str, str]]:
|
||||
cfg = yaml.safe_load(f) or {}
|
||||
cred_files = cfg.get("terminal", {}).get("credential_files")
|
||||
if isinstance(cred_files, list):
|
||||
hermes_home_resolved = hermes_home.resolve()
|
||||
for item in cred_files:
|
||||
if isinstance(item, str) and item.strip():
|
||||
host_path = hermes_home / item.strip()
|
||||
rel = item.strip()
|
||||
if os.path.isabs(rel):
|
||||
logger.warning(
|
||||
"credential_files: rejected absolute config path %r", rel,
|
||||
)
|
||||
continue
|
||||
host_path = (hermes_home / rel).resolve()
|
||||
try:
|
||||
host_path.relative_to(hermes_home_resolved)
|
||||
except ValueError:
|
||||
logger.warning(
|
||||
"credential_files: rejected config path traversal %r "
|
||||
"(resolves to %s, outside HERMES_HOME %s)",
|
||||
rel, host_path, hermes_home_resolved,
|
||||
)
|
||||
continue
|
||||
if host_path.is_file():
|
||||
container_path = f"/root/.hermes/{item.strip()}"
|
||||
container_path = f"/root/.hermes/{rel}"
|
||||
result.append({
|
||||
"host_path": str(host_path),
|
||||
"container_path": container_path,
|
||||
|
||||
Reference in New Issue
Block a user