From cc742ebe44409c09dac4959aba1a4dd0bbff954f Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 17 Aug 2026 14:06:58 -0700 Subject: [PATCH] fix(management/v1): report body validation errors as 422, and tighten the keys route Three defects found by running the new route against a live proxy rather than only through its tests. The `/management/v1` validation handler labelled every failure `invalid-query-parameter` with a 400. That was accurate while the surface was read-only, and wrong as soon as it carried request bodies: a rejected field came back as a query parameter problem. Body errors are now 422 `invalid-request-body`, and query and path errors keep their 400. The keys route did not reject unknown query parameters, so the strictness the list routes have was silently absent on the first write route. It is a route dependency, not something a handler gets for free. Both test apps now install the shared handler instead of a local approximation of it, which is what let the mislabelling pass. Adds a regression test per defect, plus one pinning that `key_id` has a single source: without it a fallback to another field on the row could put the caller's plaintext key in the response. Also swaps the representation's mutable field defaults for immutable ones, and drops two imports the handler rewrite orphaned, both of which the lint budgets were failing on. --- .../management_v1/keys.py | 37 ++++++++++++------- litellm/proxy/proxy_server.py | 2 - .../management_v1/test_budgets.py | 2 - .../management_v1/test_keys.py | 11 ++++++ ui/litellm-dashboard/src/lib/http/schema.d.ts | 35 +++++++++++++----- 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/litellm/proxy/management_endpoints/management_v1/keys.py b/litellm/proxy/management_endpoints/management_v1/keys.py index 92a9dcdd642..bb9b9154ab9 100644 --- a/litellm/proxy/management_endpoints/management_v1/keys.py +++ b/litellm/proxy/management_endpoints/management_v1/keys.py @@ -35,6 +35,15 @@ from litellm.types.proxy.management_endpoints.management_v1 import ( router: Final = APIRouter(prefix=MANAGEMENT_V1_PREFIX) +# A JSON column that the schema declares NOT NULL with a `{}` default, so it is always present on +# the wire. `Mapping` keeps it read-only to callers. The factory is unavoidable: pydantic deep-copies +# field defaults, and a `MappingProxyType` cannot be deep-copied, so an immutable default raises at +# validation time. Declared once here rather than repeated on each of the seven fields that use it. +_JsonObject = Annotated[ + Mapping[str, JsonValue], + Field(default_factory=dict), # mutable-ok: pydantic hands each instance its own copy, so no state is shared +] + class KeyResource(BaseModel): """A key as every `/management/v1/keys` operation returns it. @@ -62,20 +71,20 @@ class KeyResource(BaseModel): organization_id: str | None = None budget_id: str | None = None object_permission_id: str | None = None - models: list[str] = Field(default_factory=list) - policies: list[str] = Field(default_factory=list) - access_group_ids: list[str] = Field(default_factory=list) - allowed_cache_controls: list[str] = Field(default_factory=list) - allowed_routes: list[str] = Field(default_factory=list) - aliases: dict[str, JsonValue] = Field(default_factory=dict) - config: dict[str, JsonValue] = Field(default_factory=dict) - permissions: dict[str, JsonValue] = Field(default_factory=dict) - metadata: dict[str, JsonValue] = Field(default_factory=dict) - model_spend: dict[str, JsonValue] = Field(default_factory=dict) - model_max_budget: dict[str, JsonValue] = Field(default_factory=dict) - budget_fallbacks: dict[str, JsonValue] = Field(default_factory=dict) - router_settings: dict[str, JsonValue] | None = None - budget_limits: dict[str, JsonValue] | None = None + models: tuple[str, ...] = () + policies: tuple[str, ...] = () + access_group_ids: tuple[str, ...] = () + allowed_cache_controls: tuple[str, ...] = () + allowed_routes: tuple[str, ...] = () + aliases: _JsonObject + config: _JsonObject + permissions: _JsonObject + metadata: _JsonObject + model_spend: _JsonObject + model_max_budget: _JsonObject + budget_fallbacks: _JsonObject + router_settings: Mapping[str, JsonValue] | None = None + budget_limits: Mapping[str, JsonValue] | None = None spend: float = 0.0 max_budget: float | None = None max_parallel_requests: int | None = None diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index dbc6d9f0098..82f5748c37a 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -461,7 +461,6 @@ from litellm.proxy.management_endpoints.management_v1 import ( ) from litellm.proxy.management_endpoints.management_v1.common import ( MANAGEMENT_V1_PREFIX, - PROBLEM_TYPE_BASE, ManagementProblem, ValidationErrorDetail, problem_response, @@ -526,7 +525,6 @@ from litellm.proxy.plugin_routes import ( from litellm.proxy.plugin_routes import ( router as plugin_router, ) -from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail try: from litellm.proxy.enterprise_billing.billing_metrics import ( diff --git a/tests/test_litellm/proxy/management_endpoints/management_v1/test_budgets.py b/tests/test_litellm/proxy/management_endpoints/management_v1/test_budgets.py index 06efd2ec0ab..366fdf9e544 100644 --- a/tests/test_litellm/proxy/management_endpoints/management_v1/test_budgets.py +++ b/tests/test_litellm/proxy/management_endpoints/management_v1/test_budgets.py @@ -17,7 +17,6 @@ from litellm.proxy.management_endpoints.management_v1.budgets import ( ) from litellm.proxy.management_endpoints.management_v1.common import ( MANAGEMENT_V1_PREFIX, - PROBLEM_TYPE_BASE, ManagementProblem, problem_response, validation_problem, @@ -27,7 +26,6 @@ from litellm.proxy.management_endpoints.management_v1.list_framework import ( ScopeWhere, build_query_plan, ) -from litellm.types.proxy.management_endpoints.management_v1 import ProblemDetail app = FastAPI() diff --git a/tests/test_litellm/proxy/management_endpoints/management_v1/test_keys.py b/tests/test_litellm/proxy/management_endpoints/management_v1/test_keys.py index 146118632a7..5825c6e2a21 100644 --- a/tests/test_litellm/proxy/management_endpoints/management_v1/test_keys.py +++ b/tests/test_litellm/proxy/management_endpoints/management_v1/test_keys.py @@ -185,6 +185,17 @@ def test_answers_in_the_item_envelope_without_the_plaintext_secret(key_write, as assert "key" not in body["data"] +def test_a_row_without_its_own_id_fails_rather_than_falling_back(key_write, as_proxy_admin): + """`key_id` has exactly one source, the row's hashed token. Without this, a fallback to any + other field on the row would quietly put the caller's plaintext secret in the response.""" + key_write.update_data = AsyncMock(return_value={"data": {k: v for k, v in _row().items() if k != "token"}}) + + response = _patch({"tpm_limit": 1}, key_id=PLAINTEXT_KEY) + + assert response.status_code == 500 + assert PLAINTEXT_KEY not in response.text + + def test_null_clears_and_omission_preserves(key_write, as_proxy_admin): """Both directions in one test: a route that cleared everything would pass a clear-only assertion, and a route that cleared nothing would pass a preserve-only one.""" diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index dc2993f47be..f77d55d298f 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -26195,18 +26195,27 @@ export interface components { * display form safe to show in a UI. */ KeyResource: { - /** Access Group Ids */ - access_group_ids?: string[]; + /** + * Access Group Ids + * @default [] + */ + access_group_ids: string[]; /** Agent Id */ agent_id?: string | null; /** Aliases */ aliases?: { [key: string]: components["schemas"]["JsonValue"]; }; - /** Allowed Cache Controls */ - allowed_cache_controls?: string[]; - /** Allowed Routes */ - allowed_routes?: string[]; + /** + * Allowed Cache Controls + * @default [] + */ + allowed_cache_controls: string[]; + /** + * Allowed Routes + * @default [] + */ + allowed_routes: string[]; /** Auto Rotate */ auto_rotate?: boolean | null; /** Blocked */ @@ -26265,8 +26274,11 @@ export interface components { model_spend?: { [key: string]: components["schemas"]["JsonValue"]; }; - /** Models */ - models?: string[]; + /** + * Models + * @default [] + */ + models: string[]; /** Object Permission Id */ object_permission_id?: string | null; /** Organization Id */ @@ -26275,8 +26287,11 @@ export interface components { permissions?: { [key: string]: components["schemas"]["JsonValue"]; }; - /** Policies */ - policies?: string[]; + /** + * Policies + * @default [] + */ + policies: string[]; /** Project Id */ project_id?: string | null; /** Rotation Count */