mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix: validate SERVER_ROOT_PATH against injection and handle single-quoted asset paths
Two security/correctness gaps in the UI asset path rewriting:
1. SERVER_ROOT_PATH was injected verbatim into served JS/HTML files at
startup with no sanitization. An operator who controls this env var
could embed JavaScript in static assets, causing stored XSS for all
admin UI users. Fix: add regex validation in get_server_root_path()
that rejects any value not matching ^(/[a-zA-Z0-9_-]+)*$ and raises
ValueError at startup.
2. The /ui/assets/ replacement only handled double-quoted string literals
('"/ui/assets/'). Single-quoted variants ("'/ui/assets/") used in
HTML src= attributes and CSS url() were silently skipped, leaving
broken asset paths under a sub-path deployment. Fix: add a parallel
replacement for single-quoted variants.
Tests added:
- test_single_quoted_ui_asset_paths_rewritten
- test_invalid_server_root_path_raises_value_error (4 malicious paths)
- test_valid_server_root_path_accepted (5 valid paths)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
921b06f7cd
commit
cb481f5615
3 changed files with 85 additions and 1 deletions
|
|
@ -1377,10 +1377,15 @@ try:
|
|||
# equivalents so logo/asset requests include the root path prefix.
|
||||
# Some JS bundles emit absolute paths like "/ui/assets/logos/" which
|
||||
# resolve incorrectly when the app is served under a sub-path.
|
||||
# Both double-quoted and single-quoted variants are handled.
|
||||
modified_content = modified_content.replace(
|
||||
'"/ui/assets/',
|
||||
f'"{server_root_path}/ui/assets/',
|
||||
)
|
||||
modified_content = modified_content.replace(
|
||||
"'/ui/assets/",
|
||||
f"'{server_root_path}/ui/assets/",
|
||||
)
|
||||
|
||||
# Replace the /.well-known/litellm-ui-config with the server root path
|
||||
modified_content = modified_content.replace(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import hashlib
|
|||
import inspect
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import smtplib
|
||||
import sys
|
||||
import threading
|
||||
|
|
@ -5396,14 +5397,27 @@ def get_proxy_base_url() -> Optional[str]:
|
|||
return os.getenv("PROXY_BASE_URL")
|
||||
|
||||
|
||||
_SERVER_ROOT_PATH_PATTERN = re.compile(r"^(/[a-zA-Z0-9_-]+)*$")
|
||||
|
||||
|
||||
def get_server_root_path() -> str:
|
||||
"""
|
||||
Get the server root path from the environment variables.
|
||||
|
||||
- If SERVER_ROOT_PATH is set, return it.
|
||||
- Otherwise, default to "/".
|
||||
|
||||
Raises ValueError on startup if the value contains characters that could
|
||||
be injected into served static files (e.g. script tags).
|
||||
"""
|
||||
return os.getenv("SERVER_ROOT_PATH", "")
|
||||
value = os.getenv("SERVER_ROOT_PATH", "")
|
||||
if value and not _SERVER_ROOT_PATH_PATTERN.match(value):
|
||||
raise ValueError(
|
||||
f"Invalid SERVER_ROOT_PATH {value!r}: must be empty or match "
|
||||
r"^(/[a-zA-Z0-9_-]+)*$ (e.g. '/myapp' or '/myapp/v1'). "
|
||||
"Characters outside this set could be injected into served UI assets."
|
||||
)
|
||||
return value
|
||||
|
||||
|
||||
def normalize_route_for_root_path(route: str) -> Optional[str]:
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ def _apply_server_root_path_replacements(
|
|||
content = f.read()
|
||||
modified = content.replace(litellm_asset_prefix, server_root_path)
|
||||
modified = modified.replace('"/ui/assets/', f'"{server_root_path}/ui/assets/')
|
||||
modified = modified.replace("'/ui/assets/", f"'{server_root_path}/ui/assets/")
|
||||
modified = modified.replace(
|
||||
"/litellm/.well-known/litellm-ui-config",
|
||||
f"{server_root_path}/.well-known/litellm-ui-config",
|
||||
|
|
@ -117,3 +118,67 @@ def test_binary_files_are_skipped(tmp_path):
|
|||
_apply_server_root_path_replacements(str(ui_path), "/root")
|
||||
|
||||
assert png_file.read_bytes() == b"\x89PNG\r\n\x1a\n"
|
||||
|
||||
|
||||
def test_single_quoted_ui_asset_paths_rewritten(tmp_path):
|
||||
"""
|
||||
Single-quoted '/ui/assets/' paths must also be rewritten.
|
||||
Some HTML/CSS files use single quotes for attribute values.
|
||||
"""
|
||||
ui_path = tmp_path / "ui"
|
||||
ui_path.mkdir()
|
||||
|
||||
js_file = ui_path / "bundle.js"
|
||||
js_file.write_text("let eq='/ui/assets/logos/',r={}")
|
||||
|
||||
server_root_path = "/myapp"
|
||||
_apply_server_root_path_replacements(str(ui_path), server_root_path)
|
||||
|
||||
result = js_file.read_text()
|
||||
assert f"'{server_root_path}/ui/assets/logos/'" in result
|
||||
assert "'/ui/assets/logos/'" not in result
|
||||
|
||||
|
||||
def test_invalid_server_root_path_raises_value_error():
|
||||
"""
|
||||
get_server_root_path() must raise ValueError for values that could
|
||||
be injected into served JS/HTML files (XSS prevention).
|
||||
"""
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
from litellm.proxy.utils import get_server_root_path
|
||||
|
||||
malicious_paths = [
|
||||
'"><script>alert(1)</script>',
|
||||
"/myapp/../../etc/passwd",
|
||||
"/myapp with spaces",
|
||||
"/myapp?query=1",
|
||||
]
|
||||
for path in malicious_paths:
|
||||
with patch.dict(os.environ, {"SERVER_ROOT_PATH": path}):
|
||||
try:
|
||||
result = get_server_root_path()
|
||||
assert False, f"Expected ValueError for path {path!r}, got {result!r}"
|
||||
except ValueError:
|
||||
pass # expected
|
||||
|
||||
|
||||
def test_valid_server_root_path_accepted():
|
||||
"""Valid SERVER_ROOT_PATH values must be accepted without error."""
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
from litellm.proxy.utils import get_server_root_path
|
||||
|
||||
valid_paths = [
|
||||
"",
|
||||
"/myapp",
|
||||
"/myapp/v1",
|
||||
"/web-llmgateway/v1",
|
||||
"/a-b_c",
|
||||
]
|
||||
for path in valid_paths:
|
||||
with patch.dict(os.environ, {"SERVER_ROOT_PATH": path}):
|
||||
result = get_server_root_path()
|
||||
assert result == path
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue