mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
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.
This commit is contained in:
parent
3af54b234b
commit
5255235eb3
11 changed files with 212 additions and 15 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
(
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue