mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
8112fbf274
commit
92a07e2d6e
3 changed files with 35 additions and 16 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue