diff --git a/litellm/proxy/management_endpoints/cache_settings_endpoints.py b/litellm/proxy/management_endpoints/cache_settings_endpoints.py index 9f45cb619aa..7c0d8958a28 100644 --- a/litellm/proxy/management_endpoints/cache_settings_endpoints.py +++ b/litellm/proxy/management_endpoints/cache_settings_endpoints.py @@ -18,8 +18,9 @@ from pydantic import BaseModel, Field import litellm from litellm._logging import verbose_proxy_logger +from litellm._redis import _redis_kwargs_from_environment from litellm._uuid import uuid -from litellm.litellm_core_utils.sensitive_data_masker import mask_sensitive_keys +from litellm.litellm_core_utils.sensitive_data_masker import SensitiveDataMasker from litellm.proxy._types import ( AUDIT_ACTIONS, LiteLLM_AuditLogs, @@ -43,6 +44,17 @@ router = APIRouter() # (e.g. redis://:secret@host:6379/1). _CACHE_SENSITIVE_FIELDS: set = {"password", "sentinel_password", "url"} +# The env fallback resolves the full set of redis.Redis kwargs, which includes +# credential-bearing params (azure_client_secret, ssl_password, ...) that are +# not cache UI fields. Only overlay fields the settings page actually renders, +# so the read never surfaces a credential the UI does not manage. +_CACHE_SETTINGS_FIELD_NAMES: frozenset = frozenset(field.field_name for field in CACHE_SETTINGS_FIELDS) + +# Classifier used, alongside _CACHE_SENSITIVE_FIELDS, to redact any +# credential-bearing key before it leaves the server (`url` is kept in the +# explicit set because its name carries no sensitive segment). +_CREDENTIAL_CLASSIFIER = SensitiveDataMasker() + _REDACTED_VALUE = "***REDACTED***" @@ -67,6 +79,165 @@ def _resolve_cache_url_precedence(settings: Mapping[str, Any]) -> dict[str, Any] return {k: v for k, v in settings.items() if k not in _URL_OVERRIDDEN_CONNECTION_FIELDS} +def _parse_stored_settings(cache_settings_value: object) -> dict[str, Any]: + """Normalize a stored cache_settings blob to a dict. + + The prisma column comes back as either a JSON string or an already-parsed + dict depending on the client, so callers that json.loads unconditionally + silently drop the whole (still-encrypted) row on the dict path. + """ + parsed = json.loads(cache_settings_value) if isinstance(cache_settings_value, str) else cache_settings_value + return parsed if isinstance(parsed, dict) else {} + + +def _overlay_environment(stored: Mapping[str, Any]) -> dict[str, Any]: + """Fill connection fields from the REDIS_* environment the cache actually reads. + + A response cache pointed at Redis resolves host/port/password/etc. from the + REDIS_* env vars when the stored config leaves them unset, so a cache + configured purely through the environment works while its settings page, + which reads only the database row, shows blank. Overlaying the same env + kwargs the runtime uses makes the page reflect the effective connection. + Stored values win; the environment only fills what the stored config omits. + """ + env_kwargs = { + key: value for key, value in _redis_kwargs_from_environment().items() if key in _CACHE_SETTINGS_FIELD_NAMES + } + if not env_kwargs: + return dict(stored) + effective = {**env_kwargs, **stored} + # the env fallback is a Redis connection, so name the type when the stored + # config did not, letting the UI render the Redis fields it just populated + effective.setdefault("type", "redis") + return effective + + +def _redact_credentials(settings: Mapping[str, Any]) -> dict[str, Any]: + """Replace credential-bearing values with a fixed marker, keeping the rest. + + The marker is unambiguous on the way back in: an admin who edits an + unrelated field and re-submits sends the marker for the untouched secret, + which the update path maps back to the stored value rather than persisting + the marker over a working password. + """ + return { + key: (_REDACTED_VALUE if value is not None and _is_credential_field(key) else value) + for key, value in settings.items() + } + + +def _is_credential_field(key: str) -> bool: + """Whether a cache setting carries a credential and must be redacted on read.""" + return key in _CACHE_SENSITIVE_FIELDS or _CREDENTIAL_CLASSIFIER.is_sensitive_key(key) + + +def _has_connection_target(value: object) -> bool: + """Whether a payload value names a live discrete connection target.""" + if isinstance(value, str): + return value.strip() != "" and value != _REDACTED_VALUE + return value not in (None, [], {}) + + +# Every field that identifies which Redis a credential belongs to, across node +# (host/port/url), cluster (redis_startup_nodes), and sentinel +# (sentinel_nodes/service_name) modes. A stored secret is bound to these. +_CONNECTION_TARGET_FIELDS: tuple = ( + "host", + "port", + "url", + "redis_startup_nodes", + "sentinel_nodes", + "service_name", +) + + +def _target_repr(value: object) -> str: + """Canonical string form of a connection-target value for equality checks. + + The client may serialize the same target differently from storage (a port as + "6379" vs 6379, node lists round-tripped through JSON), so compare normalized + forms rather than raw values to avoid treating an unchanged target as a change. + """ + if isinstance(value, (list, dict)): + return json.dumps(value, sort_keys=True, default=str) + return str(value) + + +def _saved_secret_is_reusable(incoming: Mapping[str, Any], saved: Mapping[str, Any]) -> bool: + """Whether a stored credential may be restored for this request. + + A stored secret belongs to the stored connection target, so it is reused only + when the request describes that same target on every dimension the stored + config pins (host/port, url, cluster nodes, sentinel nodes/service). This + prevents credential replay: a caller cannot omit the credential, point at a + different (or incomplete) target, and have the proxy send the stored secret + to a Redis of their choosing. + + Non-secret target fields (host/port/nodes/service) must be supplied and match + in normalized form, so equivalent representations (port "6379" vs 6379) are + not seen as a change while an omitted or different value is. ``url`` is the + exception: it is itself the secret and the form never re-prefills it, so a + redacted or omitted url means "keep the stored url" (same target) and only a + different supplied url blocks reuse. + """ + for field in _CONNECTION_TARGET_FIELDS: + saved_value = saved.get(field) + if saved_value in (None, "", [], {}): + continue # the stored config does not pin this dimension + incoming_value = incoming.get(field) + if field == "url": + if incoming_value in (None, "", _REDACTED_VALUE): + continue # url kept as-is (same target) + if _target_repr(incoming_value) != _target_repr(saved_value): + return False + continue + if _target_repr(incoming_value) != _target_repr(saved_value): + return False # a pinned target field is missing or different + return True + + +def _merge_over_saved(incoming: Mapping[str, Any], saved: Mapping[str, Any]) -> dict[str, Any]: + """Keep the stored secret behind any credential the caller echoed back redacted or omitted. + + GET returns credentials as the marker and the form never re-prefills a + secret, so a save that does not touch a credential arrives with the marker + or with the field absent. Either way the real secret must survive: it is + restored from the stored row, or dropped when there is no stored row (the + value is env-sourced and the marker must never be persisted). Non-secret + fields are taken from the incoming payload as-is, so clearing one still works. + + ``url`` is the exception: it is credential-bearing (redacted) yet also a + connection-mode selector that url-precedence resolves against host/port. If + the caller supplies a discrete target (host, cluster, or sentinel nodes), a + stored url is a stale mode the caller is leaving, so it is dropped rather + than restored, otherwise url-precedence would resurrect it and discard the + submitted host/port. + """ + switching_to_discrete_target = ( + _has_connection_target(incoming.get("host")) + or _has_connection_target(incoming.get("redis_startup_nodes")) + or _has_connection_target(incoming.get("sentinel_nodes")) + ) + reuse_saved_secret = _saved_secret_is_reusable(incoming, saved) + merged = dict(incoming) + for field in _CACHE_SENSITIVE_FIELDS: + # A value the caller explicitly supplied is honored verbatim: a new + # secret, or an empty string / null to clear the stored one. Only an + # omitted field or the echoed-back marker triggers preserve-or-drop. + if field in incoming and incoming[field] != _REDACTED_VALUE: + continue + if field == "url" and switching_to_discrete_target: + merged.pop(field, None) + continue + if field in saved and reuse_saved_secret: + merged[field] = saved[field] + else: + # nothing stored to reuse, or the caller is pointing at a different + # target: never persist/replay the marker or the stored secret + merged.pop(field, None) + return merged + + def _redact_settings(settings: Optional[Mapping[str, Any]]) -> Dict[str, Any]: """Replace every value in a settings map with a fixed marker. @@ -270,34 +441,34 @@ async def get_cache_settings( # Get cache settings fields from types file cache_fields = [field.model_copy(deep=True) for field in CACHE_SETTINGS_FIELDS] - # Try to get cache settings from database - current_values = {} + # Read the stored settings (decrypted); an env-only cache has none. + stored: dict[str, Any] = {} if prisma_client is not None: cache_config = await CacheConfigRepository(prisma_client).table.find_unique(where={"id": "cache_config"}) if cache_config is not None and cache_config.cache_settings: - # Decrypt cache settings - cache_settings_json = cache_config.cache_settings - if isinstance(cache_settings_json, str): - cache_settings_dict = json.loads(cache_settings_json) - else: - cache_settings_dict = cache_settings_json + stored = proxy_config._decrypt_db_variables( + variables_dict=_parse_stored_settings(cache_config.cache_settings) + ) - # Decrypt environment variables - decrypted_settings = proxy_config._decrypt_db_variables(variables_dict=cache_settings_dict) + # Fill connection fields from the REDIS_* environment the cache resolves + # from when the stored config leaves them unset, then apply url precedence + # so a url-mode config does not surface conflicting discrete fields (which + # would otherwise let a no-op save silently switch it to host/port). + effective = _resolve_cache_url_precedence(_overlay_environment(stored)) - # Derive redis_type for UI based on settings - # UI uses redis_type to show/hide fields, backend only stores 'type' - if decrypted_settings.get("type") == "redis": - if decrypted_settings.get("redis_startup_nodes"): - decrypted_settings["redis_type"] = "cluster" - elif decrypted_settings.get("sentinel_nodes"): - decrypted_settings["redis_type"] = "sentinel" - else: - decrypted_settings["redis_type"] = "node" + # Derive redis_type for UI based on settings + # UI uses redis_type to show/hide fields, backend only stores 'type' + if effective.get("type") == "redis": + if effective.get("redis_startup_nodes"): + effective["redis_type"] = "cluster" + elif effective.get("sentinel_nodes"): + effective["redis_type"] = "sentinel" + else: + effective["redis_type"] = "node" - # Mask credential fields so the GET response never carries - # plaintext Redis / Sentinel passwords off the server. - current_values = mask_sensitive_keys(decrypted_settings, _CACHE_SENSITIVE_FIELDS) + # Redact credential fields so the GET response never carries a plaintext + # Redis / Sentinel password off the server. + current_values = _redact_credentials(effective) # Update field values with current values for field in cache_fields: @@ -331,10 +502,27 @@ async def test_cache_connection( to verify the credentials work without affecting global state. """ from litellm import Cache + from litellm.proxy.proxy_server import prisma_client, proxy_config try: - cache_settings = _resolve_cache_url_precedence(request.cache_settings) - verbose_proxy_logger.debug("Testing cache connection with settings: %s", cache_settings) + # A credential the form left untouched arrives redacted; resolve it back + # to the stored secret so the test connects with the real password. A + # lookup failure must not block the test, so fall back to no stored row. + saved_settings: dict[str, Any] = {} + if prisma_client is not None: + try: + existing_row = await CacheConfigRepository(prisma_client).table.find_unique( + where={"id": "cache_config"} + ) + if existing_row is not None and existing_row.cache_settings: + saved_settings = proxy_config._decrypt_db_variables( + variables_dict=_parse_stored_settings(existing_row.cache_settings) + ) + except Exception: # noqa: BLE001 - a saved-settings lookup failure must not block a connection test + saved_settings = {} + cache_settings = _resolve_cache_url_precedence(_merge_over_saved(request.cache_settings, saved_settings)) + # cache_settings now carries the resolved plaintext credential; never log it raw + verbose_proxy_logger.debug("Testing cache connection with settings: %s", _redact_credentials(cache_settings)) # Only support Redis for now if cache_settings.get("type") != "redis": @@ -400,19 +588,20 @@ async def update_cache_settings( ) try: - cache_settings = _resolve_cache_url_precedence(request.cache_settings) - - # Snapshot the prior settings (key set only — values get redacted in - # the audit row) so the audit-log entry shows which fields changed. + # Read the stored row first: its decrypted values back any credential the + # caller echoed back redacted, and its key set drives the audit diff. existing_row = await CacheConfigRepository(prisma_client).table.find_unique(where={"id": "cache_config"}) before_settings: Optional[Dict[str, Any]] = None + saved_settings: dict[str, Any] = {} if existing_row is not None and existing_row.cache_settings: - try: - before_settings = json.loads(existing_row.cache_settings) - except (TypeError, ValueError): - before_settings = None + before_settings = _parse_stored_settings(existing_row.cache_settings) + saved_settings = proxy_config._decrypt_db_variables(variables_dict=before_settings) action: AUDIT_ACTIONS = "updated" if existing_row is not None else "created" + # Preserve stored secrets behind any redacted or omitted credential, then + # resolve the url-vs-discrete-fields precedence. + cache_settings = _resolve_cache_url_precedence(_merge_over_saved(request.cache_settings, saved_settings)) + # Encrypt sensitive fields (keep redis_type for storage) encrypted_settings = proxy_config._encrypt_env_variables(environment_variables=cache_settings) @@ -461,7 +650,7 @@ async def update_cache_settings( return { "message": "Cache settings updated successfully", "status": "success", - "settings": cache_settings, + "settings": _redact_credentials(cache_settings), } except Exception as e: verbose_proxy_logger.error(f"Error updating cache settings: {str(e)}") 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 f4c6d4f8d15..2504b5744fc 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 @@ -17,9 +17,14 @@ 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, + _REDACTED_VALUE, CacheSettingsManager, CacheSettingsUpdateRequest, CacheTestRequest, + _merge_over_saved, + _overlay_environment, + _parse_stored_settings, + _redact_credentials, _resolve_cache_url_precedence, get_cache_settings, test_cache_connection, @@ -610,3 +615,510 @@ async def test_update_cache_settings_no_audit_when_disabled(monkeypatch): ) assert audit_calls == [] + + +class TestParseStoredSettings: + """The stored blob arrives as a JSON string or a parsed dict; both must + normalize to a dict so the secret-preservation read never silently drops it.""" + + def test_parses_a_json_string(self): + assert _parse_stored_settings('{"host": "h", "password": "pw"}') == {"host": "h", "password": "pw"} + + def test_passes_a_dict_through(self): + assert _parse_stored_settings({"host": "h", "password": "pw"}) == {"host": "h", "password": "pw"} + + def test_non_mapping_becomes_empty(self): + assert _parse_stored_settings(None) == {} + assert _parse_stored_settings("[1, 2]") == {} + + +class TestMergeOverSaved: + """The secret-preservation contract behind the redacted-resubmit fix.""" + + def test_redacted_secret_restores_stored_value(self): + # same connection target, an unrelated field edited: the stored secret + # is restored behind the redacted resubmit + merged = _merge_over_saved( + incoming={"type": "redis", "host": "samehost", "namespace": "new", "password": _REDACTED_VALUE}, + saved={"type": "redis", "host": "samehost", "password": "realpw"}, + ) + assert merged["namespace"] == "new" + assert merged["password"] == "realpw" + + def test_stored_secret_not_replayed_to_a_different_target(self): + # credential replay guard: omitting the password while pointing at a new + # host must NOT resurrect the stored secret (it would be sent elsewhere) + merged = _merge_over_saved( + incoming={"type": "redis", "host": "attacker.example.com", "password": _REDACTED_VALUE}, + saved={"type": "redis", "host": "real-redis", "password": "realpw"}, + ) + assert "password" not in merged + + def test_omitted_secret_restores_stored_value(self): + # same host (target unchanged), password field omitted entirely + merged = _merge_over_saved( + incoming={"type": "redis", "host": "samehost", "namespace": "n"}, + saved={"type": "redis", "host": "samehost", "password": "realpw"}, + ) + assert merged["password"] == "realpw" + + def test_sentinel_password_not_replayed_to_different_sentinel_nodes(self): + # sentinel target change with an omitted sentinel_password must not + # resurrect the stored one and send it to the caller's sentinels + merged = _merge_over_saved( + incoming={"type": "redis", "sentinel_nodes": [["attacker", 26379]], "service_name": "mymaster"}, + saved={ + "type": "redis", + "sentinel_nodes": [["real", 26379]], + "service_name": "mymaster", + "sentinel_password": "realsp", + }, + ) + assert "sentinel_password" not in merged + + def test_sentinel_password_preserved_when_sentinel_target_unchanged(self): + merged = _merge_over_saved( + incoming={"type": "redis", "sentinel_nodes": [["real", 26379]], "service_name": "mymaster"}, + saved={ + "type": "redis", + "sentinel_nodes": [["real", 26379]], + "service_name": "mymaster", + "sentinel_password": "realsp", + }, + ) + assert merged["sentinel_password"] == "realsp" + + def test_password_not_replayed_to_different_cluster_nodes(self): + merged = _merge_over_saved( + incoming={"type": "redis", "redis_startup_nodes": [{"host": "attacker", "port": "7001"}]}, + saved={ + "type": "redis", + "redis_startup_nodes": [{"host": "real", "port": "7001"}], + "password": "realpw", + }, + ) + assert "password" not in merged + + def test_equivalent_target_representations_still_preserve_secret(self): + # the client sends port as a string, storage holds it as an int: the + # target is unchanged, so the untouched password must not be dropped + merged = _merge_over_saved( + incoming={"type": "redis", "host": "h", "port": "6379", "password": _REDACTED_VALUE}, + saved={"type": "redis", "host": "h", "port": 6379, "password": "realpw"}, + ) + assert merged["password"] == "realpw" + + def test_explicit_empty_string_clears_the_secret(self): + merged = _merge_over_saved( + incoming={"type": "redis", "host": "h", "password": ""}, + saved={"type": "redis", "host": "h", "password": "realpw"}, + ) + assert merged.get("password") == "" + + def test_explicit_null_clears_the_secret(self): + # an explicit null is a clear, not an omission, so it must not restore + merged = _merge_over_saved( + incoming={"type": "redis", "host": "h", "password": None}, + saved={"type": "redis", "host": "h", "password": "realpw"}, + ) + assert merged.get("password") is None + + def test_secret_not_reused_when_a_pinned_target_field_is_omitted(self): + # omitting the host (a pinned target) means the request does not describe + # the stored target, so the stored secret must not be restored (and thus + # cannot be sent to whatever host the incomplete request resolves to) + merged = _merge_over_saved( + incoming={"type": "redis", "port": "6379"}, + saved={"type": "redis", "host": "real", "port": 6379, "password": "realpw"}, + ) + assert "password" not in merged + + def test_redacted_secret_with_no_stored_value_is_dropped(self): + # env-sourced secret: nothing stored to restore, so the marker must not + # be persisted; the environment stays the source at runtime + merged = _merge_over_saved( + incoming={"type": "redis", "host": "h", "password": _REDACTED_VALUE}, + saved={}, + ) + assert "password" not in merged + + def test_new_secret_value_wins(self): + merged = _merge_over_saved( + incoming={"password": "brandnewpw"}, + saved={"password": "realpw"}, + ) + assert merged["password"] == "brandnewpw" + + def test_switching_from_url_to_host_port_drops_stored_url(self): + # admin migrates a url-mode cache to discrete host/port: the stored url + # must not be resurrected (url precedence would then discard host/port) + merged = _merge_over_saved( + incoming={"type": "redis", "host": "newhost", "port": "6379"}, + saved={"type": "redis", "url": "redis://:pw@oldhost:6379/0"}, + ) + assert "url" not in merged + assert merged["host"] == "newhost" + assert merged["port"] == "6379" + + def test_untouched_url_is_preserved_without_a_discrete_target(self): + # a url-mode save that touches nothing keeps the stored url + merged = _merge_over_saved( + incoming={"type": "redis", "namespace": "ns"}, + saved={"type": "redis", "url": "redis://:pw@host:6379/0"}, + ) + assert merged["url"] == "redis://:pw@host:6379/0" + + +def test_overlay_environment_fills_unset_connection_fields(monkeypatch): + """A cache with no stored connection resolves REDIS_* env for the UI.""" + for var in ("REDIS_URL", "REDIS_HOST", "REDIS_PORT", "REDIS_PASSWORD", "REDIS_USERNAME"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("REDIS_HOST", "redis.internal") + monkeypatch.setenv("REDIS_PORT", "6380") + monkeypatch.setenv("REDIS_PASSWORD", "env-password") + + effective = _overlay_environment({}) + + assert effective["host"] == "redis.internal" + assert effective["port"] == "6380" + assert effective["password"] == "env-password" + assert effective["type"] == "redis" + + +def test_overlay_environment_stored_value_wins(monkeypatch): + monkeypatch.setenv("REDIS_HOST", "env-host") + effective = _overlay_environment({"type": "redis", "host": "stored-host"}) + assert effective["host"] == "stored-host" + + +@pytest.mark.asyncio +async def test_get_cache_settings_falls_back_to_redis_env(monkeypatch): + """A cache configured purely through REDIS_* env vars shows its effective + connection instead of a blank page, with the password redacted.""" + for var in ("REDIS_URL", "REDIS_HOST", "REDIS_PORT", "REDIS_PASSWORD", "REDIS_USERNAME"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("REDIS_HOST", "redis.internal") + monkeypatch.setenv("REDIS_PORT", "6380") + monkeypatch.setenv("REDIS_PASSWORD", "env-password") + + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=None) + + 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()) + + values = response.current_values + assert values["host"] == "redis.internal" + assert values["port"] == "6380" + assert values["type"] == "redis" + # the env password is surfaced as configured, not leaked in plaintext + assert values["password"] == _REDACTED_VALUE + + +@pytest.mark.asyncio +async def test_get_cache_settings_redacts_password_with_marker(monkeypatch): + for var in ("REDIS_URL", "REDIS_HOST", "REDIS_PORT", "REDIS_PASSWORD", "REDIS_USERNAME"): + monkeypatch.delenv(var, raising=False) + cache_row = MagicMock() + cache_row.cache_settings = json.dumps( + {"type": "redis", "host": "h", "password": "supersecret", "namespace": "ns"} + ) + 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()) + + assert response.current_values["password"] == _REDACTED_VALUE + assert response.current_values["namespace"] == "ns" + + +@pytest.mark.asyncio +async def test_get_cache_settings_url_mode_hides_env_discrete_fields(monkeypatch): + """A url-mode stored config must not surface env-overlaid host/port. + + Otherwise a no-op save would submit the env host and, via url precedence, + silently switch the cache off its configured url. + """ + for var in ("REDIS_URL", "REDIS_HOST", "REDIS_PORT", "REDIS_PASSWORD", "REDIS_USERNAME"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("REDIS_HOST", "env-host") + monkeypatch.setenv("REDIS_PORT", "6380") + + cache_row = MagicMock() + cache_row.cache_settings = {"type": "redis", "url": "redis://:pw@stored-host:6379/0"} + 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()) + + values = response.current_values + assert values["url"] == _REDACTED_VALUE + # the env host/port must not leak in and shadow the url + assert "host" not in values + assert "port" not in values + + +def _mock_proxy_config_identity_crypto(): + proxy_config = MagicMock() + proxy_config._encrypt_env_variables = MagicMock( + side_effect=lambda environment_variables: dict(environment_variables) + ) + proxy_config._decrypt_db_variables = MagicMock(side_effect=lambda variables_dict: dict(variables_dict)) + proxy_config._init_cache = MagicMock() + proxy_config.switch_on_llm_response_caching = MagicMock() + return proxy_config + + +@pytest.mark.asyncio +async def test_update_preserves_stored_password_on_redacted_resubmit(monkeypatch): + """Editing an unrelated field and re-submitting the redacted password must + keep the stored secret, not persist the marker over a working password.""" + monkeypatch.setattr(litellm, "store_audit_logs", False) + + existing = MagicMock() + # prisma returns the Json column as an already-parsed dict, not a JSON + # string; a reader that json.loads unconditionally would drop the whole row + existing.cache_settings = {"type": "redis", "host": "oldhost", "password": "realpw"} + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=existing) + mock_prisma.db.litellm_cacheconfig.upsert = AsyncMock() + proxy_config = _mock_proxy_config_identity_crypto() + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.proxy.proxy_server.store_model_in_db", True), + ): + result = await update_cache_settings( + request=CacheSettingsUpdateRequest( + # same host (the target is unchanged), an unrelated field edited + cache_settings={"type": "redis", "host": "oldhost", "namespace": "edited", "password": _REDACTED_VALUE} + ), + user_api_key_dict=_admin_auth(), + litellm_changed_by=None, + ) + + persisted = proxy_config._encrypt_env_variables.call_args.kwargs["environment_variables"] + assert persisted["host"] == "oldhost" + assert persisted["namespace"] == "edited" + assert persisted["password"] == "realpw" + # the response never echoes the plaintext secret back either + assert result["settings"]["password"] == _REDACTED_VALUE + + +@pytest.mark.asyncio +async def test_update_drops_env_sourced_redacted_secret(monkeypatch): + """With no stored row, a re-submitted redacted secret is env-sourced; the + marker must not be persisted so the environment stays the source.""" + monkeypatch.setattr(litellm, "store_audit_logs", False) + + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=None) + mock_prisma.db.litellm_cacheconfig.upsert = AsyncMock() + proxy_config = _mock_proxy_config_identity_crypto() + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.proxy.proxy_server.store_model_in_db", True), + ): + await update_cache_settings( + request=CacheSettingsUpdateRequest( + cache_settings={"type": "redis", "host": "h", "password": _REDACTED_VALUE} + ), + user_api_key_dict=_admin_auth(), + litellm_changed_by=None, + ) + + persisted = proxy_config._encrypt_env_variables.call_args.kwargs["environment_variables"] + assert "password" not in persisted + + +@pytest.mark.asyncio +async def test_update_applies_new_password(monkeypatch): + """A real new secret value replaces the stored one.""" + monkeypatch.setattr(litellm, "store_audit_logs", False) + + existing = MagicMock() + existing.cache_settings = json.dumps({"type": "redis", "host": "h", "password": "oldpw"}) + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=existing) + mock_prisma.db.litellm_cacheconfig.upsert = AsyncMock() + proxy_config = _mock_proxy_config_identity_crypto() + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.proxy.proxy_server.store_model_in_db", True), + ): + await update_cache_settings( + request=CacheSettingsUpdateRequest( + cache_settings={"type": "redis", "host": "h", "password": "brandnewpw"} + ), + user_api_key_dict=_admin_auth(), + litellm_changed_by=None, + ) + + persisted = proxy_config._encrypt_env_variables.call_args.kwargs["environment_variables"] + assert persisted["password"] == "brandnewpw" + + +@pytest.mark.asyncio +async def test_test_cache_connection_survives_saved_lookup_failure(monkeypatch): + """A failed saved-settings lookup must not block the connection test. + + The test endpoint reads the stored row to resolve a redacted credential, but + that read can raise (a misconfigured or unavailable client), and it must fall + back to the submitted settings rather than abort — otherwise a shared client + left in an odd state by another test would break every connection test. + """ + monkeypatch.setattr(litellm, "store_audit_logs", False) + + # a client whose find_unique is not awaitable, so the saved read raises + bad_prisma = MagicMock() + proxy_config = MagicMock() + proxy_config._decrypt_db_variables = MagicMock(side_effect=lambda variables_dict: dict(variables_dict)) + + cache_instance = MagicMock() + cache_instance.cache = MagicMock() + cache_instance.cache.test_connection = AsyncMock(return_value={"status": "success", "message": "ok"}) + + with ( + patch("litellm.proxy.proxy_server.prisma_client", bad_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.Cache") as mock_cache_class, + ): + mock_cache_class.return_value = cache_instance + result = await test_cache_connection( + request=CacheTestRequest(cache_settings={"type": "redis", "host": "h", "port": "6379", "password": "pw"}), + user_api_key_dict=_admin_auth(), + ) + + mock_cache_class.assert_called_once() + assert result.status == "success" + + +@pytest.mark.asyncio +async def test_get_cache_settings_does_not_surface_non_display_env_credentials(monkeypatch): + """The env overlay must not leak credential kwargs the UI does not manage. + + _redis_kwargs_from_environment resolves every redis.Redis kwarg, including + secrets like azure_client_secret; only cache display fields may be surfaced, + so a non-admin reading /cache/settings never retrieves such a credential. + """ + for var in ("REDIS_URL", "REDIS_HOST", "REDIS_PORT", "REDIS_PASSWORD", "REDIS_AZURE_CLIENT_SECRET"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("REDIS_HOST", "redis.internal") + monkeypatch.setenv("REDIS_AZURE_CLIENT_SECRET", "super-azure-secret") + + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=None) + 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()) + + values = response.current_values + assert values.get("host") == "redis.internal" + # the non-display credential must not appear in the response at all + assert "azure_client_secret" not in values + assert "super-azure-secret" not in values.values() + + +@pytest.mark.asyncio +async def test_test_cache_connection_does_not_log_plaintext_credentials(monkeypatch, caplog): + """The connection test must not write the resolved plaintext secret to logs. + + _merge_over_saved substitutes the stored password for a redacted resubmit, so + the settings dict carries the real secret; the debug log must redact it. + """ + import logging + + existing = MagicMock() + existing.cache_settings = {"type": "redis", "host": "h", "port": "6379", "password": "realredispw"} + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=existing) + proxy_config = MagicMock() + proxy_config._decrypt_db_variables = MagicMock(side_effect=lambda variables_dict: dict(variables_dict)) + + cache_instance = MagicMock() + cache_instance.cache = MagicMock() + cache_instance.cache.test_connection = AsyncMock(return_value={"status": "success", "message": "ok"}) + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.Cache") as mock_cache_class, + caplog.at_level(logging.DEBUG, logger="LiteLLM Proxy"), + ): + mock_cache_class.return_value = cache_instance + # resubmit the redacted marker; the merge resolves it to the stored secret + await test_cache_connection( + request=CacheTestRequest( + cache_settings={"type": "redis", "host": "h", "port": "6379", "password": _REDACTED_VALUE} + ), + user_api_key_dict=_admin_auth(), + ) + + # the real password was used to build the client but never written to the log + assert mock_cache_class.call_args.kwargs["password"] == "realredispw" + assert "realredispw" not in caplog.text + + +@pytest.mark.asyncio +async def test_test_cache_connection_does_not_replay_saved_password_to_new_host(monkeypatch): + """Credential-replay guard on the connection test. + + A caller that submits a different host while omitting the password must not + have the stored password restored and sent to the caller-chosen host. + """ + existing = MagicMock() + existing.cache_settings = {"type": "redis", "host": "real-redis", "port": "6379", "password": "realredispw"} + mock_prisma = MagicMock() + mock_prisma.db.litellm_cacheconfig.find_unique = AsyncMock(return_value=existing) + proxy_config = MagicMock() + proxy_config._decrypt_db_variables = MagicMock(side_effect=lambda variables_dict: dict(variables_dict)) + + cache_instance = MagicMock() + cache_instance.cache = MagicMock() + cache_instance.cache.test_connection = AsyncMock(return_value={"status": "success", "message": "ok"}) + + with ( + patch("litellm.proxy.proxy_server.prisma_client", mock_prisma), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch("litellm.Cache") as mock_cache_class, + ): + mock_cache_class.return_value = cache_instance + await test_cache_connection( + request=CacheTestRequest( + cache_settings={"type": "redis", "host": "attacker.example.com", "port": "6379"} + ), + user_api_key_dict=_admin_auth(), + ) + + called_kwargs = mock_cache_class.call_args.kwargs + # the stored password is NOT sent to the attacker-chosen host + assert called_kwargs.get("password") != "realredispw" + assert "password" not in called_kwargs diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFieldSection.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFieldSection.tsx index ced822cd796..96106869009 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFieldSection.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFieldSection.tsx @@ -10,6 +10,7 @@ interface CacheFieldSectionProps { embeddingModels: EmbeddingModelOption[]; gridCols?: string; headingLevel?: "h4" | "h5"; + configuredSecrets?: ReadonlySet; } const CacheFieldSection: React.FC = ({ @@ -19,6 +20,7 @@ const CacheFieldSection: React.FC = ({ embeddingModels, gridCols = "grid-cols-1 gap-6 sm:grid-cols-2", headingLevel = "h4", + configuredSecrets, }) => { const fields = fieldsForSection(section, redisType); if (fields.length === 0) { @@ -32,7 +34,12 @@ const CacheFieldSection: React.FC = ({ {title}
{fields.map((field) => ( - + ))}
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFormField.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFormField.tsx index d92ca302901..dbd8c32d18d 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFormField.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/CacheFormField.tsx @@ -7,22 +7,29 @@ export interface EmbeddingModelOption { label: string; } +export const SECRET_ALREADY_SET_PLACEHOLDER = "Already set. Enter a new value to replace it."; + interface CacheFormFieldProps { field: CacheField; embeddingModels: EmbeddingModelOption[]; + isSecretConfigured?: boolean; } -const renderControl = (field: CacheField, embeddingModels: EmbeddingModelOption[]): React.ReactNode => { +const renderControl = ( + field: CacheField, + embeddingModels: EmbeddingModelOption[], + placeholder: string, +): React.ReactNode => { switch (field.type) { case "boolean": return ; case "password": - return ; + return ; case "integer": case "float": - return ; + return ; case "list": - return ; + return ; case "model-select": return ( ; + return ; } }; -const CacheFormField: React.FC = ({ field, embeddingModels }) => ( +const CacheFormField: React.FC = ({ field, embeddingModels, isSecretConfigured = false }) => ( = ({ field, embeddingModels rules={field.rules} valuePropName={field.type === "boolean" ? "checked" : "value"} > - {renderControl(field, embeddingModels)} + {renderControl(field, embeddingModels, isSecretConfigured ? SECRET_ALREADY_SET_PLACEHOLDER : field.helpText)} ); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsFields.ts b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsFields.ts index 1f5b566fc5f..e33b525c3ef 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsFields.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsFields.ts @@ -8,6 +8,10 @@ export type CacheSection = "connection" | "cluster" | "sentinel" | "semantic" | export type CacheFieldRule = NonNullable[number]; +// Marker the backend returns for a configured credential and maps back to the +// stored secret on save, so the plaintext never round-trips through the form. +export const REDACTED_VALUE = "***REDACTED***"; + export interface CacheField { readonly name: string; readonly label: string; @@ -17,6 +21,9 @@ export interface CacheField { readonly redisType: RedisType | null; readonly defaultValue?: string | number | boolean; readonly rules?: CacheFieldRule[]; + // Credential field: never prefilled into the form, and dropped from the save + // payload when left untouched so the redacted marker is never persisted. + readonly secret?: boolean; } export const REDIS_TYPES: readonly RedisType[] = ["node", "cluster", "sentinel", "semantic"]; @@ -93,6 +100,7 @@ export const CACHE_FIELDS: readonly CacheField[] = [ helpText: "Full Redis/Valkey connection URL (e.g. redis://:password@host:6379/1). When set, it takes precedence over Host, Port, Password, and Database Index.", redisType: null, + secret: true, }, { name: "host", @@ -128,6 +136,7 @@ export const CACHE_FIELDS: readonly CacheField[] = [ section: "connection", helpText: "Redis server password", redisType: null, + secret: true, }, { name: "username", @@ -170,6 +179,7 @@ export const CACHE_FIELDS: readonly CacheField[] = [ section: "sentinel", helpText: "Password for Redis Sentinel authentication", redisType: "sentinel", + secret: true, }, { name: "similarity_threshold", diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts index 79f28a97842..c530519ee06 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; -import { buildCachePayload, buildInitialValues, fieldsForSection } from "./cacheSettingsUtils"; +import { buildCachePayload, buildInitialValues, configuredSecretFields, fieldsForSection } from "./cacheSettingsUtils"; +import { REDACTED_VALUE } from "./cacheSettingsFields"; describe("fieldsForSection", () => { it("should only include a redis-type-specific field when that type is selected", () => { @@ -83,4 +84,49 @@ describe("buildCachePayload", () => { const payload = buildCachePayload("node", { sentinel_nodes: '[["localhost",26379]]' }, { forTesting: false }); expect(payload).not.toHaveProperty("sentinel_nodes"); }); + + it("should drop a secret whose value is the redacted marker so it is never persisted", () => { + const payload = buildCachePayload( + "node", + { host: "localhost", password: REDACTED_VALUE, url: REDACTED_VALUE }, + { forTesting: false }, + ); + expect(payload).not.toHaveProperty("password"); + expect(payload).not.toHaveProperty("url"); + expect(payload.host).toBe("localhost"); + }); + + it("should send a real new secret value the admin typed", () => { + const payload = buildCachePayload("node", { password: "brandnewpw" }, { forTesting: false }); + expect(payload.password).toBe("brandnewpw"); + }); +}); + +describe("secret handling", () => { + it("buildInitialValues never prefills a credential, even when the server reports it configured", () => { + const serverValues = { + host: "localhost", + password: REDACTED_VALUE, + url: REDACTED_VALUE, + sentinel_password: REDACTED_VALUE, + }; + const values = buildInitialValues(serverValues); + expect(values.password).toBe(""); + expect(values.url).toBe(""); + expect(values.sentinel_password).toBe(""); + // non-secret fields are still prefilled + expect(values.host).toBe("localhost"); + }); + + it("configuredSecretFields reports which credentials the server marked as set", () => { + const configured = configuredSecretFields({ + password: REDACTED_VALUE, + url: "", + host: "localhost", + }); + expect(configured.has("password")).toBe(true); + expect(configured.has("url")).toBe(false); + // a non-secret field is never reported as a configured secret + expect(configured.has("host")).toBe(false); + }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts index 088da21961c..7b9454a37c3 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts @@ -1,4 +1,4 @@ -import { CACHE_FIELDS, CacheField, CacheSection, RedisType } from "./cacheSettingsFields"; +import { CACHE_FIELDS, CacheField, CacheSection, REDACTED_VALUE, RedisType } from "./cacheSettingsFields"; export type CacheFormValue = string | number | boolean | undefined; export type CacheFormValues = Record; @@ -11,7 +11,20 @@ export const isFieldVisible = (field: CacheField, redisType: RedisType): boolean export const fieldsForSection = (section: CacheSection, redisType: RedisType): CacheField[] => CACHE_FIELDS.filter((field) => field.section === section && isFieldVisible(field, redisType)); +const hasValue = (raw: unknown): boolean => raw !== undefined && raw !== null && raw !== ""; + +// Credential fields the server reports as configured (returned as the redacted +// marker). Used to show an "already set" hint without ever holding the secret. +export const configuredSecretFields = (currentValues: Record): ReadonlySet => + new Set(CACHE_FIELDS.filter((field) => field.secret && hasValue(currentValues[field.name])).map((f) => f.name)); + const initialValueForField = (field: CacheField, raw: unknown): CacheFormValue => { + // Never prefill a credential: the server sends the redacted marker for a + // configured secret, and echoing it back would persist the marker. + if (field.secret) { + return ""; + } + const source = raw ?? field.defaultValue; if (field.type === "boolean") { @@ -35,6 +48,11 @@ export const buildInitialValues = (currentValues: Record): Cach Object.fromEntries(CACHE_FIELDS.map((field) => [field.name, initialValueForField(field, currentValues[field.name])])); const saveValueForField = (field: CacheField, raw: CacheFormValue): CacheSavePayloadValue | undefined => { + // A redacted secret echoed back untouched must never be persisted as a value. + if (field.secret && raw === REDACTED_VALUE) { + return undefined; + } + if (field.type === "boolean") { return Boolean(raw); } diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.tsx index 4382769ae9c..fea2c04015b 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.tsx @@ -8,7 +8,7 @@ import RedisTypeSelector from "./RedisTypeSelector"; import CacheFieldSection from "./CacheFieldSection"; import { EmbeddingModelOption } from "./CacheFormField"; import { REDIS_TYPES, REDIS_TYPE_DESCRIPTIONS, RedisType } from "./cacheSettingsFields"; -import { buildCachePayload, buildInitialValues, CacheFormValues } from "./cacheSettingsUtils"; +import { buildCachePayload, buildInitialValues, CacheFormValues, configuredSecretFields } from "./cacheSettingsUtils"; interface CacheSettingsProps { accessToken: string | null; @@ -25,6 +25,7 @@ const CacheSettings: React.FC = ({ accessToken }) => { const [embeddingModels, setEmbeddingModels] = useState([]); const [isTesting, setIsTesting] = useState(false); const [isSaving, setIsSaving] = useState(false); + const [configuredSecrets, setConfiguredSecrets] = useState>(new Set()); const loadCacheSettings = useCallback(async () => { if (!accessToken) { @@ -34,6 +35,7 @@ const CacheSettings: React.FC = ({ accessToken }) => { const data = (await getCacheSettingsCall(accessToken)) as { current_values?: Record }; const currentValues = data.current_values ?? {}; form.setFieldsValue(buildInitialValues(currentValues)); + setConfiguredSecrets(configuredSecretFields(currentValues)); setRedisType(toRedisType(currentValues.redis_type)); } catch (error) { console.error("Failed to load cache settings:", error); @@ -144,6 +146,7 @@ const CacheSettings: React.FC = ({ accessToken }) => { section="connection" redisType={redisType} embeddingModels={embeddingModels} + configuredSecrets={configuredSecrets} /> @@ -166,6 +169,7 @@ const CacheSettings: React.FC = ({ accessToken }) => { section="sentinel" redisType={redisType} embeddingModels={embeddingModels} + configuredSecrets={configuredSecrets} /> )}