From 92a07e2d6e2850bef28f8c7859d6c7632cfb3c45 Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Thu, 26 Mar 2026 16:00:26 +0530 Subject: [PATCH] fix(proxy): address Greptile review feedback - Remove HTTP_PROXY/HTTPS_PROXY from blocklist (legitimately used in corporate envs) - Add NO_PROXY/no_proxy to blocklist (prevents bypassing proxy monitoring) - Remove dead code in _is_valid_user_id (space exception was unreachable) - Update tests accordingly Co-Authored-By: Claude Opus 4.6 --- .../internal_user_endpoints.py | 4 +- litellm/proxy/proxy_server.py | 6 +-- tests/test_litellm/proxy/test_proxy_server.py | 41 ++++++++++++++----- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index b2df9a1a2cb..ff7b89fe192 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -562,9 +562,9 @@ def _is_valid_user_id(user_id: str) -> bool: MAX_USER_ID_LENGTH = 512 if len(user_id) > MAX_USER_ID_LENGTH: return False - # Reject null bytes and control characters (< 0x20) except space + # Reject ASCII control characters (U+0000–U+001F) for ch in user_id: - if ch == "\x00" or (ord(ch) < 0x20 and ch != " "): + if ord(ch) < 0x20: return False return True diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 203344d452c..6765feebb14 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -2710,10 +2710,8 @@ class ProxyConfig: "USER", "SHELL", "LOGNAME", - "http_proxy", - "https_proxy", - "HTTP_PROXY", - "HTTPS_PROXY", + "NO_PROXY", + "no_proxy", } def _load_environment_variables(self, config: dict): diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 334cd7395d6..d7f1774fc1b 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -1601,9 +1601,10 @@ async def test_load_environment_variables_blocks_dangerous_keys(): @pytest.mark.asyncio -async def test_load_environment_variables_blocks_proxy_keys(): +async def test_load_environment_variables_allows_proxy_keys(): """ - Test that _load_environment_variables rejects proxy-related env var keys. + Test that HTTP_PROXY/HTTPS_PROXY are allowed since they are commonly used + in corporate environments to route outbound API calls. """ from litellm.proxy.proxy_server import ProxyConfig @@ -1611,20 +1612,40 @@ async def test_load_environment_variables_blocks_proxy_keys(): test_config = { "environment_variables": { - "HTTP_PROXY": "http://evil-proxy:8080", - "HTTPS_PROXY": "http://evil-proxy:8080", - "http_proxy": "http://evil-proxy:8080", - "https_proxy": "http://evil-proxy:8080", + "HTTP_PROXY": "http://corp-proxy:8080", + "HTTPS_PROXY": "http://corp-proxy:8080", } } with patch.dict(os.environ, {}, clear=False): proxy_config._load_environment_variables(test_config) - assert os.environ.get("HTTP_PROXY") != "http://evil-proxy:8080" - assert os.environ.get("HTTPS_PROXY") != "http://evil-proxy:8080" - assert os.environ.get("http_proxy") != "http://evil-proxy:8080" - assert os.environ.get("https_proxy") != "http://evil-proxy:8080" + assert os.environ["HTTP_PROXY"] == "http://corp-proxy:8080" + assert os.environ["HTTPS_PROXY"] == "http://corp-proxy:8080" + + +@pytest.mark.asyncio +async def test_load_environment_variables_blocks_no_proxy(): + """ + Test that NO_PROXY/no_proxy are blocked to prevent bypassing proxy-based + network monitoring. + """ + from litellm.proxy.proxy_server import ProxyConfig + + proxy_config = ProxyConfig() + + test_config = { + "environment_variables": { + "NO_PROXY": "internal-service", + "no_proxy": "internal-service", + } + } + + with patch.dict(os.environ, {}, clear=False): + proxy_config._load_environment_variables(test_config) + + assert os.environ.get("NO_PROXY") != "internal-service" + assert os.environ.get("no_proxy") != "internal-service" @pytest.mark.asyncio