mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(proxy): keep the UI asset-prefix rewrite off the committed bundle
tests/test_litellm/proxy/test_custom_proxy.py was an example server script, not a test: it declared zero test functions but set SERVER_ROOT_PATH at module scope and then imported proxy_server. proxy_server applies the asset-prefix rewrite at import time, so merely collecting that file rewrote /litellm-asset-prefix to /my-custom-path across ~475 tracked files under litellm/proxy/_experimental/out/ and left them dirty, which invites the UI bundle into unrelated commits. The leaked env var had already forced a defensive fixture in the MCP server tests. Move the example to examples/custom_root_path_server.py so pytest can never import it, and extract the rewrite loop out of module scope into _apply_server_root_path_to_ui_assets so the behaviour can be exercised against a runtime copy. The helper also skips writing files whose content is unchanged. Adds regression coverage: the rewrite is asserted against a temp bundle while the packaged bundle is hashed before and after, a root-mounted deployment is asserted to be a no-op, and a guard fails if any module under tests/test_litellm/proxy sets SERVER_ROOT_PATH at import scope again.
This commit is contained in:
parent
b0626cad8c
commit
ced3e4e4e0
4 changed files with 150 additions and 58 deletions
|
|
@ -1710,6 +1710,58 @@ def _get_cors_config(
|
|||
origins, allow_cors_credentials = _get_cors_config()
|
||||
|
||||
|
||||
_UI_NON_TEXT_ASSET_SUFFIXES: Final = (
|
||||
".png",
|
||||
".jpg",
|
||||
".jpeg",
|
||||
".gif",
|
||||
".ico",
|
||||
".woff",
|
||||
".woff2",
|
||||
".ttf",
|
||||
".eot",
|
||||
)
|
||||
|
||||
|
||||
def _apply_server_root_path_to_ui_assets(ui_dir: str, server_root_path: str, asset_prefix: str) -> None:
|
||||
"""Rewrite ``asset_prefix`` to ``server_root_path`` in the exported UI under ``ui_dir``.
|
||||
|
||||
Rewrites in place, so ``ui_dir`` must be a runtime-owned copy of the bundle and never the
|
||||
packaged one when that is a read-only or version-controlled checkout.
|
||||
"""
|
||||
if not server_root_path or server_root_path == "/":
|
||||
return
|
||||
|
||||
if not os.access(ui_dir, os.W_OK):
|
||||
verbose_proxy_logger.warning(
|
||||
"Cannot apply server_root_path replacements to UI at %s: path is not writable. Ensure server_root_path is '/' or pre-process UI files in Dockerfile with custom server_root_path.",
|
||||
ui_dir,
|
||||
)
|
||||
return
|
||||
|
||||
for current_root, _, files in os.walk(ui_dir):
|
||||
for filename in files:
|
||||
if filename.endswith(_UI_NON_TEXT_ASSET_SUFFIXES):
|
||||
continue
|
||||
|
||||
file_path = os.path.join(current_root, filename)
|
||||
try:
|
||||
with open(file_path, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
|
||||
modified_content = content.replace(asset_prefix, server_root_path).replace(
|
||||
"/litellm/.well-known/litellm-ui-config",
|
||||
f"{server_root_path}/.well-known/litellm-ui-config",
|
||||
)
|
||||
if modified_content == content:
|
||||
continue
|
||||
|
||||
with open(file_path, "w", encoding="utf-8") as f:
|
||||
f.write(modified_content)
|
||||
except (UnicodeDecodeError, PermissionError, OSError):
|
||||
continue
|
||||
|
||||
|
||||
# get current directory
|
||||
try:
|
||||
current_dir = os.path.dirname(os.path.abspath(__file__))
|
||||
|
|
@ -1888,57 +1940,7 @@ try:
|
|||
if not _validate_ui_directory(ui_path):
|
||||
verbose_proxy_logger.error("Selected UI path %s is invalid or incomplete. UI may not work correctly.", ui_path)
|
||||
|
||||
# Only modify files if a custom server root path is set AND filesystem is writable
|
||||
if server_root_path and server_root_path != "/":
|
||||
# Check if UI path is writable
|
||||
is_writable = os.access(ui_path, os.W_OK)
|
||||
|
||||
if not is_writable:
|
||||
verbose_proxy_logger.warning(
|
||||
"Cannot apply server_root_path replacements to UI at %s: path is not writable. Ensure server_root_path is '/' or pre-process UI files in Dockerfile with custom server_root_path.",
|
||||
ui_path,
|
||||
)
|
||||
else:
|
||||
# Iterate through files in the UI directory
|
||||
for root, dirs, files in os.walk(ui_path):
|
||||
for filename in files:
|
||||
file_path = os.path.join(root, filename)
|
||||
# Skip binary files and files that don't need path replacement
|
||||
if filename.endswith(
|
||||
(
|
||||
".png",
|
||||
".jpg",
|
||||
".jpeg",
|
||||
".gif",
|
||||
".ico",
|
||||
".woff",
|
||||
".woff2",
|
||||
".ttf",
|
||||
".eot",
|
||||
)
|
||||
):
|
||||
continue
|
||||
try:
|
||||
with open(file_path, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
|
||||
# Replace the asset prefix with the server root path
|
||||
modified_content = content.replace(
|
||||
f"{litellm_asset_prefix}",
|
||||
f"{server_root_path}",
|
||||
)
|
||||
|
||||
# Replace the /.well-known/litellm-ui-config with the server root path
|
||||
modified_content = modified_content.replace(
|
||||
"/litellm/.well-known/litellm-ui-config",
|
||||
f"{server_root_path}/.well-known/litellm-ui-config",
|
||||
)
|
||||
|
||||
with open(file_path, "w", encoding="utf-8") as f:
|
||||
f.write(modified_content)
|
||||
except (UnicodeDecodeError, PermissionError, OSError):
|
||||
# Skip binary files or files we can't write to
|
||||
continue
|
||||
_apply_server_root_path_to_ui_assets(ui_path, server_root_path, litellm_asset_prefix)
|
||||
|
||||
# # Mount the _next directory at the root level
|
||||
app.mount(
|
||||
|
|
|
|||
|
|
@ -5,14 +5,13 @@ import pytest
|
|||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _hermetic_server_root_path():
|
||||
"""Isolate MCP discovery tests from a leaked ``SERVER_ROOT_PATH``.
|
||||
"""Isolate MCP discovery tests from an ambient ``SERVER_ROOT_PATH``.
|
||||
|
||||
``tests/test_litellm/proxy/test_custom_proxy.py`` sets ``SERVER_ROOT_PATH`` at import time
|
||||
(its app mounts under a custom path) and never restores it, so in a shared shard the value
|
||||
leaks into this process. The discovery routes and the 401 challenges read it, so a leaked
|
||||
value would silently rewrite every ``resource_metadata`` URL and make these tests depend on
|
||||
shard ordering. Clearing it here pins the default (root-mounted) deployment; a test that
|
||||
exercises a sub-path deployment sets the value explicitly within its own body.
|
||||
The discovery routes and the 401 challenges read it, so a value inherited from the
|
||||
environment would silently rewrite every ``resource_metadata`` URL and make these tests
|
||||
depend on how the shard was invoked. Clearing it here pins the default (root-mounted)
|
||||
deployment; a test that exercises a sub-path deployment sets the value explicitly within
|
||||
its own body.
|
||||
"""
|
||||
saved = os.environ.pop("SERVER_ROOT_PATH", None)
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
import ast
|
||||
import asyncio
|
||||
import hashlib
|
||||
import importlib
|
||||
import json
|
||||
import os
|
||||
|
|
@ -10798,3 +10800,92 @@ async def test_ptu_rollup_job_not_registered_without_opt_in(monkeypatch):
|
|||
|
||||
assert scheduler.get_job(PTU_ROLLUP_JOB_ID) is None
|
||||
assert len(scheduler.get_jobs()) > 0
|
||||
|
||||
|
||||
def _hash_tree(root: Path) -> dict:
|
||||
return {
|
||||
str(p.relative_to(root)): hashlib.sha256(p.read_bytes()).hexdigest()
|
||||
for p in sorted(root.rglob("*"))
|
||||
if p.is_file()
|
||||
}
|
||||
|
||||
|
||||
def _write_fake_ui_bundle(root: Path) -> None:
|
||||
(root / "_next" / "static").mkdir(parents=True)
|
||||
(root / "index.html").write_text(
|
||||
'<script src="/litellm-asset-prefix/_next/static/app.js"></script>'
|
||||
'<link href="/litellm/.well-known/litellm-ui-config"/>',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(root / "_next" / "static" / "app.js").write_text(
|
||||
'fetch("/litellm-asset-prefix/_next/static/chunk.js")', encoding="utf-8"
|
||||
)
|
||||
(root / "favicon.ico").write_bytes(b"\x00\x01/litellm-asset-prefix\xff")
|
||||
|
||||
|
||||
def test_apply_server_root_path_rewrites_copy_and_leaves_packaged_bundle_untouched(tmp_path):
|
||||
"""The rewrite must land on the runtime copy only.
|
||||
|
||||
Regression: an example script under tests/ set SERVER_ROOT_PATH at import time, so importing
|
||||
proxy_server rewrote the committed bundle in litellm/proxy/_experimental/out/ in place and
|
||||
left ~475 tracked files dirty.
|
||||
"""
|
||||
packaged_bundle = Path(proxy_server_module.packaged_ui_path)
|
||||
assert packaged_bundle.is_dir(), "packaged UI bundle missing; this guard would be inert"
|
||||
packaged_before = _hash_tree(packaged_bundle)
|
||||
assert packaged_before, "packaged UI bundle is empty; this guard would be inert"
|
||||
|
||||
runtime_copy = tmp_path / "out"
|
||||
_write_fake_ui_bundle(runtime_copy)
|
||||
|
||||
proxy_server_module._apply_server_root_path_to_ui_assets(
|
||||
str(runtime_copy), "/my-custom-path", "/litellm-asset-prefix"
|
||||
)
|
||||
|
||||
index_html = (runtime_copy / "index.html").read_text(encoding="utf-8")
|
||||
assert '<script src="/my-custom-path/_next/static/app.js"></script>' in index_html
|
||||
assert '<link href="/my-custom-path/.well-known/litellm-ui-config"/>' in index_html
|
||||
assert "litellm-asset-prefix" not in index_html
|
||||
assert "litellm-asset-prefix" not in (runtime_copy / "_next" / "static" / "app.js").read_text(encoding="utf-8")
|
||||
assert (runtime_copy / "favicon.ico").read_bytes() == b"\x00\x01/litellm-asset-prefix\xff"
|
||||
|
||||
assert _hash_tree(packaged_bundle) == packaged_before
|
||||
|
||||
|
||||
@pytest.mark.parametrize("root_path", ["/", ""])
|
||||
def test_apply_server_root_path_is_a_noop_for_root_deployments(tmp_path, root_path):
|
||||
"""A root-mounted deployment must not touch the bundle at all."""
|
||||
runtime_copy = tmp_path / "out"
|
||||
_write_fake_ui_bundle(runtime_copy)
|
||||
before = _hash_tree(runtime_copy)
|
||||
|
||||
proxy_server_module._apply_server_root_path_to_ui_assets(
|
||||
str(runtime_copy), root_path, "/litellm-asset-prefix"
|
||||
)
|
||||
|
||||
assert _hash_tree(runtime_copy) == before
|
||||
|
||||
|
||||
def test_no_proxy_test_module_sets_server_root_path_at_import_time():
|
||||
"""Import-time SERVER_ROOT_PATH in a collected module rewrites the committed UI bundle.
|
||||
|
||||
proxy_server applies the asset-prefix rewrite at module scope, so any test module that sets
|
||||
the variable before pytest imports proxy_server corrupts litellm/proxy/_experimental/out/ for
|
||||
the whole run and leaks the value into every sibling test.
|
||||
"""
|
||||
proxy_tests_root = Path(__file__).parent
|
||||
module_scope_writes = (ast.Assign, ast.AugAssign, ast.AnnAssign, ast.Expr)
|
||||
offenders = sorted(
|
||||
str(module_path.relative_to(proxy_tests_root))
|
||||
for module_path in proxy_tests_root.rglob("*.py")
|
||||
if any(
|
||||
"SERVER_ROOT_PATH" in ast.dump(node)
|
||||
for node in ast.parse(module_path.read_text(encoding="utf-8")).body
|
||||
if isinstance(node, module_scope_writes)
|
||||
)
|
||||
)
|
||||
|
||||
assert offenders == [], (
|
||||
f"module-scope SERVER_ROOT_PATH assignment in {offenders}; set it inside a test with "
|
||||
"monkeypatch.setenv so it cannot rewrite the committed UI bundle at import time"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue