feat(middleware): auto-synthesize allowed_hosts from SubdomainTenantMiddleware router#527
Closed
feat(middleware): auto-synthesize allowed_hosts from SubdomainTenantMiddleware router#527
Conversation
…iddleware router Fixes the asymmetry where InMemorySubdomainTenantRouter strips ports at lookup (via _normalize_host) but the FastMCP allowed_hosts allowlist required both bare and :* variants to be registered explicitly. serve() now calls _synthesize_allowed_hosts() after _prepend_debug_endpoint, which scans asgi_middleware for SubdomainTenantMiddleware entries and, when the router exposes hosts(), auto-produces bare+:* pairs for each registered host. Adopters need only the router's host list — no separate _allowed_hosts() helper. Existing explicit allowed_hosts pass-through is unchanged. Closes #518 https://claude.ai/code/session_0198Bk2S7eJCQtvUALXrQXHj
- _synthesize_allowed_hosts: use issubclass (with TypeError guard for non-class callables) instead of identity check so subclasses of SubdomainTenantMiddleware also trigger synthesis - Emit a UserWarning at serve() startup when a SubdomainTenantMiddleware router lacks hosts() — silent skip + 421 was a confusing failure mode - Add comment at _serve_a2a call site noting allowed_hosts is MCP-only - Document startup-time snapshot semantics in _synthesize_allowed_hosts docstring; note hosts() convention in SubdomainTenantRouter Protocol - Tests: warning assertion for missing hosts(), subclass synthesis coverage https://claude.ai/code/session_0198Bk2S7eJCQtvUALXrQXHj
Contributor
Author
|
Superseded by #537 (merged) which already auto-synthesizes :* host variants for bare allowed_hosts. |
Contributor
Author
|
Acknowledged — #537 covers this. No further action on this PR. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #518
Summary
InMemorySubdomainTenantRouterstrips ports at lookup time (acme.localhost:3001→acme.localhostvia_normalize_host), but the FastMCPallowed_hostsallowlist previously required adopters to register bothacme.localhostandacme.localhost:*explicitly. This created an asymmetry: the router was port-agnostic at lookup, but the allowlist was port-strict unless the adopter maintained a separate_allowed_hosts()helper (asmulti_platform_sellerdid).This PR closes that gap.
serve()now calls_synthesize_allowed_hosts()after_prepend_debug_endpoint, which scansasgi_middlewareforSubdomainTenantMiddlewareentries (including subclasses) and, when the router exposeshosts(), auto-produces bare +:*pairs for each registered host. Existing explicitallowed_hostspass-through is unchanged and deduplicated against synthesized entries.Before:
After:
Changes
InMemorySubdomainTenantRouter.hosts()— new method returning normalized registered host names; the synthesis hook_synthesize_allowed_hosts()inserve.py— pure helper that scansasgi_middlewareand builds the merged allowlist; usesissubclass(withTypeErrorguard) so subclasses ofSubdomainTenantMiddlewarealso trigger synthesisUserWarningwhen aSubdomainTenantMiddlewarerouter lackshosts()— silent 421 was a confusing failure mode for SQL-backed router adopters graduating fromInMemorySubdomainTenantRouter_serve_a2acall site notingallowed_hostsis MCP-onlySubdomainTenantRouterProtocol comment documenting the optionalhosts()conventionmulti_platform_sellerexample simplified:_allowed_hosts()helper andallowed_hosts=kwarg removedWhat-tested
pytest tests/test_subdomain_tenant_router.py tests/test_serve_transport_security.py— 27/27 passed (9 new tests:hosts()normalization/empty, synthesis happy path, merge/dedup, no-middleware noop, warning for custom router withouthosts(), subclass match, callable-factory skip)ruff check src/adcp/server/serve.py src/adcp/server/tenant_router.py— cleanmypy src/adcp/server/serve.py src/adcp/server/tenant_router.py— no new errors (all pre-existing import-not-found for third-party stubs)Nit from pre-PR review (not fixed — out of scope): O(n) deduplication in
_synthesize_allowed_hostsis idiomatic for single-digit tenant counts;dict.fromkeyswould be cleaner but not worth the churn.Pre-PR review:
issubclass, warning, and A2A comment issues addressed in fixup commit:*synthesis does not expand attack surface (FastMCP's_validate_hostuses anchored prefix match;evil.acme.localhost:3001does not matchacme.localhost:*); synthesis is a startup-time snapshot documented in docstring; SQL-backed router warning addedSession: https://claude.ai/code/session_0198Bk2S7eJCQtvUALXrQXHj
Generated by Claude Code