mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(security): guard against double-encryption on key update
This commit is contained in:
parent
47d90255ba
commit
74a7b9c9a2
4 changed files with 114 additions and 15 deletions
|
|
@ -1,6 +1,11 @@
|
|||
import copy
|
||||
from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Literal, Optional
|
||||
|
||||
from litellm.proxy.common_utils.encrypt_decrypt_utils import (
|
||||
decrypt_value_helper,
|
||||
encrypt_value_helper,
|
||||
)
|
||||
|
||||
import litellm
|
||||
from litellm import get_secret
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
|
@ -546,8 +551,6 @@ def encrypt_logging_callback_vars(metadata: Optional[Dict]) -> Optional[Dict]:
|
|||
if not isinstance(logging_configs, list):
|
||||
return metadata
|
||||
|
||||
from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper
|
||||
|
||||
for entry in logging_configs:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
|
|
@ -568,11 +571,14 @@ def encrypt_logging_callback_vars(metadata: Optional[Dict]) -> Optional[Dict]:
|
|||
def redact_sensitive_logging_metadata(metadata: Optional[Dict]) -> Optional[Dict]:
|
||||
"""
|
||||
Return a copy of `metadata` with credential values inside
|
||||
`metadata["logging"][*]["callback_vars"]` replaced by "***".
|
||||
`metadata["logging"][*]["callback_vars"]` partially masked.
|
||||
|
||||
Values that are just environment-variable references
|
||||
(e.g. "os.environ/LANGFUSE_SECRET_KEY") are left as-is because they
|
||||
don't expose the actual secret — they're just pointers.
|
||||
Each value is replaced with "...XYZ" where XYZ is the last 3 characters
|
||||
of the plaintext (decrypting first if the value is encrypted), giving
|
||||
admins a visual hint without exposing the full secret.
|
||||
|
||||
Values that are environment-variable references
|
||||
(e.g. "os.environ/LANGFUSE_SECRET_KEY") are left as-is.
|
||||
"""
|
||||
if not metadata:
|
||||
return metadata
|
||||
|
|
@ -590,9 +596,23 @@ def redact_sensitive_logging_metadata(metadata: Optional[Dict]) -> Optional[Dict
|
|||
if not isinstance(callback_vars, dict):
|
||||
continue
|
||||
for key, value in callback_vars.items():
|
||||
# Keep env-var pointers; scrub anything that looks like a real secret
|
||||
if isinstance(value, str) and value.startswith("os.environ/"):
|
||||
if not isinstance(value, str):
|
||||
callback_vars[key] = "***"
|
||||
continue
|
||||
callback_vars[key] = "***"
|
||||
# Keep env-var pointers as-is
|
||||
if value.startswith("os.environ/"):
|
||||
continue
|
||||
# Decrypt to get plaintext (no-op for already-plaintext rows)
|
||||
plaintext = str(
|
||||
decrypt_value_helper(
|
||||
value=value,
|
||||
key=key,
|
||||
exception_type="debug",
|
||||
return_original_value=True,
|
||||
)
|
||||
or value
|
||||
)
|
||||
suffix = plaintext[-3:] if len(plaintext) >= 3 else plaintext
|
||||
callback_vars[key] = f"...{suffix}"
|
||||
|
||||
return metadata
|
||||
|
|
|
|||
|
|
@ -1516,6 +1516,7 @@ def prepare_metadata_fields(
|
|||
"""
|
||||
Check LiteLLM_ManagementEndpoint_MetadataFields (proxy/_types.py) for fields that are allowed to be updated
|
||||
"""
|
||||
metadata_provided_in_update = "metadata" in non_default_values
|
||||
if "metadata" not in non_default_values: # allow user to set metadata to none
|
||||
non_default_values["metadata"] = existing_metadata.copy()
|
||||
|
||||
|
|
@ -1544,7 +1545,8 @@ def prepare_metadata_fields(
|
|||
)
|
||||
|
||||
non_default_values["metadata"] = casted_metadata
|
||||
encrypt_logging_callback_vars(non_default_values["metadata"])
|
||||
if metadata_provided_in_update and "logging" in casted_metadata:
|
||||
encrypt_logging_callback_vars(non_default_values["metadata"])
|
||||
return non_default_values
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ def test_normalize_callback_names_lowercases_strings():
|
|||
|
||||
|
||||
def test_redact_scrubs_real_secret_values():
|
||||
"""Real credential values must be replaced with '***'."""
|
||||
"""Real credential values must be partially masked showing last 3 chars."""
|
||||
metadata = {
|
||||
"logging": [
|
||||
{
|
||||
|
|
@ -110,9 +110,9 @@ def test_redact_scrubs_real_secret_values():
|
|||
}
|
||||
result = redact_sensitive_logging_metadata(metadata)
|
||||
vars_ = result["logging"][0]["callback_vars"]
|
||||
assert vars_["langfuse_public_key"] == "***"
|
||||
assert vars_["langfuse_secret_key"] == "***"
|
||||
assert vars_["langfuse_host"] == "***"
|
||||
assert vars_["langfuse_public_key"] == "...123"
|
||||
assert vars_["langfuse_secret_key"] == "...ret"
|
||||
assert vars_["langfuse_host"] == "...com"
|
||||
|
||||
|
||||
def test_redact_keeps_env_var_references():
|
||||
|
|
@ -178,7 +178,7 @@ def test_redact_mixed_env_and_real_values():
|
|||
result = redact_sensitive_logging_metadata(metadata)
|
||||
vars_ = result["logging"][0]["callback_vars"]
|
||||
assert vars_["langfuse_public_key"] == "os.environ/LANGFUSE_PUBLIC_KEY"
|
||||
assert vars_["langfuse_secret_key"] == "***"
|
||||
assert vars_["langfuse_secret_key"] == "...ret"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -8922,3 +8922,80 @@ async def test_execute_virtual_key_regeneration_cache_invalidation_with_token_ha
|
|||
call_kwargs = mock_delete_cache.call_args.kwargs
|
||||
# The token hash should be passed as-is, NOT double-hashed
|
||||
assert call_kwargs["hashed_token"] == token_hash
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# prepare_metadata_fields double-encryption guard tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_prepare_metadata_fields_no_double_encrypt_when_metadata_not_in_update(
|
||||
monkeypatch,
|
||||
):
|
||||
"""
|
||||
When metadata is NOT in the update request, existing_metadata (which may
|
||||
already have encrypted callback_vars) must be copied as-is.
|
||||
encrypt_logging_callback_vars must NOT be called.
|
||||
"""
|
||||
monkeypatch.setenv("LITELLM_SALT_KEY", "test-salt-key-1234567890123456")
|
||||
from litellm.proxy.management_endpoints.key_management_endpoints import (
|
||||
prepare_metadata_fields,
|
||||
)
|
||||
|
||||
already_encrypted = "someencryptedblob=="
|
||||
existing_metadata = {
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langfuse",
|
||||
"callback_vars": {"langfuse_secret_key": already_encrypted},
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
data = UpdateKeyRequest(key="sk-test")
|
||||
non_default_values: dict = {} # metadata NOT provided in update
|
||||
|
||||
result = prepare_metadata_fields(
|
||||
data=data,
|
||||
non_default_values=non_default_values,
|
||||
existing_metadata=existing_metadata,
|
||||
)
|
||||
|
||||
# Value must be unchanged — no second encryption pass
|
||||
assert (
|
||||
result["metadata"]["logging"][0]["callback_vars"]["langfuse_secret_key"]
|
||||
== already_encrypted
|
||||
)
|
||||
|
||||
|
||||
def test_prepare_metadata_fields_encrypts_when_metadata_in_update(monkeypatch):
|
||||
"""
|
||||
When metadata IS in the update request with fresh callback_vars, they must
|
||||
be encrypted.
|
||||
"""
|
||||
monkeypatch.setenv("LITELLM_SALT_KEY", "test-salt-key-1234567890123456")
|
||||
from litellm.proxy.management_endpoints.key_management_endpoints import (
|
||||
prepare_metadata_fields,
|
||||
)
|
||||
|
||||
plaintext = "sk-lf-supersecret"
|
||||
fresh_metadata = {
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langfuse",
|
||||
"callback_vars": {"langfuse_secret_key": plaintext},
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
data = UpdateKeyRequest(key="sk-test", metadata=fresh_metadata)
|
||||
non_default_values: dict = {"metadata": fresh_metadata} # metadata WAS provided
|
||||
|
||||
result = prepare_metadata_fields(
|
||||
data=data,
|
||||
non_default_values=non_default_values,
|
||||
existing_metadata={},
|
||||
)
|
||||
|
||||
encrypted = result["metadata"]["logging"][0]["callback_vars"]["langfuse_secret_key"]
|
||||
assert encrypted != plaintext, "plaintext credential must be encrypted on write"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue