From 20dea51f28398efb328d4f7a24a277abd395fce8 Mon Sep 17 00:00:00 2001 From: yucheng Date: Mon, 5 Oct 2026 08:33:46 +0000 Subject: [PATCH] fix(proxy): drop unreadable legacy credential values instead of serving the ciphertext and restore the non-root image from main Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- docker/Dockerfile.non_root | 38 ++++++------------- litellm/proxy/_experimental/mcp_server/db.py | 8 ++-- .../common_utils/credential_hydration.py | 22 +++++++---- .../common_utils/encrypt_decrypt_utils.py | 6 +++ .../credential_migration.py | 14 +++---- .../key_management_endpoints.py | 6 +-- litellm/proxy/proxy_server.py | 8 +--- .../common_utils/test_credential_hydration.py | 32 ++++++++++++++++ .../test_credential_migration.py | 4 +- .../proxy/proxy_server/test_proxy_config.py | 22 +++++++++++ 10 files changed, 104 insertions(+), 56 deletions(-) diff --git a/docker/Dockerfile.non_root b/docker/Dockerfile.non_root index 364a763c7fa..6bf74c294fc 100644 --- a/docker/Dockerfile.non_root +++ b/docker/Dockerfile.non_root @@ -3,7 +3,6 @@ # Base images ARG LITELLM_BUILD_IMAGE=cgr.dev/chainguard/wolfi-base@sha256:1d95114038f76513a9ace6fca107d5582b08c65981f81f61cb56bf7fd2ef216d ARG LITELLM_RUNTIME_IMAGE=cgr.dev/chainguard/wolfi-base@sha256:1d95114038f76513a9ace6fca107d5582b08c65981f81f61cb56bf7fd2ef216d -ARG PROXY_EXTRAS_SOURCE=published ARG UV_IMAGE=ghcr.io/astral-sh/uv:0.11.7@sha256:240fb85ab0f263ef12f492d8476aa3a2e4e1e333f7d67fbdd923d00a506a516a # Pinned by digest like the other base images; bump explicitly on Node upgrades. ARG UI_BUILD_IMAGE=node:24.19-alpine3.24@sha256:d32cdf619f63fe0471182d08996dd516c6275bb5fd31ae06e55a570bd9e1ad43 @@ -44,7 +43,6 @@ COPY ui/litellm-dashboard/ ./ RUN npm run build FROM $LITELLM_BUILD_IMAGE AS builder -ARG PROXY_EXTRAS_SOURCE WORKDIR /app USER root @@ -104,32 +102,19 @@ ENV LITELLM_NON_ROOT=true RUN mkdir -p /var/lib/litellm/ui /var/lib/litellm/assets && \ cp -r /app/litellm/proxy/_experimental/out/. /var/lib/litellm/ui/ && \ - cp /app/litellm/proxy/logo.jpg /var/lib/litellm/assets/logo.jpg && \ + cp /app/litellm/proxy/logo.png /var/lib/litellm/assets/logo.png && \ touch /var/lib/litellm/ui/.litellm_ui_ready RUN --mount=type=cache,target=/app/.cache/uv,id=litellm-uv-cache \ - if [ "$PROXY_EXTRAS_SOURCE" = "published" ]; then \ - uv sync --frozen --no-default-groups --no-editable \ - --extra proxy \ - --extra legacy-encryption \ - --extra proxy-runtime \ - --extra extra_proxy \ - --extra semantic-router \ - --extra saml \ - --extra bedrock-realtime \ - --python python3.13 \ - --no-sources-package litellm-proxy-extras; \ - else \ - uv sync --frozen --no-default-groups --no-editable \ - --extra proxy \ - --extra legacy-encryption \ - --extra proxy-runtime \ - --extra extra_proxy \ - --extra semantic-router \ - --extra saml \ - --extra bedrock-realtime \ - --python python3.13; \ - fi + uv sync --frozen --no-default-groups --no-editable \ + --extra proxy \ + --extra legacy-encryption \ + --extra proxy-runtime \ + --extra extra_proxy \ + --extra semantic-router \ + --extra saml \ + --extra bedrock-realtime \ + --python python3.13 RUN HOME=/opt/prisma XDG_CACHE_HOME=/opt/prisma/.cache PRISMA_BINARY_CACHE_DIR=/opt/prisma/binaries \ npm_config_cache=/root/.npm \ @@ -139,7 +124,8 @@ RUN sed -i 's/\r$//' docker/entrypoint.sh && chmod +x docker/entrypoint.sh && \ sed -i 's/\r$//' docker/prod_entrypoint.sh && chmod +x docker/prod_entrypoint.sh FROM $LITELLM_RUNTIME_IMAGE AS runtime -ARG PROXY_EXTRAS_SOURCE +ARG LITELLM_RELEASE_TAG="" +ENV LITELLM_RELEASE_TAG=${LITELLM_RELEASE_TAG} WORKDIR /app USER root diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index 5b8469d4209..503a0c722f8 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -180,7 +180,7 @@ def _drop_stale_minted_on_client_rotation(merged: dict[str, object], new_creds: } -def _is_global_env_var_scope(scope: object) -> bool: +def is_global_env_var_scope(scope: object) -> bool: """``scope="user"`` entries are placeholders the user fills in; everything else (including a missing scope) is an admin-supplied global value.""" return scope != MCPEnvVarScope.user and scope != "user" @@ -195,7 +195,7 @@ def _encrypt_global_env_var_values(env_vars: Iterable[dict[str, str]]) -> None: secrets and are stored verbatim. """ for entry in env_vars: - if not _is_global_env_var_scope(entry.get("scope")): + if not is_global_env_var_scope(entry.get("scope")): continue value = entry.get("value") if value: @@ -216,7 +216,7 @@ def decrypt_global_env_var_values(env_vars: Iterable[MCPEnvVar | dict[str, str]] for entry in env_vars: is_dict = isinstance(entry, dict) scope = entry.get("scope") if is_dict else getattr(entry, "scope", None) - if not _is_global_env_var_scope(scope): + if not is_global_env_var_scope(scope): continue value = entry.get("value") if is_dict else getattr(entry, "value", None) if not value: @@ -296,7 +296,7 @@ def _reencrypt_global_env_var_values( rebuilt: Final = [dict(v) for v in entries] rotated = False for entry in rebuilt: - if not _is_global_env_var_scope(entry.get("scope")): + if not is_global_env_var_scope(entry.get("scope")): continue value = entry.get("value") if not value: diff --git a/litellm/proxy/common_utils/credential_hydration.py b/litellm/proxy/common_utils/credential_hydration.py index 2aabe8cad5c..7f4819b49fb 100644 --- a/litellm/proxy/common_utils/credential_hydration.py +++ b/litellm/proxy/common_utils/credential_hydration.py @@ -12,7 +12,7 @@ from types import MappingProxyType from typing import Final import litellm -from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper +from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper, legacy_unreadable from litellm.proxy.utils import PrismaClient from litellm.repositories.credentials_repository import CredentialsRepository from litellm.router_utils.clientside_credential_handler import clientside_credential_keys @@ -58,20 +58,26 @@ def stored_credential_provider(credential_provider: object) -> str | None: return lowered if lowered in _LITELLM_PROVIDER_IDS else None -def decrypted_or_stored(key: str, value: str) -> str: - """The stored value decrypted, or as stored when it was never encrypted (a config.yaml value).""" +def decrypted_or_stored(key: str, value: str) -> str | None: + """The stored value decrypted, as stored when it was never encrypted (a config.yaml value), or None when it + is legacy ciphertext this install has no PyNaCl to read: a credential, never the ciphertext blob.""" decrypted: Final = decrypt_value_helper(value=value, key=key) - return value if decrypted is None else decrypted + if decrypted is not None: + return decrypted + return None if legacy_unreadable(value) else value + + +def decrypted_values(values: Mapping[str, str]) -> Mapping[str, str]: + """``values`` with every entry decrypted, plaintext entries kept, and unreadable legacy entries dropped.""" + resolved: Final = {key: decrypted_or_stored(key, value) for key, value in values.items()} + return MappingProxyType({key: value for key, value in resolved.items() if value is not None}) def _decrypted(db_credential: CredentialItem) -> CredentialItem: """The stored credential with every value decrypted, leaving already-plaintext values alone.""" - decrypted_values: Final = MappingProxyType( - {key: decrypted_or_stored(key, value) for key, value in db_credential.credential_values.items()} - ) return CredentialItem( credential_name=db_credential.credential_name, - credential_values=decrypted_values, # pyright: ignore[reportArgumentType] # declared dict[str, str], and pydantic copies this mapping into one on validation; LIT002 rules out building that dict here + credential_values=decrypted_values(db_credential.credential_values), # pyright: ignore[reportArgumentType] # declared dict[str, str], and pydantic copies this mapping into one on validation; LIT002 rules out building that dict here credential_info=db_credential.credential_info, ) diff --git a/litellm/proxy/common_utils/encrypt_decrypt_utils.py b/litellm/proxy/common_utils/encrypt_decrypt_utils.py index 1472e380010..cb2be0e672d 100644 --- a/litellm/proxy/common_utils/encrypt_decrypt_utils.py +++ b/litellm/proxy/common_utils/encrypt_decrypt_utils.py @@ -263,6 +263,12 @@ def needs_legacy_reader(value: object) -> bool: return isinstance(value, str) and value != "" and not is_versioned_gcm(value) +def legacy_unreadable(value: object) -> bool: + """True for a stored value that only PyNaCl could read and PyNaCl is missing: such a value must read as unset, + never as a plaintext credential.""" + return needs_legacy_reader(value) and not legacy_encryption_available() + + 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 diff --git a/litellm/proxy/management_endpoints/credential_migration.py b/litellm/proxy/management_endpoints/credential_migration.py index dc7f5699982..05d0de23a13 100644 --- a/litellm/proxy/management_endpoints/credential_migration.py +++ b/litellm/proxy/management_endpoints/credential_migration.py @@ -495,7 +495,7 @@ def _mcp_encrypted_leaves(col: str, raw: object) -> Iterator[str]: """Only the strings an MCP column encrypts at rest: a credentials blob keeps auth_type, scopes and urls in plaintext and env_vars keeps every name and every per user placeholder, so without PyNaCl those must not be mistaken for legacy ciphertext and refuse the scan""" - from litellm.proxy._experimental.mcp_server.db import MCP_CREDENTIAL_SECRET_FIELDS, _is_global_env_var_scope + from litellm.proxy._experimental.mcp_server.db import MCP_CREDENTIAL_SECRET_FIELDS, is_global_env_var_scope if col == "credentials": if not isinstance(raw, dict): @@ -506,7 +506,7 @@ def _mcp_encrypted_leaves(col: str, raw: object) -> Iterator[str]: return ( e["value"] for e in raw - if isinstance(e, dict) and _is_global_env_var_scope(e.get("scope")) and isinstance(e.get("value"), str) + if isinstance(e, dict) and is_global_env_var_scope(e.get("scope")) and isinstance(e.get("value"), str) ) @@ -609,7 +609,7 @@ async def _scan_config_env_vars(prisma_client: object) -> LocationReport: return report -async def _scan_covered_tables(prisma_client: object) -> list[LocationReport]: +async def scan_covered_tables(prisma_client: object) -> list[LocationReport]: """Read-only classification of every rotation-covered table. No writes.""" reports: Final[list[LocationReport]] = [] for location, db_attr, json_cols, scalar_cols in _COVERED_TABLE_SPECS: @@ -642,7 +642,7 @@ async def _migrate_covered_tables(prisma_client: object, user_api_key_dict: obje _rotate_master_key, ) - pre: Final = {r.location: r for r in await _scan_covered_tables(prisma_client)} + pre: Final = {r.location: r for r in await scan_covered_tables(prisma_client)} current_key: Final = _get_salt_key() if current_key is None: @@ -656,7 +656,7 @@ async def _migrate_covered_tables(prisma_client: object, user_api_key_dict: obje new_master_key=current_key, # same key, algorithm-only switch ) - post: Final = await _scan_covered_tables(prisma_client) + post: Final = await scan_covered_tables(prisma_client) for post_report in post: pre_report = pre.get(post_report.location) pre_legacy = pre_report.legacy if pre_report else 0 @@ -689,7 +689,7 @@ async def migrate_encryption( # delegate to the rotation path (with bracketing scans for counts); on a dry # run only classify them read-only. if dry_run: - for covered in await _scan_covered_tables(prisma_client): + for covered in await scan_covered_tables(prisma_client): report.add(covered) else: for covered in await _migrate_covered_tables(prisma_client, user_api_key_dict): @@ -717,7 +717,7 @@ async def check_encryption(prisma_client: object) -> MigrationReport: report: Final = MigrationReport() # Rotation-covered tables (read-only classification). - for covered in await _scan_covered_tables(prisma_client): + for covered in await scan_covered_tables(prisma_client): report.add(covered) # Net-new walker locations, in dry-run (read-only) mode. diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 5adb0f96d0d..531c8a6969c 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -5222,9 +5222,9 @@ async def _require_legacy_reader_for_stored_values(prisma_client: PrismaClient) 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 + from litellm.proxy.management_endpoints.credential_migration import scan_covered_tables - await _scan_covered_tables(prisma_client) + await scan_covered_tables(prisma_client) async def _rotate_master_key( @@ -5255,7 +5255,7 @@ async def _rotate_master_key( except LegacyEncryptionUnavailableError as error: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail={"error": str(error)}, # mutable-ok: FastAPI detail contract + detail={"error": str(error)}, ) from error try: diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index df6f478ec45..fef11f521a4 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -413,7 +413,7 @@ from litellm.proxy.common_utils.callback_utils import initialize_callbacks_on_pr from litellm.proxy.common_utils.codex_model_catalog import codex_model_list_body from litellm.proxy.common_utils.config_includes import resolve_include_file_path, resolve_includes from litellm.proxy.common_utils.config_sync_pubsub import ConfigSyncSubscriber -from litellm.proxy.common_utils.credential_hydration import decrypted_or_stored +from litellm.proxy.common_utils.credential_hydration import decrypted_values from litellm.proxy.common_utils.debug_utils import init_verbose_loggers from litellm.proxy.common_utils.debug_utils import router as debugging_endpoints_router from litellm.proxy.common_utils.discoverable_model_filter import discoverable_rows, undiscoverable_model_names @@ -9155,11 +9155,7 @@ class ProxyConfig: elif isinstance(credential, BaseModel): credential_object = CredentialItem(**credential.model_dump()) - decrypted_credential_values: Final = {} - for k, v in credential_object.credential_values.items(): - decrypted_credential_values[k] = decrypted_or_stored(k, v) - - credential_object.credential_values = decrypted_credential_values + credential_object.credential_values = dict(decrypted_values(credential_object.credential_values)) return credential_object async def delete_credentials(self, db_credentials: list[CredentialItem]): diff --git a/tests/unit/proxy/common_utils/test_credential_hydration.py b/tests/unit/proxy/common_utils/test_credential_hydration.py index f40b0114d71..5a811c0cd6c 100644 --- a/tests/unit/proxy/common_utils/test_credential_hydration.py +++ b/tests/unit/proxy/common_utils/test_credential_hydration.py @@ -1,3 +1,6 @@ +import base64 +import hashlib +import sys from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -26,3 +29,32 @@ async def test_authoritative_hydrate_returns_an_encrypted_empty_value_as_empty(m assert resolved is not None assert resolved.credential_values == {"api_base": "", "openai_service_account_id": "user-1"} + + +def _legacy_nacl_ciphertext(plaintext: str, salt_key: str) -> str: + import nacl.secret + + box = nacl.secret.SecretBox(hashlib.sha256(salt_key.encode()).digest()) + return base64.urlsafe_b64encode(bytes(box.encrypt(plaintext.encode()))).decode() + + +@pytest.mark.asyncio +async def test_authoritative_hydrate_without_pynacl_drops_a_legacy_value_instead_of_serving_the_blob(monkeypatch): + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-hydration-test-salt") + legacy = _legacy_nacl_ciphertext("sk-legacy-upstream", "sk-hydration-test-salt") + row = { + "credential_name": "openai-legacy", + "credential_values": {"api_key": legacy, "api_base": encrypt_value_helper("https://api.example.test")}, + "credential_info": {"custom_llm_provider": "openai"}, + } + prisma = MagicMock() + prisma.db.litellm_credentialstable.find_unique = AsyncMock(return_value=row) + monkeypatch.setitem(sys.modules, "nacl", None) + monkeypatch.setitem(sys.modules, "nacl.secret", None) + + with patch.object(litellm, "credential_list", []): # test-quality-ok: the row under test must win over memory + resolved = await hydrate_named_credential_authoritative("openai-legacy", prisma) + + assert resolved is not None + assert resolved.credential_values == {"api_base": "https://api.example.test"} + assert legacy not in resolved.credential_values.values() diff --git a/tests/unit/proxy/management_endpoints/test_credential_migration.py b/tests/unit/proxy/management_endpoints/test_credential_migration.py index 2e9d09c4ea9..070556d83b2 100644 --- a/tests/unit/proxy/management_endpoints/test_credential_migration.py +++ b/tests/unit/proxy/management_endpoints/test_credential_migration.py @@ -562,7 +562,7 @@ async def test_scan_covered_tables_classifies_legacy_and_v2(salt_key, monkeypatc ) client.db.litellm_config.find_unique = AsyncMock(return_value=None) - by_loc = {r.location: r for r in await cm._scan_covered_tables(client)} + by_loc = {r.location: r for r in await cm.scan_covered_tables(client)} assert by_loc["model_table"].legacy == 1 assert by_loc["model_table"].plaintext == 1 # "gpt-4" model name, not ciphertext @@ -586,7 +586,7 @@ async def test_scan_covered_tables_classifies_search_tool_params(salt_key, monke ) client.db.litellm_config.find_unique = AsyncMock(return_value=None) - by_loc = {r.location: r for r in await cm._scan_covered_tables(client)} + by_loc = {r.location: r for r in await cm.scan_covered_tables(client)} assert (by_loc["search_tools"].legacy, by_loc["search_tools"].already_v2) == (1, 1) assert by_loc["search_tools"].plaintext == 1 diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index 6986ff0fb8d..d6e3fa8b065 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -3348,6 +3348,28 @@ def test_ProxyConfig_decrypt_credentials_returns_an_encrypted_empty_value_as_emp assert decrypted.credential_values == {"api_base": "", "openai_service_account_id": "user-1"} +def test_ProxyConfig_decrypt_credentials_without_pynacl_drops_a_legacy_value_instead_of_serving_the_blob(monkeypatch): + import base64 + import hashlib + + import nacl.secret + + monkeypatch.setenv("LITELLM_SALT_KEY", "sk-decrypt-credentials-test-salt") + box = nacl.secret.SecretBox(hashlib.sha256(b"sk-decrypt-credentials-test-salt").digest()) + legacy = base64.urlsafe_b64encode(bytes(box.encrypt(b"sk-legacy-upstream"))).decode() + monkeypatch.setitem(sys.modules, "nacl", None) + monkeypatch.setitem(sys.modules, "nacl.secret", None) + + decrypted = ProxyConfig().decrypt_credentials( + { + "credential_name": "openai-legacy", + "credential_values": {"api_key": legacy, "api_base": encrypt_value_helper("https://api.example.test")}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + assert decrypted.credential_values == {"api_base": "https://api.example.test"} + + def test_ProxyConfig_decrypt_model_list_from_db_returns_decrypted(monkeypatch): monkeypatch.setattr( "litellm.proxy.proxy_server.decrypt_value_helper",