mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-13 09:01:54 +00:00
fix(dashboard): profile-scope Channels endpoints and seed per-profile .env (#44792)
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.
This commit is contained in:
parent
e20e0bd744
commit
88dbf95105
5 changed files with 328 additions and 56 deletions
|
|
@ -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 <name> 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"
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
182
tests/hermes_cli/test_web_server_messaging_profiles.py
Normal file
182
tests/hermes_cli/test_web_server_messaging_profiles.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue