scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-11 22:51:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 124

refactor(ui): extract the MCP server edit payload into a pure module

Browse source 
`handleSave` was 280 lines that parsed stdio JSON, filtered credentials,
resolved half a dozen conditional flags, built the request body, and then made
the network call, all inside one try block reading ten closure variables. None
of it could be tested without rendering the whole modal and pressing Save.

The payload building moves to `editServerPayload.ts` as `buildEditServerPayload`,
mirroring the `createServerPayload.ts` module that already sits beside it: a
pure function taking form values, UI state, and the server being edited, and
returning a tagged union rather than throwing. The six ways it can fail are now
values in that union, and the component maps them to the same toasts it showed
before via an exhaustive switch, so no message text changes.

`AUTH_TYPES_REQUIRING_CREDENTIALS` and the static-header reducer were duplicated
between the create and edit paths; the extracted module imports the create
module's copies instead. The credential filter and stdio parser stay separate,
because the edit versions genuinely differ: edit writes an explicit null to
clear a blanked admin-config key, and stringifies stdio args and env values
where create passes them through

Behaviour is unchanged, and the payload net added earlier is what proves it:
all 20 of its cases still pass against the extracted builder without any
edit to their expectations.

40 unit tests cover the new module in single-digit milliseconds, reaching the
rejection paths and parsing branches that were previously only reachable
through a full render. A ten-mutant battery kills 10/10, each mutant taking
down exactly one test, which is the profile worth having: the tests are
targeted rather than broadly overlapping

One of those mutants is worth noting. Removing the access-group name
normalisation survives the integration net but dies here, because the antd
Select hands the form plain strings and the unit test does not. That mapping is
dead only while that control stays; it is covered now either way
This commit is contained in:
Yuneng Jiang 2026-08-18 22:06:44 -07:00
parent 198ea7e85f
commit 770a4feb8d
No known key found for this signature in database
3 changed files with 577 additions and 283 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

250
ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.test.ts Normal file
View file

@ -0,0 +1,250 @@
import { describe, it, expect } from "vitest";
import { buildEditServerPayload, EditServerUiState } from "./editServerPayload";
import { MCPServer } from "@/components/mcp_tools/types";
const SERVER: MCPServer = {
server_id: "srv_1",
server_name: "srv",
alias: "srv_alias",
description: "a server",
transport: "http",
url: "https://example.com/mcp",
auth_type: "none",
created_at: "2024-01-01T00:00:00Z",
created_by: "user-1",
updated_at: "2024-01-01T00:00:00Z",
updated_by: "user-1",
mcp_access_groups: [],
};
const UI: EditServerUiState = {
costConfig: {},
allowedTools: [],
hasExistingToolAllowlist: false,
hasToolAllowlistInteraction: false,
toolNameToDisplayName: {},
toolNameToDescription: {},
logoUrl: undefined,
removeStoredApp: false,
};
const VALUES = { transport: "http", auth_type: "none", url: "https://example.com/mcp" };
const build = (values: Record<string, unknown> = {}, ui: Partial<EditServerUiState> = {}, server: MCPServer = SERVER) =>
buildEditServerPayload({ ...VALUES, ...values }, { ...UI, ...ui }, server);
const ok = (result: ReturnType<typeof build>): Record<string, unknown> => {
expect(result.kind).toBe("ok");
if (result.kind !== "ok") throw new Error("unreachable");
return result.payload;
};
describe("buildEditServerPayload rejections", () => {
it("rejects a tool display name with a space before anything else is parsed", () => {
const result = build({ stdio_config: "{" }, { toolNameToDisplayName: { a: "bad name" } });
expect(result).toStrictEqual({ kind: "invalid_tool_display_name", displayName: "bad name" });
});
it("accepts a display name made of letters, digits, underscores and hyphens", () => {
expect(build({}, { toolNameToDisplayName: { a: "Good_name-1" } }).kind).toBe("ok");
});
it("rejects unparseable stdio config json", () => {
expect(build({ transport: "stdio", stdio_config: "{not json" }).kind).toBe("invalid_stdio_json");
});
it("rejects stdio config json that parses but carries no command", () => {
const result = build({ transport: "stdio", stdio_config: JSON.stringify({ args: ["x"] }) });
expect(result.kind).toBe("stdio_config_missing_command");
});
it("rejects unparseable stdio env json on the dedicated-fields path", () => {
const result = build({ transport: "stdio", command: "npx", env_json: "{not json" });
expect(result.kind).toBe("invalid_stdio_env_json");
});
it("rejects a blank command on the dedicated-fields path", () => {
expect(build({ transport: "stdio", command: " " }).kind).toBe("stdio_missing_command");
});
it("rejects unparseable token validation json", () => {
expect(build({ token_validation_json: "{not json" }).kind).toBe("invalid_token_validation_json");
});
it("ignores a whitespace-only token validation body rather than rejecting it", () => {
expect(build({ token_validation_json: " " }).kind).toBe("ok");
});
it("never leaves the stdio branch reachable for a non-stdio transport", () => {
expect(build({ transport: "http", stdio_config: "{not json" }).kind).toBe("ok");
});
});
describe("buildEditServerPayload stdio parsing", () => {
it("unwraps a pasted client config keyed under mcpServers", () => {
const config = JSON.stringify({ mcpServers: { fs: { command: "npx", args: ["-y", "pkg"], env: { A: "1" } } } });
const payload = ok(build({ transport: "stdio", stdio_config: config }));
expect(payload.command).toBe("npx");
expect(payload.args).toStrictEqual(["-y", "pkg"]);
expect(payload.env).toStrictEqual({ A: "1" });
});
it("accepts a bare command object with no mcpServers wrapper", () => {
const config = JSON.stringify({ command: "uvx", args: [], env: {} });
expect(ok(build({ transport: "stdio", stdio_config: config })).command).toBe("uvx");
});
it("stringifies non-string args and drops blank ones", () => {
const config = JSON.stringify({ command: "npx", args: [1, " ", "keep", true] });
expect(ok(build({ transport: "stdio", stdio_config: config })).args).toStrictEqual(["1", "keep", "true"]);
});
it("stringifies env values and drops blank keys", () => {
const config = JSON.stringify({ command: "npx", env: { A: 1, "": "x", B: null } });
expect(ok(build({ transport: "stdio", stdio_config: config })).env).toStrictEqual({ A: "1", B: "" });
});
it("trims the command on the dedicated-fields path", () => {
expect(ok(build({ transport: "stdio", command: " npx " })).command).toBe("npx");
});
it("defaults env to an empty object when no env json is supplied", () => {
expect(ok(build({ transport: "stdio", command: "npx" })).env).toStrictEqual({});
});
it("prefers the pasted json over the dedicated fields when both are present", () => {
const config = JSON.stringify({ command: "from-json" });
expect(ok(build({ transport: "stdio", stdio_config: config, command: "from-field" })).command).toBe("from-json");
});
});
describe("buildEditServerPayload credentials", () => {
it("drops blank credential values so the backend keeps what it stored", () => {
const payload = ok(build({ auth_type: "api_key", credentials: { auth_value: "", client_id: "cid" } }));
expect(payload.credentials).toStrictEqual({ client_id: "cid" });
});
it("sends an explicit null for a blanked admin-config key so the stored value is cleared", () => {
const payload = ok(build({ auth_type: "oauth2", credentials: { upstream_resource: "", client_id: "cid" } }));
expect(payload.credentials).toStrictEqual({ upstream_resource: null, client_id: "cid" });
});
it("drops a scopes array once every entry is blank", () => {
const payload = ok(build({ auth_type: "oauth2", credentials: { client_id: "cid", scopes: ["", null] } }));
expect(payload.credentials).toStrictEqual({ client_id: "cid" });
});
it("keeps the surviving scopes when only some are blank", () => {
const payload = ok(build({ auth_type: "oauth2", credentials: { client_id: "cid", scopes: ["read", ""] } }));
expect(payload.credentials).toStrictEqual({ client_id: "cid", scopes: ["read"] });
});
it("omits credentials entirely for an auth type that takes none", () => {
const payload = ok(build({ auth_type: "none", credentials: { auth_value: "x" } }));
expect(payload).not.toHaveProperty("credentials");
});
it("writes explicit nulls when a stored app is being removed", () => {
const payload = ok(
build({ auth_type: "true_passthrough", credentials: { client_id: "cid" } }, { removeStoredApp: true }),
);
expect(payload.credentials).toStrictEqual({ client_id: null, client_secret: null });
});
it("ignores removeStoredApp for a mode that does not forward client tokens", () => {
const payload = ok(build({ auth_type: "api_key", credentials: { auth_value: "v" } }, { removeStoredApp: true }));
expect(payload.credentials).toStrictEqual({ auth_value: "v" });
});
});
describe("buildEditServerPayload flags and clearing", () => {
it("maps the openapi transport to http for the backend", () => {
expect(ok(build({ transport: "openapi" })).transport).toBe("http");
});
it("leaves a transport the backend already understands alone", () => {
expect(ok(build({ transport: "http" })).transport).toBe("http");
});
it("nulls the oauth2 endpoints when moving off oauth2", () => {
const payload = ok(build({ auth_type: "api_key" }, {}, { ...SERVER, auth_type: "oauth2" }));
expect(payload.issuer).toBeNull();
expect(payload.authorization_url).toBeNull();
expect(payload.token_url).toBeNull();
expect(payload.registration_url).toBeNull();
});
it("leaves the oauth2 endpoints untouched when staying on oauth2", () => {
const payload = ok(build({ auth_type: "oauth2" }, {}, { ...SERVER, auth_type: "oauth2" }));
expect(payload).not.toHaveProperty("issuer");
});
it("nulls the token exchange fields when moving off token exchange", () => {
const payload = ok(build({ auth_type: "none" }, {}, { ...SERVER, auth_type: "oauth2_token_exchange" }));
expect(payload.token_exchange_endpoint).toBeNull();
expect(payload.audience).toBeNull();
expect(payload.subject_token_type).toBeNull();
expect(payload.token_exchange_profile).toBeNull();
});
it("forces delegate_auth_to_upstream false for any auth type other than oauth2", () => {
const payload = ok(build({ auth_type: "api_key", delegate_auth_to_upstream: true }));
expect(payload.delegate_auth_to_upstream).toBe(false);
});
it("honours delegate_auth_to_upstream for oauth2", () => {
const payload = ok(build({ auth_type: "oauth2", delegate_auth_to_upstream: true }));
expect(payload.delegate_auth_to_upstream).toBe(true);
});
it("forces oauth_passthrough false unless an Authorization header is forwarded", () => {
const payload = ok(build({ auth_type: "none", oauth_passthrough: true, extra_headers: [] }));
expect(payload.oauth_passthrough).toBe(false);
});
it("honours oauth_passthrough for a none-auth server forwarding Authorization", () => {
const payload = ok(build({ auth_type: "none", oauth_passthrough: true, extra_headers: ["authorization"] }));
expect(payload.oauth_passthrough).toBe(true);
});
it("forces dcr_bridge false outside the client-forwarded modes", () => {
expect(ok(build({ auth_type: "oauth2", dcr_bridge: true })).dcr_bridge).toBe(false);
});
it("honours dcr_bridge for a client-forwarded mode", () => {
expect(ok(build({ auth_type: "true_passthrough", dcr_bridge: true })).dcr_bridge).toBe(true);
});
it("sends token_validation as null to clear a value the server already had", () => {
const payload = ok(build({ token_validation_json: "" }, {}, { ...SERVER, token_validation: { a: 1 } }));
expect(payload.token_validation).toBeNull();
});
it("omits token_validation entirely when neither side has one", () => {
expect(ok(build({ token_validation_json: "" }))).not.toHaveProperty("token_validation");
});
it("falls back through the name chain to the server url", () => {
const server = { ...SERVER, server_name: "", url: "https://fallback" };
const payload = ok(build({ server_name: "", url: "" }, {}, server));
expect((payload.mcp_info as Record<string, unknown>).server_name).toBe("https://fallback");
});
it("falls back to the literal unknown when every name candidate is blank", () => {
const server = { ...SERVER, server_name: "", url: "", alias: "" };
const payload = ok(build({ server_name: "", url: "", alias: "" }, {}, server));
expect((payload.mcp_info as Record<string, unknown>).server_name).toBe("unknown");
});
it("emits allowed_tools only once the allowlist is enforced", () => {
expect(ok(build())).not.toHaveProperty("allowed_tools");
expect(ok(build({}, { allowedTools: ["a"] })).allowed_tools).toStrictEqual(["a"]);
expect(ok(build({}, { hasExistingToolAllowlist: true })).allowed_tools).toStrictEqual([]);
});
it("normalises object-shaped access groups to their names", () => {
const payload = ok(build({ mcp_access_groups: [{ name: "eng" }, "ops"] }));
expect(payload.mcp_access_groups).toStrictEqual(["eng", "ops"]);
});
});

285
ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.ts Normal file
View file

@ -0,0 +1,285 @@
import {
ADMIN_CONFIG_CREDENTIAL_KEYS,
AUTH_TYPE,
MCPServer,
MCPServerCostInfo,
MCP_OAUTH2_FLOW_INTERACTIVE,
MCP_OAUTH2_FLOW_M2M,
OAUTH_FLOW,
TRANSPORT,
isClientForwardedTokenMode,
preservedAdminCredentials,
} from "@/components/mcp_tools/types";
import { TOOL_DISPLAY_NAME_PATTERN, normalizeEnvVars } from "./utils";
import { AUTH_TYPES_REQUIRING_CREDENTIALS, reduceStaticHeaders } from "./createServerPayload";
export interface EditServerUiState {
readonly costConfig: MCPServerCostInfo;
readonly allowedTools: readonly string[];
readonly hasExistingToolAllowlist: boolean;
readonly hasToolAllowlistInteraction: boolean;
readonly toolNameToDisplayName: Readonly<Record<string, string>>;
readonly toolNameToDescription: Readonly<Record<string, string>>;
readonly logoUrl: string | undefined;
readonly removeStoredApp: boolean;
}
export type BuildEditPayloadResult =
| { readonly kind: "ok"; readonly payload: Record<string, unknown> }
| { readonly kind: "invalid_tool_display_name"; readonly displayName: string }
| { readonly kind: "invalid_stdio_json" }
| { readonly kind: "stdio_config_missing_command" }
| { readonly kind: "invalid_stdio_env_json" }
| { readonly kind: "stdio_missing_command" }
| { readonly kind: "invalid_token_validation_json" };
type StdioFieldsResult =
| { readonly kind: "ok"; readonly fields: Record<string, unknown> }
| { readonly kind: "invalid_stdio_json" }
| { readonly kind: "stdio_config_missing_command" }
| { readonly kind: "invalid_stdio_env_json" }
| { readonly kind: "stdio_missing_command" };
type JsonParseResult = { readonly kind: "ok"; readonly value: unknown } | { readonly kind: "invalid" };
const tryParseJson = (raw: string): JsonParseResult => {
try {
return { kind: "ok", value: JSON.parse(raw) };
} catch {
return { kind: "invalid" };
}
};
const normalizeStdioArgs = (args: unknown): string[] =>
Array.isArray(args) ? args.map((v: unknown) => String(v)).filter((v: string) => v.trim() !== "") : [];
const normalizeStdioEnv = (env: unknown): Record<string, string> =>
env && typeof env === "object" && !Array.isArray(env)
? Object.entries(env as Record<string, unknown>).reduce((acc: Record<string, string>, [k, v]) => {
if (k == null || String(k).trim() === "") return acc;
acc[String(k)] = v == null ? "" : String(v);
return acc;
}, {})
: {};
const unwrapStdioConfig = (parsed: unknown): Record<string, unknown> => {
const config = parsed as Record<string, unknown> | null;
const nested = config?.mcpServers;
if (!nested || typeof nested !== "object") return (config ?? {}) as Record<string, unknown>;
const names = Object.keys(nested as Record<string, unknown>);
if (names.length === 0) return config as Record<string, unknown>;
return (nested as Record<string, unknown>)[names[0]] as Record<string, unknown>;
};
const buildStdioFields = (
rawStdioConfig: unknown,
rawEnvJson: unknown,
rawCommand: unknown,
rawArgs: unknown,
): StdioFieldsResult => {
if (rawStdioConfig) {
const parsed = tryParseJson(rawStdioConfig as string);
if (parsed.kind === "invalid") return { kind: "invalid_stdio_json" };
const config = unwrapStdioConfig(parsed.value);
const command = config?.command ? String(config.command) : undefined;
if (!command) return { kind: "stdio_config_missing_command" };
return {
kind: "ok",
fields: { command, args: normalizeStdioArgs(config?.args), env: normalizeStdioEnv(config?.env) },
};
}
const envResult: JsonParseResult = rawEnvJson ? tryParseJson(rawEnvJson as string) : { kind: "ok", value: null };
if (envResult.kind === "invalid") return { kind: "invalid_stdio_env_json" };
const command = rawCommand ? String(rawCommand).trim() : "";
if (!command) return { kind: "stdio_missing_command" };
return {
kind: "ok",
fields: { command, args: normalizeStdioArgs(rawArgs), env: normalizeStdioEnv(envResult.value) },
};
};
const filterCredentials = (credentialValues: unknown): Record<string, unknown> | undefined => {
if (!credentialValues || typeof credentialValues !== "object") return undefined;
return Object.entries(credentialValues as Record<string, unknown>).reduce(
(acc: Record<string, unknown>, [key, value]) => {
if (value === undefined || value === null || value === "") {
// Blank is the keep-existing convention, except for the admin-config keys, where an
// explicit null is how the backend is told to clear a previously stored value.
if (value === "" && (ADMIN_CONFIG_CREDENTIAL_KEYS as readonly string[]).includes(key)) {
acc[key] = null;
}
return acc;
}
if (key === "scopes") {
if (Array.isArray(value)) {
const filteredScopes = value.filter((scope) => scope != null && scope !== "");
if (filteredScopes.length > 0) acc[key] = filteredScopes;
}
} else {
acc[key] = value;
}
return acc;
},
{},
);
};
const firstInvalidToolDisplayName = (toolNameToDisplayName: Readonly<Record<string, string>>): string | undefined =>
Object.entries(toolNameToDisplayName).find(
([, displayName]) => displayName && !TOOL_DISPLAY_NAME_PATTERN.test(displayName),
)?.[1];
export const buildEditServerPayload = (
values: Record<string, unknown>,
ui: EditServerUiState,
mcpServer: MCPServer,
): BuildEditPayloadResult => {
const badDisplayName = firstInvalidToolDisplayName(ui.toolNameToDisplayName);
if (badDisplayName !== undefined) {
return { kind: "invalid_tool_display_name", displayName: badDisplayName };
}
const {
static_headers: staticHeadersList,
env_vars: envVarsList,
credentials: credentialValues,
stdio_config: rawStdioConfig,
env_json: rawEnvJson,
command: rawCommand,
args: rawArgs,
allow_all_keys: allowAllKeysRaw,
available_on_public_internet: availableOnPublicInternetRaw,
delegate_auth_to_upstream: delegateAuthToUpstreamRaw,
oauth_passthrough: oauthPassthroughRaw,
dcr_bridge: dcrBridgeRaw,
token_validation_json: rawTokenValidationJson,
...restValues
} = values;
const stdio: StdioFieldsResult =
restValues.transport === "stdio"
? buildStdioFields(rawStdioConfig, rawEnvJson, rawCommand, rawArgs)
: { kind: "ok", fields: {} };
if (stdio.kind !== "ok") return stdio;
const rawTokenValidation = rawTokenValidationJson as string | undefined;
const tokenValidationResult: JsonParseResult =
rawTokenValidation && rawTokenValidation.trim() !== ""
? tryParseJson(rawTokenValidation)
: { kind: "ok", value: null };
if (tokenValidationResult.kind === "invalid") return { kind: "invalid_token_validation_json" };
const tokenValidation = tokenValidationResult.value as Record<string, unknown> | null;
// "openapi" is a UI-only transport; the backend stores those servers as plain http.
const transport = restValues.transport === TRANSPORT.OPENAPI ? "http" : restValues.transport;
const authType = restValues.auth_type as string | undefined;
const accessGroups = ((restValues.mcp_access_groups as unknown[] | undefined) || []).map((g: unknown) =>
typeof g === "string" ? g : (g as { name?: string })?.name || String(g),
);
const mcpInfoServerName =
[
restValues.server_name as string | undefined,
restValues.url as string | undefined,
mcpServer.server_name,
mcpServer.url,
restValues.alias as string | undefined,
mcpServer.alias,
].find(Boolean) ?? "unknown";
const toolAllowlistEnforced =
ui.hasExistingToolAllowlist || ui.hasToolAllowlistInteraction || ui.allowedTools.length > 0;
const extraHeaders = Array.isArray(restValues.extra_headers) ? restValues.extra_headers : [];
const hasAuthorizationHeader = extraHeaders.some(
(h: unknown) => typeof h === "string" && h.toLowerCase() === "authorization",
);
const isNoneAuth = authType === AUTH_TYPE.NONE || authType == null;
const credentialsPayload = filterCredentials(credentialValues);
const includeCredentials = authType !== undefined && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(authType);
// Client-forwarded rows persist ONLY the declared app; strip any token material lingering in the
// form (e.g. from a prior oauth2 authorize this session) so it can never reach the row.
const submitCredentials = isClientForwardedTokenMode(authType)
? preservedAdminCredentials(credentialsPayload)
: credentialsPayload;
const persistedCredentials =
includeCredentials && submitCredentials && Object.keys(submitCredentials).length > 0
? submitCredentials
: undefined;
// Explicit removal of a saved app wins over the filter above. Blank fields are the keep-existing
// convention (the backend merges partial credential updates), so removal must be an explicit-null
// write: encrypt skips nulls and the merge overrides the stored keys, returning the server to
// dynamic client registration.
const credentials =
ui.removeStoredApp && isClientForwardedTokenMode(authType)
? { client_id: null, client_secret: null }
: persistedCredentials;
return {
kind: "ok",
payload: {
...restValues,
...(transport === restValues.transport ? {} : { transport }),
...stdio.fields,
stdio_config: undefined,
env_json: undefined,
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2 && authType !== AUTH_TYPE.OAUTH2
? { issuer: null, authorization_url: null, token_url: null, registration_url: null }
: {}),
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE && authType !== AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE
? { token_exchange_endpoint: null, audience: null, subject_token_type: null, token_exchange_profile: null }
: {}),
server_id: mcpServer.server_id,
mcp_info: {
...(mcpServer.mcp_info ?? {}),
server_name: mcpInfoServerName,
description: restValues.description,
logo_url: ui.logoUrl || undefined,
mcp_server_cost_info: Object.keys(ui.costConfig).length > 0 ? ui.costConfig : null,
tool_allowlist_enforced: toolAllowlistEnforced,
},
mcp_access_groups: accessGroups,
alias: restValues.alias,
extra_headers: restValues.extra_headers || [],
...(toolAllowlistEnforced ? { allowed_tools: [...ui.allowedTools] } : {}),
tool_name_to_display_name: Object.keys(ui.toolNameToDisplayName).length > 0 ? ui.toolNameToDisplayName : null,
tool_name_to_description: Object.keys(ui.toolNameToDescription).length > 0 ? ui.toolNameToDescription : null,
disallowed_tools: restValues.disallowed_tools || [],
static_headers: reduceStaticHeaders(staticHeadersList),
env_vars: normalizeEnvVars(envVarsList),
allow_all_keys: Boolean(allowAllKeysRaw ?? mcpServer.allow_all_keys),
available_on_public_internet: Boolean(availableOnPublicInternetRaw ?? mcpServer.available_on_public_internet),
// ``delegate_auth_to_upstream`` is only honored server-side for ``auth_type=oauth2`` (PKCE
// passthrough). The field is conditionally rendered so the value drops out of the form on
// auth_type change; force false for any other configuration to avoid persisting a stale
// ``true`` that would silently re-activate if the configuration is later switched back.
delegate_auth_to_upstream:
authType === AUTH_TYPE.OAUTH2
? Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream)
: false,
// ``oauth_passthrough`` is the dedicated, non-oauth2 opt-in, only honored for
// ``auth_type=none`` servers that forward ``Authorization`` upstream. Kept separate from
// ``delegate_auth_to_upstream`` so enabling pass-through never regresses oauth2 servers.
oauth_passthrough:
isNoneAuth && hasAuthorizationHeader ? Boolean(oauthPassthroughRaw ?? mcpServer.oauth_passthrough) : false,
// ``dcr_bridge`` is only meaningful for the client-forwarded token modes. Same stale-value
// reasoning as the two flags above.
dcr_bridge: isClientForwardedTokenMode(authType) ? Boolean(dcrBridgeRaw ?? mcpServer.dcr_bridge) : false,
...(authType === AUTH_TYPE.OAUTH2 && restValues.oauth_flow_type
? {
oauth2_flow:
restValues.oauth_flow_type === OAUTH_FLOW.M2M ? MCP_OAUTH2_FLOW_M2M : MCP_OAUTH2_FLOW_INTERACTIVE,
}
: {}),
...(tokenValidation !== null || mcpServer.token_validation ? { token_validation: tokenValidation } : {}),
...(credentials === undefined ? {} : { credentials }),
},
};
};

325
ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx
View file

@ -11,7 +11,6 @@ import {
isHeldOAuthTokenStale,
preservedAdminCredentials,
preservedDeclaredAppCredentials,
ADMIN_CONFIG_CREDENTIAL_KEYS,
withoutMintedTokenCredentials,
OAUTH_FLOW,
MCP_OAUTH2_FLOW_M2M,
@ -41,13 +40,8 @@ import IdJagFormFields from "./IdJagFormFields";
import OAuthFormFields from "./OAuthFormFields";
import MCPLogoSelector from "./MCPLogoSelector";
import EnvVarsSection from "./EnvVarsSection";
import {
validateMCPServerUrl,
validateMCPServerName,
normalizeEnvVars,
normalizeToolOverrideMap,
TOOL_DISPLAY_NAME_PATTERN,
} from "./utils";
import { validateMCPServerUrl, validateMCPServerName, normalizeToolOverrideMap } from "./utils";
import { BuildEditPayloadResult, buildEditServerPayload } from "./editServerPayload";
import { toast } from "@/lib/toast";
import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow";
import { getSecureItem, setSecureItem } from "@/utils/secureStorage";
@ -71,6 +65,23 @@ const AUTH_TYPES_REQUIRING_CREDENTIALS = [
AUTH_TYPE.TRUE_PASSTHROUGH,
AUTH_TYPE.OAUTH_DELEGATE,
];
const editPayloadErrorMessage = (result: Exclude<BuildEditPayloadResult, { kind: "ok" }>): string => {
switch (result.kind) {
case "invalid_tool_display_name":
return `Tool display name "${result.displayName}" is invalid. Only letters, digits, underscores, and hyphens are allowed (no spaces).`;
case "invalid_stdio_json":
return "Invalid JSON in stdio configuration";
case "stdio_config_missing_command":
return "Stdio configuration must include a command";
case "invalid_stdio_env_json":
return "Invalid JSON in stdio env configuration";
case "stdio_missing_command":
return "Stdio transport requires a command";
case "invalid_token_validation_json":
return "Invalid JSON in Token Validation Rules";
}
};
export const EDIT_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-edit-state";
const MCPServerEdit: React.FC<MCPServerEditProps> = ({
@ -672,283 +683,31 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
const handleSave = async (values: Record<string, any>) => {
if (!accessToken) return;
const invalidDisplayName = Object.entries(toolNameToDisplayName).find(
([, displayName]) => displayName && !TOOL_DISPLAY_NAME_PATTERN.test(displayName),
const result = buildEditServerPayload(
values,
{
costConfig,
allowedTools,
hasExistingToolAllowlist,
hasToolAllowlistInteraction,
toolNameToDisplayName,
toolNameToDescription,
logoUrl,
removeStoredApp,
},
mcpServer,
);
if (invalidDisplayName) {
toast.fromError(
`Tool display name "${invalidDisplayName[1]}" is invalid. Only letters, digits, underscores, and hyphens are allowed (no spaces).`,
);
if (result.kind !== "ok") {
toast.fromError(editPayloadErrorMessage(result));
return;
}
const payload = result.payload;
const authType = values.auth_type;
const delegateAuthToUpstreamRaw = values.delegate_auth_to_upstream;
try {
// Ensure access groups is always a string array
const {
static_headers: staticHeadersList,
env_vars: envVarsList,
credentials: credentialValues,
stdio_config: rawStdioConfig,
env_json: rawEnvJson,
command: rawCommand,
args: rawArgs,
allow_all_keys: allowAllKeysRaw,
available_on_public_internet: availableOnPublicInternetRaw,
delegate_auth_to_upstream: delegateAuthToUpstreamRaw,
oauth_passthrough: oauthPassthroughRaw,
dcr_bridge: dcrBridgeRaw,
token_validation_json: rawTokenValidationJson,
...restValues
} = values;
const accessGroups = (restValues.mcp_access_groups || []).map((g: any) =>
typeof g === "string" ? g : g.name || String(g),
);
const staticHeaders = Array.isArray(staticHeadersList)
? staticHeadersList.reduce((acc: Record<string, string>, entry: Record<string, string>) => {
const header = entry?.header?.trim();
if (!header) {
return acc;
}
acc[header] = (entry?.value ?? "").trim();
return acc;
}, {})
: ({} as Record<string, string>);
const envVars = normalizeEnvVars(envVarsList);
const credentialsPayload =
credentialValues && typeof credentialValues === "object"
? Object.entries(credentialValues).reduce((acc: Record<string, any>, [key, value]) => {
if (value === undefined || value === null || value === "") {
if (value === "" && (ADMIN_CONFIG_CREDENTIAL_KEYS as readonly string[]).includes(key)) {
acc[key] = null;
}
return acc;
}
if (key === "scopes") {
if (Array.isArray(value)) {
const filteredScopes = value.filter((scope) => scope != null && scope !== "");
if (filteredScopes.length > 0) {
acc[key] = filteredScopes;
}
}
} else {
acc[key] = value;
}
return acc;
}, {})
: undefined;
let stdioFields: Record<string, any> = {};
if (restValues.transport === "stdio") {
// Prefer JSON config if provided (matches Create screen behavior)
if (rawStdioConfig) {
try {
const stdioConfig = JSON.parse(rawStdioConfig);
let actualConfig = stdioConfig;
if (stdioConfig?.mcpServers && typeof stdioConfig.mcpServers === "object") {
const serverNames = Object.keys(stdioConfig.mcpServers);
if (serverNames.length > 0) {
actualConfig = stdioConfig.mcpServers[serverNames[0]];
}
}
const parsedArgs = Array.isArray(actualConfig?.args)
? actualConfig.args.map((v: any) => String(v)).filter((v: string) => v.trim() !== "")
: [];
const parsedEnv =
actualConfig?.env && typeof actualConfig.env === "object" && !Array.isArray(actualConfig.env)
? Object.entries(actualConfig.env).reduce((acc: Record<string, string>, [k, v]) => {
if (k == null || String(k).trim() === "") return acc;
acc[String(k)] = v == null ? "" : String(v);
return acc;
}, {})
: {};
stdioFields = {
command: actualConfig?.command ? String(actualConfig.command) : undefined,
args: parsedArgs,
env: parsedEnv,
};
if (!stdioFields.command) {
toast.fromError("Stdio configuration must include a command");
return;
}
} catch {
toast.fromError("Invalid JSON in stdio configuration");
return;
}
} else {
// Dedicated fields path (command/args + env JSON)
let parsedEnv: Record<string, string> = {};
if (rawEnvJson) {
try {
const env = JSON.parse(rawEnvJson);
if (env && typeof env === "object" && !Array.isArray(env)) {
parsedEnv = Object.entries(env).reduce((acc: Record<string, string>, [k, v]) => {
if (k == null || String(k).trim() === "") return acc;
acc[String(k)] = v == null ? "" : String(v);
return acc;
}, {});
}
} catch {
toast.fromError("Invalid JSON in stdio env configuration");
return;
}
}
const parsedArgs = Array.isArray(rawArgs)
? rawArgs.map((v: any) => String(v)).filter((v: string) => v.trim() !== "")
: [];
const parsedCommand = rawCommand ? String(rawCommand).trim() : "";
if (!parsedCommand) {
toast.fromError("Stdio transport requires a command");
return;
}
stdioFields = {
command: parsedCommand,
args: parsedArgs,
env: parsedEnv,
};
}
}
// Map "openapi" transport to "http" for the backend
if (restValues.transport === TRANSPORT.OPENAPI) {
restValues.transport = "http";
}
// Parse token_validation JSON if provided
let tokenValidation: Record<string, any> | null = null;
if (rawTokenValidationJson && rawTokenValidationJson.trim() !== "") {
try {
tokenValidation = JSON.parse(rawTokenValidationJson);
} catch {
toast.fromError("Invalid JSON in Token Validation Rules");
return;
}
}
// Prepare the payload with cost configuration and permission fields
const mcpInfoServerName =
restValues.server_name ||
restValues.url ||
mcpServer.server_name ||
mcpServer.url ||
restValues.alias ||
mcpServer.alias ||
"unknown";
const toolAllowlistEnforced = hasExistingToolAllowlist || hasToolAllowlistInteraction || allowedTools.length > 0;
const payload: Record<string, any> = {
...restValues,
...stdioFields,
// Remove UI-only fields
stdio_config: undefined,
env_json: undefined,
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2 && restValues.auth_type !== AUTH_TYPE.OAUTH2
? { issuer: null, authorization_url: null, token_url: null, registration_url: null }
: {}),
...(mcpServer.auth_type === AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE &&
restValues.auth_type !== AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE
? { token_exchange_endpoint: null, audience: null, subject_token_type: null, token_exchange_profile: null }
: {}),
server_id: mcpServer.server_id,
mcp_info: {
...(mcpServer.mcp_info ?? {}),
server_name: mcpInfoServerName,
description: restValues.description,
logo_url: logoUrl || undefined,
mcp_server_cost_info: Object.keys(costConfig).length > 0 ? costConfig : null,
tool_allowlist_enforced: toolAllowlistEnforced,
},
mcp_access_groups: accessGroups,
alias: restValues.alias,
// Include permission management fields
extra_headers: restValues.extra_headers || [],
...(toolAllowlistEnforced
? {
allowed_tools: allowedTools,
}
: {}),
tool_name_to_display_name: Object.keys(toolNameToDisplayName).length > 0 ? toolNameToDisplayName : null,
tool_name_to_description: Object.keys(toolNameToDescription).length > 0 ? toolNameToDescription : null,
disallowed_tools: restValues.disallowed_tools || [],
static_headers: staticHeaders,
env_vars: envVars,
allow_all_keys: Boolean(allowAllKeysRaw ?? mcpServer.allow_all_keys),
available_on_public_internet: Boolean(availableOnPublicInternetRaw ?? mcpServer.available_on_public_internet),
// ``delegate_auth_to_upstream`` is only honored server-side for
// ``auth_type=oauth2`` (PKCE passthrough). The Form.Item is
// conditionally rendered so the value drops out of the form on
// auth_type change; force false for any other configuration to avoid
// persisting a stale ``true`` that would silently re-activate if the
// configuration is later switched back.
delegate_auth_to_upstream: (() => {
const isOauth2 = restValues.auth_type === AUTH_TYPE.OAUTH2;
return isOauth2 ? Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream) : false;
})(),
// ``oauth_passthrough`` is the dedicated, non-oauth2 opt-in. It is only
// honored for ``auth_type=none`` servers that forward ``Authorization``
// upstream. Kept separate from ``delegate_auth_to_upstream`` so enabling
// pass-through never regresses oauth2 servers. Force false otherwise.
oauth_passthrough: (() => {
const isNoneAuth = restValues.auth_type === AUTH_TYPE.NONE || restValues.auth_type == null;
const extraHeaders = Array.isArray(restValues.extra_headers) ? restValues.extra_headers : [];
const hasAuthorizationHeader = extraHeaders.some(
(h: unknown) => typeof h === "string" && h.toLowerCase() === "authorization",
);
return isNoneAuth && hasAuthorizationHeader
? Boolean(oauthPassthroughRaw ?? mcpServer.oauth_passthrough)
: false;
})(),
// ``dcr_bridge`` is only meaningful for the client-forwarded token
// modes (true_passthrough / oauth_delegate). The Form.Item is
// conditionally rendered so the value drops out of the form on
// auth_type change; force false for any other configuration to avoid
// persisting a stale ``true`` that would silently re-activate if the
// mode is later switched back.
dcr_bridge: isClientForwardedTokenMode(restValues.auth_type)
? Boolean(dcrBridgeRaw ?? mcpServer.dcr_bridge)
: false,
...(restValues.auth_type === AUTH_TYPE.OAUTH2 && restValues.oauth_flow_type
? {
oauth2_flow:
restValues.oauth_flow_type === OAUTH_FLOW.M2M ? MCP_OAUTH2_FLOW_M2M : MCP_OAUTH2_FLOW_INTERACTIVE,
}
: {}),
// Include token_validation when it is set (non-null) or when clearing an existing value
...(tokenValidation !== null || mcpServer.token_validation ? { token_validation: tokenValidation } : {}),
};
const includeCredentials =
restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
// Client-forwarded rows persist ONLY the declared app; strip any token material lingering in the
// form (e.g. from a prior oauth2 authorize this session) so it can never reach the row.
const submitCredentials = isClientForwardedTokenMode(restValues.auth_type)
? preservedAdminCredentials(credentialsPayload)
: credentialsPayload;
if (includeCredentials && submitCredentials && Object.keys(submitCredentials).length > 0) {
payload.credentials = submitCredentials;
}
// Explicit removal of a saved app for the client-forwarded modes, applied AFTER the filter so it
// always wins. Blank fields are the keep-existing convention (the backend merges partial
// credential updates), so removal must be an explicit-null write: encrypt skips nulls and the
// merge overrides the stored keys, returning the server to dynamic client registration.
if (removeStoredApp && isClientForwardedTokenMode(restValues.auth_type)) {
payload.credentials = { client_id: null, client_secret: null };
}
const updated = await updateMCPServer(accessToken, payload);
// Persist the token staged via "Authorize & Fetch" (mirrors the create flow's
@ -957,7 +716,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
// never in the server row. M2M/static auth resolve server-side and need neither.
if (oauthTokenResponse?.access_token) {
const oauthMode = getMcpOAuthMode({
auth_type: restValues.auth_type,
auth_type: authType,
oauth2_flow: isM2MFlow ? MCP_OAUTH2_FLOW_M2M : null,
delegate_auth_to_upstream: Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream),
});
@ -971,7 +730,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
scopes: typeof scope === "string" && scope ? scope.split(" ") : undefined,
};
await storeMCPOAuthUserCredential(accessToken, mcpServer.server_id, oauthCredentialPayload);
} else if (oauthMode === "passthrough" || isClientForwardedTokenMode(restValues.auth_type)) {
} else if (oauthMode === "passthrough" || isClientForwardedTokenMode(authType)) {
const browserHeldToken = {
access_token: oauthTokenResponse.access_token,
expires_in: oauthTokenResponse.expires_in,