From dd9e75335c85d9b05a9406aba15ea66c57295206 Mon Sep 17 00:00:00 2001 From: Christopher <210261288+Christopher-Schulze@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:17:05 +0200 Subject: [PATCH] fix(gateway): skip port-conflicting multiplex profiles --- gateway/run.py | 64 ++++--- .../test_multiplex_adapter_registry.py | 158 +++++++++++++++++- .../docs/user-guide/multi-profile-gateways.md | 22 ++- 3 files changed, 203 insertions(+), 41 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index f5bd6459af79..a73f0924cff0 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -1408,25 +1408,31 @@ def _current_max_iterations() -> int: from contextlib import contextmanager as _contextmanager -# 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. +# 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. We skip that secondary +# profile (SecondaryPortBindingConfigError) so a single bad profile cannot +# take down the whole multiplexer. 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): - """A profile multiplexer config is invalid (fail-fast at startup). + """A profile multiplexer config is invalid. - Distinct from a transient adapter-connect failure: a transient error is - logged and the gateway stays alive to retry, but a config error means the - operator must fix config.yaml, so it aborts startup cleanly. + Distinct from a transient adapter-connect failure: a config error means the + operator must fix config.yaml. Fatal configuration errors propagate to the + startup guard instead of being treated as retryable adapter noise. """ +class SecondaryPortBindingConfigError(MultiplexConfigError): + """A secondary profile conflicts with the multiplexer's shared listener.""" + + @_contextmanager def _profile_runtime_scope(profile_home: "Path"): """Scope config/skills/memory AND credentials to a profile for one turn. @@ -8744,10 +8750,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew connected += await self._start_one_profile_adapters( profile_name, profile_home, claimed ) + except SecondaryPortBindingConfigError as e: + logger.warning( + "Skipping secondary profile '%s' due to port-binding config error: %s", + profile_name, + e, + ) except MultiplexConfigError: - # Config error (e.g. a secondary profile binding a port) is not - # transient — propagate so startup aborts cleanly instead of - # limping along with a half-configured multiplexer. raise except Exception as e: logger.error( @@ -8790,6 +8799,22 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew "'open'." ) + port_binding_platforms = sorted( + platform.value + for platform, platform_config in profile_cfg.platforms.items() + if platform_config.enabled and platform.value in _PORT_BINDING_PLATFORM_VALUES + ) + if port_binding_platforms: + joined = ", ".join(port_binding_platforms) + raise SecondaryPortBindingConfigError( + f"Profile '{profile_name}' enables port-binding platform(s) " + f"{joined}, but gateway.multiplex_profiles is on. The default " + f"profile owns the single shared HTTP listener and serves every " + f"profile through the /p/{profile_name}/ URL prefix. Remove " + f"these platform entries from profile '{profile_name}'s config.yaml " + f"or configure them only on the default profile." + ) + profile_map = self._profile_adapters.setdefault(profile_name, {}) connected = 0 for platform, platform_config in profile_cfg.platforms.items(): @@ -8803,21 +8828,6 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew and platform is Platform.RELAY ): continue - # A secondary profile must NOT enable a port-binding platform: the - # default profile's listener already serves every profile via the - # /p// prefix, so a second bind can only collide. This is a - # config error, not a transient failure — fail fast and loud. - if platform.value in _PORT_BINDING_PLATFORM_VALUES: - raise MultiplexConfigError( - f"Profile '{profile_name}' enables the port-binding platform " - f"'{platform.value}', but gateway.multiplex_profiles is on. The " - f"default profile owns the single shared HTTP listener and " - f"serves every profile through the /p/{profile_name}/ URL " - f"prefix — a secondary profile cannot bind its own port. " - f"Remove platforms.{platform.value} from profile " - f"'{profile_name}'s config.yaml (configure it only on the " - f"default profile)." - ) try: with _profile_runtime_scope(profile_home): adapter = self._create_adapter(platform, platform_config) diff --git a/tests/gateway/test_multiplex_adapter_registry.py b/tests/gateway/test_multiplex_adapter_registry.py index 9a0f49bfb04e..d2c324c5001a 100644 --- a/tests/gateway/test_multiplex_adapter_registry.py +++ b/tests/gateway/test_multiplex_adapter_registry.py @@ -1,4 +1,6 @@ """Phase 3: secondary-profile adapter registry + same-token conflict detection.""" +import logging + import pytest from gateway.run import GatewayRunner @@ -142,12 +144,12 @@ class TestProfileMessageHandler: assert seen["profile"] == "writer" -class TestPortBindingHardError: - """A secondary profile enabling a port-binding platform aborts startup.""" +class TestSecondaryProfileConfigHandling: + """Secondary config errors degrade only when the profile is safe to skip.""" @pytest.mark.asyncio - async def test_secondary_webhook_raises(self, monkeypatch): - from gateway.run import MultiplexConfigError + async def test_secondary_webhook_uses_degradable_error(self, monkeypatch): + from gateway.run import SecondaryPortBindingConfigError from gateway.config import GatewayConfig, Platform, PlatformConfig runner = GatewayRunner.__new__(GatewayRunner) @@ -163,10 +165,143 @@ class TestPortBindingHardError: "gateway.config.load_gateway_config", lambda: reviewer_cfg ) - with pytest.raises(MultiplexConfigError) as ei: + with pytest.raises(SecondaryPortBindingConfigError) as ei: await runner._start_one_profile_adapters("reviewer", "/tmp/x", {}) assert "webhook" in str(ei.value) assert "reviewer" in str(ei.value) + assert "reviewer" not in runner._profile_adapters + + @pytest.mark.asyncio + async def test_secondary_reports_all_port_binding_platforms(self, monkeypatch): + from gateway.run import SecondaryPortBindingConfigError + from gateway.config import GatewayConfig, Platform, PlatformConfig + + runner = GatewayRunner.__new__(GatewayRunner) + runner.config = GatewayConfig(multiplex_profiles=True) + runner._profile_adapters = {} + + reviewer_cfg = GatewayConfig(multiplex_profiles=True) + reviewer_cfg.platforms = { + Platform.FEISHU: PlatformConfig(enabled=True), + Platform.WEBHOOK: PlatformConfig(enabled=True, extra={"port": 8644}), + Platform.TELEGRAM: PlatformConfig(enabled=True, token="t"), + } + monkeypatch.setattr( + "gateway.config.load_gateway_config", lambda: reviewer_cfg + ) + + with pytest.raises(SecondaryPortBindingConfigError) as ei: + await runner._start_one_profile_adapters("reviewer", "/tmp/x", {}) + message = str(ei.value) + assert "feishu" in message + assert "webhook" in message + assert "telegram" not in message + assert "reviewer" not in runner._profile_adapters + + @pytest.mark.asyncio + async def test_multiplexer_skips_bad_profile_and_continues(self, monkeypatch, caplog): + from pathlib import Path + from gateway.config import GatewayConfig + + runner = GatewayRunner.__new__(GatewayRunner) + runner.config = GatewayConfig(multiplex_profiles=True) + runner.adapters = {} + runner._profile_adapters = {} + + async def fake_start_one(profile_name, profile_home, claimed): + if profile_name == "bad": + from gateway.run import SecondaryPortBindingConfigError + raise SecondaryPortBindingConfigError("bad enables webhook") + runner._profile_adapters[profile_name] = {} + return 2 + + monkeypatch.setattr( + "hermes_cli.profiles.profiles_to_serve", + lambda multiplex: [ + ("default", Path("/tmp/default")), + ("bad", Path("/tmp/bad")), + ("good", Path("/tmp/good")), + ], + ) + monkeypatch.setattr( + "hermes_cli.profiles.get_active_profile_name", + lambda: "default", + ) + monkeypatch.setattr(runner, "_start_one_profile_adapters", fake_start_one) + monkeypatch.setattr( + "gateway.status.write_runtime_status", + lambda **kwargs: None, + ) + + caplog.set_level(logging.WARNING, logger="gateway.run") + connected = await runner._start_secondary_profile_adapters() + + assert connected == 2 + assert "good" in runner._profile_adapters + assert "bad" not in runner._profile_adapters + assert "Skipping secondary profile 'bad'" in caplog.text + + @pytest.mark.asyncio + async def test_multiplexer_propagates_security_config_error(self, monkeypatch): + from pathlib import Path + from gateway.config import GatewayConfig + from gateway.run import MultiplexConfigError + + runner = GatewayRunner.__new__(GatewayRunner) + runner.config = GatewayConfig(multiplex_profiles=True) + runner.adapters = {} + runner._profile_adapters = {} + + async def fake_start_one(profile_name, profile_home, claimed): + raise MultiplexConfigError( + f"Profile '{profile_name}' enables open policy without allow-all opt-in" + ) + + monkeypatch.setattr( + "hermes_cli.profiles.profiles_to_serve", + lambda multiplex: [ + ("default", Path("/tmp/default")), + ("unsafe", Path("/tmp/unsafe")), + ], + ) + monkeypatch.setattr( + "hermes_cli.profiles.get_active_profile_name", + lambda: "default", + ) + monkeypatch.setattr(runner, "_start_one_profile_adapters", fake_start_one) + + with pytest.raises(MultiplexConfigError, match="open policy"): + await runner._start_secondary_profile_adapters() + + @pytest.mark.asyncio + async def test_open_policy_uses_fatal_config_error(self, monkeypatch): + from gateway.config import GatewayConfig, Platform, PlatformConfig + from gateway.run import ( + MultiplexConfigError, + SecondaryPortBindingConfigError, + ) + + monkeypatch.delenv("GATEWAY_ALLOW_ALL_USERS", raising=False) + monkeypatch.delenv("WECOM_ALLOW_ALL_USERS", raising=False) + + runner = GatewayRunner.__new__(GatewayRunner) + runner.config = GatewayConfig(multiplex_profiles=True) + runner._profile_adapters = {} + + unsafe_cfg = GatewayConfig(multiplex_profiles=True) + unsafe_cfg.platforms = { + Platform.WECOM: PlatformConfig( + enabled=True, + extra={"dm_policy": "open"}, + ), + } + monkeypatch.setattr("gateway.config.load_gateway_config", lambda: unsafe_cfg) + + with pytest.raises(MultiplexConfigError, match="open policy") as exc_info: + await runner._start_one_profile_adapters("unsafe", "/tmp/unsafe", {}) + + assert not isinstance(exc_info.value, SecondaryPortBindingConfigError) + assert "unsafe" not in runner._profile_adapters @pytest.mark.asyncio async def test_secondary_non_binding_platform_ok(self, monkeypatch): @@ -383,6 +518,15 @@ class TestPortBindingHardError: def test_port_binding_set_covers_known_listeners(self): from gateway.run import _PORT_BINDING_PLATFORM_VALUES # Every adapter that binds a TCP port must be in the guard set. - for p in ("webhook", "api_server", "msgraph_webhook", "feishu", - "wecom_callback", "bluebubbles", "sms"): + for p in ( + "webhook", + "api_server", + "msgraph_webhook", + "feishu", + "wecom_callback", + "bluebubbles", + "sms", + "whatsapp_cloud", + "line", + ): assert p in _PORT_BINDING_PLATFORM_VALUES diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index 92c4f9ef9eda..e4251506b7ce 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -142,18 +142,26 @@ POST http://host:8644/p/coder/webhooks/ An unknown or unconfigured profile in the prefix returns `404`. Because the one shared listener already serves every profile this way, a **secondary profile must not enable a port-binding platform itself** — doing so is a config error -and the gateway refuses to start, naming the profile and platform: +that skips the entire secondary profile while the default and other healthy +profiles continue. The warning names the skipped profile and every conflicting +platform: ``` -Profile 'coder' enables the port-binding platform 'webhook', but -gateway.multiplex_profiles is on. ... Remove platforms.webhook from profile -'coder's config.yaml (configure it only on the default profile). +Skipping secondary profile 'coder' due to port-binding config error: Profile +'coder' enables port-binding platform(s) webhook, but gateway.multiplex_profiles +is on. ... Remove these platform entries from profile 'coder's config.yaml or +configure them only on the default profile. ``` Port-binding platforms covered by this rule: `webhook`, `api_server`, -`msgraph_webhook`, `feishu`, `wecom_callback`, `bluebubbles`, `sms`. Configure -any of these **only on the default profile**; every profile is reachable through -its `/p//` prefix. +`msgraph_webhook`, `feishu`, `wecom_callback`, `bluebubbles`, `sms`, +`whatsapp_cloud`, `line`. Configure any of these **only on the default profile**; +every profile is reachable through its `/p//` prefix. + +Only this shared-listener conflict degrades to a skipped profile. Security +configuration errors remain fatal: for example, an `open` own-policy platform +without `GATEWAY_ALLOW_ALL_USERS` or its platform-specific allow-all opt-in +still aborts gateway startup rather than silently dropping the unsafe profile. #### 3. Per-credential platforms still need their own token per profile