mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(gateway): skip port-conflicting multiplex profiles
This commit is contained in:
parent
fe2d847aca
commit
dd9e75335c
3 changed files with 203 additions and 41 deletions
|
|
@ -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/<profile>/ 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/<profile>/ 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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -142,18 +142,26 @@ POST http://host:8644/p/coder/webhooks/<route>
|
|||
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/<profile>/` 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/<profile>/` 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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue