mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(config): reject reserved __proto__ MCP server name (#116112)
This commit is contained in:
committed by
GitHub
parent
42e21ad13b
commit
23e151a016
+1
-1
@@ -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
|
||||
|
||||
|
||||
@@ -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<string, { command: string }>;
|
||||
|
||||
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({
|
||||
|
||||
@@ -159,7 +159,7 @@ export function partitionMcpServersByConnectionScope<T>(mcpServers: Record<strin
|
||||
requesterScopedServerNames: string[];
|
||||
} {
|
||||
const resolvers = listMcpServerConnectionResolversByServerName();
|
||||
const staticServers: Record<string, T> = {};
|
||||
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<T>(mcpServers: Record<strin
|
||||
requesterScopedServerNames.push(serverName);
|
||||
continue;
|
||||
}
|
||||
staticServers[serverName] = rawServer;
|
||||
staticServerEntries.push([serverName, rawServer]);
|
||||
}
|
||||
// Data-property construction preserves every own key from unvalidated inputs,
|
||||
// including "__proto__", without invoking Object.prototype's legacy setter.
|
||||
const staticServers = Object.fromEntries(staticServerEntries);
|
||||
return { staticServers, requesterScopedServerNames };
|
||||
}
|
||||
|
||||
|
||||
@@ -302,8 +302,11 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
note(sanitizeDoctorNote(unsupportedInternalHookEntryWarnings.join("\n")), "Doctor warnings");
|
||||
}
|
||||
|
||||
// Parsed config supplies invalid-key evidence only; migrations still mutate the
|
||||
// include/env-resolved candidate so doctor never writes unresolved source values.
|
||||
const normalized = normalizeCompatibilityConfigValues(state.candidate, {
|
||||
blockedModelIdentities: blockedCodexModelIdentities,
|
||||
sourceRaw: snapshot.parsed,
|
||||
});
|
||||
applyConfigMutation(normalized, {
|
||||
fixHint: `Run "${doctorFixCommand}" to apply these changes.`,
|
||||
|
||||
@@ -179,6 +179,40 @@ describe("normalizeCompatibilityConfigValues", () => {
|
||||
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: {
|
||||
|
||||
@@ -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<LegacyCodexModelIdentity>;
|
||||
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({
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
@@ -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({
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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<string>) {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user