From 88dbf95105644efb067500136c4a73277d3936d4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 12 Jun 2026 02:09:28 -0700 Subject: [PATCH] fix(dashboard): profile-scope Channels endpoints and seed per-profile .env (#44792) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of the same community report (dashboard Profile Builder): 1. A fresh dashboard/CLI-created profile got no .env file unless cloned, so it silently inherited API keys and messaging tokens from the shell environment / root install. create_profile() now seeds a placeholder .env (0600) for non-clone profiles, matching the SOUL.md seeding. 2. The Channels endpoints (/api/messaging/platforms GET/PUT/test) were not profile-scoped: they read/wrote the dashboard process's own .env via load_env()/save_env_value() regardless of the global profile switcher. They now accept the standard optional profile param (body beats query on the PUT, matching other scoped writes) and run inside _profile_scope(). When scoped, the payload no longer falls back to os.environ or load_gateway_config()'s env-override layer — both carry the ROOT install's credentials and would misreport them as the profile's. /api/messaging/platforms added to PROFILE_SCOPED_PREFIXES so the sidebar switcher scopes the Channels page automatically. --- hermes_cli/profiles.py | 19 ++ hermes_cli/web_server.py | 149 +++++++++----- tests/hermes_cli/test_profiles.py | 28 ++- .../test_web_server_messaging_profiles.py | 182 ++++++++++++++++++ web/src/lib/api.ts | 6 +- 5 files changed, 328 insertions(+), 56 deletions(-) create mode 100644 tests/hermes_cli/test_web_server_messaging_profiles.py diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index b800665f6a8..d15cd0f7e87 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -835,6 +835,25 @@ def create_profile( dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dst) + # Seed an empty .env so the profile has its own credentials file from + # day one. Without it, profile-scoped env writes (dashboard Channels / + # Keys pages, `hermes -p auth add`) had no file until first + # write, and the profile silently inherited API keys from the shell + # environment — users reasonably read that as "the new profile reads + # the root .env". Skipped when --clone/--clone-all already copied one. + env_path = profile_dir / ".env" + if not env_path.exists(): + try: + env_path.write_text( + "# Per-profile secrets for this Hermes profile.\n" + "# API keys and tokens set here override the shell environment.\n" + "# Behavioral settings belong in config.yaml, not here.\n", + encoding="utf-8", + ) + os.chmod(str(env_path), 0o600) + except OSError: + pass # best-effort — save_env_value creates the file on demand + # Seed a default SOUL.md so the user has a file to customize immediately. # Skipped when the profile already has one (from --clone / --clone-all). soul_path = profile_dir / "SOUL.md" diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 6ae7727d686..9e295d5ccea 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -654,6 +654,9 @@ class MessagingPlatformUpdate(BaseModel): enabled: Optional[bool] = None env: Dict[str, str] = {} clear_env: List[str] = [] + # Explicit body profile beats the query param injected by the global + # dashboard profile switcher (same precedence as other scoped writes). + profile: Optional[str] = None class TelegramOnboardingStart(BaseModel): @@ -4209,7 +4212,10 @@ def _gateway_platform_config(platform_id: str): def _messaging_platform_payload( - entry: dict[str, Any], env_on_disk: dict[str, str], runtime: dict | None + entry: dict[str, Any], + env_on_disk: dict[str, str], + runtime: dict | None, + scoped: bool = False, ) -> dict[str, Any]: platform_id = entry["id"] gateway_running = get_running_pid() is not None @@ -4222,7 +4228,11 @@ def _messaging_platform_payload( env_vars = [] for key in entry["env_vars"]: - value = env_on_disk.get(key) or os.getenv(key, "") + # When profile-scoped, judge only the profile's own .env — the + # dashboard process's os.environ carries the ROOT install's .env + # (loaded at startup) and would falsely report the root credentials + # as the profile's. + value = env_on_disk.get(key) or ("" if scoped else os.getenv(key, "")) env_vars.append( { "key": key, @@ -4233,26 +4243,46 @@ def _messaging_platform_payload( } ) - try: - gateway_config, platform, platform_config = _gateway_platform_config( - platform_id - ) - enabled = bool(platform_config and platform_config.enabled) - configured = bool( - platform_config - and gateway_config._is_platform_connected(platform, platform_config) - ) - home_channel = ( - platform_config.home_channel.to_dict() - if platform_config and platform_config.home_channel - else None - ) - except Exception: - enabled = False - configured = all( - env_on_disk.get(key) or os.getenv(key, "") for key in entry["required_env"] - ) - home_channel = None + if scoped: + # Profile-scoped view: derive enablement/configuration from the + # profile's config.yaml + .env only. load_gateway_config()'s + # env-override layer reads os.environ and would leak the root + # install's tokens into the profile's reported state. + try: + cfg = load_config() + platforms_cfg = cfg.get("platforms") or {} + plat_cfg = platforms_cfg.get(platform_id) + if not isinstance(plat_cfg, dict): + plat_cfg = {} + enabled = bool(plat_cfg.get("enabled")) + hc = plat_cfg.get("home_channel") + home_channel = hc if isinstance(hc, dict) else None + except Exception: + enabled = False + home_channel = None + configured = all(env_on_disk.get(key) for key in entry["required_env"]) + else: + try: + gateway_config, platform, platform_config = _gateway_platform_config( + platform_id + ) + enabled = bool(platform_config and platform_config.enabled) + configured = bool( + platform_config + and gateway_config._is_platform_connected(platform, platform_config) + ) + home_channel = ( + platform_config.home_channel.to_dict() + if platform_config and platform_config.home_channel + else None + ) + except Exception: + enabled = False + configured = all( + env_on_disk.get(key) or os.getenv(key, "") + for key in entry["required_env"] + ) + home_channel = None state = ( runtime_platform.get("state") if isinstance(runtime_platform, dict) else None @@ -4675,19 +4705,28 @@ async def cancel_telegram_onboarding(pairing_id: str): @app.get("/api/messaging/platforms") -async def get_messaging_platforms(): - env_on_disk = load_env() - runtime = read_runtime_status() - return { - "platforms": [ - _messaging_platform_payload(entry, env_on_disk, runtime) - for entry in _messaging_platform_catalog() - ] - } +async def get_messaging_platforms(profile: Optional[str] = None): + # Profile-scoped so the dashboard's global profile switcher shows the + # TARGET profile's channel credentials/state, not the root install's. + # Inside _profile_scope, load_env()/read_runtime_status()/get_running_pid() + # all resolve against the requested profile's HERMES_HOME. + with _profile_scope(profile) as scoped_dir: + env_on_disk = load_env() + runtime = read_runtime_status() + return { + "platforms": [ + _messaging_platform_payload( + entry, env_on_disk, runtime, scoped=scoped_dir is not None + ) + for entry in _messaging_platform_catalog() + ] + } @app.put("/api/messaging/platforms/{platform_id}") -async def update_messaging_platform(platform_id: str, body: MessagingPlatformUpdate): +async def update_messaging_platform( + platform_id: str, body: MessagingPlatformUpdate, profile: Optional[str] = None +): entry = _catalog_lookup(platform_id) if not entry: raise HTTPException( @@ -4696,26 +4735,27 @@ async def update_messaging_platform(platform_id: str, body: MessagingPlatformUpd allowed_env = set(entry["env_vars"]) try: - for key in body.clear_env: - if key not in allowed_env: - raise HTTPException( - status_code=400, - detail=f"{key} is not configurable for {entry['name']}", - ) - remove_env_value(key) + with _profile_scope(body.profile or profile): + for key in body.clear_env: + if key not in allowed_env: + raise HTTPException( + status_code=400, + detail=f"{key} is not configurable for {entry['name']}", + ) + remove_env_value(key) - for key, value in body.env.items(): - if key not in allowed_env: - raise HTTPException( - status_code=400, - detail=f"{key} is not configurable for {entry['name']}", - ) - trimmed = value.strip() - if trimmed: - save_env_value(key, trimmed) + for key, value in body.env.items(): + if key not in allowed_env: + raise HTTPException( + status_code=400, + detail=f"{key} is not configurable for {entry['name']}", + ) + trimmed = value.strip() + if trimmed: + save_env_value(key, trimmed) - if body.enabled is not None: - _write_platform_enabled(platform_id, body.enabled) + if body.enabled is not None: + _write_platform_enabled(platform_id, body.enabled) return {"ok": True, "platform": platform_id} except HTTPException: @@ -4726,15 +4766,18 @@ async def update_messaging_platform(platform_id: str, body: MessagingPlatformUpd @app.post("/api/messaging/platforms/{platform_id}/test") -async def test_messaging_platform(platform_id: str): +async def test_messaging_platform(platform_id: str, profile: Optional[str] = None): entry = _catalog_lookup(platform_id) if not entry: raise HTTPException( status_code=404, detail=f"Unknown messaging platform: {platform_id}" ) - env_on_disk = load_env() - payload = _messaging_platform_payload(entry, env_on_disk, read_runtime_status()) + with _profile_scope(profile) as scoped_dir: + env_on_disk = load_env() + payload = _messaging_platform_payload( + entry, env_on_disk, read_runtime_status(), scoped=scoped_dir is not None + ) if not payload["enabled"]: message = f"{entry['name']} is disabled. Enable it, then restart the gateway." return {"ok": False, "state": payload["state"], "message": message} diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 310a47515d0..31d56b9b983 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -158,6 +158,30 @@ class TestCreateProfile: "plans", "workspace", "cron"]: assert (profile_dir / subdir).is_dir(), f"Missing subdir: {subdir}" + def test_seeds_placeholder_env_file(self, profile_env): + """Fresh profiles get their own .env (owner-only) so channel/env + writes are profile-scoped from day one instead of falling through + to the shell environment / root install.""" + import stat + profile_dir = create_profile("coder", no_alias=True) + env_path = profile_dir / ".env" + assert env_path.exists() + content = env_path.read_text(encoding="utf-8") + # Placeholder only — no credentials leak in from anywhere. + assert all( + line.startswith("#") or not line.strip() + for line in content.splitlines() + ) + mode = stat.S_IMODE(env_path.stat().st_mode) + assert mode == 0o600 + + def test_seeded_env_does_not_clobber_cloned_env(self, profile_env): + tmp_path = profile_env + default_home = tmp_path / ".hermes" + (default_home / ".env").write_text("KEY=val") + profile_dir = create_profile("coder", clone_config=True, no_alias=True) + assert (profile_dir / ".env").read_text() == "KEY=val" + def test_duplicate_raises_file_exists(self, profile_env): create_profile("coder", no_alias=True) with pytest.raises(FileExistsError): @@ -304,7 +328,9 @@ class TestCreateProfile: profile_dir = create_profile("coder", clone_config=True, no_alias=True) # No error; optional files just not copied assert not (profile_dir / "config.yaml").exists() - assert not (profile_dir / ".env").exists() + # .env is always seeded (placeholder) so the profile has its own + # credentials file even when the clone source lacked one. + assert (profile_dir / ".env").exists() # SOUL.md is always seeded with the default even when clone source lacks it assert (profile_dir / "SOUL.md").exists() diff --git a/tests/hermes_cli/test_web_server_messaging_profiles.py b/tests/hermes_cli/test_web_server_messaging_profiles.py new file mode 100644 index 00000000000..3627ad6eea6 --- /dev/null +++ b/tests/hermes_cli/test_web_server_messaging_profiles.py @@ -0,0 +1,182 @@ +"""Regression tests for profile-scoped dashboard Channels endpoints. + +Before the ``profile`` parameter existed, ``/api/messaging/platforms`` always +read/wrote the dashboard process's own (root) ``.env`` via ``load_env()`` / +``save_env_value()`` — so a dashboard switched to a freshly created profile +still displayed and persisted the ROOT install's messaging credentials. +These tests pin the new behavior: reads and writes land in the REQUESTED +profile's HERMES_HOME, and the dashboard's own profile stays untouched. +""" +import pytest +import yaml + + +@pytest.fixture +def isolated_profiles(tmp_path, monkeypatch, _isolate_hermes_home): + """Isolated default home + one named profile, each with its own .env.""" + from hermes_constants import get_hermes_home + from hermes_cli import profiles + + default_home = get_hermes_home() + profiles_root = default_home / "profiles" + worker_home = profiles_root / "worker_alpha" + for home in (default_home, worker_home): + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text("{}\n", encoding="utf-8") + + (default_home / ".env").write_text( + "TELEGRAM_BOT_TOKEN=root-token\n", encoding="utf-8" + ) + (worker_home / ".env").write_text("", encoding="utf-8") + + monkeypatch.setattr(profiles, "_get_default_hermes_home", lambda: default_home) + monkeypatch.setattr(profiles, "_get_profiles_root", lambda: profiles_root) + return {"default": default_home, "worker_alpha": worker_home} + + +@pytest.fixture +def client(monkeypatch, isolated_profiles): + try: + from starlette.testclient import TestClient + except ImportError: + pytest.skip("fastapi/starlette not installed") + + import hermes_state + from hermes_constants import get_hermes_home + from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN + + monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db") + # The dashboard process's os.environ may carry root-install credentials; + # make sure the scoped path never falls back to them. + monkeypatch.delenv("TELEGRAM_BOT_TOKEN", raising=False) + c = TestClient(app) + c.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return c + + +def _telegram(payload): + return next(p for p in payload["platforms"] if p["id"] == "telegram") + + +def _env_field(platform, key): + return next(f for f in platform["env_vars"] if f["key"] == key) + + +class TestProfileScopedMessagingReads: + def test_scoped_read_does_not_show_root_credentials( + self, client, isolated_profiles + ): + resp = client.get( + "/api/messaging/platforms", params={"profile": "worker_alpha"} + ) + assert resp.status_code == 200 + telegram = _telegram(resp.json()) + token = _env_field(telegram, "TELEGRAM_BOT_TOKEN") + # The worker profile has an empty .env — the root token must not leak. + assert token["is_set"] is False + assert telegram["configured"] is False + + def test_unscoped_read_shows_dashboard_profile_env( + self, client, isolated_profiles + ): + resp = client.get("/api/messaging/platforms") + assert resp.status_code == 200 + telegram = _telegram(resp.json()) + token = _env_field(telegram, "TELEGRAM_BOT_TOKEN") + assert token["is_set"] is True + + def test_unknown_profile_returns_404(self, client, isolated_profiles): + resp = client.get( + "/api/messaging/platforms", params={"profile": "no_such_profile"} + ) + assert resp.status_code == 404 + + +class TestProfileScopedMessagingWrites: + def test_scoped_write_lands_in_target_profile_env( + self, client, isolated_profiles + ): + resp = client.put( + "/api/messaging/platforms/telegram", + params={"profile": "worker_alpha"}, + json={ + "enabled": True, + "env": {"TELEGRAM_BOT_TOKEN": "worker-token"}, + }, + ) + assert resp.status_code == 200 + + worker_env = ( + isolated_profiles["worker_alpha"] / ".env" + ).read_text(encoding="utf-8") + assert "TELEGRAM_BOT_TOKEN=worker-token" in worker_env + + # The dashboard's own .env must stay untouched — this was the bug. + root_env = (isolated_profiles["default"] / ".env").read_text( + encoding="utf-8" + ) + assert "worker-token" not in root_env + assert "TELEGRAM_BOT_TOKEN=root-token" in root_env + + # Enablement lands in the target profile's config.yaml. + worker_cfg = yaml.safe_load( + (isolated_profiles["worker_alpha"] / "config.yaml").read_text() + ) or {} + assert worker_cfg.get("platforms", {}).get("telegram", {}).get("enabled") is True + root_cfg = yaml.safe_load( + (isolated_profiles["default"] / "config.yaml").read_text() + ) or {} + assert "telegram" not in (root_cfg.get("platforms") or {}) + + def test_body_profile_beats_query_param(self, client, isolated_profiles): + resp = client.put( + "/api/messaging/platforms/telegram", + json={ + "env": {"TELEGRAM_BOT_TOKEN": "body-token"}, + "profile": "worker_alpha", + }, + ) + assert resp.status_code == 200 + worker_env = ( + isolated_profiles["worker_alpha"] / ".env" + ).read_text(encoding="utf-8") + assert "TELEGRAM_BOT_TOKEN=body-token" in worker_env + + def test_scoped_read_after_scoped_write_round_trips( + self, client, isolated_profiles + ): + client.put( + "/api/messaging/platforms/telegram", + params={"profile": "worker_alpha"}, + json={"enabled": True, "env": {"TELEGRAM_BOT_TOKEN": "worker-token"}}, + ) + resp = client.get( + "/api/messaging/platforms", params={"profile": "worker_alpha"} + ) + telegram = _telegram(resp.json()) + assert telegram["enabled"] is True + assert _env_field(telegram, "TELEGRAM_BOT_TOKEN")["is_set"] is True + assert telegram["configured"] is True + + def test_scoped_clear_env_removes_from_target_only( + self, client, isolated_profiles + ): + client.put( + "/api/messaging/platforms/telegram", + params={"profile": "worker_alpha"}, + json={"env": {"TELEGRAM_BOT_TOKEN": "worker-token"}}, + ) + resp = client.put( + "/api/messaging/platforms/telegram", + params={"profile": "worker_alpha"}, + json={"clear_env": ["TELEGRAM_BOT_TOKEN"]}, + ) + assert resp.status_code == 200 + worker_env = ( + isolated_profiles["worker_alpha"] / ".env" + ).read_text(encoding="utf-8") + assert "worker-token" not in worker_env + root_env = (isolated_profiles["default"] / ".env").read_text( + encoding="utf-8" + ) + assert "TELEGRAM_BOT_TOKEN=root-token" in root_env diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index f3b62fb0f87..6af6e8a6cc6 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -60,14 +60,16 @@ export function getManagementProfile(): string { // Endpoint families that honor ?profile= on the backend (web_server.py // _profile_scope). Anything else — sessions, analytics, ops, pairing, -// channels, cron (which has its own per-job profile params), profiles -// themselves — is machine-global or self-scoped and must NOT be rewritten. +// telegram onboarding, cron (which has its own per-job profile params), +// profiles themselves — is machine-global or self-scoped and must NOT be +// rewritten. const PROFILE_SCOPED_PREFIXES = [ "/api/skills", "/api/tools/toolsets", "/api/config", "/api/env", "/api/mcp", + "/api/messaging/platforms", "/api/model/info", "/api/model/set", "/api/model/auxiliary",