From be557c8103a23358f1c13fd6736ea567fce4d940 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 26 May 2026 23:45:24 -0700 Subject: [PATCH] chore(proxy): cherry-pick #28547 onto patch/v1.86.1 (#28969) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(proxy): cherry-pick #28547 onto patch/v1.86.1 Backport of #28547 (`d480ffda3c`) onto the `patch/v1.86.1` branch. Routes the remaining path-dependent call sites in auth, ACL, routing, and audit-log decisions through `get_request_route(request)` so they read from the ASGI `scope["path"]` instead of `request.url.path`. The helper itself already exists on v1.86.1 (added by #27878); this PR extends the helper's usage to the additional sites listed below. Sites routed through get_request_route: - _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 Regression tests in tests/proxy_unit_tests/test_proxy_routes.py 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"]. Conflict resolution ------------------- Cherry-pick applied cleanly with no conflicts. All 11 files plus the test file are pure `request.url.path` → `get_request_route(request)` swaps with the lazy auth_utils import (no feature drift). * bump: version 1.86.1 → 1.86.2 --- .../mcp_server/auth/user_api_key_auth_mcp.py | 14 +- 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 +- .../mcp_management_endpoints.py | 5 +- 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 ++- pyproject.toml | 4 +- tests/proxy_unit_tests/test_proxy_routes.py | 121 +++++++++++++++++- 13 files changed, 192 insertions(+), 27 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 708ec7f117..70fc2c233e 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 @@ -118,15 +118,19 @@ class MCPRequestHandler: return b"{}" request.body = mock_body # type: ignore + # Inline import — auth_utils participates in a proxy import cycle. + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) + + request_route = get_request_route(request) # Only OAuth metadata routes registered under /.well-known/ are public. - # Match on request.url.path (path-only, exact prefix) so the substring - # cannot be smuggled via query string, hostname, or a deeper URL segment. - if request.url.path.startswith("/.well-known/"): + if request_route.startswith("/.well-known/"): validated_user_api_key_auth = UserAPIKeyAuth() elif ( not litellm_api_key and MCPRequestHandler._target_servers_delegate_auth_to_upstream( # noqa: E501 - path=request.url.path, mcp_servers=mcp_servers + path=request_route, mcp_servers=mcp_servers ) ): # Operator opted this oauth2 server into upstream-delegated auth @@ -174,7 +178,7 @@ class MCPRequestHandler: "401", "403", ) and MCPRequestHandler._target_servers_use_oauth2( - path=request.url.path, mcp_servers=mcp_servers + path=request_route, mcp_servers=mcp_servers ): verbose_logger.debug( "MCP OAuth2: target server is OAuth2-mode, treating " diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 637a4a070c..201012c867 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -497,9 +497,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 b2878ba0ae..3144c5cd25 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -627,7 +627,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 71abdfa5e9..7abf1ef676 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 ff3df11c44..ba3aee7504 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -151,7 +151,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 8cb9a11ffe..8283c26ba5 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -332,8 +332,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_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index e9d9c243e7..431ff49c7c 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -1568,6 +1568,9 @@ if MCP_AVAILABLE: from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415 global_mcp_server_manager, ) + from litellm.proxy.auth.auth_utils import ( # noqa: PLC0415 + get_request_route, + ) server_id = request.path_params.get("server_id", "") if server_id: @@ -1584,7 +1587,7 @@ if MCP_AVAILABLE: ): # For /token, require PKCE authorization_code; refresh_token # grants must NOT bypass auth (see comment above). - path_lower = (request.url.path or "").rstrip("/").lower() + path_lower = get_request_route(request).rstrip("/").lower() if path_lower.endswith("/token"): body_data = await _read_request_body(request=request) grant_type = (body_data or {}).get("grant_type", "") diff --git a/litellm/proxy/management_helpers/utils.py b/litellm/proxy/management_helpers/utils.py index f2d6e9612f..b05c12be5f 100644 --- a/litellm/proxy/management_helpers/utils.py +++ b/litellm/proxy/management_helpers/utils.py @@ -469,7 +469,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 ) @@ -514,7 +519,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 df52c0fe20..26f89e6b31 100644 --- a/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py +++ b/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py @@ -1272,11 +1272,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 d030fabe8b..335f917aa1 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -1817,7 +1817,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 657b520b27..1221ccf119 100644 --- a/litellm/proxy/vector_store_endpoints/utils.py +++ b/litellm/proxy/vector_store_endpoints/utils.py @@ -330,11 +330,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 @@ -342,7 +347,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 @@ -392,10 +397,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 @@ -403,7 +413,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/pyproject.toml b/pyproject.toml index 625a36d117..9e5a01c910 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "litellm" -version = "1.86.1" +version = "1.86.2" description = "Library to easily interface with LLM API providers" readme = "README.md" requires-python = ">=3.10, <3.14" @@ -251,7 +251,7 @@ source-exclude = [ profile = "black" [tool.commitizen] -version = "1.86.1" +version = "1.86.2" version_files = [ "pyproject.toml:^version", ] diff --git a/tests/proxy_unit_tests/test_proxy_routes.py b/tests/proxy_unit_tests/test_proxy_routes.py index 34123e992c..db41bd6540 100644 --- a/tests/proxy_unit_tests/test_proxy_routes.py +++ b/tests/proxy_unit_tests/test_proxy_routes.py @@ -217,9 +217,120 @@ def _create_request_with_host_header(path: str, host_header: str) -> Request: ], ) 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}" + 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