diff --git a/tests/tools/test_credential_files.py b/tests/tools/test_credential_files.py index c46f73fae..b6e43d4a8 100644 --- a/tests/tools/test_credential_files.py +++ b/tests/tools/test_credential_files.py @@ -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() == [] diff --git a/tools/credential_files.py b/tools/credential_files.py index 53ddd79d5..95f068a81 100644 --- a/tools/credential_files.py +++ b/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