From b86c01c624ef4d1146d74095c30abe6d47768e37 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 3 Jul 2026 13:55:45 -0700 Subject: [PATCH] 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. --- .../cache_settings_endpoints.py | 19 ++++---- .../cache_settings_endpoints.py | 2 +- .../test_cache_settings_endpoints.py | 45 ++++++++++++++++++- 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/litellm/proxy/management_endpoints/cache_settings_endpoints.py b/litellm/proxy/management_endpoints/cache_settings_endpoints.py index f3903e9d003..97697cc7518 100644 --- a/litellm/proxy/management_endpoints/cache_settings_endpoints.py +++ b/litellm/proxy/management_endpoints/cache_settings_endpoints.py @@ -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() != "" diff --git a/litellm/types/management_endpoints/cache_settings_endpoints.py b/litellm/types/management_endpoints/cache_settings_endpoints.py index 7bb26245a1e..32ae70ac92e 100644 --- a/litellm/types/management_endpoints/cache_settings_endpoints.py +++ b/litellm/types/management_endpoints/cache_settings_endpoints.py @@ -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, diff --git a/tests/test_litellm/proxy/management_endpoints/test_cache_settings_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_cache_settings_endpoints.py index a2e461f0b42..f4c6d4f8d15 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_cache_settings_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_cache_settings_endpoints.py @@ -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"""