fix(terminal): bridge terminal.backend config in serve/desktop processes lacking a launcher env bridge
terminal_tool reads all settings from TERMINAL_* env vars, bridged from config.yaml by the CLI, gateway, and TUI-PTY launchers. Processes that skip every launcher bridge — hermes serve / the Desktop app backend's in-process agents, the desktop cron ticker — saw an unset TERMINAL_ENV and silently ran every command on the host even when config.yaml selects terminal.backend: docker. A user who configured Docker isolation got unsandboxed host execution with no warning. Two layers: - _ensure_terminal_env_bridged() in _get_env_config(): when TERMINAL_ENV is unset, backfill TERMINAL_* from config.yaml via apply_terminal_config_to_env(override=False). Explicit env always wins (honor explicit choice; only fix the accidental fallback). One-shot, fail-open to the historical local default. - cmd_dashboard/serve: run the same bridge at startup so every consumer in the backend process (in-process agents, desktop cron ticker, tui_gateway cwd resolution) sees the bridged env directly. Fixes #63141, #54449, #61115, #65696.
This commit is contained in:
parent
fdbfae825e
commit
23526148d1
3 changed files with 178 additions and 0 deletions
|
|
@ -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.
|
||||
|
|
|
|||
119
tests/tools/test_terminal_env_bridge.py
Normal file
119
tests/tools/test_terminal_env_bridge.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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"}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue