mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-06-04 07:31:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

fix(docker): skip credential/skills/cache mounts when source is invalid (#24490) (#34331)

Browse source 
Salvages #24490 by @liuhao1024 against current main.

The Docker daemon will silently auto-create a directory at the host
path of any `-v <host>:<container>` bind mount when the host path
doesn't exist.  In Docker-in-Docker setups (where the outer host's
real credential file isn't visible inside the agent's parent
container), this leaves a directory at the credential mount source —
and the inner `docker run` then refuses to mount a directory over a
file destination with exit 125.

Add defensive shape guards to all three mount loops in
DockerEnvironment.__init__:

  * credentials (expected: file)  — skip + warn on directory or missing
  * skills      (expected: dir)   — skip + warn when not a directory
  * cache       (expected: dir)   — skip + warn when not a directory

Failed mounts surface as WARN logs rather than crashing the container
start.  Existing well-formed sources mount unchanged.

The original PR's branch was on a pre-container-reuse-rework base
(May 12) and conflicted with the post-May-28 driver work (label
tagging, container reuse, orphan reaper).  Reconstructed the same
intent on current main; the three guard blocks slot cleanly into
`tools/environments/docker.py` around the existing mount loops.

Three new tests pinned in `tests/tools/test_docker_environment.py`:
directory-source skip, missing-source skip, valid-file mounts.  Test-
first regression verification: reverted just the production code to
`origin/main` and confirmed the new tests fail with
`'deleted_token.json' is contained here: /root/.hermes/...` — the
fixed code makes them pass.  Full file passes (54/54).

Closes #24490

Co-authored-by: liuhao1024 <11816344+liuhao1024@users.noreply.github.com>
This commit is contained in:
Ben Barclay 2026-05-29 14:09:04 +10:00 • committed by GitHub
parent 69b74c15a3
commit 40fa0c1d19
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 148 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

117
tests/tools/test_docker_environment.py
View file

@ -1368,3 +1368,120 @@ def test_container_finished_at_returns_none_on_zero_value():
):
result = docker_env._container_finished_at("/usr/bin/docker", "never-finished")
assert result is None
def test_credential_mount_skipped_when_source_is_directory(monkeypatch, tmp_path, caplog):
"""Credential mount should be skipped when source path is a directory.
In Docker-in-Docker scenarios, Docker may auto-create the source path as
a directory when it doesn't exist on the host. Mounting a directory over
a file destination causes exit 125.
"""
# Create a directory that looks like a corrupted credential file path
corrupted_dir = tmp_path / "google_token.json"
corrupted_dir.mkdir()
monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
calls = _mock_subprocess_run(monkeypatch)
# Mock get_credential_file_mounts to return the corrupted entry
fake_mounts = [
{"host_path": str(corrupted_dir), "container_path": "/root/.hermes/google_token.json"},
]
monkeypatch.setattr(
"tools.credential_files.get_credential_file_mounts",
lambda: fake_mounts,
)
monkeypatch.setattr(
"tools.credential_files.get_skills_directory_mount",
lambda: [],
)
monkeypatch.setattr(
"tools.credential_files.get_cache_directory_mounts",
lambda: [],
)
with caplog.at_level(logging.WARNING):
_make_dummy_env()
# The corrupted mount should be skipped
run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"]
assert run_calls, "docker run should have been called"
run_args_str = " ".join(run_calls[0][0])
assert "google_token.json" not in run_args_str
# Should log a warning about the directory source
assert any(
"source is a directory" in rec.getMessage()
for rec in caplog.records
)
def test_credential_mount_skipped_when_source_missing(monkeypatch, tmp_path, caplog):
"""Credential mount should be skipped when source file no longer exists."""
missing_path = tmp_path / "deleted_token.json"
# Don't create the file — it's "missing"
monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
calls = _mock_subprocess_run(monkeypatch)
fake_mounts = [
{"host_path": str(missing_path), "container_path": "/root/.hermes/deleted_token.json"},
]
monkeypatch.setattr(
"tools.credential_files.get_credential_file_mounts",
lambda: fake_mounts,
)
monkeypatch.setattr(
"tools.credential_files.get_skills_directory_mount",
lambda: [],
)
monkeypatch.setattr(
"tools.credential_files.get_cache_directory_mounts",
lambda: [],
)
with caplog.at_level(logging.WARNING):
_make_dummy_env()
run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"]
assert run_calls, "docker run should have been called"
run_args_str = " ".join(run_calls[0][0])
assert "deleted_token.json" not in run_args_str
assert any(
"source not found" in rec.getMessage()
for rec in caplog.records
)
def test_credential_mount_works_when_source_is_valid_file(monkeypatch, tmp_path):
"""Credential mount should proceed normally when source is a valid file."""
valid_file = tmp_path / "token.json"
valid_file.write_text('{"token": "REDACTED"}')
monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
calls = _mock_subprocess_run(monkeypatch)
fake_mounts = [
{"host_path": str(valid_file), "container_path": "/root/.hermes/token.json"},
]
monkeypatch.setattr(
"tools.credential_files.get_credential_file_mounts",
lambda: fake_mounts,
)
monkeypatch.setattr(
"tools.credential_files.get_skills_directory_mount",
lambda: [],
)
monkeypatch.setattr(
"tools.credential_files.get_cache_directory_mounts",
lambda: [],
)
_make_dummy_env()
run_calls = [c for c in calls if isinstance(c[0], list) and len(c[0]) >= 2 and c[0][1] == "run"]
assert run_calls, "docker run should have been called"
run_args_str = " ".join(run_calls[0][0])
assert "token.json" in run_args_str

31
tools/environments/docker.py
View file

@ -12,6 +12,7 @@ import shutil
import subprocess
import sys
import uuid
from pathlib import Path
from typing import Optional
from tools.environments.base import BaseEnvironment, _popen_bash
@ -577,6 +578,22 @@ class DockerEnvironment(BaseEnvironment):
)
for mount_entry in get_credential_file_mounts():
src = Path(mount_entry["host_path"])
if src.is_dir():
# Docker-in-Docker: Docker auto-created the source path as
# a directory when it didn't exist on the host. Mounting a
# directory over a file destination causes exit 125.
logger.warning(
"Docker: skipping credential mount — source is a directory "
"(likely Docker-in-Docker auto-creation): %s",
src,
)
continue
if not src.is_file():
logger.warning(
"Docker: skipping credential mount — source not found: %s", src,
)
continue
volume_args.extend([
"-v",
f"{mount_entry['host_path']}:{mount_entry['container_path']}:ro",
@ -590,6 +607,13 @@ class DockerEnvironment(BaseEnvironment):
# Mount skill directories (local + external) so skill
# scripts/templates are available inside the container.
for skills_mount in get_skills_directory_mount():
src = Path(skills_mount["host_path"])
if not src.is_dir():
logger.warning(
"Docker: skipping skills mount — source is not a directory: %s",
src,
)
continue
volume_args.extend([
"-v",
f"{skills_mount['host_path']}:{skills_mount['container_path']}:ro",
@ -605,6 +629,13 @@ class DockerEnvironment(BaseEnvironment):
# cached media from inside the container. Read-only — the
# container reads these but the host gateway manages writes.
for cache_mount in get_cache_directory_mounts():
src = Path(cache_mount["host_path"])
if not src.is_dir():
logger.warning(
"Docker: skipping cache mount — source is not a directory: %s",
src,
)
continue
volume_args.extend([
"-v",
f"{cache_mount['host_path']}:{cache_mount['container_path']}:ro",