From 92876effe2894ec114e61d2cae69814e9e7d55fb Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:23:57 +0530 Subject: [PATCH] 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. --- gateway/platforms/webhook.py | 38 ++++++++++++------- hermes_cli/webhook.py | 7 ++-- .../hermes-agent/references/webhooks.md | 4 +- tests/gateway/test_webhook_adapter.py | 33 ++++++++++++++++ tests/hermes_cli/test_webhook_cli.py | 18 +++++++++ .../docs/guides/webhook-github-pr-review.md | 1 - 6 files changed, 83 insertions(+), 18 deletions(-) diff --git a/gateway/platforms/webhook.py b/gateway/platforms/webhook.py index 24d4d1c77122..7f104f0d7e9e 100644 --- a/gateway/platforms/webhook.py +++ b/gateway/platforms/webhook.py @@ -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)" diff --git a/hermes_cli/webhook.py b/hermes_cli/webhook.py index 4d090511ae82..9b9de6cd5a6c 100644 --- a/hermes_cli/webhook.py +++ b/hermes_cli/webhook.py @@ -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" diff --git a/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md b/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md index a815cac50d44..9de582473f18 100644 --- a/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md +++ b/skills/autonomous-ai-agents/hermes-agent/references/webhooks.md @@ -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 diff --git a/tests/gateway/test_webhook_adapter.py b/tests/gateway/test_webhook_adapter.py index 4e83bd6bbde4..10165d73c825 100644 --- a/tests/gateway/test_webhook_adapter.py +++ b/tests/gateway/test_webhook_adapter.py @@ -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() diff --git a/tests/hermes_cli/test_webhook_cli.py b/tests/hermes_cli/test_webhook_cli.py index 1ac5add13eb6..c9f24a76c775 100644 --- a/tests/hermes_cli/test_webhook_cli.py +++ b/tests/hermes_cli/test_webhook_cli.py @@ -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")) diff --git a/website/docs/guides/webhook-github-pr-review.md b/website/docs/guides/webhook-github-pr-review.md index 8935a5d48a01..644dca0fd591 100644 --- a/website/docs/guides/webhook-github-pr-review.md +++ b/website/docs/guides/webhook-github-pr-review.md @@ -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