feat: Add transport security configuration with DNS rebinding protection

[DavidFHCh]

No description provided.

[bobrnor]

+1 — would love to see this land.

We hit the same DNS-rebinding 421 wiring postgres-mcp 0.3.0 (rebuilt from main HEAD 07eb329 for streamable-http support) behind an internal AWS ALB fronted by AWS Bedrock AgentCore Gateway. AgentCore's MCP probe sends the user-facing public domain in the Host: header, which TransportSecurityMiddleware rejected because FastMCP had locked allowed_hosts to localhost variants at module-load time (before the args.streamable_http_host=0.0.0.0 override). Result: target stuck FAILED with "MCP server is not reachable" for 4 separate attempts before tracing the actual response code in our server logs.

Two CLI flags you've added (--disable-dns-rebinding-protection, --allowed-hosts) are exactly the right shape — env-var alternates and Docker-friendly defaults (postgres-mcp-server:*, host.docker.internal:*) are nice touches.

Issue #145 reports the same gotcha for a docker compose deployment. Common pattern: any non-localhost host binding triggers this.

While waiting for #144 to land, we vendored a minimal hotfix in our fork as a stopgap (drops transport_security to enable_dns_rebinding_protection=False for the non-localhost host case, no CLI flag): https://github.com/T-S-Studio/postgres-mcp/tree/fix/streamable-http-dns-rebinding-on-non-localhost-host — happy to drop it once #144 ships in a release.

[jssmith]

Review: Transport security configuration with DNS rebinding protection

Verdict: Approve with comments.

Assessment

Exposes MCP's built-in TransportSecuritySettings (DNS rebinding protection, allowed hosts/origins) via CLI flags and env vars. Applies only to sse and streamable-http transports, not stdio.

Strengths

1. Env vars take precedence over CLI flags — sensible for Docker deployments where env vars are the primary config method.
2. Test suite is comprehensive — 11 tests covering CLI flag, env var, env-overrides-CLI, default behavior, and arg ordering (database URL after flags). Parametrized across both sse and streamable-http.
3. The _preserve_mcp_state fixture restores mcp.settings.transport_security and sys.argv after each test — good test hygiene.
4. README table clearly documents flags, env vars, defaults, and includes a Docker example.

Comments

1. from mcp.server.transport_security import TransportSecuritySettings — this import depends on the mcp package version supporting transport_security. The mcp < 2 constraint from PR #197 should be fine, but verify this module exists in mcp 1.29.0. If it was added later, this could break on older 1.x versions.

2. The **{...} if hosts else {} dict-spread pattern is used to conditionally pass allowed_hosts/allowed_origins to TransportSecuritySettings. This works but is slightly hard to read. Consider building a kwargs dict explicitly.

3. protection_off = dns_env.lower() in ("false", "0", "no") — good handling of boolean env var parsing. The CLI flag --disable-dns-rebinding-protection is store_true, so the logic correctly prefers the env var when set.

This review was created by an AI agent (OpenHands) on behalf of @jssmith.

[jssmith]

Review — concise

Verdict: Approve with comments. Well-structured security feature with comprehensive tests.

Strengths

• DNS rebinding protection for SSE and streamable-http transports. Correctly skips stdio.
• CLI flags + env vars with env taking precedence. Standard pattern.
• TransportSecuritySettings applied only when overrides are present — defaults defer to FastMCP.
• Tests are excellent: 10 test cases covering CLI flags, env vars, env-overrides-CLI, env-true-overrides-CLI-disable, default behavior, and positional arg after flags. Parametrized for both sse and streamable-http.
• _preserve_mcp_state fixture cleanly restores state between tests.
• README documentation includes a table and JSON config example.

Issues to address

1. TransportSecuritySettings construction with conditional kwargs (non-blocking)
The **{...} if hosts else {} pattern is functional but hard to read. Consider building a dict first, then unpacking:

settings_kwargs = {"enable_dns_rebinding_protection": not protection_off}
if hosts:
    settings_kwargs["allowed_hosts"] = [h.strip() for h in hosts.split(",") if h.strip()]
if origins:
    settings_kwargs["allowed_origins"] = [o.strip() for o in origins.split(",") if o.strip()]
mcp.settings.transport_security = TransportSecuritySettings(**settings_kwargs)

2. Env var naming inconsistency (non-blocking)
MCP_ENABLE_DNS_REBINDING_PROTECTION is a positive flag (set to false to disable), while the CLI flag is --disable-dns-rebinding-protection (negative flag, set to enable). This inversion could confuse users. The README documents it clearly, but consider aligning the naming.

3. No test for stdio transport (non-blocking)
The tests verify SSE and streamable-http but don't verify that stdio skips transport security setup. A test asserting mcp.settings.transport_security is unchanged for stdio would be useful.

This review was created by an AI agent (OpenHands) on behalf of @jssmith.

[DavidFHCh]

@jssmith feedback addressed, I hope this lands and can help others :D.