mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-30 01:52:18 +00:00
chore(proxy): close credential/SSRF/spend carve-outs using litellm's authoritative field sets
Several gates used hand-maintained field subsets that drifted behind litellm's own definitions. Pricing authorization now keys off the authoritative custom-pricing set the cost calculator actually reads (CustomPricingLiteLLMParams, e.g. tiered_pricing and the per-tier cache costs) plus base_model, which redirects the cost-lookup model; a name heuristic stays only as a fallback. The SSRF and destination sets now include the provider endpoint overrides litellm already bans at request time (sagemaker_base_url, s3_endpoint_url, deployment_url), and azure_ad_token joins the credential set. A non-admin destination change now also rejects a previously key-less model (it would fall back to the proxy environment key at the new endpoint), and the connection-test guard takes the resolved deployment's params so a caller cannot leave a stored credential riding along by overriding api_base while supplying an unrelated credential field. Adds regression tests for each: authoritative pricing fields and base_model, the new endpoint URLs (SSRF and resupply), key-less repoint, and partial-resupply credential exfil on test_connection.
This commit is contained in:
parent
ed6bba8d91
commit
debc2cf057
4 changed files with 209 additions and 41 deletions
|
|
@ -83,40 +83,39 @@ _HEALTH_CREDENTIAL_FIELDS = (
|
|||
"litellm_credential_name",
|
||||
"aws_secret_access_key",
|
||||
"aws_session_token",
|
||||
"azure_ad_token",
|
||||
"vertex_credentials",
|
||||
)
|
||||
_HEALTH_DESTINATION_FIELDS = (
|
||||
"api_base",
|
||||
"base_url",
|
||||
"aws_bedrock_runtime_endpoint",
|
||||
"aws_sts_endpoint",
|
||||
"api_version",
|
||||
"vertex_location",
|
||||
"vertex_project",
|
||||
"aws_region_name",
|
||||
)
|
||||
# Caller-controlled endpoint URLs: overriding one re-points the request (and any
|
||||
# credential) at a new host, so it is SSRF-validated.
|
||||
# credential) at a new host, so it is SSRF-validated. Mirrors the endpoint-redirect
|
||||
# subset of auth_utils._BANNED_REQUEST_BODY_PARAMS.
|
||||
_HEALTH_URL_FIELDS = (
|
||||
"api_base",
|
||||
"base_url",
|
||||
"aws_bedrock_runtime_endpoint",
|
||||
"aws_sts_endpoint",
|
||||
"sagemaker_base_url",
|
||||
"s3_endpoint_url",
|
||||
"deployment_url",
|
||||
)
|
||||
|
||||
|
||||
def _assert_non_admin_destination_override_is_safe(
|
||||
request_litellm_params: dict, user_api_key_dict: UserAPIKeyAuth
|
||||
config_litellm_params: dict,
|
||||
request_litellm_params: dict,
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
) -> None:
|
||||
"""Confused-deputy / SSRF guard for /health/test_connection.
|
||||
|
||||
When a non-admin overrides the connection target (api_base/base_url/provider
|
||||
endpoint), the request must carry its own non-empty credential. Otherwise the
|
||||
downstream provider falls back to a stored or environment secret (e.g. the
|
||||
resolved deployment's key, or OPENAI_API_KEY) and sends it to the
|
||||
caller-chosen address. The overridden URL is also SSRF-validated so it cannot
|
||||
point at an internal host or cloud-metadata endpoint. Proxy admins are
|
||||
unaffected.
|
||||
endpoint), every credential the resolved deployment stores must be re-supplied
|
||||
by the request (non-empty), and the request must carry a non-empty credential
|
||||
at all. Otherwise the downstream provider sends a stored credential, or falls
|
||||
back to an environment secret (e.g. OPENAI_API_KEY), to the caller-chosen
|
||||
address — including the case where the caller supplies an unrelated credential
|
||||
field to leave the stored one riding along. The overridden URL is also
|
||||
SSRF-validated so it cannot point at an internal or cloud-metadata host.
|
||||
Proxy admins are unaffected.
|
||||
"""
|
||||
from litellm.litellm_core_utils.url_utils import SSRFError, validate_url
|
||||
from litellm.proxy.common_utils.resource_ownership import is_proxy_admin
|
||||
|
|
@ -125,6 +124,14 @@ def _assert_non_admin_destination_override_is_safe(
|
|||
return
|
||||
if not any(request_litellm_params.get(field) for field in _HEALTH_URL_FIELDS):
|
||||
return
|
||||
for field in _HEALTH_CREDENTIAL_FIELDS:
|
||||
if config_litellm_params.get(field) and not request_litellm_params.get(field):
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": f"Re-provide {field} when overriding the connection target; a stored credential cannot be sent to a caller-chosen endpoint."
|
||||
},
|
||||
)
|
||||
if not any(
|
||||
request_litellm_params.get(field) for field in _HEALTH_CREDENTIAL_FIELDS
|
||||
):
|
||||
|
|
@ -1926,6 +1933,7 @@ async def test_model_connection( # noqa: PLR0915
|
|||
# A non-admin overriding the destination must bring their own credential
|
||||
# and a non-internal URL, so no stored/environment secret is sent out.
|
||||
_assert_non_admin_destination_override_is_safe(
|
||||
config_litellm_params=config_litellm_params,
|
||||
request_litellm_params=request_litellm_params,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ from litellm.proxy._types import (
|
|||
TeamModelDeleteRequest,
|
||||
UserAPIKeyAuth,
|
||||
)
|
||||
from litellm.litellm_core_utils.litellm_logging import _CUSTOM_PRICING_KEYS
|
||||
from litellm.litellm_core_utils.url_utils import SSRFError, validate_url
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.encrypt_decrypt_utils import (
|
||||
|
|
@ -75,14 +76,20 @@ _CREDENTIAL_LITELLM_PARAMS = (
|
|||
"litellm_credential_name",
|
||||
"aws_secret_access_key",
|
||||
"aws_session_token",
|
||||
"azure_ad_token",
|
||||
"vertex_credentials",
|
||||
)
|
||||
# Caller-controlled endpoint URLs that must be SSRF-validated on write.
|
||||
# Caller-controlled endpoint URLs that must be SSRF-validated on write. Mirrors
|
||||
# the endpoint-redirect subset of auth_utils._BANNED_REQUEST_BODY_PARAMS so a
|
||||
# stored config can't reach an internal host that the request-time guard blocks.
|
||||
_URL_LITELLM_PARAMS = (
|
||||
"api_base",
|
||||
"base_url",
|
||||
"aws_bedrock_runtime_endpoint",
|
||||
"aws_sts_endpoint",
|
||||
"sagemaker_base_url",
|
||||
"s3_endpoint_url",
|
||||
"deployment_url",
|
||||
)
|
||||
# Destination fields whose change must drop an inherited credential (URLs above
|
||||
# plus the provider selector, which isn't a URL so isn't SSRF-validated).
|
||||
|
|
@ -135,18 +142,31 @@ def _assert_credential_resupplied_on_destination_change(
|
|||
db_plaintext: "Callable[[str], object]",
|
||||
) -> None:
|
||||
"""Changing a model's destination (api_base/base_url/provider endpoint) must
|
||||
not send an inherited credential to the new endpoint. Clearing the credential
|
||||
is NOT sufficient: an empty/absent key falls back to the proxy's environment
|
||||
secret (e.g. OPENAI_API_KEY) and would still be sent there. So require the
|
||||
caller to re-supply, with a non-empty value, every credential the model
|
||||
already had; otherwise reject. A non-empty re-supplied value is the caller's
|
||||
own credential going to their own endpoint, which is allowed."""
|
||||
not send a credential to the new endpoint that the caller did not themselves
|
||||
provide. Two things are required. First, the patch must carry a non-empty
|
||||
credential at all: even a model stored without one falls back to the proxy's
|
||||
environment secret (e.g. OPENAI_API_KEY) at call time, which would then be
|
||||
sent to the new endpoint. Second, every credential the model already had must
|
||||
be re-supplied non-empty, so an inherited secret is never kept alongside an
|
||||
unrelated new one. A non-empty re-supplied value is the caller's own
|
||||
credential going to their own endpoint, which is allowed."""
|
||||
destination_changed = any(
|
||||
field in patch_plaintext and patch_plaintext[field] != db_plaintext(field)
|
||||
for field in _DESTINATION_LITELLM_PARAMS
|
||||
)
|
||||
if not destination_changed:
|
||||
return
|
||||
if not any(patch_plaintext.get(field) for field in _CREDENTIAL_LITELLM_PARAMS):
|
||||
raise ProxyException(
|
||||
message=(
|
||||
"Provide a non-empty credential (e.g. api_key) when changing the "
|
||||
"model endpoint; otherwise the proxy's environment credential "
|
||||
"would be used at the new destination."
|
||||
),
|
||||
type=ProxyErrorTypes.validation_error.value,
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
param="api_key",
|
||||
)
|
||||
for field in _CREDENTIAL_LITELLM_PARAMS:
|
||||
if db_plaintext(field) and not patch_plaintext.get(field):
|
||||
raise ProxyException(
|
||||
|
|
@ -162,10 +182,15 @@ def _assert_credential_resupplied_on_destination_change(
|
|||
|
||||
|
||||
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")
|
||||
"""A field that influences cost/spend accounting, so it is proxy-admin-only.
|
||||
|
||||
The authoritative boundary is litellm's own custom-pricing key set (the same
|
||||
fields the cost calculator reads from a deployment), so this cannot drift
|
||||
behind the calculator. base_model redirects the cost-lookup model, and the
|
||||
"cost" substring is a belt-and-suspenders catch for any future field that
|
||||
follows the naming convention before it lands in the type.
|
||||
"""
|
||||
return field in _CUSTOM_PRICING_KEYS or field == "base_model" or "cost" in field
|
||||
|
||||
|
||||
def _contains_env_reference(value: object) -> bool:
|
||||
|
|
|
|||
|
|
@ -1967,27 +1967,35 @@ def test_non_admin_destination_override_guard():
|
|||
non_admin = MagicMock(user_role="internal_user")
|
||||
admin = MagicMock(user_role=LitellmUserRoles.PROXY_ADMIN)
|
||||
|
||||
def guard(config, request, user):
|
||||
_assert_non_admin_destination_override_is_safe(config, request, user)
|
||||
|
||||
# Non-admin overrides api_base without supplying a credential -> rejected
|
||||
# (a stored or environment key would otherwise be sent there).
|
||||
with pytest.raises(HTTPException):
|
||||
_assert_non_admin_destination_override_is_safe(
|
||||
{"api_base": "https://attacker.example"}, non_admin
|
||||
guard({}, {"api_base": "https://attacker.example"}, non_admin)
|
||||
# Stored credential not re-supplied (caller sends an UNRELATED field) ->
|
||||
# rejected; the stored api_key must not ride along to the new endpoint.
|
||||
with pytest.raises(HTTPException):
|
||||
guard(
|
||||
{"api_key": "sk-victim", "api_base": "https://victim"},
|
||||
{"api_base": "https://attacker.example", "aws_session_token": "x"},
|
||||
non_admin,
|
||||
)
|
||||
with patch.object(litellm, "user_url_validation", False):
|
||||
# Non-admin overrides WITH their own credential -> allowed.
|
||||
_assert_non_admin_destination_override_is_safe(
|
||||
{"api_base": "https://attacker.example", "api_key": "sk-own"}, non_admin
|
||||
guard(
|
||||
{}, {"api_base": "https://attacker.example", "api_key": "sk-own"}, non_admin
|
||||
)
|
||||
# No destination override -> allowed.
|
||||
_assert_non_admin_destination_override_is_safe({"model": "gpt-4o"}, non_admin)
|
||||
guard({"api_key": "sk-victim"}, {"model": "gpt-4o"}, non_admin)
|
||||
# Proxy admin is exempt even when overriding without a credential.
|
||||
_assert_non_admin_destination_override_is_safe(
|
||||
{"api_base": "https://attacker.example"}, admin
|
||||
)
|
||||
guard({"api_key": "sk-victim"}, {"api_base": "https://attacker.example"}, admin)
|
||||
# Non-admin override to an internal/metadata IP -> SSRF-rejected.
|
||||
with patch.object(litellm, "user_url_validation", True):
|
||||
with pytest.raises(HTTPException):
|
||||
_assert_non_admin_destination_override_is_safe(
|
||||
guard(
|
||||
{},
|
||||
{
|
||||
"api_base": "http://169.254.169.254/latest/meta-data/",
|
||||
"api_key": "sk-own",
|
||||
|
|
@ -2072,3 +2080,52 @@ async def test_test_connection_model_name_path_authorizes_against_resolved_owner
|
|||
)
|
||||
|
||||
assert captured.get("team_id") == "team-OWNER"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_test_connection_rejects_partial_credential_resupply_on_override():
|
||||
"""A non-admin overriding api_base while supplying only an UNRELATED credential
|
||||
field must not let the resolved deployment's stored api_key ride along to the
|
||||
new endpoint."""
|
||||
from fastapi import HTTPException
|
||||
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
victim = Deployment(
|
||||
model_name="victim",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="openai/gpt-4o",
|
||||
api_key="sk-victim",
|
||||
api_base="https://victim.example/v1",
|
||||
),
|
||||
model_info={"id": "victim-id", "team_id": "team-OWNER"},
|
||||
)
|
||||
mock_router = MagicMock()
|
||||
mock_router.get_deployment.side_effect = lambda model_id: (
|
||||
victim if model_id == "victim-id" else None
|
||||
)
|
||||
mock_auth = AsyncMock(return_value=True)
|
||||
mock_ahealth = AsyncMock(return_value={"status": "healthy"})
|
||||
|
||||
with contextlib.ExitStack() as stack:
|
||||
for p in _health_test_connection_patches(
|
||||
MagicMock(), mock_router, mock_auth, mock_ahealth
|
||||
):
|
||||
stack.enter_context(p)
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await health_test_model_connection(
|
||||
request=MagicMock(),
|
||||
mode="chat",
|
||||
litellm_params={
|
||||
"model": "openai/gpt-4o",
|
||||
"api_base": "https://attacker.example/v1",
|
||||
"aws_session_token": "x", # unrelated; api_key NOT re-supplied
|
||||
},
|
||||
model_info={"id": "victim-id"},
|
||||
user_api_key_dict=MagicMock(
|
||||
user_id="attacker", token="t", user_role="internal_user"
|
||||
),
|
||||
)
|
||||
assert exc_info.value.status_code == 400
|
||||
|
||||
mock_ahealth.assert_not_called() # stored sk-victim never went out
|
||||
|
|
|
|||
|
|
@ -1996,10 +1996,22 @@ class TestModelMgmtAuthzHardening:
|
|||
db = {"api_base": "https://real.example.com", "api_key": "sk-stored"}
|
||||
self._assert_resupply({"api_base": "https://real.example.com"}, db)
|
||||
|
||||
def test_keyless_model_destination_change_is_allowed(self):
|
||||
# A model that never carried a credential has nothing to leak.
|
||||
def test_keyless_model_destination_change_requires_credential(self):
|
||||
from litellm.proxy._types import ProxyException
|
||||
|
||||
# Even a model stored without a credential falls back to the proxy env
|
||||
# key (e.g. OPENAI_API_KEY) at call time, so repointing it without
|
||||
# supplying a credential must be rejected.
|
||||
db = {"api_base": "https://real.example.com"}
|
||||
self._assert_resupply({"api_base": "https://attacker.example.com"}, db)
|
||||
with pytest.raises(ProxyException) as e:
|
||||
self._assert_resupply({"api_base": "https://attacker.example.com"}, db)
|
||||
assert str(e.value.code) == "400"
|
||||
|
||||
def test_keyless_model_destination_change_with_credential_is_allowed(self):
|
||||
db = {"api_base": "https://real.example.com"}
|
||||
self._assert_resupply(
|
||||
{"api_base": "https://attacker.example.com", "api_key": "sk-own"}, db
|
||||
)
|
||||
|
||||
def test_validate_model_url_params_blocks_internal_ip(self):
|
||||
import litellm
|
||||
|
|
@ -2018,6 +2030,10 @@ class TestModelMgmtAuthzHardening:
|
|||
_validate_model_url_params(
|
||||
{"aws_bedrock_runtime_endpoint": "http://169.254.169.254/"}
|
||||
)
|
||||
with pytest.raises(ProxyException):
|
||||
_validate_model_url_params(
|
||||
{"sagemaker_base_url": "http://169.254.169.254/"}
|
||||
)
|
||||
# Honors the opt-out toggle (internal endpoints / Ollama).
|
||||
with (
|
||||
patch.object(litellm, "user_url_validation", False),
|
||||
|
|
@ -2466,3 +2482,65 @@ class TestModelMgmtAuthzHardening:
|
|||
db,
|
||||
)
|
||||
assert e.value.param == "api_key"
|
||||
|
||||
# --- Veria sweep follow-ups: authoritative pricing set, base_model, extra URLs ---
|
||||
|
||||
def test_spend_affecting_fields_are_proxy_admin_only(self):
|
||||
from litellm.proxy._types import ProxyException
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
_assert_privileged_model_fields_authorized,
|
||||
_is_pricing_field,
|
||||
)
|
||||
from litellm.types.router import ModelInfo, updateLiteLLMParams
|
||||
|
||||
# Gate uses litellm's authoritative custom-pricing key set (not a subset),
|
||||
# plus base_model which redirects the cost-lookup model.
|
||||
for f in (
|
||||
"tiered_pricing",
|
||||
"cache_read_input_token_cost_flex",
|
||||
"cache_creation_input_token_cost_above_1hr",
|
||||
"base_model",
|
||||
"input_cost_per_second",
|
||||
):
|
||||
assert _is_pricing_field(f), f
|
||||
for f in ("api_key", "api_base", "model", "custom_llm_provider"):
|
||||
assert not _is_pricing_field(f), f
|
||||
|
||||
# A non-admin cannot zero a cache cost field outside the old subset...
|
||||
with pytest.raises(ProxyException) as e:
|
||||
_assert_privileged_model_fields_authorized(
|
||||
litellm_params=updateLiteLLMParams(
|
||||
cache_read_input_token_cost_flex=0.0
|
||||
),
|
||||
model_info=None,
|
||||
user_api_key_dict=self._non_admin(),
|
||||
)
|
||||
assert str(e.value.code) == "403"
|
||||
# ...nor repoint cost lookup via model_info.base_model.
|
||||
with pytest.raises(ProxyException) as e:
|
||||
_assert_privileged_model_fields_authorized(
|
||||
litellm_params=None,
|
||||
model_info=ModelInfo(base_model="gpt-3.5-turbo"),
|
||||
user_api_key_dict=self._non_admin(),
|
||||
)
|
||||
assert str(e.value.code) == "403"
|
||||
# Proxy admin may set them.
|
||||
_assert_privileged_model_fields_authorized(
|
||||
litellm_params=updateLiteLLMParams(cache_read_input_token_cost_flex=0.0),
|
||||
model_info=ModelInfo(base_model="gpt-3.5-turbo"),
|
||||
user_api_key_dict=self._admin(),
|
||||
)
|
||||
|
||||
def test_sagemaker_base_url_change_requires_credential_resupply(self):
|
||||
from litellm.proxy._types import ProxyException
|
||||
|
||||
# sagemaker_base_url retargets outbound SageMaker traffic, so changing it
|
||||
# without re-supplying the stored AWS credential must be rejected.
|
||||
db = {
|
||||
"sagemaker_base_url": "https://real.example",
|
||||
"aws_secret_access_key": "stored",
|
||||
}
|
||||
with pytest.raises(ProxyException):
|
||||
self._assert_resupply(
|
||||
{"sagemaker_base_url": "https://attacker.example"}, db
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue