Harden plugin MCP tool registration

.changeset/plugin-mcp-tools.md

@@ -0,0 +1,5 @@
+---
+"emdash": patch
+---
+
+Fixes plugin MCP registration for scoped IDs and malformed serialized schemas so valid tools remain available instead of emitting invalid names or disabling the MCP endpoint.

docs/src/content/docs/plugins/creating-plugins/api-routes.mdx

@@ -155,7 +155,7 @@ export default {
 } satisfies SandboxedPlugin;
 ```
 
-EmDash exposes this as `<pluginId>__createEvent`. The referenced route must be private and declare `permission`. Input schemas are required; output schemas are optional. Set `destructive: true` for tools that delete, overwrite, publish, charge, or otherwise perform a difficult-to-reverse action.
+EmDash exposes this as `<pluginId>__createEvent`. Scoped IDs are normalized to MCP-safe segments, so `@emdash-cms/plugin-forms` with a `submit_form` tool becomes `emdash_cms__plugin_forms__submit_form`. The referenced route must be private and declare `permission`. Input schemas are required; output schemas are optional. Set `destructive: true` for tools that delete, overwrite, publish, charge, or otherwise perform a difficult-to-reverse action.
 
 An administrator must separately enable a plugin's MCP tools after reviewing their names, descriptions, routes, permissions, and destructive flags. Calling the tool then requires both the route permission and either the `mcp:tools` token scope or `mcp:tools:<pluginId>`.
 

docs/src/content/docs/reference/mcp-server.mdx

@@ -53,7 +53,7 @@ The `admin` scope grants access to core operations, but it does not grant plugin
 
 In addition to scopes, some tools require a minimum RBAC role. Both must be satisfied -- a token with the right scope still fails if the calling user's role is too low.
 
-Plugin tools use the permission declared by their underlying route. They are absent from `tools/list` until an administrator enables that plugin's MCP surface. Tool names use the deterministic `<pluginId>__<localName>` form, and invocations are recorded in the audit log with plugin, tool, route, and actor provenance.
+Plugin tools use the permission declared by their underlying route. They are absent from `tools/list` until an administrator enables that plugin's MCP surface. Tool names use deterministic MCP-safe plugin ID segments followed by `__<localName>` (for example, `@emdash-cms/plugin-forms` becomes `emdash_cms__plugin_forms__submit_form`), and invocations are recorded in the audit log with plugin, tool, route, and actor provenance.
 
 | Operation | Minimum role |
 | --- | --- |

packages/core/src/emdash-runtime.ts

@@ -38,6 +38,7 @@ import type {
 import { validateIdentifier } from "./database/validate.js";
 import { getI18nConfig } from "./i18n/config.js";
 import { repairLocaleCasing } from "./i18n/repair-locale-casing.js";
+import { safeJsonSchemaToZod } from "./mcp/json-schema.js";
 import { normalizeMediaValue } from "./media/normalize.js";
 import type { MediaProvider, MediaProviderCapabilities } from "./media/types.js";
 import {
@@ -3518,15 +3519,23 @@ export class EmDashRuntime {
 					continue;
 				}
 				seen.add(key);
+				const warnInvalidSchema = (schemaKind: "input" | "output") => (error: unknown) => {
+					console.warn(
+						`[emdash] Falling back after invalid ${schemaKind} schema for plugin MCP tool ${id}/${tool.name}:`,
+						error,
+					);
+				};
 				tools.push({
 					pluginId: id,
 					name: tool.name,
 					description: tool.description,
 					route: tool.route,
 					permission: tool.permission,
 					destructive: tool.destructive,
-					inputSchema: z.fromJSONSchema({ ...tool.inputSchema }),
-					outputSchema: tool.outputSchema ? z.fromJSONSchema({ ...tool.outputSchema }) : undefined,
+					inputSchema: safeJsonSchemaToZod({ ...tool.inputSchema }, warnInvalidSchema("input")),
+					outputSchema: tool.outputSchema
+						? safeJsonSchemaToZod({ ...tool.outputSchema }, warnInvalidSchema("output"))
+						: undefined,
 				});
 			}
 		};

packages/core/src/mcp/json-schema.ts

@@ -0,0 +1,15 @@
+import { z } from "zod";
+
+const fallbackSchema = z.record(z.string(), z.unknown());
+
+export function safeJsonSchemaToZod(
+	schema: Record<string, unknown>,
+	onError?: (error: unknown) => void,
+): z.ZodType {
+	try {
+		return z.fromJSONSchema(schema);
+	} catch (error) {
+		onError?.(error);
+		return fallbackSchema;
+	}
+}

packages/core/src/mcp/plugin-tool-name.ts

@@ -0,0 +1,39 @@
+const FORWARD_SLASH_PATTERN = "/";
+const MCP_SAFE_PLUGIN_ID_PATTERN = /^[A-Za-z0-9_.-]+$/;
+const PLUGIN_SCOPE_PREFIX_PATTERN = /^@/;
+const PLUGIN_ID_DASH_PATTERN = /-/g;
+const PLUGIN_ID_UNDERSCORE_PATTERN = /_/;
+const AMBIGUOUS_PLUGIN_SEGMENT_PATTERN = /__/;
+const AMBIGUOUS_JOINED_PLUGIN_ID_PATTERN = /_{3,}/;
+
+export function pluginToolName(pluginId: string, toolName: string): string {
+	if (
+		MCP_SAFE_PLUGIN_ID_PATTERN.test(pluginId) &&
+		!AMBIGUOUS_PLUGIN_SEGMENT_PATTERN.test(pluginId)
+	) {
+		return `${pluginId}__${toolName}`;
+	}
+
+	const readableSegments = pluginId
+		.replace(PLUGIN_SCOPE_PREFIX_PATTERN, "")
+		.split(FORWARD_SLASH_PATTERN)
+		.map((segment) => segment.replace(PLUGIN_ID_DASH_PATTERN, "_"));
+	const readablePluginId = readableSegments.join("__");
+	if (
+		PLUGIN_ID_UNDERSCORE_PATTERN.test(pluginId) ||
+		AMBIGUOUS_JOINED_PLUGIN_ID_PATTERN.test(readablePluginId) ||
+		readableSegments.some((segment) => AMBIGUOUS_PLUGIN_SEGMENT_PATTERN.test(segment))
+	) {
+		return `${readablePluginId}__${stableToolNameHash(pluginId)}__${toolName}`;
+	}
+	return `${readablePluginId}__${toolName}`;
+}
+
+function stableToolNameHash(value: string): string {
+	let hash = 0x811c9dc5;
+	for (let index = 0; index < value.length; index++) {
+		hash ^= value.charCodeAt(index);
+		hash = Math.imul(hash, 0x01000193);
+	}
+	return (hash >>> 0).toString(16).padStart(8, "0");
+}

packages/core/src/mcp/server.ts

@@ -27,6 +27,7 @@ import type { EmDashHandlers } from "../astro/types.js";
 import { hasScope } from "../auth/api-tokens.js";
 import { convertDataForRead, convertDataForWrite } from "../client/portable-text.js";
 import type { FieldSchema } from "../client/portable-text.js";
+import { pluginToolName } from "./plugin-tool-name.js";
 
 const COLLECTION_SLUG_PATTERN = /^[a-z][a-z0-9_]*$/;
 /** http(s) scheme matcher used by `settings_update` URL validation. */
@@ -533,7 +534,7 @@ export function createMcpServer(
 	for (const tool of pluginTools) {
 		if (!(tool.permission in Permissions)) continue;
 		server.registerTool(
-			`${tool.pluginId}__${tool.name}`,
+			pluginToolName(tool.pluginId, tool.name),
 			{
 				description: tool.description,
 				inputSchema: tool.inputSchema,

packages/core/tests/unit/mcp/authorization.test.ts

@@ -278,6 +278,20 @@ describe("MCP Authorization", () => {
 			outputSchema: z.object({ id: z.string() }),
 		};
 
+		it("normalizes scoped plugin IDs into valid, stable tool names", async () => {
+			({ client, cleanup } = await setupMcpPair({
+				userId: ADMIN_USER_ID,
+				userRole: Role.ADMIN,
+				pluginTools: [{ ...pluginTool, pluginId: "@emdash-cms/calendar" }],
+			}));
+
+			const { tools } = await client.listTools();
+			const toolNames = tools.map((tool) => tool.name);
+
+			expect(toolNames).toContain("emdash_cms__calendar__createEvent");
+			expect(toolNames).not.toContain("@emdash-cms/calendar__createEvent");
+		});
+
 		it("does not let the legacy admin scope bypass plugin-tool scope", async () => {
 			const handlers = createMockHandlers();
 			handlers.handlePluginMcpDenied = vi.fn().mockResolvedValue(undefined);

packages/core/tests/unit/mcp/json-schema.test.ts

@@ -0,0 +1,13 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { safeJsonSchemaToZod } from "../../../src/mcp/json-schema.js";
+
+describe("safeJsonSchemaToZod", () => {
+	it("falls back instead of throwing for a malformed serialized schema", () => {
+		const onError = vi.fn();
+		const fallback = safeJsonSchemaToZod({ type: "definitely-not-json-schema" }, onError);
+
+		expect(() => fallback.parse({ unexpected: true })).not.toThrow();
+		expect(onError).toHaveBeenCalledOnce();
+	});
+});

packages/core/tests/unit/mcp/plugin-tool-name.test.ts

@@ -0,0 +1,35 @@
+import { describe, expect, it } from "vitest";
+
+import { pluginToolName } from "../../../src/mcp/plugin-tool-name.js";
+
+const HASHED_TOOL_NAME_PATTERN = /^[a-z0-9_]+__[0-9a-f]{8}__[a-z][a-z0-9_]*$/;
+
+describe("pluginToolName", () => {
+	it("uses readable names for unambiguous scoped plugin IDs", () => {
+		expect(pluginToolName("@emdash-cms/plugin-forms", "submit_form")).toBe(
+			"emdash_cms__plugin_forms__submit_form",
+		);
+	});
+
+	it("preserves existing names for already MCP-safe plugin IDs", () => {
+		expect(pluginToolName("calendar-plugin", "createEvent")).toBe("calendar-plugin__createEvent");
+		expect(pluginToolName("foo--bar", "summarize")).toBe("foo--bar__summarize");
+	});
+
+	it("hashes safe-looking IDs with ambiguous separators", () => {
+		expect(pluginToolName("foo__bar", "summarize")).toMatch(HASHED_TOOL_NAME_PATTERN);
+	});
+
+	it("keeps scoped IDs distinct when dashes sit next to the slash boundary", () => {
+		const trailingDashScope = pluginToolName("@a-/b", "summarize");
+		const leadingDashName = pluginToolName("@a/-b", "summarize");
+
+		expect(trailingDashScope).toMatch(HASHED_TOOL_NAME_PATTERN);
+		expect(leadingDashName).toMatch(HASHED_TOOL_NAME_PATTERN);
+		expect(trailingDashScope).not.toBe(leadingDashName);
+	});
+
+	it("keeps normalized dashes distinct from literal underscores", () => {
+		expect(pluginToolName("foo-bar", "summarize")).not.toBe(pluginToolName("foo_bar", "summarize"));
+	});
+});