From 53a4cee0d82b5358821383acc4937dfc2f43fd1e Mon Sep 17 00:00:00 2001 From: tin-berri Date: Mon, 26 Oct 2026 23:00:03 -0400 Subject: [PATCH] fix(ui): honor dark and local logos across Connect and Chat (#48532) --- .../proxy_setting_endpoints.py | 30 +++++--- .../test_proxy_setting_endpoints.py | 77 +++++++++++++------ .../ui-theme/UIThemeSettings.test.tsx | 33 +++++++- .../(dashboard)/ui-theme/UIThemeSettings.tsx | 9 ++- .../src/app/chat/layout.integration.test.tsx | 58 +++++++++++++- .../chat/ChatShell.integration.test.tsx | 16 +++- .../src/components/chat/ChatShell.tsx | 25 ++++++ .../src/components/chat/design.md | 2 + .../src/components/navbar.test.tsx | 32 +++++--- .../src/components/navbar.tsx | 4 +- .../src/contexts/ThemeContext.tsx | 38 ++++++--- ui/litellm-dashboard/src/lib/http/schema.d.ts | 10 +++ 12 files changed, 267 insertions(+), 67 deletions(-) diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index e04ddbc75cc..1bb88d5ff69 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -234,6 +234,9 @@ class DefaultTeamSettingsResponse(SettingsResponse): class UIThemeSettingsResponse(SettingsResponse): """Response model for UI theme settings""" + logo_url_via_proxy: bool = False + logo_url_dark_via_proxy: bool = False + _TEAM_ADMIN_FIELD_ENUM: Final = tuple(sorted(SUPPORTED_TEAM_ADMIN_PERMISSIONS)) @@ -1377,7 +1380,7 @@ async def update_sso_settings( tags=["UI Theme Settings"], response_model=UIThemeSettingsResponse, ) -async def get_ui_theme_settings(): +async def get_ui_theme_settings() -> UIThemeSettingsResponse: """ Get UI theme configuration from the litellm_settings. Returns current logo settings for UI customization. @@ -1390,18 +1393,23 @@ async def get_ui_theme_settings(): # Load existing config config: Final = await proxy_config.get_config() - result: Final = await _get_settings_with_schema( - settings_key="ui_theme_config", - settings_class=UIThemeConfig, - config=config, + result: Final = _SettingsWithSchema.model_validate( + await _get_settings_with_schema( + settings_key="ui_theme_config", + settings_class=UIThemeConfig, + config=config, + ) ) - - stored_values: Final = result.get("values", {}) - result["values"] = { - **stored_values, - **{field: _resolve_ui_theme_field(stored_values, field) for field in _UI_THEME_FIELD_ENV_VARS}, + values: Final = { + **result.values, + **{field: _resolve_ui_theme_field(result.values, field) for field in _UI_THEME_FIELD_ENV_VARS}, } - return result + return UIThemeSettingsResponse( + values=values, + field_schema=result.field_schema, + logo_url_via_proxy=values["logo_url"] is None and bool(os.getenv("UI_LOGO_PATH", "").strip()), + logo_url_dark_via_proxy=values["logo_url_dark"] is None and bool(os.getenv("UI_LOGO_PATH_DARK", "").strip()), + ) def _validate_public_image_url(value: str | None, field_name: str) -> None: diff --git a/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py b/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py index 57b68e6db05..4360655c8ca 100644 --- a/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py +++ b/tests/unit/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py @@ -1359,35 +1359,45 @@ class TestProxySettingEndpoints: assert values["logo_url"] == "https://cdn.example.com/logo.png" assert values["favicon_url"] == "https://cdn.example.com/favicon.ico" + @pytest.mark.parametrize("env_logo", [ + "https://env.example.com/logo.png", "/mnt/secret/internal/logo.png", + ]) def test_get_ui_theme_settings_stored_value_wins_over_env( - self, mock_auth, monkeypatch - ): + self, monkeypatch: pytest.MonkeyPatch, env_logo: str, + ) -> None: """A stored ui_theme_config field outranks the env var for that field. The env fallback only fills fields the stored config leaves blank, so the UI-driven flow is unchanged while an unstored field still resolves. """ from litellm.proxy.proxy_server import proxy_config + from litellm.proxy.ui_crud_endpoints.proxy_setting_endpoints import UIThemeSettingsResponse - stored_config = { + stored_config: Final[dict[str, object]] = { "litellm_settings": { - "ui_theme_config": {"logo_url": "https://db.example.com/logo.png"} + "ui_theme_config": { + "logo_url": "https://db.example.com/logo.png", + "logo_url_dark": "https://db.example.com/logo-dark.png", + } } } - async def mock_get_config(): + async def mock_get_config() -> dict[str, object]: return stored_config monkeypatch.setattr(proxy_config, "get_config", mock_get_config) - monkeypatch.setenv("UI_LOGO_PATH", "https://env.example.com/logo.png") + monkeypatch.setenv("UI_LOGO_PATH", env_logo) + monkeypatch.setenv("UI_LOGO_PATH_DARK", env_logo) monkeypatch.setenv("LITELLM_FAVICON_URL", "https://env.example.com/favicon.ico") - response = client.get("/get/ui_theme_settings") + response: Final = client.get("/get/ui_theme_settings") assert response.status_code == 200 - values = response.json()["values"] - assert values["logo_url"] == "https://db.example.com/logo.png" - assert values["favicon_url"] == "https://env.example.com/favicon.ico" + result: Final = UIThemeSettingsResponse.model_validate(response.json()) + assert result.values["logo_url"] == "https://db.example.com/logo.png" + assert result.values["logo_url_dark"] == "https://db.example.com/logo-dark.png" + assert result.values["favicon_url"] == "https://env.example.com/favicon.ico" + assert (result.logo_url_via_proxy, result.logo_url_dark_via_proxy) == (False, False) def test_get_ui_theme_settings_reports_unset_when_absent_everywhere( self, mock_proxy_config, monkeypatch @@ -1403,22 +1413,45 @@ class TestProxySettingEndpoints: assert values["logo_url"] is None assert values["favicon_url"] is None - def test_get_ui_theme_settings_does_not_disclose_local_path_env_value( - self, mock_proxy_config, monkeypatch - ): - """This endpoint is public, so an env-configured local filesystem branding - path must never be surfaced to anonymous callers; only public http(s) URLs. - """ - monkeypatch.setenv("UI_LOGO_PATH", "/mnt/secret/internal/logo.png") + @pytest.mark.usefixtures("mock_proxy_config") + @pytest.mark.parametrize("light,public_light,light_via_proxy", [ + (None, None, False), + (" ", None, False), + ("https://cdn.example.com/light.png", "https://cdn.example.com/light.png", False), + ("/mnt/secret/internal/light.png", None, True), + ]) + @pytest.mark.parametrize("dark,public_dark,dark_via_proxy", [ + (None, None, False), + (" ", None, False), + ("https://cdn.example.com/dark.png", "https://cdn.example.com/dark.png", False), + ("/mnt/secret/internal/dark.png", None, True), + ]) + def test_get_ui_theme_settings_separates_private_logo_rendering_from_settings( + self, monkeypatch: pytest.MonkeyPatch, + light: str | None, public_light: str | None, light_via_proxy: bool, + dark: str | None, public_dark: str | None, dark_via_proxy: bool, + ) -> None: + from litellm.proxy.ui_crud_endpoints.proxy_setting_endpoints import UIThemeSettingsResponse + + for key, value in (("UI_LOGO_PATH", light), ("UI_LOGO_PATH_DARK", dark)): + if value is None: + monkeypatch.delenv(key, raising=False) + else: + monkeypatch.setenv(key, value) monkeypatch.setenv("LITELLM_FAVICON_URL", "file:///etc/favicon.ico") - response = client.get("/get/ui_theme_settings") + response: Final = client.get("/get/ui_theme_settings") assert response.status_code == 200 - values = response.json()["values"] - # the local path / file scheme is withheld rather than disclosed - assert values["logo_url"] is None - assert values["favicon_url"] is None + result: Final = UIThemeSettingsResponse.model_validate(response.json()) + assert (result.values["logo_url"], result.values["logo_url_dark"]) == (public_light, public_dark) + assert (result.logo_url_via_proxy, result.logo_url_dark_via_proxy) == (light_via_proxy, dark_via_proxy) + assert result.values["favicon_url"] is None + assert "/mnt/secret/internal/" not in response.text + assert "file:///etc/favicon.ico" not in response.text + for hint in ("logo_url_via_proxy", "logo_url_dark_via_proxy"): + assert hint not in result.values + assert hint not in json.dumps(result.field_schema) def test_get_ui_settings(self, mock_auth, monkeypatch): """Test retrieving UI settings with allowlist sanitization""" diff --git a/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.test.tsx index 4b58590ffd1..86ef33c0d8b 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.test.tsx @@ -10,7 +10,8 @@ const setLogoUrl = vi.fn(); const setLogoUrlDark = vi.fn(); const setFaviconUrl = vi.fn(); -vi.mock("@/contexts/ThemeContext", () => ({ +vi.mock("@/contexts/ThemeContext", async (importOriginal) => ({ + ...(await importOriginal()), useTheme: () => ({ logoUrl: null, setLogoUrl, @@ -22,7 +23,7 @@ vi.mock("@/contexts/ThemeContext", () => ({ })); vi.mock("@/components/networking", () => ({ - getProxyBaseUrl: () => "", + getProxyBaseUrl: () => "/gateway", getGlobalLitellmHeaderName: () => "Authorization", })); @@ -111,6 +112,34 @@ describe("UIThemeSettings", () => { expect(bodyOf(patchCalls()[0]).logo_url_dark).toBe("https://a.test/logo-dark.png"); }); + it.each(["Save Changes", "Reset to Default"])( + "should keep local logo routes out of settings and clear preview overrides with %s", + async (action) => { + const user = userEvent.setup(); + fetchMock.mockImplementation(async () => + Response.json({ + values: { logo_url: null, logo_url_dark: null }, + logo_url_via_proxy: true, + logo_url_dark_via_proxy: true, + }), + ); + + render(); + + await waitFor(() => expect(setLogoUrlDark).toHaveBeenCalledWith("/gateway/get_image?theme=dark")); + expect(setLogoUrl).toHaveBeenCalledWith("/gateway/get_image"); + expect(screen.getByPlaceholderText(LOGO_PLACEHOLDER)).toHaveValue(""); + expect(screen.getByPlaceholderText(LOGO_DARK_PLACEHOLDER)).toHaveValue(""); + + await user.click(screen.getByRole("button", { name: action })); + await waitFor(() => expect(patchCalls()).toHaveLength(1)); + expect(bodyOf(patchCalls()[0])).toEqual({ logo_url: null, logo_url_dark: null, favicon_url: null }); + + expect(setLogoUrl).toHaveBeenLastCalledWith(null); + expect(setLogoUrlDark).toHaveBeenLastCalledWith(null); + }, + ); + it("should surface a backend failure when saving fails", async () => { const user = userEvent.setup(); render(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.tsx index b6bc0f28dd5..c0b3bd26001 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/ui-theme/UIThemeSettings.tsx @@ -4,7 +4,7 @@ import { Card, CardContent } from "@/components/ui/card"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; -import { useTheme } from "@/contexts/ThemeContext"; +import { resolveThemeImages, useTheme } from "@/contexts/ThemeContext"; import { getProxyBaseUrl, getGlobalLitellmHeaderName } from "@/components/networking"; import { toast } from "@/lib/toast"; @@ -40,12 +40,13 @@ const UIThemeSettings: React.FC = ({ userID, userRole, acc }); if (response.ok) { const data = await response.json(); + const images = resolveThemeImages(data, proxyBaseUrl); setLogoUrlInput(data.values?.logo_url || ""); setLogoUrlDarkInput(data.values?.logo_url_dark || ""); setFaviconUrlInput(data.values?.favicon_url || ""); - setLogoUrl(data.values?.logo_url || null); - setLogoUrlDark(data.values?.logo_url_dark || null); - setFaviconUrl(data.values?.favicon_url || null); + setLogoUrl(images.logoUrl); + setLogoUrlDark(images.logoUrlDark); + setFaviconUrl(images.faviconUrl); } } catch (error) { console.error("Error fetching theme settings:", error); diff --git a/ui/litellm-dashboard/src/app/chat/layout.integration.test.tsx b/ui/litellm-dashboard/src/app/chat/layout.integration.test.tsx index a7189ea6cd7..2efe698ec69 100644 --- a/ui/litellm-dashboard/src/app/chat/layout.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/chat/layout.integration.test.tsx @@ -1,7 +1,8 @@ import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { renderWithProviders, screen, testQueryClient } from "@/../tests/test-utils"; +import { renderWithProviders, screen, testQueryClient, within } from "@/../tests/test-utils"; import { setServerRootPath } from "@/lib/serverRootPath"; +import { getProxyBaseUrl } from "@/components/networking"; import ChatLayout from "./layout"; const { router, route } = vi.hoisted(() => ({ @@ -33,9 +34,13 @@ vi.mock("@/app/(dashboard)/hooks/uiSettings/useUISettings", () => ({ const plugin = { name: "observability", display_name: "Observability", url: "https://plugin.example.com" }; const originalLocation = Object.getOwnPropertyDescriptor(window, "location")!; const assign = vi.fn(); +const LIGHT_LOGO = "https://cdn.test/light.svg"; +const DARK_LOGO = "https://cdn.test/dark.svg"; +let themeResponse: unknown = { values: {} }; describe("Chat layout product navigation", () => { beforeEach(() => { + themeResponse = { values: {} }; route.pathname = "/ui/chat"; localStorage.clear(); localStorage.setItem("litellm_plugin_mode", plugin.name); @@ -55,7 +60,7 @@ describe("Chat layout product navigation", () => { expect(new Headers(init?.headers).get("Authorization")).toBe("Bearer chat-layout-test-token"); return Response.json([plugin]); case "/get/ui_theme_settings": - return Response.json({ values: {} }); + return Response.json(themeResponse); case "/sso/get/ui_settings": return Response.json({}); default: @@ -75,6 +80,55 @@ describe("Chat layout product navigation", () => { vi.clearAllMocks(); }); + it.each([ + ["public", [LIGHT_LOGO, DARK_LOGO], [false, false], [LIGHT_LOGO, DARK_LOGO]], + ["local", [null, null], [true, true], ["/get_image", "/get_image?theme=dark"]], + ["public light and local dark", [LIGHT_LOGO, null], [false, true], [LIGHT_LOGO, "/get_image?theme=dark"]], + ["local light and public dark", [null, DARK_LOGO], [true, false], ["/get_image", DARK_LOGO]], + ["public light only", [LIGHT_LOGO, null], [false, false], [LIGHT_LOGO, LIGHT_LOGO]], + ["public dark only", [null, DARK_LOGO], [false, false], [DARK_LOGO, DARK_LOGO]], + ["local light only", [null, null], [true, false], ["/get_image", "/get_image"]], + ["local dark only", [null, null], [false, true], ["/get_image?theme=dark", "/get_image?theme=dark"]], + ] as const)( + "should render %s organization logos from the theme response", + async ( + _label, + [logo_url, logo_url_dark], + [logo_url_via_proxy, logo_url_dark_via_proxy], + [lightSource, darkSource], + ) => { + themeResponse = { values: { logo_url, logo_url_dark }, logo_url_via_proxy, logo_url_dark_via_proxy }; + const user = userEvent.setup(); + renderWithProviders( + +

Chat content

+ , + ); + + await user.click(screen.getByRole("button", { name: "Open chat navigation" })); + const logo = await screen.findByRole("img", { name: "Organization logo" }); + const [light, dark] = within(logo).getAllByAltText(""); + const source = (path: string) => (path.startsWith("/") ? `${getProxyBaseUrl()}${path}` : path); + expect(light).toHaveAttribute("src", source(lightSource)); + expect(dark).toHaveAttribute("src", source(darkSource)); + expect(light).toHaveClass("dark:hidden"); + expect(dark).toHaveClass("hidden", "dark:block"); + }, + ); + + it("should omit organization branding when no logo is configured", async () => { + const user = userEvent.setup(); + renderWithProviders( + +

Chat content

+ , + ); + + await user.click(screen.getByRole("button", { name: "Open chat navigation" })); + expect(await screen.findByRole("dialog", { name: "LiteLLM Chat" })).toBeInTheDocument(); + expect(screen.queryByRole("img", { name: "Organization logo" })).not.toBeInTheDocument(); + }); + it.each([ { pathname: "/ui/chat", root: "/", gateway: "/ui/" }, { pathname: "/gateway/ui/chat/models/", root: "/gateway", gateway: "/gateway/ui/" }, diff --git a/ui/litellm-dashboard/src/components/chat/ChatShell.integration.test.tsx b/ui/litellm-dashboard/src/components/chat/ChatShell.integration.test.tsx index c1af91845ee..3c7b9ef1b1a 100644 --- a/ui/litellm-dashboard/src/components/chat/ChatShell.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/chat/ChatShell.integration.test.tsx @@ -1,9 +1,13 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { renderWithProviders as render } from "@/../tests/test-utils"; +import { renderWithProviders } from "@/../tests/test-utils"; +import type { ReactElement } from "react"; +import { ThemeProvider } from "@/contexts/ThemeContext"; import ChatShell from "./ChatShell"; +const render = (ui: ReactElement) => renderWithProviders({ui}); + const { mockPush, mockUsePathname, mockUseChatShell, mockUseMediaQuery } = vi.hoisted(() => ({ mockPush: vi.fn(), mockUsePathname: vi.fn(() => "/ui/chat"), @@ -34,7 +38,15 @@ vi.mock("usehooks-ts", () => ({ useMediaQuery: mockUseMediaQuery })); vi.mock("./ConversationList", () => ({ default: () =>
})); describe("ChatShell", () => { + beforeEach(() => { + vi.stubGlobal( + "fetch", + vi.fn(async () => Response.json({ values: {} })), + ); + }); + afterEach(() => { + vi.unstubAllGlobals(); mockPush.mockClear(); mockUsePathname.mockReturnValue("/ui/chat"); mockUseMediaQuery.mockReturnValue(true); diff --git a/ui/litellm-dashboard/src/components/chat/ChatShell.tsx b/ui/litellm-dashboard/src/components/chat/ChatShell.tsx index c089a709b64..280e2d10b8c 100644 --- a/ui/litellm-dashboard/src/components/chat/ChatShell.tsx +++ b/ui/litellm-dashboard/src/components/chat/ChatShell.tsx @@ -1,6 +1,7 @@ "use client"; import React, { useState } from "react"; +import Image from "next/image"; import { usePathname, useRouter } from "next/navigation"; import { Plus, @@ -34,6 +35,7 @@ import ThemeToggle from "@/components/ThemeToggle/ThemeToggle"; import { useLogout } from "@/app/(dashboard)/hooks/useLogout"; import { uiHref } from "@/utils/uiHref"; import { useChatShell } from "@/contexts/ChatShellContext"; +import { useTheme } from "@/contexts/ThemeContext"; import ViewSwitcher from "@/components/Navbar/ViewSwitcher"; import ConversationList from "./ConversationList"; @@ -94,6 +96,9 @@ const ChatShell: React.FC<{ children: React.ReactNode }> = ({ children }) => { const [searchOpen, setSearchOpen] = useState(false); if (isDesktop && sidebarOpen) setSidebarOpen(false); const collapsed = isDesktop && sidebarCollapsed; + const { logoUrl, logoUrlDark } = useTheme(); + const lightLogo = logoUrl || logoUrlDark; + const darkLogo = logoUrlDark || logoUrl; const routes = getChatRoutes(); const isChatsRoute = pathname === routes.chats; const accountName = userEmail?.split("@")[0] || userId || "Account"; @@ -216,6 +221,26 @@ const ChatShell: React.FC<{ children: React.ReactNode }> = ({ children }) => { hideSearchButton />
+ {lightLogo && darkLogo && ( +
+ + +
+ )}