From 23e151a016c89caf7516418d7d990cebeff0499d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Jul 2026 16:09:26 -0700 Subject: [PATCH] fix(config): reject reserved __proto__ MCP server name (#116112) --- docs/tools/mcp.md | 2 +- src/agents/mcp-connection-resolver.test.ts | 16 ++++++ src/agents/mcp-connection-resolver.ts | 7 ++- src/commands/doctor-config-flow.ts | 3 + .../doctor-legacy-config.migrations.test.ts | 34 +++++++++++ .../shared/legacy-config-core-migrate.ts | 6 +- .../reserved-mcp-server-name-migrate.ts | 51 +++++++++++++++++ src/config/schema.test.ts | 50 ++++++++++++++++ src/config/validation-core.ts | 57 ++++++++++++++++++- src/config/zod-schema.root-support.ts | 47 ++++++++++++--- 10 files changed, 258 insertions(+), 15 deletions(-) create mode 100644 src/commands/doctor/shared/reserved-mcp-server-name-migrate.ts diff --git a/docs/tools/mcp.md b/docs/tools/mcp.md index 82888a5281c9..cb79b398d39e 100644 --- a/docs/tools/mcp.md +++ b/docs/tools/mcp.md @@ -91,7 +91,7 @@ The same `docs` server, written straight into config: } ``` -An enabled server needs either a command (stdio) or a URL (SSE or Streamable HTTP). Setting `enabled: false` keeps the definition around without connecting it. Keep credentials out of config literals — store sensitive headers and environment values through the supported secret mechanisms. +An enabled server needs either a command (stdio) or a URL (SSE or Streamable HTTP). The exact server name `__proto__` is reserved; choose a different name. Setting `enabled: false` keeps the definition around without connecting it. Keep credentials out of config literals — store sensitive headers and environment values through the supported secret mechanisms. ## Troubleshooting diff --git a/src/agents/mcp-connection-resolver.test.ts b/src/agents/mcp-connection-resolver.test.ts index 94820b0f90bb..afa62c06b89b 100644 --- a/src/agents/mcp-connection-resolver.test.ts +++ b/src/agents/mcp-connection-resolver.test.ts @@ -207,6 +207,22 @@ describe("mcp connection resolver helpers", () => { expect(requesterScopedServerNames).toEqual(["user-mail"]); }); + it("preserves own __proto__ server entries while keeping deterministic order", () => { + const servers = JSON.parse( + '{"zebra":{"command":"z"},"__proto__":{"command":"proto"}}', + ) as Record; + + const { staticServers, requesterScopedServerNames } = + partitionMcpServersByConnectionScope(servers); + + expect(Object.keys(staticServers)).toEqual( + Object.keys(servers).toSorted((a, b) => a.localeCompare(b)), + ); + expect(staticServers).toStrictEqual(servers); + expect(Object.hasOwn(staticServers, "__proto__")).toBe(true); + expect(requesterScopedServerNames).toEqual([]); + }); + it("keeps connection resolvers from pinned live registries", async () => { const pinnedRegistry = createEmptyPluginRegistry(); pinnedRegistry.mcpServerConnectionResolvers.push({ diff --git a/src/agents/mcp-connection-resolver.ts b/src/agents/mcp-connection-resolver.ts index beb2d513ef45..3fdee5e7f434 100644 --- a/src/agents/mcp-connection-resolver.ts +++ b/src/agents/mcp-connection-resolver.ts @@ -159,7 +159,7 @@ export function partitionMcpServersByConnectionScope(mcpServers: Record = {}; + const staticServerEntries: Array<[string, T]> = []; const requesterScopedServerNames: string[] = []; for (const [serverName, rawServer] of Object.entries(mcpServers).toSorted(([a], [b]) => a.localeCompare(b), @@ -168,8 +168,11 @@ export function partitionMcpServersByConnectionScope(mcpServers: Record { fs.rmSync(tempOauthDir, { recursive: true, force: true }); }); + it("drops reserved MCP server names without touching sibling servers", () => { + const raw = JSON.parse( + '{"mcp":{"servers":{"__proto__":{"command":"bad"},"docs":{"command":"docs"}}},"nodeHost":{"mcp":{"servers":{"__proto__":{"command":"bad-node"},"local":{"command":"local"}}}}}', + ) as OpenClawConfig; + + const normalized = { + mcp: { servers: { docs: { command: "docs" } } }, + nodeHost: { mcp: { servers: { local: { command: "local" } } } }, + } as OpenClawConfig; + const migrated = normalizeCompatibilityConfigValues(normalized, { sourceRaw: raw }); + + expect(migrated.config.mcp?.servers).toStrictEqual({ docs: { command: "docs" } }); + expect(migrated.config.nodeHost?.mcp?.servers).toStrictEqual({ + local: { command: "local" }, + }); + expect(Object.hasOwn(migrated.config.mcp?.servers ?? {}, "__proto__")).toBe(false); + expect(Object.hasOwn(migrated.config.nodeHost?.mcp?.servers ?? {}, "__proto__")).toBe(false); + expect(migrated.changes).toStrictEqual([ + 'Dropped MCP server "__proto__" from mcp.servers because the name is reserved; re-add it under a different name.', + 'Dropped MCP server "__proto__" from nodeHost.mcp.servers because the name is reserved; re-add it under a different name.', + ]); + + const secondPass = normalizeCompatibilityConfigValues(migrated.config); + expect(secondPass.config).toStrictEqual(migrated.config); + expect(secondPass.changes).toStrictEqual([]); + + const candidateOnly = normalizeCompatibilityConfigValues(raw, { sourceRaw: {} }); + expect(Object.hasOwn(candidateOnly.config.mcp?.servers ?? {}, "__proto__")).toBe(false); + expect(Object.hasOwn(candidateOnly.config.nodeHost?.mcp?.servers ?? {}, "__proto__")).toBe( + false, + ); + expect(candidateOnly.changes).toStrictEqual(migrated.changes); + }); + it("does not materialize a group visible reply default for configured channels", () => { const res = normalizeCompatibilityConfigValues({ channels: { diff --git a/src/commands/doctor/shared/legacy-config-core-migrate.ts b/src/commands/doctor/shared/legacy-config-core-migrate.ts index b21f9c24e37b..ee242a295452 100644 --- a/src/commands/doctor/shared/legacy-config-core-migrate.ts +++ b/src/commands/doctor/shared/legacy-config-core-migrate.ts @@ -8,6 +8,7 @@ import { pruneBindingsForMissingAgents } from "./legacy-config-binding-repair.js import { normalizeBaseCompatibilityConfigValues } from "./legacy-config-compatibility-base.js"; import { normalizeLegacyOpenAICodexModelsAddMetadata } from "./legacy-config-core-normalizers.js"; import { stripRetiredTuningKnobs } from "./legacy-config-migrations.runtime.retired-media.js"; +import { migrateReservedMcpServerNames } from "./reserved-mcp-server-name-migrate.js"; function repairNullAgentWorkspaces(cfg: OpenClawConfig, changes: string[]): OpenClawConfig { const agents = cfg.agents?.list; @@ -52,14 +53,17 @@ export function normalizeCompatibilityConfigValues( cfg: OpenClawConfig, options: { blockedModelIdentities?: ReadonlySet; + sourceRaw?: unknown; } = {}, ): { config: OpenClawConfig; changes: string[]; } { const changes: string[] = []; + const reservedMcpServerNames = migrateReservedMcpServerNames(cfg, options.sourceRaw); + changes.push(...reservedMcpServerNames.changes); let next = normalizeBaseCompatibilityConfigValues( - cfg, + reservedMcpServerNames.config, changes, (config) => { const setupMigration = runPluginSetupConfigMigrations({ diff --git a/src/commands/doctor/shared/reserved-mcp-server-name-migrate.ts b/src/commands/doctor/shared/reserved-mcp-server-name-migrate.ts new file mode 100644 index 000000000000..b5acf219fcd1 --- /dev/null +++ b/src/commands/doctor/shared/reserved-mcp-server-name-migrate.ts @@ -0,0 +1,51 @@ +// Removes MCP server entries whose names are reserved by config validation. +import type { OpenClawConfig } from "../../../config/types.openclaw.js"; +import { isRecord, type JsonRecord } from "./legacy-config-record-shared.js"; + +const RESERVED_MCP_SERVER_NAME = "__proto__"; + +function resolveMcpServers(raw: unknown, nodeHost: boolean): JsonRecord | undefined { + if (!isRecord(raw)) { + return undefined; + } + const owner = nodeHost ? raw.nodeHost : raw; + if (!isRecord(owner)) { + return undefined; + } + const mcp = isRecord(owner.mcp) ? owner.mcp : undefined; + return isRecord(mcp?.servers) ? mcp.servers : undefined; +} + +/** Drop reserved MCP server names before canonical config validation runs. */ +export function migrateReservedMcpServerNames( + cfg: OpenClawConfig, + sourceRaw: unknown = cfg, +): { + config: OpenClawConfig; + changes: string[]; +} { + const locations = [ + { path: "mcp.servers", nodeHost: false }, + { path: "nodeHost.mcp.servers", nodeHost: true }, + ].filter(({ nodeHost }) => + [sourceRaw, cfg].some((value) => + Object.hasOwn(resolveMcpServers(value, nodeHost) ?? {}, RESERVED_MCP_SERVER_NAME), + ), + ); + if (locations.length === 0) { + return { config: cfg, changes: [] }; + } + + const next = structuredClone(cfg); + const changes: string[] = []; + for (const { path, nodeHost } of locations) { + const servers = resolveMcpServers(next, nodeHost); + if (servers) { + delete servers[RESERVED_MCP_SERVER_NAME]; + } + changes.push( + `Dropped MCP server "${RESERVED_MCP_SERVER_NAME}" from ${path} because the name is reserved; re-add it under a different name.`, + ); + } + return { config: next, changes }; +} diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 50f6b2d23556..dca8681029f6 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -5,6 +5,7 @@ import { beforeAll, describe, expect, it } from "vitest"; import { buildConfigSchema, lookupConfigSchema } from "./schema.js"; import { applyDerivedTags } from "./schema.tags.js"; import { applyResolvedConfigTierHints } from "./schema.tiers.js"; +import { validateConfigObjectRaw } from "./validation.js"; import { ToolsSchema } from "./zod-schema.agent-runtime.js"; import { OpenClawSchema } from "./zod-schema.js"; @@ -258,6 +259,55 @@ describe("config schema", () => { } }); + it("rejects the reserved __proto__ MCP server name without tightening other names", () => { + for (const raw of [ + '{"mcp":{"servers":{"__proto__":{"command":"server"}}}}', + '{"nodeHost":{"mcp":{"servers":{"__proto__":{"command":"server"}}}}}', + ]) { + const result = OpenClawSchema.safeParse(JSON.parse(raw)); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues).toContainEqual( + expect.objectContaining({ + message: 'MCP server name "__proto__" is reserved; rename the server', + }), + ); + } + } + + for (const serverName of ["docs", "_internal"]) { + expect( + OpenClawSchema.safeParse({ + mcp: { servers: { [serverName]: { command: "server" } } }, + nodeHost: { mcp: { servers: { [serverName]: { command: "server" } } } }, + }).success, + ).toBe(true); + } + }); + + it("rejects reserved MCP server names from the pre-normalization config", () => { + const sourceRaw = JSON.parse('{"mcp":{"servers":{"__proto__":{"command":"server"}}}}'); + const result = validateConfigObjectRaw({ mcp: { servers: {} } }, { sourceRaw }); + + expect(result).toEqual({ + ok: false, + issues: [ + expect.objectContaining({ + path: "mcp.servers.__proto__", + message: 'MCP server name "__proto__" is reserved; rename the server', + }), + ], + }); + + const directResult = validateConfigObjectRaw(sourceRaw); + expect(directResult.ok).toBe(false); + if (!directResult.ok) { + expect( + directResult.issues.filter((issue) => issue.path === "mcp.servers.__proto__"), + ).toHaveLength(1); + } + }); + it("rejects empty Codex MCP agent scopes", () => { expect(() => OpenClawSchema.parse({ diff --git a/src/config/validation-core.ts b/src/config/validation-core.ts index c469aed59759..39192212503d 100644 --- a/src/config/validation-core.ts +++ b/src/config/validation-core.ts @@ -39,6 +39,7 @@ import { } from "./validation-issues.js"; import { isBuiltInModelProviderOverlayId } from "./zod-schema.core.js"; import { OpenClawSchema } from "./zod-schema.js"; +import { McpServerNameSchema, NodeHostMcpServerNameSchema } from "./zod-schema.root-support.js"; function materializeBundledModelProviderOverlays(config: OpenClawConfig): OpenClawConfig { const providers = config.models?.providers; @@ -79,6 +80,46 @@ function stripPreservedLegacyRootKeysForValidation( return next; } +function collectMcpServerNameIssues(raw: unknown): ConfigValidationIssue[] { + if (!isRecord(raw)) { + return []; + } + const mcp = isRecord(raw.mcp) ? raw.mcp : undefined; + const nodeHost = isRecord(raw.nodeHost) ? raw.nodeHost : undefined; + const nodeHostMcp = isRecord(nodeHost?.mcp) ? nodeHost.mcp : undefined; + const locations = [ + { + path: ["mcp", "servers"] as const, + servers: isRecord(mcp?.servers) ? mcp.servers : undefined, + schema: McpServerNameSchema, + }, + { + path: ["nodeHost", "mcp", "servers"] as const, + servers: isRecord(nodeHostMcp?.servers) ? nodeHostMcp.servers : undefined, + schema: NodeHostMcpServerNameSchema, + }, + ]; + const issues: ConfigValidationIssue[] = []; + for (const location of locations) { + for (const serverName of Object.keys(location.servers ?? {})) { + const result = location.schema.safeParse(serverName); + if (result.success) { + continue; + } + const pathSegments = [...location.path, serverName]; + for (const issue of result.error.issues) { + issues.push( + withConfigIssuePath( + { path: pathSegments.join("."), message: issue.message }, + pathSegments, + ), + ); + } + } + } + return issues; +} + function isWorkspaceAvatarPath(value: string, workspaceDir: string): boolean { const workspaceRoot = path.resolve(workspaceDir); const resolved = path.resolve(workspaceRoot, value); @@ -261,10 +302,22 @@ export function validateConfigObjectRaw( raw, opts?.preservedLegacyRootKeys, ); + // Generic config transforms can rebuild records before schema validation, so + // validate authored MCP names from the parsed source when it is available. + const normalizedMcpServerNameIssueKeys = new Set( + collectMcpServerNameIssues(normalizedRaw).map((issue) => + JSON.stringify([issue.path, issue.message]), + ), + ); + const mcpServerNameIssues = collectMcpServerNameIssues(opts?.sourceRaw).filter( + (issue) => !normalizedMcpServerNameIssueKeys.has(JSON.stringify([issue.path, issue.message])), + ); const policyIssues = collectUnsupportedSecretRefPolicyIssues(normalizedRaw); const validated = OpenClawSchema.safeParse(normalizedRaw); - if (!validated.success) { - const schemaIssues = validated.error.issues.map(mapZodIssueToConfigIssue); + if (!validated.success || mcpServerNameIssues.length > 0) { + const schemaIssues = validated.success + ? mcpServerNameIssues + : [...mcpServerNameIssues, ...validated.error.issues.map(mapZodIssueToConfigIssue)]; return { ok: false, issues: mergeUnsupportedMutableSecretRefIssues(policyIssues, schemaIssues), diff --git a/src/config/zod-schema.root-support.ts b/src/config/zod-schema.root-support.ts index 8400a6b1afa5..f8c59d84077f 100644 --- a/src/config/zod-schema.root-support.ts +++ b/src/config/zod-schema.root-support.ts @@ -390,9 +390,45 @@ const McpServerSchema = z }) .catchall(z.unknown()); +const RESERVED_MCP_SERVER_NAME = "__proto__"; +const RESERVED_MCP_SERVER_NAME_ERROR = 'MCP server name "__proto__" is reserved; rename the server'; + +export const McpServerNameSchema = z + .string() + .refine((value) => value !== RESERVED_MCP_SERVER_NAME, RESERVED_MCP_SERVER_NAME_ERROR); + +export const NodeHostMcpServerNameSchema = McpServerNameSchema.refine( + (value) => value.length > 0 && value === value.trim(), + "MCP server name must be non-empty and must not have surrounding whitespace", +); + +function createMcpServersSchema(serverNameSchema: z.ZodType) { + return z.preprocess( + (value, ctx) => { + // Plain assignment treats "__proto__" as a setter, so one unhardened map builder + // can silently drop the server. Reject the name at the config boundary instead. + if ( + value !== null && + typeof value === "object" && + !Array.isArray(value) && + Object.hasOwn(value, RESERVED_MCP_SERVER_NAME) + ) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [RESERVED_MCP_SERVER_NAME], + message: RESERVED_MCP_SERVER_NAME_ERROR, + }); + return z.NEVER; + } + return value; + }, + z.record(serverNameSchema, McpServerSchema), + ); +} + export const McpConfigSchema = z .strictObject({ - servers: z.record(z.string(), McpServerSchema).optional(), + servers: createMcpServersSchema(McpServerNameSchema).optional(), apps: z .strictObject({ enabled: z.boolean().optional(), @@ -419,13 +455,6 @@ export const McpConfigSchema = z }) .optional(); -const NodeHostMcpServerNameSchema = z - .string() - .refine( - (value) => value.length > 0 && value === value.trim(), - "MCP server name must be non-empty and must not have surrounding whitespace", - ); - export const NodeHostSchema = z .strictObject({ agentRuns: NodeHostAgentRunsSchema, @@ -437,7 +466,7 @@ export const NodeHostSchema = z .optional(), mcp: z .strictObject({ - servers: z.record(NodeHostMcpServerNameSchema, McpServerSchema).optional(), + servers: createMcpServersSchema(NodeHostMcpServerNameSchema).optional(), }) .optional(), skills: z