fix(security): reject path traversal in credential file registration

Author: maymuneth

tests/tools/test_credential_files.py

@@ -197,3 +197,110 @@ def test_empty_when_no_skills_dir(self, tmp_path):
 
         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() == []

tools/credential_files.py

@@ -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