Merge 41fa14f867 into c2c2a623c0

litellm/proxy/list_api/common.py

@@ -1,11 +1,11 @@
 """Contract machinery shared by every LiteLLM-defined list route, on any surface."""
 
+from collections.abc import Iterator
 from typing import Final
 from urllib.parse import urlencode
 
 from fastapi import Request
-from fastapi.dependencies.utils import get_flat_params
-from fastapi.params import ParamTypes
+from fastapi.dependencies.models import Dependant
 from fastapi.responses import JSONResponse
 
 from litellm.types.proxy.management_endpoints.management_v1 import (
@@ -37,18 +37,19 @@ def problem_response(problem: ProblemDetail) -> JSONResponse:
     )
 
 
+def _query_param_aliases(dependant: Dependant) -> Iterator[str]:
+    for field in dependant.query_params:
+        yield field.alias
+    for sub_dependant in dependant.dependencies:
+        yield from _query_param_aliases(sub_dependant)
+
+
 def _declared_query_params(request: Request) -> frozenset[str]:
     route: Final = request.scope.get("route")
     dependant: Final = getattr(route, "dependant", None)
-    if dependant is None:
+    if not isinstance(dependant, Dependant):
         return frozenset()
-    # fastapi>=0.140.7 removed get_flat_dependant(); get_flat_params() returns the
-    # flattened (deduped) param list. Filter to query params to match the old behavior.
-    return frozenset(
-        field.alias
-        for field in get_flat_params(dependant)
-        if getattr(field.field_info, "in_", None) == ParamTypes.query
-    )
+    return frozenset(_query_param_aliases(dependant))
 
 
 def escape_like(value: str) -> str:

tests/code_coverage_tests/recursive_detector.py

@@ -67,6 +67,7 @@ IGNORE_FUNCTIONS = [
     "_redact_agent_params_tree",  # max depth set (default 10), same shape as _redact_sensitive_litellm_params.
     "_restore_redacted_nested_value",  # max depth set (default 10), mirrors _redact_agent_params_tree on the write side.
     "_unqualified",  # bounded by the qualifier depth of a static TypedDict annotation (Annotated, Required/NotRequired, ReadOnly around one type, no cycles possible).
+    "_query_param_aliases",  # bounded by the route's Depends() tree, fixed at registration and already resolved acyclically by fastapi itself.
 ]
 
 

tests/test_litellm/proxy/list_api/test_common.py

@@ -52,11 +52,8 @@ def test_an_unknown_query_param_is_rejected_as_a_problem():
 
 
 def test_a_path_param_name_is_not_a_declared_query_param():
-    """The flatten step returns path+query+header together; only query names count as declared.
-
-    If the ParamTypes.query filter were dropped, `thing_id` (a path param) would leak
-    into the declared set and this request would be wrongly accepted.
-    """
+    """`_query_param_aliases` walks `Dependant.query_params`; if it were ever widened
+    to also read `.path_params`, `thing_id` would leak into the declared set."""
     response = _client().get("/things/abc", params={"thing_id": "x"})
     assert response.status_code == 400
     assert "thing_id" in response.json()["detail"]
@@ -69,7 +66,6 @@ def test_a_header_param_name_is_not_a_declared_query_param():
 
 
 def test_declared_query_params_isolates_query_aliases_from_other_param_types():
-    captured: dict[str, frozenset[str]] = {}
     app = FastAPI()
 
     @app.get("/things/{thing_id}")
@@ -79,12 +75,11 @@ def test_declared_query_params_isolates_query_aliases_from_other_param_types():
         status: Annotated[str | None, Query(alias="filter[status]")] = None,
         page: Annotated[int, Query(ge=1)] = 1,
         x_trace: Annotated[str | None, Header()] = None,
-    ) -> dict[str, bool]:
-        captured["declared"] = _declared_query_params(request)
-        return {"ok": True}
+    ) -> dict[str, list[str]]:
+        return {"declared": sorted(_declared_query_params(request))}
 
-    TestClient(app).get("/things/abc")
-    assert captured["declared"] == frozenset({"filter[status]", "page"})
+    response = TestClient(app).get("/things/abc")
+    assert frozenset(response.json()["declared"]) == frozenset({"filter[status]", "page"})
 
 
 def test_declared_query_params_is_empty_when_the_route_has_no_dependant():
@@ -102,9 +97,23 @@ def test_declared_query_params_is_empty_when_the_route_has_no_dependant():
     assert _declared_query_params(request) == frozenset()
 
 
-# fastapi removed these in 0.140.7, which `pyproject.toml` still allows via
-# `fastapi>=0.136.3,<1.0`. Add a name here whenever a supported release drops one.
-FASTAPI_NAMES_REMOVED_IN_0_140_7 = frozenset({"get_flat_dependant"})
+def _shared_pagination(page: Annotated[int, Query()] = 1) -> int:
+    return page
+
+
+def test_declared_query_params_includes_a_param_declared_only_on_a_shared_sub_dependency():
+    """`page` sits on `_shared_pagination`'s own Dependant, nested under the route's
+    `dependencies`, not on the route function's own `query_params` — the case a
+    non-recursive walk over just the top-level Dependant would miss."""
+    app = FastAPI()
+
+    @app.get("/probe")
+    def _handler(request: Request, page: Annotated[int, Depends(_shared_pagination)]) -> dict[str, list[str]]:
+        return {"declared": sorted(_declared_query_params(request))}
+
+    response = TestClient(app).get("/probe")
+    assert frozenset(response.json()["declared"]) == frozenset({"page"})
+
 
 LIST_API_PACKAGE = Path(str(common_module.__file__)).parent
 PROXY_PACKAGE = LIST_API_PACKAGE.parent
@@ -123,37 +132,24 @@ def _public_names(module: ModuleType) -> frozenset[str]:
     return frozenset(name for name in vars(module) if not name.startswith("_"))
 
 
-def _fastapi_names_imported_by(source_file: Path) -> frozenset[str]:
+def _modules_imported_from_by(source_file: Path) -> frozenset[str]:
     tree = ast.parse(source_file.read_text())
-    return frozenset(
-        alias.name
-        for node in ast.walk(tree)
-        if isinstance(node, ast.ImportFrom) and (node.module or "").startswith("fastapi")
-        for alias in node.names
-    )
+    return frozenset(node.module for node in ast.walk(tree) if isinstance(node, ast.ImportFrom) and node.module)
 
 
 @pytest.mark.parametrize("source_file", FRAMEWORK_SOURCE_FILES, ids=lambda path: f"{path.parent.name}/{path.name}")
-def test_no_module_imports_a_fastapi_name_removed_in_a_supported_release(source_file: Path):
-    """`pyproject.toml` allows fastapi up to <1.0, but CI only ever resolves 0.136.3.
-
-    Every other test here passes just as well against a module importing a name
-    fastapi has since deleted, because the pinned fastapi still has it. On a user's
-    fastapi>=0.140.7 that import is an ImportError, and `proxy_server` imports every one
-    of these packages unguarded at module level, so it takes the whole proxy down rather
-    than just these routes. Globbing them means a new module is covered on sight.
+def test_no_module_imports_from_the_private_fastapi_dependencies_utils_module(source_file: Path):
+    """fastapi.dependencies.utils is a private module fastapi has already removed a name
+    from once (get_flat_dependant, in 0.140.7) without notice; the names it still exposes
+    (e.g. get_flat_params) carry the same risk. `proxy_server` imports every one of these
+    packages unguarded at module level, so an ImportError here takes down the whole proxy
+    rather than just these routes. Globbing them means a new module is covered on sight.
     """
-    assert not _fastapi_names_imported_by(source_file) & FASTAPI_NAMES_REMOVED_IN_0_140_7
+    assert "fastapi.dependencies.utils" not in _modules_imported_from_by(source_file)
 
 
-def test_common_still_imports_when_fastapi_has_dropped_those_names(monkeypatch: pytest.MonkeyPatch):
-    """The static check above cannot prove the module actually loads; this does.
-
-    Behaviour cannot be asserted under the same simulation: on 0.136.3
-    `get_flat_params` calls `get_flat_dependant` internally, so it raises NameError
-    once the name is gone. Loading is the part this pins.
-    """
-    for name in FASTAPI_NAMES_REMOVED_IN_0_140_7:
+def test_common_still_imports_with_fastapi_dependencies_utils_emptied_out(monkeypatch: pytest.MonkeyPatch):
+    for name in ("get_flat_dependant", "get_flat_params"):
         monkeypatch.delattr(fastapi_dependency_utils, name, raising=False)
     spec = importlib.util.spec_from_file_location(
         "list_api_common__simulated_fastapi", Path(str(common_module.__file__))