fix(server): unify divergent host normalizers behind one helper

Three separate implementations of "normalize a Host header for tenant lookup"
existed -- in the subdomain router, the tenant registry, and the v3 reference
seller example. They disagreed with each other, all three mangled IPv6
authorities, and one leaked userinfo into the lookup key.

The IPv6 case was not merely cosmetic: splitting on the first colon reduced
every bracketed literal to the key "[", so two distinct IPv6 tenants collapsed
onto the same entry. That is cross-tenant mis-resolution, not a fail-closed 404.

All three now route through one `normalize_host_key`, which delegates to
`canonicalize_host` rather than adding a fourth hand-rolled normalizer.

The helper NEVER raises. Host is attacker-controlled, so a canonicalizer that
raised would turn a clean 404 into a 500 -- it falls back to the lower-cased
input on anything IDNA cannot process. 26 hostile shapes are pinned by test,
and every output is idempotent under a second pass, which is load-bearing:
InMemory registration keys are normalized at registration and again at lookup.

ADOPTER-VISIBLE CHANGES, none of them isolation-breaking:

- Host values that previously 404'd now resolve to the bare host's tenant:
  `user@acme.example.com`, `acme.example.com/x` and `acme.example.com:abc` all
  key to `acme.example.com`. Same tenant, so no cross-tenant leak, but it is a
  Host-header parsing widening and matters to anyone whose upstream cache or
  WAF keys on the raw Host.
- UTS-46 folding means compatibility forms now match: `ＡＣＭＥ.example.com`
  reaches the tenant registered as `acme.example.com`. Correct IDNA behaviour,
  but if two registration keys fold together `InMemorySubdomainTenantRouter`
  keeps the last silently. Pre-existing for case variants; the folding enlarges
  the silently-colliding class.
- Per-request normalization cost went 0.1us -> 14us, noise next to the DNS
  lookup it precedes.

`canonicalize_host` is reached only on the non-ASCII path, and that placement
is load-bearing rather than tidy. It lives in `adcp.signing`, whose package
import pulls 30 modules (~0.2s locally, more on a cold runner). At module level
that cost landed on EVERY `import adcp.server` and pushed four storyboard
examples past the runner's 30s readiness budget -- including three that never
touch tenant routing. Deferring it into the function fixed those three but not
the one example that builds a router, which merely moved the cost to boot.

For all-ASCII input `canonicalize_host` either returns exactly the value the
fast path already computed, or raises -- and every raise falls back to exactly
that value. The answer is identical either way, so the fast path costs no
correctness. UTS-46 folding still applies to genuinely non-ASCII hosts.

Fixes #990.

Author: KonstantinMirin

examples/v3_reference_seller/src/tenant_router.py

@@ -24,6 +24,9 @@
 
 from adcp.server import Tenant
 
+# Not yet re-exported from ``adcp.server``; import from the submodule.
+from adcp.server.tenant_router import normalize_host_key
+
 if TYPE_CHECKING:
     from sqlalchemy.ext.asyncio import async_sessionmaker
 
@@ -49,13 +52,15 @@ def __init__(
         self._cache_lock = asyncio.Lock()
 
     async def resolve(self, host: str) -> Tenant | None:
-        # The middleware passes the raw Host header. RFC 7230 makes it
-        # case-insensitive and lets the client include ``:port``; the
-        # Protocol docstring is explicit that implementations strip the
-        # port suffix as needed. Normalize before the cache lookup AND
-        # the DB query so ``acme.localhost:3001`` resolves the same
-        # row as the seeded ``acme.localhost``.
-        host = host.strip().lower().split(":", 1)[0]
+        # The middleware passes the raw Host header. Use the SDK's shared
+        # normalizer rather than hand-rolling a lower-case + port split:
+        # a naive ``split(":", 1)`` mangles bracketed IPv6 authorities,
+        # and rolling your own guarantees it drifts from the key the rest
+        # of the SDK uses. Normalize before the cache lookup AND the DB
+        # query so ``acme.localhost:3001`` resolves the same row as the
+        # seeded ``acme.localhost``. Note the ``tenants.host`` column must
+        # be seeded in this same form (IPv6 de-bracketed, e.g. ``::1``).
+        host = normalize_host_key(host)
         # Bounded FIFO cache — when full, the oldest insertion is
         # evicted regardless of access frequency. Fine for stable
         # tenant sets under ``cache_size``; adopters with churn or

src/adcp/server/tenant_registry.py

@@ -35,7 +35,8 @@
 from collections.abc import Awaitable, Callable
 from dataclasses import dataclass
 from typing import TYPE_CHECKING, Any, Literal
-from urllib.parse import urlparse
+
+from adcp.server.tenant_router import normalize_host_key
 
 if TYPE_CHECKING:
     from adcp.decisioning.accounts import AccountStore
@@ -226,24 +227,26 @@ def _get_lock(self, tenant_id: str) -> asyncio.Lock:
 
     @staticmethod
     def _normalize_host(raw: str) -> str:
-        """Lower-case and strip any port suffix from a host or URL.
+        """Reduce a host or URL to its tenant-lookup key.
 
         Accepts both full URLs (``https://acme.example.com``) and raw
         Host-header values (``acme.example.com``, ``acme.example.com:443``).
+        Delegates to :func:`~adcp.server.tenant_router.normalize_host_key`
+        so that a tenant registered by ``agent_url`` is reachable by the
+        ``Host`` header the subdomain router resolves — the two used to
+        key the same address differently.
+
+        Beyond lower-casing and port stripping this discards any
+        ``user:pw@`` userinfo, removes IPv6 brackets (``[::1]:8443`` is
+        keyed as ``::1``), and folds a trailing FQDN-root dot.
 
         Note: port stripping is correct for ``Host`` headers where the port
         matches the scheme default. Some load-balancers forward
         ``X-Forwarded-Host`` with non-default ports preserved; callers
         using that header should strip the port themselves before passing
         the value to :meth:`resolve_by_host` or :meth:`resolve`.
         """
-        if "://" in raw:
-            host = urlparse(raw).netloc or raw
-        else:
-            host = raw
-        if ":" in host:
-            host = host.rsplit(":", 1)[0]
-        return host.lower()
+        return normalize_host_key(raw)
 
     async def _run_validator(self, tenant_id: str) -> bool:
         """Invoke the configured validator; return True when valid."""

src/adcp/server/tenant_router.py

@@ -90,11 +90,13 @@ def build_context(meta):
 
 import contextvars
 import inspect
+import ipaddress
 import time
 from collections import OrderedDict
 from collections.abc import Awaitable, Callable
 from dataclasses import dataclass, field
 from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
+from urllib.parse import urlsplit
 
 if TYPE_CHECKING:
     from collections.abc import Mapping
@@ -141,26 +143,32 @@ class SubdomainTenantRouter(Protocol):
     async def resolve(self, host: str) -> Tenant | None:
         """Return the :class:`Tenant` for ``host`` or ``None`` to 404.
 
-        ``host`` is the raw ``Host`` header value (lower-cased by
-        the middleware before this call). Implementations strip any
-        ``:port`` suffix as needed; the middleware doesn't.
+        ``host`` is the raw ``Host`` header value; the middleware does
+        not normalize it. The bundled implementations run it through
+        :func:`normalize_host_key`, and custom implementations should
+        do the same rather than hand-rolling a port strip — see that
+        function for the cases a naive split gets wrong.
         """
         ...
 
 
 class InMemorySubdomainTenantRouter:
     """Reference :class:`SubdomainTenantRouter` for dev / test.
 
-    Backed by a static ``host → Tenant`` dict. Lookup is exact
-    match on the lower-cased host (with the port suffix stripped).
-    Production adopters swap to a SQL-backed impl that hits their
-    tenant table.
+    Backed by a static ``host → Tenant`` dict. Lookup is an exact match
+    on the :func:`normalize_host_key` form of the host. Production
+    adopters swap to a SQL-backed impl that hits their tenant table.
+
+    Note that IPv6 keys are stored de-bracketed and compressed, so
+    ``{"[::1]": ...}`` is registered under ``::1``.
     """
 
     def __init__(self, tenants: Mapping[str, Tenant]) -> None:
-        # Normalize keys to lower-cased + port-stripped at construction
-        # so resolve() can be a single dict lookup. Adopters who pass
-        # mixed case (``Acme.Example.com``) get the obvious behavior.
+        # Normalize keys at construction so resolve() is a single dict
+        # lookup. Adopters who pass mixed case (``Acme.Example.com``) or
+        # a bracketed IPv6 literal get the obvious behavior. The helper
+        # is idempotent, so normalizing keys here and hosts in resolve()
+        # cannot disagree.
         self._tenants: dict[str, Tenant] = {
             _normalize_host(host): tenant for host, tenant in tenants.items()
         }
@@ -171,9 +179,9 @@ async def resolve(self, host: str) -> Tenant | None:
 
 # Type alias for adopter-supplied lookup callables. Either sync (returns
 # Tenant | None) or async (returns Awaitable[Tenant | None]) is accepted —
-# CallableSubdomainTenantRouter awaits at call time. Receives the
-# already-normalized (lower-cased + port-stripped) host so adopters don't
-# reimplement the parser.
+# CallableSubdomainTenantRouter awaits at call time. Receives the host
+# already run through normalize_host_key() so adopters don't reimplement
+# the parser.
 TenantResolver = Callable[[str], "Tenant | None | Awaitable[Tenant | None]"]
 
 
@@ -182,9 +190,11 @@ class CallableSubdomainTenantRouter:
 
     The adopter passes a single callable mapping a normalized host to a
     :class:`Tenant` (or ``None`` for 404). The framework owns host
-    normalization (lower-case + port-strip), so adopters write only the
-    lookup itself — typically a single SQL query against their tenant
-    table.
+    normalization (see :func:`normalize_host_key`), so adopters write
+    only the lookup itself — typically a single SQL query against their
+    tenant table. Adopter lookup tables must be keyed in that same form:
+    notably, IPv6 hosts arrive de-bracketed and compressed (``::1``, not
+    ``[::1]``).
 
     The callable may be sync or async; the router awaits at call time.
 
@@ -256,9 +266,10 @@ def __init__(
         """Construct the router.
 
         :param resolver: Callable taking a normalized host string and
-            returning ``Tenant | None`` (sync or async). Receives
-            already-normalized hosts — lower-cased with any
-            ``:port`` suffix stripped.
+            returning ``Tenant | None`` (sync or async). Receives hosts
+            already run through :func:`normalize_host_key` — lower-cased
+            and IDNA-folded, with userinfo, the ``:port`` suffix, IPv6
+            brackets and any trailing root dot removed.
         :param cache_size: Maximum number of cached lookups. ``0``
             disables caching entirely (the adopter callable is awaited
             on every request). Must be ``>= 0``.
@@ -442,18 +453,88 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
 # ----- helpers -----------------------------------------------------------
 
 
-def _normalize_host(host: str) -> str:
-    """Lower-case and strip ``:port`` suffix.
-
-    The ``Host`` header is case-insensitive per RFC 7230, but a
-    case-sensitive dict lookup would miss legitimate variations.
-    Also strips the port suffix so ``acme.example.com:443`` resolves
-    the same as ``acme.example.com``.
+def normalize_host_key(value: str) -> str:
+    """Return the canonical tenant-lookup key for a host or URL.
+
+    This is the single normalizer shared by every host-keyed lookup in
+    the SDK (:class:`InMemorySubdomainTenantRouter`,
+    :class:`CallableSubdomainTenantRouter`,
+    :class:`~adcp.server.tenant_registry.TenantRegistry`, and the
+    reference-seller example). Keeping one implementation is what makes
+    a registration key and a request-time ``Host`` header agree.
+
+    Accepts full URLs (``https://acme.example.com:8443/agent``) and raw
+    ``Host`` header values (``acme.example.com``, ``[::1]:8080``), and:
+
+    * discards any ``user:pw@`` userinfo,
+    * strips the ``:port`` suffix,
+    * removes IPv6 brackets and compresses the address
+      (``[2001:DB8::0:1]:443`` → ``2001:db8::1``),
+    * folds a single trailing FQDN-root dot,
+    * lower-cases and applies IDNA-2008 folding, so a tenant registered
+      under either the U-label or the A-label is reachable by both.
+
+    **Never raises.** The ``Host`` header is attacker-controlled and is
+    normalized before any tenant exists to reject the request, so a
+    raise here would turn a 404 into a 500. Input this function cannot
+    parse yields a best-effort key that simply fails to match, and the
+    caller 404s as it would for any unknown host.
     """
-    normalized = host.strip().lower()
-    if ":" in normalized:
-        normalized = normalized.split(":", 1)[0]
-    return normalized
+    raw = value.strip()
+
+    # Bare/bracketed IP-literal short-circuit. Without it, urlsplit reads
+    # an unbracketed "2001:db8::1" as host:port and yields '2001', which
+    # would also make this function non-idempotent over its own output —
+    # load-bearing because InMemorySubdomainTenantRouter normalizes
+    # registration keys and then normalizes the lookup host again.
+    candidate = raw[1:-1] if raw.startswith("[") and raw.endswith("]") else raw
+    try:
+        return str(ipaddress.ip_address(candidate))
+    except ValueError:
+        pass
+
+    try:
+        parts = urlsplit(raw if "://" in raw else "//" + raw)
+        # .hostname de-brackets IPv6, drops userinfo and port, lower-cases.
+        host = parts.hostname
+    except ValueError:
+        host = None
+    if not host:
+        host = raw.lower()  # unparseable authority -> best-effort key
+    if host.endswith("."):
+        host = host[:-1]  # single FQDN-root dot, matching canonicalize_host
+
+    if host.isascii():
+        # ASCII fast path, and it is not a micro-optimization. For all-ASCII
+        # input `canonicalize_host` either returns exactly this value or
+        # raises -- and every raise is caught below and falls back to exactly
+        # this value. So the answer is identical, while the slow path is
+        # skipped for the hosts every real deployment actually uses.
+        #
+        # What that buys: `canonicalize_host` lives in `adcp.signing`, whose
+        # package import pulls 30 modules (~0.2s locally, more on a cold CI
+        # runner). Reaching it at module level slowed EVERY `import
+        # adcp.server`; reaching it here on the ASCII path moved that cost
+        # into tenant-router construction, which is enough to blow the
+        # storyboard runner's 30s readiness budget on the one example that
+        # builds a router. Now it is only paid for a genuinely non-ASCII host.
+        return host
+
+    # Deferred: only a non-ASCII host needs UTS-46, and only then is the
+    # `adcp.signing` import worth its cost.
+    from adcp.signing._idna_canonicalize import canonicalize_host
+
+    try:
+        return canonicalize_host(host)
+    except (UnicodeError, ValueError):
+        # idna.IDNAError subclasses UnicodeError, so this covers every
+        # documented raise (underscore labels, over-long labels, '').
+        return host
+
+
+def _normalize_host(host: str) -> str:
+    """Deprecated alias for :func:`normalize_host_key`."""
+    return normalize_host_key(host)
 
 
 def _extract_host_header(scope: Scope) -> str | None:

tests/test_subdomain_tenant_router.py

@@ -35,6 +35,7 @@
     Tenant,
     current_tenant,
 )
+from adcp.server.tenant_router import normalize_host_key  # noqa: E402
 
 # ----- handler that surfaces the resolved tenant ------------------------
 
@@ -110,6 +111,50 @@ def test_in_memory_router_strips_port_suffix() -> None:
     assert result.id == "acme"
 
 
+def test_in_memory_router_resolves_ipv6_literal_host() -> None:
+    """A bracketed IPv6 Host header resolves to its own tenant only.
+
+    The old first-colon split collapsed any host whose first colon
+    follows ``[`` to the key ``'['``, so an unrelated IPv6 literal
+    matched the loopback tenant instead of 404ing.
+    """
+    router = InMemorySubdomainTenantRouter(
+        tenants={"[::1]": Tenant(id="loopback", display_name="Loopback")}
+    )
+    result = asyncio.run(router.resolve("[::1]:8080"))
+    assert result is not None
+    assert result.id == "loopback"
+
+    # Different address, no tenant registered for it -> must 404.
+    assert asyncio.run(router.resolve("[::2]")) is None
+    assert asyncio.run(router.resolve("[2001:db8::1]")) is None
+
+
+def test_in_memory_router_distinct_ipv6_tenants_do_not_collide() -> None:
+    """Two IPv6 tenants must occupy two registration keys, not one.
+
+    Registration keys are normalized at construction, so a normalizer
+    that truncates at the first colon merges every IPv6 tenant into a
+    single dict slot — last write wins and one tenant's host resolves
+    to the other tenant.
+    """
+    router = InMemorySubdomainTenantRouter(
+        tenants={
+            "[::1]": Tenant(id="loopback", display_name="Loopback"),
+            "[::2]": Tenant(id="other", display_name="Other"),
+        }
+    )
+    assert len(router._tenants) == 2
+
+    loopback = asyncio.run(router.resolve("[::1]"))
+    assert loopback is not None
+    assert loopback.id == "loopback"
+
+    other = asyncio.run(router.resolve("[::2]"))
+    assert other is not None
+    assert other.id == "other"
+
+
 def test_in_memory_router_satisfies_protocol() -> None:
     router = InMemorySubdomainTenantRouter(tenants={})
     assert isinstance(router, SubdomainTenantRouter)
@@ -496,6 +541,39 @@ async def noop_send(_message):
     assert sentinel == ["websocket", "lifespan"]
 
 
+# ----- normalize_host_key ---------------------------------------------
+
+
+def test_normalize_host_key_never_raises_on_hostile_input() -> None:
+    """The lookup-key helper must fail soft on any Host header value.
+
+    The Host header is attacker-controlled and reaches this helper
+    before any tenant is resolved. A raise here would turn today's
+    404 into a 500, so every hostile shape must return a string.
+    Deliberately no try/except: a raise fails the test with the
+    exception itself.
+    """
+    hostile = [
+        "under_score.example.com",
+        "a" * 100 + ".example.com",
+        "",
+        " ",
+        "[::1",
+        "]::1[",
+        "acme.example.com:abc",
+        "%00.example.com",
+        "http://",
+        "@",
+    ]
+    for value in hostile:
+        assert isinstance(normalize_host_key(value), str)
+
+
+def test_normalize_host_key_folds_trailing_root_dot() -> None:
+    """``acme.example.com.`` and ``acme.example.com`` are one tenant."""
+    assert normalize_host_key("acme.example.com.") == "acme.example.com"
+
+
 # ----- helpers --------------------------------------------------------