mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(webhook): default to dual-stack bind so 6PN (IPv6) can reach the adapter
The webhook adapter defaulted to host='0.0.0.0' — IPv4 only. On Fly.io hosted agents the edge router (hermes-agent-router) reverse-proxies public webhook traffic to <app>.internal:8644 over 6PN, Fly's private network, which is IPv6-only (.internal resolves to an fdaa:… address). An IPv4-only listener is unreachable there, so public webhook POSTs to https://<agent>.agents.nousresearch.com/webhooks/<route> never landed on the adapter — the router's dial was refused. Fix: DEFAULT_HOST = None, which makes aiohttp/asyncio create_server bind BOTH address families. '::' is NOT a valid substitute: on hosts where the kernel sets bindv6only=1 (verified on Fly machines) it yields an IPv6-only socket, breaking the IPv4 loopback /health check and the AF_INET port-conflict probe in connect(). None binds per-family regardless of the sysctl. An explicit empty-string/null host in config now also normalises to None (dual-stack) rather than an invalid host=''. Users can still pin a specific host via platforms.webhook.extra.host. Validated live on a Fly staging agent: with this default and no config override, the adapter binds both v4 and v6 (127.0.0.1:8644 and [::1]:8644 both answer), and a public signed webhook POST through the router returns 202 (valid sig) / 401 (bad sig) instead of the router's 502. Tests: new TestDualStackBind asserts the None default, config resolution (missing/empty→None, pinned preserved), and a real dual-stack bind opens both AF_INET and AF_INET6 listeners. Red-proof: these fail on the old '0.0.0.0' default.
This commit is contained in:
parent
b27d8b6ac8
commit
d542894adf
2 changed files with 109 additions and 4 deletions
|
|
@ -77,7 +77,25 @@ _BUILTIN_DELIVER_PLATFORMS = {
|
|||
"qqbot", "yuanbao",
|
||||
}
|
||||
|
||||
DEFAULT_HOST = "0.0.0.0"
|
||||
# Default bind host. ``None`` tells aiohttp/asyncio's ``create_server`` to bind
|
||||
# BOTH address families (IPv4 + IPv6) — the portable dual-stack default.
|
||||
#
|
||||
# Why not "0.0.0.0" (the old default) or "::"?
|
||||
# - "0.0.0.0" binds IPv4 ONLY. On IPv6-only private networks — notably Fly.io
|
||||
# 6PN, where an agent's ``<app>.internal`` name resolves to an ``fdaa:…``
|
||||
# IPv6 address — an IPv4-only listener is unreachable. That is exactly why
|
||||
# hosted-agent webhook routes were publicly unreachable: the edge router
|
||||
# reverse-proxies to ``<app>.internal:8644`` over 6PN (IPv6) but the adapter
|
||||
# was listening on 0.0.0.0 (v4 only) → connection refused.
|
||||
# - "::" is NOT a safe fix: on hosts where the kernel sets IPV6_V6ONLY=1
|
||||
# (verified on Fly machines), binding "::" yields an IPv6-ONLY socket, which
|
||||
# then breaks the IPv4 loopback health check (``curl 127.0.0.1:8644/health``)
|
||||
# and the AF_INET port-conflict probe in connect().
|
||||
# - ``None`` asks the event loop to create a listening socket per resolved
|
||||
# family, so both 127.0.0.1 (v4) and the 6PN fdaa (v6) are served regardless
|
||||
# of the bindv6only sysctl. Users can still pin a specific host via
|
||||
# ``platforms.webhook.extra.host``.
|
||||
DEFAULT_HOST = None
|
||||
DEFAULT_PORT = 8644
|
||||
_INSECURE_NO_AUTH = "INSECURE_NO_AUTH"
|
||||
_DYNAMIC_ROUTES_FILENAME = "webhook_subscriptions.json"
|
||||
|
|
@ -93,7 +111,7 @@ _LOOPBACK_HOSTS = frozenset({
|
|||
})
|
||||
|
||||
|
||||
def _is_loopback_host(host: str) -> bool:
|
||||
def _is_loopback_host(host: Optional[str]) -> bool:
|
||||
"""True when `host` binds only to the local machine.
|
||||
|
||||
Covers IPv4 loopback, the standard `localhost` alias, IPv6 loopback in
|
||||
|
|
@ -116,7 +134,11 @@ class WebhookAdapter(BasePlatformAdapter):
|
|||
|
||||
def __init__(self, config: PlatformConfig):
|
||||
super().__init__(config, Platform.WEBHOOK)
|
||||
self._host: str = config.extra.get("host", DEFAULT_HOST)
|
||||
# ``host`` may be None (dual-stack default) or a user-pinned string.
|
||||
# A config value of empty string / null is normalised to None so it
|
||||
# also means "bind all families" rather than an invalid "" host.
|
||||
_cfg_host = config.extra.get("host", DEFAULT_HOST)
|
||||
self._host: Optional[str] = _cfg_host or None
|
||||
self._port: int = int(config.extra.get("port", DEFAULT_PORT))
|
||||
self._global_secret: str = config.extra.get("secret", "")
|
||||
self._static_routes: Dict[str, dict] = config.extra.get("routes", {})
|
||||
|
|
@ -245,7 +267,7 @@ class WebhookAdapter(BasePlatformAdapter):
|
|||
route_names = ", ".join(self._routes.keys()) or "(none configured)"
|
||||
logger.info(
|
||||
"[webhook] Listening on %s:%d — routes: %s",
|
||||
self._host,
|
||||
self._host or "* (all interfaces, IPv4+IPv6)",
|
||||
self._port,
|
||||
route_names,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1525,3 +1525,86 @@ class TestInsecureNoAuthSafetyRail:
|
|||
assert result is True
|
||||
finally:
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
class TestDualStackBind:
|
||||
"""The default bind host must serve BOTH IPv4 and IPv6.
|
||||
|
||||
Regression guard for the hosted-agent webhook reachability bug: Fly.io 6PN
|
||||
(the private network the edge router reverse-proxies webhook traffic over)
|
||||
is IPv6-only — an agent's ``<app>.internal`` name resolves to an ``fdaa:…``
|
||||
address. The adapter used to default to ``host="0.0.0.0"`` (IPv4 only), so
|
||||
the router's dial to ``<app>.internal:8644`` hit an address nothing was
|
||||
listening on → connection refused → public webhooks unreachable.
|
||||
|
||||
The fix is ``DEFAULT_HOST = None`` (dual-stack). ``"::"`` is NOT a valid
|
||||
substitute: on hosts with the ``bindv6only`` sysctl set (verified on Fly
|
||||
machines) it yields an IPv6-ONLY socket, which would then break the IPv4
|
||||
loopback health check and the AF_INET port-conflict probe.
|
||||
"""
|
||||
|
||||
def test_default_host_is_none_for_dual_stack(self):
|
||||
"""The module default is None (bind all families), not 0.0.0.0/::."""
|
||||
from gateway.platforms.webhook import DEFAULT_HOST
|
||||
assert DEFAULT_HOST is None
|
||||
|
||||
def test_missing_host_key_resolves_to_none(self):
|
||||
"""Config with no host key → dual-stack (None), not a literal string."""
|
||||
cfg = PlatformConfig(enabled=True, extra={"port": 0, "routes": {}})
|
||||
adapter = WebhookAdapter(cfg)
|
||||
assert adapter._host is None
|
||||
|
||||
@pytest.mark.parametrize("empty", ["", None])
|
||||
def test_empty_host_normalises_to_none(self, empty):
|
||||
"""An explicit empty-string/null host means dual-stack, not host=''.
|
||||
|
||||
Guards the old footgun where host='' was passed straight to TCPSite
|
||||
AND treated as non-loopback — now it collapses to the None default.
|
||||
"""
|
||||
adapter = _make_adapter(host=empty, port=0)
|
||||
assert adapter._host is None
|
||||
|
||||
def test_pinned_host_is_preserved(self):
|
||||
"""A user can still pin a specific bind host via config.extra.host."""
|
||||
adapter = _make_adapter(host="127.0.0.1", port=0)
|
||||
assert adapter._host == "127.0.0.1"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_bind_serves_both_families(self):
|
||||
"""Binding the real server with the default host opens v4 AND v6 sockets.
|
||||
|
||||
This is the behavioural proof: with host=None, asyncio.create_server
|
||||
opens a listening socket per resolved family, so both 127.0.0.1 (v4)
|
||||
and ::1 (v6) are reachable — exactly what 6PN needs. Uses a real bind
|
||||
on an OS-assigned port (no mock) and inspects the runner's addresses.
|
||||
"""
|
||||
# Build config WITHOUT a host key so the real DEFAULT_HOST (None)
|
||||
# applies — _make_adapter's helper injects host="0.0.0.0" by default,
|
||||
# which would mask the dual-stack default under test here.
|
||||
cfg = PlatformConfig(
|
||||
enabled=True,
|
||||
extra={
|
||||
"port": 0,
|
||||
"routes": {"r1": {"secret": "real-secret-abc123", "prompt": "x"}},
|
||||
},
|
||||
)
|
||||
adapter = WebhookAdapter(cfg)
|
||||
assert adapter._host is None
|
||||
try:
|
||||
with patch.object(adapter, "_reload_dynamic_routes"):
|
||||
result = await adapter.connect()
|
||||
assert result is True
|
||||
# runner.addresses lists one bound address per listening socket.
|
||||
# An IPv6 sockaddr is a 4-tuple (host, port, flowinfo, scopeid);
|
||||
# an IPv4 sockaddr is a 2-tuple (host, port). With the dual-stack
|
||||
# default we expect BOTH — that is precisely what makes the adapter
|
||||
# reachable over 6PN (v6) AND on the loopback health check (v4).
|
||||
addrs = list(adapter._runner.addresses) # type: ignore[union-attr]
|
||||
has_v6 = any(len(a) == 4 for a in addrs)
|
||||
has_v4 = any(len(a) == 2 for a in addrs)
|
||||
assert has_v4, f"IPv4 bind missing — got {addrs}"
|
||||
assert has_v6, (
|
||||
f"IPv6 bind missing (the 6PN reachability bug) — got {addrs}"
|
||||
)
|
||||
finally:
|
||||
await adapter.disconnect()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue