From c2a3b7fcf559bc6191dc5ac5a82a226bd37a4bac Mon Sep 17 00:00:00 2001 From: server Date: Sat, 16 May 2026 23:09:30 -0700 Subject: [PATCH 1/2] [verified] fix(gateway): map Docker MEDIA paths to host Resolve outbound MEDIA file paths emitted from Docker terminal sandboxes by translating configured bind-mounted container paths back to host-visible paths before gateway delivery. Add regression coverage for mapped paths, common Docker volume options, path-boundary handling, existing host-visible files, and malformed volume config. --- gateway/media_paths.py | 185 ++++++++++++++++++ gateway/platforms/base.py | 3 +- gateway/platforms/telegram.py | 4 +- gateway/run.py | 26 +-- hermes_cli/config.py | 4 +- tests/gateway/test_runner_startup_failures.py | 29 +++ tests/gateway/test_send_image_file.py | 147 ++++++++++++++ website/docs/user-guide/configuration.md | 11 +- website/docs/user-guide/messaging/telegram.md | 8 +- 9 files changed, 380 insertions(+), 37 deletions(-) create mode 100644 gateway/media_paths.py diff --git a/gateway/media_paths.py b/gateway/media_paths.py new file mode 100644 index 000000000000..a9b8bbb95c7a --- /dev/null +++ b/gateway/media_paths.py @@ -0,0 +1,185 @@ +"""Path helpers for gateway MEDIA file delivery.""" + +from __future__ import annotations + +import json +import logging +import os +import posixpath +from typing import Iterable + +logger = logging.getLogger(__name__) + + +MEDIA_EXPORT_CONTAINER_ROOTS = ("/output", "/outputs") + + +def parse_docker_volume_spec(spec: str) -> tuple[str, str] | None: + """Parse a Docker ``-v`` bind mount as ``(host_path, container_path)``. + + Hermes stores ``terminal.docker_volumes`` in ``TERMINAL_DOCKER_VOLUMES`` as + JSON strings like ``/host/exports:/output[:options]``. Docker's optional + third field can be ``ro``, ``rw``, ``cached``, ``delegated``, propagation + flags, SELinux flags, or comma-separated combinations. Named volumes and + malformed entries are ignored because the host gateway cannot derive a + readable filesystem path from them. + """ + if not isinstance(spec, str): + return None + + raw = spec.strip() + if not raw: + return None + + parts = raw.split(":") + if len(parts) < 2: + return None + + if parts[-1].strip().startswith("/"): + # host:container with no options. Join earlier parts so unusual but + # valid POSIX host paths containing ':' still round-trip. + host_path = ":".join(parts[:-1]) + container_path = parts[-1] + elif len(parts) >= 3 and parts[-2].strip().startswith("/"): + # host:container:options. The options field is intentionally not + # enumerated: Docker supports more than ro/rw, and unknown options do + # not change the host/container path relationship. + host_path = ":".join(parts[:-2]) + container_path = parts[-2] + else: + return None + + if not host_path or not container_path: + return None + + host_path = os.path.expandvars(os.path.expanduser(host_path.strip())) + container_path = posixpath.normpath(container_path.strip()) + + if not os.path.isabs(host_path) or not container_path.startswith("/"): + return None + + return os.path.normpath(host_path), container_path + + +def docker_bind_mounts_from_env(raw_volumes: str | None = None) -> tuple[tuple[str, str], ...]: + """Return configured Docker bind mounts from ``TERMINAL_DOCKER_VOLUMES``. + + Invalid or missing environment values return an empty tuple so gateway + response post-processing can continue with the original MEDIA path. + """ + raw = os.getenv("TERMINAL_DOCKER_VOLUMES", "") if raw_volumes is None else raw_volumes + raw = str(raw or "").strip() + if not raw: + return () + + try: + parsed = json.loads(raw) + except Exception: + logger.debug("Could not parse TERMINAL_DOCKER_VOLUMES", exc_info=True) + return () + + if not isinstance(parsed, list): + return () + + mounts = [] + for entry in parsed: + mount = parse_docker_volume_spec(entry) + if mount: + mounts.append(mount) + return tuple(mounts) + + +def _container_path_is_within(path: str, root: str) -> bool: + if root == "/": + return path.startswith("/") + return path == root or path.startswith(f"{root}/") + + +def is_media_export_mount( + container_root: str, + *, + allowed_roots: Iterable[str] = MEDIA_EXPORT_CONTAINER_ROOTS, +) -> bool: + """Return whether a container mount root is intended for MEDIA exports.""" + container_root = posixpath.normpath(container_root) + if container_root == "/": + return False + + for root in allowed_roots: + root = posixpath.normpath(str(root)) + if root.startswith("/") and _container_path_is_within(container_root, root): + return True + return False + + +def docker_media_bind_mounts_from_env(raw_volumes: str | None = None) -> tuple[tuple[str, str], ...]: + """Return Docker bind mounts eligible for outbound MEDIA path mapping.""" + return tuple( + (host_root, container_root) + for host_root, container_root in docker_bind_mounts_from_env(raw_volumes) + if is_media_export_mount(container_root) + ) + + +def _translate_docker_path_to_host( + path: str, + mounts: Iterable[tuple[str, str]], +) -> str | None: + container_path = posixpath.normpath(path) + if not container_path.startswith("/"): + return None + + best_mount = None + for host_root, container_root in mounts: + if not _container_path_is_within(container_path, container_root): + continue + if best_mount is None or len(container_root) > len(best_mount[1]): + best_mount = (host_root, container_root) + + if best_mount is None: + return None + + host_root, container_root = best_mount + rel_path = posixpath.relpath(container_path, container_root) + if rel_path == ".": + return host_root + return os.path.normpath(os.path.join(host_root, *rel_path.split("/"))) + + +def resolve_outbound_media_path(path: str) -> str: + """Return a host-readable path for an extracted ``MEDIA:`` directive. + + The gateway delivers files from the host process, but terminal tools can run + in a Docker sandbox. When Docker is the active backend and the MEDIA path is + inside a configured Docker bind mount, this maps the container path to the + corresponding host path. The automatic mapping is intentionally limited to + explicit MEDIA export mounts under ``/output`` or ``/outputs``; other Docker + bind mounts continue to behave like unmapped paths. Host-visible paths + outside those mapped export roots, non-Docker backends, malformed volume + config, and unmapped paths fall back to the expanded original path without + raising. + """ + try: + expanded = os.path.expanduser(path) + + if os.getenv("TERMINAL_ENV", "local").strip().lower() == "docker": + translated = _translate_docker_path_to_host( + expanded, + docker_media_bind_mounts_from_env(), + ) + if translated: + return translated + + try: + if os.path.exists(expanded): + return expanded + except OSError: + pass + + return expanded + except Exception: + logger.debug("Could not resolve outbound MEDIA path", exc_info=True) + try: + return os.path.expanduser(path) + except Exception: + return path diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 96b56d29cc7d..d343be614a53 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -19,6 +19,7 @@ from abc import ABC, abstractmethod from urllib.parse import urlsplit +from gateway.media_paths import resolve_outbound_media_path from utils import normalize_proxy_url logger = logging.getLogger(__name__) @@ -2145,7 +2146,7 @@ def extract_media(content: str) -> Tuple[List[Tuple[str, bool]], str]: path = path[1:-1].strip() path = path.lstrip("`\"'").rstrip("`\"',.;:)}]") if path: - media.append((os.path.expanduser(path), has_voice_tag)) + media.append((resolve_outbound_media_path(path), has_voice_tag)) # Remove MEDIA tags from content (including surrounding quote/backtick wrappers) if media: diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index d893b8115cf4..77365da0d661 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -2955,8 +2955,8 @@ def _missing_media_path_error(self, label: str, path: str) -> str: if path.startswith(("/workspace/", "/output/", "/outputs/")): error += ( " (path may only exist inside the Docker sandbox. " - "Bind-mount a host directory and emit the host-visible " - "path in MEDIA: for gateway file delivery.)" + "Bind-mount a host directory for that container path or " + "emit a host-visible path in MEDIA: for gateway file delivery.)" ) return error diff --git a/gateway/run.py b/gateway/run.py index 818bd282ddbf..3c58fb1fabf9 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -52,6 +52,7 @@ from agent.account_usage import fetch_account_usage, render_account_usage_lines from agent.async_utils import safe_schedule_threadsafe from agent.i18n import t +from gateway.media_paths import docker_media_bind_mounts_from_env from hermes_cli.config import cfg_get # --- Agent cache tuning --------------------------------------------------- @@ -418,9 +419,6 @@ def _reload_runtime_env_preserving_config_authority() -> None: os.environ["HERMES_MAX_ITERATIONS"] = str(agent_cfg["max_turns"]) -_DOCKER_VOLUME_SPEC_RE = re.compile(r"^(?P.+):(?P/[^:]+?)(?::(?P[^:]+))?$") -_DOCKER_MEDIA_OUTPUT_CONTAINER_PATHS = {"/output", "/outputs"} - # Bridge config.yaml values into the environment so os.getenv() picks them up. # config.yaml is authoritative for terminal settings — overrides .env. _config_path = _hermes_home / 'config.yaml' @@ -1436,27 +1434,7 @@ def _warn_if_docker_media_delivery_is_risky(self) -> None: if not messaging_platforms: return - raw_volumes = os.getenv("TERMINAL_DOCKER_VOLUMES", "").strip() - volumes: List[str] = [] - if raw_volumes: - try: - parsed = json.loads(raw_volumes) - if isinstance(parsed, list): - volumes = [str(v) for v in parsed if isinstance(v, str)] - except Exception: - logger.debug("Could not parse TERMINAL_DOCKER_VOLUMES for gateway media warning", exc_info=True) - - has_explicit_output_mount = False - for spec in volumes: - match = _DOCKER_VOLUME_SPEC_RE.match(spec) - if not match: - continue - container_path = match.group("container") - if container_path in _DOCKER_MEDIA_OUTPUT_CONTAINER_PATHS: - has_explicit_output_mount = True - break - - if has_explicit_output_mount: + if docker_media_bind_mounts_from_env(): return logger.warning( diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 3f9bdd69ed4d..d0c663fac126 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -612,8 +612,8 @@ def _ensure_hermes_home_managed(home: Path): # Example: # ["/home/user/projects:/workspace/projects", # "/home/user/.hermes/cache/documents:/output"] - # For gateway MEDIA delivery, write inside Docker to /output/... and emit - # the host-visible path in MEDIA:, not the container path. + # Gateway MEDIA delivery accepts host paths or mounted export paths + # under /output or /outputs, such as MEDIA:/output/report.txt. "docker_volumes": [], # Explicit opt-in: mount the host cwd into /workspace for Docker sessions. # Default off because passing host directories into a sandbox weakens isolation. diff --git a/tests/gateway/test_runner_startup_failures.py b/tests/gateway/test_runner_startup_failures.py index 438553f34edb..74b3fa96f8b4 100644 --- a/tests/gateway/test_runner_startup_failures.py +++ b/tests/gateway/test_runner_startup_failures.py @@ -407,3 +407,32 @@ def test_runner_warns_when_docker_gateway_lacks_explicit_output_mount(monkeypatc "host-visible output mount" in record.message for record in caplog.records ) + + +@pytest.mark.parametrize("container_path", ["/output", "/outputs", "/output/reports"]) +def test_runner_accepts_explicit_docker_media_export_mount( + monkeypatch, + tmp_path, + caplog, + container_path, +): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + f'["{tmp_path}/exports:{container_path}:rw"]', + ) + config = GatewayConfig( + platforms={ + Platform.TELEGRAM: PlatformConfig(enabled=True, token="***") + }, + sessions_dir=tmp_path / "sessions", + ) + + with caplog.at_level("WARNING"): + GatewayRunner(config) + + assert not any( + "host-visible output mount" in record.message + for record in caplog.records + ) diff --git a/tests/gateway/test_send_image_file.py b/tests/gateway/test_send_image_file.py index cb0e436739ed..74af6f412ca1 100644 --- a/tests/gateway/test_send_image_file.py +++ b/tests/gateway/test_send_image_file.py @@ -7,6 +7,7 @@ """ import asyncio +import json import os import sys from unittest.mock import AsyncMock, MagicMock, patch @@ -57,6 +58,152 @@ def test_mixed_audio_and_image(self): assert "/audio.ogg" in paths assert "/screenshot.png" in paths + def test_docker_container_media_path_translates_to_host_volume(self, monkeypatch, tmp_path): + """Docker MEDIA paths should be translated before the host gateway sends them.""" + host_output = tmp_path / "gateway-output" + expected = host_output / "reports" / "daily.pdf" + expected.parent.mkdir(parents=True) + expected.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{host_output}:/output:rw"]), + ) + + media, cleaned = BasePlatformAdapter.extract_media("Done\nMEDIA:/output/reports/daily.pdf") + + assert media == [(str(expected), False)] + assert "MEDIA:" not in cleaned + assert "Done" in cleaned + + def test_docker_media_path_prefers_export_mount_over_host_output_path( + self, + monkeypatch, + tmp_path, + ): + """In Docker, /output should mean the configured export mount first.""" + host_output = tmp_path / "gateway-output" + expected = host_output / "report.pdf" + expected.parent.mkdir(parents=True) + expected.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{host_output}:/output:rw"]), + ) + + with patch("gateway.media_paths.os.path.exists", return_value=True): + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + + assert media == [(str(expected), False)] + + @pytest.mark.parametrize("options", ["cached", "delegated", "rw,z", "ro,Z"]) + def test_docker_container_media_path_translates_common_volume_options( + self, monkeypatch, tmp_path, options + ): + """Common Docker option suffixes should not prevent MEDIA path mapping.""" + host_output = tmp_path / "gateway-output" + expected = host_output / "report.pdf" + expected.parent.mkdir(parents=True) + expected.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{host_output}:/output:{options}"]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + + assert media == [(str(expected), False)] + + def test_docker_media_path_translation_requires_path_boundary(self, monkeypatch, tmp_path): + """A /output mount must not rewrite unrelated /output-other paths.""" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{tmp_path}:/output"]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output-other/report.pdf") + + assert media == [("/output-other/report.pdf", False)] + + def test_docker_media_path_translates_output_submount(self, monkeypatch, tmp_path): + """Explicit export submounts under /output should also be eligible.""" + host_output = tmp_path / "reports" + expected = host_output / "daily.pdf" + expected.parent.mkdir(parents=True) + expected.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{host_output}:/output/reports:rw"]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/reports/daily.pdf") + + assert media == [(str(expected), False)] + + def test_docker_media_path_ignores_non_export_mount(self, monkeypatch, tmp_path): + """Only explicit /output or /outputs export mounts should be auto-mapped.""" + workspace_report = tmp_path / "workspace" / "report.pdf" + workspace_report.parent.mkdir(parents=True) + workspace_report.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{workspace_report.parent}:/workspace"]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/workspace/report.pdf") + + assert media == [("/workspace/report.pdf", False)] + + def test_docker_media_path_ignores_root_mount(self, monkeypatch, tmp_path): + """A root bind mount should not make every container path media-sendable.""" + root_report = tmp_path / "output" / "report.pdf" + root_report.parent.mkdir(parents=True) + root_report.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{tmp_path}:/"]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + + assert media == [("/output/report.pdf", False)] + + def test_docker_media_path_keeps_existing_host_file(self, monkeypatch, tmp_path): + """Already host-visible paths should not be rewritten by Docker volume rules.""" + host_file = tmp_path / "already-visible.pdf" + host_file.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([f"{tmp_path / 'exports'}:/output"]), + ) + + media, _ = BasePlatformAdapter.extract_media(f"MEDIA:{host_file}") + + assert media == [(str(host_file), False)] + + def test_docker_media_path_invalid_volume_env_falls_back(self, monkeypatch): + """Malformed volume env should not prevent MEDIA extraction or sending fallback.""" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", "not-json") + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + + assert media == [("/output/report.pdf", False)] + # --------------------------------------------------------------------------- # Telegram send_image_file tests diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index d972b38b3848..ff1a380a1af1 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -343,11 +343,12 @@ If you use a messaging gateway and want the agent to send generated files via `MEDIA:/...`, prefer a dedicated host-visible export mount such as `/home/user/.hermes/cache/documents:/output`. -- Write files inside Docker to `/output/...` -- Emit the **host path** in `MEDIA:`, for example: - `MEDIA:/home/user/.hermes/cache/documents/report.txt` -- Do **not** emit `/workspace/...` or `/output/...` unless that exact path also - exists for the gateway process on the host +- Write files inside Docker to `/output/...` or `/outputs/...` +- Emit either the host path or the mounted container path, for example + `MEDIA:/output/report.txt` +- The gateway maps mounted `/output` or `/outputs` container paths back through + `docker_volumes` +- Do **not** emit unmapped container-only paths like `/workspace/...` :::warning YAML duplicate keys silently override earlier ones. If you already have a diff --git a/website/docs/user-guide/messaging/telegram.md b/website/docs/user-guide/messaging/telegram.md index 95d9313c05e6..84f9bd37fa85 100644 --- a/website/docs/user-guide/messaging/telegram.md +++ b/website/docs/user-guide/messaging/telegram.md @@ -137,9 +137,11 @@ terminal: Then: -- write files inside Docker to `/output/...` -- emit the **host-visible** path in `MEDIA:`, for example: - `MEDIA:/home/user/.hermes/cache/documents/report.txt` +- write files inside Docker to `/output/...` or `/outputs/...` +- emit either the host path or the mounted container path, for example + `MEDIA:/output/report.txt` +- the gateway maps mounted `/output` or `/outputs` container paths back through + `docker_volumes` If you already have a `docker_volumes:` section, add the new mount to the same list. YAML duplicate keys silently override earlier ones. From e0faa47dec7930057a75881129806e9e20490955 Mon Sep 17 00:00:00 2001 From: server Date: Sun, 17 May 2026 04:55:12 -0700 Subject: [PATCH 2/2] fix(gateway): derive Docker MEDIA paths from bind mounts --- gateway/media_paths.py | 72 ++++----- gateway/platforms/telegram.py | 11 +- gateway/run.py | 23 +-- hermes_cli/config.py | 7 +- tests/gateway/test_runner_startup_failures.py | 12 +- tests/gateway/test_send_image_file.py | 137 +++++++++++++++--- website/docs/user-guide/configuration.md | 22 +-- website/docs/user-guide/messaging/telegram.md | 13 +- 8 files changed, 203 insertions(+), 94 deletions(-) diff --git a/gateway/media_paths.py b/gateway/media_paths.py index a9b8bbb95c7a..c230c192aeb5 100644 --- a/gateway/media_paths.py +++ b/gateway/media_paths.py @@ -11,18 +11,15 @@ logger = logging.getLogger(__name__) -MEDIA_EXPORT_CONTAINER_ROOTS = ("/output", "/outputs") - - def parse_docker_volume_spec(spec: str) -> tuple[str, str] | None: """Parse a Docker ``-v`` bind mount as ``(host_path, container_path)``. Hermes stores ``terminal.docker_volumes`` in ``TERMINAL_DOCKER_VOLUMES`` as - JSON strings like ``/host/exports:/output[:options]``. Docker's optional - third field can be ``ro``, ``rw``, ``cached``, ``delegated``, propagation - flags, SELinux flags, or comma-separated combinations. Named volumes and - malformed entries are ignored because the host gateway cannot derive a - readable filesystem path from them. + JSON strings like ``/host/exports:/container/path[:options]``. Docker's + optional third field can be ``ro``, ``rw``, ``cached``, ``delegated``, + propagation flags, SELinux flags, or comma-separated combinations. Named + volumes and malformed entries are ignored because the host gateway cannot + derive a readable filesystem path from them. """ if not isinstance(spec, str): return None @@ -95,21 +92,18 @@ def _container_path_is_within(path: str, root: str) -> bool: return path == root or path.startswith(f"{root}/") -def is_media_export_mount( - container_root: str, - *, - allowed_roots: Iterable[str] = MEDIA_EXPORT_CONTAINER_ROOTS, -) -> bool: - """Return whether a container mount root is intended for MEDIA exports.""" - container_root = posixpath.normpath(container_root) - if container_root == "/": - return False +def is_docker_media_bind_mount(container_root: str) -> bool: + """Return whether a Docker bind mount can map outbound MEDIA paths. - for root in allowed_roots: - root = posixpath.normpath(str(root)) - if root.startswith("/") and _container_path_is_within(container_root, root): - return True - return False + The user chooses the export path through ``terminal.docker_volumes`` / + ``TERMINAL_DOCKER_VOLUMES``. Do not hard-code a container directory such as + ``/output``; any explicit non-root bind mount is mappable because the host + side is user-specified and readable by the gateway. Root mounts are still + ignored because they would make every absolute container path look + host-readable. + """ + container_root = posixpath.normpath(str(container_root or "")) + return container_root.startswith("/") and container_root != "/" def docker_media_bind_mounts_from_env(raw_volumes: str | None = None) -> tuple[tuple[str, str], ...]: @@ -117,10 +111,20 @@ def docker_media_bind_mounts_from_env(raw_volumes: str | None = None) -> tuple[t return tuple( (host_root, container_root) for host_root, container_root in docker_bind_mounts_from_env(raw_volumes) - if is_media_export_mount(container_root) + if is_docker_media_bind_mount(container_root) ) +def _host_path_is_within_root(path: str, root: str) -> bool: + """Return True when ``path`` resolves inside ``root`` on the host FS.""" + try: + real_root = os.path.normcase(os.path.realpath(os.path.abspath(root))) + real_path = os.path.normcase(os.path.realpath(os.path.abspath(path))) + return os.path.commonpath([real_root, real_path]) == real_root + except (OSError, ValueError): + return False + + def _translate_docker_path_to_host( path: str, mounts: Iterable[tuple[str, str]], @@ -141,9 +145,12 @@ def _translate_docker_path_to_host( host_root, container_root = best_mount rel_path = posixpath.relpath(container_path, container_root) - if rel_path == ".": - return host_root - return os.path.normpath(os.path.join(host_root, *rel_path.split("/"))) + candidate = host_root if rel_path == "." else os.path.normpath( + os.path.join(host_root, *rel_path.split("/")) + ) + if not _host_path_is_within_root(candidate, host_root): + return None + return candidate def resolve_outbound_media_path(path: str) -> str: @@ -151,13 +158,12 @@ def resolve_outbound_media_path(path: str) -> str: The gateway delivers files from the host process, but terminal tools can run in a Docker sandbox. When Docker is the active backend and the MEDIA path is - inside a configured Docker bind mount, this maps the container path to the - corresponding host path. The automatic mapping is intentionally limited to - explicit MEDIA export mounts under ``/output`` or ``/outputs``; other Docker - bind mounts continue to behave like unmapped paths. Host-visible paths - outside those mapped export roots, non-Docker backends, malformed volume - config, and unmapped paths fall back to the expanded original path without - raising. + inside a user-configured Docker bind mount, this maps the container path to + the corresponding host path. The mapping is derived from + ``TERMINAL_DOCKER_VOLUMES`` / ``terminal.docker_volumes``; it is not tied to + a hard-coded container directory such as ``/output``. Named volumes, + malformed volume config, root container mounts, non-Docker backends, and + unmapped paths fall back to the expanded original path without raising. """ try: expanded = os.path.expanduser(path) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 77365da0d661..3f8e7329d1d0 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -2948,11 +2948,16 @@ async def _handle_callback_query( def _missing_media_path_error(self, label: str, path: str) -> str: """Build an actionable file-not-found error for gateway MEDIA delivery. - Paths like /workspace/... or /output/... often only exist inside the - Docker sandbox, while the gateway process runs on the host. + Paths under Docker bind mounts may use arbitrary user-chosen container + roots (for example /workspace/..., /output/..., or /agent-artifacts/...), + while the gateway process runs on the host. """ error = f"{label} file not found: {path}" - if path.startswith(("/workspace/", "/output/", "/outputs/")): + looks_like_container_path = path.startswith( + ("/workspace/", "/output/", "/outputs/") + ) + docker_backend = os.getenv("TERMINAL_ENV", "").strip().lower() == "docker" + if looks_like_container_path or (path.startswith("/") and docker_backend): error += ( " (path may only exist inside the Docker sandbox. " "Bind-mount a host directory for that container path or " diff --git a/gateway/run.py b/gateway/run.py index 3c58fb1fabf9..acb953135019 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1418,13 +1418,14 @@ def _wire_teams_pipeline_runtime(self) -> None: def _warn_if_docker_media_delivery_is_risky(self) -> None: - """Warn when Docker-backed gateways lack an explicit export mount. - - MEDIA delivery happens in the gateway process, so paths emitted by the model - must be readable from the host. A plain container-local path like - `/workspace/report.txt` or `/output/report.txt` often exists only inside - Docker, so users commonly need a dedicated export mount such as - `host-dir:/output`. + """Warn when Docker-backed gateways lack host-readable bind mounts. + + MEDIA delivery happens in the gateway process, so paths emitted by the + model must be readable from the host. A plain container-local path like + `/workspace/report.txt` or `/artifacts/report.txt` often exists only + inside Docker, so users commonly need a dedicated bind mount such as + `host-dir:/artifacts`. The container path is user-chosen through + ``terminal.docker_volumes`` / ``TERMINAL_DOCKER_VOLUMES``. """ if os.getenv("TERMINAL_ENV", "").strip().lower() != "docker": return @@ -1439,9 +1440,11 @@ def _warn_if_docker_media_delivery_is_risky(self) -> None: logger.warning( "Docker backend is enabled for the messaging gateway but no explicit host-visible " - "output mount (for example '/home/user/.hermes/cache/documents:/output') is configured. " - "This is fine if the model already emits host-visible paths, but MEDIA file delivery can fail " - "for container-local paths like '/workspace/...' or '/output/...'." + "Docker bind mount is configured. This is fine if the model already emits " + "host-visible paths, but MEDIA file delivery can fail for container-local paths " + "like '/workspace/...' or '/artifacts/...'. Configure terminal.docker_volumes " + "with a host_path:container_path bind mount and emit MEDIA: paths under that " + "container_path." ) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index d0c663fac126..3e29d4ee4889 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -611,9 +611,10 @@ def _ensure_hermes_home_managed(home: Path): # Each entry is "host_path:container_path" (standard Docker -v syntax). # Example: # ["/home/user/projects:/workspace/projects", - # "/home/user/.hermes/cache/documents:/output"] - # Gateway MEDIA delivery accepts host paths or mounted export paths - # under /output or /outputs, such as MEDIA:/output/report.txt. + # "/home/user/.hermes/cache/documents:/agent-artifacts"] + # Gateway MEDIA delivery accepts host paths or mounted container paths + # derived from this list, such as MEDIA:/agent-artifacts/report.txt for + # the example above (or any other non-root container_path you choose). "docker_volumes": [], # Explicit opt-in: mount the host cwd into /workspace for Docker sessions. # Default off because passing host directories into a sandbox weakens isolation. diff --git a/tests/gateway/test_runner_startup_failures.py b/tests/gateway/test_runner_startup_failures.py index 74b3fa96f8b4..24ffa6944f09 100644 --- a/tests/gateway/test_runner_startup_failures.py +++ b/tests/gateway/test_runner_startup_failures.py @@ -389,10 +389,10 @@ async def test_runner_degrades_gracefully_when_all_adapters_missing(monkeypatch, ), "Expected degraded-mode warning when all adapters are missing" -def test_runner_warns_when_docker_gateway_lacks_explicit_output_mount(monkeypatch, tmp_path, caplog): +def test_runner_warns_when_docker_gateway_lacks_host_visible_bind_mount(monkeypatch, tmp_path, caplog): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) monkeypatch.setenv("TERMINAL_ENV", "docker") - monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", '["/etc/localtime:/etc/localtime:ro"]') + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", '["exports:/agent-media:rw"]') config = GatewayConfig( platforms={ Platform.TELEGRAM: PlatformConfig(enabled=True, token="***") @@ -404,13 +404,13 @@ def test_runner_warns_when_docker_gateway_lacks_explicit_output_mount(monkeypatc GatewayRunner(config) assert any( - "host-visible output mount" in record.message + "host-visible Docker bind mount" in record.message for record in caplog.records ) -@pytest.mark.parametrize("container_path", ["/output", "/outputs", "/output/reports"]) -def test_runner_accepts_explicit_docker_media_export_mount( +@pytest.mark.parametrize("container_path", ["/output", "/outputs", "/output/reports", "/agent-artifacts"]) +def test_runner_accepts_user_specified_docker_media_bind_mount( monkeypatch, tmp_path, caplog, @@ -433,6 +433,6 @@ def test_runner_accepts_explicit_docker_media_export_mount( GatewayRunner(config) assert not any( - "host-visible output mount" in record.message + "host-visible Docker bind mount" in record.message for record in caplog.records ) diff --git a/tests/gateway/test_send_image_file.py b/tests/gateway/test_send_image_file.py index 74af6f412ca1..1ddf3b55e227 100644 --- a/tests/gateway/test_send_image_file.py +++ b/tests/gateway/test_send_image_file.py @@ -59,8 +59,9 @@ def test_mixed_audio_and_image(self): assert "/screenshot.png" in paths def test_docker_container_media_path_translates_to_host_volume(self, monkeypatch, tmp_path): - """Docker MEDIA paths should be translated before the host gateway sends them.""" + """Docker MEDIA paths should follow the user-specified container mount.""" host_output = tmp_path / "gateway-output" + container_output = "/agent-artifacts" expected = host_output / "reports" / "daily.pdf" expected.parent.mkdir(parents=True) expected.write_bytes(b"pdf") @@ -68,22 +69,25 @@ def test_docker_container_media_path_translates_to_host_volume(self, monkeypatch monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{host_output}:/output:rw"]), + json.dumps([f"{host_output}:{container_output}:rw"]), ) - media, cleaned = BasePlatformAdapter.extract_media("Done\nMEDIA:/output/reports/daily.pdf") + media, cleaned = BasePlatformAdapter.extract_media( + f"Done\nMEDIA:{container_output}/reports/daily.pdf" + ) assert media == [(str(expected), False)] assert "MEDIA:" not in cleaned assert "Done" in cleaned - def test_docker_media_path_prefers_export_mount_over_host_output_path( + def test_docker_media_path_prefers_bind_mount_over_host_path( self, monkeypatch, tmp_path, ): - """In Docker, /output should mean the configured export mount first.""" + """In Docker, a matched container path should prefer the configured bind mount.""" host_output = tmp_path / "gateway-output" + container_output = "/custom-output" expected = host_output / "report.pdf" expected.parent.mkdir(parents=True) expected.write_bytes(b"pdf") @@ -91,11 +95,11 @@ def test_docker_media_path_prefers_export_mount_over_host_output_path( monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{host_output}:/output:rw"]), + json.dumps([f"{host_output}:{container_output}:rw"]), ) with patch("gateway.media_paths.os.path.exists", return_value=True): - media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + media, _ = BasePlatformAdapter.extract_media(f"MEDIA:{container_output}/report.pdf") assert media == [(str(expected), False)] @@ -105,6 +109,7 @@ def test_docker_container_media_path_translates_common_volume_options( ): """Common Docker option suffixes should not prevent MEDIA path mapping.""" host_output = tmp_path / "gateway-output" + container_output = "/agent-media" expected = host_output / "report.pdf" expected.parent.mkdir(parents=True) expected.write_bytes(b"pdf") @@ -112,28 +117,29 @@ def test_docker_container_media_path_translates_common_volume_options( monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{host_output}:/output:{options}"]), + json.dumps([f"{host_output}:{container_output}:{options}"]), ) - media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + media, _ = BasePlatformAdapter.extract_media(f"MEDIA:{container_output}/report.pdf") assert media == [(str(expected), False)] def test_docker_media_path_translation_requires_path_boundary(self, monkeypatch, tmp_path): - """A /output mount must not rewrite unrelated /output-other paths.""" + """A configured mount must not rewrite unrelated similarly-prefixed paths.""" monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{tmp_path}:/output"]), + json.dumps([f"{tmp_path}:/custom-output"]), ) - media, _ = BasePlatformAdapter.extract_media("MEDIA:/output-other/report.pdf") + media, _ = BasePlatformAdapter.extract_media("MEDIA:/custom-output-other/report.pdf") - assert media == [("/output-other/report.pdf", False)] + assert media == [("/custom-output-other/report.pdf", False)] - def test_docker_media_path_translates_output_submount(self, monkeypatch, tmp_path): - """Explicit export submounts under /output should also be eligible.""" + def test_docker_media_path_translates_nested_user_mount(self, monkeypatch, tmp_path): + """Explicit nested bind mounts under any user path should be eligible.""" host_output = tmp_path / "reports" + container_reports = "/agent-media/reports" expected = host_output / "daily.pdf" expected.parent.mkdir(parents=True) expected.write_bytes(b"pdf") @@ -141,15 +147,15 @@ def test_docker_media_path_translates_output_submount(self, monkeypatch, tmp_pat monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{host_output}:/output/reports:rw"]), + json.dumps([f"{host_output}:{container_reports}:rw"]), ) - media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/reports/daily.pdf") + media, _ = BasePlatformAdapter.extract_media(f"MEDIA:{container_reports}/daily.pdf") assert media == [(str(expected), False)] - def test_docker_media_path_ignores_non_export_mount(self, monkeypatch, tmp_path): - """Only explicit /output or /outputs export mounts should be auto-mapped.""" + def test_docker_media_path_maps_user_specified_workspace_mount(self, monkeypatch, tmp_path): + """User-specified non-/output mounts should be mapped from docker_volumes.""" workspace_report = tmp_path / "workspace" / "report.pdf" workspace_report.parent.mkdir(parents=True) workspace_report.write_bytes(b"pdf") @@ -162,7 +168,7 @@ def test_docker_media_path_ignores_non_export_mount(self, monkeypatch, tmp_path) media, _ = BasePlatformAdapter.extract_media("MEDIA:/workspace/report.pdf") - assert media == [("/workspace/report.pdf", False)] + assert media == [(str(workspace_report), False)] def test_docker_media_path_ignores_root_mount(self, monkeypatch, tmp_path): """A root bind mount should not make every container path media-sendable.""" @@ -188,7 +194,7 @@ def test_docker_media_path_keeps_existing_host_file(self, monkeypatch, tmp_path) monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv( "TERMINAL_DOCKER_VOLUMES", - json.dumps([f"{tmp_path / 'exports'}:/output"]), + json.dumps([f"{tmp_path / 'exports'}:/custom-output"]), ) media, _ = BasePlatformAdapter.extract_media(f"MEDIA:{host_file}") @@ -200,9 +206,94 @@ def test_docker_media_path_invalid_volume_env_falls_back(self, monkeypatch): monkeypatch.setenv("TERMINAL_ENV", "docker") monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", "not-json") - media, _ = BasePlatformAdapter.extract_media("MEDIA:/output/report.pdf") + media, _ = BasePlatformAdapter.extract_media("MEDIA:/custom-output/report.pdf") - assert media == [("/output/report.pdf", False)] + assert media == [("/custom-output/report.pdf", False)] + + def test_docker_media_path_ignores_named_volume(self, monkeypatch): + """Named volumes have no derivable host path for the gateway process.""" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", json.dumps(["exports:/agent-media:rw"])) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/agent-media/report.pdf") + + assert media == [("/agent-media/report.pdf", False)] + + def test_docker_media_path_prefers_longest_matching_mount(self, monkeypatch, tmp_path): + """Nested mounts should use the most specific container prefix.""" + broad_host = tmp_path / "broad" + reports_host = tmp_path / "reports" + expected = reports_host / "daily.pdf" + expected.parent.mkdir(parents=True) + expected.write_bytes(b"pdf") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv( + "TERMINAL_DOCKER_VOLUMES", + json.dumps([ + f"{broad_host}:/agent-media:rw", + f"{reports_host}:/agent-media/reports:rw", + ]), + ) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/agent-media/reports/daily.pdf") + + assert media == [(str(expected), False)] + + @pytest.mark.parametrize( + "wrapped_path", + [ + "MEDIA:/agent-media/../secret.pdf", + "`MEDIA:/agent-media/../secret.pdf`", + '"MEDIA:/agent-media/../secret.pdf"', + ], + ) + def test_docker_media_path_does_not_map_traversal_outside_mount( + self, + monkeypatch, + tmp_path, + wrapped_path, + ): + """Normalized paths that escape a mount must not translate to the host.""" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", json.dumps([f"{tmp_path}:/agent-media"])) + + media, _ = BasePlatformAdapter.extract_media(wrapped_path) + + assert media == [("/agent-media/../secret.pdf", False)] + + def test_docker_media_path_does_not_map_symlink_escape(self, monkeypatch, tmp_path): + """Host symlinks inside an export mount must not expose files outside it.""" + export_root = tmp_path / "exports" + export_root.mkdir() + outside_file = tmp_path / "secret.pdf" + outside_file.write_bytes(b"secret") + link = export_root / "leak.pdf" + try: + link.symlink_to(outside_file) + except (NotImplementedError, OSError): + pytest.skip("symlinks are not available on this platform") + + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_DOCKER_VOLUMES", json.dumps([f"{export_root}:/agent-media"])) + + media, _ = BasePlatformAdapter.extract_media("MEDIA:/agent-media/leak.pdf") + + assert media == [("/agent-media/leak.pdf", False)] + + def test_docker_media_path_rejects_windows_separator_escape(self, monkeypatch): + """Backslashes in a Linux container filename must not become host separators.""" + import ntpath + import gateway.media_paths as media_paths + + monkeypatch.setattr(media_paths.os, "path", ntpath) + + translated = media_paths._translate_docker_path_to_host( + r"/agent-media/..\secret.pdf", + ((r"C:\exports", "/agent-media"),), + ) + + assert translated is None # --------------------------------------------------------------------------- diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index ff1a380a1af1..4df8bb3e476f 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -331,7 +331,7 @@ terminal: docker_volumes: - "/home/user/projects:/workspace/projects" # Read-write (default) - "/home/user/datasets:/data:ro" # Read-only - - "/home/user/.hermes/cache/documents:/output" # Gateway-visible exports + - "/home/user/.hermes/cache/documents:/agent-artifacts" # Gateway-visible exports ``` This is useful for: @@ -340,15 +340,17 @@ This is useful for: - **Shared workspaces** where both you and the agent access the same files If you use a messaging gateway and want the agent to send generated files via -`MEDIA:/...`, prefer a dedicated host-visible export mount such as -`/home/user/.hermes/cache/documents:/output`. - -- Write files inside Docker to `/output/...` or `/outputs/...` -- Emit either the host path or the mounted container path, for example - `MEDIA:/output/report.txt` -- The gateway maps mounted `/output` or `/outputs` container paths back through - `docker_volumes` -- Do **not** emit unmapped container-only paths like `/workspace/...` +`MEDIA:/...`, configure a host-visible bind mount. The container path is your +choice; `/output` is only a convention, not a requirement. For example: +`/home/user/.hermes/cache/documents:/agent-artifacts`. + +- Write files inside Docker under the mounted container path, e.g. + `/agent-artifacts/report.txt` +- Emit either the host path or the mounted container path, e.g. + `MEDIA:/agent-artifacts/report.txt` +- The gateway maps mounted container paths back through `docker_volumes` / + `TERMINAL_DOCKER_VOLUMES` using the longest matching non-root bind mount +- Do **not** emit unmapped container-only paths :::warning YAML duplicate keys silently override earlier ones. If you already have a diff --git a/website/docs/user-guide/messaging/telegram.md b/website/docs/user-guide/messaging/telegram.md index 84f9bd37fa85..3f14d2d9f88e 100644 --- a/website/docs/user-guide/messaging/telegram.md +++ b/website/docs/user-guide/messaging/telegram.md @@ -132,16 +132,17 @@ Recommended pattern: terminal: backend: docker docker_volumes: - - "/home/user/.hermes/cache/documents:/output" + - "/home/user/.hermes/cache/documents:/agent-artifacts" ``` Then: -- write files inside Docker to `/output/...` or `/outputs/...` -- emit either the host path or the mounted container path, for example - `MEDIA:/output/report.txt` -- the gateway maps mounted `/output` or `/outputs` container paths back through - `docker_volumes` +- write files inside Docker under the mounted container path, e.g. + `/agent-artifacts/report.txt` +- emit either the host path or the mounted container path, e.g. + `MEDIA:/agent-artifacts/report.txt` +- the gateway maps mounted container paths back through `docker_volumes` / + `TERMINAL_DOCKER_VOLUMES`; `/output` is only a convention, not a requirement If you already have a `docker_volumes:` section, add the new mount to the same list. YAML duplicate keys silently override earlier ones.