refactor(proxy): stop reading from the private fastapi.dependencies.utils module

Issue #36922 (fastapi 0.141 removed get_flat_dependant) was already fixed by
f9b86b253a, which switched to get_flat_params(). That helper lives in the same
private fastapi.dependencies.utils module fastapi silently dropped
get_flat_dependant from, so it carries the same removal risk.

Read Dependant.query_params/.dependencies directly instead: the public
dataclass fields fastapi's own routing engine depends on, and the same fields
get_flat_params itself reads under the hood. Also generalizes the list-route
regression suite's private-import guard from a specific removed-names list to
any import from fastapi.dependencies.utils, so a future name removed from that
module fails the guard on sight.

Related to #36922

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGKSBA4M3B9gY52md5QpSo
This commit is contained in:
Neel-K26 2026-08-24 17:20:56 +05:30
parent c2c2a623c0
commit dab22eb192
2 changed files with 45 additions and 48 deletions

View file

@ -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:

View file

@ -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__))