fix(proxy): never return unreadable legacy ciphertext as plaintext and refuse rewrite passes without PyNaCl

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

litellm/proxy/common_utils/encrypt_decrypt_utils.py

@@ -196,7 +196,7 @@ def decrypt_value_helper(
         return value
     except LegacyEncryptionUnavailableError as error:
         verbose_proxy_logger.error("Cannot decrypt value for key: %s. %s", key, error)
-        return value if return_original_value else None
+        return None
     except Exception as e:
         error_message = f"Error decrypting value for key: {key}, Did your master_key/salt key change recently? \nError: {e}\nSet permanent salt key - https://docs.litellm.ai/docs/proxy/prod#5-set-litellm-salt-key"
         if exception_type == "debug":
@@ -221,6 +221,15 @@ def legacy_encryption_available() -> bool:
     return True
 
 
+def require_legacy_reader(purpose: str) -> None:
+    """Refuse a decrypt-then-rewrite pass when PyNaCl is missing: every unprefixed value would read as
+    unreadable and be dropped, double wrapped or miscounted as plaintext, so the pass cannot be trusted."""
+    if not legacy_encryption_available():
+        raise LegacyEncryptionUnavailableError(
+            f"Cannot {purpose}: PyNaCl is needed to read legacy {_ALGO_XSALSA20} values. {_LEGACY_ENCRYPTION_HELP}"
+        )
+
+
 def _legacy_secret_box(signing_key: str, purpose: str) -> "SecretBox":
     try:
         import nacl.secret

litellm/proxy/management_endpoints/credential_migration.py

@@ -42,7 +42,6 @@ if TYPE_CHECKING:
 from litellm.proxy.common_utils.encrypt_decrypt_utils import (
     _ALGO_AES_GCM,
     _ENCRYPTION_ALGORITHM_SETTING,
-    _LEGACY_ENCRYPTION_HELP,
     SecretMapDecodeError,
     _get_encryption_algorithm,
     _get_salt_key,
@@ -50,7 +49,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import (
     decrypt_value_helper,
     encrypt_value_helper,
     is_versioned_gcm,
-    legacy_encryption_available,
+    require_legacy_reader,
 )
 
 ValueClass = Literal["migrated", "legacy", "plaintext", "undecryptable", "not-a-string"]
@@ -206,15 +205,6 @@ def _assert_aes_gate_enabled() -> None:
         )
 
 
-def _assert_legacy_reader_available() -> None:
-    """Fail fast when PyNaCl is missing: without it every unprefixed value is unreadable and would be
-    miscounted as plaintext, so the scan cannot attest anything and the migration cannot rewrite legacy rows."""
-    if not legacy_encryption_available():
-        raise RuntimeError(
-            f"Encryption scan and migration need PyNaCl to read legacy values. {_LEGACY_ENCRYPTION_HELP}"
-        )
-
-
 # ---------------------------------------------------------------------------
 # Walkers for the locations with no pre-existing rotation path.
 # Each walker delegates the structural transform to the existing, tested helper
@@ -644,7 +634,7 @@ async def migrate_encryption(
     net-new walkers run in dry-run mode.
     """
     _assert_aes_gate_enabled()
-    _assert_legacy_reader_available()
+    require_legacy_reader("migrate stored encryption")
 
     report: Final = MigrationReport()
 
@@ -677,7 +667,7 @@ async def check_encryption(prisma_client: object) -> MigrationReport:
     config rows, SSO config). Reports how many values are still ``legacy``;
     ``residual_legacy == 0`` across this full scan is the compliance attestation.
     """
-    _assert_legacy_reader_available()
+    require_legacy_reader("scan stored encryption")
     report: Final = MigrationReport()
 
     # Rotation-covered tables (read-only classification).

litellm/proxy/management_endpoints/key_management_endpoints.py

@@ -79,6 +79,10 @@ from litellm.proxy.common_utils.config_sync_pubsub import (
     coordination_redis_cache,
     publish_config_change,
 )
+from litellm.proxy.common_utils.encrypt_decrypt_utils import (
+    LegacyEncryptionUnavailableError,
+    require_legacy_reader,
+)
 from litellm.proxy.common_utils.rbac_utils import check_org_admin_can_generate_keys
 from litellm.proxy.common_utils.timezone_utils import get_budget_reset_time
 from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache
@@ -5175,6 +5179,11 @@ async def _rotate_master_key(
 
     from litellm.proxy.proxy_server import proxy_config
 
+    try:
+        require_legacy_reader("rotate the master key")
+    except LegacyEncryptionUnavailableError as error:
+        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail={"error": str(error)}) from error
+
     try:
         models: list | None = cast(  # cast-ok: find_many returns a real list, which TableActions widens to Sequence
             "list[object]", await _prisma_table(ModelRepository(prisma_client)).find_many()

litellm/proxy/proxy_server.py

@@ -403,6 +403,7 @@ from litellm.proxy.common_utils.discoverable_model_filter import discoverable_ro
 from litellm.proxy.common_utils.encrypt_decrypt_utils import (
     decrypt_value_helper,
     encrypt_value_helper,
+    require_legacy_reader,
 )
 from litellm.proxy.common_utils.error_body_call_id import JSON_OBJECT, error_body_call_id, with_call_id
 from litellm.proxy.common_utils.fips import (
@@ -7325,6 +7326,7 @@ class ProxyConfig:
         _decrypt_and_set_db_env_variables): this is a write path, and
         loading values into os.environ is the read path's responsibility.
         """
+        require_legacy_reader("re-encrypt environment variables for the config save")
         decrypted_env_vars: Final = self._decrypt_db_variables(environment_variables)
         return self._encrypt_env_variables(
             environment_variables=decrypted_env_vars,

tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py

@@ -29,6 +29,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import (
     encrypt_value_helper,
     is_versioned_gcm,
     legacy_encryption_available,
+    require_legacy_reader,
 )
 from litellm.proxy.common_utils.fips import FipsModeError
 
@@ -177,7 +178,7 @@ def test_versioned_gcm_values_never_import_nacl(monkeypatch):
     assert decrypt_value_helper(legacy_v2, key="t") == "v2-secret"
 
 
-def test_legacy_ciphertext_without_pynacl_logs_the_reencrypt_path_and_keeps_the_stored_value(monkeypatch, caplog):
+def test_legacy_ciphertext_without_pynacl_logs_the_reencrypt_path_and_never_returns_the_blob(monkeypatch, caplog):
     legacy = _legacy_nacl_ciphertext("legacy-secret", _sha256_key())
     assert legacy_encryption_available()
     monkeypatch.setitem(sys.modules, "nacl", None)
@@ -185,7 +186,7 @@ def test_legacy_ciphertext_without_pynacl_logs_the_reencrypt_path_and_keeps_the_
     assert not legacy_encryption_available()
 
     with caplog.at_level(logging.ERROR, logger="LiteLLM Proxy"):
-        assert decrypt_value_helper(legacy, key="t", exception_type="debug", return_original_value=True) == legacy
+        assert decrypt_value_helper(legacy, key="t", exception_type="debug", return_original_value=True) is None
         assert decrypt_value_helper(legacy, key="t", exception_type="debug") is None
         assert decrypt_if_encrypted_with(legacy, _SALT_KEY) is None
     assert len(caplog.records) == 3, caplog.text
@@ -197,6 +198,15 @@ def test_legacy_ciphertext_without_pynacl_logs_the_reencrypt_path_and_keeps_the_
     assert "legacy-secret" not in caplog.text
 
 
+def test_require_legacy_reader_refuses_rewrite_passes_without_pynacl(monkeypatch):
+    require_legacy_reader("rotate the master key")
+    monkeypatch.setitem(sys.modules, "nacl", None)
+    monkeypatch.setitem(sys.modules, "nacl.secret", None)
+
+    with pytest.raises(LegacyEncryptionUnavailableError, match="rotate the master key.*legacy-encryption"):
+        require_legacy_reader("rotate the master key")
+
+
 def test_legacy_opt_in_without_pynacl_fails_the_write_not_silently(monkeypatch):
     _use_legacy(monkeypatch)
     monkeypatch.setitem(sys.modules, "nacl", None)

tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py

@@ -9435,6 +9435,36 @@ async def test_rotate_master_key_reencrypts_model_params_in_place(
     ), "api_key must be stored re-encrypted under the new master key, not in plaintext"
 
 
+async def test_rotate_master_key_refuses_without_pynacl_before_touching_any_row(monkeypatch):
+    import sys
+    from unittest.mock import AsyncMock, MagicMock
+
+    from fastapi import HTTPException
+
+    from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth
+    from litellm.proxy.management_endpoints.key_management_endpoints import (
+        _rotate_master_key,
+    )
+
+    monkeypatch.setitem(sys.modules, "nacl", None)
+    monkeypatch.setitem(sys.modules, "nacl.secret", None)
+    mock_prisma_client = AsyncMock()
+    mock_prisma_client.db = MagicMock()
+    mock_prisma_client.db.litellm_proxymodeltable.find_many = AsyncMock(return_value=[])
+
+    with pytest.raises(HTTPException) as exc_info:
+        await _rotate_master_key(
+            prisma_client=mock_prisma_client,
+            user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, user_id="test-user"),
+            current_master_key="sk-old-master-key",
+            new_master_key="sk-new-master-key",
+        )
+
+    assert exc_info.value.status_code == 400
+    assert "legacy-encryption" in exc_info.value.detail["error"]
+    mock_prisma_client.db.litellm_proxymodeltable.find_many.assert_not_awaited()
+
+
 async def test_default_key_generate_params_duration(monkeypatch):
     """
     Test that default_key_generate_params with 'duration' is applied

tests/test_litellm/proxy/proxy_server/test_proxy_config.py

@@ -13,6 +13,7 @@ import json
 import logging
 import os
 import re
+import sys
 from collections.abc import Mapping
 from dataclasses import dataclass
 from datetime import datetime
@@ -22,10 +23,14 @@ from typing import Any, Dict, Final
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
+from pydantic import JsonValue, TypeAdapter, ValidationError
 
 import litellm
 from litellm.proxy._types import CommonProxyErrors
-from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper
+from litellm.proxy.common_utils.encrypt_decrypt_utils import (
+    LegacyEncryptionUnavailableError,
+    encrypt_value_helper,
+)
 from litellm.proxy.proxy_server import (
     ProxyConfig,
     _is_remote_module_url,
@@ -33,14 +38,13 @@ from litellm.proxy.proxy_server import (
     _scrub_guardrail_inner,
     resolve_complexity_router_plugins,
     resolve_routing_plugins,
+    validate_auto_router_capability_limits,
     validate_deployment_access_windows,
     validate_deployment_complexity_router_placement,
     validate_deployment_max_agentic_loops,
-    validate_auto_router_capability_limits,
 )
 
 from .conftest import normalize
-from pydantic import JsonValue, TypeAdapter, ValidationError
 
 # ---------------------------------------------------------------------------
 # _is_remote_module_url
@@ -3499,6 +3503,16 @@ def test_ProxyConfig__encrypt_env_variables_for_db_idempotent(monkeypatch):
     assert out == {"A": "ENC[1]", "B": "ENC[2]", "C": "ENC[3]"}
 
 
+def test_ProxyConfig__encrypt_env_variables_for_db_refuses_without_pynacl_instead_of_dropping_legacy_values(
+    monkeypatch,
+):
+    monkeypatch.setitem(sys.modules, "nacl", None)
+    monkeypatch.setitem(sys.modules, "nacl.secret", None)
+    pc = ProxyConfig()
+    with pytest.raises(LegacyEncryptionUnavailableError, match="config save.*legacy-encryption"):
+        pc._encrypt_env_variables_for_db({"A": "1"})
+
+
 def test_ProxyConfig__encrypt_env_variables_for_db_invalid_raises():
     pc = ProxyConfig()
     with pytest.raises(AttributeError):