mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-10 22:41:41 +00:00
fix(cache): mask inline url credentials and drop discrete username under url precedence
Two follow-ups from review of the url/db work. A Redis/Valkey url can embed a password (redis://:secret@host:6379/1), but _CACHE_SENSITIVE_FIELDS only masked the discrete password and sentinel_password, so a stored password-bearing url came back in plaintext from every GET /cache/settings. Add url to the masked set so it gets the same masked-on-read treatment as password. The url-precedence resolver dropped host/port/db/password but not username, even though a url can encode a username too (redis://user:pass@host). Left in, the discrete username rode along and could contradict the url. Add username to the overridden set and update the Redis URL help text to list it among the fields url takes precedence over. Tests: GET masks a password-bearing url (secret never returned verbatim) while a non-credential field is untouched, and the resolver drops a discrete username when a url is present.
This commit is contained in:
parent
611b8dee18
commit
b86c01c624
3 changed files with 55 additions and 11 deletions
|
|
@ -38,24 +38,27 @@ from litellm.types.management_endpoints import (
|
|||
router = APIRouter()
|
||||
|
||||
# Cache fields holding credentials. Masked on read so plaintext Redis /
|
||||
# Sentinel passwords never leave the server in a GET response.
|
||||
_CACHE_SENSITIVE_FIELDS: set = {"password", "sentinel_password"}
|
||||
# Sentinel passwords never leave the server in a GET response. `url` is here
|
||||
# because a Redis/Valkey URL can embed a password inline
|
||||
# (e.g. redis://:secret@host:6379/1).
|
||||
_CACHE_SENSITIVE_FIELDS: set = {"password", "sentinel_password", "url"}
|
||||
|
||||
|
||||
_REDACTED_VALUE = "***REDACTED***"
|
||||
|
||||
|
||||
_URL_OVERRIDDEN_CONNECTION_FIELDS: frozenset = frozenset({"host", "port", "db", "password"})
|
||||
_URL_OVERRIDDEN_CONNECTION_FIELDS: frozenset = frozenset({"host", "port", "db", "password", "username"})
|
||||
|
||||
|
||||
def _resolve_cache_url_precedence(settings: Mapping[str, Any]) -> Dict[str, Any]:
|
||||
"""Return cache settings with the url-vs-discrete-fields ambiguity resolved.
|
||||
|
||||
When a full ``url`` is supplied it wins: the discrete host/port/db/password
|
||||
fields are dropped so the persisted config is unambiguous and matches
|
||||
runtime resolution in ``litellm._redis`` (``redis.Redis.from_url`` ignores
|
||||
them). Cluster mode (``redis_startup_nodes``) is exempt because it
|
||||
authenticates via the discrete fields rather than a url.
|
||||
When a full ``url`` is supplied it wins: the discrete
|
||||
host/port/db/password/username fields are dropped so the persisted config
|
||||
is unambiguous and matches runtime resolution in ``litellm._redis``
|
||||
(``redis.Redis.from_url`` ignores them). Cluster mode
|
||||
(``redis_startup_nodes``) is exempt because it authenticates via the
|
||||
discrete fields rather than a url.
|
||||
"""
|
||||
url = settings.get("url")
|
||||
has_url = isinstance(url, str) and url.strip() != ""
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ CACHE_SETTINGS_FIELDS: List[CacheSettingsField] = [
|
|||
field_name="url",
|
||||
field_type="String",
|
||||
field_value=None,
|
||||
field_description="Full Redis/Valkey connection URL (e.g. redis://:password@host:6379/1). When set, it takes precedence over Host, Port, Password, and Database Index.",
|
||||
field_description="Full Redis/Valkey connection URL (e.g. redis://:password@host:6379/1). When set, it takes precedence over Host, Port, Username, Password, and Database Index.",
|
||||
field_default=None,
|
||||
ui_field_name="Redis URL",
|
||||
redis_type=None,
|
||||
|
|
|
|||
|
|
@ -16,10 +16,12 @@ import litellm
|
|||
from litellm.proxy._types import LitellmTableNames, LitellmUserRoles
|
||||
from litellm.proxy.auth.user_api_key_auth import UserAPIKeyAuth
|
||||
from litellm.proxy.management_endpoints.cache_settings_endpoints import (
|
||||
_CACHE_SENSITIVE_FIELDS,
|
||||
CacheSettingsManager,
|
||||
CacheSettingsUpdateRequest,
|
||||
CacheTestRequest,
|
||||
_resolve_cache_url_precedence,
|
||||
get_cache_settings,
|
||||
test_cache_connection,
|
||||
update_cache_settings,
|
||||
)
|
||||
|
|
@ -95,10 +97,11 @@ class TestResolveCacheUrlPrecedence:
|
|||
def test_url_overrides_discrete_connection_fields(self):
|
||||
settings = {
|
||||
"type": "redis",
|
||||
"url": "redis://:pw@host:6379/1",
|
||||
"url": "redis://user:pw@host:6379/1",
|
||||
"host": "host",
|
||||
"port": "6379",
|
||||
"db": 1,
|
||||
"username": "user",
|
||||
"password": "pw",
|
||||
"namespace": "ns",
|
||||
"ttl": 60,
|
||||
|
|
@ -106,10 +109,13 @@ class TestResolveCacheUrlPrecedence:
|
|||
|
||||
result = _resolve_cache_url_precedence(settings)
|
||||
|
||||
assert result["url"] == "redis://:pw@host:6379/1"
|
||||
assert result["url"] == "redis://user:pw@host:6379/1"
|
||||
assert "host" not in result
|
||||
assert "port" not in result
|
||||
assert "db" not in result
|
||||
# username and password are both encodable in the url, so the discrete
|
||||
# copies must not ride along and override it
|
||||
assert "username" not in result
|
||||
assert "password" not in result
|
||||
# Non-connection fields survive
|
||||
assert result["type"] == "redis"
|
||||
|
|
@ -234,6 +240,41 @@ async def test_update_cache_settings_persists_url_precedence(monkeypatch):
|
|||
assert init_params["url"] == "redis://:pw@host:6379/1"
|
||||
|
||||
|
||||
def test_url_is_a_masked_field():
|
||||
"""A Redis URL can carry an inline password, so it must be masked on read
|
||||
alongside the discrete password fields."""
|
||||
assert "url" in _CACHE_SENSITIVE_FIELDS
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_cache_settings_masks_password_bearing_url():
|
||||
"""GET /cache/settings must not leak an inline url password in plaintext,
|
||||
while non-credential fields (e.g. namespace) come back untouched."""
|
||||
stored_url = "redis://:supersecretpassword@host:6379/1"
|
||||
stored_settings = {"type": "redis", "url": stored_url, "namespace": "ns"}
|
||||
|
||||
cache_row = MagicMock()
|
||||
cache_row.cache_settings = json.dumps(stored_settings)
|
||||
|
||||
mock_prisma = MagicMock()
|
||||
mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=cache_row)
|
||||
|
||||
proxy_config = MagicMock()
|
||||
proxy_config._decrypt_db_variables = MagicMock(side_effect=lambda variables_dict: dict(variables_dict))
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.prisma_client", mock_prisma),
|
||||
patch("litellm.proxy.proxy_server.proxy_config", proxy_config),
|
||||
):
|
||||
response = await get_cache_settings(user_api_key_dict=_admin_auth())
|
||||
|
||||
returned_url = response.current_values["url"]
|
||||
assert returned_url != stored_url
|
||||
assert "supersecretpassword" not in returned_url
|
||||
# non-credential field is not masked
|
||||
assert response.current_values["namespace"] == "ns"
|
||||
|
||||
|
||||
class TestCacheSettingsManager:
|
||||
"""Tests for CacheSettingsManager class"""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue