diff --git a/README.md b/README.md index 5538342f4..aa7db09f9 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,13 @@ bun add -g @open-pencil/mcp openpencil-mcp-http # http://localhost:3100/mcp ``` +Security defaults for HTTP transport: +- Binds to `127.0.0.1` by default (`HOST` to override) +- `eval` tool is disabled +- File access is restricted to `OPENPENCIL_MCP_ROOT` (defaults to current working directory) +- Optional auth: set `OPENPENCIL_MCP_AUTH_TOKEN` and send `Authorization: Bearer ` (or `x-mcp-token`) +- CORS is disabled by default; set `OPENPENCIL_MCP_CORS_ORIGIN` to allow a specific origin + 75 tools: create shapes, set fills/strokes/layout, variables, vectors, boolean ops, viewport, find nodes, open/save `.fig` files, render JSX to design nodes. ## Scripts diff --git a/packages/docs/reference/mcp-tools.md b/packages/docs/reference/mcp-tools.md index 4331034dc..40664ebd6 100644 --- a/packages/docs/reference/mcp-tools.md +++ b/packages/docs/reference/mcp-tools.md @@ -59,7 +59,15 @@ openpencil-mcp-http Or from source: `bun packages/mcp/src/http.ts` / `npx tsx packages/mcp/src/http.ts` -Starts on port 3100 (override with `PORT` env var). Endpoints: +Security defaults (HTTP transport): + +- Binds to `127.0.0.1` by default (`HOST` to override) +- `eval` tool is disabled +- File operations are limited to `OPENPENCIL_MCP_ROOT` (defaults to current working directory) +- CORS is disabled by default; set `OPENPENCIL_MCP_CORS_ORIGIN` to allow one origin +- Optional auth token: `OPENPENCIL_MCP_AUTH_TOKEN` (client sends `Authorization: Bearer ` or `x-mcp-token`) + +Server starts on port 3100 (override with `PORT` env var). Endpoints: - `GET /health` — server status - `POST /mcp` — MCP Streamable HTTP (SSE). Sessions via `mcp-session-id` header. @@ -138,3 +146,5 @@ Starts on port 3100 (override with `PORT` env var). Endpoints: | Tool | Description | |------|-------------| | `eval` | Execute JavaScript with full Figma Plugin API access | + +Note: `eval` is available over stdio, but disabled in HTTP mode for security. diff --git a/packages/mcp/src/http.ts b/packages/mcp/src/http.ts index f5951703f..d6effa3ff 100644 --- a/packages/mcp/src/http.ts +++ b/packages/mcp/src/http.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import { randomUUID } from 'node:crypto' import { readFile } from 'node:fs/promises' +import { resolve } from 'node:path' import { Hono } from 'hono' import { cors } from 'hono/cors' @@ -9,6 +10,11 @@ import { WebStandardStreamableHTTPServerTransport } from '@modelcontextprotocol/ import { createServer } from './server.js' const pkg = JSON.parse(await readFile(new URL('../package.json', import.meta.url), 'utf-8')) +const port = parseInt(process.env.PORT ?? '3100', 10) +const host = process.env.HOST ?? '127.0.0.1' +const authToken = process.env.OPENPENCIL_MCP_AUTH_TOKEN?.trim() || null +const corsOrigin = process.env.OPENPENCIL_MCP_CORS_ORIGIN?.trim() || null +const fileRoot = resolve(process.env.OPENPENCIL_MCP_ROOT ?? process.cwd()) const sessions = new Map; transport: WebStandardStreamableHTTPServerTransport }>() @@ -18,7 +24,10 @@ async function getOrCreateSession(sessionId?: string) { } const id = sessionId ?? randomUUID() - const server = createServer(pkg.version) + const server = createServer(pkg.version, { + enableEval: false, + fileRoot + }) const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: () => id }) await server.connect(transport) sessions.set(id, { server, transport }) @@ -27,32 +36,62 @@ async function getOrCreateSession(sessionId?: string) { const app = new Hono() -app.use('*', cors({ - origin: '*', - allowMethods: ['GET', 'POST', 'DELETE', 'OPTIONS'], - allowHeaders: ['Content-Type', 'mcp-session-id', 'Last-Event-ID', 'mcp-protocol-version'], - exposeHeaders: ['mcp-session-id', 'mcp-protocol-version'] -})) +if (corsOrigin) { + app.use('*', cors({ + origin: corsOrigin, + allowMethods: ['GET', 'POST', 'DELETE', 'OPTIONS'], + allowHeaders: [ + 'Content-Type', + 'Authorization', + 'x-mcp-token', + 'mcp-session-id', + 'Last-Event-ID', + 'mcp-protocol-version' + ], + exposeHeaders: ['mcp-session-id', 'mcp-protocol-version'] + })) +} -app.get('/health', (c) => c.json({ status: 'ok', version: pkg.version, tools: 29 })) +app.get('/health', (c) => + c.json({ + status: 'ok', + version: pkg.version, + authRequired: Boolean(authToken), + evalEnabled: false, + fileRoot + }) +) app.all('/mcp', async (c) => { + if (authToken) { + const authHeader = c.req.header('authorization') + const token = + authHeader?.startsWith('Bearer ') + ? authHeader.slice('Bearer '.length) + : c.req.header('x-mcp-token') + if (token !== authToken) { + return c.json({ error: 'Unauthorized' }, 401) + } + } + const sessionId = c.req.header('mcp-session-id') ?? undefined const { transport } = await getOrCreateSession(sessionId) return transport.handleRequest(c.req.raw) }) -const port = parseInt(process.env.PORT ?? '3100', 10) - const isBun = typeof globalThis.Bun !== 'undefined' if (isBun) { - Bun.serve({ fetch: app.fetch, port }) + Bun.serve({ fetch: app.fetch, port, hostname: host }) } else { const { serve } = await import('@hono/node-server') - serve({ fetch: app.fetch, port }) + serve({ fetch: app.fetch, port, hostname: host }) } console.log(`OpenPencil MCP server v${pkg.version}`) -console.log(` Health: http://localhost:${port}/health`) -console.log(` MCP: http://localhost:${port}/mcp`) +console.log(` Health: http://${host}:${port}/health`) +console.log(` MCP: http://${host}:${port}/mcp`) +console.log(` Auth: ${authToken ? 'required (OPENPENCIL_MCP_AUTH_TOKEN)' : 'disabled'}`) +console.log(` CORS: ${corsOrigin ?? 'disabled'}`) +console.log(` Eval: disabled`) +console.log(` Root: ${fileRoot}`) diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 7533b963b..933f95a7f 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -1,4 +1,5 @@ import { readFile, writeFile } from 'node:fs/promises' +import { isAbsolute, relative, resolve } from 'node:path' import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { z } from 'zod' @@ -8,6 +9,10 @@ import { ALL_TOOLS, FigmaAPI, parseFigFile, computeAllLayouts, SceneGraph } from import type { ToolDef, ParamDef, ParamType } from '@open-pencil/core' type McpResult = { content: { type: 'text'; text: string }[]; isError?: boolean } +export interface CreateServerOptions { + enableEval?: boolean + fileRoot?: string | null +} function ok(data: unknown): McpResult { return { content: [{ type: 'text', text: JSON.stringify(data, null, 2) }] } @@ -39,8 +44,12 @@ function paramToZod(param: ParamDef): z.ZodTypeAny { return param.required ? schema : schema.optional() } -export function createServer(version: string): McpServer { +export function createServer(version: string, options: CreateServerOptions = {}): McpServer { const server = new McpServer({ name: 'open-pencil', version }) + const enableEval = options.enableEval ?? true + const fileRoot = options.fileRoot === null || options.fileRoot === undefined + ? null + : resolve(options.fileRoot) let graph: SceneGraph | null = null let currentPageId: string | null = null @@ -52,6 +61,16 @@ export function createServer(version: string): McpServer { return api } + function resolveAndCheckPath(filePath: string): string { + const resolved = resolve(filePath) + if (!fileRoot) return resolved + const rel = relative(fileRoot, resolved) + if (rel === '' || (!rel.startsWith('..') && !isAbsolute(rel))) { + return resolved + } + throw new Error(`Path "${filePath}" is outside allowed root "${fileRoot}"`) + } + function registerTool(def: ToolDef) { const shape: Record = {} for (const [key, param] of Object.entries(def.params)) { @@ -78,7 +97,8 @@ export function createServer(version: string): McpServer { }, async ({ path: filePath }: { path: string }) => { try { - const buf = await readFile(filePath) + const path = resolveAndCheckPath(filePath) + const buf = await readFile(path) graph = await parseFigFile(buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength)) computeAllLayouts(graph) const pages = graph.getPages() @@ -100,9 +120,10 @@ export function createServer(version: string): McpServer { try { if (!graph) throw new Error('No document loaded') const { exportFigFile } = await import('@open-pencil/core') + const path = resolveAndCheckPath(filePath) const data = await exportFigFile(graph) - await writeFile(filePath, new Uint8Array(data)) - return ok({ saved: filePath, bytes: data.byteLength }) + await writeFile(path, new Uint8Array(data)) + return ok({ saved: path, bytes: data.byteLength }) } catch (e) { return fail(e) } @@ -128,6 +149,7 @@ export function createServer(version: string): McpServer { ) for (const tool of ALL_TOOLS) { + if (!enableEval && tool.name === 'eval') continue registerTool(tool) } diff --git a/tests/engine/mcp-server.test.ts b/tests/engine/mcp-server.test.ts index 716082801..216089c52 100644 --- a/tests/engine/mcp-server.test.ts +++ b/tests/engine/mcp-server.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test, beforeEach, afterEach } from 'bun:test' import { join } from 'node:path' -import { writeFile, unlink } from 'node:fs/promises' +import { mkdtemp, rm, unlink, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' @@ -12,6 +13,21 @@ function parseResult(result: { content: { type: string; text: string }[] }): unk return JSON.parse(result.content[0].text) } +async function createLinkedClient(options?: Parameters[1]) { + const server = createServer('0.0.0-test', options) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + await server.connect(serverTransport) + const client = new Client({ name: 'test-client', version: '0.0.0' }) + await client.connect(clientTransport) + return { + client, + close: async () => { + await client.close() + await server.close() + } + } +} + describe('MCP server', () => { let client: Client let cleanup: () => Promise @@ -261,4 +277,53 @@ describe('MCP server', () => { const text = r.content[0].text expect(r.isError === true || text.toLowerCase().includes('required')).toBe(true) }) + + test('createServer option enableEval=false removes eval tool', async () => { + const custom = await createLinkedClient({ enableEval: false }) + try { + const { tools } = await custom.client.listTools() + const names = tools.map((t) => t.name) + expect(names).not.toContain('eval') + expect(names).toContain('create_shape') + } finally { + await custom.close() + } + }) + + test('createServer option fileRoot restricts open_file and save_file paths', async () => { + const rootDir = await mkdtemp(join(tmpdir(), 'openpencil-mcp-root-')) + const insidePath = join(rootDir, 'inside.fig') + const outsidePath = join(tmpdir(), `outside-${Date.now()}.fig`) + + const graph = new SceneGraph() + const bytes = exportFigFile(graph) + await writeFile(insidePath, new Uint8Array(bytes)) + + const custom = await createLinkedClient({ fileRoot: rootDir }) + try { + const openInside = await custom.client.callTool({ + name: 'open_file', + arguments: { path: insidePath } + }) + expect(openInside.isError).not.toBe(true) + + const saveOutside = await custom.client.callTool({ + name: 'save_file', + arguments: { path: outsidePath } + }) + expect(saveOutside.isError).toBe(true) + const saveOutsideErr = parseResult(saveOutside) as { error: string } + expect(saveOutsideErr.error).toContain('outside allowed root') + + const saveInside = await custom.client.callTool({ + name: 'save_file', + arguments: { path: insidePath } + }) + expect(saveInside.isError).not.toBe(true) + } finally { + await custom.close() + await unlink(outsidePath).catch(() => {}) + await rm(rootDir, { recursive: true, force: true }) + } + }) })