From dc34d1d8cd5dc69a4d600dc55f7d16723ae47c19 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Sun, 31 May 2026 06:15:03 +0000 Subject: [PATCH] chore(proxy): broaden non-admin model field restrictions to all cost fields and reject os.environ refs Two gaps in the non-admin model-write authorization: the pricing gate only covered a six-field subset (input/output_cost_per_token/character + cache costs) while litellm accepts many more cost fields (per-second, per-pixel, per-request, tiered, etc.), and the team-credential requirement treated any non-empty api_key as team-owned even when it was an os.environ/ reference that resolves to the proxy's own environment secret at call time. Match pricing fields by naming convention (cost_per / _cost) so the set can't drift, and reject os.environ/ references in non-admin model parameters (proxy admins still use them as the normal config pattern), consistent with the connection-test endpoint. --- .../model_management_endpoints.py | 63 ++++++++++++++----- .../test_model_management_endpoints.py | 52 +++++++++++++++ 2 files changed, 100 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index 2c55bd9e199..ed5a3ea92f8 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -142,30 +142,63 @@ def _strip_credentials_on_destination_change( merged_litellm_params.pop(field, None) +def _is_pricing_field(field: str) -> bool: + """Custom per-token / per-second / per-pixel / per-request (and tiered) cost + overrides all follow this naming, so match by convention rather than an + enumerated subset that drifts out of date.""" + return "cost_per" in field or field.endswith("_cost") + + +def _contains_env_reference(value: object) -> bool: + """True if any (possibly nested) string is an `os.environ/` reference, which + resolves to a server-side environment secret at call time.""" + if isinstance(value, str): + return value.startswith("os.environ/") + if isinstance(value, dict): + return any(_contains_env_reference(v) for v in value.values()) + if isinstance(value, list): + return any(_contains_env_reference(v) for v in value) + return False + + def _assert_privileged_model_fields_authorized( litellm_params: Optional[BaseModel], model_info: Optional[BaseModel], user_api_key_dict: UserAPIKeyAuth, ) -> None: - """Pricing overrides and credential-name binding are proxy-admin-only. + """Restrict privileged model fields on non-admin (team-admin) writes. - Per-token/character pricing (SPECIAL_MODEL_INFO_PARAMS) gates spend/budget - enforcement, so a non-admin must not set or clear it. litellm_credential_name - resolves a globally-stored credential by name with no ownership model, so - only a proxy admin may bind one to a model. + - Any custom pricing/cost field gates spend/budget enforcement, so a + non-admin must not set or clear one. + - `litellm_credential_name` resolves a globally-stored credential by name + with no ownership model, so only a proxy admin may bind one. + - `os.environ/` references resolve to the server's environment secrets + (e.g. the operator's global provider key), so a non-admin must supply + literal values, not references (mirrors /health/test_connection). """ if is_proxy_admin(user_api_key_dict): return - for field in SPECIAL_MODEL_INFO_PARAMS: - if _field_explicitly_set(litellm_params, field) or _field_explicitly_set( - model_info, field - ): - raise ProxyException( - message=f"Only proxy admins can set model pricing fields (e.g. {field}).", - type=ProxyErrorTypes.auth_error.value, - code=status.HTTP_403_FORBIDDEN, - param=field, - ) + for model in (litellm_params, model_info): + if model is None: + continue + for field, value in model.model_dump(exclude_unset=True).items(): + if _is_pricing_field(field): + raise ProxyException( + message=f"Only proxy admins can set model pricing fields (e.g. {field}).", + type=ProxyErrorTypes.auth_error.value, + code=status.HTTP_403_FORBIDDEN, + param=field, + ) + if _contains_env_reference(value): + raise ProxyException( + message=( + f"os.environ/ references are not permitted in non-admin " + f"model parameters (field: {field}); supply a literal value." + ), + type=ProxyErrorTypes.auth_error.value, + code=status.HTTP_403_FORBIDDEN, + param=field, + ) if ( _field_explicitly_set(litellm_params, "litellm_credential_name") and litellm_params.litellm_credential_name is not None # type: ignore diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index eaab7e61263..8b7cde9e166 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -2423,3 +2423,55 @@ class TestModelMgmtAuthzHardening: ) assert str(e.value.code) == "403" mock_prisma.db.litellm_proxymodeltable.update.assert_not_called() + + # --- Veria review follow-ups: broader pricing set + os.environ/ refs --- + + def test_non_special_pricing_field_is_proxy_admin_only(self): + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _assert_privileged_model_fields_authorized, + ) + from litellm.types.router import updateLiteLLMParams + + # input_cost_per_second is a cost field NOT in SPECIAL_MODEL_INFO_PARAMS. + with pytest.raises(ProxyException) as e: + _assert_privileged_model_fields_authorized( + litellm_params=updateLiteLLMParams(input_cost_per_second=0.0), + model_info=None, + user_api_key_dict=self._non_admin(), + ) + assert str(e.value.code) == "403" + # Proxy admin may set it. + _assert_privileged_model_fields_authorized( + litellm_params=updateLiteLLMParams(input_cost_per_second=0.0), + model_info=None, + user_api_key_dict=self._admin(), + ) + + def test_env_reference_rejected_for_non_admin(self): + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _assert_privileged_model_fields_authorized, + ) + from litellm.types.router import updateLiteLLMParams + + # os.environ/ api_key would resolve to the proxy's own global secret. + with pytest.raises(ProxyException) as e: + _assert_privileged_model_fields_authorized( + litellm_params=updateLiteLLMParams(api_key="os.environ/OPENAI_API_KEY"), + model_info=None, + user_api_key_dict=self._non_admin(), + ) + assert str(e.value.code) == "403" + # A literal credential is fine for a non-admin. + _assert_privileged_model_fields_authorized( + litellm_params=updateLiteLLMParams(api_key="sk-literal"), + model_info=None, + user_api_key_dict=self._non_admin(), + ) + # Proxy admins use env references as the normal config pattern. + _assert_privileged_model_fields_authorized( + litellm_params=updateLiteLLMParams(api_key="os.environ/OPENAI_API_KEY"), + model_info=None, + user_api_key_dict=self._admin(), + )