mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-06 16:24:46 +00:00
fix(audit): close NameError + mypy + asyncio-import nits
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.
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user