mirror of
https://github.com/penpot/penpot.git
synced 2026-08-29 08:08:46 +00:00
🐛 Gate MCP REPL server behind isDevEnv check
The ReplServer was starting unconditionally on every MCP server instance, regardless of configuration. This exposed an unauthenticated POST /execute endpoint that forwarded arbitrary JavaScript to connected Penpot plugins. Gate ReplServer creation, startup, and shutdown behind isDevEnv(), consistent with how CljsReplTool and other dev tools are already protected. Log an info message when the REPL server is disabled. Consolidate the dev-env check into a single static isDevEnvEnabled() method that isDevEnv() delegates to, avoiding duplicate logic. Add PluginBridge.close() for proper WebSocket server cleanup on shutdown. Add regression tests that construct PenpotMcpServer and verify hasReplServer() returns the correct value based on the dev-env flag. AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
parent
4d90fe9126
commit
eb21d4e899
93
mcp/packages/server/src/PenpotMcpServer.test.ts
Normal file
93
mcp/packages/server/src/PenpotMcpServer.test.ts
Normal file
@ -0,0 +1,93 @@
|
|||||||
|
import assert from "node:assert/strict";
|
||||||
|
import test from "node:test";
|
||||||
|
import { PenpotMcpServer } from "./PenpotMcpServer";
|
||||||
|
|
||||||
|
// ── Pure function tests ────────────────────────────────────────
|
||||||
|
|
||||||
|
test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is not set", () => {
|
||||||
|
assert.equal(PenpotMcpServer.isDevEnvEnabled({}), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is 'false'", () => {
|
||||||
|
assert.equal(PenpotMcpServer.isDevEnvEnabled({ PENPOT_MCP_DEVENV: "false" }), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("isDevEnvEnabled returns true when PENPOT_MCP_DEVENV is 'true'", () => {
|
||||||
|
assert.equal(PenpotMcpServer.isDevEnvEnabled({ PENPOT_MCP_DEVENV: "true" }), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── Integration tests: constructor gating ──────────────────────
|
||||||
|
//
|
||||||
|
// Each test uses unique ports to avoid conflicts when tests run
|
||||||
|
// in the same process. The server is stopped in the finally block
|
||||||
|
// to release the WebSocket port.
|
||||||
|
|
||||||
|
let portCounter = 14_500;
|
||||||
|
function uniquePorts() {
|
||||||
|
const base = portCounter;
|
||||||
|
portCounter += 10;
|
||||||
|
return { server: base, ws: base + 1, repl: base + 2 };
|
||||||
|
}
|
||||||
|
|
||||||
|
test("constructor does not create ReplServer when PENPOT_MCP_DEVENV is unset", async () => {
|
||||||
|
const prev = process.env.PENPOT_MCP_DEVENV;
|
||||||
|
const prevPorts = setUniqueEnv();
|
||||||
|
delete process.env.PENPOT_MCP_DEVENV;
|
||||||
|
let server: PenpotMcpServer | undefined;
|
||||||
|
try {
|
||||||
|
server = new PenpotMcpServer(false);
|
||||||
|
assert.equal(server.hasReplServer(), false);
|
||||||
|
} finally {
|
||||||
|
await server?.stop();
|
||||||
|
restoreEnv(prev, prevPorts);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("constructor creates ReplServer when PENPOT_MCP_DEVENV is 'true'", async () => {
|
||||||
|
const prev = process.env.PENPOT_MCP_DEVENV;
|
||||||
|
const prevPorts = setUniqueEnv();
|
||||||
|
process.env.PENPOT_MCP_DEVENV = "true";
|
||||||
|
let server: PenpotMcpServer | undefined;
|
||||||
|
try {
|
||||||
|
server = new PenpotMcpServer(false);
|
||||||
|
assert.equal(server.hasReplServer(), true);
|
||||||
|
} finally {
|
||||||
|
await server?.stop();
|
||||||
|
restoreEnv(prev, prevPorts);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── Helpers ────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
function setUniqueEnv() {
|
||||||
|
const ports = uniquePorts();
|
||||||
|
const prevServer = process.env.PENPOT_MCP_SERVER_PORT;
|
||||||
|
const prevWs = process.env.PENPOT_MCP_WEBSOCKET_PORT;
|
||||||
|
const prevRepl = process.env.PENPOT_MCP_REPL_PORT;
|
||||||
|
process.env.PENPOT_MCP_SERVER_PORT = String(ports.server);
|
||||||
|
process.env.PENPOT_MCP_WEBSOCKET_PORT = String(ports.ws);
|
||||||
|
process.env.PENPOT_MCP_REPL_PORT = String(ports.repl);
|
||||||
|
return { prevServer, prevWs, prevRepl };
|
||||||
|
}
|
||||||
|
|
||||||
|
function restoreEnv(
|
||||||
|
devEnv: string | undefined,
|
||||||
|
ports: { prevServer: string | undefined; prevWs: string | undefined; prevRepl: string | undefined }
|
||||||
|
) {
|
||||||
|
if (devEnv !== undefined) {
|
||||||
|
process.env.PENPOT_MCP_DEVENV = devEnv;
|
||||||
|
} else {
|
||||||
|
delete process.env.PENPOT_MCP_DEVENV;
|
||||||
|
}
|
||||||
|
restoreOrDelete("PENPOT_MCP_SERVER_PORT", ports.prevServer);
|
||||||
|
restoreOrDelete("PENPOT_MCP_WEBSOCKET_PORT", ports.prevWs);
|
||||||
|
restoreOrDelete("PENPOT_MCP_REPL_PORT", ports.prevRepl);
|
||||||
|
}
|
||||||
|
|
||||||
|
function restoreOrDelete(key: string, value: string | undefined) {
|
||||||
|
if (value !== undefined) {
|
||||||
|
process.env[key] = value;
|
||||||
|
} else {
|
||||||
|
delete process.env[key];
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -56,6 +56,16 @@ export class PenpotMcpServer {
|
|||||||
*/
|
*/
|
||||||
private static readonly SESSION_TIMEOUT_MINUTES = 60;
|
private static readonly SESSION_TIMEOUT_MINUTES = 60;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determines whether the server is running in a Penpot development
|
||||||
|
* environment, based on the given environment variables.
|
||||||
|
*
|
||||||
|
* Returns ``true`` only when ``PENPOT_MCP_DEVENV`` is ``"true"``.
|
||||||
|
*/
|
||||||
|
public static isDevEnvEnabled(env: Record<string, string | undefined>): boolean {
|
||||||
|
return env.PENPOT_MCP_DEVENV === "true";
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns a short, non-reversible fingerprint of a user token, suitable for
|
* Returns a short, non-reversible fingerprint of a user token, suitable for
|
||||||
* correlating log lines without exposing the full credential.
|
* correlating log lines without exposing the full credential.
|
||||||
@ -83,7 +93,7 @@ export class PenpotMcpServer {
|
|||||||
public readonly configLoader: ConfigurationLoader;
|
public readonly configLoader: ConfigurationLoader;
|
||||||
private app: any;
|
private app: any;
|
||||||
public readonly pluginBridge: PluginBridge;
|
public readonly pluginBridge: PluginBridge;
|
||||||
private readonly replServer: ReplServer;
|
private readonly replServer: ReplServer | null;
|
||||||
private apiDocs: ApiDocs;
|
private apiDocs: ApiDocs;
|
||||||
private readonly penpotHighLevelOverview: string;
|
private readonly penpotHighLevelOverview: string;
|
||||||
private readonly connectionInstructions: string;
|
private readonly connectionInstructions: string;
|
||||||
@ -149,7 +159,12 @@ export class PenpotMcpServer {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge);
|
this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge);
|
||||||
this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host);
|
|
||||||
|
if (PenpotMcpServer.isDevEnvEnabled(process.env)) {
|
||||||
|
this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host);
|
||||||
|
} else {
|
||||||
|
this.replServer = null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -190,7 +205,16 @@ export class PenpotMcpServer {
|
|||||||
* additional developer tools such as ClojureScript expression evaluation are exposed.
|
* additional developer tools such as ClojureScript expression evaluation are exposed.
|
||||||
*/
|
*/
|
||||||
public isDevEnv(): boolean {
|
public isDevEnv(): boolean {
|
||||||
return process.env.PENPOT_MCP_DEVENV === "true";
|
return PenpotMcpServer.isDevEnvEnabled(process.env);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Indicates whether the REPL server was created.
|
||||||
|
*
|
||||||
|
* The REPL server is created only in development environment mode.
|
||||||
|
*/
|
||||||
|
public hasReplServer(): boolean {
|
||||||
|
return this.replServer !== null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -421,8 +445,12 @@ export class PenpotMcpServer {
|
|||||||
this.logger.info(`Legacy SSE endpoint: http://${this.host}:${this.port}/sse`);
|
this.logger.info(`Legacy SSE endpoint: http://${this.host}:${this.port}/sse`);
|
||||||
this.logger.info(`WebSocket server URL: ws://${this.host}:${this.webSocketPort}`);
|
this.logger.info(`WebSocket server URL: ws://${this.host}:${this.webSocketPort}`);
|
||||||
|
|
||||||
// start the REPL server and session timeout checker
|
// start the REPL server (devenv only) and session timeout checker
|
||||||
await this.replServer.start();
|
if (this.replServer) {
|
||||||
|
await this.replServer.start();
|
||||||
|
} else {
|
||||||
|
this.logger.info("REPL server disabled (set PENPOT_MCP_DEVENV=true to enable)");
|
||||||
|
}
|
||||||
this.startSessionTimeoutChecker();
|
this.startSessionTimeoutChecker();
|
||||||
|
|
||||||
resolve();
|
resolve();
|
||||||
@ -438,8 +466,11 @@ export class PenpotMcpServer {
|
|||||||
public async stop(): Promise<void> {
|
public async stop(): Promise<void> {
|
||||||
this.logger.info("Stopping Penpot MCP Server...");
|
this.logger.info("Stopping Penpot MCP Server...");
|
||||||
clearInterval(this.sessionTimeoutInterval);
|
clearInterval(this.sessionTimeoutInterval);
|
||||||
|
await this.pluginBridge.close();
|
||||||
await this.redisBridge?.close();
|
await this.redisBridge?.close();
|
||||||
await this.replServer.stop();
|
if (this.replServer) {
|
||||||
|
await this.replServer.stop();
|
||||||
|
}
|
||||||
this.logger.info("Penpot MCP Server stopped");
|
this.logger.info("Penpot MCP Server stopped");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -467,4 +467,16 @@ export class PluginBridge {
|
|||||||
task.rejectWithError(error instanceof Error ? error : new Error(String(error)));
|
task.rejectWithError(error instanceof Error ? error : new Error(String(error)));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Closes the WebSocket server and all connected client sockets.
|
||||||
|
*/
|
||||||
|
public async close(): Promise<void> {
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
this.wsServer.close(() => {
|
||||||
|
this.logger.info("WebSocket server closed");
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user