fix(dashboard): isolate the environment of named-profile actions
_spawn_hermes_action copied the dashboard's os.environ verbatim into every detached `hermes ...` action. The dashboard runs inside the gateway and has loaded its own profile's .env into the process environment, so `hermes -p <other> gateway restart` (and every other dashboard-driven profile action) started with the DEFAULT profile's platform credentials and ports already present. load_hermes_dotenv does not override keys that are already set, so the named profile's own .env could not displace them: an A2A-only profile ended up claiming the default Discord bot token and binding the default API server / BlueBubbles ports. For actions carrying a profile selector (`-p X`, `--profile X`, `--profile=X`), build the child env from the standard scrubbed subprocess environment, drop _PROFILE_MANAGED_ENV_KEYS plus every key defined by the dashboard/default profile's .env and its hydrated secret sources, and pin HERMES_HOME to the target profile so the child's normal startup loads that profile's .env. Only the leading selector is inspected: argv after the subcommand may legitimately contain -p for a nested process. Actions without a selector keep the historical environment byte-for-byte. Test: the new case asserts the leaked keys are gone, benign keys survive, and (by running the real dotenv loader in a fresh interpreter with the captured env) that the target profile's values load without reviving any default-profile value. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -322,6 +322,78 @@ def _dashboard_spawn_executable() -> str:
|
||||
return sys.executable
|
||||
|
||||
|
||||
def _named_profile_from_action(subcommand: List[str]) -> Optional[str]:
|
||||
"""Return the named-profile selector that :func:`_profile_cli_args` puts in front of an action.
|
||||
|
||||
Deliberately inspects only the leading selector: values after the real subcommand may
|
||||
legitimately contain ``-p`` / ``--profile`` for a nested process (``mcp add --args ...``).
|
||||
"""
|
||||
if len(subcommand) >= 2 and subcommand[0] in {"-p", "--profile"}:
|
||||
return str(subcommand[1]).strip() or None
|
||||
if subcommand and str(subcommand[0]).startswith("--profile="):
|
||||
return str(subcommand[0]).split("=", 1)[1].strip() or None
|
||||
return None
|
||||
|
||||
|
||||
def _profile_action_environment(
|
||||
subcommand: List[str], env_overrides: Optional[Dict[str, str]] = None,
|
||||
) -> Dict[str, str]:
|
||||
"""Environment for a detached ``hermes <subcommand>`` action.
|
||||
|
||||
The dashboard loads its own profile's ``.env`` into process-global ``os.environ``. Copying
|
||||
that mapping verbatim into ``hermes -p <other> ...`` lets the named child see the dashboard
|
||||
profile's platform credentials and ports *before* its own dotenv loads (``load_hermes_dotenv``
|
||||
does not override keys already present): a supposedly A2A-only profile then claims the default
|
||||
Discord token and binds the default API/BlueBubbles ports.
|
||||
|
||||
Named-profile actions therefore start from Hermes' standard scrubbed subprocess env, then drop
|
||||
the profile-managed keys plus every key declared by the dashboard/default profile dotenv files
|
||||
and their hydrated secret sources, and pin ``HERMES_HOME`` to the target profile. The child's
|
||||
normal startup then loads that profile's own ``.env``. Actions without a profile selector keep
|
||||
the historical environment exactly.
|
||||
"""
|
||||
profile = _named_profile_from_action(subcommand)
|
||||
if profile is None:
|
||||
action_env = dict(os.environ)
|
||||
else:
|
||||
from hermes_cli.env_loader import (
|
||||
_PROFILE_MANAGED_ENV_KEYS, _env_keys_defined_in_dotenv, get_secret_source_values,
|
||||
)
|
||||
from hermes_cli.web_server_profiles import _resolve_profile_dir
|
||||
from hermes_constants import apply_subprocess_home_env, get_default_hermes_root
|
||||
from tools.environments.local import build_subprocess_env
|
||||
|
||||
target_home = _resolve_profile_dir(profile)
|
||||
action_env = build_subprocess_env(base=os.environ, scrub_secrets=True)
|
||||
|
||||
profile_keys = set(_PROFILE_MANAGED_ENV_KEYS)
|
||||
try:
|
||||
source_homes = {str(get_default_hermes_root()), str(get_hermes_home())}
|
||||
except Exception:
|
||||
source_homes = set()
|
||||
for source_home in source_homes:
|
||||
profile_keys.update(_env_keys_defined_in_dotenv(Path(source_home) / ".env"))
|
||||
# Secret managers contribute locally named credentials that never appear in .env;
|
||||
# the dashboard already hydrated its own sources, so their key names are a boundary too.
|
||||
profile_keys.update(get_secret_source_values(source_home).keys())
|
||||
for key in profile_keys:
|
||||
action_env.pop(key, None)
|
||||
|
||||
# Pin the child before import-time startup runs; the explicit -p flag stays authoritative
|
||||
# and resolves to the same validated directory.
|
||||
action_env["HERMES_HOME"] = str(target_home)
|
||||
apply_subprocess_home_env(action_env)
|
||||
|
||||
action_env["HERMES_NONINTERACTIVE"] = "1"
|
||||
# The dashboard runs inside the gateway process, so os.environ carries _HERMES_GATEWAY=1;
|
||||
# inheriting it trips the child's in-process restart-loop guard (exit 1). Drop it, like
|
||||
# the gateway's own restart watcher does (gateway/run.py, #52470).
|
||||
action_env.pop("_HERMES_GATEWAY", None)
|
||||
if env_overrides:
|
||||
action_env.update(env_overrides)
|
||||
return action_env
|
||||
|
||||
|
||||
def _spawn_hermes_action(
|
||||
subcommand: List[str], name: str, *, env_overrides: Optional[Dict[str, str]] = None
|
||||
) -> subprocess.Popen:
|
||||
@@ -332,16 +404,13 @@ def _spawn_hermes_action(
|
||||
log_file.write(f"\n=== {name} started {time.strftime('%Y-%m-%d %H:%M:%S')} ===\n".encode())
|
||||
|
||||
cmd = [_dashboard_spawn_executable(), "-m", "hermes_cli.main", *subcommand]
|
||||
# The dashboard runs inside the gateway process, so os.environ carries _HERMES_GATEWAY=1;
|
||||
# inheriting it trips the child's in-process restart-loop guard (exit 1). Drop it, like
|
||||
# the gateway's own restart watcher does.
|
||||
# The gateway's own restart watcher already drops it (gateway/run.py); mirror that here (#52470).
|
||||
action_env = {**os.environ, "HERMES_NONINTERACTIVE": "1"}
|
||||
action_env.pop("_HERMES_GATEWAY", None)
|
||||
# Named-profile actions get a scrubbed, pinned environment so the child cannot inherit the
|
||||
# dashboard profile's credentials; see _profile_action_environment (also drops _HERMES_GATEWAY).
|
||||
action_env = _profile_action_environment(subcommand, env_overrides)
|
||||
detach = {"creationflags": windows_detach_flags()} if sys.platform == "win32" else {"start_new_session": True}
|
||||
proc = subprocess.Popen(
|
||||
cmd, cwd=str(PROJECT_ROOT), stdin=subprocess.DEVNULL, stdout=log_file, stderr=subprocess.STDOUT,
|
||||
env={**action_env, **(env_overrides or {})}, **detach,
|
||||
env=action_env, **detach,
|
||||
)
|
||||
log_file.close() # child holds its own dup'd fd; keeping ours leaks one per action
|
||||
_ACTION_RESULTS.pop(name, None)
|
||||
|
||||
@@ -1015,6 +1015,7 @@ def test_spawn_hermes_action_scrubs_gateway_loop_guard_env(monkeypatch, tmp_path
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
monkeypatch.setenv("_HERMES_GATEWAY", "1")
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "default-action-provider-key")
|
||||
monkeypatch.setattr(_web_server_gateway, "_ACTION_LOG_DIR", tmp_path)
|
||||
# Isolate the module-global proc registry: _spawn_hermes_action stores
|
||||
# _FakeProc (no poll()) in _ACTION_PROCS, and later tests' lifespan
|
||||
@@ -1036,6 +1037,135 @@ def test_spawn_hermes_action_scrubs_gateway_loop_guard_env(monkeypatch, tmp_path
|
||||
|
||||
assert "_HERMES_GATEWAY" not in captured["env"]
|
||||
assert captured["env"]["HERMES_NONINTERACTIVE"] == "1"
|
||||
# Default-profile actions preserve the historical process environment.
|
||||
assert captured["env"]["OPENAI_API_KEY"] == "default-action-provider-key"
|
||||
|
||||
|
||||
def test_named_profile_action_isolates_parent_env_and_loads_target_env(monkeypatch, tmp_path):
|
||||
"""A dashboard action for a named profile must not borrow the dashboard profile's
|
||||
platform/provider environment, while the target profile's own dotenv still loads in the child."""
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import hermes_cli.env_loader as env_loader
|
||||
import hermes_cli.web_server as ws
|
||||
|
||||
user_home = tmp_path / "user"
|
||||
default_home = user_home / ".hermes"
|
||||
target_home = default_home / "profiles" / "verifier"
|
||||
target_home.mkdir(parents=True)
|
||||
|
||||
(default_home / ".env").write_text(
|
||||
"\n".join([
|
||||
"DISCORD_BOT_TOKEN=default-discord",
|
||||
"API_SERVER_ENABLED=true",
|
||||
"API_SERVER_KEY=default-api-server",
|
||||
"BLUEBUBBLES_SERVER_URL=http://127.0.0.1:1234",
|
||||
"BLUEBUBBLES_PASSWORD=default-bluebubbles",
|
||||
"NTFY_TOPIC=default-topic",
|
||||
"NTFY_TOKEN=default-ntfy",
|
||||
"OPENAI_API_KEY=default-openai",
|
||||
"ZAI_API_KEY=default-zai",
|
||||
# Locally named routing credentials must be isolated too, even without a secret suffix.
|
||||
"A2A_AUTH_MINI=default-a2a-auth",
|
||||
]) + "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
(target_home / ".env").write_text(
|
||||
"\n".join(["A2A_PORT=9917", "OPENAI_API_KEY=target-openai", "TARGET_ONLY_TOKEN=target-only"]) + "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setattr(Path, "home", lambda: user_home)
|
||||
monkeypatch.setenv("HERMES_HOME", str(default_home))
|
||||
for key, value in {
|
||||
"DISCORD_BOT_TOKEN": "default-discord",
|
||||
"API_SERVER_ENABLED": "true",
|
||||
"API_SERVER_KEY": "default-api-server",
|
||||
"BLUEBUBBLES_SERVER_URL": "http://127.0.0.1:1234",
|
||||
"BLUEBUBBLES_PASSWORD": "default-bluebubbles",
|
||||
"NTFY_TOPIC": "default-topic",
|
||||
"NTFY_TOKEN": "default-ntfy",
|
||||
"OPENAI_API_KEY": "default-openai",
|
||||
"ZAI_API_KEY": "default-zai",
|
||||
"A2A_AUTH_MINI": "default-a2a-auth",
|
||||
"EXTERNAL_PROFILE_AUTH": "default-secret-source-auth",
|
||||
"HERMES_ACP_AUTH_METHOD": "default-acp",
|
||||
"PROFILE_ENV_TEST_BENIGN": "keep-me",
|
||||
}.items():
|
||||
monkeypatch.setenv(key, value)
|
||||
monkeypatch.setattr(_web_server_gateway, "_ACTION_LOG_DIR", tmp_path / "logs")
|
||||
monkeypatch.setattr(_web_server_gateway, "_ACTION_PROCS", {})
|
||||
monkeypatch.setattr(
|
||||
env_loader,
|
||||
"get_secret_source_values",
|
||||
lambda home: (
|
||||
{"EXTERNAL_PROFILE_AUTH": "default-secret-source-auth"}
|
||||
if Path(home).resolve() == default_home.resolve() else {}
|
||||
),
|
||||
)
|
||||
|
||||
captured = {}
|
||||
real_popen = subprocess.Popen
|
||||
|
||||
class _FakeProc:
|
||||
pid = 4321
|
||||
|
||||
def _fake_popen(cmd, **kwargs):
|
||||
captured["cmd"] = cmd
|
||||
captured["env"] = kwargs["env"]
|
||||
return _FakeProc()
|
||||
|
||||
monkeypatch.setattr(ws.subprocess, "Popen", _fake_popen)
|
||||
_web_server_gateway._spawn_hermes_action(["-p", "verifier", "gateway", "restart"], "gateway-restart")
|
||||
|
||||
child_env = captured["env"]
|
||||
assert captured["cmd"][-4:] == ["-p", "verifier", "gateway", "restart"]
|
||||
assert child_env["HERMES_HOME"] == str(target_home)
|
||||
assert child_env["HERMES_NONINTERACTIVE"] == "1"
|
||||
assert child_env["PROFILE_ENV_TEST_BENIGN"] == "keep-me"
|
||||
for leaked_key in (
|
||||
"DISCORD_BOT_TOKEN", "API_SERVER_ENABLED", "API_SERVER_KEY", "BLUEBUBBLES_SERVER_URL",
|
||||
"BLUEBUBBLES_PASSWORD", "NTFY_TOPIC", "NTFY_TOKEN", "OPENAI_API_KEY", "ZAI_API_KEY",
|
||||
"A2A_AUTH_MINI", "EXTERNAL_PROFILE_AUTH", "HERMES_ACP_AUTH_METHOD",
|
||||
):
|
||||
assert leaked_key not in child_env, leaked_key
|
||||
|
||||
# Exercise the real dotenv loader in a fresh interpreter with precisely the environment handed
|
||||
# to the named child: the target profile's own values must load without reviving any
|
||||
# default-profile value.
|
||||
monkeypatch.setattr(ws.subprocess, "Popen", real_popen)
|
||||
probe = subprocess.run(
|
||||
[
|
||||
sys.executable, "-c",
|
||||
"import json, os; "
|
||||
"from hermes_cli.env_loader import load_hermes_dotenv; "
|
||||
"load_hermes_dotenv(hermes_home=os.environ['HERMES_HOME']); "
|
||||
"keys=['A2A_PORT','OPENAI_API_KEY','TARGET_ONLY_TOKEN','DISCORD_BOT_TOKEN',"
|
||||
"'API_SERVER_ENABLED','API_SERVER_KEY','BLUEBUBBLES_SERVER_URL','BLUEBUBBLES_PASSWORD',"
|
||||
"'NTFY_TOPIC','NTFY_TOKEN','ZAI_API_KEY','A2A_AUTH_MINI','EXTERNAL_PROFILE_AUTH']; "
|
||||
"print(json.dumps({key: os.environ.get(key) for key in keys}))",
|
||||
],
|
||||
cwd=Path(ws.PROJECT_ROOT), env=child_env, check=True, capture_output=True, text=True,
|
||||
)
|
||||
loaded = json.loads(probe.stdout.strip().splitlines()[-1])
|
||||
assert loaded == {
|
||||
"A2A_PORT": "9917",
|
||||
"OPENAI_API_KEY": "target-openai",
|
||||
"TARGET_ONLY_TOKEN": "target-only",
|
||||
"DISCORD_BOT_TOKEN": None,
|
||||
"API_SERVER_ENABLED": None,
|
||||
"API_SERVER_KEY": None,
|
||||
"BLUEBUBBLES_SERVER_URL": None,
|
||||
"BLUEBUBBLES_PASSWORD": None,
|
||||
"NTFY_TOPIC": None,
|
||||
"NTFY_TOKEN": None,
|
||||
"ZAI_API_KEY": None,
|
||||
"A2A_AUTH_MINI": None,
|
||||
"EXTERNAL_PROFILE_AUTH": None,
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user