diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.cases.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.cases.ts new file mode 100644 index 00000000000..c350ac085b6 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.cases.ts @@ -0,0 +1,391 @@ +import { MCPServer } from "@/components/mcp_tools/types"; +import type { EditServerUiState } from "./editServerPayload"; + +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: [], +}; + +export const baseUi: EditServerUiState = { + mcpServer: SERVER, + logoUrl: undefined, + costConfig: {}, + allowedTools: [], + hasExistingToolAllowlist: false, + hasToolAllowlistInteraction: false, + toolNameToDisplayName: {}, + toolNameToDescription: {}, + removeStoredApp: false, +}; + +export interface DifferentialCase { + readonly label: string; + readonly values: Record; + readonly ui: Partial; +} + +// Always-mounted root fields. A "collapsed" section is modelled as its keys being +// ABSENT, which is what antd's mounted-only onFinish produces, and an expanded but +// untouched one as the key present holding `undefined`. +const ROOT = { + server_name: "srv", + alias: "srv_alias", + description: "a server", + transport: "http", + url: "https://example.com/mcp", + auth_type: "none", + max_concurrent_requests: undefined, + mcp_access_groups: [], + extra_headers: [], + static_headers: [], + env_vars: [], + allow_all_keys: false, + available_on_public_internet: true, +}; + +const CREDS = { + auth_value: "secret-value", + client_id: "cid", + client_secret: "csec", + scopes: ["read", "write"], + aws_region_name: "us-east-1", + aws_service_name: "bedrock", + aws_access_key_id: "AKIA", + aws_secret_access_key: "shh", + aws_session_token: "tok", + aws_role_name: "role", + aws_session_name: "sess", + id_jag_resource: "api://res", + id_jag_resource_token_endpoint: "https://idp/jag", + client_private_key: "-----KEY-----", + client_private_key_id: "kid", + client_assertion_signing_alg: "RS256", + token_endpoint_auth_method: "client_secret_basic", + upstream_resource: "api://up", +}; + +const AUTH_TYPES = [ + "none", + "api_key", + "bearer_token", + "token", + "basic", + "oauth2", + "oauth2_token_exchange", + "oauth2_id_jag", + "aws_sigv4", + "true_passthrough", + "oauth_delegate", +] as const; + +export const CASES: readonly DifferentialCase[] = [ + // --- mount-state axis: everything collapsed / expanded-undefined / expanded-valued --- + { label: "everything collapsed, only the always-mounted root", values: { ...ROOT }, ui: {} }, + { + label: "everything expanded, all optional keys present as undefined", + values: { + ...ROOT, + credentials: {}, + stdio_config: undefined, + env_json: undefined, + command: undefined, + args: undefined, + delegate_auth_to_upstream: undefined, + oauth_passthrough: undefined, + dcr_bridge: undefined, + token_validation_json: undefined, + spec_path: undefined, + oauth_flow_type: undefined, + issuer: undefined, + authorization_url: undefined, + token_url: undefined, + registration_url: undefined, + token_storage_ttl_seconds: undefined, + token_exchange_endpoint: undefined, + token_exchange_profile: undefined, + audience: undefined, + subject_token_type: undefined, + disallowed_tools: undefined, + }, + ui: {}, + }, + { + label: "expand then collapse: the key is absent again even though a value was typed", + values: { ...ROOT }, + ui: {}, + }, + + // --- transport axis --- + { + label: "stdio via the JSON config path", + values: { + ...ROOT, + transport: "stdio", + url: undefined, + stdio_config: JSON.stringify({ command: "npx", args: ["-y", "pkg"], env: { A: "1" } }), + }, + ui: {}, + }, + { + label: "stdio via a wrapped mcpServers config", + values: { + ...ROOT, + transport: "stdio", + url: undefined, + stdio_config: JSON.stringify({ mcpServers: { first: { command: "uvx", args: [1, " ", "b"] } } }), + }, + ui: {}, + }, + { + label: "stdio via the dedicated command/args/env fields", + values: { + ...ROOT, + transport: "stdio", + url: undefined, + command: " npx ", + args: ["-y", " ", "pkg"], + env_json: JSON.stringify({ A: 1, "": "skipped", B: null }), + }, + ui: {}, + }, + { + label: "openapi swaps url for spec_path", + values: { ...ROOT, transport: "openapi", url: undefined, spec_path: "https://x/openapi.json" }, + ui: {}, + }, + + // --- auth-type axis, one case per type, credentials mounted --- + ...AUTH_TYPES.map((auth) => ({ + label: `auth_type ${auth} with the full credential bag mounted`, + values: { ...ROOT, auth_type: auth, credentials: { ...CREDS } }, + ui: {}, + })), + + // --- oauth2 flow sub-branches --- + { + label: "oauth2 m2m flow", + values: { + ...ROOT, + auth_type: "oauth2", + oauth_flow_type: "m2m", + token_url: "https://idp/token", + credentials: { ...CREDS }, + }, + ui: {}, + }, + { + label: "oauth2 interactive flow", + values: { + ...ROOT, + auth_type: "oauth2", + oauth_flow_type: "interactive", + issuer: "https://idp", + credentials: { ...CREDS }, + }, + ui: {}, + }, + { + label: "oauth2 with delegate_auth_to_upstream on", + values: { ...ROOT, auth_type: "oauth2", delegate_auth_to_upstream: true, credentials: { ...CREDS } }, + ui: {}, + }, + + // --- the five coalesced keys: unbound raw against each server-side fallback --- + { + label: "coalesced booleans unbound, server has them all true", + values: { ...ROOT, auth_type: "oauth2", allow_all_keys: undefined, available_on_public_internet: undefined }, + ui: { + mcpServer: { + ...SERVER, + auth_type: "oauth2", + allow_all_keys: true, + available_on_public_internet: true, + delegate_auth_to_upstream: true, + oauth_passthrough: true, + dcr_bridge: true, + }, + }, + }, + { + label: "coalesced booleans unbound, server has them all false", + values: { ...ROOT, allow_all_keys: undefined, available_on_public_internet: undefined }, + ui: { mcpServer: { ...SERVER, allow_all_keys: false, available_on_public_internet: false } }, + }, + { + label: "dcr_bridge bound true on a client-forwarded mode", + values: { ...ROOT, auth_type: "true_passthrough", dcr_bridge: true }, + ui: { mcpServer: { ...SERVER, auth_type: "true_passthrough" } }, + }, + { + label: "dcr_bridge bound true but auth switched away, forced false", + values: { ...ROOT, auth_type: "api_key", dcr_bridge: true }, + ui: {}, + }, + { + label: "oauth_passthrough with an Authorization extra header", + values: { ...ROOT, auth_type: "none", extra_headers: ["Authorization"], oauth_passthrough: true }, + ui: {}, + }, + { + label: "oauth_passthrough without the Authorization header, forced false", + values: { ...ROOT, auth_type: "none", extra_headers: ["X-Other"], oauth_passthrough: true }, + ui: {}, + }, + + // --- auth-type transitions that null out the previous subtree --- + { + label: "was oauth2, now api_key: nulls the four oauth endpoints", + values: { ...ROOT, auth_type: "api_key", credentials: { auth_value: "v" } }, + ui: { mcpServer: { ...SERVER, auth_type: "oauth2" } }, + }, + { + label: "was token_exchange, now none: nulls the four exchange fields", + values: { ...ROOT, auth_type: "none" }, + ui: { mcpServer: { ...SERVER, auth_type: "oauth2_token_exchange" } }, + }, + + // --- credentials filtering --- + // ADMIN_CONFIG_CREDENTIAL_KEYS is exactly ["upstream_resource"], so only that key + // takes the blank-to-explicit-null branch. A blank client_id is dropped instead. + { + label: "blank upstream_resource becomes an explicit null", + values: { ...ROOT, auth_type: "oauth2", credentials: { upstream_resource: "", client_secret: "keep" } }, + ui: {}, + }, + { + label: "blank non-admin credential is dropped, not nulled", + values: { ...ROOT, auth_type: "oauth2", credentials: { client_id: "", client_secret: "keep", scopes: [] } }, + ui: {}, + }, + { + label: "scopes array filters empties and drops when nothing survives", + values: { ...ROOT, auth_type: "oauth2", credentials: { scopes: ["", null, "read"] } }, + ui: {}, + }, + { + label: "scopes entirely empty drops the key", + values: { ...ROOT, auth_type: "oauth2", credentials: { scopes: ["", null] } }, + ui: {}, + }, + { label: "credentials absent entirely", values: { ...ROOT, auth_type: "oauth2" }, ui: {} }, + { + label: "removeStoredApp forces an explicit-null app write", + values: { ...ROOT, auth_type: "true_passthrough", credentials: { ...CREDS } }, + ui: { removeStoredApp: true, mcpServer: { ...SERVER, auth_type: "true_passthrough" } }, + }, + { + label: "removeStoredApp ignored outside a client-forwarded mode", + values: { ...ROOT, auth_type: "api_key", credentials: { ...CREDS } }, + ui: { removeStoredApp: true }, + }, + + // --- headers, env vars, access groups --- + { + label: "static headers trim and drop blank names", + values: { + ...ROOT, + static_headers: [ + { header: " X-A ", value: " v " }, + { header: " ", value: "x" }, + ], + }, + ui: {}, + }, + { + label: "env vars normalize", + values: { ...ROOT, env_vars: [{ name: "A", value: "1", scope: "server", description: "d" }] }, + ui: {}, + }, + { label: "access groups given as objects", values: { ...ROOT, mcp_access_groups: [{ name: "g1" }, "g2"] }, ui: {} }, + { label: "extra_headers absent falls back to an empty array", values: { ...ROOT, extra_headers: undefined }, ui: {} }, + + // --- tool allowlist and overrides --- + { + label: "existing allowlist enforces and emits allowed_tools", + values: { ...ROOT }, + ui: { hasExistingToolAllowlist: true, allowedTools: ["alpha"] }, + }, + { + label: "allowlist interaction with an empty list still enforces", + values: { ...ROOT }, + ui: { hasToolAllowlistInteraction: true }, + }, + { + label: "tool display name and description maps present", + values: { ...ROOT }, + ui: { toolNameToDisplayName: { a: "Alpha" }, toolNameToDescription: { a: "desc" } }, + }, + + // --- cost config, logo, token validation --- + // mcp_info.server_name walks a six-step fallback chain. Every step needs a case whose + // earlier terms are falsy, or the chain is unreachable and a mutation deleting it survives. + { label: "mcp_info server name falls back to the form url", values: { ...ROOT, server_name: "" }, ui: {} }, + { + label: "mcp_info server name falls back to the stored server_name", + values: { ...ROOT, server_name: "", url: "" }, + ui: {}, + }, + { + label: "mcp_info server name falls back to the stored url", + values: { ...ROOT, server_name: "", url: "" }, + ui: { mcpServer: { ...SERVER, server_name: "" } }, + }, + { + label: "mcp_info server name falls back to the form alias", + values: { ...ROOT, server_name: "", url: "" }, + ui: { mcpServer: { ...SERVER, server_name: "", url: "" } }, + }, + { + label: "mcp_info server name falls back to unknown", + values: { ...ROOT, server_name: "", url: "", alias: "" }, + ui: { mcpServer: { ...SERVER, server_name: "", url: "", alias: "" } }, + }, + + { label: "cost config present", values: { ...ROOT }, ui: { costConfig: { default_cost_per_query: 0.01 } as never } }, + { label: "logo url present", values: { ...ROOT }, ui: { logoUrl: "https://cdn/logo.png" } }, + { label: "token validation JSON parses", values: { ...ROOT, token_validation_json: '{"aud":"x"}' }, ui: {} }, + { + label: "token validation blank clears an existing value", + values: { ...ROOT, token_validation_json: " " }, + ui: { mcpServer: { ...SERVER, token_validation: { aud: "old" } } }, + }, + { + label: "token validation blank with no existing value omits the key", + values: { ...ROOT, token_validation_json: "" }, + ui: {}, + }, + + // --- the six failure branches --- + { label: "ERR invalid tool display name", values: { ...ROOT }, ui: { toolNameToDisplayName: { a: "has spaces" } } }, + { + label: "ERR stdio config missing a command", + values: { ...ROOT, transport: "stdio", stdio_config: JSON.stringify({ args: [] }) }, + ui: {}, + }, + { + label: "ERR stdio config invalid JSON", + values: { ...ROOT, transport: "stdio", stdio_config: "{not json" }, + ui: {}, + }, + { + label: "ERR stdio env invalid JSON", + values: { ...ROOT, transport: "stdio", command: "npx", env_json: "{not json" }, + ui: {}, + }, + { + label: "ERR stdio dedicated path with a blank command", + values: { ...ROOT, transport: "stdio", command: " " }, + ui: {}, + }, + { label: "ERR token validation invalid JSON", values: { ...ROOT, token_validation_json: "{not json" }, ui: {} }, +]; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.test.ts new file mode 100644 index 00000000000..05e64468f4f --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.differential.test.ts @@ -0,0 +1,331 @@ +import { describe, expect, it } from "vitest"; +import { + ADMIN_CONFIG_CREDENTIAL_KEYS, + AUTH_TYPE, + MCPServer, + MCP_OAUTH2_FLOW_INTERACTIVE, + MCP_OAUTH2_FLOW_M2M, + OAUTH_FLOW, + TRANSPORT, + isClientForwardedTokenMode, + preservedAdminCredentials, +} from "@/components/mcp_tools/types"; +import { AUTH_TYPES_REQUIRING_CREDENTIALS } from "./createServerPayload"; +import { TOOL_DISPLAY_NAME_PATTERN, normalizeEnvVars } from "./utils"; +import { buildEditServerPayload, type EditServerUiState } from "./editServerPayload"; +import { CASES, baseUi } from "./editServerPayload.differential.cases"; + +// GENERATED by scratchpad/emit_test.py. The body below is machine-extracted from +// mcp_server_edit.tsx lines 675-950 and converted by exact string replacement; do +// not hand-edit it. Regenerate instead, so the baseline can never drift toward +// the implementation it is supposed to check. +const legacyBuild = (values: Record, ui: EditServerUiState) => { + const { + mcpServer, + logoUrl, + costConfig, + allowedTools, + hasExistingToolAllowlist, + hasToolAllowlistInteraction, + toolNameToDisplayName, + toolNameToDescription, + removeStoredApp, + } = ui; + const invalidDisplayName = Object.entries(toolNameToDisplayName).find( + ([, displayName]) => displayName && !TOOL_DISPLAY_NAME_PATTERN.test(displayName), + ); + if (invalidDisplayName) { + return { kind: "invalid_tool_display_name" as const, displayName: String(invalidDisplayName[1]) }; + } + // 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, entry: Record) => { + const header = entry?.header?.trim(); + if (!header) { + return acc; + } + acc[header] = (entry?.value ?? "").trim(); + return acc; + }, {}) + : ({} as Record); + + const envVars = normalizeEnvVars(envVarsList); + + const credentialsPayload = + credentialValues && typeof credentialValues === "object" + ? Object.entries(credentialValues).reduce((acc: Record, [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 = {}; + + 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, [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) { + return { kind: "stdio_config_missing_command" as const }; + } + } catch { + return { kind: "invalid_stdio_json" as const }; + } + } else { + // Dedicated fields path (command/args + env JSON) + let parsedEnv: Record = {}; + if (rawEnvJson) { + try { + const env = JSON.parse(rawEnvJson); + if (env && typeof env === "object" && !Array.isArray(env)) { + parsedEnv = Object.entries(env).reduce((acc: Record, [k, v]) => { + if (k == null || String(k).trim() === "") return acc; + acc[String(k)] = v == null ? "" : String(v); + return acc; + }, {}); + } + } catch { + return { kind: "invalid_stdio_env_json" as const }; + } + } + const parsedArgs = Array.isArray(rawArgs) + ? rawArgs.map((v: any) => String(v)).filter((v: string) => v.trim() !== "") + : []; + + const parsedCommand = rawCommand ? String(rawCommand).trim() : ""; + if (!parsedCommand) { + return { kind: "stdio_command_required" as const }; + } + + 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 | null = null; + if (rawTokenValidationJson && rawTokenValidationJson.trim() !== "") { + try { + tokenValidation = JSON.parse(rawTokenValidationJson); + } catch { + return { kind: "invalid_token_validation_json" as const }; + } + } + + // 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 = { + ...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 }; + } + return { kind: "ok" as const, payload }; +}; + +const clone = (v: T): T => structuredClone(v); + +describe("buildEditServerPayload matches the pre-extraction handleSave body", () => { + it.each(CASES.map((c) => [c.label, c] as const))("%s", (_label, testCase) => { + const ui: EditServerUiState = { ...baseUi, ...testCase.ui } as EditServerUiState; + const legacy = legacyBuild(clone(testCase.values), clone(ui) as EditServerUiState); + const next = buildEditServerPayload(clone(testCase.values), clone(ui) as EditServerUiState); + + expect(next.kind).toBe(legacy.kind); + if (legacy.kind !== "ok" || next.kind !== "ok") { + expect(next).toStrictEqual(legacy); + return; + } + expect(next.payload).toStrictEqual(legacy.payload); + expect(Object.keys(next.payload)).toStrictEqual(Object.keys(legacy.payload)); + expect(JSON.stringify(next.payload)).toBe(JSON.stringify(legacy.payload)); + }); +}); + +void ADMIN_CONFIG_CREDENTIAL_KEYS; +void AUTH_TYPE; +void AUTH_TYPES_REQUIRING_CREDENTIALS; +void MCP_OAUTH2_FLOW_INTERACTIVE; +void MCP_OAUTH2_FLOW_M2M; +void OAUTH_FLOW; +void TOOL_DISPLAY_NAME_PATTERN; +void TRANSPORT; +void isClientForwardedTokenMode; +void normalizeEnvVars; +void preservedAdminCredentials; +export type { MCPServer }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.ts new file mode 100644 index 00000000000..3930f1edb9b --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/editServerPayload.ts @@ -0,0 +1,405 @@ +import { + ADMIN_CONFIG_CREDENTIAL_KEYS, + AUTH_TYPE, + MCPEnvVar, + MCPInfo, + MCPServer, + MCPServerCostInfo, + MCP_OAUTH2_FLOW_INTERACTIVE, + MCP_OAUTH2_FLOW_M2M, + OAUTH_FLOW, + TRANSPORT, + isClientForwardedTokenMode, + preservedAdminCredentials, +} from "@/components/mcp_tools/types"; +import { AUTH_TYPES_REQUIRING_CREDENTIALS, reduceStaticHeaders } from "./createServerPayload"; +import { TOOL_DISPLAY_NAME_PATTERN, normalizeEnvVars } from "./utils"; + +export type MCPAccessGroupValue = string | { readonly name?: string }; + +export interface EditServerFormValues { + readonly static_headers?: unknown; + readonly env_vars?: unknown; + readonly credentials?: unknown; + readonly stdio_config?: string; + readonly env_json?: string; + readonly command?: string; + readonly args?: readonly string[]; + readonly allow_all_keys?: boolean; + readonly available_on_public_internet?: boolean; + readonly delegate_auth_to_upstream?: boolean; + readonly oauth_passthrough?: boolean; + readonly dcr_bridge?: boolean; + readonly token_validation_json?: string; + readonly mcp_access_groups?: readonly MCPAccessGroupValue[]; + readonly transport?: string; + readonly server_name?: string; + readonly url?: string; + readonly alias?: string; + readonly description?: string; + readonly auth_type?: string; + readonly extra_headers?: readonly string[]; + readonly disallowed_tools?: readonly string[]; + readonly oauth_flow_type?: string; + readonly [key: string]: unknown; +} + +export interface EditServerPayload { + readonly server_id: string; + readonly mcp_info: MCPInfo; + readonly mcp_access_groups: readonly string[]; + readonly alias: string | undefined; + readonly extra_headers: readonly string[]; + readonly disallowed_tools: readonly string[]; + readonly static_headers: Readonly>; + readonly env_vars: readonly MCPEnvVar[]; + readonly allow_all_keys: boolean; + readonly available_on_public_internet: boolean; + readonly delegate_auth_to_upstream: boolean; + readonly oauth_passthrough: boolean; + readonly dcr_bridge: boolean; + readonly stdio_config: undefined; + readonly env_json: undefined; + readonly command?: string; + readonly args?: readonly string[]; + readonly env?: Readonly>; + readonly allowed_tools?: readonly string[]; + readonly tool_name_to_display_name: Readonly> | null; + readonly tool_name_to_description: Readonly> | null; + readonly oauth2_flow?: string; + readonly token_validation?: unknown; + readonly credentials?: Readonly>; + readonly [key: string]: unknown; +} + +export interface EditServerUiState { + readonly mcpServer: MCPServer; + readonly logoUrl: string | undefined; + readonly costConfig: MCPServerCostInfo; + readonly allowedTools: readonly string[]; + readonly hasExistingToolAllowlist: boolean; + readonly hasToolAllowlistInteraction: boolean; + readonly toolNameToDisplayName: Readonly>; + readonly toolNameToDescription: Readonly>; + readonly removeStoredApp: boolean; +} + +export type BuildEditPayloadResult = + | { readonly kind: "ok"; readonly payload: EditServerPayload } + | { readonly kind: "invalid_tool_display_name"; readonly displayName: string } + | { readonly kind: "stdio_config_missing_command" } + | { readonly kind: "invalid_stdio_json" } + | { readonly kind: "invalid_stdio_env_json" } + | { readonly kind: "stdio_command_required" } + | { readonly kind: "invalid_token_validation_json" }; + +interface StdioFields { + readonly command: string; + readonly args: readonly string[]; + readonly env: Readonly>; +} + +type StdioFieldsResult = + | { readonly kind: "ok"; readonly fields: StdioFields | Record } + | { readonly kind: "stdio_config_missing_command" } + | { readonly kind: "invalid_stdio_json" } + | { readonly kind: "invalid_stdio_env_json" } + | { readonly kind: "stdio_command_required" }; + +type TokenValidationResult = { readonly kind: "ok"; readonly value: unknown } | { readonly kind: "invalid" }; + +const assertNever = (value: never): never => { + throw new Error(`unhandled edit payload result: ${JSON.stringify(value)}`); +}; + +export const editPayloadErrorMessage = (result: Exclude): 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 "stdio_config_missing_command": + return "Stdio configuration must include a command"; + case "invalid_stdio_json": + return "Invalid JSON in stdio configuration"; + case "invalid_stdio_env_json": + return "Invalid JSON in stdio env configuration"; + case "stdio_command_required": + return "Stdio transport requires a command"; + case "invalid_token_validation_json": + return "Invalid JSON in Token Validation Rules"; + default: + return assertNever(result); + } +}; + +const toStringArgs = (raw: unknown): readonly string[] => + Array.isArray(raw) ? raw.map((v: unknown) => String(v)).filter((v: string) => v.trim() !== "") : []; + +const toEnvRecord = (raw: unknown): Readonly> => + raw && typeof raw === "object" && !Array.isArray(raw) + ? Object.fromEntries( + Object.entries(raw) + .filter(([k]) => k != null && String(k).trim() !== "") + .map(([k, v]) => [String(k), v == null ? "" : String(v)]), + ) + : {}; + +const buildStdioFields = ( + rawStdioConfig: string | undefined, + rawEnvJson: string | undefined, + rawCommand: string | undefined, + rawArgs: readonly string[] | undefined, +): StdioFieldsResult => { + if (rawStdioConfig) { + try { + const stdioConfig: unknown = JSON.parse(rawStdioConfig); + const configRecord = stdioConfig && typeof stdioConfig === "object" ? (stdioConfig as StdioConfigShape) : null; + const namedServers = + configRecord?.mcpServers && typeof configRecord.mcpServers === "object" ? configRecord.mcpServers : null; + const named = namedServers ? Object.keys(namedServers) : []; + const actualConfig = named.length > 0 && namedServers ? namedServers[named[0]] : configRecord; + const command = actualConfig?.command ? String(actualConfig.command) : undefined; + if (!command) { + return { kind: "stdio_config_missing_command" }; + } + return { + kind: "ok", + fields: { command, args: toStringArgs(actualConfig?.args), env: toEnvRecord(actualConfig?.env) }, + }; + } catch { + return { kind: "invalid_stdio_json" }; + } + } + + const envResult = ((): Readonly> | "invalid" => { + if (!rawEnvJson) return {}; + try { + return toEnvRecord(JSON.parse(rawEnvJson)); + } catch { + return "invalid"; + } + })(); + if (envResult === "invalid") { + return { kind: "invalid_stdio_env_json" }; + } + + const parsedCommand = rawCommand ? String(rawCommand).trim() : ""; + if (!parsedCommand) { + return { kind: "stdio_command_required" }; + } + return { kind: "ok", fields: { command: parsedCommand, args: toStringArgs(rawArgs), env: envResult } }; +}; + +interface StdioConfigShape { + readonly mcpServers?: Readonly>; + readonly command?: unknown; + readonly args?: unknown; + readonly env?: unknown; +} + +const buildCredentials = (credentialValues: unknown): Readonly> | undefined => { + if (!credentialValues || typeof credentialValues !== "object") return undefined; + const kept = Object.entries(credentialValues).flatMap(([key, value]): readonly (readonly [string, unknown])[] => { + if (value === undefined || value === null || value === "") { + return value === "" && (ADMIN_CONFIG_CREDENTIAL_KEYS as readonly string[]).includes(key) + ? [[key, null] as const] + : []; + } + if (key !== "scopes") return [[key, value] as const]; + if (!Array.isArray(value)) return []; + const filteredScopes = value.filter((scope: unknown) => scope != null && scope !== ""); + return filteredScopes.length > 0 ? [[key, filteredScopes] as const] : []; + }); + return Object.fromEntries(kept); +}; + +interface CredentialsEntryInput { + readonly authType: string | undefined; + readonly credentials: Readonly> | undefined; + readonly includeCredentials: boolean; + readonly removeStoredApp: boolean; +} + +// 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. +const resolveCredentialsEntry = ({ + authType, + credentials, + includeCredentials, + removeStoredApp, +}: CredentialsEntryInput): { readonly credentials?: Readonly> } => { + if (removeStoredApp && isClientForwardedTokenMode(authType)) { + return { credentials: { client_id: null, client_secret: null } }; + } + if (includeCredentials && credentials && Object.keys(credentials).length > 0) { + return { credentials }; + } + return {}; +}; + +export const buildEditServerPayload = (values: EditServerFormValues, ui: EditServerUiState): BuildEditPayloadResult => { + const { + mcpServer, + logoUrl, + costConfig, + allowedTools, + hasExistingToolAllowlist, + hasToolAllowlistInteraction, + toolNameToDisplayName, + toolNameToDescription, + removeStoredApp, + } = ui; + + const invalidDisplayName = Object.entries(toolNameToDisplayName).find( + ([, displayName]) => displayName && !TOOL_DISPLAY_NAME_PATTERN.test(displayName), + ); + if (invalidDisplayName) { + return { kind: "invalid_tool_display_name", displayName: String(invalidDisplayName[1]) }; + } + + 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, + ...rawRestValues + } = values; + + const accessGroups = (rawRestValues.mcp_access_groups || []).map((g) => + typeof g === "string" ? g : g.name || String(g), + ); + const staticHeaders = reduceStaticHeaders(staticHeadersList); + const envVars = normalizeEnvVars(envVarsList); + const credentialsPayload = buildCredentials(credentialValues); + + const stdio = + rawRestValues.transport === "stdio" + ? buildStdioFields(rawStdioConfig, rawEnvJson, rawCommand, rawArgs) + : ({ kind: "ok", fields: {} } as StdioFieldsResult); + if (stdio.kind !== "ok") { + return stdio; + } + + // Map "openapi" transport to "http" for the backend. Rewriting the existing key keeps its + // position, which the payload's serialised key order depends on. + const restValues = + rawRestValues.transport === TRANSPORT.OPENAPI ? { ...rawRestValues, transport: "http" } : rawRestValues; + + const tokenValidation = ((): TokenValidationResult => { + if (!rawTokenValidationJson || rawTokenValidationJson.trim() === "") return { kind: "ok", value: null }; + try { + return { kind: "ok", value: JSON.parse(rawTokenValidationJson) }; + } catch { + return { kind: "invalid" }; + } + })(); + if (tokenValidation.kind === "invalid") { + return { kind: "invalid_token_validation_json" }; + } + + 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 extraHeaders = restValues.extra_headers || []; + const hasAuthorizationHeader = extraHeaders.some((h) => typeof h === "string" && h.toLowerCase() === "authorization"); + const isNoneAuth = restValues.auth_type === AUTH_TYPE.NONE || restValues.auth_type == null; + + // 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; + const includeCredentials = restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type); + + const credentialsEntryInput: CredentialsEntryInput = { + authType: restValues.auth_type, + credentials: submitCredentials, + includeCredentials: Boolean(includeCredentials), + removeStoredApp, + }; + const credentialsEntry = resolveCredentialsEntry(credentialsEntryInput); + + const payload: EditServerPayload = { + ...restValues, + ...stdio.fields, + // 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: extraHeaders, + ...(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 field unmounts on auth_type change, so force false for any other + // configuration to avoid persisting a stale ``true`` that would silently re-activate. + delegate_auth_to_upstream: + restValues.auth_type === AUTH_TYPE.OAUTH2 + ? Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream) + : false, + // ``oauth_passthrough`` is the dedicated, non-oauth2 opt-in, honored only 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. The field unmounts on + // auth_type change, so force false otherwise rather than persisting a stale ``true``. + 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.value !== null || mcpServer.token_validation + ? { token_validation: tokenValidation.value } + : {}), + ...credentialsEntry, + }; + + return { kind: "ok", payload }; +}; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx index 5c3fe6d8cda..c6ebde13bb8 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_edit.tsx @@ -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 { EditServerFormValues, buildEditServerPayload, editPayloadErrorMessage } from "./editServerPayload"; import { toast } from "@/lib/toast"; import { useMcpOAuthFlow } from "@/hooks/useMcpOAuthFlow"; import { getSecureItem, setSecureItem } from "@/utils/secureStorage"; @@ -62,15 +56,6 @@ interface MCPServerEditProps { } const AUTH_TYPES_REQUIRING_AUTH_VALUE = [AUTH_TYPE.API_KEY, AUTH_TYPE.BEARER_TOKEN, AUTH_TYPE.TOKEN, AUTH_TYPE.BASIC]; -const AUTH_TYPES_REQUIRING_CREDENTIALS = [ - ...AUTH_TYPES_REQUIRING_AUTH_VALUE, - AUTH_TYPE.OAUTH2, - AUTH_TYPE.OAUTH2_TOKEN_EXCHANGE, - AUTH_TYPE.OAUTH2_ID_JAG, - AUTH_TYPE.AWS_SIGV4, - AUTH_TYPE.TRUE_PASSTHROUGH, - AUTH_TYPE.OAUTH_DELEGATE, -]; export const EDIT_OAUTH_UI_STATE_KEY = "litellm-mcp-oauth-edit-state"; const MCPServerEdit: React.FC = ({ @@ -670,284 +655,25 @@ const MCPServerEdit: React.FC = ({ } }; - const handleSave = async (values: Record) => { + const handleSave = async (values: EditServerFormValues) => { if (!accessToken) return; - const invalidDisplayName = Object.entries(toolNameToDisplayName).find( - ([, displayName]) => displayName && !TOOL_DISPLAY_NAME_PATTERN.test(displayName), - ); - if (invalidDisplayName) { - toast.fromError( - `Tool display name "${invalidDisplayName[1]}" is invalid. Only letters, digits, underscores, and hyphens are allowed (no spaces).`, - ); - return; - } 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, entry: Record) => { - const header = entry?.header?.trim(); - if (!header) { - return acc; - } - acc[header] = (entry?.value ?? "").trim(); - return acc; - }, {}) - : ({} as Record); - - const envVars = normalizeEnvVars(envVarsList); - - const credentialsPayload = - credentialValues && typeof credentialValues === "object" - ? Object.entries(credentialValues).reduce((acc: Record, [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 = {}; - - 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, [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 = {}; - if (rawEnvJson) { - try { - const env = JSON.parse(rawEnvJson); - if (env && typeof env === "object" && !Array.isArray(env)) { - parsedEnv = Object.entries(env).reduce((acc: Record, [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 | 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 = { - ...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 built = buildEditServerPayload(values, { + mcpServer, + logoUrl, + costConfig, + allowedTools, + hasExistingToolAllowlist, + hasToolAllowlistInteraction, + toolNameToDisplayName, + toolNameToDescription, + removeStoredApp, + }); + if (built.kind !== "ok") { + toast.fromError(editPayloadErrorMessage(built)); + return; } + const payload = built.payload; const updated = await updateMCPServer(accessToken, payload); @@ -957,9 +683,9 @@ const MCPServerEdit: React.FC = ({ // 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: values.auth_type, oauth2_flow: isM2MFlow ? MCP_OAUTH2_FLOW_M2M : null, - delegate_auth_to_upstream: Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream), + delegate_auth_to_upstream: Boolean(values.delegate_auth_to_upstream ?? mcpServer.delegate_auth_to_upstream), }); try { if (oauthMode === "authorization_code") { @@ -971,7 +697,7 @@ const MCPServerEdit: React.FC = ({ 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(values.auth_type)) { const browserHeldToken = { access_token: oauthTokenResponse.access_token, expires_in: oauthTokenResponse.expires_in,