From 17740daaf5c04c2a0271de9e89931a210886a3a7 Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Thu, 12 Jun 2025 17:57:41 +0100 Subject: [PATCH] fix: address @jenn-newton comments - Add authentication and origin validation middleware to /config endpoint - Implement timing-safe token comparison using crypto.timingSafeEqual() - Add config read to initial env setup s.t. /config call is authenticated --- client/src/App.tsx | 12 +++++++++--- server/src/index.ts | 32 ++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/client/src/App.tsx b/client/src/App.tsx index c44a0e5f..8aca8697 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -63,6 +63,7 @@ import ToolsTab from "./components/ToolsTab"; import { InspectorConfig } from "./lib/configurationTypes"; import { getMCPProxyAddress, + getMCPProxyAuthToken, getInitialSseUrl, getInitialTransportType, getInitialCommand, @@ -345,7 +346,13 @@ const App = () => { }, [sseUrl]); useEffect(() => { - fetch(`${getMCPProxyAddress(config)}/config`) + const headers: HeadersInit = {}; + const proxyAuthToken = getMCPProxyAuthToken(config); + if (proxyAuthToken) { + headers['Authorization'] = `Bearer ${proxyAuthToken}`; + } + + fetch(`${getMCPProxyAddress(config)}/config`, { headers }) .then((response) => response.json()) .then((data) => { setEnv(data.defaultEnvironment); @@ -359,8 +366,7 @@ const App = () => { .catch((error) => console.error("Error fetching default environment:", error), ); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [config]); useEffect(() => { rootsRef.current = roots; diff --git a/server/src/index.ts b/server/src/index.ts index c82227ac..a6ca2f42 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -19,7 +19,7 @@ import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import express from "express"; import { findActualExecutable } from "spawn-rx"; import mcpProxy from "./mcpProxy.js"; -import { randomUUID, randomBytes } from "node:crypto"; +import { randomUUID, randomBytes, timingSafeEqual } from "node:crypto"; const SSE_HEADERS_PASSTHROUGH = ["authorization"]; const STREAMABLE_HTTP_HEADERS_PASSTHROUGH = [ @@ -120,14 +120,38 @@ const authMiddleware = (req: express.Request, res: express.Response, next: expre return next(); } - const authHeader = req.headers.authorization; - if (!authHeader || authHeader !== `Bearer ${sessionToken}`) { + const sendUnauthorized = () => { res.status(401).json({ error: "Unauthorized", message: "Authentication required. Use the session token shown in the console when starting the server." }); + }; + + const authHeader = req.headers.authorization; + if (!authHeader || !authHeader.startsWith('Bearer ')) { + sendUnauthorized(); return; } + + const providedToken = authHeader.substring(7); // Remove 'Bearer ' prefix + const expectedToken = sessionToken; + + // Convert to buffers for timing-safe comparison + const providedBuffer = Buffer.from(providedToken); + const expectedBuffer = Buffer.from(expectedToken); + + // Check length first to prevent timing attacks + if (providedBuffer.length !== expectedBuffer.length) { + sendUnauthorized(); + return; + } + + // Perform timing-safe comparison + if (!timingSafeEqual(providedBuffer, expectedBuffer)) { + sendUnauthorized(); + return; + } + next(); }; @@ -444,7 +468,7 @@ app.get("/health", (req, res) => { }); }); -app.get("/config", (req, res) => { +app.get("/config", originValidationMiddleware, authMiddleware, (req, res) => { try { res.json({ defaultEnvironment,