mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
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.
This commit is contained in:
parent
3d00d3ee7c
commit
cc742ebe44
5 changed files with 59 additions and 28 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
35
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
35
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -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 */
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue