diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 2a29443d17845..53d8b30d4b607 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -12431,6 +12431,23 @@ def cmd_dashboard(args): # backend is the desktop's primary entrypoint and needs the same. _sync_bundled_skills_quietly() + # Bridge terminal.* config into the TERMINAL_* env vars for THIS process, + # mirroring the CLI (cli.py env_mappings) and gateway (gateway/run.py + # _terminal_env_map) startup bridges. The dashboard/serve backend runs + # agents in-process (tui_gateway.ws → server._make_agent) and ticks cron + # jobs itself when desktop-spawned — without this bridge those consumers + # saw an unset TERMINAL_ENV and silently ran every command on the host + # even when config.yaml selects `terminal.backend: docker` + # (#63141, #54449, #61115, #65696). PTY chat spawns already bridge their + # child env copy; this covers the in-process consumers. + try: + from hermes_cli.config import apply_terminal_config_to_env + + apply_terminal_config_to_env() + except Exception: + logger.debug("terminal config → env bridge failed for dashboard/serve", + exc_info=True) + if _headless_backend: # Don't build the SPA, and tell mount_spa() (read at web_server import # below) to disable it even if a stray dist exists. Set it first. diff --git a/tests/tools/test_terminal_env_bridge.py b/tests/tools/test_terminal_env_bridge.py new file mode 100644 index 0000000000000..3456650e9ac70 --- /dev/null +++ b/tests/tools/test_terminal_env_bridge.py @@ -0,0 +1,119 @@ +"""Regression tests for the terminal config → env fallback bridge. + +``terminal_tool._get_env_config()`` reads all settings from TERMINAL_* env +vars, which the CLI / gateway / TUI-PTY launchers bridge from config.yaml at +startup. Processes that skip every launcher bridge (``hermes serve`` and the +Desktop app's in-process agents, the desktop cron ticker, ACP) used to fall +back silently to the local backend even when config.yaml selected +``terminal.backend: docker`` — commands the user intended to sandbox ran on +the host (#63141 / #54449 / #61115 / #65696). + +``_ensure_terminal_env_bridged()`` closes that hole at the chokepoint: when +TERMINAL_ENV is unset, backfill TERMINAL_* from config.yaml before the +local default applies. An explicitly-set TERMINAL_ENV always wins. +""" + +import os + +import pytest + +import tools.terminal_tool as terminal_tool +from hermes_constants import get_hermes_home + + +@pytest.fixture(autouse=True) +def _reset_bridge_state(monkeypatch): + """Each test starts with an un-attempted bridge and no TERMINAL_ENV.""" + monkeypatch.setattr(terminal_tool, "_terminal_config_bridge_attempted", False) + monkeypatch.delenv("TERMINAL_ENV", raising=False) + monkeypatch.delenv("TERMINAL_CWD", raising=False) + monkeypatch.delenv("TERMINAL_DOCKER_IMAGE", raising=False) + # The config layer caches by (path, mtime, size); leave it alone — each + # test writes its own config.yaml which changes the signature. + yield + + +def _write_config(text: str) -> None: + home = get_hermes_home() + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text(text) + + +def test_unset_terminal_env_backfills_backend_from_config(): + """The core #63141 fix: config's docker backend reaches _get_env_config + even when no launcher bridged TERMINAL_ENV into this process.""" + _write_config( + "terminal:\n" + " backend: docker\n" + " docker_image: custom/image:1\n" + ) + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "docker" + assert config["docker_image"] == "custom/image:1" + assert os.environ.get("TERMINAL_ENV") == "docker" + + +def test_explicit_terminal_env_wins_over_config(monkeypatch): + """An explicit env choice (launcher bridge or .env) is never overridden — + honor explicit choice vs accidental fallback.""" + _write_config("terminal:\n backend: docker\n") + monkeypatch.setenv("TERMINAL_ENV", "local") + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "local" + + +def test_preset_terminal_vars_survive_backfill(monkeypatch): + """override=False: already-set sibling TERMINAL_* values stay + authoritative; only missing ones are backfilled.""" + _write_config( + "terminal:\n" + " backend: docker\n" + " docker_image: config/image:1\n" + ) + monkeypatch.setenv("TERMINAL_DOCKER_IMAGE", "env/image:2") + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "docker" + assert config["docker_image"] == "env/image:2" + + +def test_bridge_failure_falls_back_to_local(monkeypatch): + """A broken config layer must not take the terminal tool down.""" + + def _boom(*_a, **_k): + raise RuntimeError("config exploded") + + import hermes_cli.config as config_mod + + monkeypatch.setattr(config_mod, "apply_terminal_config_to_env", _boom) + + config = terminal_tool._get_env_config() + + assert config["env_type"] == "local" + + +def test_bridge_only_attempted_once(monkeypatch): + """The config load runs at most once per process when TERMINAL_ENV stays + unset (e.g. empty config) — later calls skip the bridge entirely.""" + calls = [] + + import hermes_cli.config as config_mod + + real = config_mod.apply_terminal_config_to_env + + def _counting(*a, **k): + calls.append(1) + return real(*a, **k) + + monkeypatch.setattr(config_mod, "apply_terminal_config_to_env", _counting) + _write_config("{}\n") + + terminal_tool._get_env_config() + terminal_tool._get_env_config() + + assert len(calls) == 1 diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 3eb32af274a79..9f7ed245cc57c 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -1305,10 +1305,52 @@ def _is_unusable_container_cwd(cwd: str) -> bool: return False +# One-shot guard for the config-fallback bridge below. Purely an +# optimization: after the first attempt either TERMINAL_ENV is set (bridge +# succeeded — merged config always carries terminal.backend) or the import +# failed and retrying every call would be wasted work. +_terminal_config_bridge_attempted = False + + +def _ensure_terminal_env_bridged() -> None: + """Backfill TERMINAL_* env vars from config.yaml when no launcher did. + + terminal_tool reads ALL terminal settings from os.environ (TERMINAL_*). + The CLI (cli.py ``env_mappings``), the gateway (gateway/run.py + ``_terminal_env_map``), and TUI/dashboard PTY launches + (``apply_terminal_config_to_env``) bridge ``terminal.*`` config into env + vars at startup — but processes that skip all of those paths (``hermes + serve`` / the Desktop app backend's in-process agents, the desktop cron + ticker, ACP) used to silently fall back to the local backend even when + config.yaml selects ``terminal.backend: docker``, running commands on the + host the user intended to sandbox (#63141, #54449, #61115, #65696). + + Explicit env always wins: when TERMINAL_ENV is already set (a launcher's + bridge or the user's .env made a deliberate choice) this is a no-op. The + config bridge only fills the unset case, so it changes an accidental + default — never an explicit selection. + """ + global _terminal_config_bridge_attempted + if "TERMINAL_ENV" in os.environ or _terminal_config_bridge_attempted: + return + _terminal_config_bridge_attempted = True + try: + from hermes_cli.config import apply_terminal_config_to_env + + # env=None targets os.environ inside the helper; override=False keeps + # any already-set TERMINAL_* values (e.g. from .env) authoritative. + apply_terminal_config_to_env(env=None, override=False) + except Exception: + # Never let a config problem take the terminal tool down — the + # historical local default still applies. + logger.debug("terminal config → env fallback bridge failed", exc_info=True) + + def _get_env_config() -> Dict[str, Any]: """Get terminal environment configuration from environment variables.""" # Default image with Python and Node.js for maximum compatibility default_image = "nikolaik/python-nodejs:python3.11-nodejs20" + _ensure_terminal_env_bridged() env_type = os.getenv("TERMINAL_ENV", "local") mount_docker_cwd = os.getenv("TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE", "false").lower() in {"true", "1", "yes"}