fix(gateway): remote media fetch fails closed on unresolvable symlinks, skips strict mode, keys the sandbox by session id

readlink returning nothing no longer falls back to the unresolved name (a sandbox symlink to a credential would otherwise be followed by [ -f ]). Strict delivery mode keeps its recency gate: no fetch, since the copy lands in an allowlisted root. The active sandbox is looked up by the HERMES_SESSION_ID the turn registered it under (per-session-isolated docker keys by task id, not session key). MEDIA path normalisation (quotes/backticks/trailing punctuation) now matches the host path; the denylist tables are built once at import.
2026-09-05 16:05:07 +05:30
parent 90db26ad55
commit b499ab11fe
2 changed files with 45 additions and 29 deletions
@@ -1,13 +1,18 @@
 """Deliver ``MEDIA:<path>`` files that live inside a remote terminal sandbox (#466).

 ``validate_media_delivery_path`` only accepts files on the gateway host. When the terminal backend
-is ssh / modal / daytona / singularity / vercel (or a docker path outside its bind mounts), the
-agent's artifact is on another filesystem, so the tag was silently dropped. This module pulls the
-file through the active environment's ``fetch_file`` into the document cache (already an
-allowlisted delivery root) and hands back the host copy.
+is ssh / daytona / vercel (any backend that reports its ``_remote_home``), the agent's artifact is
+on another filesystem, so the tag was silently dropped. This module pulls the file through the
+active environment's ``fetch_file`` into the document cache (already an allowlisted delivery root)
+and hands back the host copy. Backends without a known remote home (docker outside its mounts,
+modal, singularity) still get the conservative any-component denylist, so a ``/root/...`` artifact
+there is not fetched — set ``_remote_home`` on the environment to opt in.

 The remote path is screened against the same denylist as local deliveries BEFORE any bytes move,
-and again after ``readlink -f`` — a remote fetch must never become a bypass of the host denylist.
+and again after ``readlink -f`` (fail closed when it cannot resolve) — a remote fetch must never
+become a bypass of the host denylist. Strict mode (``HERMES_MEDIA_DELIVERY_STRICT``) keeps its
+pre-existing behaviour: nothing is fetched, since a fetched copy would land in an allowlisted root
+and skip the recency gate strict mode exists for.
 """

 from __future__ import annotations
@@ -15,24 +20,30 @@ from __future__ import annotations
 import logging
 import os
 import posixpath
+import re
 import uuid
 from pathlib import Path, PurePosixPath
 from typing import Optional

+from gateway.platforms.base import (
+    _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS, _MEDIA_DELIVERY_DENIED_PREFIXES, _ROOT_CREDENTIAL_PATHS,
+    _TRUTHY, MEDIA_DELIVERY_STRICT_ENV)
+
 logger = logging.getLogger(__name__)

 # Telegram bot uploads cap at 50 MB; the other platforms are in the same range. Mirrors
 # tools.image_source._MAX_INGEST_BYTES.
 _FETCH_MAX_BYTES = 50 * 1024 * 1024

+_DENIED_PREFIXES = tuple(PurePosixPath(p) for p in _MEDIA_DELIVERY_DENIED_PREFIXES)
+# Credential dirs under the sandbox home plus the Hermes stores, which live at ``~/.hermes`` there.
+_DENIED_HOME_RELATIVE = tuple(PurePosixPath(s) for s in _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS) + tuple(
+    PurePosixPath(".hermes", *PurePosixPath(rel.replace(os.sep, "/")).parts) for rel in _ROOT_CREDENTIAL_PATHS)
+

 def remote_path_is_denied(path: str, remote_home: Optional[str]) -> bool:
    """Pure string check (the remote fs can't be stat'd from here) applying the host denylist to a
-    sandbox path: system prefixes, credential dirs under the sandbox home, Hermes credential stores.
-    Unknown home ⇒ home-relative entries match ANY path component (conservative)."""
-    from gateway.platforms.base import (
-        _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS, _MEDIA_DELIVERY_DENIED_PREFIXES, _ROOT_CREDENTIAL_PATHS)
-
+    sandbox path. Unknown home ⇒ home-relative entries match ANY path component (conservative)."""
    target = PurePosixPath(posixpath.normpath(path))
    if not target.is_absolute():
        return True
@@ -43,44 +54,42 @@ def remote_path_is_denied(path: str, remote_home: Optional[str]) -> bool:

    # The sandbox's own home may be a denied system prefix (/root); its credential subpaths are
    # separate, more specific entries — same exception as _path_under_denied_prefix.
-    if any(_under(PurePosixPath(p)) for p in _MEDIA_DELIVERY_DENIED_PREFIXES if PurePosixPath(p) != home):
+    if any(_under(p) for p in _DENIED_PREFIXES if p != home):
        return True
-    home_relative = [PurePosixPath(s) for s in _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS]
-    home_relative += [PurePosixPath(".hermes", *PurePosixPath(rel.replace(os.sep, "/")).parts) for rel in _ROOT_CREDENTIAL_PATHS]
    if home is not None:
-        return any(_under(home / rel) for rel in home_relative)
+        return any(_under(home / rel) for rel in _DENIED_HOME_RELATIVE)
    parts = target.parts
    return any(parts[i:i + len(rel.parts)] == rel.parts
-               for rel in home_relative for i in range(len(parts) - len(rel.parts) + 1))
+               for rel in _DENIED_HOME_RELATIVE for i in range(len(parts) - len(rel.parts) + 1))


 def _active_remote_env():
-    """The live remote BaseEnvironment for the current session, or None (local backend / no env yet)."""
+    """The live remote BaseEnvironment for the current session, or None (local backend / no env yet).
+    Keyed by the session id the turn registered its sandbox under (falls back to the session key)."""
    from agent.prompt_builder import _REMOTE_TERMINAL_BACKENDS, _plugin_backend_is_remote
    from gateway.platforms.base import _tenv
+    from gateway.session_context import get_session_env
+    from tools.terminal_tool_lifecycle import get_active_env
    backend = _tenv("TERMINAL_ENV", "local").strip().lower()
    if backend not in _REMOTE_TERMINAL_BACKENDS and not _plugin_backend_is_remote(backend):
        return None
-    try:
-        from tools.terminal_tool import _current_session_key
-        from tools.terminal_tool_lifecycle import get_active_env
-        return get_active_env(_current_session_key() or "default")
-    except Exception as exc:
-        logger.debug("Remote media fetch: no active env: %s", exc)
-        return None
+    return get_active_env(get_session_env("HERMES_SESSION_ID") or get_session_env("HERMES_SESSION_KEY") or "default")


 def fetch_remote_media(path: str) -> Optional[str]:
    """Host path of a validated copy of sandbox file ``path``, or None (never raises). Only fires
    when a remote backend is active; the caller has already failed local validation."""
+    if os.environ.get(MEDIA_DELIVERY_STRICT_ENV, "0").strip().lower() in _TRUTHY:
+        return None
    env = _active_remote_env()
    if env is None:
        return None
-    from gateway.platforms.base import DOCUMENT_CACHE_DIR, _log_safe_path, validate_media_delivery_path
+    from gateway.platforms.base import (
+        DOCUMENT_CACHE_DIR, _log_safe_path, _normalize_media_tag_path, validate_media_delivery_path)
    from tools.environments.base import FileFetchError

    remote_home = getattr(env, "_remote_home", None)
-    candidate = posixpath.normpath(str(path).strip())
+    candidate = posixpath.normpath(_normalize_media_tag_path(str(path)) or "")
    if candidate == "~" or candidate.startswith("~/"):
        if not remote_home:
            return None
@@ -88,10 +97,12 @@ def fetch_remote_media(path: str) -> Optional[str]:
    if not candidate.startswith("/") or remote_path_is_denied(candidate, remote_home):
        return None
    try:
-        resolved = env.fetch_realpath(candidate) or candidate
-        if remote_path_is_denied(resolved, remote_home):
+        # ``[ -f ]`` in fetch_file follows symlinks, so the link TARGET is what gets screened;
+        # an unresolvable path fails closed rather than trusting the unresolved name.
+        resolved = env.fetch_realpath(candidate)
+        if resolved is None or remote_path_is_denied(resolved, remote_home):
            return None
-        basename = "".join(c for c in posixpath.basename(resolved) if c.isprintable() and c not in '/\\:*?"<>|') or "file"
+        basename = re.sub(r"[^\w.\-]", "_", posixpath.basename(resolved)) or "file"
        dest = Path(DOCUMENT_CACHE_DIR) / f"remote_{uuid.uuid4().hex[:12]}_{basename}"
        dest.parent.mkdir(parents=True, exist_ok=True)
        env.fetch_file(resolved, dest, max_bytes=_FETCH_MAX_BYTES)
@@ -56,9 +56,14 @@ def test_sandbox_artifact_is_fetched_but_credentials_and_symlinks_to_them_are_no
    assert remote_env.fetched == ["/home/agent/out/report.txt", "/home/agent/out/report.txt"]


-def test_local_backend_does_not_fetch(monkeypatch, tmp_path):
+def test_local_backend_and_strict_mode_do_not_fetch(monkeypatch, tmp_path, remote_env):
+    """Strict mode keeps its recency gate: a fetched copy would land in an allowlisted root and skip it."""
+    monkeypatch.setenv("HERMES_MEDIA_DELIVERY_STRICT", "1")
+    assert BasePlatformAdapter.filter_media_delivery_paths([("/home/agent/out/report.txt", False)]) == []
+    monkeypatch.delenv("HERMES_MEDIA_DELIVERY_STRICT")
    monkeypatch.setattr(media_fetch, "_active_remote_env", lambda: None)
    assert BasePlatformAdapter.filter_media_delivery_paths([(str(tmp_path / "nope.txt"), False)]) == []
+    assert remote_env.fetched == []


 class _ScriptedEnv(BaseEnvironment):