From 5255235eb3c20b07c7327704e14bf051e0dae622 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Mon, 25 May 2026 11:54:48 -0700 Subject: [PATCH] chore(proxy): backport host-derived-path call site fix from #28547 Surgical port to v1.83.10 patch series of upstream PR #28547 (merge commit c854fc86). Replaces request.url.path with get_request_route() at call sites whose path-based decisions must not be smuggled via the Host header, since Starlette reconstructs url.path from the Host header but FastAPI dispatches on scope["path"]. Mechanical substitutions applied (9 files, auto-merged): - litellm/proxy/auth/auth_utils.py (docstring update only) - litellm/proxy/auth/route_checks.py - litellm/proxy/common_utils/http_parsing_utils.py - litellm/proxy/health_endpoints/_health_endpoints.py - litellm/proxy/litellm_pre_call_utils.py - litellm/proxy/management_helpers/utils.py - litellm/proxy/pass_through_endpoints/pass_through_endpoints.py - litellm/proxy/spend_tracking/spend_management_endpoints.py - litellm/proxy/vector_store_endpoints/utils.py Conflicted files (manual surgical resolution): - litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py: Replaced '.well-known' in str(request.url) (host-header vulnerable substring check) with get_request_route(request).startswith( '/.well-known/'). Dropped the upstream PR's other substitutions (_target_servers_delegate_auth_to_upstream, _target_servers_use_oauth2, ProxyException-union handler) because the call sites do not exist in this branch. - litellm/proxy/management_endpoints/mcp_management_endpoints.py: No mechanical substitution applies. The upstream PR's only call site for this file (_mcp_oauth_user_api_key_auth, an OAuth dependency for /authorize and /token PKCE endpoints) does not exist in this branch. Kept HEAD's sync _get_cached_temporary_mcp_server_or_404 unchanged. - tests/proxy_unit_tests/test_proxy_routes.py: Added the upstream PR's regression tests. The pkce_token_suffix case exercises helper-level behavior only (no production call site for it in this branch); kept for symmetry and helper coverage. --- .../mcp_server/auth/user_api_key_auth_mcp.py | 7 +- litellm/proxy/auth/auth_utils.py | 13 +- litellm/proxy/auth/route_checks.py | 6 +- .../proxy/common_utils/http_parsing_utils.py | 5 +- .../health_endpoints/_health_endpoints.py | 5 +- litellm/proxy/litellm_pre_call_utils.py | 4 +- litellm/proxy/management_helpers/utils.py | 14 +- .../pass_through_endpoints.py | 5 +- .../spend_management_endpoints.py | 5 +- litellm/proxy/vector_store_endpoints/utils.py | 18 ++- tests/proxy_unit_tests/test_proxy_routes.py | 145 ++++++++++++++++++ 11 files changed, 212 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index 357d21eb09a..78258d416dc 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -117,7 +117,12 @@ class MCPRequestHandler: return b"{}" request.body = mock_body # type: ignore - if ".well-known" in str(request.url): # public routes + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) + + if get_request_route(request).startswith("/.well-known/"): # public routes validated_user_api_key_auth = UserAPIKeyAuth() elif has_explicit_litellm_key: # Explicit x-litellm-api-key provided - always validate normally diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 394e32d1331..fe060197c22 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -294,9 +294,18 @@ def route_in_additonal_public_routes(current_route: str): def get_request_route(request: Request) -> str: """ - Helper to get the route from the request + Resolve the request route from the ASGI scope, with ``root_path`` stripped. - remove base url from path if set e.g. `/genai/chat/completions` -> `/chat/completions + Prefer this over ``request.url.path`` for any auth, ACL, routing, or + audit-log decision: Starlette reconstructs ``url.path`` by interpolating + the Host header into a URL string and re-parsing with ``urlsplit``, so a + malformed Host (e.g. ``localhost/?x=1``) collapses ``url.path`` to ``"/"`` + while FastAPI continues to dispatch on ``scope["path"]``. ``scope["path"]`` + is uvicorn's parse of the HTTP request line and matches the actual + handler, so it's the authoritative route. + + Also normalizes sub-path deployments by stripping ``scope["root_path"]`` + e.g. ``/genai/chat/completions`` -> ``/chat/completions``. """ try: scope = request.scope diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index 26bbdef3090..9acc941f44d 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -577,7 +577,11 @@ class RouteChecks: Returns: bool: True if `thread` or `assistant` is in the request path, False otherwise """ - if "thread" in request.url.path or "assistant" in request.url.path: + # Inline import — auth_utils participates in a proxy import cycle. + from .auth_utils import get_request_route # noqa: PLC0415 + + route = get_request_route(request) + if "thread" in route or "assistant" in route: return True return False diff --git a/litellm/proxy/common_utils/http_parsing_utils.py b/litellm/proxy/common_utils/http_parsing_utils.py index 71abdfa5e9e..7abf1ef6760 100644 --- a/litellm/proxy/common_utils/http_parsing_utils.py +++ b/litellm/proxy/common_utils/http_parsing_utils.py @@ -508,7 +508,10 @@ def _add_vector_store_id_from_path(request_data: dict, request: Request) -> None request_data: The request data dictionary to populate request: The FastAPI Request object """ - path = request.url.path + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + + path = get_request_route(request) vector_store_match = re.search(r"/vector_stores/([^/]+)/", path) if vector_store_match: vector_store_id = vector_store_match.group(1) diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 8fd19548cbb..02e73d01b6e 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -148,7 +148,10 @@ async def test_endpoint(request: Request): dict: A dictionary containing the route of the request URL. """ # ping the proxy server to check if its healthy - return {"route": request.url.path} + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + + return {"route": get_request_route(request)} @router.get( diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 7467bbae232..8986a34f4f1 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -104,8 +104,10 @@ def _get_metadata_variable_name(request: Request) -> str: For ALL other endpoints we call this "metadata" """ - path = request.url.path + # Inline imports — auth_utils/route_checks participate in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + path = get_request_route(request) if "thread" in path or "assistant" in path: return "litellm_metadata" diff --git a/litellm/proxy/management_helpers/utils.py b/litellm/proxy/management_helpers/utils.py index 3e42d392077..07764aa03b5 100644 --- a/litellm/proxy/management_helpers/utils.py +++ b/litellm/proxy/management_helpers/utils.py @@ -393,7 +393,12 @@ def management_endpoint_wrapper(func): if open_telemetry_logger is not None: if _http_request: - _route = _http_request.url.path + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) + + _route = get_request_route(_http_request) _request_body: dict = await _read_request_body( request=_http_request ) @@ -438,7 +443,12 @@ def management_endpoint_wrapper(func): if open_telemetry_logger is not None: _http_request = kwargs.get("http_request") if _http_request: - _route = _http_request.url.path + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) + + _route = get_request_route(_http_request) _request_body: dict = await _read_request_body( request=_http_request ) diff --git a/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py b/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py index d582240395f..c39598fca35 100644 --- a/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py +++ b/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py @@ -1152,11 +1152,14 @@ def create_pass_through_route( user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), subpath: str = "", # captures sub-paths when include_subpath=True ): + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) from litellm.proxy.pass_through_endpoints.pass_through_endpoints import ( InitPassThroughEndpointHelpers, ) - path = request.url.path + path = get_request_route(request) # Parse request data based on content type ( diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index 805d0ec1953..ef3de124f76 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -1815,7 +1815,10 @@ async def ui_view_spend_logs( # noqa: PLR0915 ) try: - is_v2 = "/spend/logs/v2" in request.url.path + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + + is_v2 = "/spend/logs/v2" in get_request_route(request) formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"] def parse_date(date_str: str) -> datetime: diff --git a/litellm/proxy/vector_store_endpoints/utils.py b/litellm/proxy/vector_store_endpoints/utils.py index 5499abb19d5..7bcb2ce0a00 100644 --- a/litellm/proxy/vector_store_endpoints/utils.py +++ b/litellm/proxy/vector_store_endpoints/utils.py @@ -102,11 +102,16 @@ def is_allowed_to_call_vector_store_endpoint( provider_config.get_vector_store_endpoints_by_type() ) + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + + request_route = get_request_route(request) + # Determine the permission type based on the request permission_type = None for endpoint in provider_vector_store_endpoints["read"]: if request.method == endpoint[0] and _does_endpoint_match( - endpoint[1], request.url.path + endpoint[1], request_route ): permission_type = "read" break @@ -114,7 +119,7 @@ def is_allowed_to_call_vector_store_endpoint( if permission_type is None: for endpoint in provider_vector_store_endpoints["write"]: if request.method == endpoint[0] and _does_endpoint_match( - endpoint[1], request.url.path + endpoint[1], request_route ): permission_type = "write" break @@ -164,10 +169,15 @@ def is_allowed_to_call_vector_store_files_endpoint( provider_config.get_vector_store_file_endpoints_by_type() ) + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import get_request_route # noqa: PLC0415 + + request_route = get_request_route(request) + permission_type: Optional[str] = None for endpoint in provider_vector_store_endpoints.get("read", ()): if request.method == endpoint[0] and _does_endpoint_match( - endpoint[1], request.url.path + endpoint[1], request_route ): permission_type = "read" break @@ -175,7 +185,7 @@ def is_allowed_to_call_vector_store_files_endpoint( if permission_type is None: for endpoint in provider_vector_store_endpoints.get("write", ()): if request.method == endpoint[0] and _does_endpoint_match( - endpoint[1], request.url.path + endpoint[1], request_route ): permission_type = "write" break diff --git a/tests/proxy_unit_tests/test_proxy_routes.py b/tests/proxy_unit_tests/test_proxy_routes.py index 812e4e1ac41..545e1208453 100644 --- a/tests/proxy_unit_tests/test_proxy_routes.py +++ b/tests/proxy_unit_tests/test_proxy_routes.py @@ -172,3 +172,148 @@ def test_get_request_route_with_base_url_not_at_start(): request = create_request("/api/genai/test") result = get_request_route(request) assert result == "/api/genai/test" + + +def _create_request_with_host_header(path: str, host_header: str) -> Request: + return Request( + { + "type": "http", + "method": "GET", + "scheme": "http", + "server": ("localhost", 4000), + "path": path, + "query_string": b"", + "headers": [(b"host", host_header.encode())], + "client": ("127.0.0.1", 50000), + "root_path": "", + } + ) + + +@pytest.mark.parametrize( + "host_header", + [ + "localhost/?x=1", + "localhost:4000/?x=1", + "localhost/#test", + "localhost:4000/#test", + ], +) +def test_get_request_route_not_bypassed_by_malformed_host(host_header: str): + for protected_path in [ + "/health", + "/user/new", + "/key/generate", + "/get/internal_user_settings", + ]: + request = _create_request_with_host_header( + path=protected_path, host_header=host_header + ) + result = get_request_route(request) + assert ( + result == protected_path + ), f"Host: {host_header!r} caused route {protected_path!r} to resolve as {result!r}" + + +# --------------------------------------------------------------------------- +# Regression tests for variant call sites that previously read request.url.path +# (Host-derived) instead of the ASGI scope path. Each test sends a Host header +# crafted to collapse url.path to a substring the call site's decision logic +# would match on, while scope["path"] is the real (unmatching) route. +# --------------------------------------------------------------------------- + +_BYPASS_HOSTS = [ + "localhost/?x=1", + "localhost:4000/?x=1", + "localhost/#test", + "localhost:4000/#test", +] + + +def _is_assistants(req): + return RouteChecks._is_assistants_api_request(req) + + +def _metadata_var_name(req): + from litellm.proxy.litellm_pre_call_utils import _get_metadata_variable_name + + return _get_metadata_variable_name(req) + + +def _vector_store_id_in_path(req): + from litellm.proxy.common_utils.http_parsing_utils import ( + _add_vector_store_id_from_path, + ) + + data: dict = {} + _add_vector_store_id_from_path(request_data=data, request=req) + return "vector_store_id" in data + + +# (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