diff --git a/.changeset/plugin-mcp-tools.md b/.changeset/plugin-mcp-tools.md new file mode 100644 index 0000000000..3009a14135 --- /dev/null +++ b/.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. diff --git a/docs/src/content/docs/plugins/creating-plugins/api-routes.mdx b/docs/src/content/docs/plugins/creating-plugins/api-routes.mdx index 52b00de50f..ffee0ad555 100644 --- a/docs/src/content/docs/plugins/creating-plugins/api-routes.mdx +++ b/docs/src/content/docs/plugins/creating-plugins/api-routes.mdx @@ -155,7 +155,7 @@ export default { } satisfies SandboxedPlugin; ``` -EmDash exposes this as `__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 `__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:`. diff --git a/docs/src/content/docs/reference/mcp-server.mdx b/docs/src/content/docs/reference/mcp-server.mdx index 68b8eb45c2..c8f05fd595 100644 --- a/docs/src/content/docs/reference/mcp-server.mdx +++ b/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 `__` 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 `__` (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 | | --- | --- | diff --git a/packages/core/src/emdash-runtime.ts b/packages/core/src/emdash-runtime.ts index 5e4a247255..2191b83ded 100644 --- a/packages/core/src/emdash-runtime.ts +++ b/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,6 +3519,12 @@ 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, @@ -3525,8 +3532,10 @@ export class EmDashRuntime { 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, }); } }; diff --git a/packages/core/src/mcp/json-schema.ts b/packages/core/src/mcp/json-schema.ts new file mode 100644 index 0000000000..2bfa1054cc --- /dev/null +++ b/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, + onError?: (error: unknown) => void, +): z.ZodType { + try { + return z.fromJSONSchema(schema); + } catch (error) { + onError?.(error); + return fallbackSchema; + } +} diff --git a/packages/core/src/mcp/plugin-tool-name.ts b/packages/core/src/mcp/plugin-tool-name.ts new file mode 100644 index 0000000000..2a9a9006d7 --- /dev/null +++ b/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"); +} diff --git a/packages/core/src/mcp/server.ts b/packages/core/src/mcp/server.ts index e25338de24..8ca56585af 100644 --- a/packages/core/src/mcp/server.ts +++ b/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, diff --git a/packages/core/tests/unit/mcp/authorization.test.ts b/packages/core/tests/unit/mcp/authorization.test.ts index 80aa519106..da0c214471 100644 --- a/packages/core/tests/unit/mcp/authorization.test.ts +++ b/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); diff --git a/packages/core/tests/unit/mcp/json-schema.test.ts b/packages/core/tests/unit/mcp/json-schema.test.ts new file mode 100644 index 0000000000..d79583b8d0 --- /dev/null +++ b/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(); + }); +}); diff --git a/packages/core/tests/unit/mcp/plugin-tool-name.test.ts b/packages/core/tests/unit/mcp/plugin-tool-name.test.ts new file mode 100644 index 0000000000..51f2f5ba26 --- /dev/null +++ b/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")); + }); +});