Repository navigation
fix(mcp)!: require allowed hosts for non-loopback HTTP binds - #7172
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Medium impact] The PR appears safe to merge; no new actionable issues were found. SummaryRequires declared client hostnames for non-loopback MCP HTTP listeners while keeping Host and Origin checks active.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI flags or environment] --> B[Parse and deduplicate hostnames]
B --> C[Bind HTTP listener]
C --> D{Bound address is loopback?}
D -->|Yes| E[Use loopback hostname list]
D -->|No| F{Declared list exists?}
F -->|No| G[Close listener and report repair]
F -->|Yes| H[Use declared hostname list]
E --> I[Announce an admitted URL]
H --> I
I --> J[Check Host and Origin]
J -->|Allowed| K[MCP or environment route]
J -->|Refused| L[403 response]
Reviews (8) · Last reviewed commit: "chore: refresh HTTP hardening against cu..." · Reviewed by Greptile |
42ba5ba to
d5215ae
Compare
…owlist A streamable HTTP bind outside loopback previously disabled both the Host (DNS rebinding) and Origin (browser page) gates, because the allowlist was classified solely from a loopback bound address. MCP_HOST already accepts any address, so an opt-out listener served the whole MCP catalog with no browser protection at all. A non-loopback bind now refuses to serve until the operator declares the host names clients will use, through MCP_ALLOWED_HOSTS or the new rea mcp --http --allowed-hosts flag. Every declared name must parse; one invalid entry refuses startup instead of silently narrowing the allowlist. Loopback classification, the SDK localhost allowlist, and every existing refusal behavior are unchanged. Exported environment bundles bind 0.0.0.0 inside their container, so the bundle Dockerfile now ships MCP_ALLOWED_HOSTS naming the published loopback port and the environment's compose service name; the agentenv runner lane deploys and verifies it end to end.
d5215ae to
8c17da5
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e8bbab920
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e96b0704c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Final head
Proceeding with the explicitly authorized merge bypass for the reproduced base failure; it is not represented as green CI. No unrelated baseline code changes are included in this PR. |
Summary
A non-loopback MCP HTTP bind previously skipped Host and Origin validation. Require an operator-declared hostname allowlist and keep both request gates active. Publish a client URL admitted by that list, using the actual bound port.
Configuration validates complete hostname/IP tokens, canonicalizes and deduplicates them once, and passes typed identities through CLI, environment serving and HTTP. URL components, ports and encoded/control syntax are refused at input. Missing declarations have a typed listener failure and one recovery owner shared by MCP and
environment-serve; native bind failures retain their recovery meanings. No DNS lookup or extra protocol request is added.Exported AgentEnv bundles declare loopback client names and their compose service name. One bundle-name owner validates folder/id/HTTP identity and the DNS label bound before preparing state or writing files. CLI guides, transport contracts and current specifications describe the new behavior; historical verification records retain their original scope.
Migration
BREAKING CHANGE: non-loopback deployments must set
MCP_ALLOWED_HOSTS=127.0.0.1,rea.lan, or pass--allowed-hosts 127.0.0.1,rea.lantorea mcp --http, listing hostnames/IPs clients use without URLs or ports.environment-serveuses the environment setting. Loopback defaults retain their behavior. Bundle names must remain unchanged as HTTP hostnames and fit one 63-octet DNS label; use a name such asreplay-1for IP-shaped labels.Validation
Integration base:
19447516a4bdbc9437d112ebd3f12a5fd062db83. Final head:505ad66013b7936dbbe547594ea7d9b36f3ebef4.keyedArchiveScalarStructure.test.ts(same assertion reproduced on exact main 1944751); six boundary shards were cancelled by fail-fast, so their final-head coverage is unverified. Windows capability/package lane also passes. Codex completed with no findings and Greptile 5/5 on the current head. Earlier same-base failures and exact attribution remain in validation comments. Main independently fixed the prior Zod CLI-default regression in 2b84efe; the full 18-case CLI schema suite now passes in this integration. Other earlier client/cleanup failures were attributed against the pre-upgrade baseline and their latest main fixes are inherited here.Ordinary tightening of the existing listener boundary; no new provider, target execution, installation or dependency policy. No local Docker, Windows or analysis-engine execution is claimed; hosted platform/bundle lanes are reported for their exact heads.
Author readiness
Human owner: @DeryFerd