refactor(robonix-api): make MCP DNS-rebinding opt-out explicit (follow-up to #86)#87
Closed
enkerewpo wants to merge 1 commit into
Closed
refactor(robonix-api): make MCP DNS-rebinding opt-out explicit (follow-up to #86)#87enkerewpo wants to merge 1 commit into
enkerewpo wants to merge 1 commit into
Conversation
Replace FastMCP(self.id, host="0.0.0.0") with an explicit TransportSecuritySettings(enable_dns_rebinding_protection=False). Same runtime behavior — cross-host Host headers are accepted and the listen socket is bound 0.0.0.0 by uvicorn (_start_mcp_server) regardless — but the intent is now explicit rather than a side-effect of the host arg dodging FastMCP's host-in-loopback auto-allowlist branch (server.py:178). The host arg never affected the bind address, so "0.0.0.0" there was misleading. Clarifies the security-review note: the 0.0.0.0 listen socket pre-dates this change; only the app-layer Host-header allowlist is relaxed. DNS-rebinding protection defends a browser attack vector, not the server-to-server executor->provider call. Endpoint auth remains a separate wire-wide item.
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.
Optional follow-up to the fix merged in #86, which used
FastMCP(self.id, host="0.0.0.0"). No behavior change — this only makes the intent explicit and responds to the automated security review on the commit.Background
host="0.0.0.0"toFastMCPdoes not change the listen socket. The socket is bound by_start_mcp_server'suvicorn host="0.0.0.0"(capability.py:750), and the lifecycle gRPC has always bound0.0.0.0(:688). Thehostarg only matters whentransport_securityis unset and the host is loopback, where the SDK adds a localhost-onlyallowed_hosts(mcp/server/fastmcp/server.py:178). So"0.0.0.0"was just a side-effect way of skipping that auto-allowlist, and reads misleadingly as if it changed the bind address./mcp/trailing slash: the 307 redirect is incidental, and a directPOST /mcp421s under the old code too. The cause is the default host=127.0.0.1 triggering the localhost-only Host-header allowlist.Change
host="0.0.0.0"→ explicittransport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), with a clarifying comment.Security note (responding to the automated review)
Does not touch the v0.1 frozen surface; purely an internal
robonix-apiexpression change.