From aa1312ef75a2687e5aa676cecb5e758daef44d17 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 01:47:11 +0000 Subject: [PATCH] fix(audit): close NameError + mypy + asyncio-import nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Greptile/CI findings on the prior commit: 1. **P1 (real bug):** ``before_config = existing_decrypted if existing_record is not None else env_values`` would NameError when ``existing_record`` exists but its ``config_value`` is null (a valid nullable DB state) — the upper branch only defines ``existing_decrypted`` when *both* conditions are met, but the ternary only checked the first. Pre-bind ``existing_decrypted: Optional[Dict] = None`` and ``env_values = {}`` above the if/else so both names are always in scope, and key the audit-log decision off ``existing_decrypted is not None`` instead. 2. **mypy lint:** ``action: str`` rejected — the field is typed ``AUDIT_ACTIONS = Literal[...]``. Annotate both helper signatures with ``AUDIT_ACTIONS`` and pre-bind the call-site ternary so mypy infers the literal correctly. 3. **P2:** ``import asyncio`` was at the bottom of the test file. Moved to the stdlib import block at top. --- .../cache_settings_endpoints.py | 5 +++-- .../config_override_endpoints.py | 16 ++++++++++++---- .../test_cache_settings_endpoints.py | 8 ++------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/litellm/proxy/management_endpoints/cache_settings_endpoints.py b/litellm/proxy/management_endpoints/cache_settings_endpoints.py index f643105b9b..55eb321185 100644 --- a/litellm/proxy/management_endpoints/cache_settings_endpoints.py +++ b/litellm/proxy/management_endpoints/cache_settings_endpoints.py @@ -20,6 +20,7 @@ import litellm from litellm._logging import verbose_proxy_logger from litellm._uuid import uuid from litellm.proxy._types import ( + AUDIT_ACTIONS, LiteLLM_AuditLogs, LitellmTableNames, UserAPIKeyAuth, @@ -67,7 +68,7 @@ def _log_audit_task_exception(task: "asyncio.Task[None]") -> None: async def _emit_cache_settings_audit_log( *, - action: str, + action: AUDIT_ACTIONS, before_settings: Optional[Mapping[str, Any]], after_settings: Optional[Mapping[str, Any]], user_api_key_dict: UserAPIKeyAuth, @@ -416,7 +417,7 @@ async def update_cache_settings( before_settings = json.loads(existing_row.cache_settings) except (TypeError, ValueError): before_settings = None - action = "updated" if existing_row is not None else "created" + action: AUDIT_ACTIONS = "updated" if existing_row is not None else "created" # Encrypt sensitive fields (keep redis_type for storage) encrypted_settings = proxy_config._encrypt_env_variables( diff --git a/litellm/proxy/management_endpoints/config_override_endpoints.py b/litellm/proxy/management_endpoints/config_override_endpoints.py index e957e48650..230ed7e425 100644 --- a/litellm/proxy/management_endpoints/config_override_endpoints.py +++ b/litellm/proxy/management_endpoints/config_override_endpoints.py @@ -21,6 +21,7 @@ from litellm._logging import verbose_proxy_logger from litellm.litellm_core_utils.sensitive_data_masker import SensitiveDataMasker from litellm.llms.custom_httpx.http_handler import get_async_httpx_client from litellm.proxy._types import ( + AUDIT_ACTIONS, CommonProxyErrors, KeyManagementSystem, LiteLLM_AuditLogs, @@ -66,7 +67,7 @@ def _log_audit_task_exception(task: "asyncio.Task[None]") -> None: async def _emit_hashicorp_vault_audit_log( *, - action: str, + action: AUDIT_ACTIONS, before_config: Optional[Mapping[str, Any]], after_config: Optional[Mapping[str, Any]], user_api_key_dict: UserAPIKeyAuth, @@ -256,6 +257,8 @@ async def update_hashicorp_vault_config( existing_record = await prisma_client.db.litellm_configoverrides.find_unique( where={"config_type": "hashicorp_vault"} ) + existing_decrypted: Optional[Dict[str, Any]] = None + env_values: Dict[str, Any] = {} if existing_record is not None and existing_record.config_value is not None: existing_data = _parse_config_value(existing_record.config_value) existing_decrypted = proxy_config._decrypt_db_variables(existing_data) @@ -263,7 +266,8 @@ async def update_hashicorp_vault_config( if field not in config_data and existing_decrypted.get(field): config_data[field] = existing_decrypted[field] else: - # No DB record yet — merge from current env vars + # No DB record (or DB record with null config_value) — merge from + # current env vars instead. env_values = _get_current_env_values(HASHICORP_ENV_VAR_MAPPING) for field in HASHICORP_ENV_VAR_MAPPING: if field not in config_data and env_values.get(field): @@ -336,9 +340,13 @@ async def update_hashicorp_vault_config( # Mutating the proxy's KMS config affects every secret retrieval going # forward — emit an audit-log row so the action is traceable even # though the secret_manager_client itself was just swapped under us. - before_config = existing_decrypted if existing_record is not None else env_values + # ``existing_decrypted`` is only set when the DB row had a non-null + # ``config_value`` (the same branch that ran the merge above); + # otherwise fall back to whatever env vars were in scope. + before_config = existing_decrypted if existing_decrypted is not None else env_values + action: AUDIT_ACTIONS = "updated" if existing_decrypted is not None else "created" await _emit_hashicorp_vault_audit_log( - action="updated" if existing_record is not None else "created", + action=action, before_config=before_config, after_config=config_data, user_api_key_dict=user_api_key_dict, 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 849b9a077c..b892c4e556 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 @@ -2,6 +2,8 @@ Unit tests for cache settings management endpoints """ +import asyncio +import json import os import sys from unittest.mock import AsyncMock, MagicMock, patch @@ -12,8 +14,6 @@ sys.path.insert( 0, os.path.abspath("../../../..") ) # Adds the parent directory to the system path -import json - import litellm from litellm.proxy._types import LitellmTableNames, LitellmUserRoles from litellm.proxy.auth.user_api_key_auth import UserAPIKeyAuth @@ -395,7 +395,3 @@ async def test_update_cache_settings_no_audit_when_disabled(monkeypatch): ) assert audit_calls == [] - - -# Need an asyncio import for the eager-task drain pattern above. -import asyncio # noqa: E402