mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(webhook): make dual-stack bind exclusive
Disable address reuse so an existing family-specific listener cannot silently split traffic with the webhook server. Normalize wildcard bind hosts for local CLI URLs and align setup documentation with the dual-stack default.
This commit is contained in:
parent
d542894adf
commit
92876effe2
6 changed files with 83 additions and 18 deletions
|
|
@ -247,21 +247,33 @@ class WebhookAdapter(BasePlatformAdapter):
|
|||
"/p/{profile}/webhooks/{route_name}", self._handle_webhook
|
||||
)
|
||||
|
||||
# Port conflict detection — fail fast if port is already in use
|
||||
import socket as _socket
|
||||
try:
|
||||
with _socket.socket(_socket.AF_INET, _socket.SOCK_STREAM) as _s:
|
||||
_s.settimeout(1)
|
||||
_s.connect(('127.0.0.1', self._port))
|
||||
logger.error('[webhook] Port %d already in use. Set a different port in config.yaml: platforms.webhook.port', self._port)
|
||||
return False
|
||||
except (ConnectionRefusedError, OSError):
|
||||
pass # port is free
|
||||
|
||||
self._runner = web.AppRunner(app)
|
||||
await self._runner.setup()
|
||||
site = web.TCPSite(self._runner, self._host, self._port)
|
||||
await site.start()
|
||||
# Do not probe only one address family before binding. With the
|
||||
# dual-stack default, an IPv6-only listener can already own this port
|
||||
# while 127.0.0.1 still looks free. Also disable SO_REUSEADDR for the
|
||||
# listener: on macOS, two wildcard/specific sockets with SO_REUSEADDR
|
||||
# can silently split traffic while both servers report success.
|
||||
site = web.TCPSite(
|
||||
self._runner,
|
||||
self._host,
|
||||
self._port,
|
||||
reuse_address=False,
|
||||
)
|
||||
try:
|
||||
await site.start()
|
||||
except OSError as exc:
|
||||
await self._runner.cleanup()
|
||||
self._runner = None
|
||||
logger.error(
|
||||
"[webhook] Could not bind %s:%d: %s. "
|
||||
"Set a different host or port in config.yaml under "
|
||||
"platforms.webhook.extra.",
|
||||
self._host or "all IPv4+IPv6 interfaces",
|
||||
self._port,
|
||||
exc,
|
||||
)
|
||||
return False
|
||||
self._mark_connected()
|
||||
|
||||
route_names = ", ".join(self._routes.keys()) or "(none configured)"
|
||||
|
|
|
|||
|
|
@ -96,9 +96,11 @@ def _is_webhook_enabled() -> bool:
|
|||
|
||||
def _get_webhook_base_url() -> str:
|
||||
wh = _get_webhook_config().get("extra", {})
|
||||
host = wh.get("host", "0.0.0.0")
|
||||
host = wh.get("host")
|
||||
port = wh.get("port", 8644)
|
||||
display_host = "localhost" if host == "0.0.0.0" else host
|
||||
display_host = "localhost" if not host or host in {"0.0.0.0", "::"} else host
|
||||
if ":" in display_host and not display_host.startswith("["):
|
||||
display_host = f"[{display_host}]"
|
||||
return f"http://{display_host}:{port}"
|
||||
|
||||
|
||||
|
|
@ -115,7 +117,6 @@ def _setup_hint() -> str:
|
|||
webhook:
|
||||
enabled: true
|
||||
extra:
|
||||
host: "0.0.0.0"
|
||||
port: 8644
|
||||
secret: "your-global-hmac-secret"
|
||||
|
||||
|
|
|
|||
|
|
@ -24,11 +24,13 @@ platforms:
|
|||
webhook:
|
||||
enabled: true
|
||||
extra:
|
||||
host: "0.0.0.0"
|
||||
port: 8644
|
||||
secret: "generate-a-strong-secret-here"
|
||||
```
|
||||
|
||||
Omitting `host` uses the dual-stack default and listens on both IPv4 and IPv6.
|
||||
Set a specific address only when you intentionally want to restrict the bind.
|
||||
|
||||
### Option 3: Environment variables
|
||||
Add to `${HERMES_HOME:-~/.hermes}/.env`:
|
||||
```bash
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import base64
|
|||
import hashlib
|
||||
import hmac
|
||||
import json
|
||||
import socket
|
||||
import time
|
||||
from collections import deque
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
|
@ -1608,3 +1609,35 @@ class TestDualStackBind:
|
|||
)
|
||||
finally:
|
||||
await adapter.disconnect()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_bind_rejects_existing_ipv6_listener(self):
|
||||
"""A specific IPv6 listener must block the wildcard dual-stack bind."""
|
||||
blocker = await asyncio.start_server(
|
||||
lambda _reader, _writer: None,
|
||||
host="::1",
|
||||
port=0,
|
||||
family=socket.AF_INET6,
|
||||
reuse_address=False,
|
||||
)
|
||||
port = blocker.sockets[0].getsockname()[1]
|
||||
cfg = PlatformConfig(
|
||||
enabled=True,
|
||||
extra={
|
||||
"port": port,
|
||||
"routes": {
|
||||
"r1": {"secret": "real-secret-abc123", "prompt": "x"}
|
||||
},
|
||||
},
|
||||
)
|
||||
adapter = WebhookAdapter(cfg)
|
||||
try:
|
||||
with patch.object(adapter, "_reload_dynamic_routes"):
|
||||
result = await adapter.connect()
|
||||
assert result is False
|
||||
assert adapter._runner is None
|
||||
assert adapter.is_connected is False
|
||||
finally:
|
||||
await adapter.disconnect()
|
||||
blocker.close()
|
||||
await blocker.wait_closed()
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ from argparse import Namespace
|
|||
|
||||
from hermes_cli.webhook import (
|
||||
webhook_command,
|
||||
_get_webhook_base_url,
|
||||
_load_subscriptions,
|
||||
_save_subscriptions,
|
||||
_subscriptions_path,
|
||||
|
|
@ -41,6 +42,23 @@ def _make_args(**kwargs):
|
|||
return Namespace(**defaults)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("host", [None, "", "0.0.0.0", "::"])
|
||||
def test_webhook_base_url_maps_wildcard_hosts_to_localhost(monkeypatch, host):
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.webhook._get_webhook_config",
|
||||
lambda: {"extra": {"host": host, "port": 9123}},
|
||||
)
|
||||
assert _get_webhook_base_url() == "http://localhost:9123"
|
||||
|
||||
|
||||
def test_webhook_base_url_brackets_pinned_ipv6_host(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.webhook._get_webhook_config",
|
||||
lambda: {"extra": {"host": "::1", "port": 9123}},
|
||||
)
|
||||
assert _get_webhook_base_url() == "http://[::1]:9123"
|
||||
|
||||
|
||||
class TestSubscribe:
|
||||
def test_basic_create(self, capsys):
|
||||
webhook_command(_make_args(webhook_action="subscribe", name="test-hook"))
|
||||
|
|
|
|||
|
|
@ -311,7 +311,6 @@ platforms:
|
|||
webhook:
|
||||
enabled: true
|
||||
extra:
|
||||
host: "0.0.0.0" # bind address (default: 0.0.0.0)
|
||||
port: 8644 # listen port (default: 8644)
|
||||
secret: "" # optional global fallback secret
|
||||
rate_limit: 30 # requests per minute per route
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue