mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(dashboard): reject port-binding channels on secondary multiplexed profiles
The Channels API (PUT /api/messaging/platforms/{id}) accepted and persisted
enabling a port-binding platform on a secondary profile while
gateway.multiplex_profiles is on — a config the gateway only rejects on its
next start, aborting startup with MultiplexConfigError for every multiplexed
profile.
Validate before any .env/config.yaml write and return 409 for the enable
attempt. Disabling and clearing env stay allowed so an already-invalid
profile can be repaired. The port-binding platform set moves to
gateway/config.py (PORT_BINDING_PLATFORM_VALUES) as the single source of
truth shared by gateway startup validation and the dashboard, so the two
policies cannot drift. Platform config mutations now get a names-only audit
log line.
Fixes #62791
This commit is contained in:
parent
3366204474
commit
e984a61306
4 changed files with 251 additions and 19 deletions
|
|
@ -313,6 +313,27 @@ class Platform(Enum):
|
|||
_BUILTIN_PLATFORM_VALUES = frozenset(m.value for m in Platform.__members__.values())
|
||||
|
||||
|
||||
# Platforms that bind a host TCP port (HTTP/webhook listeners). In a profile
|
||||
# multiplexer the default profile owns the single shared listener and serves
|
||||
# every profile through the /p/<profile>/ URL prefix, so a SECONDARY profile
|
||||
# enabling one of these is always a misconfiguration: it would try to bind a
|
||||
# port already held by the default's listener. Single source of truth for
|
||||
# both the gateway's fail-fast startup validation (gateway/run.py) and the
|
||||
# dashboard's pre-write mutation validation (hermes_cli/web_server.py) so
|
||||
# the two policies cannot drift. Stored as platform .value strings.
|
||||
PORT_BINDING_PLATFORM_VALUES = frozenset({
|
||||
"webhook",
|
||||
"api_server",
|
||||
"msgraph_webhook",
|
||||
"feishu",
|
||||
"wecom_callback",
|
||||
"bluebubbles",
|
||||
"sms",
|
||||
"whatsapp_cloud",
|
||||
"line",
|
||||
})
|
||||
|
||||
|
||||
@dataclass
|
||||
class HomeChannel:
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -1408,24 +1408,14 @@ def _current_max_iterations() -> int:
|
|||
from contextlib import contextmanager as _contextmanager
|
||||
|
||||
|
||||
# Platforms that bind a host TCP port (HTTP/webhook listeners). In a profile
|
||||
# multiplexer the default profile owns the single shared listener and serves
|
||||
# every profile through the /p/<profile>/ URL prefix, so a SECONDARY profile
|
||||
# enabling one of these is always a misconfiguration: it would try to bind a
|
||||
# port already held by the default's listener. We hard-error on it rather than
|
||||
# silently dropping the adapter (see _start_one_profile_adapters).
|
||||
# Stored as platform .value strings since the Platform enum is imported below.
|
||||
_PORT_BINDING_PLATFORM_VALUES = frozenset({
|
||||
"webhook",
|
||||
"api_server",
|
||||
"msgraph_webhook",
|
||||
"feishu",
|
||||
"wecom_callback",
|
||||
"bluebubbles",
|
||||
"sms",
|
||||
"whatsapp_cloud",
|
||||
"line",
|
||||
})
|
||||
# Platforms that bind a host TCP port (HTTP/webhook listeners). A SECONDARY
|
||||
# profile enabling one of these under gateway.multiplex_profiles is always a
|
||||
# misconfiguration — we hard-error on it rather than silently dropping the
|
||||
# adapter (see _start_one_profile_adapters). The set lives in gateway.config
|
||||
# so the dashboard's pre-write validation enforces the same policy.
|
||||
from gateway.config import (
|
||||
PORT_BINDING_PLATFORM_VALUES as _PORT_BINDING_PLATFORM_VALUES,
|
||||
)
|
||||
|
||||
|
||||
class MultiplexConfigError(RuntimeError):
|
||||
|
|
|
|||
|
|
@ -2467,7 +2467,7 @@ async def git_branch_switch_route(body: GitBranchSwitchBody):
|
|||
|
||||
# Host TCP ports each port-binding gateway platform listens on, as
|
||||
# ``platform-name -> (config port key, adapter default)``. Mirrors
|
||||
# ``_PORT_BINDING_PLATFORM_VALUES`` in gateway/run.py and each adapter's
|
||||
# ``PORT_BINDING_PLATFORM_VALUES`` in gateway/config.py and each adapter's
|
||||
# DEFAULT_PORT / DEFAULT_WEBHOOK_PORT constant. Used only for the dashboard's
|
||||
# gateway-topology readout — best-effort display data, not a bind source.
|
||||
_PORT_BINDING_PLATFORM_PORTS: Dict[str, Tuple[str, int]] = {
|
||||
|
|
@ -8088,6 +8088,57 @@ async def get_messaging_platforms(profile: Optional[str] = None):
|
|||
}
|
||||
|
||||
|
||||
def _multiplex_port_binding_conflict(
|
||||
platform_id: str, requested_profile: Optional[str]
|
||||
) -> Optional[str]:
|
||||
"""Reason enabling ``platform_id`` on the target profile would break a
|
||||
multiplexed gateway, or ``None`` when the change is allowed.
|
||||
|
||||
Mirrors the gateway's startup rule (``_start_one_profile_adapters`` in
|
||||
gateway/run.py): with ``gateway.multiplex_profiles`` on, the default
|
||||
profile owns the single shared HTTP listener and serves every profile via
|
||||
the ``/p/<profile>/`` prefix, so a SECONDARY profile must never enable a
|
||||
port-binding platform. Without this pre-write check the dashboard happily
|
||||
persisted the invalid config and the shared gateway died with
|
||||
``MultiplexConfigError`` on its next start — for ALL profiles. Only
|
||||
*enabling* is blocked; disabling/clearing stays allowed so users can
|
||||
repair an already-invalid profile.
|
||||
"""
|
||||
from gateway.config import PORT_BINDING_PLATFORM_VALUES, load_gateway_config
|
||||
|
||||
if platform_id not in PORT_BINDING_PLATFORM_VALUES:
|
||||
return None
|
||||
|
||||
requested = (requested_profile or "").strip()
|
||||
if not requested or requested.lower() == "current":
|
||||
from hermes_cli.profiles import get_active_profile_name
|
||||
|
||||
# The dashboard's own profile. "custom" (an unrecognized HERMES_HOME)
|
||||
# is outside the profiles tree, so a multiplexed gateway never serves
|
||||
# it — nothing to guard.
|
||||
target = get_active_profile_name()
|
||||
else:
|
||||
_resolve_profile_dir(requested) # same 400/404 as _profile_scope
|
||||
target = requested
|
||||
if target in ("default", "custom"):
|
||||
return None
|
||||
|
||||
# The multiplex flag that matters is the one the shared gateway reads at
|
||||
# startup: the DEFAULT profile's gateway config (plus the process-wide
|
||||
# GATEWAY_MULTIPLEX_PROFILES override, which load_gateway_config applies).
|
||||
with _config_profile_scope("default"):
|
||||
if not load_gateway_config().multiplex_profiles:
|
||||
return None
|
||||
|
||||
return (
|
||||
f"Cannot enable '{platform_id}' on profile '{target}': it binds its "
|
||||
"own listener port, and gateway.multiplex_profiles is on, so the "
|
||||
"default profile owns the single shared HTTP listener for every "
|
||||
"profile. Configure this channel on the default profile instead "
|
||||
"(disabling or clearing it here is still allowed)."
|
||||
)
|
||||
|
||||
|
||||
@app.put("/api/messaging/platforms/{platform_id}")
|
||||
async def update_messaging_platform(
|
||||
platform_id: str, body: MessagingPlatformUpdate, profile: Optional[str] = None
|
||||
|
|
@ -8098,6 +8149,20 @@ async def update_messaging_platform(
|
|||
status_code=404, detail=f"Unknown messaging platform: {platform_id}"
|
||||
)
|
||||
|
||||
target_profile = body.profile or profile
|
||||
if body.enabled:
|
||||
conflict = _multiplex_port_binding_conflict(platform_id, target_profile)
|
||||
if conflict:
|
||||
# Reject BEFORE any .env/config.yaml write so the profile stays
|
||||
# loadable by the multiplexed gateway.
|
||||
_log.info(
|
||||
"Rejected messaging platform update: platform=%s profile=%s "
|
||||
"(multiplex port-binding conflict)",
|
||||
platform_id,
|
||||
target_profile or "current",
|
||||
)
|
||||
raise HTTPException(status_code=409, detail=conflict)
|
||||
|
||||
allowed_env = set(entry["env_vars"])
|
||||
try:
|
||||
with _profile_scope(body.profile or profile):
|
||||
|
|
@ -8123,6 +8188,16 @@ async def update_messaging_platform(
|
|||
if body.enabled is not None:
|
||||
_write_platform_enabled(platform_id, body.enabled)
|
||||
|
||||
# Audit trail for channel config mutations: names only, never values.
|
||||
_log.info(
|
||||
"Messaging platform updated: platform=%s profile=%s enabled=%s "
|
||||
"env_keys=%s cleared_keys=%s",
|
||||
platform_id,
|
||||
target_profile or "current",
|
||||
body.enabled,
|
||||
sorted(body.env),
|
||||
sorted(body.clear_env),
|
||||
)
|
||||
return {"ok": True, "platform": platform_id}
|
||||
except HTTPException:
|
||||
raise
|
||||
|
|
|
|||
|
|
@ -228,3 +228,149 @@ class TestProfileScopedMessagingWrites:
|
|||
encoding="utf-8"
|
||||
)
|
||||
assert "TELEGRAM_BOT_TOKEN=root-token" in root_env
|
||||
|
||||
|
||||
def _enable_multiplex(default_home):
|
||||
(default_home / "config.yaml").write_text(
|
||||
yaml.safe_dump({"gateway": {"multiplex_profiles": True}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
class TestMultiplexPortBindingGuard:
|
||||
"""Enabling a port-binding channel on a secondary multiplexed profile
|
||||
must be rejected BEFORE anything is persisted.
|
||||
|
||||
The gateway fail-fasts with ``MultiplexConfigError`` when a secondary
|
||||
profile enables a port-binding platform under
|
||||
``gateway.multiplex_profiles`` — but the dashboard used to persist that
|
||||
exact config, so the next gateway start died for EVERY profile (#62791).
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_multiplex_env_override(self, monkeypatch):
|
||||
# The operator env override must not leak into these tests: the
|
||||
# multiplex flag under test comes from the default profile's config.
|
||||
monkeypatch.delenv("GATEWAY_MULTIPLEX_PROFILES", raising=False)
|
||||
|
||||
def test_rejects_every_port_binding_platform_on_secondary(
|
||||
self, client, isolated_profiles
|
||||
):
|
||||
from gateway.config import PORT_BINDING_PLATFORM_VALUES
|
||||
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
assert PORT_BINDING_PLATFORM_VALUES # guard set must not be empty
|
||||
for platform_id in sorted(PORT_BINDING_PLATFORM_VALUES):
|
||||
resp = client.put(
|
||||
f"/api/messaging/platforms/{platform_id}",
|
||||
params={"profile": "worker_alpha"},
|
||||
json={"enabled": True},
|
||||
)
|
||||
assert resp.status_code == 409, platform_id
|
||||
assert "default profile" in resp.json()["detail"]
|
||||
|
||||
def test_body_profile_target_is_also_guarded(self, client, isolated_profiles):
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
json={"enabled": True, "profile": "worker_alpha"},
|
||||
)
|
||||
assert resp.status_code == 409
|
||||
|
||||
def test_rejected_request_leaves_env_and_config_untouched(
|
||||
self, client, isolated_profiles
|
||||
):
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
worker_home = isolated_profiles["worker_alpha"]
|
||||
env_before = (worker_home / ".env").read_text(encoding="utf-8")
|
||||
cfg_before = (worker_home / "config.yaml").read_text(encoding="utf-8")
|
||||
|
||||
catalog = client.get(
|
||||
"/api/messaging/platforms", params={"profile": "worker_alpha"}
|
||||
).json()
|
||||
api_server = next(p for p in catalog["platforms"] if p["id"] == "api_server")
|
||||
env = {f["key"]: "rejected-value" for f in api_server["env_vars"][:1]}
|
||||
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
params={"profile": "worker_alpha"},
|
||||
json={"enabled": True, "env": env},
|
||||
)
|
||||
|
||||
assert resp.status_code == 409
|
||||
assert (worker_home / ".env").read_text(encoding="utf-8") == env_before
|
||||
assert (worker_home / "config.yaml").read_text(encoding="utf-8") == cfg_before
|
||||
|
||||
def test_default_profile_still_allowed_with_multiplex_on(
|
||||
self, client, isolated_profiles
|
||||
):
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
params={"profile": "default"},
|
||||
json={"enabled": True},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
cfg = yaml.safe_load(
|
||||
(isolated_profiles["default"] / "config.yaml").read_text()
|
||||
)
|
||||
assert cfg["platforms"]["api_server"]["enabled"] is True
|
||||
|
||||
def test_secondary_allowed_when_multiplex_off(self, client, isolated_profiles):
|
||||
# Fixture default config is {} — multiplexing disabled.
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
params={"profile": "worker_alpha"},
|
||||
json={"enabled": True},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
cfg = yaml.safe_load(
|
||||
(isolated_profiles["worker_alpha"] / "config.yaml").read_text()
|
||||
)
|
||||
assert cfg["platforms"]["api_server"]["enabled"] is True
|
||||
|
||||
def test_secondary_can_disable_and_clear_invalid_config(
|
||||
self, client, isolated_profiles
|
||||
):
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
worker_home = isolated_profiles["worker_alpha"]
|
||||
(worker_home / "config.yaml").write_text(
|
||||
yaml.safe_dump({"platforms": {"api_server": {"enabled": True}}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
params={"profile": "worker_alpha"},
|
||||
json={"enabled": False},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
cfg = yaml.safe_load((worker_home / "config.yaml").read_text())
|
||||
assert cfg["platforms"]["api_server"]["enabled"] is False
|
||||
|
||||
catalog = client.get(
|
||||
"/api/messaging/platforms", params={"profile": "worker_alpha"}
|
||||
).json()
|
||||
api_server = next(p for p in catalog["platforms"] if p["id"] == "api_server")
|
||||
if api_server["env_vars"]:
|
||||
resp = client.put(
|
||||
"/api/messaging/platforms/api_server",
|
||||
params={"profile": "worker_alpha"},
|
||||
json={"clear_env": [api_server["env_vars"][0]["key"]]},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
|
||||
def test_non_port_binding_platform_unaffected_on_secondary(
|
||||
self, client, isolated_profiles
|
||||
):
|
||||
_enable_multiplex(isolated_profiles["default"])
|
||||
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
|
||||
cfg = yaml.safe_load(
|
||||
(isolated_profiles["worker_alpha"] / "config.yaml").read_text()
|
||||
)
|
||||
assert cfg["platforms"]["telegram"]["enabled"] is True
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue