From 494f423c9e2a73994ff1ed0e82a049542bdffb3c Mon Sep 17 00:00:00 2001 From: yucheng Date: Thu, 24 Sep 2026 11:59:12 +0000 Subject: [PATCH] fix(proxy): require PyNaCl only when an unprefixed stored value must be read Config save, migration, scan and master key rotation now pass on plaintext or v3 only stores without PyNaCl and refuse only when a non empty unprefixed value is met Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../common_utils/encrypt_decrypt_utils.py | 23 ++++++++++++------- .../credential_migration.py | 6 ++--- .../key_management_endpoints.py | 14 +++++++++-- litellm/proxy/proxy_server.py | 6 +++-- .../test_encrypt_decrypt_utils.py | 14 +++++++---- .../test_credential_migration.py | 22 ++++++++++++++---- .../test_key_management_endpoints.py | 22 ++++++++++++++++-- .../proxy/proxy_server/test_proxy_config.py | 9 +++++++- 8 files changed, 89 insertions(+), 27 deletions(-) diff --git a/litellm/proxy/common_utils/encrypt_decrypt_utils.py b/litellm/proxy/common_utils/encrypt_decrypt_utils.py index 444a8590ef4..8b32b7348b7 100644 --- a/litellm/proxy/common_utils/encrypt_decrypt_utils.py +++ b/litellm/proxy/common_utils/encrypt_decrypt_utils.py @@ -1,7 +1,7 @@ import base64 import hashlib import os -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from typing import TYPE_CHECKING, Final, Literal, cast from pydantic import TypeAdapter, ValidationError @@ -221,13 +221,20 @@ 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 needs_legacy_reader(value: object) -> bool: + """True for a stored string that only PyNaCl can tell apart from plaintext: non empty and unprefixed.""" + return isinstance(value, str) and value != "" and not is_versioned_gcm(value) + + +def require_legacy_reader_for(values: Iterable[object], purpose: str) -> None: + """Refuse a decrypt-then-rewrite pass when PyNaCl is missing and one of the values is unprefixed: it would + read as unreadable and be dropped, double wrapped or miscounted as plaintext. Versioned gcm and non string + values never need PyNaCl, so a fully migrated store passes.""" + if legacy_encryption_available() or not any(needs_legacy_reader(value) for value in values): + return + 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": diff --git a/litellm/proxy/management_endpoints/credential_migration.py b/litellm/proxy/management_endpoints/credential_migration.py index 0cbdb4f0a78..2e98364c7db 100644 --- a/litellm/proxy/management_endpoints/credential_migration.py +++ b/litellm/proxy/management_endpoints/credential_migration.py @@ -49,7 +49,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( decrypt_value_helper, encrypt_value_helper, is_versioned_gcm, - require_legacy_reader, + require_legacy_reader_for, ) ValueClass = Literal["migrated", "legacy", "plaintext", "undecryptable", "not-a-string"] @@ -148,6 +148,7 @@ def classify_value(value: object, key: str = "scan") -> ValueClass: return "plaintext" if is_versioned_gcm(value): return "migrated" + require_legacy_reader_for((value,), "scan stored encryption") decrypted: Final = decrypt_value_helper(value=value, key=key, exception_type="debug", return_original_value=False) if decrypted is None: # Did not decrypt under nacl and has no v2 marker: legacy plaintext. @@ -167,6 +168,7 @@ def reencrypt_value(value: object, key: str = "migrate") -> object: return value if is_versioned_gcm(value): return value # idempotent: already migrated + require_legacy_reader_for((value,), "migrate stored encryption") decrypted: Final = decrypt_value_helper(value=value, key=key, exception_type="debug", return_original_value=False) if decrypted is None: # Either legacy plaintext (no ciphertext to migrate) or corrupt. Either @@ -634,7 +636,6 @@ async def migrate_encryption( net-new walkers run in dry-run mode. """ _assert_aes_gate_enabled() - require_legacy_reader("migrate stored encryption") report: Final = MigrationReport() @@ -667,7 +668,6 @@ 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. """ - require_legacy_reader("scan stored encryption") report: Final = MigrationReport() # Rotation-covered tables (read-only classification). diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 68a2c3fa7d7..6cc45489fa3 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -81,7 +81,7 @@ from litellm.proxy.common_utils.config_sync_pubsub import ( ) from litellm.proxy.common_utils.encrypt_decrypt_utils import ( LegacyEncryptionUnavailableError, - require_legacy_reader, + legacy_encryption_available, ) 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 @@ -5156,6 +5156,16 @@ async def delete_key_aliases( ) +async def _require_legacy_reader_for_stored_values(prisma_client: PrismaClient) -> None: + """Without PyNaCl, scan the rotation covered tables read only so an unprefixed stored value refuses the + rotation before any row is rewritten. A fully migrated store passes; with PyNaCl installed nothing runs""" + if legacy_encryption_available(): + return + from litellm.proxy.management_endpoints.credential_migration import _scan_covered_tables + + await _scan_covered_tables(prisma_client) + + async def _rotate_master_key( prisma_client: PrismaClient, user_api_key_dict: UserAPIKeyAuth, @@ -5180,7 +5190,7 @@ async def _rotate_master_key( from litellm.proxy.proxy_server import proxy_config try: - require_legacy_reader("rotate the master key") + await _require_legacy_reader_for_stored_values(prisma_client) except LegacyEncryptionUnavailableError as error: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index e2524148597..b460b1bff91 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -403,7 +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, + require_legacy_reader_for, ) 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 ( @@ -7326,7 +7326,9 @@ 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") + require_legacy_reader_for( + environment_variables.values(), "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, diff --git a/tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py b/tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py index cf9ad5b1c1e..229f46630f9 100644 --- a/tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py +++ b/tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py @@ -29,7 +29,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import ( encrypt_value_helper, is_versioned_gcm, legacy_encryption_available, - require_legacy_reader, + require_legacy_reader_for, ) from litellm.proxy.common_utils.fips import FipsModeError @@ -198,13 +198,17 @@ def test_legacy_ciphertext_without_pynacl_logs_the_reencrypt_path_and_never_retu 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") +def test_require_legacy_reader_for_refuses_only_unprefixed_values_without_pynacl(monkeypatch): + legacy = _legacy_nacl_ciphertext("legacy-secret", _sha256_key()) + v3 = encrypt_value_helper("v3-secret") + require_legacy_reader_for((legacy, v3, "plain"), "rotate the master key") monkeypatch.setitem(sys.modules, "nacl", None) monkeypatch.setitem(sys.modules, "nacl.secret", None) - with pytest.raises(LegacyEncryptionUnavailableError, match=r"rotate the master key.*legacy-encryption"): - require_legacy_reader("rotate the master key") + require_legacy_reader_for((v3, "", None, 3), "rotate the master key") + for unreadable in (legacy, "plain-or-legacy"): + with pytest.raises(LegacyEncryptionUnavailableError, match=r"rotate the master key.*legacy-encryption"): + require_legacy_reader_for((v3, unreadable), "rotate the master key") def test_legacy_opt_in_without_pynacl_fails_the_write_not_silently(monkeypatch): diff --git a/tests/test_litellm/proxy/management_endpoints/test_credential_migration.py b/tests/test_litellm/proxy/management_endpoints/test_credential_migration.py index 4c4ee2e95de..eb00462141e 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_credential_migration.py +++ b/tests/test_litellm/proxy/management_endpoints/test_credential_migration.py @@ -130,17 +130,31 @@ async def test_migrate_requires_aes_gate(salt_key, monkeypatch): @pytest.mark.asyncio async def test_migrate_and_check_refuse_without_pynacl_instead_of_miscounting_legacy_rows(salt_key, monkeypatch): + legacy: Final = _legacy_ct("model-secret", monkeypatch) + _enable_aes(monkeypatch) monkeypatch.setitem(sys.modules, "nacl", None) monkeypatch.setitem(sys.modules, "nacl.secret", None) client: Final = MagicMock() _empty_covered_tables(client) + client.db.litellm_teamtable.find_many = AsyncMock(return_value=[]) + client.db.litellm_verificationtoken.find_many = AsyncMock(return_value=[]) + client.db.litellm_ssoconfig.find_unique = AsyncMock(return_value=None) + client.db.litellm_config.find_unique = AsyncMock(return_value=None) + client.db.litellm_config.update = AsyncMock() - with pytest.raises(RuntimeError, match="legacy-encryption"): - await cm.migrate_encryption(prisma_client=client, user_api_key_dict=MagicMock()) + v3_only: Final = await cm.check_encryption(prisma_client=client) + assert v3_only.residual_legacy == 0 + + client.db.litellm_proxymodeltable.find_many = AsyncMock( + return_value=[SimpleNamespace(litellm_params={"api_key": legacy})] + ) + client.db.litellm_proxymodeltable.update_many = AsyncMock() with pytest.raises(RuntimeError, match="legacy-encryption"): await cm.check_encryption(prisma_client=client) - for _, db_attr, _, _ in cm._COVERED_TABLE_SPECS: - getattr(client.db, db_attr).find_many.assert_not_awaited() + with pytest.raises(RuntimeError, match="legacy-encryption"): + await cm.migrate_encryption(prisma_client=client, user_api_key_dict=MagicMock()) + client.db.litellm_proxymodeltable.update_many.assert_not_awaited() + client.db.litellm_config.update.assert_not_awaited() # --------------------------- config-row walker --------------------------- diff --git a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py index 0738441c59c..46602c10888 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py @@ -9437,20 +9437,37 @@ async def test_rotate_master_key_reencrypts_model_params_in_place( async def test_rotate_master_key_refuses_without_pynacl_before_touching_any_row(monkeypatch): import sys + from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock from fastapi import HTTPException + from litellm.proxy import proxy_server from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper from litellm.proxy.management_endpoints.key_management_endpoints import ( _rotate_master_key, ) + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-salt-rotate") + monkeypatch.setattr(proxy_server, "general_settings", {"encryption_algorithm": "xsalsa20-poly1305"}) + legacy_row = SimpleNamespace(litellm_params={"api_key": encrypt_value_helper("legacy-secret")}) + monkeypatch.setattr(proxy_server, "general_settings", {}) 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=[]) + mock_prisma_client.db.litellm_proxymodeltable.find_many = AsyncMock(return_value=[legacy_row]) + mock_prisma_client.db.litellm_proxymodeltable.update_many = AsyncMock() + mock_prisma_client.db.litellm_config.find_unique = AsyncMock(return_value=None) + empty_tables = ( + "litellm_credentialstable", + "litellm_mcpservertable", + "litellm_mcpusercredentials", + "litellm_mcpuserenvvars", + ) + for table in empty_tables: + getattr(mock_prisma_client.db, table).find_many = AsyncMock(return_value=[]) with pytest.raises(HTTPException) as exc_info: await _rotate_master_key( @@ -9462,7 +9479,8 @@ async def test_rotate_master_key_refuses_without_pynacl_before_touching_any_row( 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() + mock_prisma_client.db.litellm_proxymodeltable.update_many.assert_not_awaited() + mock_prisma_client.db.tx.assert_not_called() async def test_default_key_generate_params_duration(monkeypatch): diff --git a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py index b50dbc17e21..ac4993a4327 100644 --- a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py @@ -29,6 +29,7 @@ import litellm from litellm.proxy._types import CommonProxyErrors from litellm.proxy.common_utils.encrypt_decrypt_utils import ( LegacyEncryptionUnavailableError, + decrypt_value_helper, encrypt_value_helper, ) from litellm.proxy.proxy_server import ( @@ -3506,11 +3507,17 @@ def test_ProxyConfig__encrypt_env_variables_for_db_idempotent(monkeypatch): def test_ProxyConfig__encrypt_env_variables_for_db_refuses_without_pynacl_instead_of_dropping_legacy_values( monkeypatch, ): + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-salt-config-save") + monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {}) + v3 = encrypt_value_helper("already-migrated") monkeypatch.setitem(sys.modules, "nacl", None) monkeypatch.setitem(sys.modules, "nacl.secret", None) pc = ProxyConfig() + + saved = pc._encrypt_env_variables_for_db({"A": v3}) + assert decrypt_value_helper(saved["A"], key="A") == "already-migrated" with pytest.raises(LegacyEncryptionUnavailableError, match=r"config save.*legacy-encryption"): - pc._encrypt_env_variables_for_db({"A": "1"}) + pc._encrypt_env_variables_for_db({"A": v3, "B": "plain-or-legacy"}) def test_ProxyConfig__encrypt_env_variables_for_db_invalid_raises():