mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(proxy): resolve os.environ/ refs universally in DB-sourced models
Root cause: PR #30867 removed request-time os.environ/ expansion in
BaseAWSLLM.get_credentials. That is only safe if config-load pre-resolves
os.environ/ refs so the value reaching get_credentials is already the real
secret. The YAML config path has always done this. The DB-load path
(ProxyConfig._resolve_db_litellm_param) only re-expanded keys in a hardcoded
whitelist (_DB_LITELLM_PARAM_ENV_REF_KEYS) plus short-circuited env-ref
resolution entirely for team-scoped rows. PR #32256 extended that whitelist
to 18 keys to unblock a customer whose Bedrock model with aws_role_name:
os.environ/BEDROCK_ASSUME_ROLE_ARN broke on v1.90+, but the whitelist is
structurally fragile: every future auth field breaks the same way until
someone remembers to add it
Fix: remove the whitelist and the team-scope short-circuit. The DB-load
resolver now expands os.environ/ on every string field, matching the YAML
path. Trust boundary stays on the write side: only PROXY_ADMIN can create
team_id=None rows, only team admins of a team can create rows scoped to
that team, and the request-body vector is still blocked by
_BANNED_REQUEST_BODY_PARAMS. Team-scoped rows now resolve env refs — this
is a deliberate LIT-3831 threat-model expansion trusting team admins for
env-var reads
Regression tests in tests/test_litellm/proxy/proxy_server/test_proxy_config.py:
- test_ProxyConfig__add_deployment_resolves_env_refs_after_db_decrypt pins
admin-scoped rows resolve every field (previously api_base stayed literal)
- test_ProxyConfig__add_deployment_resolves_team_env_refs pins team rows
resolve env refs (previously stayed literal)
- test_ProxyConfig__add_deployment_resolves_env_refs_on_arbitrary_field pins
the no-whitelist invariant against a made-up field name
- test_ProxyConfig__add_deployment_resolves_env_refs_for_aws_bedrock_auth_params
(from #32256) still passes
- Path B counterparts (decrypt_model_list_from_db) mirror the above
Left as followups (not fixed here):
- /model/info and /v2/model/info still echo resolved values for fields not
in the current pop-list (aws_role_name, aws_sts_endpoint, api_base, etc.).
Fix is to extend remove_sensitive_info_from_deployment; separate PR
- Master-key rotation reads DB rows via decrypt_model_list_from_db which
now resolves universally, so rotation collapses env-refs into hardcoded
values. Pre-existing bug for the 6 previously-whitelisted fields; wider
surface after this PR. Separate PR
(cherry picked from commit 5862be3e79)
This commit is contained in:
yucheng-berri
Yuneng Jiang
parent
f270281652
commit
485d5c1025
2 changed files with 218 additions and 10 deletions
|
|
@ -4958,6 +4958,19 @@ class ProxyConfig:
|
|||
deleted_deployments += 1
|
||||
return deleted_deployments
|
||||
|
||||
def _resolve_db_litellm_param(self, key: str, value: object) -> object:
|
||||
if not isinstance(value, str):
|
||||
return value
|
||||
|
||||
decrypted_value = decrypt_value_helper(
|
||||
value=value, key=key, return_original_value=True
|
||||
)
|
||||
if isinstance(decrypted_value, str) and decrypted_value.startswith(
|
||||
"os.environ/"
|
||||
):
|
||||
return get_secret(decrypted_value)
|
||||
return decrypted_value
|
||||
|
||||
def _add_deployment(self, db_models: list) -> int:
|
||||
"""
|
||||
Iterate through db models
|
||||
|
|
@ -4978,12 +4991,7 @@ class ProxyConfig:
|
|||
if isinstance(_litellm_params, dict):
|
||||
# decrypt values
|
||||
for k, v in _litellm_params.items():
|
||||
if isinstance(v, str):
|
||||
# decrypt value - returns original value if decryption fails or no key is set
|
||||
_value = decrypt_value_helper(
|
||||
value=v, key=k, return_original_value=True
|
||||
)
|
||||
_litellm_params[k] = _value
|
||||
_litellm_params[k] = self._resolve_db_litellm_param(key=k, value=v)
|
||||
_litellm_params = LiteLLM_Params(**_litellm_params)
|
||||
|
||||
else:
|
||||
|
|
@ -5016,10 +5024,7 @@ class ProxyConfig:
|
|||
if isinstance(_litellm_params, dict):
|
||||
# decrypt values
|
||||
for k, v in _litellm_params.items():
|
||||
decrypted_value = decrypt_value_helper(
|
||||
value=v, key=k, return_original_value=True
|
||||
)
|
||||
_litellm_params[k] = decrypted_value
|
||||
_litellm_params[k] = self._resolve_db_litellm_param(key=k, value=v)
|
||||
_litellm_params = LiteLLM_Params(**_litellm_params)
|
||||
else:
|
||||
verbose_proxy_logger.error(
|
||||
|
|
|
|||
|
|
@ -795,6 +795,172 @@ def test_ProxyConfig__add_deployment_invalid_litellm_params_skips(monkeypatch):
|
|||
assert pc._add_deployment(db_models=[bad]) == 0
|
||||
|
||||
|
||||
def test_ProxyConfig__add_deployment_resolves_env_refs_after_db_decrypt(monkeypatch):
|
||||
"""Every ``os.environ/`` value on an admin-scoped DB row resolves at
|
||||
load time, regardless of the field name. Replaces the earlier
|
||||
behavior where only fields in ``_DB_LITELLM_PARAM_ENV_REF_KEYS``
|
||||
resolved: the whitelist has been removed so the resolver applies to
|
||||
every string field."""
|
||||
monkeypatch.setenv("LITELLM_DB_MODEL_API_KEY", "resolved-secret")
|
||||
monkeypatch.setenv("LITELLM_MASTER_KEY", "master-secret")
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.decrypt_value_helper",
|
||||
lambda value, key, return_original_value: value,
|
||||
)
|
||||
fake_router = MagicMock()
|
||||
fake_router.upsert_deployment = MagicMock(return_value=True)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", fake_router)
|
||||
pc = ProxyConfig()
|
||||
db_model = SimpleNamespace(
|
||||
model_id="model-1",
|
||||
model_name="env-model",
|
||||
model_info={"id": "model-1"},
|
||||
litellm_params={
|
||||
"model": "openai/gpt-4o-mini",
|
||||
"api_key": "os.environ/LITELLM_DB_MODEL_API_KEY",
|
||||
"api_base": "os.environ/LITELLM_MASTER_KEY",
|
||||
},
|
||||
blocked=False,
|
||||
)
|
||||
|
||||
added = pc._add_deployment(db_models=[db_model])
|
||||
deployment = fake_router.upsert_deployment.call_args.kwargs["deployment"]
|
||||
|
||||
assert added == 1
|
||||
assert deployment.litellm_params.api_key == "resolved-secret"
|
||||
assert deployment.litellm_params.api_base == "master-secret"
|
||||
|
||||
|
||||
def test_ProxyConfig__add_deployment_resolves_team_env_refs(monkeypatch):
|
||||
"""Team-scoped DB rows now resolve ``os.environ/`` refs the same way
|
||||
admin rows do. The prior team-scoped short-circuit and the
|
||||
field-by-field whitelist have both been removed; the write-side team
|
||||
auth check in ``ModelManagementAuthChecks.can_user_make_model_call``
|
||||
remains the single trust boundary. A literal (non-``os.environ/``)
|
||||
value still passes through unchanged."""
|
||||
monkeypatch.setenv("LITELLM_MASTER_KEY", "master-secret")
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.decrypt_value_helper",
|
||||
lambda value, key, return_original_value: value,
|
||||
)
|
||||
fake_router = MagicMock()
|
||||
fake_router.upsert_deployment = MagicMock(return_value=True)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", fake_router)
|
||||
pc = ProxyConfig()
|
||||
db_model = SimpleNamespace(
|
||||
model_id="model-1",
|
||||
model_name="model_name_team-1_abc",
|
||||
model_info={"id": "model-1", "team_id": "team-1"},
|
||||
litellm_params={
|
||||
"model": "openai/gpt-4o-mini",
|
||||
"api_key": "os.environ/LITELLM_MASTER_KEY",
|
||||
"api_base": "https://team.example",
|
||||
},
|
||||
blocked=False,
|
||||
)
|
||||
|
||||
added = pc._add_deployment(db_models=[db_model])
|
||||
deployment = fake_router.upsert_deployment.call_args.kwargs["deployment"]
|
||||
|
||||
assert added == 1
|
||||
assert deployment.litellm_params.api_key == "master-secret"
|
||||
assert deployment.litellm_params.api_base == "https://team.example"
|
||||
|
||||
|
||||
def test_ProxyConfig__add_deployment_resolves_env_refs_for_aws_bedrock_auth_params(
|
||||
monkeypatch,
|
||||
):
|
||||
"""Regression: DB-stored Bedrock/SageMaker auth params like
|
||||
``aws_role_name: os.environ/BEDROCK_ASSUME_ROLE_ARN`` must resolve at
|
||||
DB-load time. PR #30867 removed request-time expansion in
|
||||
``BaseAWSLLM.get_credentials``; without DB-load resolution the literal
|
||||
string reaches STS and fails with ``ValidationError: ... is invalid``."""
|
||||
aws_env = {
|
||||
"aws_session_token": ("BEDROCK_SESSION_TOKEN", "resolved-session-token"),
|
||||
"aws_region_name": ("BEDROCK_REGION", "us-east-1"),
|
||||
"aws_session_name": ("BEDROCK_SESSION_NAME", "resolved-session"),
|
||||
"aws_profile_name": ("BEDROCK_PROFILE", "resolved-profile"),
|
||||
"aws_role_name": (
|
||||
"BEDROCK_ASSUME_ROLE_ARN",
|
||||
"arn:aws:iam::123456789012:role/resolved",
|
||||
),
|
||||
"aws_web_identity_token": ("BEDROCK_WEB_IDENTITY_TOKEN", "resolved-token"),
|
||||
"aws_sts_endpoint": (
|
||||
"BEDROCK_STS_ENDPOINT",
|
||||
"https://sts.us-east-1.amazonaws.com",
|
||||
),
|
||||
"aws_external_id": ("BEDROCK_EXTERNAL_ID", "resolved-external-id"),
|
||||
"aws_bedrock_runtime_endpoint": (
|
||||
"BEDROCK_RUNTIME_ENDPOINT",
|
||||
"https://bedrock-runtime.us-east-1.amazonaws.com",
|
||||
),
|
||||
"aws_bedrock_project_id": ("BEDROCK_PROJECT_ID", "resolved-project-id"),
|
||||
"aws_batch_role_arn": (
|
||||
"BEDROCK_BATCH_ROLE_ARN",
|
||||
"arn:aws:iam::123456789012:role/batch",
|
||||
),
|
||||
"aws_workspace_id": ("BEDROCK_WORKSPACE_ID", "resolved-workspace-id"),
|
||||
}
|
||||
for _, (env_name, env_value) in aws_env.items():
|
||||
monkeypatch.setenv(env_name, env_value)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.decrypt_value_helper",
|
||||
lambda value, key, return_original_value: value,
|
||||
)
|
||||
fake_router = MagicMock()
|
||||
fake_router.upsert_deployment = MagicMock(return_value=True)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", fake_router)
|
||||
pc = ProxyConfig()
|
||||
litellm_params: Dict[str, Any] = {"model": "bedrock/anthropic.claude-v2"}
|
||||
for key, (env_name, _) in aws_env.items():
|
||||
litellm_params[key] = f"os.environ/{env_name}"
|
||||
db_model = SimpleNamespace(
|
||||
model_id="model-1",
|
||||
model_name="bedrock-model",
|
||||
model_info={"id": "model-1"},
|
||||
litellm_params=litellm_params,
|
||||
blocked=False,
|
||||
)
|
||||
|
||||
added = pc._add_deployment(db_models=[db_model])
|
||||
deployment = fake_router.upsert_deployment.call_args.kwargs["deployment"]
|
||||
|
||||
assert added == 1
|
||||
for key, (_, expected) in aws_env.items():
|
||||
assert getattr(deployment.litellm_params, key) == expected, key
|
||||
|
||||
|
||||
def test_ProxyConfig__add_deployment_resolves_env_refs_on_arbitrary_field(monkeypatch):
|
||||
"""A made-up field name that was never on the removed whitelist still
|
||||
resolves ``os.environ/`` refs. Pins the "no whitelist" invariant:
|
||||
the resolver applies to every string field, not a curated list."""
|
||||
monkeypatch.setenv("SOME_CUSTOM_ENV", "resolved-custom-value")
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.decrypt_value_helper",
|
||||
lambda value, key, return_original_value: value,
|
||||
)
|
||||
fake_router = MagicMock()
|
||||
fake_router.upsert_deployment = MagicMock(return_value=True)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", fake_router)
|
||||
pc = ProxyConfig()
|
||||
db_model = SimpleNamespace(
|
||||
model_id="model-1",
|
||||
model_name="custom-field-model",
|
||||
model_info={"id": "model-1"},
|
||||
litellm_params={
|
||||
"model": "openai/gpt-4o-mini",
|
||||
"some_future_field": "os.environ/SOME_CUSTOM_ENV",
|
||||
},
|
||||
blocked=False,
|
||||
)
|
||||
|
||||
added = pc._add_deployment(db_models=[db_model])
|
||||
deployment = fake_router.upsert_deployment.call_args.kwargs["deployment"]
|
||||
|
||||
assert added == 1
|
||||
assert deployment.litellm_params.some_future_field == "resolved-custom-value"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig.decrypt_model_list_from_db
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -837,6 +1003,43 @@ def test_ProxyConfig_decrypt_model_list_from_db_invalid_params_skips():
|
|||
assert out == []
|
||||
|
||||
|
||||
def test_ProxyConfig_decrypt_model_list_from_db_resolves_env_refs_after_db_decrypt(
|
||||
monkeypatch,
|
||||
):
|
||||
"""Path B (feeding /v2/model/info fallback and /model/info fallback)
|
||||
resolves every ``os.environ/`` field on admin-scoped rows, mirroring
|
||||
path A. Both paths now share the same universal-resolution shape."""
|
||||
monkeypatch.setenv("LITELLM_DB_MODEL_API_KEY", "resolved-secret")
|
||||
monkeypatch.setenv("LITELLM_MASTER_KEY", "master-secret")
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.decrypt_value_helper",
|
||||
lambda value, key, return_original_value: (
|
||||
"os.environ/LITELLM_DB_MODEL_API_KEY"
|
||||
if key == "api_key"
|
||||
else "os.environ/LITELLM_MASTER_KEY"
|
||||
if key == "api_base"
|
||||
else value
|
||||
),
|
||||
)
|
||||
pc = ProxyConfig()
|
||||
m = SimpleNamespace(
|
||||
model_id="model-1",
|
||||
model_name="env-model",
|
||||
model_info={"id": "model-1"},
|
||||
litellm_params={
|
||||
"api_key": "encrypted-env-ref",
|
||||
"api_base": "encrypted-api-base-env-ref",
|
||||
"model": "openai/gpt-4o-mini",
|
||||
},
|
||||
blocked=False,
|
||||
)
|
||||
|
||||
out = pc.decrypt_model_list_from_db(new_models=[m])
|
||||
|
||||
assert out[0]["litellm_params"]["api_key"] == "resolved-secret"
|
||||
assert out[0]["litellm_params"]["api_base"] == "master-secret"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig._update_llm_router
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue