fix(security): address ROI CodeQL alerts

Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>

litellm/proxy/common_utils/encrypt_decrypt_utils.py

@@ -111,7 +111,7 @@ def encrypt_value_helper(value: str, new_encryption_key: str | None = None):
             return encrypted_value
 
         verbose_proxy_logger.debug(
-            "Invalid value type passed to encrypt_value: %s for Value: %s\n Value must be a string", type(value), value
+            "Invalid value type passed to encrypt_value: %s. Value must be a string", type(value)
         )
         # if it's not a string - do not encrypt it and return the value
         return value

litellm/proxy/roi_calculator/analytics.py

@@ -14,11 +14,12 @@ from litellm.types.roi_calculator import (
 )
 
 _EMAIL_PATTERN: Final = re.compile(r"[^\s@]+@[^\s@]+\.[^\s@]+")
+_NOREPLY_GITHUB_SUFFIX: Final = re.compile(r"noreply\.github\.com\Z")
 
 
 def normalize_email(value: str | None) -> str:
     normalized: Final = (value or "").strip().casefold()
-    if _EMAIL_PATTERN.fullmatch(normalized) is None or normalized.endswith("noreply.github.com"):
+    if _EMAIL_PATTERN.fullmatch(normalized) is None or _NOREPLY_GITHUB_SUFFIX.search(normalized) is not None:
         return ""
     return normalized
 

litellm/proxy/roi_calculator/sync.py

@@ -1,5 +1,6 @@
 import asyncio
 from collections.abc import Awaitable, Mapping, Sequence
+from contextlib import suppress
 from datetime import date, datetime, timedelta, timezone
 from itertools import chain
 from types import MappingProxyType
@@ -258,10 +259,8 @@ class SyncManager:
         if task is None or task.done():
             return False
         task.cancel()
-        try:
+        with suppress(asyncio.CancelledError):
             await task
-        except asyncio.CancelledError:
-            pass
         self._update_status(running=False, phase="cancelled", stage="Sync cancelled")
         return True
 

tests/unit/proxy/common_utils/test_proxy_encrypt_decrypt.py

@@ -1,11 +1,11 @@
+import logging
 import os
+from typing import Final, cast
 
 import pytest
 from dotenv import load_dotenv
 
 load_dotenv()
-import io
-
 
 from litellm.proxy import proxy_server
 from litellm.proxy.common_utils.encrypt_decrypt_utils import (
@@ -20,9 +20,7 @@ def test_encrypt_decrypt_with_master_key():
     assert decrypt_value_helper(encrypt_value_helper(10), key="test_key") == 10
     assert decrypt_value_helper(encrypt_value_helper(True), key="test_key") is True
     assert decrypt_value_helper(encrypt_value_helper(None), key="test_key") is None
-    assert decrypt_value_helper(encrypt_value_helper({"rpm": 10}), key="test_key") == {
-        "rpm": 10
-    }
+    assert decrypt_value_helper(encrypt_value_helper({"rpm": 10}), key="test_key") == {"rpm": 10}
 
     # encryption should actually occur for strings
     assert encrypt_value_helper("test") != "test"
@@ -30,16 +28,25 @@ def test_encrypt_decrypt_with_master_key():
 
 def test_encrypt_decrypt_with_salt_key():
     os.environ["LITELLM_SALT_KEY"] = "sk-salt-key2222"
-    print(f"LITELLM_SALT_KEY: {os.environ['LITELLM_SALT_KEY']}")
     assert decrypt_value_helper(encrypt_value_helper("test"), key="test_key") == "test"
     assert decrypt_value_helper(encrypt_value_helper(10), key="test_key") == 10
     assert decrypt_value_helper(encrypt_value_helper(True), key="test_key") is True
     assert decrypt_value_helper(encrypt_value_helper(None), key="test_key") is None
-    assert decrypt_value_helper(encrypt_value_helper({"rpm": 10}), key="test_key") == {
-        "rpm": 10
-    }
+    assert decrypt_value_helper(encrypt_value_helper({"rpm": 10}), key="test_key") == {"rpm": 10}
 
     # encryption should actually occur for strings
     assert encrypt_value_helper("test") != "test"
 
     os.environ.pop("LITELLM_SALT_KEY", None)
+
+
+def test_encrypt_value_helper_does_not_log_invalid_value(caplog: pytest.LogCaptureFixture) -> None:
+    caplog.set_level(logging.DEBUG, logger="LiteLLM Proxy")
+    secret: Final[str] = "must-not-be-logged"
+    value: Final[dict[str, str]] = {"token": secret}
+
+    encrypt_value_helper(cast(str, value))
+
+    messages: Final = tuple(record.getMessage() for record in caplog.records)
+    assert any("Invalid value type passed to encrypt_value" in message for message in messages)
+    assert all(secret not in message for message in messages)