From 99d2873634f2c10bc3a7fc9d7f6cbb8c01d238eb Mon Sep 17 00:00:00 2001 From: Adolanium <94890352+Adolanium@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:17:05 +0300 Subject: [PATCH 1/2] fix(docker): do not leak the shared-container key across multiplex profiles Under multiplex, os.environ holds the default profile's TERMINAL_DOCKER_SHARED_CONTAINER_KEY. A secondary profile with no key in its own config.yaml was joining that container anyway. MEDIA lookup also used the process profile, so agent:coder:... files missed profile:coder. Read terminal.docker_shared_container_key from that profile's config. If multiplex is on and the file has no key, stay isolated. If the multiplex probe or a MEDIA helper throws, a secondary profile still does not inherit the process env key or collapse onto default. --- gateway/platforms/base.py | 47 ++++-- tests/tools/test_shared_container_task_id.py | 168 +++++++++++++++++++ tools/terminal_tool.py | 99 ++++++++++- 3 files changed, 301 insertions(+), 13 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index e990255794bfa..8ee713178ecc2 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1551,20 +1551,45 @@ def _docker_sandbox_dir_candidates(session_key: str = "") -> List[str]: try: from tools.environments.base import sanitize_task_id_for_path except Exception: - return ["default"] - # Explicit trusted-profiles opt-in: one shared container identity. - shared = os.getenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "").strip() + return [] + + profile: Optional[str] = None + if session_key: + try: + from tools.terminal_tool import profile_name_from_session_key + + profile = profile_name_from_session_key(session_key) + except Exception: + parts = str(session_key).split(":") + if len(parts) >= 2 and parts[0] == "agent": + namespace = parts[1] or "main" + profile = "default" if namespace == "main" else namespace + else: + try: + from hermes_cli.profiles import get_active_profile_name + + profile = get_active_profile_name() or "default" + except Exception: + profile = None + if profile == "custom": + profile = "default" + + shared = "" + if profile is not None: + try: + from tools.terminal_tool import docker_shared_container_key_for_profile + + shared = docker_shared_container_key_for_profile(profile) + except Exception: + shared = "" if shared: candidates.append(sanitize_task_id_for_path(f"shared:{shared}")) - try: - from hermes_cli.profiles import get_active_profile_name - - profile = get_active_profile_name() or "default" - except Exception: - profile = "default" - if profile != "default": + if profile and profile != "default": candidates.append(sanitize_task_id_for_path(f"profile:{profile}")) - candidates.append("default") + if profile is not None: + # Known profile still falls back to the shared default sandbox so a + # file that landed there still delivers. Unknown profile does not. + candidates.append("default") if session_key: # Bug-window legacy layout: per-session sandboxes. candidates.append(sanitize_task_id_for_path(f"session:{session_key}")) diff --git a/tests/tools/test_shared_container_task_id.py b/tests/tools/test_shared_container_task_id.py index 7e5c8da224505..c7cbeb9f08107 100644 --- a/tests/tools/test_shared_container_task_id.py +++ b/tests/tools/test_shared_container_task_id.py @@ -15,6 +15,8 @@ Docker Backend in ``website/docs/user-guide/configuration.md``. """ +from pathlib import Path + import pytest from tools import terminal_tool @@ -309,3 +311,169 @@ def test_shared_key_ignored_outside_persistent_docker(monkeypatch): assert terminal_tool._resolve_container_task_id(None) == "session:sess-A" finally: clear_session_vars(tokens) + + +def test_profile_name_from_session_key(): + assert terminal_tool.profile_name_from_session_key("agent:main:telegram:dm:1") == "default" + assert terminal_tool.profile_name_from_session_key("agent:coder:telegram:dm:1") == "coder" + assert terminal_tool.profile_name_from_session_key("") == "default" + assert terminal_tool.profile_name_from_session_key("sess-A") == "default" + + +def test_multiplex_does_not_leak_default_shared_key(tmp_path, monkeypatch): + # os.environ holds the default profile's key. A secondary profile with + # no key in its own config must stay isolated. + from agent import secret_scope + from gateway.session_context import clear_session_vars, set_session_vars + + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + work = home / "profiles" / "work" + work.mkdir(parents=True) + (work / "config.yaml").write_text( + "terminal:\n docker_shared_container_key: ''\n", encoding="utf-8" + ) + + _persistent_docker(monkeypatch) + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + previous = secret_scope.is_multiplex_active() + secret_scope.set_multiplex_active(True) + tokens = set_session_vars(session_key="agent:work:telegram:dm:1", profile="work") + try: + assert terminal_tool._resolve_container_task_id(None) == "profile:work" + finally: + clear_session_vars(tokens) + secret_scope.set_multiplex_active(previous) + + +def test_multiplex_honors_secondary_profile_shared_key(tmp_path, monkeypatch): + from agent import secret_scope + from gateway.session_context import clear_session_vars, set_session_vars + + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + work = home / "profiles" / "work" + work.mkdir(parents=True) + (work / "config.yaml").write_text( + "terminal:\n docker_shared_container_key: work-lab\n", encoding="utf-8" + ) + + _persistent_docker(monkeypatch) + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + previous = secret_scope.is_multiplex_active() + secret_scope.set_multiplex_active(True) + tokens = set_session_vars(session_key="agent:work:telegram:dm:1", profile="work") + try: + assert terminal_tool._resolve_container_task_id(None) == "shared:work-lab" + finally: + clear_session_vars(tokens) + secret_scope.set_multiplex_active(previous) + + +def test_sandbox_candidates_follow_session_profile(monkeypatch): + from gateway.platforms.base import _docker_sandbox_dir_candidates + from tools.environments.base import sanitize_task_id_for_path + + monkeypatch.delenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", raising=False) + names = _docker_sandbox_dir_candidates("agent:coder:telegram:dm:1") + assert names[0] == sanitize_task_id_for_path("profile:coder") + assert "default" in names + assert names[-1] == sanitize_task_id_for_path("session:agent:coder:telegram:dm:1") + + +def test_sandbox_candidates_default_session_stays_default(monkeypatch): + from gateway.platforms.base import _docker_sandbox_dir_candidates + from tools.environments.base import sanitize_task_id_for_path + + monkeypatch.delenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", raising=False) + names = _docker_sandbox_dir_candidates("agent:main:telegram:dm:123456") + assert names[0] == "default" + assert sanitize_task_id_for_path("profile:custom") not in names + assert sanitize_task_id_for_path("session:agent:main:telegram:dm:123456") in names + + +def test_sandbox_candidates_multiplex_does_not_use_default_shared_key(tmp_path, monkeypatch): + from agent import secret_scope + from gateway.platforms.base import _docker_sandbox_dir_candidates + from tools.environments.base import sanitize_task_id_for_path + + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + work = home / "profiles" / "work" + work.mkdir(parents=True) + (work / "config.yaml").write_text("terminal: {}\n", encoding="utf-8") + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + previous = secret_scope.is_multiplex_active() + secret_scope.set_multiplex_active(True) + try: + names = _docker_sandbox_dir_candidates("agent:work:telegram:dm:1") + finally: + secret_scope.set_multiplex_active(previous) + assert names[0] == sanitize_task_id_for_path("profile:work") + assert sanitize_task_id_for_path("shared:team/workspace") not in names + + +def test_multiplex_probe_error_does_not_leak_default_shared_key(tmp_path, monkeypatch): + # If is_multiplex_active() throws, a secondary profile must not inherit + # the process env key. + from agent import secret_scope + from gateway.session_context import clear_session_vars, set_session_vars + + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + work = home / "profiles" / "work" + work.mkdir(parents=True) + (work / "config.yaml").write_text("terminal: {}\n", encoding="utf-8") + + _persistent_docker(monkeypatch) + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + + def _boom(): + raise RuntimeError("multiplex probe failed") + + monkeypatch.setattr(secret_scope, "is_multiplex_active", _boom) + tokens = set_session_vars(session_key="agent:work:telegram:dm:1", profile="work") + try: + assert terminal_tool._resolve_container_task_id(None) == "profile:work" + finally: + clear_session_vars(tokens) + + +def test_sandbox_candidates_key_lookup_error_does_not_use_default_shared_key(monkeypatch): + from gateway.platforms.base import _docker_sandbox_dir_candidates + from tools.environments.base import sanitize_task_id_for_path + + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + + def _boom(profile=None): + raise RuntimeError("key lookup failed") + + monkeypatch.setattr( + "tools.terminal_tool.docker_shared_container_key_for_profile", _boom + ) + names = _docker_sandbox_dir_candidates("agent:coder:telegram:dm:1") + assert sanitize_task_id_for_path("shared:team/workspace") not in names + assert names[0] == sanitize_task_id_for_path("profile:coder") + assert "default" in names + + +def test_sandbox_candidates_unknown_profile_does_not_use_default(monkeypatch): + from gateway.platforms.base import _docker_sandbox_dir_candidates + + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + + def _boom(): + raise RuntimeError("no active profile") + + monkeypatch.setattr("hermes_cli.profiles.get_active_profile_name", _boom) + names = _docker_sandbox_dir_candidates("") + assert names == [] + assert "default" not in names diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index fe1b17039de3e..3a6eeefc40266 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -1438,6 +1438,101 @@ def _current_session_profile() -> str: return get_session_env("HERMES_SESSION_PROFILE", "") +def _normalize_docker_profile_name(profile: Optional[str]) -> str: + name = (profile or "").strip() + if not name or name in ("default", "main"): + return "default" + return name + + +def profile_name_from_session_key(session_key: Optional[str]) -> str: + """Profile namespace encoded in a gateway session key (agent::...).""" + parts = str(session_key or "").split(":") + if len(parts) < 2 or parts[0] != "agent": + return "default" + namespace = parts[1] or "main" + return "default" if namespace == "main" else namespace + + +def _is_process_docker_profile(name: str) -> bool: + """True when *name* is the profile this process was launched as.""" + try: + from hermes_cli.profiles import get_active_profile_name + + active = _normalize_docker_profile_name(get_active_profile_name()) + except Exception: + active = "default" + if name == active: + return True + # Tests and custom HERMES_HOME paths report "custom". The process + # config is still get_hermes_home(). Treat default as this process. + if name == "default" and active in ("default", "custom"): + return True + return False + + +def _read_profile_docker_shared_key(name: str) -> str: + """Read terminal.docker_shared_container_key from that profile's config.yaml.""" + try: + from hermes_constants import get_hermes_home + from hermes_cli.profiles import get_profile_dir + + cfg_path = ( + get_hermes_home() / "config.yaml" + if _is_process_docker_profile(name) + else get_profile_dir(name) / "config.yaml" + ) + if not cfg_path.is_file(): + return "" + import yaml + + data = yaml.safe_load(cfg_path.read_text(encoding="utf-8")) or {} + if not isinstance(data, dict): + return "" + terminal = data.get("terminal") or {} + if not isinstance(terminal, dict): + return "" + return str(terminal.get("docker_shared_container_key") or "").strip() + except Exception: + return "" + + +def docker_shared_container_key_for_profile(profile: Optional[str] = None) -> str: + """Read ``terminal.docker_shared_container_key`` for this profile. + + Under multiplex, ``os.environ`` holds the default profile. A secondary + profile must not inherit that key. Read the named profile's config.yaml. + If multiplex is on and that file has no key, return empty (isolated). + If the multiplex probe throws, a secondary profile still returns empty. + Process env is used only for this process's own profile, or when + multiplex is known to be off (single-profile CLI/gateway). + """ + name = _normalize_docker_profile_name( + profile if profile is not None else _current_session_profile() + ) + from_file = _read_profile_docker_shared_key(name) + if from_file: + return from_file + try: + process_owned = _is_process_docker_profile(name) + except Exception: + process_owned = False + if process_owned: + return os.getenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "").strip() + multiplex_known = False + multiplex = False + try: + from agent.secret_scope import is_multiplex_active + + multiplex = bool(is_multiplex_active()) + multiplex_known = True + except Exception: + multiplex_known = False + if multiplex_known and not multiplex: + return os.getenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "").strip() + return "" + + _ISOLATION_OVERRIDE_KEYS = frozenset({ "docker_image", "modal_image", "singularity_image", "daytona_image", "env_type", @@ -1516,7 +1611,7 @@ def _resolve_container_task_id(task_id: Optional[str]) -> str: # Explicit opt-in: trusted profiles configuring the same # terminal.docker_shared_container_key share ONE container/cache # slot (and sandbox dir) regardless of profile name (#84671). - shared = os.getenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "").strip() + shared = docker_shared_container_key_for_profile() if shared: return f"shared:{shared}" profile = _current_session_profile() or "default" @@ -1529,7 +1624,7 @@ def _resolve_container_task_id(task_id: Optional[str]) -> str: # sessions land in "shared:" — splitting the very container the # setting exists to unify. if _docker_persistent_profile_scoped(): - shared = os.getenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "").strip() + shared = docker_shared_container_key_for_profile() if shared: return f"shared:{shared}" return "default" From f1c7382677cfe9b05125b83470695b37998249c6 Mon Sep 17 00:00:00 2001 From: Adolanium <94890352+Adolanium@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:03:41 +0300 Subject: [PATCH 2/2] fix(docker): stop named-profile MEDIA from searching the default sandbox A routed agent:coder:... session still listed the unowned default workspace as a MEDIA candidate. First-existing-file lookup then returned the default profile's foo.png when coder had no copy. Named profiles now search only their own profile/shared sandbox plus the same-session legacy session: path. default stays for the default profile and CLI. --- gateway/platforms/base.py | 12 ++- tests/tools/test_shared_container_task_id.py | 81 +++++++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 8ee713178ecc2..715bc8ba35e4c 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -1542,6 +1542,10 @@ def _docker_sandbox_dir_candidates(session_key: str = "") -> List[str]: live (``session:``) are kept as a fallback candidate so files produced in that window still deliver (self-heal, no migration). + Named profiles never search the unowned ``default`` workspace. MEDIA + lookup is first-existing-file, so appending that sandbox would leak + same-name files from the default profile into a coder/work session. + Takes the key explicitly because the delivery pipeline runs after ``_handle_message_with_agent`` cleared the turn's session contextvars (#93950) — an ambient lookup here would silently collapse onto @@ -1586,9 +1590,11 @@ def _docker_sandbox_dir_candidates(session_key: str = "") -> List[str]: candidates.append(sanitize_task_id_for_path(f"shared:{shared}")) if profile and profile != "default": candidates.append(sanitize_task_id_for_path(f"profile:{profile}")) - if profile is not None: - # Known profile still falls back to the shared default sandbox so a - # file that landed there still delivers. Unknown profile does not. + if profile == "default": + # Default profile (and CLI) owns the shared "default" sandbox. + # Named profiles must not search it: that directory belongs to + # another profile, and first-existing-file lookup would leak + # same-name files across profiles. candidates.append("default") if session_key: # Bug-window legacy layout: per-session sandboxes. diff --git a/tests/tools/test_shared_container_task_id.py b/tests/tools/test_shared_container_task_id.py index c7cbeb9f08107..c7ffbfdf0f81c 100644 --- a/tests/tools/test_shared_container_task_id.py +++ b/tests/tools/test_shared_container_task_id.py @@ -381,7 +381,7 @@ def test_sandbox_candidates_follow_session_profile(monkeypatch): monkeypatch.delenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", raising=False) names = _docker_sandbox_dir_candidates("agent:coder:telegram:dm:1") assert names[0] == sanitize_task_id_for_path("profile:coder") - assert "default" in names + assert "default" not in names assert names[-1] == sanitize_task_id_for_path("session:agent:coder:telegram:dm:1") @@ -417,6 +417,7 @@ def test_sandbox_candidates_multiplex_does_not_use_default_shared_key(tmp_path, secret_scope.set_multiplex_active(previous) assert names[0] == sanitize_task_id_for_path("profile:work") assert sanitize_task_id_for_path("shared:team/workspace") not in names + assert "default" not in names def test_multiplex_probe_error_does_not_leak_default_shared_key(tmp_path, monkeypatch): @@ -462,7 +463,7 @@ def _boom(profile=None): names = _docker_sandbox_dir_candidates("agent:coder:telegram:dm:1") assert sanitize_task_id_for_path("shared:team/workspace") not in names assert names[0] == sanitize_task_id_for_path("profile:coder") - assert "default" in names + assert "default" not in names def test_sandbox_candidates_unknown_profile_does_not_use_default(monkeypatch): @@ -477,3 +478,79 @@ def _boom(): names = _docker_sandbox_dir_candidates("") assert names == [] assert "default" not in names + + +def _enable_docker_sandbox(tmp_path, monkeypatch): + sandbox = tmp_path / "sandboxes" + monkeypatch.setenv("TERMINAL_ENV", "docker") + monkeypatch.setenv("TERMINAL_CONTAINER_PERSISTENT", "true") + monkeypatch.setenv("TERMINAL_SANDBOX_DIR", str(sandbox)) + monkeypatch.delenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", raising=False) + monkeypatch.delenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", raising=False) + return sandbox + + +def _workspace(sandbox: Path, task_id: str) -> Path: + from tools.environments.base import sanitize_task_id_for_path + + name = task_id if task_id == "default" else sanitize_task_id_for_path(task_id) + ws = sandbox / "docker" / name / "workspace" + ws.mkdir(parents=True, exist_ok=True) + return ws + + +def _first_existing(roots, relative: str): + for root in roots: + candidate = root / relative + if candidate.is_file(): + return candidate.resolve() + return None + + +def test_named_profile_media_skips_unowned_default_workspace(tmp_path, monkeypatch): + # Default has foo.png, coder does not. Coder-routed MEDIA must not + # pick up the default profile's file. + from gateway.platforms.base import _default_docker_workspace_host_roots + + sandbox = _enable_docker_sandbox(tmp_path, monkeypatch) + default_ws = _workspace(sandbox, "default") + (default_ws / "foo.png").write_bytes(b"default-copy") + + roots = _default_docker_workspace_host_roots("agent:coder:telegram:dm:1") + assert default_ws.resolve() not in [r.resolve() for r in roots] + assert _first_existing(roots, "foo.png") is None + + +def test_named_profile_media_prefers_own_workspace_copy(tmp_path, monkeypatch): + from gateway.platforms.base import _default_docker_workspace_host_roots + + sandbox = _enable_docker_sandbox(tmp_path, monkeypatch) + default_ws = _workspace(sandbox, "default") + (default_ws / "foo.png").write_bytes(b"default-copy") + coder_ws = _workspace(sandbox, "profile:coder") + (coder_ws / "foo.png").write_bytes(b"coder-copy") + + roots = _default_docker_workspace_host_roots("agent:coder:telegram:dm:1") + found = _first_existing(roots, "foo.png") + assert found is not None + assert found.read_bytes() == b"coder-copy" + assert default_ws.resolve() not in [r.resolve() for r in roots] + + +def test_sandbox_roots_key_lookup_error_does_not_read_default_workspace(tmp_path, monkeypatch): + from gateway.platforms.base import _default_docker_workspace_host_roots + + sandbox = _enable_docker_sandbox(tmp_path, monkeypatch) + monkeypatch.setenv("TERMINAL_DOCKER_SHARED_CONTAINER_KEY", "team/workspace") + default_ws = _workspace(sandbox, "default") + (default_ws / "foo.png").write_bytes(b"default-copy") + + def _boom(profile=None): + raise RuntimeError("key lookup failed") + + monkeypatch.setattr( + "tools.terminal_tool.docker_shared_container_key_for_profile", _boom + ) + roots = _default_docker_workspace_host_roots("agent:coder:telegram:dm:1") + assert default_ws.resolve() not in [r.resolve() for r in roots] + assert _first_existing(roots, "foo.png") is None