feat(mcp): OMNIVOICE_MCP_ALLOWED_HOSTS — configurable host allowlist for MCP transport security (#1249) - #1250
Conversation
… allowlist (debpalash#1249) Agents running in Docker containers (or on other machines) connect via a hostname like host.containers.internal, which the MCP SDK's DNS-rebinding guard rejects with 421. Add OMNIVOICE_MCP_ALLOWED_HOSTS (comma-separated host patterns) that extends both allowed_hosts and allowed_origins in create_mcp_server(). Default empty → no behavior change. Test: assert the env var extends the allowlist + origins. Docs: mcp.md notes the env var for Docker/LAN agents.
|
| Filename | Overview |
|---|---|
| backend/mcp_server.py | Reads OMNIVOICE_MCP_ALLOWED_HOSTS, extends allowed_hosts and allowed_origins (both schemes) on the MCP transport security object; failure is logged and non-fatal. |
| tests/test_mcp_mount.py | Adds test verifying the env var extends both allowed_hosts and allowed_origins with correct http/https variants. |
| docs/mcp.md | Documents OMNIVOICE_MCP_ALLOWED_HOSTS with example patterns and correct security guidance. |
| CHANGELOG.md | Adds changelog entry under ### Added for the new env var with usage example and issue reference. |
Reviews (4): Last reviewed commit: "docs(mcp): add security note for remote ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds opt-in MCP host allowlist configuration through ChangesMCP host allowlist
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/mcp_server.py`:
- Around line 125-129: Extend the allowed origins configured by the MCP server
alongside the existing HTTP entries to also include HTTPS origins for each host.
In tests/test_mcp_mount.py lines 108-118, update the matching assertion to
require the corresponding https://...:* origin as well as the existing HTTP
origin.
In `@docs/mcp.md`:
- Around line 33-37: Update the remote MCP connection guidance near
OMNIVOICE_MCP_ALLOWED_HOSTS to state that plain HTTP is only acceptable on a
fully trusted LAN; require Tailscale, WireGuard, or TLS for connections beyond
that boundary, and explicitly warn users not to expose the backend publicly.
- Around line 33-37: Update the documentation around OMNIVOICE_MCP_ALLOWED_HOSTS
to describe the server’s Host header: specify that it should contain the
hostname and port used to reach the /mcp endpoint, rather than the address the
agent connects from. Preserve the comma-separated host-pattern examples.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c82dce6-fc81-4e2f-b8b9-06930fba3e84
📒 Files selected for processing (4)
CHANGELOG.mdbackend/mcp_server.pydocs/mcp.mdtests/test_mcp_mount.py
| **Agents in Docker or on another machine:** the MCP SDK rejects non-localhost | ||
| Host headers by default (DNS-rebinding guard). Set | ||
| `OMNIVOICE_MCP_ALLOWED_HOSTS` to a comma-separated list of host patterns the | ||
| agent connects from (e.g. `host.containers.internal:*,192.168.1.50:*`): | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document the secure transport boundary.
This newly documented remote path can expose voice operations over plain HTTP unless users are told to stay on a trusted LAN or use Tailscale/WireGuard/TLS, and not to expose the backend publicly. As per path instructions, remote connections beyond a fully trusted LAN must use Tailscale/WireGuard or TLS and must not be exposed publicly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/mcp.md` around lines 33 - 37, Update the remote MCP connection guidance
near OMNIVOICE_MCP_ALLOWED_HOSTS to state that plain HTTP is only acceptable on
a fully trusted LAN; require Tailscale, WireGuard, or TLS for connections beyond
that boundary, and explicitly warn users not to expose the backend publicly.
Source: Path instructions
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^docs/mcp\.md$|^docs/|mcp)' || true
echo "== docs/mcp.md relevant section =="
if [ -f docs/mcp.md ]; then
nl -ba docs/mcp.md | sed -n '1,90p'
fi
echo "== OMNIVOICE_MCP_ALLOWED_HOSTS usages =="
rg -n "OMNIVOICE_MCP_ALLOWED_HOSTS|MCP_ALLOWED_HOSTS|Host|host-header|transport_security|allowed_hosts" .Repository: debpalash/OmniVoice-Studio
Length of output: 4046
🌐 Web query:
mcp python SDK transport_security.py ALLOWED_HOSTS Host header DNS rebinding guard
💡 Result:
The transport_security.py module in the Model Context Protocol (MCP) Python SDK provides critical security middleware designed to protect against DNS rebinding attacks [1][2]. When enabled, this middleware validates the Host header of incoming HTTP requests to ensure they match a trusted allowlist [1][3]. Key details regarding DNS rebinding protection and ALLOWED_HOSTS: - Mechanism: The TransportSecurityMiddleware intercepts incoming HTTP requests [2]. If enable_dns_rebinding_protection is set to True (which is the default behavior when running on localhost in modern SDK versions), the middleware checks the Host header against the allowed_hosts configuration [1][3][4]. - Default Behavior: For servers created via FastMCP() or standard HTTP transport helpers, DNS rebinding protection is automatically enabled when the host is 127.0.0.1, localhost, or ::1 [4][5]. If a request's Host header does not match the expected value (e.g., when behind a reverse proxy using a custom domain), the server will reject the request with an HTTP 421 ("Misdirected Request") status code [6][7]. - Configuration: When deploying behind a reverse proxy (such as Nginx or Cloudflare), you must explicitly configure TransportSecuritySettings to include your public hostname [6][8]. - Example: python from mcp.server.transport_security import TransportSecuritySettings transport_security = TransportSecuritySettings( allowed_hosts=["mcp.example.com", "mcp.example.com:443"], ) - Note: allowed_hosts supports exact string matches and wildcard port patterns (e.g., hostname:*) [6][3]. - Error Handling: Recent updates ensure that rejected requests return a structured JSON body indicating the failure, such as {"error": "host_not_allowed", "received_host": "...", "configure": "TransportSecuritySettings.allowed_hosts"}, to assist in debugging configuration issues [8]. - Disabling Protection: While it is possible to set enable_dns_rebinding_protection to False, this is generally discouraged for production deployments unless your infrastructure (e.g., a reverse proxy) already guarantees host-header validation [6].
Citations:
- 1: https://github.com/modelcontextprotocol/python-sdk/blob/main/src/mcp/server/transport_security.py
- 2: https://deepwiki.com/modelcontextprotocol/python-sdk/4.4-transport-security-and-configuration
- 3: https://github.com/modelcontextprotocol/python-sdk/blob/7ba41dcf/src/mcp/server/transport_security.py
- 4: GHSA-9h52-p55h-vw2f
- 5: [Security] DNS rebinding protection disabled by default — all default MCP servers vulnerable modelcontextprotocol/python-sdk#2269
- 6: https://py.sdk.modelcontextprotocol.io/v2/run/deploy/
- 7:
TransportSecuritySettings.allowed_hosts=[](default) silently rejects external Host headers with HTTP 421 modelcontextprotocol/python-sdk#2688 - 8: modelcontextprotocol/python-sdk@b737756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs/mcp.md relevant section =="
awk '{printf "%5d\t%s\n", NR, $0}' docs/mcp.md | sed -n '1,90p'
echo "== OMNIVOICE_MCP_ALLOWED_HOSTS usages =="
rg -n "OMNIVOICE_MCP_ALLOWED_HOSTS|MCP_ALLOWED_HOSTS|allowed_hosts|Host|transport_security" .
echo "== backend mcp server relevant sections =="
for f in backend/mcp_server.py backend/mcp_shim/__main__.py backend/services/mcp_bindings.py tests/test_mcp_mount.py tests/test_mcp_bindings.py; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n -C 4 "OMNIVOICE_MCP_ALLOWED_HOSTS|MCP_ALLOWED_HOSTS|TransportSecuritySettings|server\.run|http_transport|allowed_hosts|Host" "$f" || true
fi
doneRepository: debpalash/OmniVoice-Studio
Length of output: 26391
Describe OMNIVOICE_MCP_ALLOWED_HOSTS as the server’s Host header.
The MCP transport-security allowlist validates requests for that Host value, not the client address, so “connects from” can lead to wrong configuration. Use wording like “hostname and port used to reach the /mcp endpoint” at docs/mcp.md:35-36.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/mcp.md` around lines 33 - 37, Update the documentation around
OMNIVOICE_MCP_ALLOWED_HOSTS to describe the server’s Host header: specify that
it should contain the hostname and port used to reach the /mcp endpoint, rather
than the address the agent connects from. Preserve the comma-separated
host-pattern examples.
|
All three bot review comments are addressed in the latest commits — they were posted on
|
2 participants
Closes #1249.
What
Agents in Docker containers (or on other machines) connect via
host.containers.internal, which the MCP SDK rejects with 421 (DNS-rebinding guard). AddOMNIVOICE_MCP_ALLOWED_HOSTS(comma-separated host patterns) that extends bothallowed_hostsANDallowed_origins— so agents and browser-based MCP clients behind a proxy both work.Default empty → zero behavior change. Same opt-in pattern as
OMNIVOICE_TRUSTED_NETWORKS.Review
/simplify (4 angles): reuse/simplification/efficiency clean. Altitude: bare
except: passshould log (applied — operator's opted-in setting shouldn't fail silently)./code-review high (correctness + conventions): found Origin-header gap (applied — now extends origins too), fix-quality test gap (applied — test added), docs-sync (applied — docs/mcp.md updated), bare except (applied).
Files
backend/mcp_server.py— read env var, extend allowed_hosts + allowed_origins, log on failuretests/test_mcp_mount.py— test the env var extends both listsdocs/mcp.md— document the env var for Docker/LAN agentsCHANGELOG.md— ### Added entryAdds the opt-in
OMNIVOICE_MCP_ALLOWED_HOSTSenvironment variable to extend the MCP SDK transport-securityallowed_hostsand correspondingallowed_originsfor bothhttp://andhttps://from a comma-separated list of host patterns; default remains empty to preserve the current localhost-only behavior. This enables agents in Docker/LAN/proxied setups to reach the/mcpendpoint using non-local Host headers while keeping operators in control via allowlisting. A human should review the host-pattern parsing and the warning-only failure path to ensure malformed input or SDK exceptions can’t leave transport security misconfigured.