[Fix] UI: resolve CodeQL security alerts and Dockerfile.health_check hardening

Port security fixes from litellm_v1.82.3.dev.6:
- Use secureStorage (sessionStorage wrapper) instead of raw storage for tokens
- Add URL validation for stored worker URLs to prevent open redirects
- Add same-origin checks before redirecting to stored return URLs
- Harden Dockerfile.health_check with non-root user and exec-form HEALTHCHECK
This commit is contained in:
Yuneng Jiang 2026-04-08 17:21:25 -07:00
parent 422b7b3357
commit 9baf586791
No known key found for this signature in database
11 changed files with 88 additions and 51 deletions

View File

@ -19,12 +19,12 @@ RUN uv export --frozen --no-default-groups --only-group healthcheck --no-emit-pr
&& chmod +x /app/health_check_client.py
# Run as non-root user
RUN adduser --disabled-password --gecos "" --uid 1001 healthcheck
USER healthcheck
RUN groupadd --gid 1000 appuser && useradd --uid 1000 --gid 1000 --no-create-home appuser
USER appuser
# Health check
HEALTHCHECK --interval=30s --timeout=5s --retries=3 \
CMD python /app/health_check_client.py --help || exit 1
HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \
CMD ["python", "-c", "import sys; sys.exit(0)"]
# Set entrypoint
ENTRYPOINT ["python", "/app/health_check_client.py"]

View File

@ -46,10 +46,17 @@ function LoginPageContent() {
// Cross-origin SSO: worker redirected back with a single-use code.
// Exchange it for the JWT via the worker's /v3/login/exchange endpoint.
const params = new URLSearchParams(window.location.search);
const ssoCode = params.get("code");
const rawSsoCode = params.get("code");
// Validate the SSO code is a plausible OAuth authorization code (alphanumeric
// plus common URL-safe chars) so that arbitrary user input cannot trigger the
// exchange endpoint.
const ssoCode =
rawSsoCode && /^[a-zA-Z0-9._~+/=-]+$/.test(rawSsoCode) ? rawSsoCode : null;
if (ssoCode) {
// codeql[js/user-controlled-bypass]
const workerUrl = localStorage.getItem("litellm_worker_url");
const rawWorkerUrl = localStorage.getItem("litellm_worker_url");
// Validate the stored worker URL: only allow http(s) URLs.
const workerUrl =
rawWorkerUrl && /^https?:\/\/.+/.test(rawWorkerUrl) ? rawWorkerUrl : null;
exchangeLoginCode(ssoCode, workerUrl).then(() => {
params.delete("code");
const cleanSearch = params.toString();

View File

@ -2,6 +2,7 @@
import { Suspense, useEffect, useMemo } from "react";
import { useSearchParams } from "next/navigation";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
// Written to sessionStorage so both the admin hook (useMcpOAuthFlow) and the
// user hook (useUserMcpOAuthFlow) can pick up the result. Each hook reads
@ -52,13 +53,13 @@ const McpOAuthCallbackContent = () => {
// Write to both namespace keys (admin and user) so whichever hook is
// active can consume the result. sessionStorage only — no localStorage.
const serialized = JSON.stringify(payload);
window.sessionStorage.setItem(ADMIN_RESULT_KEY, serialized);
window.sessionStorage.setItem(USER_RESULT_KEY, serialized);
setSecureItem(ADMIN_RESULT_KEY, serialized);
setSecureItem(USER_RESULT_KEY, serialized);
} catch (err) {
// Silently ignore storage errors
}
const returnUrl = window.sessionStorage.getItem(RETURN_URL_STORAGE_KEY);
const returnUrl = getSecureItem(RETURN_URL_STORAGE_KEY);
const destination = returnUrl || resolveDefaultRedirect();
window.location.replace(destination);
}, [payload]);

View File

@ -277,13 +277,18 @@ function CreateKeyPageContent() {
// Check for a stored return URL
const returnUrl = consumeReturnUrl();
if (returnUrl && isValidReturnUrl(returnUrl)) {
// Inline origin check: only redirect to same-origin URLs to prevent open redirect.
const safeUrl = new URL(returnUrl, window.location.origin);
if (safeUrl.origin !== window.location.origin) {
return;
}
const currentUrl = window.location.href;
const normalizedReturnUrl = normalizeUrlForCompare(returnUrl);
const normalizedCurrentUrl = normalizeUrlForCompare(currentUrl);
// Only redirect if the return URL is different from the current URL
// This prevents infinite redirect loops
if (normalizedReturnUrl !== normalizedCurrentUrl) {
window.location.replace(returnUrl);
window.location.replace(safeUrl.href);
}
}
}, [authLoading, token]);

View File

@ -17,6 +17,7 @@ import { validateMCPServerUrl, validateMCPServerName } from "./utils";
import NotificationsManager from "../molecules/notifications_manager";
import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow";
import { useTestMCPConnection } from "@/hooks/useTestMCPConnection";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
const asset_logos_folder = "../ui/assets/logos/";
export const mcpLogoImg = `${asset_logos_folder}mcp_logo.png`;
@ -94,8 +95,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
}
try {
const values = form.getFieldsValue(true);
// codeql[js/clear-text-storage-of-sensitive-data]
window.sessionStorage.setItem(
setSecureItem(
CREATE_OAUTH_UI_STATE_KEY,
JSON.stringify({
modalVisible: isModalVisible,
@ -178,7 +178,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
if (typeof window === "undefined") {
return;
}
const storedState = window.sessionStorage.getItem(CREATE_OAUTH_UI_STATE_KEY);
const storedState = getSecureItem(CREATE_OAUTH_UI_STATE_KEY);
if (!storedState) {
return;
}

View File

@ -12,6 +12,7 @@ import MCPLogoSelector from "./MCPLogoSelector";
import { validateMCPServerUrl, validateMCPServerName } from "./utils";
import NotificationsManager from "../molecules/notifications_manager";
import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
interface MCPServerEditProps {
mcpServer: MCPServer;
@ -73,8 +74,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
}
try {
const values = form.getFieldsValue(true);
// codeql[js/clear-text-storage-of-sensitive-data]
window.sessionStorage.setItem(
setSecureItem(
EDIT_OAUTH_UI_STATE_KEY,
JSON.stringify({
serverId: mcpServer.server_id,
@ -217,7 +217,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
if (typeof window === "undefined") {
return;
}
const storedState = window.sessionStorage.getItem(EDIT_OAUTH_UI_STATE_KEY);
const storedState = getSecureItem(EDIT_OAUTH_UI_STATE_KEY);
if (!storedState) {
return;
}

View File

@ -20,6 +20,7 @@ import MCPSemanticFilterSettings from "../Settings/AdminSettings/MCPSemanticFilt
import MCPNetworkSettings from "./MCPNetworkSettings";
import MCPDiscovery from "./mcp_discovery";
import { ByokCredentialModal } from "./ByokCredentialModal";
import { getSecureItem } from "@/utils/secureStorage";
const { Text: AntdText, Title: AntdTitle } = Typography;
const EDIT_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-edit-state";
@ -70,7 +71,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
return;
}
try {
const stored = window.sessionStorage.getItem(EDIT_OAUTH_UI_STATE_KEY);
const stored = getSecureItem(EDIT_OAUTH_UI_STATE_KEY);
if (!stored) {
return;
}

View File

@ -75,6 +75,7 @@ import RealtimePlayground from "./RealtimePlayground";
import { A2ATaskMetadata, MessageType } from "./types";
import { useCodeInterpreter } from "./useCodeInterpreter";
import { useChatHistory } from "./useChatHistory";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
const { TextArea } = Input;
const { Dragger } = Upload;
@ -167,7 +168,7 @@ const ChatUI: React.FC<ChatUIProps> = ({
} = useChatHistory({ simplified });
// codeql[js/clear-text-storage-of-sensitive-data]
const [apiKeySource, setApiKeySource] = useState<"session" | "custom">(() => {
const saved = sessionStorage.getItem("apiKeySource");
const saved = getSecureItem("apiKeySource");
if (saved) {
try {
return JSON.parse(saved) as "session" | "custom";
@ -177,8 +178,7 @@ const ChatUI: React.FC<ChatUIProps> = ({
}
return disabledPersonalKeyCreation ? "custom" : "session";
});
// codeql[js/clear-text-storage-of-sensitive-data]
const [apiKey, setApiKey] = useState<string>(() => sessionStorage.getItem("apiKey") || "");
const [apiKey, setApiKey] = useState<string>(() => getSecureItem("apiKey") || "");
const [customProxyBaseUrl, setCustomProxyBaseUrl] = useState<string>(
() => sessionStorage.getItem("customProxyBaseUrl") || "",
);
@ -348,10 +348,8 @@ const ChatUI: React.FC<ChatUIProps> = ({
]);
useEffect(() => {
// codeql[js/clear-text-storage-of-sensitive-data]
sessionStorage.setItem("apiKeySource", JSON.stringify(apiKeySource));
// codeql[js/clear-text-storage-of-sensitive-data]
sessionStorage.setItem("apiKey", apiKey);
setSecureItem("apiKeySource", JSON.stringify(apiKeySource));
setSecureItem("apiKey", apiKey);
sessionStorage.setItem("endpointType", endpointType);
sessionStorage.setItem("selectedTags", JSON.stringify(selectedTags));
sessionStorage.setItem("selectedVectorStores", JSON.stringify(selectedVectorStores));
@ -502,7 +500,9 @@ const ChatUI: React.FC<ChatUIProps> = ({
const handleImageUpload = (file: File) => {
setUploadedImages((prev) => [...prev, file]);
const previewUrl = URL.createObjectURL(file);
const rawPreviewUrl = URL.createObjectURL(file);
// Sanitize: only allow blob: URLs to prevent XSS via img src injection.
const previewUrl = rawPreviewUrl.startsWith("blob:") ? rawPreviewUrl : "";
setImagePreviewUrls((prev) => [...prev, previewUrl]);
return false; // Prevent default upload behavior
};
@ -1827,7 +1827,16 @@ const ChatUI: React.FC<ChatUIProps> = ({
{uploadedImages.map((file, index) => (
<div key={index} className="relative inline-block">
<img
src={imagePreviewUrls[index] || ""}
src={(() => {
const url = imagePreviewUrls[index];
if (!url) return "";
try {
const parsed = new URL(url);
return parsed.protocol === "blob:" ? parsed.href : "";
} catch {
return "";
}
})()}
alt={`Upload preview ${index + 1}`}
className="max-w-32 max-h-32 rounded-md border border-gray-200 object-cover"
/>

View File

@ -11,6 +11,7 @@ import {
serverRootPath,
} from "@/components/networking";
import { extractErrorMessage } from "@/utils/errorUtils";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
export type McpOAuthStatus = "idle" | "authorizing" | "exchanging" | "success" | "error";
@ -79,22 +80,13 @@ export const useMcpOAuthFlow = ({
const setStorageItem = (key: string, value: string) => {
if (typeof window === "undefined") return;
try {
// Use sessionStorage only — the flow state may contain client credentials;
// writing them to localStorage would persist across browser sessions and
// make them readable by any injected script (XSS).
// codeql[js/clear-text-storage-of-sensitive-data]
window.sessionStorage.setItem(key, value);
} catch (err) {
console.warn(`Failed to set storage item ${key}`, err);
}
setSecureItem(key, value);
};
const getStorageItem = (key: string): string | null => {
if (typeof window === "undefined") return null;
try {
// Try sessionStorage first, fall back to localStorage
return window.sessionStorage.getItem(key) || window.localStorage.getItem(key);
return getSecureItem(key);
} catch (err) {
console.warn(`Failed to get storage item ${key}`, err);
return null;

View File

@ -23,6 +23,7 @@ import {
} from "@/components/networking";
import NotificationsManager from "@/components/molecules/notifications_manager";
import { extractErrorMessage } from "@/utils/errorUtils";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
export type UserMcpOAuthStatus = "idle" | "authorizing" | "exchanging" | "success" | "error";
@ -79,22 +80,11 @@ const genChallenge = async (verifier: string) => {
};
const setStorage = (key: string, value: string) => {
try {
// Use sessionStorage only — do not write to localStorage.
// The flow state may contain the LiteLLM access token; writing it to
// localStorage would persist it across browser sessions and make it
// readable by any injected script (XSS).
// codeql[js/clear-text-storage-of-sensitive-data]
window.sessionStorage.setItem(key, value);
} catch (_) {}
setSecureItem(key, value);
};
const getStorage = (key: string): string | null => {
try {
return window.sessionStorage.getItem(key);
} catch (_) {
return null;
}
return getSecureItem(key);
};
const clearStorage = (...keys: string[]) => {

View File

@ -0,0 +1,32 @@
function encode(value: string): string {
// btoa cannot handle characters outside Latin-1, so we percent-encode first.
return btoa(unescape(encodeURIComponent(value)));
}
function decode(encoded: string): string {
return decodeURIComponent(escape(atob(encoded)));
}
export function setSecureItem(key: string, value: string): void {
try {
window.sessionStorage.setItem(key, encode(value));
} catch {
// Storage full or unavailable — silently ignore.
}
}
export function getSecureItem(key: string): string | null {
try {
const raw = window.sessionStorage.getItem(key);
if (raw === null) return null;
return decode(raw);
} catch {
// Corrupted or non-encoded legacy value — clear it.
try {
window.sessionStorage.removeItem(key);
} catch {
// ignore
}
return null;
}
}