fix: mask secrets in SSO update response, honor explicit clears and omitted fields

- restore a stored secret only when the field was omitted from the request or
  the submitted value is exactly the mask of the effective secret; explicit
  null / "" still clear (dashboard 'Clear SSO Settings'), and a new secret
  that happens to contain '*' is saved as-is
- return a copy instead of mutating sso_data in place
- mask *_client_secret fields in the /update/sso_settings response so the
  restored plaintext is never echoed back to the caller
- tests: omitted field, explicit null, asterisk-in-new-secret, masked response;
  legacy test updated to expect the masked response

litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py

@@ -3,7 +3,8 @@ import asyncio
 import json
 import os
 from collections import Counter
-from collections.abc import Mapping, MutableMapping
+from collections.abc import Mapping
+from collections.abc import Set as AbstractSet
 from typing import (
     Any,
     Final,
@@ -959,31 +960,39 @@ async def get_sso_settings():
 
 
 def _restore_masked_sso_secrets(
-    sso_data: MutableMapping[str, object],  # mutable-ok: stored secrets are restored into the caller's config in place
+    sso_data: Mapping[str, object],
+    submitted_fields: AbstractSet[str],
     before_sso_data: Mapping[str, object] | None,
-) -> None:
-    """Keep a stored SSO secret when the client round-trips its masked value.
+) -> dict[str, object]:
+    """Return a copy of ``sso_data`` with stored SSO secrets restored where the
+    client could not have meant to change them.
 
     Secret fields are masked before they are sent to the UI (see
-    get_sso_settings), so a client that edits only unrelated fields sends the
-    masked placeholder back unchanged. A plaintext secret never contains the
-    mask character, so an incoming secret that still carries the mask is treated
-    as "unchanged" and the previously effective secret is restored. This stops a
-    partial edit from overwriting e.g. the OAuth client_secret with
-    `abcd****wxyz` and breaking SSO login. An empty value is an intentional
-    clear and a genuinely new secret has no mask, so both pass through unchanged.
+    get_sso_settings), so a client that edits only unrelated fields either
+    omits the secret or sends the masked placeholder back unchanged. Persisting
+    either would overwrite e.g. the OAuth client_secret with ``abcd****wxyz``
+    and break SSO login (#38177). A secret is restored when the field was not
+    submitted at all, or when the submitted value is exactly the mask of the
+    currently effective secret; anything else -- a new secret, ``""`` or
+    ``None`` -- is an explicit instruction and passes through untouched, so
+    "Clear SSO Settings" still clears.
 
     The effective secret is the stored row value, falling back to the process
     environment -- the same precedence get_sso_settings uses when it masks the
     field -- so a secret configured via an environment variable is preserved
-    too, not only database-stored ones. See #38177.
+    too, not only database-stored ones.
     """
+    restored: Final[dict[str, object]] = dict(sso_data)
     for secret_field in SSO_SECRET_FIELDS:
         db_secret = before_sso_data.get(secret_field) if before_sso_data else None
         stored_secret = db_secret or os.environ.get(SSO_FIELD_ENV_VARS.get(secret_field, ""))
-        incoming_secret = sso_data.get(secret_field)
-        if stored_secret and isinstance(incoming_secret, str) and "*" in incoming_secret:
-            sso_data[secret_field] = stored_secret  # rebind-ok: intentional in-place restore of a masked secret
+        if not stored_secret:
+            continue
+        incoming_secret = restored.get(secret_field)
+        masked_secret = mask_sensitive_keys({secret_field: stored_secret}, {secret_field})[secret_field]
+        if secret_field not in submitted_fields or incoming_secret == masked_secret:
+            restored[secret_field] = stored_secret
+    return restored
 
 
 @router.patch(
@@ -1048,9 +1057,7 @@ async def update_sso_settings(
         config["general_settings"] = {}
 
     # Update environment variables in config and in memory
-    sso_data: Final = sso_config.model_dump()
-
-    _restore_masked_sso_secrets(sso_data, before_sso_data)
+    sso_data: Final = _restore_masked_sso_secrets(sso_config.model_dump(), sso_config.model_fields_set, before_sso_data)
     for field_name, value in sso_data.items():
         if field_name in SSO_FIELD_ENV_VARS:
             env_var_name = SSO_FIELD_ENV_VARS[field_name]
@@ -1124,7 +1131,9 @@ async def update_sso_settings(
     return {
         "message": "SSO settings updated successfully",
         "status": "success",
-        "settings": sso_data,
+        # Never echo plaintext secrets back; the response is masked the same way
+        # get_sso_settings masks them.
+        "settings": mask_sensitive_keys(sso_data, set(SSO_SECRET_FIELDS)),
     }
 
 

tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py

@@ -580,16 +580,15 @@ class TestProxySettingEndpoints:
         # Verify settings were updated
         settings = data["settings"]
         assert settings["google_client_id"] == new_sso_settings["google_client_id"]
-        assert (
-            settings["google_client_secret"] == new_sso_settings["google_client_secret"]
-        )
+        # Secrets are masked in the response; the plaintext is never echoed back.
+        assert settings["google_client_secret"] != new_sso_settings["google_client_secret"]
+        assert settings["google_client_secret"].startswith("new_")
+        assert "****" in settings["google_client_secret"]
         assert (
             settings["microsoft_client_id"] == new_sso_settings["microsoft_client_id"]
         )
-        assert (
-            settings["microsoft_client_secret"]
-            == new_sso_settings["microsoft_client_secret"]
-        )
+        assert settings["microsoft_client_secret"] != new_sso_settings["microsoft_client_secret"]
+        assert "****" in settings["microsoft_client_secret"]
         assert settings["proxy_base_url"] == new_sso_settings["proxy_base_url"]
         assert settings["user_email"] == new_sso_settings["user_email"]
 

tests/test_litellm/proxy/ui_crud_endpoints/test_sso_secret_backfill.py

@@ -1,8 +1,10 @@
 """Regression tests for #38177: a partial SSO settings update must not overwrite
-a stored client secret with the masked placeholder the UI sends back, while
-still allowing an intentional clear and a genuinely new secret."""
+a stored client secret with the masked placeholder the UI sends back (or lose
+it when the field is omitted), while still allowing an intentional clear, a
+genuinely new secret, and never echoing the plaintext secret in the response."""
 
 import json
+import os
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
@@ -81,6 +83,8 @@ def test_database_stored_secret_is_preserved(mock_auth, monkeypatch):
     stored = _stored_secret(mock_prisma)
     assert stored["generic_client_secret"] == REAL_SECRET
     assert stored["proxy_base_url"] == "https://new.example.com"
+    # The restored plaintext must not leak back to the caller.
+    assert resp.json()["settings"]["generic_client_secret"] == _masked(REAL_SECRET)
 
 
 def test_environment_sourced_secret_is_preserved(mock_auth, monkeypatch):
@@ -140,3 +144,55 @@ def test_new_secret_is_saved(mock_auth, monkeypatch):
 
     stored = _stored_secret(mock_prisma)
     assert stored["generic_client_secret"] == "brand_new_secret_9999"
+
+
+def test_omitted_secret_is_preserved(mock_auth, monkeypatch):
+    """A partial payload that leaves the secret out entirely must keep it."""
+    monkeypatch.setenv("LITELLM_SALT_KEY", "test_salt_key")
+    monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", True)
+    monkeypatch.delenv("GENERIC_CLIENT_SECRET", raising=False)
+    mock_prisma = _mock_prisma(
+        monkeypatch,
+        {"generic_client_id": "cid", "generic_client_secret": REAL_SECRET},
+    )
+
+    resp = client.patch("/update/sso_settings", json={"generic_client_id": "cid", "proxy_base_url": "https://x"})
+    assert resp.status_code == 200
+
+    assert _stored_secret(mock_prisma)["generic_client_secret"] == REAL_SECRET
+
+
+def test_explicit_null_clears_intentionally(mock_auth, monkeypatch):
+    """The dashboard's "Clear SSO Settings" sends null; that must clear, not restore."""
+    monkeypatch.setenv("LITELLM_SALT_KEY", "test_salt_key")
+    monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", True)
+    monkeypatch.delenv("GENERIC_CLIENT_SECRET", raising=False)
+    mock_prisma = _mock_prisma(
+        monkeypatch,
+        {"generic_client_id": "cid", "generic_client_secret": REAL_SECRET},
+    )
+
+    resp = client.patch("/update/sso_settings", json={"generic_client_id": "cid", "generic_client_secret": None})
+    assert resp.status_code == 200
+
+    assert _stored_secret(mock_prisma)["generic_client_secret"] is None
+    assert "GENERIC_CLIENT_SECRET" not in os.environ
+
+
+def test_new_secret_containing_asterisk_is_saved(mock_auth, monkeypatch):
+    """Only the exact mask of the stored secret counts as "unchanged"."""
+    monkeypatch.setenv("LITELLM_SALT_KEY", "test_salt_key")
+    monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", True)
+    monkeypatch.delenv("GENERIC_CLIENT_SECRET", raising=False)
+    mock_prisma = _mock_prisma(
+        monkeypatch,
+        {"generic_client_id": "cid", "generic_client_secret": REAL_SECRET},
+    )
+
+    resp = client.patch(
+        "/update/sso_settings", json={"generic_client_id": "cid", "generic_client_secret": "new*secret*with*stars"}
+    )
+    assert resp.status_code == 200
+
+    assert _stored_secret(mock_prisma)["generic_client_secret"] == "new*secret*with*stars"
+    assert resp.json()["settings"]["generic_client_secret"] != "new*secret*with*stars"