fix(mcp): stop 404ing internal-only MCP servers behind a load balancer - #31148
fix(mcp): stop 404ing internal-only MCP servers behind a load balancer#31148mateo-berri wants to merge 4 commits into
Conversation
get_mcp_client_ip returned "" when use_x_forwarded_for was enabled and an
X-Forwarded-For header was present but mcp_trusted_proxy_ranges was not
configured. That empty string flowed into the IP access-control gate, where
is_internal_ip("") is False, so every available_on_public_internet=false
server was filtered out and the named-server lookup raised 404.
Honour X-Forwarded-For when the direct peer is itself internal (a private
reverse proxy in front of the gateway); only ignore it and classify by the
direct connection when the peer is a public address or falls outside the
configured trusted ranges. This restores access for internal clients behind
a load balancer while still blocking a public caller that spoofs an internal
XFF directly against the gateway.
Resolves LIT-3964
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Generated by Claude Code |
Greptile SummaryFixes LIT-3964:
Confidence Score: 5/5Safe to merge — the change is tightly scoped to the MCP client-IP extraction path, all affected branches are covered by new and updated unit tests, and the spoofing-prevention properties are explicitly verified end-to-end. The fix correctly handles all four peer cases (public direct, internal direct, unknown, custom CIDR) and returns No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/ip_address_utils.py | Core fix: get_mcp_client_ip now trusts XFF when the direct peer is an internal private proxy (using mcp_internal_ip_ranges), restoring access for internal MCP clients behind load balancers. Public, out-of-range, and unknown peers still ignore XFF and fail closed to direct_ip or "". Docstring and one-shot warning text updated to match new behaviour. Logic is correct across all cases. |
| tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py | Adds eight new tests covering: the LIT-3964 regression path, the spoofing prevention cases (public direct peer, unknown peer), external clients via private proxies, custom mcp_internal_ip_ranges, and two end-to-end filter_server_ids_by_ip scenarios. Existing tests renamed and assertions updated to match the more-precise (public IP rather than "") return value while retaining the is_internal_ip security assertion. |
Reviews (4): Last reviewed commit: "docs(mcp): correct the XFF-without-trust..." | Re-trigger Greptile
When request.client is None (Unix-socket transports, some ASGI test clients) the direct peer IP is unknown. The prior untrusted_peer check collapsed to False in that state and fell through to trust the forged X-Forwarded-For header, and the untrusted path also returned None, which means "no filtering" downstream and grants internal access. Both leak available_on_public_internet=false servers to a spoofing caller. Reframe the check as peer_is_private_proxy: honour X-Forwarded-For only when the direct peer is a private reverse proxy and no mcp_trusted_proxy_ranges are pinned. A public, out-of-range, or unknown peer is classified by its own address, with an unknown peer returning the external "" sentinel so it can never fall open to None. Update the docstring to describe the full trust model and add regression tests for the request.client is None edge case at both the unit and filter levels.
|
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Custom internal ranges bypass XFF
- The branch already contains commit 87f7fc7 which parses mcp_internal_ip_ranges and passes it into the peer_is_private_proxy is_internal_ip call so the private-proxy carve-out and the downstream access-control gate share one definition of internal, plus regression tests covering this exact scenario.
You can send follow-ups to the cloud agent here.
The peer_is_private_proxy carve-out classified the direct connection with the RFC1918 defaults while the access-control gate (_is_server_accessible_from_ip) classifies client IPs with the operator's mcp_internal_ip_ranges. When an operator configures a custom internal CIDR and their reverse proxy sits inside it but outside the defaults, the peer was treated as untrusted: its X-Forwarded-For was dropped and the proxy's own IP returned, which the gate then classifies internal. A public client forwarded by that proxy inherited the proxy's internal access to available_on_public_internet=false servers. Parse mcp_internal_ip_ranges and pass it to is_internal_ip so the private-proxy decision and the access-control gate share one definition of "internal". Add a unit regression asserting the forwarded client IP is honoured when the peer is internal only under a custom CIDR, and an end-to-end regression asserting a public forwarded client cannot reach an internal-only server through such a proxy.
|
Generated by Claude Code |
With the private-proxy carve-out, MCP access-control now honours X-Forwarded-For when the direct peer is internal, so the one-shot warning in is_request_from_trusted_proxy claiming access-control client IPs "will use the proxy's literal request values" was misleading for exactly the load-balancer operators this fix helps. Reword it to state that OAuth discovery URLs fall back to literal values while access-control honours X-Forwarded-For only when the direct peer is internal, and to point at mcp_trusted_proxy_ranges as the way to validate the proxy explicitly.
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a02e058. Configure here.
|
Generated by Claude Code |
|
This would cause a security regression. Closing |
Relevant issues
Linear ticket
Resolves LIT-3964
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
Regression: with
use_x_forwarded_for: trueand nomcp_trusted_proxy_rangesconfigured, every internal-only MCP server (available_on_public_internet: false) started returning 404 as soon as anX-Forwarded-Forheader was present, which is what every load balancer sends. So this hit real MCP clients (Cursor, mcp-inspector, etc.) during discovery and connection in production while a direct localhost call without XFF looked fineRoot cause is
IPAddressUtils.get_mcp_client_ipinlitellm/proxy/auth/ip_address_utils.py. When XFF was enabled and present but no trusted ranges were configured, it returned""to "fail closed". That empty string then flowed straight into the IP access-control gate, whereis_internal_ip("")isFalse, so the caller was classified external; the internal-only server got filtered out of the registry and the named-server lookup (get_mcp_server_by_name) raised 404This was introduced in #28008 (merged to the release line as its internal-staging copy #28356, commit
6d6eda81). Before that change the same branch fell through to parse XFF, which is why 1.88.0 worked and 1.89.0 / 1.89.2 did notThe fix honours
X-Forwarded-Forwhen the direct peer is itself internal (a private reverse proxy in front of the gateway), and only ignores it, classifying by the direct connection IP, when the peer is a public address or falls outside the configured trusted ranges. This restores access for internal clients behind a load balancer while actually closing a gap the pre-1.89 behaviour had: a public caller connecting directly to the gateway and spoofing an internalX-Forwarded-Foris now classified by its real public peer instead of being trusted. The hardened path is unchanged; setmcp_trusted_proxy_rangesto validate the proxy explicitlyA follow-up commit hardens the unknown-peer case raised in review (
request.client is None, possible with Unix-socket transports and some ASGI test clients). The check is now framed aspeer_is_private_proxy, and the untrusted path returnsdirect_ip or ""so a request with no identifiable peer fails closed to the external""sentinel rather than returningNone, which would mean "no filtering" downstream and leak internal accessA further commit makes the private-proxy check classify the direct peer with the same
mcp_internal_ip_rangesthe access-control gate uses, instead of the RFC1918 defaults. Otherwise a reverse proxy that is internal only under a custom CIDR was treated as untrusted, itsX-Forwarded-Fordropped and its own IP returned, so a public client forwarded through it could inherit the proxy's internal classification and reach internal-only serversA final commit corrects the one-shot
use_x_forwarded_forwarning text, which still claimed access-control client IPs would use the proxy's literal request values; with the carve-out, access-control now honoursX-Forwarded-Forwhen the direct peer is internal, so the message would have misled the very operators this fix helpsTests
tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py:test_internal_client_behind_private_proxy_without_trusted_ranges_is_internalis the direct regression for LIT-3964; it would return""(and 404) before this changetest_internal_client_behind_proxy_can_reach_internal_only_serverties the two halves together end to end the way production does (extract client IP, thenfilter_server_ids_by_ip) and asserts the internal-only server survives the filtertest_public_direct_peer_spoofing_xff_stays_externalandtest_external_client_behind_private_proxy_stays_externalkeep the security assertions of the two cases Support MCP OAuth passthrough and issuer-scoped JWT auth #28008 added; both callers are still classified external, just by their real address rather than by the empty-string sentineltest_unknown_direct_peer_spoofing_xff_stays_externalandtest_unknown_peer_spoofing_xff_cannot_reach_internal_only_servercover therequest.client is Nonecase: the first asserts the result is non-None and external, the second asserts the internal-only server is dropped byfilter_server_ids_by_ipso a forged XFF from an unidentifiable peer cannot reach ittest_xff_honoured_when_peer_internal_under_custom_rangesandtest_public_client_via_custom_internal_proxy_cannot_reach_internal_only_servercover custommcp_internal_ip_ranges: the first asserts the forwarded client IP is honoured when the peer is internal only under a custom CIDR, the second asserts a public forwarded client cannot reach an internal-only server through such a proxyScreenshots / Proof of Fix
Minimal config (internal-only server, XFF on, no trusted ranges):
On 1.89.x the
with XFFline returns 404 (server filtered out); after this fix both lines return the same non-404 (e.g. 400 client_id required, meaning the server was found), matching 1.88.0Note
Medium Risk
Changes MCP access-control IP extraction and trust rules for X-Forwarded-For; mistakes could expose internal-only servers or break clients behind proxies, but behavior is heavily tested and tightens some spoofing cases.
Overview
Fixes LIT-3964: with
use_x_forwarded_foron and nomcp_trusted_proxy_ranges,get_mcp_client_ipno longer always returns""whenX-Forwarded-Foris present. That empty string made every caller look external and internal-only MCP servers 404 behind load balancers.get_mcp_client_ipnow trustsX-Forwarded-Forwhen the direct peer is an internal reverse proxy (mcp_internal_ip_ranges, same rules as access control) and trusted ranges are not pinned. Public, out-of-range, or missingrequest.clientpeers still ignore forged XFF and classify by the direct connection (direct_ip or ""), so spoofing cannot reachavailable_on_public_internet=falseservers and unknown peers do not returnNone(which would skip filtering).The one-shot operator warning text is updated to describe this carve-out vs explicit
mcp_trusted_proxy_ranges. Tests cover the regression, spoofing, custom internal CIDRs, and end-to-endfilter_server_ids_by_ip.Reviewed by Cursor Bugbot for commit a02e058. Bugbot is set up for automated code reviews on this repo. Configure here.