diff --git a/cron/scheduler.py b/cron/scheduler.py index 77c2772762238..be43ad5eacc31 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -1333,6 +1333,13 @@ def _send_media_via_adapter( from gateway.platforms.base import BasePlatformAdapter, should_send_media_as_audio + # Docker terminal backend writes inside the container; remap container + # paths to host equivalents before the host-side filter runs. ``job["id"]`` + # is not the job's actual terminal task_id (cron's agent.run_conversation() + # call doesn't pass task_id, so each run gets an unrelated random one — see + # NousResearch/hermes-agent#64889), so translation here degrades to + # mount-table-only / single-Docker-environment-only. + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) for media_path, _is_voice in media_files: @@ -1507,9 +1514,14 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option else: delivery_content = content - # Extract MEDIA: tags so attachments are forwarded as files, not raw text + # Extract MEDIA: tags so attachments are forwarded as files, not raw text. + # Docker terminal backend writes inside the container; remap container + # paths to host equivalents before the host-side filter runs. Degrades to + # mount-table-only / single-Docker-environment-only — see the sibling call + # in _send_media_via_adapter above and NousResearch/hermes-agent#64889. from gateway.platforms.base import BasePlatformAdapter media_files, cleaned_delivery_content = BasePlatformAdapter.extract_media(delivery_content) + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) # Resolve the delivery-mirror gate ONCE (default off). When on, each diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index d7654ff6c147a..6f33e73ba5d8a 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -744,6 +744,60 @@ def get_inbound_media_max_bytes() -> int: return DEFAULT_INBOUND_MEDIA_MAX_BYTES +# --------------------------------------------------------------------------- +# Docker-extraction size cap — bounds BasePlatformAdapter._extract_docker_path +# (the docker-exec-cat fallback in Docker-terminal-backend media delivery). +# Extraction is only useful for files that are subsequently deliverable, so +# the cap tracks the delivery ceiling: the largest outbound attachment size +# among the platforms actually enabled on ``gateway.platforms``. Genuinely +# large media should go through Docker's persistent_filesystem mode instead, +# where stage-1 bind-mount translation delivers it with zero copy and no cap. +# +# Configurable via ``gateway.max_docker_extraction_bytes`` in config.yaml. +# --------------------------------------------------------------------------- +_KNOWN_PLATFORM_UPLOAD_LIMITS_BYTES = { + "whatsapp_cloud": 100 * 1024 * 1024, # whatsapp_cloud.py _MEDIA_SIZE_LIMITS["document"] + "signal": 100 * 1024 * 1024, # signal.py SIGNAL_MAX_ATTACHMENT_SIZE + "telegram": 50 * 1024 * 1024, # Bot API document/file upload limit + "yuanbao": 50 * 1024 * 1024, # yuanbao.py MessageSender.MEDIA_MAX_SIZE_MB +} +DEFAULT_DOCKER_EXTRACTION_MAX_BYTES = max(_KNOWN_PLATFORM_UPLOAD_LIMITS_BYTES.values()) + + +def get_docker_extraction_max_bytes() -> int: + """Return the max bytes the Docker-extraction fallback will read out of a container. + + An explicit ``gateway.max_docker_extraction_bytes`` always wins. + Otherwise defaults to the largest outbound upload ceiling among the + platforms actually enabled under ``gateway.platforms`` (falling back to + ``DEFAULT_DOCKER_EXTRACTION_MAX_BYTES`` if none of the enabled platforms + are in the known-limits table, or if config is unreadable). + """ + try: + from hermes_cli.config import load_config as _load_config + cfg = _load_config() + except Exception: + return DEFAULT_DOCKER_EXTRACTION_MAX_BYTES + gw = cfg.get("gateway", {}) if isinstance(cfg, dict) else {} + if not isinstance(gw, dict): + return DEFAULT_DOCKER_EXTRACTION_MAX_BYTES + if "max_docker_extraction_bytes" in gw: + try: + return int(gw["max_docker_extraction_bytes"]) + except (TypeError, ValueError): + pass + platforms = gw.get("platforms", {}) + if not isinstance(platforms, dict): + return DEFAULT_DOCKER_EXTRACTION_MAX_BYTES + enabled_limits = [ + _KNOWN_PLATFORM_UPLOAD_LIMITS_BYTES[name] + for name, block in platforms.items() + if isinstance(block, dict) and block.get("enabled") + and name in _KNOWN_PLATFORM_UPLOAD_LIMITS_BYTES + ] + return max(enabled_limits) if enabled_limits else DEFAULT_DOCKER_EXTRACTION_MAX_BYTES + + def validate_inbound_media_size( size: int, *, @@ -4342,6 +4396,287 @@ def filter_local_delivery_paths(file_paths) -> List[str]: logger.warning("Skipping unsafe local file path: %s", _log_safe_path(raw)) return safe_paths + # Container-side root the Docker exec-cat extraction fallback (stage 2 of + # _translate_one_docker_path) is allowed to read from. This is the agent's + # own writable scratch/output directory; tools/environments/docker.py + # mounts credential files at /root/.hermes/* — never under here — so + # confining extraction to this root keeps it from ever reading a secret. + _DOCKER_EXTRACTION_ROOT = "/workspace" + + @staticmethod + def _get_docker_mount_table( + task_id: Optional[str] = None, + ) -> Tuple[List[Tuple[str, str]], Optional[str]]: + """Return ([(container_dest, host_src), ...], container_id) for a Docker env. + + ``task_id``, when given, resolves *only* that task's own environment + (``_active_environments[task_id]``) — callers that know which task + produced a path must always pass it, so this never mixes up two + concurrently-running tasks' containers. + + When ``task_id`` is None — the caller can't identify the producing + task (see the generic per-platform delivery paths) — this only + proceeds if there is exactly one active Docker environment; with + zero or multiple concurrent environments there's no way to guess + correctly, so it returns ([], None) rather than risk resolving + against the wrong container. + + Mounts are sorted longest-destination-first so the most specific + prefix wins. + """ + try: + import json + import subprocess + from tools.terminal_tool import _active_environments # type: ignore[import] + + if task_id is not None: + env = _active_environments.get(task_id) + candidates = [env] if env is not None else [] + else: + all_envs = list(_active_environments.values()) + candidates = all_envs if len(all_envs) == 1 else [] + + for env in candidates: + cid = getattr(env, "_container_id", None) + if not cid: + continue + result = subprocess.run( + ["docker", "inspect", "--format", "{{json .Mounts}}", cid], + capture_output=True, text=True, timeout=5, + ) + if result.returncode != 0: + continue + mounts = [ + (m["Destination"].rstrip("/"), m["Source"].rstrip("/")) + for m in json.loads(result.stdout) + if m.get("Type") == "bind" + ] + mounts.sort(key=lambda x: len(x[0]), reverse=True) + return mounts, cid + except Exception: + pass + return [], None + + @staticmethod + def _extract_docker_path(path: str, container_id: str) -> Optional[str]: + """Copy a container-only file to a private host temp file via ``docker exec``. + + Only called for a path that didn't resolve through a declared bind + mount — the common case is the default non-persistent Docker mode, + where ``/workspace`` is a tmpfs mount and never appears in + ``docker inspect``'s mount list, so this is the only way to retrieve + a legitimately-produced agent output. + + Gated before any container read happens: + + 1. Lexically confined to ``_DOCKER_EXTRACTION_ROOT`` (POSIX + ``normpath``; any ``..`` segment is rejected outright). + 2. Symlink-safe: resolved *inside the container* via ``realpath -e`` + first, and the resolved path re-checked against the root — a + symlink planted at ``/workspace/x`` -> ``/root/.hermes/.env`` must + not be followed out. (Hardlinks aren't a bypass: ``/workspace`` + and the credential mounts are always separate filesystems, in + both persistent and tmpfs mode.) + 3. Size-bounded via ``docker exec stat`` before the actual read, so + a container read is never spent on a file no enabled platform + could even deliver. + + The realpath-then-cat sequence has a TOCTOU window, but delivery only + runs after the agent's turn has finished producing its response, so + there's no attacker-controlled process racing the swap. + + Returns the new host path, or None if disqualified or the extraction + failed — the caller keeps the original container-only path, which + then simply fails downstream host-path validation instead of being + treated as resolved. + """ + import os + import posixpath + import subprocess + import tempfile + + root = BasePlatformAdapter._DOCKER_EXTRACTION_ROOT + normalized = posixpath.normpath(path) + if ".." in normalized.split("/") or not ( + normalized == root or normalized.startswith(root + "/") + ): + logger.warning( + "Refusing Docker extraction outside %s: %s", root, _log_safe_path(path), + ) + return None + + try: + resolved = subprocess.run( + ["docker", "exec", container_id, "realpath", "-e", "--", normalized], + capture_output=True, text=True, timeout=5, + ) + if resolved.returncode != 0: + logger.warning( + "Docker extraction: realpath failed for %s in container %s", + _log_safe_path(path), container_id, + ) + return None + real_path = resolved.stdout.strip() + if not (real_path == root or real_path.startswith(root + "/")): + logger.warning( + "Refusing Docker extraction: %s resolves to %s, outside %s", + _log_safe_path(path), _log_safe_path(real_path), root, + ) + return None + + size_check = subprocess.run( + ["docker", "exec", container_id, "stat", "-c", "%s", "--", real_path], + capture_output=True, text=True, timeout=5, + ) + if size_check.returncode != 0: + logger.warning( + "Docker extraction: stat failed for %s in container %s", + _log_safe_path(real_path), container_id, + ) + return None + try: + size = int(size_check.stdout.strip()) + except ValueError: + logger.warning( + "Docker extraction: unparseable size for %s", _log_safe_path(real_path), + ) + return None + + max_bytes = get_docker_extraction_max_bytes() + if size > max_bytes: + logger.warning( + "Refusing Docker extraction of %s: %d bytes exceeds the %d-byte cap. " + "Use persistent_filesystem mode to deliver larger files via " + "zero-copy bind mount instead.", + _log_safe_path(real_path), size, max_bytes, + ) + return None + + dest_dir = tempfile.mkdtemp(prefix="hermes_docker_extract_") + dest_path = os.path.join(dest_dir, os.path.basename(real_path)) + with open(dest_path, "wb") as fh: + r = subprocess.run( + ["docker", "exec", container_id, "cat", "--", real_path], + stdout=fh, stderr=subprocess.DEVNULL, timeout=30, + ) + if r.returncode == 0 and os.path.getsize(dest_path) > 0: + return dest_path + logger.warning( + "Docker extraction: cat failed or produced an empty file for %s", + _log_safe_path(real_path), + ) + try: + os.unlink(dest_path) + os.rmdir(dest_dir) + except OSError: + pass + except Exception: + logger.warning( + "Docker path extraction failed for %s", _log_safe_path(path), exc_info=True, + ) + return None + + @staticmethod + def _translate_one_docker_path( + path: str, + mounts: List[Tuple[str, str]], + container_id: Optional[str], + *, + allow_extraction: bool, + ) -> str: + """Translate one container path to a host-readable path. + + 1. Map via the longest matching bind-mount prefix (declarative, + always safe). + 2. If still unmapped and ``allow_extraction`` is True, try + extracting it from the container — see ``_extract_docker_path`` + for the containment/symlink/size gates that make this safe. + ``allow_extraction`` must only be True when the caller knows + which specific task/container produced *path*: extraction reads + whatever the container has at that path, so it must never run + against a container we can't confirm produced the request. + """ + from pathlib import Path + + host_path = path + for dest, src in mounts: + if path == dest: + host_path = src + break + if path.startswith(dest + "/"): + host_path = src + path[len(dest):] + break + + if host_path == path and allow_extraction and container_id: + try: + exists_on_host = Path(path).exists() + except OSError: + # e.g. PermissionError probing a host dir the gateway process + # can't traverse (/root/* when not running as root) — treat + # as "can't confirm it exists here", not a crash. + exists_on_host = False + if not exists_on_host: + extracted = BasePlatformAdapter._extract_docker_path(path, container_id) + if extracted: + host_path = extracted + return host_path + + @staticmethod + def translate_docker_media_paths(media_files: list, task_id: Optional[str] = None) -> list: + """Translate ``(path, is_voice)`` container paths to host equivalents. + + Pass ``task_id`` whenever the caller knows which agent task produced + these paths (tool calls, background tasks) — translation then + resolves only that task's own container, and the docker-exec-cat + extraction fallback (for paths not covered by a declared bind mount, + e.g. the default ephemeral ``/workspace`` tmpfs) is available. + + When ``task_id`` is omitted — the generic per-platform + response/delivery paths can't identify which task produced a MEDIA: + tag — this degrades to mount-table-only translation, and only when + exactly one Docker environment is active (see + ``_get_docker_mount_table``). It never shells out to extract a file + in this mode. NousResearch/hermes-agent#64889 tracks threading a real + task_id through those paths so they can get the same task-bound + behavior as the tool-call paths. + + No-op when no Docker environment is active or eligible. + """ + if not media_files: + return media_files + mounts, container_id = BasePlatformAdapter._get_docker_mount_table(task_id) + if not mounts and not container_id: + return media_files + allow_extraction = task_id is not None + return [ + ( + BasePlatformAdapter._translate_one_docker_path( + p, mounts, container_id, allow_extraction=allow_extraction + ), + is_voice, + ) + for p, is_voice in media_files + ] + + @staticmethod + def translate_docker_local_paths(file_paths: list, task_id: Optional[str] = None) -> list: + """Translate bare container file-path strings to host equivalents. + + Plain-path counterpart to translate_docker_media_paths (no is_voice + flag) — see its docstring for the ``task_id`` contract. + """ + if not file_paths: + return file_paths + mounts, container_id = BasePlatformAdapter._get_docker_mount_table(task_id) + if not mounts and not container_id: + return file_paths + allow_extraction = task_id is not None + return [ + BasePlatformAdapter._translate_one_docker_path( + p, mounts, container_id, allow_extraction=allow_extraction + ) + for p in file_paths + ] @staticmethod def _mask_protected_spans(content: str) -> str: @@ -5857,6 +6192,14 @@ async def _stop_typing_task() -> None: # Extract MEDIA: tags (from TTS tool) before other processing media_files, response = self.extract_media(response) + # Docker terminal backend writes inside the container; remap + # container paths to host equivalents before the host-side + # filter runs. This generic path has no task_id, so + # translation degrades to mount-table-only / + # single-Docker-environment-only (see + # translate_docker_media_paths's docstring and + # NousResearch/hermes-agent#64889). + media_files = self.translate_docker_media_paths(media_files) media_files = self.filter_media_delivery_paths(media_files) # Do NOT deduplicate MEDIA tags against prior turns here. @@ -5886,6 +6229,7 @@ async def _stop_typing_task() -> None: # system/command notices so config paths stay visible text # instead of becoming native uploads. local_files, text_content = self.extract_local_files(text_content) + local_files = self.translate_docker_local_paths(local_files) local_files = self.filter_local_delivery_paths(local_files) if _history_media_paths: _suppressed = [p for p in local_files if p in _history_media_paths] diff --git a/gateway/platforms/weixin.py b/gateway/platforms/weixin.py index b44ce1ee698fb..e805d3722e436 100644 --- a/gateway/platforms/weixin.py +++ b/gateway/platforms/weixin.py @@ -1871,10 +1871,18 @@ async def send( last_message_id: Optional[str] = None # Extract MEDIA: tags and bare local file paths before text delivery. + # Docker terminal backend writes inside the container; remap container + # paths to host equivalents before the host-side filters run. This + # generic send() path has no task_id, so translation degrades to + # mount-table-only / single-Docker-environment-only (see + # BasePlatformAdapter.translate_docker_media_paths's docstring and + # NousResearch/hermes-agent#64889). media_files, cleaned_content = self.extract_media(content) + media_files = self.translate_docker_media_paths(media_files) media_files = self.filter_media_delivery_paths(media_files) _, image_cleaned = self.extract_images(cleaned_content) local_files, final_content = self.extract_local_files(image_cleaned) + local_files = self.translate_docker_local_paths(local_files) local_files = self.filter_local_delivery_paths(local_files) _AUDIO_EXTS = {".ogg", ".opus", ".mp3", ".wav", ".m4a", ".flac"} diff --git a/gateway/run.py b/gateway/run.py index 06a26d73aeabf..1e518a836e2ca 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -18389,6 +18389,13 @@ async def _deliver_media_from_response( from gateway.platforms.base import BasePlatformAdapter, should_send_media_as_audio media_files, cleaned = adapter.extract_media(response) + # Docker terminal backend writes inside the container; remap container + # paths to host equivalents before the host-side path filter runs. + # No task_id reaches this path (it's called after streaming, from + # the raw response text) — translation degrades to mount-table-only + # and single-Docker-environment-only; see translate_docker_media_paths's + # docstring and NousResearch/hermes-agent#64889. + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) # Do NOT deduplicate explicit MEDIA tags against prior turns here # (#73771). This rescan is already EXPLICIT-ONLY (see docstring): @@ -18611,6 +18618,7 @@ def run_sync(): if response: media_files, response = adapter.extract_media(response) from gateway.platforms.base import BasePlatformAdapter + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files, task_id=task_id) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) images, text_content = adapter.extract_images(response) diff --git a/tests/gateway/test_platform_base.py b/tests/gateway/test_platform_base.py index 8f781f47549ed..0838da04b3cc7 100644 --- a/tests/gateway/test_platform_base.py +++ b/tests/gateway/test_platform_base.py @@ -1,6 +1,8 @@ """Tests for gateway/platforms/base.py — MessageEvent, media extraction, message truncation.""" +import json import os +import tempfile import time from unittest.mock import patch @@ -13,6 +15,7 @@ cache_audio_from_bytes, cache_image_from_bytes, cache_video_from_bytes, + get_docker_extraction_max_bytes, safe_url_for_log, utf16_len, validate_inbound_media_size, @@ -1117,3 +1120,291 @@ async def test_caption_is_preserved_in_fallback(self): sent_text = adapter.sent[0]["content"] assert "Here's the daily summary." in sent_text assert self.SENSITIVE_PATH not in sent_text + + +def _docker_exec_stub(*, realpath_rc=0, realpath_out="", stat_rc=0, stat_out="0", + cat_rc=0, cat_bytes=b"", allow_cat=True): + """Build a ``subprocess.run`` stub for the realpath -> stat -> cat sequence + ``BasePlatformAdapter._extract_docker_path`` issues. Raises if a call is + made past a point the caller expected extraction to have already stopped + (``allow_cat=False``), so tests can assert *which* gate rejected a path.""" + def fake_run(cmd, **kwargs): + assert cmd[:2] == ["docker", "exec"] + sub = cmd[3] + if sub == "realpath": + assert cmd[3:6] == ["realpath", "-e", "--"] + return type("R", (), {"returncode": realpath_rc, "stdout": realpath_out})() + if sub == "stat": + assert cmd[3:6] == ["stat", "-c", "%s"] + return type("R", (), {"returncode": stat_rc, "stdout": stat_out})() + if sub == "cat": + if not allow_cat: + raise AssertionError(f"must not reach cat: {cmd}") + assert cmd[3:5] == ["cat", "--"] + kwargs["stdout"].write(cat_bytes) + return type("R", (), {"returncode": cat_rc})() + raise AssertionError(f"unexpected docker exec subcommand: {cmd}") + return fake_run + + +class TestDockerPathTranslation: + """Shared Docker container→host path translation (used by send_message and the + gateway delivery loop). The mount table is stubbed so tests never shell out to + docker; container_id=None disables the extraction fallback, exercising pure + bind-mount mapping. The extraction fallback (docker-exec-cat) only runs when + ``task_id`` is passed — see NousResearch/hermes-agent#64889 for why callers + that can't identify the producing task get the degraded mount-only / + single-environment-only behavior instead.""" + + def test_media_paths_passthrough_when_no_docker_env(self): + mf = [("/workspace/a.png", False)] + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", return_value=([], None)): + assert BasePlatformAdapter.translate_docker_media_paths(mf) == mf + + def test_media_path_mapped_via_bind_mount(self): + mounts = [("/workspace", "/host/sandbox/workspace")] + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", return_value=(mounts, None)): + out = BasePlatformAdapter.translate_docker_media_paths([("/workspace/img.png", True)]) + assert out == [("/host/sandbox/workspace/img.png", True)] + + def test_longest_prefix_wins(self): + # _get_docker_mount_table returns mounts pre-sorted longest-first. + mounts = [("/workspace/out", "/host/out"), ("/workspace", "/host/ws")] + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", return_value=(mounts, None)): + out = BasePlatformAdapter.translate_docker_media_paths([("/workspace/out/f.mp3", False)]) + assert out == [("/host/out/f.mp3", False)] + + def test_exact_destination_match(self): + mounts = [("/data/file.bin", "/host/data/file.bin")] + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", return_value=(mounts, None)): + out = BasePlatformAdapter.translate_docker_media_paths([("/data/file.bin", False)]) + assert out == [("/host/data/file.bin", False)] + + def test_unmapped_path_passthrough_without_container(self): + # No matching mount and container_id=None → extraction skipped, path unchanged. + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([("/workspace", "/host/ws")], None)): + out = BasePlatformAdapter.translate_docker_media_paths([("/other/x.png", False)]) + assert out == [("/other/x.png", False)] + + def test_local_paths_mapped(self): + mounts = [("/workspace", "/host/ws")] + with patch.object(BasePlatformAdapter, "_get_docker_mount_table", return_value=(mounts, None)): + out = BasePlatformAdapter.translate_docker_local_paths(["/workspace/doc.pdf", "/elsewhere/y"]) + assert out == ["/host/ws/doc.pdf", "/elsewhere/y"] + + def test_empty_inputs_are_noops(self): + # Early return before any docker call. + assert BasePlatformAdapter.translate_docker_media_paths([]) == [] + assert BasePlatformAdapter.translate_docker_local_paths([]) == [] + + # -- task-bound extraction (task_id given -> allow_extraction=True) ----- + + def test_extraction_uses_exec_cat_not_cp(self): + # docker cp fails against the overlayfs storage driver on some hosts; + # exec+cat must be used instead for the last-resort extraction path. + container_path = "/workspace/song_deadbeef01.mp3" + fake_run = _docker_exec_stub( + realpath_out=container_path + "\n", stat_out="16\n", cat_bytes=b"fake-audio-bytes", + ) + with patch("subprocess.run", side_effect=fake_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths([container_path], task_id="task-1") + try: + assert len(out) == 1 + assert out[0] != container_path + assert os.path.basename(out[0]) == "song_deadbeef01.mp3" + with open(out[0], "rb") as fh: + assert fh.read() == b"fake-audio-bytes" + finally: + if os.path.exists(out[0]): + os.unlink(out[0]) + os.rmdir(os.path.dirname(out[0])) + + def test_extraction_preserves_original_basename(self): + # Regression: an earlier implementation renamed extracted files to + # hermes_docker_media_., which some platform adapters + # then used as a fallback display title instead of the real filename. + container_path = "/workspace/my_generated_track.mp3" + fake_run = _docker_exec_stub( + realpath_out=container_path + "\n", stat_out="16\n", cat_bytes=b"fake-audio-bytes", + ) + with patch("subprocess.run", side_effect=fake_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths([container_path], task_id="task-1") + try: + assert os.path.basename(out[0]) == "my_generated_track.mp3" + assert "hermes_docker_media_" not in out[0] + # Regression: the destination must not be the old predictable + # /tmp/ path — a private per-extraction directory instead. + assert out[0] != os.path.join("/tmp", "my_generated_track.mp3") + finally: + if os.path.exists(out[0]): + os.unlink(out[0]) + os.rmdir(os.path.dirname(out[0])) + + def test_extraction_cat_failure_falls_back_to_original_path_and_cleans_up(self): + container_path = "/workspace/missing_in_container.mp3" + fake_run = _docker_exec_stub(realpath_out=container_path + "\n", stat_out="10\n", cat_rc=1) + created_dirs = [] + real_mkdtemp = tempfile.mkdtemp + + def spy_mkdtemp(*a, **kw): + d = real_mkdtemp(*a, **kw) + created_dirs.append(d) + return d + + with patch("subprocess.run", side_effect=fake_run), \ + patch("tempfile.mkdtemp", side_effect=spy_mkdtemp), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths([container_path], task_id="task-1") + assert out == [container_path] + for d in created_dirs: + assert not os.path.exists(d) + + def test_extraction_refuses_oversize_file_and_leaves_no_host_file(self): + container_path = "/workspace/huge.mp4" + oversize = get_docker_extraction_max_bytes() + 1 + fake_run = _docker_exec_stub( + realpath_out=container_path + "\n", stat_out=f"{oversize}\n", allow_cat=False, + ) + created_dirs = [] + real_mkdtemp = tempfile.mkdtemp + + def spy_mkdtemp(*a, **kw): + d = real_mkdtemp(*a, **kw) + created_dirs.append(d) + return d + + with patch("subprocess.run", side_effect=fake_run), \ + patch("tempfile.mkdtemp", side_effect=spy_mkdtemp), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths([container_path], task_id="task-1") + assert out == [container_path] + # Refused before ever creating an extraction destination. + assert created_dirs == [] + + # -- containment: out-of-root and symlink-escape paths ------------------- + + def test_extraction_rejects_path_outside_workspace_before_any_docker_call(self): + # A path outside /workspace that also happens not to exist on the + # *host* (e.g. /etc, unlike /root, is world-traversable, so this + # exercises the lexical containment check rather than a host + # permission error on the probe path). + outside_path = "/etc/does-not-exist/credential.env" + + def fake_run(cmd, **kwargs): + raise AssertionError(f"must not shell out for an out-of-root path: {cmd}") + + with patch("subprocess.run", side_effect=fake_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths( + [outside_path], task_id="task-1", + ) + assert out == [outside_path] + + def test_extraction_rejects_dotdot_traversal_before_any_docker_call(self): + def fake_run(cmd, **kwargs): + raise AssertionError(f"must not shell out for a traversal path: {cmd}") + + traversal_path = "/workspace/../etc/does-not-exist/credential.env" + with patch("subprocess.run", side_effect=fake_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths( + [traversal_path], task_id="task-1", + ) + assert out == [traversal_path] + + def test_extraction_rejects_symlink_escape_after_realpath_but_before_stat_or_cat(self): + # /workspace/report.pdf is lexically fine, but a symlink planted at + # that path resolves (inside the container) to a credential file — + # this must be caught by the post-realpath containment re-check, and + # must never reach stat or cat. + fake_run = _docker_exec_stub( + realpath_out="/root/.hermes/.env\n", allow_cat=False, + ) + + def guarded_run(cmd, **kwargs): + if cmd[3] in ("stat", "cat"): + raise AssertionError(f"must not proceed past realpath for a symlink escape: {cmd}") + return fake_run(cmd, **kwargs) + + with patch("subprocess.run", side_effect=guarded_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths( + ["/workspace/report.pdf"], task_id="task-1", + ) + assert out == ["/workspace/report.pdf"] + + # -- task-bound container selection (point 1: no more "first env wins") -- + + def test_task_bound_lookup_uses_own_container_not_another_active_one(self): + env_a = type("Env", (), {"_container_id": "container-a"})() + env_b = type("Env", (), {"_container_id": "container-b"})() + fake_envs = {"task-a": env_a, "task-b": env_b} + mounts_by_cid = { + "container-a": [{"Destination": "/workspace", "Source": "/host/a/workspace", "Type": "bind"}], + "container-b": [{"Destination": "/workspace", "Source": "/host/b/workspace", "Type": "bind"}], + } + + def fake_run(cmd, **kwargs): + cid = cmd[-1] + return type("R", (), {"returncode": 0, "stdout": json.dumps(mounts_by_cid[cid])})() + + with patch("subprocess.run", side_effect=fake_run), \ + patch("tools.terminal_tool._active_environments", fake_envs): + out_a = BasePlatformAdapter.translate_docker_local_paths(["/workspace/x.txt"], task_id="task-a") + out_b = BasePlatformAdapter.translate_docker_local_paths(["/workspace/x.txt"], task_id="task-b") + + assert out_a == ["/host/a/workspace/x.txt"] + assert out_b == ["/host/b/workspace/x.txt"] + + # -- degraded/unscoped path (task_id=None): mount-only + single-env-only - + + def test_get_docker_mount_table_without_task_id_uses_sole_environment(self): + env = type("Env", (), {"_container_id": "solo"})() + fake_envs = {"task-x": env} + + def fake_run(cmd, **kwargs): + return type("R", (), {"returncode": 0, "stdout": json.dumps( + [{"Destination": "/workspace", "Source": "/host/solo/workspace", "Type": "bind"}] + )})() + + with patch("subprocess.run", side_effect=fake_run), \ + patch("tools.terminal_tool._active_environments", fake_envs): + mounts, cid = BasePlatformAdapter._get_docker_mount_table() + assert cid == "solo" + assert mounts == [("/workspace", "/host/solo/workspace")] + + def test_get_docker_mount_table_without_task_id_refuses_when_multiple_environments(self): + env_a = type("Env", (), {"_container_id": "container-a"})() + env_b = type("Env", (), {"_container_id": "container-b"})() + fake_envs = {"task-a": env_a, "task-b": env_b} + + def fake_run(cmd, **kwargs): + raise AssertionError("must not inspect any container when the active env is ambiguous") + + with patch("subprocess.run", side_effect=fake_run), \ + patch("tools.terminal_tool._active_environments", fake_envs): + mounts, cid = BasePlatformAdapter._get_docker_mount_table() + assert (mounts, cid) == ([], None) + + def test_unscoped_translation_never_extracts_even_with_a_container_available(self): + # task_id omitted (generic/degraded callers): even if a mount-table + # lookup somehow yields a container_id, extraction must not fire — + # allow_extraction is gated purely on task_id being provided. + def fake_run(cmd, **kwargs): + raise AssertionError(f"degraded/unscoped path must never shell out to extract: {cmd}") + + with patch("subprocess.run", side_effect=fake_run), \ + patch.object(BasePlatformAdapter, "_get_docker_mount_table", + return_value=([], "abc123")): + out = BasePlatformAdapter.translate_docker_local_paths(["/workspace/x.mp3"]) + assert out == ["/workspace/x.mp3"] diff --git a/tools/send_message_tool.py b/tools/send_message_tool.py index 4e57dee104c5a..bed1d1f73a0c8 100644 --- a/tools/send_message_tool.py +++ b/tools/send_message_tool.py @@ -251,7 +251,7 @@ def send_message_tool(args, **kw): if action == "unreact": return _handle_react(args, remove=True) - return _handle_send(args) + return _handle_send(args, task_id=kw.get("task_id")) def _handle_list(): @@ -355,7 +355,7 @@ def _handle_react(args, remove=False): return json.dumps({"success": bool(result)}) -def _handle_send(args): +def _handle_send(args, task_id=None): """Send a message to a platform target.""" target = args.get("target", "") message = args.get("message", "") @@ -440,6 +440,7 @@ def _handle_send(args): force_document_attachments = "[[as_document]]" in message media_files, cleaned_message = BasePlatformAdapter.extract_media(message) + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files, task_id=task_id) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) mirror_text = cleaned_message.strip() or _describe_media_for_mirror(media_files) diff --git a/tools/yuanbao_tools.py b/tools/yuanbao_tools.py index 46f635c9829dc..7eb5c8ad10497 100644 --- a/tools/yuanbao_tools.py +++ b/tools/yuanbao_tools.py @@ -472,6 +472,7 @@ async def _handle_yb_send_dm(args, **kw): embedded_media, message = BasePlatformAdapter.extract_media(message) if embedded_media: media_files.extend(embedded_media) + media_files = BasePlatformAdapter.translate_docker_media_paths(media_files, task_id=kw.get("task_id")) media_files = BasePlatformAdapter.filter_media_delivery_paths(media_files) return tool_result(await send_dm(