From 0d7757e3dca3fe7e2392f0ed9f757bd17ae7bd13 Mon Sep 17 00:00:00 2001 From: Cliff Hall Date: Sat, 11 Apr 2026 20:26:22 -0400 Subject: [PATCH] fix: validate serverInfo.websiteUrl before rendering as link (#1186) * fix: validate serverInfo.websiteUrl before rendering as link A connected MCP server's serverInfo.websiteUrl was rendered directly as an with no protocol check, allowing a malicious server to supply javascript: (or other unsafe) URLs that would execute on click. Run the value through validateRedirectUrl and, on failure, show a warning-colored "Potentially malicious websiteURL field" message instead of a link. The offending URL is still surfaced via tooltip for inspection. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: render websiteUrl warning below server name Move the "Potentially malicious websiteURL field" message onto its own row beneath the icon+name row so it doesn't crowd the server name horizontally in the narrow sidebar. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- client/src/components/Sidebar.tsx | 104 +++++++++++------- .../src/components/__tests__/Sidebar.test.tsx | 64 +++++++++++ 2 files changed, 131 insertions(+), 37 deletions(-) diff --git a/client/src/components/Sidebar.tsx b/client/src/components/Sidebar.tsx index 13a4f24f..762678f2 100644 --- a/client/src/components/Sidebar.tsx +++ b/client/src/components/Sidebar.tsx @@ -15,6 +15,7 @@ import { Copy, CheckCheck, Server, + AlertTriangle, } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; @@ -42,6 +43,7 @@ import CustomHeaders from "./CustomHeaders"; import { CustomHeaders as CustomHeadersType } from "@/lib/types/customHeaders"; import { useToast } from "../lib/hooks/useToast"; import IconDisplay, { WithIcons } from "./IconDisplay"; +import { validateRedirectUrl } from "@/utils/urlValidation"; interface SidebarProps { connectionStatus: ConnectionStatus; @@ -782,44 +784,72 @@ const Sidebar = ({ - {connectionStatus === "connected" && serverImplementation && ( -
-
- {(serverImplementation as WithIcons).icons && - (serverImplementation as WithIcons).icons!.length > 0 ? ( - - ) : ( - - )} - {(serverImplementation as { websiteUrl?: string }) - .websiteUrl ? ( - - {serverImplementation.name || "MCP Server"} - - ) : ( - - {serverImplementation.name || "MCP Server"} - - )} -
- {serverImplementation.version && ( -
- Version: {serverImplementation.version} + {connectionStatus === "connected" && + serverImplementation && + (() => { + const websiteUrl = ( + serverImplementation as { websiteUrl?: string } + ).websiteUrl; + let isValidWebsiteUrl = false; + if (websiteUrl) { + try { + validateRedirectUrl(websiteUrl); + isValidWebsiteUrl = true; + } catch { + isValidWebsiteUrl = false; + } + } + const serverName = serverImplementation.name || "MCP Server"; + return ( +
+
+ {(serverImplementation as WithIcons).icons && + (serverImplementation as WithIcons).icons!.length > 0 ? ( + + ) : ( + + )} + {websiteUrl && isValidWebsiteUrl ? ( + + {serverName} + + ) : ( + + {serverName} + + )} +
+ {websiteUrl && !isValidWebsiteUrl && ( + + +
+ + + Potentially malicious websiteURL field + +
+ + +

{websiteUrl}

+ + + )} + {serverImplementation.version && ( +
+ Version: {serverImplementation.version} +
+ )}
- )} -
- )} + ); + })()} {loggingSupported && connectionStatus === "connected" && (
diff --git a/client/src/components/__tests__/Sidebar.test.tsx b/client/src/components/__tests__/Sidebar.test.tsx index 03e898ca..460161e5 100644 --- a/client/src/components/__tests__/Sidebar.test.tsx +++ b/client/src/components/__tests__/Sidebar.test.tsx @@ -1046,4 +1046,68 @@ describe("Sidebar", () => { ); }); }); + + describe("Server Implementation websiteUrl", () => { + it("should render a warning instead of a link when websiteUrl uses an unsafe protocol", () => { + renderSidebar({ + connectionStatus: "connected", + serverImplementation: { + name: "Evil Server", + version: "1.0.0", + websiteUrl: "javascript:alert('xss')", + }, + }); + + // The warning message should be displayed + expect( + screen.getByText("Potentially malicious websiteURL field"), + ).toBeInTheDocument(); + + // The server name should still be shown + expect(screen.getByText("Evil Server")).toBeInTheDocument(); + + // No link should have been rendered for the malicious URL + expect( + screen.queryByRole("link", { name: "Evil Server" }), + ).not.toBeInTheDocument(); + }); + + it("should render a link when websiteUrl is a valid https URL", () => { + renderSidebar({ + connectionStatus: "connected", + serverImplementation: { + name: "Good Server", + version: "1.0.0", + websiteUrl: "https://example.com", + }, + }); + + const link = screen.getByRole("link", { name: "Good Server" }); + expect(link).toBeInTheDocument(); + expect(link).toHaveAttribute("href", "https://example.com"); + + expect( + screen.queryByText("Potentially malicious websiteURL field"), + ).not.toBeInTheDocument(); + }); + + it("should render a link when websiteUrl is a valid http URL", () => { + renderSidebar({ + connectionStatus: "connected", + serverImplementation: { + name: "Local Server", + version: "1.0.0", + websiteUrl: "http://localhost:3000", + }, + }); + + const link = screen.getByRole("link", { name: "Local Server" }); + expect(link).toBeInTheDocument(); + expect(link).toHaveAttribute("href", "http://localhost:3000"); + + expect( + screen.queryByText("Potentially malicious websiteURL field"), + ).not.toBeInTheDocument(); + }); + }); });