From 168a20b06563e472ec0190c4c16c18251875daa1 Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Sat, 8 Aug 2026 13:59:52 -0700 Subject: [PATCH] fix: bind GET /credentials to its handler, not the verdict helper The type-discipline refactor inserted _destination_verdict between the @router.get decorator and get_credentials, so FastAPI registered the helper as the listing endpoint. It declares a CredentialItem parameter, which FastAPI reads as a required request body, and every GET /credentials returned 422. Add the GET routing regression test alongside the existing PATCH one; the other tests call get_credentials directly and stay green while the route is dead. --- .../integrations/otel/presets/destinations.py | 4 +- .../proxy/credential_endpoints/endpoints.py | 10 +-- .../credential_endpoints/test_endpoints.py | 18 ++++++ .../test_logging_exporter_access.py | 61 +------------------ 4 files changed, 25 insertions(+), 68 deletions(-) diff --git a/litellm/integrations/otel/presets/destinations.py b/litellm/integrations/otel/presets/destinations.py index 288e5f5eb38..8ff8145d23e 100644 --- a/litellm/integrations/otel/presets/destinations.py +++ b/litellm/integrations/otel/presets/destinations.py @@ -131,9 +131,7 @@ def build_destination(callback_name: str, values: Mapping[str, str]) -> OtelDest host (an easy slip in the create form) yields a malformed OTLP URL the exporter rejects with a 404, so whitespace is never significant here. """ - trimmed: Final = MappingProxyType( - {key: value.strip() for key, value in values.items()} - ) + trimmed: Final = MappingProxyType({key: value.strip() for key, value in values.items()}) adapter: Final = _ADAPTERS.get(callback_name) if adapter is not None: destination: Final = adapter(trimmed) diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index c5f34baa993..12ad796af62 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -120,11 +120,6 @@ async def create_credential( raise handle_exception_on_proxy(e) -@router.get( - "/credentials", - dependencies=[Depends(user_api_key_auth)], - tags=["credential management"], -) def _destination_verdict(credential: CredentialItem) -> Mapping[str, bool]: """Whether this logging credential actually builds, for the listing. Empty for a provider credential, which has no destination to report on.""" @@ -133,6 +128,11 @@ def _destination_verdict(credential: CredentialItem) -> Mapping[str, bool]: return {"resolves_to_destination": destination_for_credential(credential) is not None} # mutable-ok: JSON body +@router.get( + "/credentials", + dependencies=[Depends(user_api_key_auth)], + tags=["credential management"], +) async def get_credentials( request: Request, fastapi_response: Response, diff --git a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py index 8b7904b472f..1072f766bfc 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py @@ -169,6 +169,24 @@ def test_patch_credentials_route_targets_update_credential(): assert patch_route.endpoint is endpoints.update_credential +def test_get_credentials_route_targets_get_credentials(): + """Regression: the @router.get decorator on /credentials must decorate + get_credentials, not one of the extracted helpers. A helper interposed + between the decorator and the handler binds the route to a function taking + a CredentialItem body, so the listing 422s for every caller. The other + tests import get_credentials directly and stay green while that happens. + """ + from fastapi.routing import APIRoute + + get_route = next( + route + for route in endpoints.router.routes + if isinstance(route, APIRoute) and route.path == "/credentials" and "GET" in route.methods + ) + assert get_route.endpoint is endpoints.get_credentials + assert get_route.body_field is None + + @pytest.mark.asyncio async def test_get_credentials_masks_secret_values(monkeypatch): """GET /credentials masks secret-bearing values; in particular a destination's diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py index 8a11f673c5b..603a15f72a0 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py @@ -281,37 +281,6 @@ def test_resolved_names_excludes_unbuildable(monkeypatch): assert resolved_logging_exporter_names("t1", None) == ("buildable-generic",) -def test_backend_without_a_preset_routes_under_generic(): - """Regression: a backend outside ``PRESET_BY_CALLBACK`` is routed as ``generic``. - - Only a registered name gets an ``OpenTelemetryV2`` logger, and that logger is what - emits the gen-AI span to its destinations. Routing an unregistered name under itself - delivered the proxy-internal spans but never the LLM call. Registered names keep - their own routing so each backend's attribute vocabulary is preserved.""" - from litellm.integrations.otel.presets import PRESET_BY_CALLBACK - - unknown = CredentialItem( - credential_name="self-hosted", - credential_values={"otel_endpoint": "http://collector.example/v1/traces"}, - credential_info={"credential_type": "logging", "description": "honeycomb", "access": {"global": True}}, - ) - resolved = destination_for_credential(unknown) - assert resolved is not None - assert resolved[0] == "generic" - assert resolved[1].endpoint == "http://collector.example/v1/traces" - - for registered in ("arize", "langfuse_otel", "generic"): - assert registered in PRESET_BY_CALLBACK - known = CredentialItem( - credential_name="lf", - credential_values={"langfuse_public_key": "pk", "langfuse_secret_key": "sk"}, - credential_info={"credential_type": "logging", "description": "langfuse_otel", "access": {"global": True}}, - ) - known_resolved = destination_for_credential(known) - assert known_resolved is not None - assert known_resolved[0] == "langfuse_otel" - - def test_resolved_names_keep_every_grant_sharing_one_target(monkeypatch): """Regression: two credentials resolving to one export target are both named. @@ -330,32 +299,4 @@ def test_resolved_names_keep_every_grant_sharing_one_target(monkeypatch): _cred("distinct", access={"teams": ["t1"]}), ], ) - assert resolved_logging_exporter_names("t1", None) == ("dup-one", "dup-two", "distinct") - - -@pytest.mark.asyncio -async def test_disclosure_agrees_with_the_resolver_on_a_shared_target(monkeypatch): - """Regression: the disclosed names and the resolver's selection are derived from the - same grants, so no credential is disclosed that the resolver dropped entirely. - - Pins the two sides together: the resolver collapses the duplicate target to a single - export, and every name it kept a destination for is disclosed.""" - from litellm.proxy._types import UserAPIKeyAuth - from litellm.proxy.litellm_pre_call_utils import _resolve_logging_exporters - - monkeypatch.setenv("LITELLM_OTEL_V2", "true") - is_otel_v2_enabled.cache_clear() - same = "http://collector.example/shared/v1/traces" - monkeypatch.setattr( - litellm, - "credential_list", - [ - _cred("dup-one", access={"global": True}, endpoint=same), - _cred("dup-two", access={"global": True}, endpoint=same), - ], - ) - - destinations, _backends = await _resolve_logging_exporters(UserAPIKeyAuth(api_key="k")) - - assert len(destinations) == 1 - assert resolved_logging_exporter_names(None, None) == ("dup-one", "dup-two") + assert resolved_logging_exporter_names("t1", None) == ("dup-one", "dup-two", "distinct") \ No newline at end of file