github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-09-01 17:49:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Gate MCP REPL server behind isDevEnv check (#11282)

Browse Source 
* 🐛 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

* ✨ Add PENPOT_MCP_REPL_ENABLE env var for explicit REPL control

Allow the REPL server to be enabled independently of the devenv
setting via a new PENPOT_MCP_REPL_ENABLE environment variable.
When set to "true", the REPL server starts regardless of
PENPOT_MCP_DEVENV; when set to any other value, it is disabled.
When unset, the previous isDevEnv fallback applies.

Addresses review feedback on PR #11282.

AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
Andrey Antukh 2026-09-01 08:50:05 +02:00 committed by GitHub
parent 810f8ef671
commit 23869f3b1b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 217 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
mcp/README.md
View File

@ -265,6 +265,7 @@ The Penpot MCP server can be configured using environment variables.
| `PENPOT_MCP_SERVER_PORT` | Port for the HTTP/SSE server | `4401` |
| `PENPOT_MCP_WEBSOCKET_PORT` | Port for the WebSocket server (plugin connection) | `4402` |
| `PENPOT_MCP_REPL_PORT` | Port for the REPL server (development/debugging) | `4403` |
| `PENPOT_MCP_REPL_ENABLE` | Explicitly enable/disable the REPL server. Set to `true` to enable. When unset, defaults to the value of `PENPOT_MCP_DEVENV`. | (unset) |
| `PENPOT_MCP_REMOTE_MODE` | Enable remote mode (disables file system access). Set to `true` to enable. | `false` |
| `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools. Set to `true` to enable. | `false` |
| `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` |

149
mcp/packages/server/src/PenpotMcpServer.test.ts Normal file
View File

@ -0,0 +1,149 @@
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);
});
// ── Pure function tests: isReplEnabled ──────────────────────────
test("isReplEnabled returns false when neither env var is set", () => {
assert.equal(PenpotMcpServer.isReplEnabled({}), false);
});
test("isReplEnabled returns true when PENPOT_MCP_DEVENV is 'true' (fallback)", () => {
assert.equal(PenpotMcpServer.isReplEnabled({ PENPOT_MCP_DEVENV: "true" }), true);
});
test("isReplEnabled returns true when PENPOT_MCP_REPL_ENABLE is 'true'", () => {
assert.equal(PenpotMcpServer.isReplEnabled({ PENPOT_MCP_REPL_ENABLE: "true" }), true);
});
test("isReplEnabled returns false when PENPOT_MCP_REPL_ENABLE is 'false' even if DEVENV is true", () => {
assert.equal(PenpotMcpServer.isReplEnabled({ PENPOT_MCP_REPL_ENABLE: "false", PENPOT_MCP_DEVENV: "true" }), false);
});
test("isReplEnabled returns true when PENPOT_MCP_REPL_ENABLE is 'true' regardless of DEVENV", () => {
assert.equal(PenpotMcpServer.isReplEnabled({ PENPOT_MCP_REPL_ENABLE: "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);
}
});
test("constructor creates ReplServer when PENPOT_MCP_REPL_ENABLE is 'true' without DEVENV", async () => {
const prevDevEnv = process.env.PENPOT_MCP_DEVENV;
const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE;
const prevPorts = setUniqueEnv();
delete process.env.PENPOT_MCP_DEVENV;
process.env.PENPOT_MCP_REPL_ENABLE = "true";
let server: PenpotMcpServer | undefined;
try {
server = new PenpotMcpServer(false);
assert.equal(server.hasReplServer(), true);
} finally {
await server?.stop();
restoreEnv(prevDevEnv, prevPorts);
restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable);
}
});
test("constructor does not create ReplServer when PENPOT_MCP_REPL_ENABLE is 'false' even with DEVENV", async () => {
const prevDevEnv = process.env.PENPOT_MCP_DEVENV;
const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE;
const prevPorts = setUniqueEnv();
process.env.PENPOT_MCP_DEVENV = "true";
process.env.PENPOT_MCP_REPL_ENABLE = "false";
let server: PenpotMcpServer | undefined;
try {
server = new PenpotMcpServer(false);
assert.equal(server.hasReplServer(), false);
} finally {
await server?.stop();
restoreEnv(prevDevEnv, prevPorts);
restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable);
}
});
// ── 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];
}
}

61
mcp/packages/server/src/PenpotMcpServer.ts
View File

@ -56,6 +56,30 @@ export class PenpotMcpServer {
*/
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";
}
/**
* Determines whether the REPL server should be enabled.
*
* If ``PENPOT_MCP_REPL_ENABLE`` is set, its value controls the result
* (``"true"`` enables, any other value disables). When the variable is
* not set, the result falls back to {@link isDevEnvEnabled}.
*/
public static isReplEnabled(env: Record<string, string | undefined>): boolean {
if (env.PENPOT_MCP_REPL_ENABLE !== undefined) {
return env.PENPOT_MCP_REPL_ENABLE === "true";
}
return PenpotMcpServer.isDevEnvEnabled(env);
}
/**
* Returns a short, non-reversible fingerprint of a user token, suitable for
* correlating log lines without exposing the full credential.
@ -83,7 +107,7 @@ export class PenpotMcpServer {
public readonly configLoader: ConfigurationLoader;
private app: any;
public readonly pluginBridge: PluginBridge;
private readonly replServer: ReplServer;
private readonly replServer: ReplServer | null;
private apiDocs: ApiDocs;
private readonly penpotHighLevelOverview: string;
private readonly connectionInstructions: string;
@ -149,7 +173,12 @@ export class PenpotMcpServer {
}
this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge);
this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host);
if (PenpotMcpServer.isReplEnabled(process.env)) {
this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host);
} else {
this.replServer = null;
}
}
/**
@ -190,7 +219,18 @@ export class PenpotMcpServer {
* additional developer tools such as ClojureScript expression evaluation are exposed.
*/
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 when {@link isReplEnabled} returns true,
* which means either ``PENPOT_MCP_REPL_ENABLE=true`` or, when that
* variable is unset, ``PENPOT_MCP_DEVENV=true``.
*/
public hasReplServer(): boolean {
return this.replServer !== null;
}
/**
@ -421,8 +461,14 @@ export class PenpotMcpServer {
this.logger.info(`Legacy SSE endpoint: http://${this.host}:${this.port}/sse`);
this.logger.info(`WebSocket server URL: ws://${this.host}:${this.webSocketPort}`);
// start the REPL server and session timeout checker
await this.replServer.start();
// start the REPL server (devenv only) and session timeout checker
if (this.replServer) {
await this.replServer.start();
} else {
this.logger.info(
"REPL server disabled (set PENPOT_MCP_REPL_ENABLE=true or PENPOT_MCP_DEVENV=true to enable)"
);
}
this.startSessionTimeoutChecker();
resolve();
@ -438,8 +484,11 @@ export class PenpotMcpServer {
public async stop(): Promise<void> {
this.logger.info("Stopping Penpot MCP Server...");
clearInterval(this.sessionTimeoutInterval);
await this.pluginBridge.close();
await this.redisBridge?.close();
await this.replServer.stop();
if (this.replServer) {
await this.replServer.stop();
}
this.logger.info("Penpot MCP Server stopped");
}
}

12
mcp/packages/server/src/PluginBridge.ts
View File

@ -467,4 +467,16 @@ export class PluginBridge {
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();
});
});
}
}