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.
This commit is contained in:
user 2026-05-31 06:15:03 +00:00
parent 9d18c15fb2
commit dc34d1d8cd
No known key found for this signature in database
2 changed files with 100 additions and 15 deletions

View file

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

View file

@ -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(),
)