mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(line): dual-stack webhook bind — 0.0.0.0 default unreachable over IPv6-only networks
The LINE adapter's webhook server defaulted to host="0.0.0.0", which binds IPv4 ONLY. On IPv6-only private networks — notably Fly.io 6PN, where the hosted edge router reverse-proxies LINE ingest to <app>.internal:8646 over an fdaa: IPv6 address — nothing is listening on the dialed address: connection refused → customer-visible 502 when LINE's console verifies the webhook (NS-603). This is the same bug the generic webhook adapter fixed ind542894ad; the LINE adapter was never updated to match. Fix mirrors that commit: - DEFAULT_HOST = None → asyncio/aiohttp create_server binds one socket per address family (v4 + v6), regardless of the bindv6only sysctl. "::" is NOT a valid substitute — Fly machines set bindv6only=1, so it would yield an IPv6-only socket and break IPv4 loopback probes. - Empty-string host collapses to None; LINE_HOST/extra.host still pin a specific bind address. - reuse_address=False scoped to macOS only (BSD wildcard-socket traffic-splitting footgun), mirroring9420ad946. - The three outbound-media guards compared webhook_host == "0.0.0.0" by string equality; extracted to _missing_public_url() which treats None/0.0.0.0/::/"" as "no fetchable hostname" so the LINE_PUBLIC_URL requirement still fires under the new default. _media_url() falls back to 127.0.0.1 instead of interpolating 'None' into URLs. Tests: dual-stack default decision table (default None, empty→None, pinned preserved, LINE_HOST override), behavioural both-families bind proof via runner.addresses, and the media public-URL guard matrix. 88 passed, ruff clean. Companion router fix (hermes-agent-router) routes /webhooks/line and /line/webhook|/line/media to :8646 over 6PN; both are needed for end-to-end hosted LINE delivery. Fixes NS-603
This commit is contained in:
parent
f7c5e0d59a
commit
e24bb0b42f
2 changed files with 191 additions and 10 deletions
|
|
@ -30,8 +30,8 @@ binary uploads — images, audio, and video must be reachable HTTPS URLs.
|
|||
We register registered tempfiles under ``/line/media/<token>/<filename>``
|
||||
served by the same aiohttp app, with an allowed-roots traversal guard.
|
||||
``LINE_PUBLIC_URL`` (e.g. ``https://my-tunnel.example.com``) overrides
|
||||
the host:port construction so URLs are reachable when bind is 0.0.0.0
|
||||
or behind a reverse proxy.
|
||||
the host:port construction so URLs are reachable when the bind is a
|
||||
wildcard/dual-stack listener or behind a reverse proxy.
|
||||
|
||||
**5-message batching.** LINE accepts at most 5 message objects per
|
||||
Reply/Push call; longer responses are smart-chunked at 4500 chars
|
||||
|
|
@ -71,6 +71,7 @@ import mimetypes
|
|||
import os
|
||||
import re
|
||||
import secrets
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
import uuid
|
||||
|
|
@ -122,6 +123,29 @@ DEFAULT_WEBHOOK_PORT = 8646
|
|||
DEFAULT_WEBHOOK_PATH = "/line/webhook"
|
||||
DEFAULT_MEDIA_PATH_PREFIX = "/line/media"
|
||||
|
||||
# Default bind host. ``None`` tells aiohttp/asyncio's ``create_server`` to
|
||||
# bind BOTH address families (IPv4 + IPv6) — the portable dual-stack default.
|
||||
# Mirrors gateway/platforms/webhook.py DEFAULT_HOST (commit d542894ad).
|
||||
#
|
||||
# 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 the hosted edge router reverse-proxies LINE ingest to
|
||||
# ``<app>.internal:8646`` over an ``fdaa:…`` IPv6 address — an IPv4-only
|
||||
# listener is unreachable: dial refused → customer-visible 502 (NS-603).
|
||||
# - "::" is NOT a safe fix: on hosts where the kernel sets IPV6_V6ONLY=1
|
||||
# (verified on Fly machines), binding "::" yields an IPv6-ONLY socket,
|
||||
# breaking IPv4 loopback health probes.
|
||||
# - ``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 host via
|
||||
# ``LINE_HOST`` or ``platforms.line.extra.host``.
|
||||
DEFAULT_HOST = None
|
||||
|
||||
# Hosts that mean "listening on every interface" — i.e. the bind address is
|
||||
# not a name LINE's servers could ever fetch media from, so a public base URL
|
||||
# is required for outbound media.
|
||||
_WILDCARD_HOSTS = frozenset({"0.0.0.0", "::", ""})
|
||||
|
||||
# Slow-LLM postback button defaults
|
||||
DEFAULT_SLOW_RESPONSE_THRESHOLD = 45.0 # seconds; 0 disables
|
||||
DEFAULT_PENDING_REPLY_TEXT = (
|
||||
|
|
@ -662,8 +686,12 @@ class LineAdapter(BasePlatformAdapter):
|
|||
or extra.get("channel_secret", "")
|
||||
)
|
||||
|
||||
# Webhook server
|
||||
self.webhook_host = os.getenv("LINE_HOST") or extra.get("host", "0.0.0.0")
|
||||
# Webhook server. Host default is ``None`` → dual-stack bind (both
|
||||
# IPv4 and IPv6); see DEFAULT_HOST above. ``LINE_HOST``/extra.host pin
|
||||
# a specific address when needed; empty string collapses to None.
|
||||
self.webhook_host = (
|
||||
os.getenv("LINE_HOST") or extra.get("host", DEFAULT_HOST) or DEFAULT_HOST
|
||||
)
|
||||
try:
|
||||
self.webhook_port = int(
|
||||
os.getenv("LINE_PORT") or extra.get("port", DEFAULT_WEBHOOK_PORT)
|
||||
|
|
@ -806,12 +834,26 @@ class LineAdapter(BasePlatformAdapter):
|
|||
self._runner = web.AppRunner(self._app)
|
||||
try:
|
||||
await self._runner.setup()
|
||||
self._site = web.TCPSite(self._runner, self.webhook_host, self.webhook_port)
|
||||
# SO_REUSEADDR is platform-dependent (mirrors the generic webhook
|
||||
# adapter, commits d542894ad/9420ad946):
|
||||
# - macOS (BSD semantics): two wildcard/specific sockets with
|
||||
# SO_REUSEADDR can silently split traffic — disable it there.
|
||||
# - Linux: SO_REUSEADDR only permits rebinding past TIME_WAIT;
|
||||
# disabling it would make a quick gateway restart fail to
|
||||
# bind for up to ~60s — keep the default (enabled).
|
||||
self._site = web.TCPSite(
|
||||
self._runner,
|
||||
self.webhook_host,
|
||||
self.webhook_port,
|
||||
reuse_address=False if sys.platform == "darwin" else None,
|
||||
)
|
||||
await self._site.start()
|
||||
except OSError as exc:
|
||||
self._set_fatal_error(
|
||||
"bind_failed",
|
||||
f"Could not bind LINE webhook on {self.webhook_host}:{self.webhook_port}: {exc}",
|
||||
"Could not bind LINE webhook on "
|
||||
f"{self.webhook_host or 'all IPv4+IPv6 interfaces'}:"
|
||||
f"{self.webhook_port}: {exc}",
|
||||
retryable=True,
|
||||
)
|
||||
return False
|
||||
|
|
@ -819,7 +861,7 @@ class LineAdapter(BasePlatformAdapter):
|
|||
self._mark_connected()
|
||||
logger.info(
|
||||
"LINE: webhook listening on %s:%s%s%s",
|
||||
self.webhook_host,
|
||||
self.webhook_host or "* (all interfaces, IPv4+IPv6)",
|
||||
self.webhook_port,
|
||||
self.webhook_path,
|
||||
f" (public: {self.public_base_url})" if self.public_base_url else "",
|
||||
|
|
@ -1288,7 +1330,12 @@ class LineAdapter(BasePlatformAdapter):
|
|||
if self.public_base_url:
|
||||
base = self.public_base_url
|
||||
else:
|
||||
# A wildcard/dual-stack bind has no fetchable hostname; the
|
||||
# _missing_public_url guard should have caught this earlier.
|
||||
# Fall back to localhost so the URL is at least well-formed.
|
||||
host = self.webhook_host
|
||||
if host is None or host in _WILDCARD_HOSTS:
|
||||
host = "127.0.0.1"
|
||||
port = self.webhook_port
|
||||
if port == 443:
|
||||
base = f"https://{host}"
|
||||
|
|
@ -1297,6 +1344,14 @@ class LineAdapter(BasePlatformAdapter):
|
|||
safe_name = _urlquote(filename, safe="")
|
||||
return f"{base}{DEFAULT_MEDIA_PATH_PREFIX}/{token}/{safe_name}"
|
||||
|
||||
def _missing_public_url(self) -> bool:
|
||||
"""True when outbound media cannot work: no LINE_PUBLIC_URL and the
|
||||
bind host is a wildcard (or the dual-stack ``None`` default), i.e.
|
||||
not an address LINE's fetchers could ever reach."""
|
||||
if self.public_base_url:
|
||||
return False
|
||||
return self.webhook_host is None or self.webhook_host in _WILDCARD_HOSTS
|
||||
|
||||
async def _handle_media(self, request) -> Any:
|
||||
"""Serve a registered local file over HTTPS for LINE's media URLs.
|
||||
|
||||
|
|
@ -1358,7 +1413,7 @@ class LineAdapter(BasePlatformAdapter):
|
|||
return SendResult(success=False, error="image exceeds 10 MB LINE limit")
|
||||
if not self._client:
|
||||
return SendResult(success=False, error="LINE adapter not connected")
|
||||
if not self.public_base_url and self.webhook_host == "0.0.0.0":
|
||||
if self._missing_public_url():
|
||||
return SendResult(
|
||||
success=False,
|
||||
error="LINE_PUBLIC_URL must be set to send images "
|
||||
|
|
@ -1388,7 +1443,7 @@ class LineAdapter(BasePlatformAdapter):
|
|||
return SendResult(success=False, error="audio exceeds 200 MB LINE limit")
|
||||
if not self._client:
|
||||
return SendResult(success=False, error="LINE adapter not connected")
|
||||
if not self.public_base_url and self.webhook_host == "0.0.0.0":
|
||||
if self._missing_public_url():
|
||||
return SendResult(
|
||||
success=False,
|
||||
error="LINE_PUBLIC_URL must be set to send audio",
|
||||
|
|
@ -1412,7 +1467,7 @@ class LineAdapter(BasePlatformAdapter):
|
|||
return SendResult(success=False, error="video exceeds 200 MB LINE limit")
|
||||
if not self._client:
|
||||
return SendResult(success=False, error="LINE adapter not connected")
|
||||
if not self.public_base_url and self.webhook_host == "0.0.0.0":
|
||||
if self._missing_public_url():
|
||||
return SendResult(
|
||||
success=False,
|
||||
error="LINE_PUBLIC_URL must be set to send video",
|
||||
|
|
|
|||
|
|
@ -752,3 +752,129 @@ class TestMessageTypeMapping:
|
|||
def test_unknown_type_falls_back_to_text(self):
|
||||
MessageType = _line.MessageType
|
||||
assert _line._LINE_MESSAGE_TYPES.get("flex", MessageType.TEXT) == MessageType.TEXT
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 10. Dual-stack bind default (NS-603)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestDualStackBind:
|
||||
"""The LINE webhook server's default bind must serve BOTH IPv4 and IPv6.
|
||||
|
||||
Regression guard for the hosted LINE 502 (NS-603): Fly.io 6PN — the
|
||||
private network the edge router reverse-proxies LINE ingest over — is
|
||||
IPv6-only (``<app>.internal`` 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:8646`` hit an address nothing was
|
||||
listening on → connection refused → 502 on webhook verification.
|
||||
|
||||
Mirrors gateway/platforms/webhook.py's fix (commit d542894ad):
|
||||
``DEFAULT_HOST = None`` → asyncio binds one socket per address family.
|
||||
``"::"`` is NOT a valid substitute (bindv6only=1 on Fly machines makes
|
||||
it IPv6-only, breaking IPv4 loopback health probes).
|
||||
"""
|
||||
|
||||
def _cfg(self, **extra):
|
||||
from gateway.config import PlatformConfig
|
||||
base = {"channel_access_token": "tok", "channel_secret": "sec"}
|
||||
base.update(extra)
|
||||
return PlatformConfig(enabled=True, extra=base)
|
||||
|
||||
def test_default_host_is_none_for_dual_stack(self, monkeypatch):
|
||||
monkeypatch.delenv("LINE_HOST", raising=False)
|
||||
assert _line.DEFAULT_HOST is None
|
||||
ad = LineAdapter(self._cfg())
|
||||
assert ad.webhook_host is None
|
||||
|
||||
def test_empty_host_normalises_to_none(self, monkeypatch):
|
||||
monkeypatch.delenv("LINE_HOST", raising=False)
|
||||
ad = LineAdapter(self._cfg(host=""))
|
||||
assert ad.webhook_host is None
|
||||
|
||||
def test_pinned_host_is_preserved(self, monkeypatch):
|
||||
monkeypatch.delenv("LINE_HOST", raising=False)
|
||||
ad = LineAdapter(self._cfg(host="127.0.0.1"))
|
||||
assert ad.webhook_host == "127.0.0.1"
|
||||
|
||||
def test_line_host_env_overrides(self, monkeypatch):
|
||||
monkeypatch.setenv("LINE_HOST", "10.0.0.5")
|
||||
ad = LineAdapter(self._cfg())
|
||||
assert ad.webhook_host == "10.0.0.5"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_bind_serves_both_families(self, monkeypatch):
|
||||
"""Behavioural proof: host=None opens v4 AND v6 listening sockets."""
|
||||
monkeypatch.delenv("LINE_HOST", raising=False)
|
||||
ad = LineAdapter(self._cfg(port=0))
|
||||
ad._client = MagicMock()
|
||||
ad._client.get_bot_user_id = AsyncMock(return_value="Ubot")
|
||||
|
||||
# Skip credential/network preamble — drive the aiohttp bind directly
|
||||
# the same way connect() does.
|
||||
from aiohttp import web
|
||||
ad._app = web.Application()
|
||||
ad._runner = web.AppRunner(ad._app)
|
||||
await ad._runner.setup()
|
||||
site = web.TCPSite(ad._runner, ad.webhook_host, 0)
|
||||
try:
|
||||
await site.start()
|
||||
addrs = list(ad._runner.addresses)
|
||||
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, NS-603) — got {addrs}"
|
||||
)
|
||||
finally:
|
||||
await ad._runner.cleanup()
|
||||
|
||||
|
||||
class TestMediaPublicUrlGuard:
|
||||
"""Outbound media requires LINE_PUBLIC_URL whenever the bind host is not
|
||||
a publicly fetchable address — including the new dual-stack ``None``
|
||||
default and the legacy wildcard strings."""
|
||||
|
||||
def _adapter(self, monkeypatch, **extra):
|
||||
from gateway.config import PlatformConfig
|
||||
monkeypatch.delenv("LINE_HOST", raising=False)
|
||||
monkeypatch.delenv("LINE_PUBLIC_URL", raising=False)
|
||||
base = {"channel_access_token": "tok", "channel_secret": "sec"}
|
||||
base.update(extra)
|
||||
return LineAdapter(PlatformConfig(enabled=True, extra=base))
|
||||
|
||||
def test_missing_public_url_true_for_default_none(self, monkeypatch):
|
||||
ad = self._adapter(monkeypatch)
|
||||
assert ad.webhook_host is None
|
||||
assert ad._missing_public_url() is True
|
||||
|
||||
@pytest.mark.parametrize("wildcard", ["0.0.0.0", "::"])
|
||||
def test_missing_public_url_true_for_wildcards(self, monkeypatch, wildcard):
|
||||
ad = self._adapter(monkeypatch, host=wildcard)
|
||||
assert ad._missing_public_url() is True
|
||||
|
||||
def test_missing_public_url_false_with_public_base(self, monkeypatch):
|
||||
ad = self._adapter(monkeypatch, public_url="https://tunnel.example.com")
|
||||
if not ad.public_base_url:
|
||||
# Adapter reads env var name LINE_PUBLIC_URL / extra key —
|
||||
# set directly if the extra key differs.
|
||||
ad.public_base_url = "https://tunnel.example.com"
|
||||
assert ad._missing_public_url() is False
|
||||
|
||||
def test_missing_public_url_false_with_pinned_host(self, monkeypatch):
|
||||
ad = self._adapter(monkeypatch, host="203.0.113.7")
|
||||
assert ad._missing_public_url() is False
|
||||
|
||||
def test_send_image_blocked_without_public_url(self, monkeypatch, tmp_path):
|
||||
ad = self._adapter(monkeypatch)
|
||||
ad._client = MagicMock()
|
||||
img = tmp_path / "x.png"
|
||||
img.write_bytes(b"\x89PNG\r\n\x1a\n123")
|
||||
result = asyncio.run(ad.send_image_file("Uchat", str(img)))
|
||||
assert not result.success
|
||||
assert "LINE_PUBLIC_URL" in (result.error or "")
|
||||
|
||||
def test_media_url_never_contains_none(self, monkeypatch):
|
||||
ad = self._adapter(monkeypatch)
|
||||
url = ad._media_url("tok123", "cat.jpg")
|
||||
assert "None" not in url
|
||||
assert url.startswith("https://")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue