From 22b60624ad1746663dafde5e7a00d3ad9dd6d377 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 4 Aug 2026 09:28:28 -0700 Subject: [PATCH 01/11] feat(ui): add role capability gating, migrate Tool Policies route Internal users saw the Tool Policies page but its /v1/tool/list call always returned 401. This adds a single source of truth for which roles may trigger which UI fetches (utils/capabilities.ts) plus a useCan hook, and wires the Tool Policies route through it: the nav item, the page, and the query all read the same capability, so the sidebar hides the entry, deep links render an admin-only notice, and the query never fires. The tools list call also moves onto a queryOptions factory --- .../src/app/(dashboard)/hooks/useCan.ts | 12 ++++++ .../ToolPolicies/ToolPoliciesPanel.test.tsx | 18 ++++++++ .../ToolPolicies/ToolPoliciesPanel.tsx | 27 +++++------- .../ToolPolicies/toolPoliciesQueries.ts | 16 +++++++ .../src/components/ToolPoliciesView.test.tsx | 19 +++++++- .../src/components/ToolPoliciesView.tsx | 11 +++++ .../src/components/leftnav.test.tsx | 43 ++++++++++++++++++- .../src/components/leftnav.tsx | 9 +++- .../src/utils/capabilities.test.ts | 27 ++++++++++++ .../src/utils/capabilities.ts | 12 ++++++ 10 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/hooks/useCan.ts create mode 100644 ui/litellm-dashboard/src/components/ToolPolicies/toolPoliciesQueries.ts create mode 100644 ui/litellm-dashboard/src/utils/capabilities.test.ts create mode 100644 ui/litellm-dashboard/src/utils/capabilities.ts diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/useCan.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useCan.ts new file mode 100644 index 00000000000..f538e1dff15 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useCan.ts @@ -0,0 +1,12 @@ +"use client"; + +import { hasCapability, type Capability } from "@/utils/capabilities"; + +import useAuthorized from "./useAuthorized"; + +const useCan = (capability: Capability): boolean => { + const { userRole } = useAuthorized(); + return hasCapability(userRole, capability); +}; + +export default useCan; diff --git a/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.test.tsx b/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.test.tsx index 0a0b1c09fbb..3c8a0da3347 100644 --- a/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.test.tsx +++ b/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.test.tsx @@ -21,6 +21,11 @@ vi.mock("@/components/molecules/notifications_manager", () => ({ default: { fromBackend: (...args: unknown[]) => fromBackend(...args) }, })); +const can = vi.fn(); +vi.mock("@/app/(dashboard)/hooks/useCan", () => ({ + default: (...args: unknown[]) => can(...args), +})); + const NOW = new Date("2026-07-21T12:00:00Z"); const TOOLS: ToolRow[] = [ @@ -104,6 +109,7 @@ beforeEach(() => { fetchToolsList.mockReset().mockResolvedValue(TOOLS); updateToolPolicy.mockReset().mockResolvedValue({}); fromBackend.mockReset(); + can.mockReset().mockReturnValue(true); Element.prototype.scrollIntoView = vi.fn(); }); @@ -112,6 +118,18 @@ afterEach(() => { }); describe("ToolPoliciesPanel data loading", () => { + it("should not fetch tools when the caller lacks the viewToolPolicies capability", async () => { + can.mockReturnValue(false); + renderPanel(); + + await act(async () => { + vi.advanceTimersByTime(1_000); + }); + + expect(can).toHaveBeenCalledWith("viewToolPolicies"); + expect(fetchToolsList).not.toHaveBeenCalled(); + }); + it("should load tools once and never auto-refresh on a timer", async () => { renderPanel(); await waitForRows(); diff --git a/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.tsx b/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.tsx index 1b559352469..df5d8553948 100644 --- a/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.tsx +++ b/ui/litellm-dashboard/src/components/ToolPolicies/ToolPoliciesPanel.tsx @@ -1,12 +1,14 @@ "use client"; -import { useQuery, useQueryClient, type UseQueryOptions } from "@tanstack/react-query"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; import React, { useCallback, useMemo, useState } from "react"; +import useCan from "@/app/(dashboard)/hooks/useCan"; import { MetricCard } from "@/components/GuardrailsMonitor/MetricCard"; import NotificationsManager from "@/components/molecules/notifications_manager"; -import { fetchToolsList, ToolRow, updateToolPolicy } from "@/components/networking"; +import { ToolRow, updateToolPolicy } from "@/components/networking"; +import { toolPoliciesListOptions } from "./toolPoliciesQueries"; import { ToolPoliciesTable } from "./ToolPoliciesTable"; function getUTCDateKey(date: Date): string { @@ -41,8 +43,6 @@ const withTool = (names: ReadonlySet, toolName: string): ReadonlySet, toolName: string): ReadonlySet => new Set([...names].filter((name) => name !== toolName)); -const TOOLS_QUERY_KEY = "tool-policies"; - interface ToolPoliciesPanelProps { accessToken: string | null; onSelectTool: (toolName: string) => void; @@ -50,19 +50,12 @@ interface ToolPoliciesPanelProps { export const ToolPoliciesPanel: React.FC = ({ accessToken, onSelectTool }) => { const queryClient = useQueryClient(); + const canViewToolPolicies = useCan("viewToolPolicies"); const [savingInput, setSavingInput] = useState>(() => new Set()); const [savingOutput, setSavingOutput] = useState>(() => new Set()); - const queryKey = useMemo(() => [TOOLS_QUERY_KEY, accessToken], [accessToken]); - - const queryOptions: UseQueryOptions = { - queryKey, - queryFn: async () => (accessToken === null ? [] : fetchToolsList(accessToken)), - enabled: accessToken !== null, - refetchOnWindowFocus: false, - refetchOnReconnect: false, - }; - const query = useQuery(queryOptions); + const listOptions = useMemo(() => toolPoliciesListOptions(accessToken), [accessToken]); + const query = useQuery({ ...listOptions, enabled: canViewToolPolicies && accessToken !== null }); const tools = useMemo(() => query.data ?? [], [query.data]); @@ -70,12 +63,12 @@ export const ToolPoliciesPanel: React.FC = ({ accessToke // and overwrite the row we just wrote with its pre-save snapshot. const patchTool = useCallback( async (toolName: string, patch: Partial) => { - await queryClient.cancelQueries({ queryKey }); - queryClient.setQueryData(queryKey, (previous) => + await queryClient.cancelQueries({ queryKey: listOptions.queryKey }); + queryClient.setQueryData(listOptions.queryKey, (previous) => (previous ?? []).map((tool) => (tool.tool_name === toolName ? { ...tool, ...patch } : tool)), ); }, - [queryClient, queryKey], + [queryClient, listOptions], ); const handleInputPolicyChange = useCallback( diff --git a/ui/litellm-dashboard/src/components/ToolPolicies/toolPoliciesQueries.ts b/ui/litellm-dashboard/src/components/ToolPolicies/toolPoliciesQueries.ts new file mode 100644 index 00000000000..558f8c95c2c --- /dev/null +++ b/ui/litellm-dashboard/src/components/ToolPolicies/toolPoliciesQueries.ts @@ -0,0 +1,16 @@ +import { queryOptions } from "@tanstack/react-query"; + +import { fetchToolsList, type ToolRow } from "@/components/networking"; + +export const toolPoliciesKeys = { + all: ["tool-policies"] as const, + list: (accessToken: string | null) => [...toolPoliciesKeys.all, accessToken] as const, +}; + +export const toolPoliciesListOptions = (accessToken: string | null) => + queryOptions({ + queryKey: toolPoliciesKeys.list(accessToken), + queryFn: async (): Promise => (accessToken === null ? [] : fetchToolsList(accessToken)), + refetchOnWindowFocus: false, + refetchOnReconnect: false, + }); diff --git a/ui/litellm-dashboard/src/components/ToolPoliciesView.test.tsx b/ui/litellm-dashboard/src/components/ToolPoliciesView.test.tsx index 34c697a98d1..74e3a316850 100644 --- a/ui/litellm-dashboard/src/components/ToolPoliciesView.test.tsx +++ b/ui/litellm-dashboard/src/components/ToolPoliciesView.test.tsx @@ -1,10 +1,15 @@ import React from "react"; -import { describe, it, expect, vi } from "vitest"; +import { beforeEach, describe, it, expect, vi } from "vitest"; import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { renderWithProviders } from "../../tests/test-utils"; import ToolPoliciesView from "./ToolPoliciesView"; +const can = vi.fn(); +vi.mock("@/app/(dashboard)/hooks/useCan", () => ({ + default: (...args: unknown[]) => can(...args), +})); + vi.mock("@/components/ToolDetail", () => ({ ToolDetail: ({ toolName, onBack }: { toolName: string; onBack: () => void }) => (
@@ -26,6 +31,18 @@ vi.mock("@/components/ToolPolicies/ToolPoliciesPanel", () => ({ })); describe("ToolPoliciesView", () => { + beforeEach(() => { + can.mockReset().mockReturnValue(true); + }); + + it("should show an admin-only notice instead of the overview when the caller lacks access", () => { + can.mockReturnValue(false); + renderWithProviders(); + + expect(screen.getByText(/only available to admin users/i)).toBeInTheDocument(); + expect(screen.queryByText("Tool Policies Overview")).not.toBeInTheDocument(); + }); + it("should render the overview by default", () => { renderWithProviders(); diff --git a/ui/litellm-dashboard/src/components/ToolPoliciesView.tsx b/ui/litellm-dashboard/src/components/ToolPoliciesView.tsx index bdff40153b9..b2d53985b29 100644 --- a/ui/litellm-dashboard/src/components/ToolPoliciesView.tsx +++ b/ui/litellm-dashboard/src/components/ToolPoliciesView.tsx @@ -1,6 +1,7 @@ "use client"; import React, { useState } from "react"; +import useCan from "@/app/(dashboard)/hooks/useCan"; import { ToolDetail } from "@/components/ToolDetail"; import { ToolPoliciesPanel } from "@/components/ToolPolicies/ToolPoliciesPanel"; @@ -11,6 +12,7 @@ interface ToolPoliciesViewProps { } export default function ToolPoliciesView({ accessToken }: ToolPoliciesViewProps) { + const canViewToolPolicies = useCan("viewToolPolicies"); const [view, setView] = useState({ type: "overview" }); const handleSelectTool = (toolName: string) => { @@ -21,6 +23,15 @@ export default function ToolPoliciesView({ accessToken }: ToolPoliciesViewProps) setView({ type: "overview" }); }; + if (!canViewToolPolicies) { + return ( +
+

Tool Policies

+

Tool Policies is only available to admin users.

+
+ ); + } + return (
{view.type === "detail" ? ( diff --git a/ui/litellm-dashboard/src/components/leftnav.test.tsx b/ui/litellm-dashboard/src/components/leftnav.test.tsx index dc893643559..e07d0bb26eb 100644 --- a/ui/litellm-dashboard/src/components/leftnav.test.tsx +++ b/ui/litellm-dashboard/src/components/leftnav.test.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders } from "../../tests/test-utils"; import Sidebar, { menuGroups, getBreadcrumb } from "./leftnav"; @@ -201,6 +201,47 @@ describe("Sidebar (leftnav)", () => { }); }); + describe("capability-gated Tools children", () => { + const internalAuth = { + userId: "internal-user-id", + accessToken: "test-access-token", + userRole: "internal", + token: "test-token", + userEmail: "internal@example.com", + premiumUser: false, + disabledPersonalKeyCreation: false, + showSSOBanner: false, + }; + + afterEach(() => { + mockUseAuthorized.mockReset(); + }); + + it("should hide Tool Policies from internal users while keeping other Tools children", async () => { + mockUseAuthorized.mockReturnValue(internalAuth); + renderWithProviders(); + + act(() => { + fireEvent.click(screen.getByText("Tools")); + }); + await waitFor(() => { + expect(screen.getByText("Search Tools")).toBeInTheDocument(); + }); + expect(screen.queryByText("Tool Policies")).not.toBeInTheDocument(); + }); + + it("should show Tool Policies to admins", async () => { + renderWithProviders(); + + act(() => { + fireEvent.click(screen.getByText("Tools")); + }); + await waitFor(() => { + expect(screen.getByText("Tool Policies")).toBeInTheDocument(); + }); + }); + }); + it("should show Organizations tab for organization admins", () => { mockUseAuthorized.mockReturnValueOnce({ userId: "org-admin-user-id", diff --git a/ui/litellm-dashboard/src/components/leftnav.tsx b/ui/litellm-dashboard/src/components/leftnav.tsx index af76fccf9eb..cd92fc5bedb 100644 --- a/ui/litellm-dashboard/src/components/leftnav.tsx +++ b/ui/litellm-dashboard/src/components/leftnav.tsx @@ -64,6 +64,7 @@ import { import Link from "next/link"; import { useMemo, useState } from "react"; import { cn } from "@/lib/cva.config"; +import { rolesWithCapability } from "../utils/capabilities"; import { all_admin_roles, internalUserRoles, @@ -167,7 +168,13 @@ const menuGroups: MenuGroup[] = [ children: [ { key: "search-tools", page: "search-tools", label: "Search Tools", icon: }, { key: "vector-stores", page: "vector-stores", label: "Vector Stores", icon: }, - { key: "tool-policies", page: "tool-policies", label: "Tool Policies", icon: }, + { + key: "tool-policies", + page: "tool-policies", + label: "Tool Policies", + icon: , + roles: rolesWithCapability("viewToolPolicies"), + }, ], }, ], diff --git a/ui/litellm-dashboard/src/utils/capabilities.test.ts b/ui/litellm-dashboard/src/utils/capabilities.test.ts new file mode 100644 index 00000000000..84ceae16fc1 --- /dev/null +++ b/ui/litellm-dashboard/src/utils/capabilities.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; + +import { hasCapability, rolesWithCapability } from "./capabilities"; + +describe("hasCapability", () => { + it.each(["Admin", "Admin Viewer", "proxy_admin", "proxy_admin_viewer"])( + "should grant viewToolPolicies to %s", + (role) => { + expect(hasCapability(role, "viewToolPolicies")).toBe(true); + }, + ); + + it.each(["Internal User", "Internal Viewer", "App User", "Unknown Role", "", null, undefined])( + "should deny viewToolPolicies to %s", + (role) => { + expect(hasCapability(role, "viewToolPolicies")).toBe(false); + }, + ); +}); + +describe("rolesWithCapability", () => { + it("should return a copy so callers cannot mutate the capability map", () => { + const roles = rolesWithCapability("viewToolPolicies"); + const removed = roles.pop(); + expect(hasCapability(removed, "viewToolPolicies")).toBe(true); + }); +}); diff --git a/ui/litellm-dashboard/src/utils/capabilities.ts b/ui/litellm-dashboard/src/utils/capabilities.ts new file mode 100644 index 00000000000..77ead2568fb --- /dev/null +++ b/ui/litellm-dashboard/src/utils/capabilities.ts @@ -0,0 +1,12 @@ +import { all_admin_roles } from "./roles"; + +const CAPABILITY_ROLES = { + viewToolPolicies: all_admin_roles, +} as const satisfies Record; + +export type Capability = keyof typeof CAPABILITY_ROLES; + +export const hasCapability = (userRole: string | null | undefined, capability: Capability): boolean => + userRole != null && CAPABILITY_ROLES[capability].includes(userRole); + +export const rolesWithCapability = (capability: Capability): string[] => [...CAPABILITY_ROLES[capability]]; From a6b9cedd03a9bb738228bd141c021de0d6666c1b Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 4 Aug 2026 12:07:45 -0700 Subject: [PATCH 02/11] refactor(ui): inject the fetch client's base url instead of reading it at import api.ts read globalThis.location when the module loaded, which froze the base URL at import and pinned its test file to jsdom. The creation-time baseUrl and the middleware's runtime rebase were also two mechanisms doing overlapping work, and the rebase hand-copied eleven RequestInit fields on every call. Pass openapi-fetch's Request option instead, so the constructor applies whatever getRequestBaseUrl() returns at the moment the request is built. registerBaseUrlGetter is now the single source of the base URL, rebaseUrl and rebaseRequest are deleted, and the request is constructed once, so the init openapi-fetch assembled reaches the platform Request untouched. The abort signal is no longer copied by hand. This preserves behaviour rather than approximating it: getProxyBaseUrl() falls back to location.origin, so the runtime base was never empty in a browser and the old middleware already rebased every request, discarding the creation-time value each time. setupTests.ts gates its DOM-only tail behind a window check; setup files run for every environment, so that tail previously stopped any node-environment test file from loading. api.test.ts now runs under @vitest-environment node with its assertions intact and no location stub, plus regressions for per-call base resolution and abort forwarding. api.sameOrigin.test.ts covers the browser fallback to the page origin, which needs a DOM environment. --- .../src/lib/http/api.sameOrigin.test.ts | 46 ++++++ ui/litellm-dashboard/src/lib/http/api.test.ts | 44 +++++- ui/litellm-dashboard/src/lib/http/api.ts | 43 ++---- ui/litellm-dashboard/tests/setupTests.ts | 142 +++++++++--------- 4 files changed, 171 insertions(+), 104 deletions(-) create mode 100644 ui/litellm-dashboard/src/lib/http/api.sameOrigin.test.ts diff --git a/ui/litellm-dashboard/src/lib/http/api.sameOrigin.test.ts b/ui/litellm-dashboard/src/lib/http/api.sameOrigin.test.ts new file mode 100644 index 00000000000..094b279bdd9 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/api.sameOrigin.test.ts @@ -0,0 +1,46 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { fetchClient } from "./api"; +import { registerAuthTokenGetter, registerBaseUrlGetter, registerErrorHandler } from "./runtime"; + +const jsonResponse = (status: number, body: unknown): Response => + new Response(JSON.stringify(body), { status, headers: { "Content-Type": "application/json" } }); + +const capturingFetch = (response: Response) => { + const requests: Request[] = []; + const fetch = vi.fn(async (request: Request) => { + requests.push(request); + return response; + }); + return { fetch, requests }; +}; + +describe("typed api client on a same-origin deployment", () => { + beforeEach(() => { + registerAuthTokenGetter(() => null); + registerErrorHandler(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("sends requests to the page origin when no base url is registered", async () => { + registerBaseUrlGetter(() => ""); + const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] })); + + await fetchClient.GET("/model_group/info", { fetch }); + + expect(requests[0].url).toBe(`${window.location.origin}/model_group/info`); + }); + + it("prefers a registered cross-origin base over the page origin", async () => { + registerBaseUrlGetter(() => "https://proxy.example.com"); + const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] })); + + await fetchClient.GET("/model_group/info", { fetch }); + + expect(requests[0].url).toBe("https://proxy.example.com/model_group/info"); + expect(new URL(requests[0].url).origin).not.toBe(window.location.origin); + }); +}); diff --git a/ui/litellm-dashboard/src/lib/http/api.test.ts b/ui/litellm-dashboard/src/lib/http/api.test.ts index 7bbbf38da09..f7757e58e90 100644 --- a/ui/litellm-dashboard/src/lib/http/api.test.ts +++ b/ui/litellm-dashboard/src/lib/http/api.test.ts @@ -1,3 +1,4 @@ +// @vitest-environment node import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { fetchClient } from "./api"; import { @@ -36,7 +37,7 @@ const spyOnRequestConstruction = () => { describe("typed api client middleware", () => { beforeEach(() => { - registerBaseUrlGetter(() => ""); + registerBaseUrlGetter(() => "http://localhost:4000"); registerAuthHeaderNameGetter(() => "Authorization"); registerErrorHandler(() => {}); registerAuthTokenGetter(() => null); @@ -66,7 +67,7 @@ describe("typed api client middleware", () => { expect(requests[0].headers.get("Authorization")).toBeNull(); }); - it("rebases the request onto the registered base url, preserving path and query", async () => { + it("builds the request url from the registered base url, preserving path and query", async () => { registerBaseUrlGetter(() => "https://proxy.example.com/"); const { fetch, requests } = capturingFetch(jsonResponse(200, { data: [] })); @@ -90,7 +91,7 @@ describe("typed api client middleware", () => { expect(await requests[0].text()).toBe(JSON.stringify({ key_alias: "my-key" })); }); - it("keeps the POST body as bytes when rebasing onto a runtime base url", async () => { + it("keeps the POST body as bytes when a different runtime base url is registered", async () => { registerBaseUrlGetter(() => "https://proxy.example.com"); registerAuthTokenGetter(() => "sk-test"); const { streamBodiedInits } = spyOnRequestConstruction(); @@ -107,6 +108,43 @@ describe("typed api client middleware", () => { expect(await sent.text()).toBe(JSON.stringify({ key_alias: "my-key" })); }); + it("reads the base url on every call, so a base registered after import still takes effect", async () => { + const requests: Request[] = []; + const fetch = vi.fn(async (request: Request) => { + requests.push(request); + return jsonResponse(200, { data: [] }); + }); + + registerBaseUrlGetter(() => "https://first.example.com"); + await fetchClient.GET("/model_group/info", { fetch }); + registerBaseUrlGetter(() => "https://second.example.com"); + await fetchClient.GET("/model_group/info", { fetch }); + + expect(requests.map((request) => new URL(request.url).origin)).toEqual([ + "https://first.example.com", + "https://second.example.com", + ]); + }); + + it("forwards the caller's abort signal so an in-flight request can be cancelled", async () => { + const controller = new AbortController(); + const seen: Request[] = []; + const fetch = vi.fn( + (request: Request) => + new Promise((_resolve, reject) => { + seen.push(request); + request.signal.addEventListener("abort", () => reject(new DOMException("Aborted", "AbortError"))); + }), + ); + + const pending = fetchClient.GET("/model_group/info", { fetch, signal: controller.signal }); + await vi.waitFor(() => expect(seen).toHaveLength(1)); + controller.abort(); + + await expect(pending).rejects.toMatchObject({ name: "AbortError" }); + expect(seen[0].signal.aborted).toBe(true); + }, 5000); + it("maps a non-2xx response to an ApiError carrying status and the derived message", async () => { const { fetch } = capturingFetch(jsonResponse(403, { error: { message: "no access" } })); diff --git a/ui/litellm-dashboard/src/lib/http/api.ts b/ui/litellm-dashboard/src/lib/http/api.ts index 905fa045c26..ef628169ea1 100644 --- a/ui/litellm-dashboard/src/lib/http/api.ts +++ b/ui/litellm-dashboard/src/lib/http/api.ts @@ -4,38 +4,18 @@ import type { paths } from "./schema"; import { ApiError, deriveErrorMessage } from "./client"; import { getAuthHeaderName, getAuthToken, getRequestBaseUrl, reportError } from "./runtime"; -const rebaseUrl = (requestUrl: string, base: string): string => { - const { pathname, search } = new URL(requestUrl); - return `${base.replace(/\/+$/, "")}${pathname}${search}`; -}; +const resolveRequestBase = (): string => (getRequestBaseUrl() || globalThis.location?.origin || "").replace(/\/+$/, ""); -const rebaseRequest = async (request: Request, url: string): Promise => { - const init: RequestInit = { - method: request.method, - headers: request.headers, - body: request.body ? await request.arrayBuffer() : undefined, - mode: request.mode, - credentials: request.credentials, - cache: request.cache, - redirect: request.redirect, - referrer: request.referrer, - referrerPolicy: request.referrerPolicy, - integrity: request.integrity, - keepalive: request.keepalive, - signal: request.signal, - }; - return new Request(url, init); -}; +const BaseAwareRequest = function (url: string, init?: RequestInit): Request { + return new globalThis.Request(`${resolveRequestBase()}${url}`, init); +} as unknown as typeof Request; const middleware: Middleware = { - async onRequest({ request }) { - const base = getRequestBaseUrl(); - const next = base ? await rebaseRequest(request, rebaseUrl(request.url, base)) : request; + onRequest({ request }) { const token = getAuthToken(); if (token) { - next.headers.set(getAuthHeaderName(), `Bearer ${token}`); + request.headers.set(getAuthHeaderName(), `Bearer ${token}`); } - return next; }, async onResponse({ response }) { if (response.ok) return response; @@ -58,12 +38,13 @@ const middleware: Middleware = { * (`fetchClient.GET("/path", { params })`) and for imperative calls; path * params, query params, and request bodies are inferred from schema.d.ts. * - * The creation-time base is the current origin so request URLs are absolute; the - * middleware rebases each call onto the runtime base when one is registered (a - * split-origin proxy or worker URL), injects the auth header, and maps non-2xx - * responses to ApiError so query functions can just read `.data`. + * The base URL is injected, not fixed at import: every request is built against + * whatever registerBaseUrlGetter supplies at call time (a split-origin proxy or + * worker URL), falling back to the current origin. The middleware injects the + * auth header and maps non-2xx responses to ApiError so query functions can just + * read `.data`. */ -export const fetchClient = createFetchClient({ baseUrl: globalThis.location?.origin ?? "" }); +export const fetchClient = createFetchClient({ Request: BaseAwareRequest }); fetchClient.use(middleware); /** diff --git a/ui/litellm-dashboard/tests/setupTests.ts b/ui/litellm-dashboard/tests/setupTests.ts index a865d5c24fe..1ff0bed9862 100644 --- a/ui/litellm-dashboard/tests/setupTests.ts +++ b/ui/litellm-dashboard/tests/setupTests.ts @@ -183,78 +183,80 @@ vi.spyOn(Date.prototype, "toLocaleString").mockImplementation(function (this: Da return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`; }); -// Fixed matchMedia not found error in tests: https://github.com/vitest-dev/vitest/issues/821 -Object.defineProperty(window, "matchMedia", { - writable: true, - value: (query: string) => ({ - matches: false, - media: query, - onchange: null, - addListener: vi.fn(), - removeListener: vi.fn(), - addEventListener: vi.fn(), - removeEventListener: vi.fn(), - dispatchEvent: vi.fn(), - }), -}); +if (typeof window !== "undefined") { + // Fixed matchMedia not found error in tests: https://github.com/vitest-dev/vitest/issues/821 + Object.defineProperty(window, "matchMedia", { + writable: true, + value: (query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + }), + }); -// Silence jsdom "getComputedStyle with pseudo-elements" not implemented warnings -// by ignoring the second argument and delegating to the native implementation. -const realGetComputedStyle = window.getComputedStyle.bind(window); -window.getComputedStyle = ((elt: Element) => realGetComputedStyle(elt)) as any; + // Silence jsdom "getComputedStyle with pseudo-elements" not implemented warnings + // by ignoring the second argument and delegating to the native implementation. + const realGetComputedStyle = window.getComputedStyle.bind(window); + window.getComputedStyle = ((elt: Element) => realGetComputedStyle(elt)) as any; -// Avoid "navigation to another Document" warnings when clicking with blob: URLs -// used by download flows in tests. -Object.defineProperty(HTMLAnchorElement.prototype, "click", { - configurable: true, - writable: true, - value: vi.fn(), -}); + // Avoid "navigation to another Document" warnings when clicking with blob: URLs + // used by download flows in tests. + Object.defineProperty(HTMLAnchorElement.prototype, "click", { + configurable: true, + writable: true, + value: vi.fn(), + }); -if (!document.getAnimations) { - document.getAnimations = () => []; -} - -// Stub URL.revokeObjectURL so vi.spyOn can intercept it in tests -if (!URL.revokeObjectURL) { - URL.revokeObjectURL = () => {}; -} - -// Mock ResizeObserver for components that use it (recharts, Tremor UI components). -// JSDOM has no layout, so for observers inside a shadcn ChartContainer ([data-slot="chart"]) -// the mock immediately reports a fixed 800x400 box; recharts renders nothing until it -// observes a size. Scoped to chart subtrees only: firing for every observer re-enters -// React mid-effect for tremor/headlessui consumers whose tests assume the old no-op -// (chart text would duplicate getByText targets, popover clicks go stale). Widen or -// drop the scoping once tremor is gone. -const MOCK_RESIZE_BOX = { inlineSize: 800, blockSize: 400 }; -const MOCK_RESIZE_RECT: DOMRectReadOnly = { - width: 800, - height: 400, - top: 0, - left: 0, - bottom: 400, - right: 800, - x: 0, - y: 0, - toJSON: () => ({}), -}; -global.ResizeObserver = class ResizeObserver { - private readonly callback: ResizeObserverCallback; - constructor(callback: ResizeObserverCallback) { - this.callback = callback; + if (!document.getAnimations) { + document.getAnimations = () => []; } - observe(target: Element) { - if (!target.closest('[data-slot="chart"]')) return; - const entry: ResizeObserverEntry = { - target, - contentRect: MOCK_RESIZE_RECT, - borderBoxSize: [MOCK_RESIZE_BOX], - contentBoxSize: [MOCK_RESIZE_BOX], - devicePixelContentBoxSize: [MOCK_RESIZE_BOX], - }; - this.callback([entry], this); + + // Stub URL.revokeObjectURL so vi.spyOn can intercept it in tests + if (!URL.revokeObjectURL) { + URL.revokeObjectURL = () => {}; } - unobserve() {} - disconnect() {} -}; + + // Mock ResizeObserver for components that use it (recharts, Tremor UI components). + // JSDOM has no layout, so for observers inside a shadcn ChartContainer ([data-slot="chart"]) + // the mock immediately reports a fixed 800x400 box; recharts renders nothing until it + // observes a size. Scoped to chart subtrees only: firing for every observer re-enters + // React mid-effect for tremor/headlessui consumers whose tests assume the old no-op + // (chart text would duplicate getByText targets, popover clicks go stale). Widen or + // drop the scoping once tremor is gone. + const MOCK_RESIZE_BOX = { inlineSize: 800, blockSize: 400 }; + const MOCK_RESIZE_RECT: DOMRectReadOnly = { + width: 800, + height: 400, + top: 0, + left: 0, + bottom: 400, + right: 800, + x: 0, + y: 0, + toJSON: () => ({}), + }; + global.ResizeObserver = class ResizeObserver { + private readonly callback: ResizeObserverCallback; + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + } + observe(target: Element) { + if (!target.closest('[data-slot="chart"]')) return; + const entry: ResizeObserverEntry = { + target, + contentRect: MOCK_RESIZE_RECT, + borderBoxSize: [MOCK_RESIZE_BOX], + contentBoxSize: [MOCK_RESIZE_BOX], + devicePixelContentBoxSize: [MOCK_RESIZE_BOX], + }; + this.callback([entry], this); + } + unobserve() {} + disconnect() {} + }; +} From d158cf187bcb11d0ed3832bf7f689ec02f96d90c Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 4 Aug 2026 13:13:04 -0700 Subject: [PATCH 03/11] refactor(ui): move request base resolution into the shared resolveApiBase module api.ts owned the base-vs-origin precedence, the trailing-slash trim and the base+path join inline. That logic belongs with the rest of base resolution and was only reachable through a fetch client, so it could not be tested directly. Extract resolveRequestUrl into resolveApiBase.ts with its own unit tests. api.ts now only wires the shared resolver into openapi-fetch's Request option. No behaviour change: same precedence, same trimming, same output. --- ui/litellm-dashboard/src/lib/http/api.ts | 9 +++-- .../src/lib/http/resolveApiBase.test.ts | 38 ++++++++++++++++++- .../src/lib/http/resolveApiBase.ts | 12 ++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/ui/litellm-dashboard/src/lib/http/api.ts b/ui/litellm-dashboard/src/lib/http/api.ts index ef628169ea1..508a27db78d 100644 --- a/ui/litellm-dashboard/src/lib/http/api.ts +++ b/ui/litellm-dashboard/src/lib/http/api.ts @@ -3,11 +3,14 @@ import createQueryClient from "openapi-react-query"; import type { paths } from "./schema"; import { ApiError, deriveErrorMessage } from "./client"; import { getAuthHeaderName, getAuthToken, getRequestBaseUrl, reportError } from "./runtime"; - -const resolveRequestBase = (): string => (getRequestBaseUrl() || globalThis.location?.origin || "").replace(/\/+$/, ""); +import { resolveRequestUrl } from "./resolveApiBase"; const BaseAwareRequest = function (url: string, init?: RequestInit): Request { - return new globalThis.Request(`${resolveRequestBase()}${url}`, init); + const target = resolveRequestUrl(url, { + registeredBase: getRequestBaseUrl(), + pageOrigin: globalThis.location?.origin, + }); + return new globalThis.Request(target, init); } as unknown as typeof Request; const middleware: Middleware = { diff --git a/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts b/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts index 988b88cf07d..6b41ef320fc 100644 --- a/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts +++ b/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts @@ -1,5 +1,41 @@ import { describe, expect, it } from "vitest"; -import { resolveApiBase } from "./resolveApiBase"; +import { resolveApiBase, resolveRequestUrl } from "./resolveApiBase"; + +describe("resolveRequestUrl", () => { + it("targets the registered base when one is registered", () => { + expect( + resolveRequestUrl("/model_group/info", { + registeredBase: "https://proxy.example.com", + pageOrigin: "http://localhost:3000", + }), + ).toBe("https://proxy.example.com/model_group/info"); + }); + + it("falls back to the page origin when no base is registered", () => { + expect(resolveRequestUrl("/model_group/info", { registeredBase: "", pageOrigin: "http://localhost:3000" })).toBe( + "http://localhost:3000/model_group/info", + ); + }); + + it("trims a trailing slash so the path is not doubled up", () => { + expect(resolveRequestUrl("/model_group/info", { registeredBase: "https://proxy.example.com/" })).toBe( + "https://proxy.example.com/model_group/info", + ); + }); + + it("keeps the path relative when neither a base nor an origin is available", () => { + expect(resolveRequestUrl("/model_group/info", {})).toBe("/model_group/info"); + expect(resolveRequestUrl("/model_group/info", { registeredBase: null, pageOrigin: null })).toBe( + "/model_group/info", + ); + }); + + it("preserves an already-serialized query string", () => { + expect( + resolveRequestUrl("/model_group/info?model_group=gpt-4o", { registeredBase: "https://proxy.example.com" }), + ).toBe("https://proxy.example.com/model_group/info?model_group=gpt-4o"); + }); +}); describe("resolveApiBase", () => { describe("same-origin (no explicit base)", () => { diff --git a/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts b/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts index 1d40784af92..661f9bb9eca 100644 --- a/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts +++ b/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts @@ -33,3 +33,15 @@ export const resolveApiBase = ({ explicitBase, serverRootPath }: ApiBaseInputs): if (rootPath === "" || base.endsWith(rootPath)) return base; return `${base}${rootPath}`; }; + +export interface RequestUrlInputs { + /** Base registered at runtime (a split-origin proxy or worker URL); empty means none. */ + registeredBase?: string | null; + /** Origin of the page issuing the request; the same-origin fallback. */ + pageOrigin?: string | null; +} + +export const resolveRequestUrl = (path: string, { registeredBase, pageOrigin }: RequestUrlInputs): string => { + const base = (registeredBase || pageOrigin || "").replace(/\/+$/, ""); + return `${base}${path}`; +}; From 0c3020dae766da9cadeb9209b96158f10f492863 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 4 Aug 2026 14:10:26 -0700 Subject: [PATCH 04/11] test(ui): pin formatted Org Admin as denied for viewToolPolicies Backend parity check: a membership-granted org admin key gets 401 on /v1/tool/list (route absent from org_admin_allowed_routes), so the capability map denying the formatted Org Admin runtime value is the intended behavior, now pinned by a test --- ui/litellm-dashboard/src/utils/capabilities.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/utils/capabilities.test.ts b/ui/litellm-dashboard/src/utils/capabilities.test.ts index 84ceae16fc1..f48609b0b9d 100644 --- a/ui/litellm-dashboard/src/utils/capabilities.test.ts +++ b/ui/litellm-dashboard/src/utils/capabilities.test.ts @@ -10,7 +10,7 @@ describe("hasCapability", () => { }, ); - it.each(["Internal User", "Internal Viewer", "App User", "Unknown Role", "", null, undefined])( + it.each(["Internal User", "Internal Viewer", "App User", "Org Admin", "Unknown Role", "", null, undefined])( "should deny viewToolPolicies to %s", (role) => { expect(hasCapability(role, "viewToolPolicies")).toBe(false); From 22a1c3060391a60e7456eaf746c655ff5002e74d Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:59:56 -0700 Subject: [PATCH 05/11] fix(lint): move the basedpyright heap flag into the type check gate The 12 GB NODE_OPTIONS setting lived only in the Makefile export and the CI env line, so any hand-run gate pipeline forgot it and node OOMed at the ~4 GB default after 80 seconds, with || true feeding the gate empty output. The gate now spawns basedpyright itself for both the head and base passes, appends the heap flag last so it wins node's last-flag-wins resolution while preserving other caller flags, and fails loudly on crash exit codes instead of reading them as zero errors. --- .github/workflows/test-linting.yml | 3 +- Makefile | 6 +- scripts/type_check_gate.py | 67 ++++++++++++++++------ tests/test_litellm/test_type_check_gate.py | 43 ++++++++++++++ 4 files changed, 94 insertions(+), 25 deletions(-) diff --git a/.github/workflows/test-linting.yml b/.github/workflows/test-linting.yml index 8d2b2c2f972..b539ec4be88 100644 --- a/.github/workflows/test-linting.yml +++ b/.github/workflows/test-linting.yml @@ -104,9 +104,8 @@ jobs: - name: Check basedpyright budget (delta vs base) env: BASE_SHA: ${{ github.event.pull_request.base.sha }} - NODE_OPTIONS: --max-old-space-size=12288 run: | - (uv run --no-sync basedpyright --outputjson || true) | uv run --no-sync python scripts/type_check_gate.py --base "$BASE_SHA" + uv run --no-sync python scripts/type_check_gate.py --base "$BASE_SHA" - name: Check tests/e2e basedpyright (zero errors) env: diff --git a/Makefile b/Makefile index f4494680e13..0b59b2f3e95 100644 --- a/Makefile +++ b/Makefile @@ -176,10 +176,8 @@ lint-ruff-FULL-dev: install-dev if [ -n "$$files" ]; then echo "$$files" | xargs $(UV_RUN) ruff check; \ else echo "No changed .py files to check."; fi -lint-basedpyright lint-basedpyright-budget-update: export NODE_OPTIONS := --max-old-space-size=12288 - lint-basedpyright: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE) - ($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging + $(UV_RUN) python scripts/type_check_gate.py --base origin/litellm_internal_staging lint-e2e-basedpyright: $(LINT_E2E_DEP_INSTALL) $(UV_RUN) basedpyright tests/e2e @@ -192,7 +190,7 @@ lint-type-discipline: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE) # --update lowers each limit by what this branch fixed since its branch point, so # it needs the base ref fetched to resolve the merge-base. lint-basedpyright-budget-update: install-dev lint-fetch-base - ($(UV_RUN) basedpyright --outputjson || true) | $(UV_RUN) python scripts/type_check_gate.py --update + $(UV_RUN) python scripts/type_check_gate.py --update lint-format: format-check diff --git a/scripts/type_check_gate.py b/scripts/type_check_gate.py index 2c5306cec7d..1bce746b5e2 100644 --- a/scripts/type_check_gate.py +++ b/scripts/type_check_gate.py @@ -12,12 +12,16 @@ a red once two PRs each land near the limit and their sum crosses it: the bystander's count equals its base, so it is spared, while any PR that actually grows the rule past its limit still fails. -Head counts are read from stdin (the caller runs basedpyright once and pipes -``--outputjson`` in). The base count only matters once some rule is over its -limit, so when none is the base pass is skipped outright. When it is needed, it -is a second basedpyright pass over a detached worktree at the merge-base, run -under the same environment so import resolution matches, and its per-rule -counts are cached under the repo's git common dir keyed by merge-base commit, +The gate runs basedpyright itself, for both the head and the base pass, with +``NODE_OPTIONS`` raised to the heap this repo needs: basedpyright's node +process OOMs at the ~4 GB default, and when callers had to remember the flag, +every hand-copied pipeline (Makefile, CI, a dev running the recipe by hand) +was one forgotten env line away from an 80-second crash. The base count only +matters once some rule is over its limit, so when none is the base pass is +skipped outright. When it is needed, it is a second basedpyright pass over a +detached worktree at the merge-base, run under the same environment so import +resolution matches, and its per-rule counts are cached under the repo's git +common dir keyed by merge-base commit, ``pyrightconfig.json``, and ``uv.lock``, so re-runs against the same branch point pay for it once. ``--update`` ratchets each rule's ``limit`` down by the number of errors this branch fixed relative to its branch point (the merge-base), @@ -51,6 +55,11 @@ UV_LOCK = REPO_ROOT / "uv.lock" DEFAULT_BASE = "origin/litellm_internal_staging" CACHE_FILE_PREFIX = "basedpyright-base-" +# basedpyright's node process needs more than the ~4 GB default heap on this +# repo; appended last so it wins node's last-flag-wins resolution over any +# caller-set value while preserving the caller's other NODE_OPTIONS flags. +NODE_HEAP_OPTION = "--max-old-space-size=12288" + # Bucket for a basedpyright diagnostic with no `rule`. Counted so it's gated. UNCODED = "" @@ -107,6 +116,29 @@ def _run(cmd: list[str], cwd: Path = REPO_ROOT) -> str: return proc.stdout +def node_options_with_heap(base_env: Mapping[str, str]) -> str: + return f"{base_env.get('NODE_OPTIONS', '')} {NODE_HEAP_OPTION}".strip() + + +def run_basedpyright(cwd: Path = REPO_ROOT) -> str: + """One basedpyright pass over `cwd` with the raised node heap exported. + + Exit 0 (clean) and 1 (errors found) are both output-bearing runs; anything + else is a crash and fails loudly instead of reading as zero errors.""" + exe = shutil.which("basedpyright") or "basedpyright" + proc = subprocess.run( + [exe, "--outputjson"], + cwd=cwd, + capture_output=True, + text=True, + env={**os.environ, "NODE_OPTIONS": node_options_with_heap(os.environ)}, + ) + if proc.returncode not in (0, 1): + sys.stderr.write(proc.stderr) + raise SystemExit(f"basedpyright exited {proc.returncode}") + return proc.stdout + + @contextlib.contextmanager def _temp_worktree(ref: str) -> Iterator[Path]: parent = Path(tempfile.mkdtemp(prefix="bpr_base_")) @@ -128,13 +160,9 @@ def base_counts(ref: str) -> dict[str, int]: """basedpyright error counts per rule for the merge-base tree. The head config is copied in so the base is judged by today's rules, and the run uses the head environment's basedpyright (on PATH) so imports resolve the same.""" - exe = shutil.which("basedpyright") or "basedpyright" with _temp_worktree(ref) as worktree: shutil.copy(PYRIGHT_CONFIG, worktree / "pyrightconfig.json") - proc = subprocess.run( - [exe, "--outputjson"], cwd=worktree, capture_output=True, text=True - ) - return count_basedpyright(proc.stdout, root=worktree) + return count_basedpyright(run_basedpyright(worktree), root=worktree) def over_ceiling( @@ -259,9 +287,10 @@ def is_vacuous_run( counts: Mapping[str, int], budget: Mapping[str, Mapping[str, int]] ) -> bool: """True when nothing was parsed but the budget expects errors -- the - signature of a type checker that crashed or produced no output. The CI pipe - swallows the tool's exit code (`tool || true`), so without this guard an - empty run would clear every limit and pass silently.""" + signature of a type checker that produced no output. `run_basedpyright` + already fails crash exit codes, so this guards the remaining case: a run + that exits cleanly while emitting nothing, which would otherwise clear + every limit and pass silently.""" return not counts and any(spec["limit"] for spec in budget.values()) @@ -289,7 +318,7 @@ def ratcheted_budget( def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None: """Ratchet each rule's limit down by the errors this branch fixed. - `current` is the working-tree count (piped in); the reference count comes + `current` is the working-tree count; the reference count comes from a second basedpyright pass over a detached worktree at the branch point (the merge-base with `base_ref`), so a branch's fixes tighten its own ceilings by exactly what they cleared since it diverged, and limits never rise. @@ -305,9 +334,8 @@ def cmd_update(current: Mapping[str, int], base_ref: str = DEFAULT_BASE) -> None ) -def cmd_check(base_ref: str) -> None: +def cmd_check(head: Mapping[str, int], base_ref: str) -> None: budget = json.loads(BUDGET_PATH.read_text()) - head = count_basedpyright(sys.stdin.read()) if is_vacuous_run(head, budget): expected = sum(spec["limit"] for spec in budget.values()) print( @@ -355,10 +383,11 @@ def main() -> None: parser.add_argument("--base", default=DEFAULT_BASE) parser.add_argument("--update", action="store_true") args = parser.parse_args() + head = count_basedpyright(run_basedpyright()) if args.update: - cmd_update(count_basedpyright(sys.stdin.read()), args.base) + cmd_update(head, args.base) else: - cmd_check(args.base) + cmd_check(head, args.base) if __name__ == "__main__": diff --git a/tests/test_litellm/test_type_check_gate.py b/tests/test_litellm/test_type_check_gate.py index 66a28360af9..08813d5b0b0 100644 --- a/tests/test_litellm/test_type_check_gate.py +++ b/tests/test_litellm/test_type_check_gate.py @@ -1,5 +1,6 @@ import importlib.util import json +import os from pathlib import Path _MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "type_check_gate.py" @@ -69,6 +70,48 @@ def test_symlinked_root_keeps_diagnostics_in_tree(tmp_path): assert gate.count_basedpyright(payload, root=link) == {"reportArgumentType": 1} +def test_node_options_with_heap_sets_the_flag_in_a_bare_env(): + assert gate.node_options_with_heap({}) == gate.NODE_HEAP_OPTION + + +def test_node_options_with_heap_appends_after_caller_flags_so_it_wins(): + # node resolves a repeated --max-old-space-size last-wins, so ours must come + # after any caller-set value while keeping their other flags. + merged = gate.node_options_with_heap( + {"NODE_OPTIONS": "--max-old-space-size=4096 --no-warnings"} + ) + assert merged == f"--max-old-space-size=4096 --no-warnings {gate.NODE_HEAP_OPTION}" + + +def _stub_basedpyright(tmp_path, monkeypatch, script_body): + stub = tmp_path / "basedpyright" + stub.write_text(f"#!/bin/sh\n{script_body}\n") + stub.chmod(0o755) + monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep) + + +def test_run_basedpyright_exports_the_raised_heap_to_the_child(tmp_path, monkeypatch): + captured = tmp_path / "node_options.txt" + _stub_basedpyright( + tmp_path, + monkeypatch, + f'echo "$NODE_OPTIONS" > "{captured}"\necho \'{{"generalDiagnostics": []}}\'', + ) + monkeypatch.delenv("NODE_OPTIONS", raising=False) + assert json.loads(gate.run_basedpyright(cwd=tmp_path)) == {"generalDiagnostics": []} + assert captured.read_text().strip() == gate.NODE_HEAP_OPTION + + +def test_run_basedpyright_fails_loudly_on_a_crash_exit_code(tmp_path, monkeypatch): + import pytest + + # 134 is SIGABRT, what node dies with on a heap OOM; it must never read as a + # clean zero-error run. + _stub_basedpyright(tmp_path, monkeypatch, "exit 134") + with pytest.raises(SystemExit): + gate.run_basedpyright(cwd=tmp_path) + + def test_at_or_under_ceiling_passes(): budget = {"no-any-return": {"limit": 5}} assert gate.evaluate({"no-any-return": 5}, {}, budget) == [] From 27caf2892494f5102361f386243e177e51dad444 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:15:07 -0700 Subject: [PATCH 06/11] chore: remove unused .flake8 config and flake8 dev dependency --- .flake8 | 46 ---------------------------------------------- pyproject.toml | 1 - uv.lock | 36 +----------------------------------- 3 files changed, 1 insertion(+), 82 deletions(-) delete mode 100644 .flake8 diff --git a/.flake8 b/.flake8 deleted file mode 100644 index afd4596076b..00000000000 --- a/.flake8 +++ /dev/null @@ -1,46 +0,0 @@ -[flake8] -ignore = - # The following ignores can be removed when formatting using black - W191,W291,W292,W293,W391,W504 - E101,E111,E114,E116,E117,E121,E122,E123,E124,E125,E126,E127,E128,E129,E131, - E201,E202,E221,E222,E225,E226,E231,E241,E251,E252,E261,E265,E271,E272,E275, - E301,E302,E303,E305,E306, - # line break before binary operator - W503, - # inline comment should start with '# ' - E262, - # too many leading '#' for block comment - E266, - # multiple imports on one line - E401, - # module level import not at top of file - E402, - # Line too long (82 > 79 characters) - E501, - # comparison to None should be 'if cond is None:' - E711, - # comparison to True should be 'if cond is True:' or 'if cond:' - E712, - # do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` - E721, - # do not use bare 'except' - E722, - # x is imported but unused - F401, - # 'from . import *' used; unable to detect undefined names - F403, - # x may be undefined, or defined from star imports: - F405, - # f-string is missing placeholders - F541, - # dictionary key '' repeated with different values - F601, - # redefinition of unused x from line 123 - F811, - # undefined name x - F821, - # local variable x is assigned to but never used - F841, - -# https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8 -extend-ignore = E203 diff --git a/pyproject.toml b/pyproject.toml index 0f2ab412fe4..414b09eb3b4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -164,7 +164,6 @@ litellm-proxy = "litellm.proxy.client.cli:cli" [dependency-groups] dev = [ "diff-cover==9.7.2", - "flake8==7.3.0", "basedpyright==1.39.7", "pytest==9.0.3", "pytest-mock==3.15.1", diff --git a/uv.lock b/uv.lock index 21964204a65..9c2897b5e4f 100644 --- a/uv.lock +++ b/uv.lock @@ -10,7 +10,7 @@ resolution-markers = [ ] [options] -exclude-newer = "2026-08-02T01:44:17.274352Z" +exclude-newer = "2026-08-02T02:14:05.876141Z" exclude-newer-span = "P3D" [manifest] @@ -1982,20 +1982,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/81/47/dd9a212ef6e343a6857485ffe25bba537304f1913bdbed446a23f7f592e1/filelock-3.29.0-py3-none-any.whl", hash = "sha256:96f5f6344709aa1572bbf631c640e4ebeeb519e08da902c39a001882f30ac258", size = 39812, upload-time = "2026-04-19T15:39:08.752Z" }, ] -[[package]] -name = "flake8" -version = "7.3.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "mccabe" }, - { name = "pycodestyle" }, - { name = "pyflakes" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/9b/af/fbfe3c4b5a657d79e5c47a2827a362f9e1b763336a52f926126aa6dc7123/flake8-7.3.0.tar.gz", hash = "sha256:fe044858146b9fc69b551a4b490d69cf960fcb78ad1edcb84e7fbb1b4a8e3872", size = 48326, upload-time = "2025-06-20T19:31:35.838Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/9f/56/13ab06b4f93ca7cac71078fbe37fcea175d3216f31f85c3168a6bbd0bb9a/flake8-7.3.0-py2.py3-none-any.whl", hash = "sha256:b9696257b9ce8beb888cdbe31cf885c90d31928fe202be0889a7cdafad32f01e", size = 57922, upload-time = "2025-06-20T19:31:34.425Z" }, -] - [[package]] name = "flask" version = "3.1.3" @@ -4365,7 +4351,6 @@ dev = [ { name = "diff-cover" }, { name = "fakeredis" }, { name = "fastapi-offline" }, - { name = "flake8" }, { name = "langfuse" }, { name = "openapi-core" }, { name = "opentelemetry-api" }, @@ -4545,7 +4530,6 @@ dev = [ { name = "diff-cover", specifier = "==9.7.2" }, { name = "fakeredis", specifier = "==2.34.1" }, { name = "fastapi-offline", specifier = "==1.7.6" }, - { name = "flake8", specifier = "==7.3.0" }, { name = "langfuse", specifier = "==2.59.7" }, { name = "openapi-core", specifier = "==0.22.0" }, { name = "opentelemetry-api", specifier = "==1.28.0" }, @@ -7207,15 +7191,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/47/8d/d529b5d697919ba8c11ad626e835d4039be708a35b0d22de83a269a6682c/pyasn1_modules-0.4.2-py3-none-any.whl", hash = "sha256:29253a9207ce32b64c3ac6600edc75368f98473906e8fd1043bd6b5b1de2c14a", size = 181259, upload-time = "2025-03-28T02:41:19.028Z" }, ] -[[package]] -name = "pycodestyle" -version = "2.14.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/11/e0/abfd2a0d2efe47670df87f3e3a0e2edda42f055053c85361f19c0e2c1ca8/pycodestyle-2.14.0.tar.gz", hash = "sha256:c4b5b517d278089ff9d0abdec919cd97262a3367449ea1c8b49b91529167b783", size = 39472, upload-time = "2025-06-20T18:49:48.75Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/d7/27/a58ddaf8c588a3ef080db9d0b7e0b97215cee3a45df74f3a94dbbf5c893a/pycodestyle-2.14.0-py2.py3-none-any.whl", hash = "sha256:dd6bf7cb4ee77f8e016f9c8e74a35ddd9f67e1d5fd4184d86c3b98e07099f42d", size = 31594, upload-time = "2025-06-20T18:49:47.491Z" }, -] - [[package]] name = "pycparser" version = "3.0" @@ -7387,15 +7362,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a0/c4/b4d4827c93ef43c01f599ef31453ccc1c132b353284fc6c87d535c233129/pyee-13.0.1-py3-none-any.whl", hash = "sha256:af2f8fede4171ef667dfded53f96e2ed0d6e6bd7ee3bb46437f77e3b57689228", size = 15659, upload-time = "2026-02-14T21:12:26.263Z" }, ] -[[package]] -name = "pyflakes" -version = "3.4.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/45/dc/fd034dc20b4b264b3d015808458391acbf9df40b1e54750ef175d39180b1/pyflakes-3.4.0.tar.gz", hash = "sha256:b24f96fafb7d2ab0ec5075b7350b3d2d2218eab42003821c06344973d3ea2f58", size = 64669, upload-time = "2025-06-20T18:45:27.834Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/c2/2f/81d580a0fb83baeb066698975cb14a618bdbed7720678566f1b046a95fe8/pyflakes-3.4.0-py2.py3-none-any.whl", hash = "sha256:f742a7dbd0d9cb9ea41e9a24a918996e8170c799fa528688d40dd582c8265f4f", size = 63551, upload-time = "2025-06-20T18:45:26.937Z" }, -] - [[package]] name = "pygithub" version = "2.8.1" From 4a2ceed595e01ab7a8370f99a3bb6102fe045b47 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:11:12 -0700 Subject: [PATCH 07/11] Stop advising a pre-commit re-run for stale dashboard API types The stale-types failure already writes the regenerated schema.d.ts to the working tree, and staging it cannot introduce a new failure: the file is listed in .prettierignore and the eslint config ignores, so no lint pass sees it, and gen:api derives it purely from the Python proxy code, so a second regeneration is a no-op. The only reason left to re-run is when other checks also failed, so say exactly that in the script message and CLAUDE.md instead of prescribing an unconditional re-run. --- CLAUDE.md | 2 +- scripts/pre_commit_lint.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dd245fd6f4a..12967d00765 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,7 @@ Python max line length is 120, not 88 On a fresh worktree or clone, run `make bootstrap` before anything else. It provisions everything tests, `make pre-commit`, and a local proxy need -Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed. It only runs when there are staged frontend and/or backend changes and calculates violations, generates types, etc. based on the worktree, so stage what you need or stash/delete unwanted files in litellm/ or ui/ (where backend and frontend lint run, respectively) before running it. If it fails because dashboard api types are stale, it already regenerated them for you. You just need to stage the schema.d.ts, re-run `make pre-commit` to confirm it passes, and commit +Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed. It only runs when there are staged frontend and/or backend changes and calculates violations, generates types, etc. based on the worktree, so stage what you need or stash/delete unwanted files in litellm/ or ui/ (where backend and frontend lint run, respectively) before running it. If it fails because dashboard api types are stale, it already regenerated them for you. You just need to stage the schema.d.ts and commit; no re-run is needed (the file is prettier/eslint-ignored and regeneration is deterministic) unless other checks failed too When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-budget.json`, or `basedpyright-code-budget.json`, run `make lint-budget-update` and commit the lowered limits so the ceilings ratchet down instead of leaving stale headroom. It measures the working tree, so it must contain exactly the fixes you're committing diff --git a/scripts/pre_commit_lint.sh b/scripts/pre_commit_lint.sh index 150a4bbf9de..8e148be399f 100755 --- a/scripts/pre_commit_lint.sh +++ b/scripts/pre_commit_lint.sh @@ -149,7 +149,7 @@ if [ -n "$spec_files" ]; then status=1 elif ( cd ui/litellm-dashboard && LITELLM_PYTHON="uv run --no-sync python" npm run gen:api ); then if ! git diff --quiet -- ui/litellm-dashboard/src/lib/http/schema.d.ts; then - echo "✗ Dashboard API types are stale; regenerated src/lib/http/schema.d.ts. Stage it and re-run make pre-commit." >&2 + echo "✗ Dashboard API types are stale; regenerated src/lib/http/schema.d.ts. Stage it and commit; re-run make pre-commit only if other checks failed too." >&2 status=1 fi else From 38cd75342d244cea779c6d158578287298e0739b Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:30:51 -0700 Subject: [PATCH 08/11] Note the one case where staging schema.d.ts changes what runs For a backend-only commit, staging the regenerated schema.d.ts newly satisfies the ui file triggers, so the folder-wide dashboard lint budgets run locally for the first time and CI's frontend-lint job (budgets plus knip) activates on the PR. Those can only fail from pre-existing dashboard-tree state, never from the regenerated file, but the guidance should say so instead of implying a re-run is always redundant. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 12967d00765..5a30d7bb4cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,7 +41,7 @@ Python max line length is 120, not 88 On a fresh worktree or clone, run `make bootstrap` before anything else. It provisions everything tests, `make pre-commit`, and a local proxy need -Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed. It only runs when there are staged frontend and/or backend changes and calculates violations, generates types, etc. based on the worktree, so stage what you need or stash/delete unwanted files in litellm/ or ui/ (where backend and frontend lint run, respectively) before running it. If it fails because dashboard api types are stale, it already regenerated them for you. You just need to stage the schema.d.ts and commit; no re-run is needed (the file is prettier/eslint-ignored and regeneration is deterministic) unless other checks failed too +Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed. It only runs when there are staged frontend and/or backend changes and calculates violations, generates types, etc. based on the worktree, so stage what you need or stash/delete unwanted files in litellm/ or ui/ (where backend and frontend lint run, respectively) before running it. If it fails because dashboard api types are stale, it already regenerated them for you. You just need to stage the schema.d.ts and commit; the regenerated file itself can't fail anything (it's prettier/eslint-ignored and regeneration is deterministic), so re-run only if other checks failed too, or if no ui/ files were staged before: staging schema.d.ts newly triggers the folder-wide dashboard lint budgets locally and CI's frontend-lint job (budgets plus knip), which can only fail if the dashboard tree was already broken independently of your change When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-budget.json`, or `basedpyright-code-budget.json`, run `make lint-budget-update` and commit the lowered limits so the ceilings ratchet down instead of leaving stale headroom. It measures the working tree, so it must contain exactly the fixes you're committing From a0d08b81436bec473caaf0525713bac5cd1e7682 Mon Sep 17 00:00:00 2001 From: Mateo Wang <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:35:22 -0700 Subject: [PATCH 09/11] chore: remove pre-commit and bootstrap advisories They were taking too long --- CLAUDE.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5a30d7bb4cf..209d9aaf326 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,10 +39,6 @@ Don't hesitate to use values in .env to get needed API keys and other secrets, a Python max line length is 120, not 88 -On a fresh worktree or clone, run `make bootstrap` before anything else. It provisions everything tests, `make pre-commit`, and a local proxy need - -Run tests before you commit. Also, run `make pre-commit` right before each commit, which generates types (as needed) and formats/lints your code. Any errors found must be fixed. It only runs when there are staged frontend and/or backend changes and calculates violations, generates types, etc. based on the worktree, so stage what you need or stash/delete unwanted files in litellm/ or ui/ (where backend and frontend lint run, respectively) before running it. If it fails because dashboard api types are stale, it already regenerated them for you. You just need to stage the schema.d.ts and commit; the regenerated file itself can't fail anything (it's prettier/eslint-ignored and regeneration is deterministic), so re-run only if other checks failed too, or if no ui/ files were staged before: staging schema.d.ts newly triggers the folder-wide dashboard lint budgets locally and CI's frontend-lint job (budgets plus knip), which can only fail if the dashboard tree was already broken independently of your change - When you fix violations gated by `ruff-strict-budget.json`, `type-discipline-budget.json`, or `basedpyright-code-budget.json`, run `make lint-budget-update` and commit the lowered limits so the ceilings ratchet down instead of leaving stale headroom. It measures the working tree, so it must contain exactly the fixes you're committing If you're trying to create a new function that relies on untyped stuff, instead of adding more Any's and pushing `reportAny` / `reportExplicitAny` closer to their basedpyright ceilings, just validate it in the caller with Pydantic (a model or `TypeAdapter` that returns the typed thing or raises will do) and then pass the now typed variable in From 1e265dc86cb616b0402ba7f03e8070add0147eda Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Tue, 4 Aug 2026 20:05:03 -0700 Subject: [PATCH 10/11] fix(auth): name enable_jwt_auth when a JWT-shaped key is rejected (#35831) A three-segment token presented while `general_settings.enable_jwt_auth` is unset is never treated as JWT-shaped, so it falls through to the virtual-key path and is rejected for not starting with 'sk-'. That reads as a missing key in the verification table and sends the operator off to inspect virtual keys, when the real cause is one missing config line. The rejection now names `enable_jwt_auth`, appended to the existing text so the Prometheus invalid-key filter and the admin UI keep matching what they match today. The hint claims only that the key is JWT-shaped. Segment count cannot tell a JWT from any other dotted credential, so asserting the key IS a JWT would swap one confident misdiagnosis for a narrower one. The enterprise gate on that same path raised a bare `ValueError`, which the terminal handler turns into a 401. Every sibling enterprise gate answers 403, and a 401 tells the client to retry with a better credential, which no credential can satisfy while the install is unlicensed. It now raises a 403 `ProxyException` like the SSO gate does. --- litellm/proxy/auth/user_api_key_auth.py | 19 +++- .../proxy/auth/test_user_api_key_auth.py | 98 +++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index d0d084a0582..4d05447fa90 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -677,6 +677,12 @@ async def check_api_key_for_custom_headers_or_pass_through_endpoints( # the lookup and return None (caller proceeds to auth_builder). _JWT_PROXY_ADMIN_SENTINEL: Final = "__JWT_PROXY_ADMIN__" +_JWT_AUTH_DISABLED_HINT = ( + " This key has the structure of a JWT, but JWT auth is not enabled on this proxy, so it was treated as a" + " virtual key. Set `enable_jwt_auth: true` under `general_settings` in your proxy config to authenticate" + " with JWTs." +) + class _PendingAutoRegister(NamedTuple): """ @@ -1206,8 +1212,11 @@ async def _user_api_key_auth_builder( from litellm.proxy.proxy_server import premium_user if premium_user is not True: - raise ValueError( - f"JWT Auth is an enterprise only feature. {CommonProxyErrors.not_premium_user.value}" + raise ProxyException( + message=f"JWT Auth is an enterprise only feature. {CommonProxyErrors.not_premium_user.value}", + type=ProxyErrorTypes.auth_error, + param="premium_user", + code=status.HTTP_403_FORBIDDEN, ) # Try JWT-to-Virtual-Key mapping first to avoid # unnecessary DB queries in auth_builder @@ -1672,9 +1681,13 @@ async def _user_api_key_auth_builder( if isinstance(api_key, str): # if generated token, make sure it starts with sk-. _masked_key: Final = f"{api_key[:4]}****{api_key[-4:]}" if len(api_key) > 8 else "****" if not api_key.startswith("sk-"): + _hint = _JWT_AUTH_DISABLED_HINT if not enable_jwt_auth and JWTHandler.is_jwt(token=api_key) else "" raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail=(f"LiteLLM Virtual Key expected. Received={_masked_key}, expected to start with 'sk-'."), + detail=( + f"LiteLLM Virtual Key expected. Received={_masked_key}, " + f"expected to start with 'sk-'.{_hint}" + ), ) # prevent token hashes from being used else: verbose_logger.warning( diff --git a/tests/test_litellm/proxy/auth/test_user_api_key_auth.py b/tests/test_litellm/proxy/auth/test_user_api_key_auth.py index 7c355b3b925..36e9ae5f992 100644 --- a/tests/test_litellm/proxy/auth/test_user_api_key_auth.py +++ b/tests/test_litellm/proxy/auth/test_user_api_key_auth.py @@ -5681,3 +5681,101 @@ async def test_temp_budget_increase_applied_for_cached_key(): cached_after = await user_api_key_cache.async_get_cache(key=hashed_token) assert cached_after.max_budget == 2.0 + + +async def _proxy_exception_for_key( + api_key: str, + general_settings: dict[str, bool], + premium_user: bool, +) -> ProxyException: + mock_request = MagicMock() + mock_request.url.path = "/v1/chat/completions" + mock_request.method = "POST" + mock_request.headers = {"authorization": f"Bearer {api_key}"} + mock_request.query_params = {} + mock_request.state = SimpleNamespace() + + proxy_logging_obj = MagicMock() + proxy_logging_obj.post_call_failure_hook = AsyncMock(return_value=None) + + user_api_key_cache = DualCache() + jwt_handler = JWTHandler() + jwt_handler.update_environment( + prisma_client=None, + user_api_key_cache=user_api_key_cache, + litellm_jwtauth=LiteLLM_JWTAuth(), + ) + + with ( + patch("litellm.proxy.proxy_server.general_settings", general_settings), + patch("litellm.proxy.proxy_server.premium_user", premium_user), + patch("litellm.proxy.proxy_server.master_key", "sk-master"), + patch("litellm.proxy.proxy_server.prisma_client", MagicMock()), + patch("litellm.proxy.proxy_server.user_api_key_cache", user_api_key_cache), + patch("litellm.proxy.proxy_server.proxy_logging_obj", proxy_logging_obj), + patch("litellm.proxy.proxy_server.jwt_handler", jwt_handler), + ): + with pytest.raises(ProxyException) as exc_info: + await _user_api_key_auth_builder( + request=mock_request, + api_key=f"Bearer {api_key}", + azure_api_key_header="", + anthropic_api_key_header=None, + google_ai_studio_api_key_header=None, + azure_apim_header=None, + request_data={"model": "gpt-4o-mini"}, + ) + + return exc_info.value + + +@pytest.mark.asyncio +async def test_jwt_shaped_key_error_names_enable_jwt_auth_when_disabled(): + """ + A three-segment token presented while `general_settings.enable_jwt_auth` + is unset is never treated as JWT-shaped, so it falls through to the + virtual-key path and is rejected for not starting with 'sk-'. That reads + as a missing database row and sends the operator to inspect virtual keys, + when the real cause is the missing config key. The rejection must name + `enable_jwt_auth`, and must claim only that the key is JWT-shaped, since + segment count cannot tell a JWT from any other dotted credential. + + The existing 'expected to start with sk-' text has to survive: the + Prometheus invalid-key filter and the admin UI both substring-match it. + Keys that are not JWT-shaped must not pick up the hint. + """ + jwt_error = await _proxy_exception_for_key( + "eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJzdmMtMSJ9.c2lnbmF0dXJl", {}, True + ) + + assert jwt_error.code == "401" + assert "enable_jwt_auth" in jwt_error.message + assert "general_settings" in jwt_error.message + assert "expected to start with 'sk-'" in jwt_error.message + assert "structure of a JWT" in jwt_error.message + assert "is a JWT" not in jwt_error.message + + opaque_error = await _proxy_exception_for_key("not-a-jwt-at-all", {}, True) + two_segment_error = await _proxy_exception_for_key( + "eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJzdmMtMSJ9", {}, True + ) + + assert "enable_jwt_auth" not in opaque_error.message + assert "enable_jwt_auth" not in two_segment_error.message + + +@pytest.mark.asyncio +async def test_unlicensed_jwt_auth_is_forbidden_not_unauthorized(): + """ + JWT auth is enterprise-gated. An unlicensed install must answer 403 like + every other enterprise gate; a 401 tells the client its credential was + wrong and invites a retry loop that can never succeed. + """ + error = await _proxy_exception_for_key( + "eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJzdmMtMSJ9.c2lnbmF0dXJl", + {"enable_jwt_auth": True}, + False, + ) + + assert error.code == "403" + assert "enterprise" in error.message.lower() From 31a86daa85f9f1e0e7219cebfa7cacb3933de343 Mon Sep 17 00:00:00 2001 From: Abhimanyu Kapur <38531241+akapur99@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:08:49 -0700 Subject: [PATCH 11/11] feat(auto-router): make reminder marker pair configurable (#35874) * feat(auto-router): make reminder marker pair configurable Some harnesses inject internal context using their own marker pair instead of Claude Code's / convention, and some send it as a separate follow-up user message rather than inline with the ask. Both cases fall out of the same root cause: the router's marker-matching is hardcoded, so foreign markers never strip to empty and the reminder-only turn wins "newest human ask" selection instead of being skipped. Add an optional reminder_markers field to ComplexityRouterConfig so operators can override the (open, close) pair via proxy config, with the existing skip-when-empty selection logic handling both cases once the markers match. * test(auto-router): drop unsolicited comments from the reminder-markers regression test Per Greptile review on #35874: no comments unless explicitly requested. --- .../complexity_router/complexity_router.py | 67 +++++++++++++------ .../complexity_router/config.py | 21 ++++++ .../router_strategy/test_complexity_router.py | 40 +++++++++++ 3 files changed, 109 insertions(+), 19 deletions(-) diff --git a/litellm/router_strategy/complexity_router/complexity_router.py b/litellm/router_strategy/complexity_router/complexity_router.py index 7297506b178..ea2cc7297dd 100644 --- a/litellm/router_strategy/complexity_router/complexity_router.py +++ b/litellm/router_strategy/complexity_router/complexity_router.py @@ -178,7 +178,9 @@ def _message_text(content: object) -> str: return content if isinstance(content, str) else "" -def _reminder_block_spans(lowered: str) -> Iterator[tuple[int, int]]: +def _reminder_block_spans( + lowered: str, open_marker: str = _REMINDER_OPEN, close_marker: str = _REMINDER_CLOSE +) -> Iterator[tuple[int, int]]: """Span of each complete reminder block, left to right. Literal `str.find`, not a regex: the delimiters are fixed strings, and `.*?` @@ -187,17 +189,17 @@ def _reminder_block_spans(lowered: str) -> Iterator[tuple[int, int]]: and an unclosed tag ends the scan, so this is linear without bounding the input. """ cursor = 0 - while (start := lowered.find(_REMINDER_OPEN, cursor)) != -1: - end = lowered.find(_REMINDER_CLOSE, start + len(_REMINDER_OPEN)) + while (start := lowered.find(open_marker, cursor)) != -1: + end = lowered.find(close_marker, start + len(open_marker)) if end == -1: return - cursor = end + len(_REMINDER_CLOSE) + cursor = end + len(close_marker) yield start, cursor -def _strip_reminder_blocks(text: str) -> str: +def _strip_reminder_blocks(text: str, open_marker: str = _REMINDER_OPEN, close_marker: str = _REMINDER_CLOSE) -> str: """Remove every complete reminder block from text, keeping everything written around them.""" - spans: Final = tuple(_reminder_block_spans(text.lower())) + spans: Final = tuple(_reminder_block_spans(text.lower(), open_marker, close_marker)) if not spans: return text.strip() keep_from: Final = (0, *(end for _, end in spans)) @@ -205,7 +207,7 @@ def _strip_reminder_blocks(text: str) -> str: return " ".join(kept for a, b in zip(keep_from, keep_to) if (kept := text[a:b].strip())) -def _human_text(content: object) -> str: +def _human_text(content: object, open_marker: str = _REMINDER_OPEN, close_marker: str = _REMINDER_CLOSE) -> str: """Message content as the text a human wrote, with complete reminder blocks removed. Harnesses inject reminders as ordinary text alongside the live ask, so the block is stripped and @@ -214,13 +216,18 @@ def _human_text(content: object) -> str: one, and this same string drives escalation keywords and keyword_tier_rules, which choose the model and therefore the spend. An unclosed tag is not a block and is left intact. """ - return _strip_reminder_blocks(_message_text(content)) + return _strip_reminder_blocks(_message_text(content), open_marker, close_marker) -def _iter_human_asks_newest_first(messages: Sequence[Mapping[str, object]]) -> Iterator[str]: +def _iter_human_asks_newest_first( + messages: Sequence[Mapping[str, object]], markers: tuple[str, str] = (_REMINDER_OPEN, _REMINDER_CLOSE) +) -> Iterator[str]: """Yield user-turn texts that carry a real human ask, newest first, with harness noise removed.""" + open_marker, close_marker = markers return ( - text for msg in reversed(messages) if msg.get("role") == "user" and (text := _human_text(msg.get("content"))) + text + for msg in reversed(messages) + if msg.get("role") == "user" and (text := _human_text(msg.get("content"), open_marker, close_marker)) ) @@ -258,7 +265,9 @@ def _conversation_is_continuing(messages: Sequence[Mapping[str, object]] | None) return any(message.get("role") == "assistant" for message in messages) -def _newest_turn_ask(messages: Sequence[Mapping[str, object]]) -> str | None: +def _newest_turn_ask( + messages: Sequence[Mapping[str, object]], markers: tuple[str, str] = (_REMINDER_OPEN, _REMINDER_CLOSE) +) -> str | None: """The human ask on the newest user turn, or None when that turn carries only plumbing. Escalation reads this rather than the last ask in history, which survives across the plumbing @@ -268,11 +277,12 @@ def _newest_turn_ask(messages: Sequence[Mapping[str, object]]) -> str | None: newest_user_turn: Final = next((msg for msg in reversed(messages) if msg.get("role") == "user"), None) if newest_user_turn is None: return None - return _human_text(newest_user_turn.get("content")) or None + return _human_text(newest_user_turn.get("content"), *markers) or None def _extract_current_ask_and_system_prompt( messages: Sequence[Mapping[str, object]], + markers: tuple[str, str] = (_REMINDER_OPEN, _REMINDER_CLOSE), ) -> tuple[str | None, str | None]: """The last real human ask and the last system prompt; either is None if absent. @@ -280,7 +290,7 @@ def _extract_current_ask_and_system_prompt( the caller routes to its default model. That is the correct answer rather than a gap to fill: filling it would hand tier selection to harness-injected text. """ - current_ask: Final = next(_iter_human_asks_newest_first(messages), None) + current_ask: Final = next(_iter_human_asks_newest_first(messages, markers), None) system_prompt: Final = next( ( text @@ -300,6 +310,7 @@ def _truncate(text: str, limit: int) -> str: def _iter_context_turns_newest_first( messages: Sequence[Mapping[str, object]], include_assistant: bool, + markers: tuple[str, str] = (_REMINDER_OPEN, _REMINDER_CLOSE), ) -> Iterator[tuple[str, str]]: """Yield (role, text) for turns eligible as classifier context, newest first. @@ -313,7 +324,9 @@ def _iter_context_turns_newest_first( return ( (role, text) for msg in reversed(messages) - if isinstance(role := msg.get("role"), str) and role in roles and (text := _human_text(msg.get("content"))) + if isinstance(role := msg.get("role"), str) + and role in roles + and (text := _human_text(msg.get("content"), *markers)) ) @@ -323,6 +336,7 @@ def _extract_prior_turns( window_size: int, per_turn_chars: int, include_assistant: bool, + markers: tuple[str, str] = (_REMINDER_OPEN, _REMINDER_CLOSE), ) -> tuple[tuple[str, str], ...]: """Up to window_size turns other than current_ask, oldest first, as (role, text). @@ -340,7 +354,11 @@ def _extract_prior_turns( return () prior: Final = islice( - (turn for turn in _iter_context_turns_newest_first(messages, include_assistant) if turn[1] != current_ask), + ( + turn + for turn in _iter_context_turns_newest_first(messages, include_assistant, markers) + if turn[1] != current_ask + ), window_size, ) return tuple((role, _truncate(text, per_turn_chars)) for role, text in reversed(tuple(prior))) @@ -435,6 +453,7 @@ class ComplexityRouter(CustomLogger): if self.config.escalation_keywords is not None else DEFAULT_ESCALATION_KEYWORDS ) + self._reminder_markers: tuple[str, str] = self.config.reminder_markers or (_REMINDER_OPEN, _REMINDER_CLOSE) # Lazily built on first semantic request and cached for reuse (route # embeddings are static, only the prompt is embedded per request). The lock @@ -788,13 +807,21 @@ class ComplexityRouter(CustomLogger): window_size=self.config.classifier_context_window_size, per_turn_chars=self.config.classifier_context_per_turn_chars, include_assistant=include_assistant, + markers=self._reminder_markers, ) if context_enabled else () ) has_prior_conversation: Final = ( context_enabled - and len(tuple(islice(_iter_context_turns_newest_first(messages or (), include_assistant), 2))) > 1 + and len( + tuple( + islice( + _iter_context_turns_newest_first(messages or (), include_assistant, self._reminder_markers), 2 + ) + ) + ) + > 1 ) user_payload: Final = self._build_classifier_user_payload( @@ -1444,7 +1471,9 @@ class ComplexityRouter(CustomLogger): routed_model: str | None = pinned_model pin_escalation_keyword: str | None = None if self.escalation_keywords: - user_message: Final = _newest_turn_ask(resolved_messages) if resolved_messages else None + user_message: Final = ( + _newest_turn_ask(resolved_messages, self._reminder_markers) if resolved_messages else None + ) if user_message is not None: pin_escalation_keyword = self._matched_escalation_keyword(user_message) if pin_escalation_keyword is not None: @@ -1540,7 +1569,7 @@ class ComplexityRouter(CustomLogger): # Determine whether the original request used messages directly has_original_messages: Final = messages is not None and len(messages) > 0 - user_message, system_prompt = _extract_current_ask_and_system_prompt(resolved_messages) + user_message, system_prompt = _extract_current_ask_and_system_prompt(resolved_messages, self._reminder_markers) if user_message is None: verbose_router_logger.debug("ComplexityRouter: No user message found, routing to default model") @@ -1566,7 +1595,7 @@ class ComplexityRouter(CustomLogger): ), ) - newest_ask: Final = _newest_turn_ask(resolved_messages) + newest_ask: Final = _newest_turn_ask(resolved_messages, self._reminder_markers) escalation_keyword: Final = self._matched_escalation_keyword(newest_ask) if newest_ask is not None else None override: Final = await self._resolve_keyword_tier_override(user_message, request_kwargs) diff --git a/litellm/router_strategy/complexity_router/config.py b/litellm/router_strategy/complexity_router/config.py index eaa1b5e867f..f12cd59a869 100644 --- a/litellm/router_strategy/complexity_router/config.py +++ b/litellm/router_strategy/complexity_router/config.py @@ -446,6 +446,15 @@ class ComplexityRouterConfig(BaseModel): description="RoutingPlugin instances that narrow the classified tier's candidate models before selection", ) + reminder_markers: tuple[str, str] | None = Field( + default=None, + description=( + "Override the (open, close) marker pair used to recognize and strip harness-injected " + "reminder blocks before classification. Defaults to Claude Code's convention, " + "('', ''), when unset. Matching is case-insensitive." + ), + ) + model_config = ConfigDict(extra="allow", arbitrary_types_allowed=True) # Allow additional fields @field_validator("tiers", mode="before") @@ -508,6 +517,18 @@ class ComplexityRouterConfig(BaseModel): ) return self + @model_validator(mode="after") + def _normalize_reminder_markers(self) -> "ComplexityRouterConfig": + if self.reminder_markers is None: + return self + open_marker, close_marker = (marker.strip().lower() for marker in self.reminder_markers) + if not open_marker or not close_marker: + raise ValueError("reminder_markers entries must not be blank") + if open_marker == close_marker: + raise ValueError("reminder_markers open and close must be different strings") + self.reminder_markers = (open_marker, close_marker) + return self + # Combined default config DEFAULT_COMPLEXITY_CONFIG: Final = ComplexityRouterConfig() diff --git a/tests/test_litellm/router_strategy/test_complexity_router.py b/tests/test_litellm/router_strategy/test_complexity_router.py index 2f249241f21..d713fc9e0f4 100644 --- a/tests/test_litellm/router_strategy/test_complexity_router.py +++ b/tests/test_litellm/router_strategy/test_complexity_router.py @@ -2607,6 +2607,26 @@ class TestSemanticConfigValidation: assert config.keyword_tier_rules is not None assert config.keyword_tier_rules[0].keywords == ["deploy to k8s", "kubernetes"] + def test_reminder_markers_unset_defaults_to_none(self): + """Unset means the router falls back to the built-in markers.""" + config = ComplexityRouterConfig() + assert config.reminder_markers is None + + def test_reminder_markers_are_normalized(self): + """Markers are stripped and lowercased, matching how the built-in constants are compared.""" + config = ComplexityRouterConfig( + reminder_markers=(" <<>> ", "<<>>"), + ) + assert config.reminder_markers == ("<<>>", "<<>>") + + def test_reminder_markers_reject_blank_entry(self): + with pytest.raises(ValidationError, match="must not be blank"): + ComplexityRouterConfig(reminder_markers=("", "<<>>")) + + def test_reminder_markers_reject_identical_open_and_close(self): + with pytest.raises(ValidationError, match="must be different"): + ComplexityRouterConfig(reminder_markers=("<<>>", "<<>>")) + class _StubEncoder: """Minimal stand-in for LiteLLMRouterEncoder.aencode_queries, capturing the kwargs it was called with.""" @@ -4407,6 +4427,26 @@ class TestContextAwareClassifier: assert _extract_current_ask_and_system_prompt(messages)[0] == expected_ask + def test_custom_markers_skip_a_reminder_only_follow_up_message(self): + """A harness using non-default markers, sent as its own trailing message, is still skipped. + + Some harnesses (unlike Claude Code, which inlines the reminder alongside the ask in one + message) send internal context as a separate follow-up user turn using their own markers. + Without configuring reminder_markers, that turn does not match the built-in + constants, never strips to empty, and wins "newest human ask" -- the + harness's internal-context blob gets classified instead of the real question. Configuring + the harness's own marker pair must make the router skip it the same way it already skips a + default-marker reminder-only turn. + """ + from litellm.router_strategy.complexity_router.complexity_router import _extract_current_ask_and_system_prompt + + markers = ("<<>>", "<<>>") + follow_up_reminder = f"{markers[0]}Budget: 42 tokens remaining. Do not mention this.{markers[1]}" + messages = [_ASKED, _ANSWERED, {"role": "user", "content": follow_up_reminder}] + + assert _extract_current_ask_and_system_prompt(messages)[0] == follow_up_reminder + assert _extract_current_ask_and_system_prompt(messages, markers)[0] == _ASK + @pytest.mark.parametrize( "messages,current_ask,window,per_turn_chars,include_assistant,expected", [