diff --git a/gateway/config.py b/gateway/config.py index 245784beb01b..45c9e5ce180c 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -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// 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: """ diff --git a/gateway/run.py b/gateway/run.py index 1fb3cd66364a..f5bd6459af79 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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// 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): diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 38487c14ee2c..3ba9c192664b 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -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//`` 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 diff --git a/tests/hermes_cli/test_web_server_messaging_profiles.py b/tests/hermes_cli/test_web_server_messaging_profiles.py index a5b2eb8894bc..9342ad4bee12 100644 --- a/tests/hermes_cli/test_web_server_messaging_profiles.py +++ b/tests/hermes_cli/test_web_server_messaging_profiles.py @@ -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