chore(ci): merge dev branch#28801
Conversation
[Infra] Promote internal staging to main
[Infra] Promote internal staging to main
Replace direct ``request.url.path`` reads in auth, ACL, routing, and audit-log decisions with ``get_request_route(request)`` — the helper already added in ``auth/auth_utils.py`` that returns the ASGI ``scope["path"]`` with ``root_path`` stripped. Starlette reconstructs ``url.path`` from the Host header; ``scope["path"]`` is uvicorn's parse of the request line and matches what FastAPI dispatches on, so it's the authoritative route for any decision that should agree with the actual handler. Sites: - _experimental/mcp_server/auth/user_api_key_auth_mcp.py - management_endpoints/mcp_management_endpoints.py - vector_store_endpoints/utils.py - pass_through_endpoints/pass_through_endpoints.py - auth/route_checks.py - litellm_pre_call_utils.py - spend_tracking/spend_management_endpoints.py - common_utils/http_parsing_utils.py - management_helpers/utils.py - health_endpoints/_health_endpoints.py Adds regression tests in tests/proxy_unit_tests/test_proxy_routes.py that construct a Request with scope["path"] set to a benign route and the Host header crafted so url.path would resolve differently; each site's decision is asserted against scope["path"].
Move the ``from litellm.proxy.auth.auth_utils import get_request_route`` imports added in the prior commit back to the function bodies that use them. The module-level form participates in a long-standing import cycle through ``auth_utils -> _types -> ...`` and was flagged by CodeQL on the PR; the lazy form matches the pattern the proxy already uses for ``user_api_key_auth`` and related helpers elsewhere in these files. Also drop the ``RouteChecks._is_assistants_api_request`` delegation in ``_get_metadata_variable_name`` introduced in the prior commit — the delegation pulled ``RouteChecks`` into the same cycle, and the call site reuses the resolved route for its other branches, so inlining the substring check is both cycle-free and avoids a redundant second ``get_request_route`` call. Comment in test_proxy_routes.py acknowledges that the two MCP table entries exercise ``get_request_route`` directly rather than the full production handler (which needs ASGI scope + MCP state to invoke).
chore(proxy): route path-dependent call sites through get_request_route
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR hardens a Host-header injection vulnerability where a malformed
Confidence Score: 5/5Safe to merge — mechanical substitution of request.url.path to get_request_route across all affected call sites with no auth logic restructuring. Every changed call site performs a direct substitution and the helper behavior has been in production. No new code paths introduced; regression suite covers all migrated sites with four malformed-host patterns. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_utils.py | Docstring updated to document the Host-header bypass risk; function body unchanged and correct. |
| litellm/proxy/auth/route_checks.py | _is_assistants_api_request migrated from request.url.path to get_request_route; logic is unchanged, bypass is closed. |
| litellm/proxy/vector_store_endpoints/utils.py | Four request.url.path usages in permission-type classifiers replaced with get_request_route; auth bypass closed. |
| litellm/proxy/pass_through_endpoints/pass_through_endpoints.py | create_pass_through_route inner function now uses get_request_route to feed is_registered_pass_through_route, closing the bypass. |
| litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py | Three request.url.path usages (well-known bypass, upstream-delegated auth, OAuth2 check) replaced; security path tightened. |
| litellm/proxy/management_endpoints/mcp_management_endpoints.py | PKCE /token path check migrated to get_request_route; prevents Host-crafted path from bypassing grant-type validation. |
| litellm/proxy/common_utils/http_parsing_utils.py | _add_vector_store_id_from_path migrated to get_request_route; prevents spurious vector_store_id extraction from injected host paths. |
| litellm/proxy/litellm_pre_call_utils.py | _get_metadata_variable_name migrated; metadata key selection now uses authoritative scope path. |
| litellm/proxy/management_helpers/utils.py | OTel span route attribution migrated to get_request_route; telemetry now records the real request route. |
| litellm/proxy/spend_tracking/spend_management_endpoints.py | v2 spend-log path detection migrated; no longer susceptible to Host-injected /spend/logs/v2 substring. |
| litellm/proxy/health_endpoints/_health_endpoints.py | /test health endpoint now echoes get_request_route(request) instead of url.path; consistent with the fix. |
| tests/proxy_unit_tests/test_proxy_routes.py | Adds parameterised regression suite covering 7 call sites x 4 malformed-host patterns; test logic mirrors production predicates correctly. |
Reviews (2): Last reviewed commit: "Merge branch 'litellm_internal_staging' ..." | Re-trigger Greptile
|
|
||
|
|
||
| # (label, scope_path, host_suffix_template, predicate, expected) — host_suffix_template | ||
| # receives the host_header via %s substitution. The predicate is invoked on a Request | ||
| # whose scope["path"] is scope_path and whose Host header is the formatted suffix. | ||
| # | ||
| # The MCP entries (well_known_mcp_bypass, pkce_token_suffix) call | ||
| # get_request_route directly rather than the surrounding production handler | ||
| # (MCPRequestHandler.process_mcp_request / _mcp_oauth_user_api_key_auth) — | ||
| # those handlers require an ASGI scope plus MCP state to invoke, and the call | ||
| # sites do nothing with the path except feed it to this helper. The helper- | ||
| # level assertion is the relevant signal. | ||
| _CALL_SITES = [ | ||
| ("assistants_classification", "/key/generate", "%s/thread", _is_assistants, False), | ||
| ( | ||
| "metadata_variable_name", | ||
| "/chat/completions", | ||
| "%s/thread", | ||
| _metadata_var_name, | ||
| "metadata", | ||
| ), | ||
| ( | ||
| "vector_store_id_extraction", | ||
| "/key/generate", | ||
| "%s/vector_stores/x/files", | ||
| _vector_store_id_in_path, | ||
| False, | ||
| ), | ||
| ( | ||
| "well_known_mcp_bypass", | ||
| "/mcp/tools/call", | ||
| "/.well-known/%s", | ||
| lambda r: get_request_route(r).startswith("/.well-known/"), | ||
| False, | ||
| ), | ||
| ( | ||
| "pkce_token_suffix", | ||
| "/mcp/server-id/token", | ||
| "%s", | ||
| lambda r: get_request_route(r).rstrip("/").lower().endswith("/token"), | ||
| True, | ||
| ), | ||
| ( | ||
| "spend_logs_v2_classification", | ||
| "/spend/logs", | ||
| "%s/spend/logs/v2", | ||
| lambda r: "/spend/logs/v2" in get_request_route(r), | ||
| False, | ||
| ), | ||
| ("health_route_echo", "/test", "%s", lambda r: get_request_route(r), "/test"), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("host_header", _BYPASS_HOSTS) | ||
| @pytest.mark.parametrize( | ||
| "label,scope_path,host_suffix_template,predicate,expected", | ||
| _CALL_SITES, | ||
| ids=[c[0] for c in _CALL_SITES], | ||
| ) | ||
| def test_call_site_uses_scope_path( | ||
| label, scope_path, host_suffix_template, predicate, expected, host_header | ||
| ): | ||
| """Each call site that previously read request.url.path must now make its | ||
| decision against scope["path"]. The Host header is crafted so url.path | ||
| would resolve to a value that flips the decision under the old code.""" | ||
| request = _create_request_with_host_header( | ||
| path=scope_path, host_header=host_suffix_template % host_header | ||
| ) | ||
| assert predicate(request) == expected |
There was a problem hiding this comment.
Missing regression coverage for vector-store permission and pass-through route checks
_CALL_SITES covers _add_vector_store_id_from_path (path extraction) but not the two security-adjacent permission functions in vector_store_endpoints/utils.py — is_allowed_to_call_vector_store_endpoint and is_allowed_to_call_vector_store_files_endpoint — where the fixed path drives read/write permission classification. Similarly, the pass_through_endpoints.py is_registered_pass_through_route check is absent from the matrix.
A malformed Host that collapses url.path to / would have caused the endpoint loop to skip every entry (no match → permission_type = None → return None, i.e. no permission check at all) while the real scope path would have matched a write endpoint. Both call sites were fixed but neither has a Host-injection regression test here.
5f75be5
into
litellm_internal_staging
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes