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 <a href> 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
a809c2ad5a
commit
0d7757e3dc
2 changed files with 131 additions and 37 deletions
|
|
@ -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 = ({
|
|||
</span>
|
||||
</div>
|
||||
|
||||
{connectionStatus === "connected" && serverImplementation && (
|
||||
<div className="bg-gray-50 dark:bg-gray-900 p-3 rounded-lg mb-4">
|
||||
<div className="flex items-center gap-2 mb-1">
|
||||
{(serverImplementation as WithIcons).icons &&
|
||||
(serverImplementation as WithIcons).icons!.length > 0 ? (
|
||||
<IconDisplay
|
||||
icons={(serverImplementation as WithIcons).icons}
|
||||
size="sm"
|
||||
/>
|
||||
) : (
|
||||
<Server className="w-4 h-4 text-gray-500" />
|
||||
)}
|
||||
{(serverImplementation as { websiteUrl?: string })
|
||||
.websiteUrl ? (
|
||||
<a
|
||||
href={
|
||||
(serverImplementation as { websiteUrl?: string })
|
||||
.websiteUrl
|
||||
}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="text-sm font-medium text-blue-600 dark:text-blue-400 hover:text-blue-700 dark:hover:text-blue-300 hover:underline transition-colors"
|
||||
>
|
||||
{serverImplementation.name || "MCP Server"}
|
||||
</a>
|
||||
) : (
|
||||
<span className="text-sm font-medium text-gray-800 dark:text-gray-200">
|
||||
{serverImplementation.name || "MCP Server"}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
{serverImplementation.version && (
|
||||
<div className="text-xs text-gray-500 dark:text-gray-400">
|
||||
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 (
|
||||
<div className="bg-gray-50 dark:bg-gray-900 p-3 rounded-lg mb-4">
|
||||
<div className="flex items-center gap-2 mb-1">
|
||||
{(serverImplementation as WithIcons).icons &&
|
||||
(serverImplementation as WithIcons).icons!.length > 0 ? (
|
||||
<IconDisplay
|
||||
icons={(serverImplementation as WithIcons).icons}
|
||||
size="sm"
|
||||
/>
|
||||
) : (
|
||||
<Server className="w-4 h-4 text-gray-500" />
|
||||
)}
|
||||
{websiteUrl && isValidWebsiteUrl ? (
|
||||
<a
|
||||
href={websiteUrl}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="text-sm font-medium text-blue-600 dark:text-blue-400 hover:text-blue-700 dark:hover:text-blue-300 hover:underline transition-colors"
|
||||
>
|
||||
{serverName}
|
||||
</a>
|
||||
) : (
|
||||
<span className="text-sm font-medium text-gray-800 dark:text-gray-200">
|
||||
{serverName}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
{websiteUrl && !isValidWebsiteUrl && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<div className="flex items-center gap-1 text-yellow-600 dark:text-yellow-500 mb-1">
|
||||
<AlertTriangle className="w-3.5 h-3.5 flex-shrink-0" />
|
||||
<span className="text-xs font-normal">
|
||||
Potentially malicious websiteURL field
|
||||
</span>
|
||||
</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
<p className="max-w-xs break-all">{websiteUrl}</p>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)}
|
||||
{serverImplementation.version && (
|
||||
<div className="text-xs text-gray-500 dark:text-gray-400">
|
||||
Version: {serverImplementation.version}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
);
|
||||
})()}
|
||||
|
||||
{loggingSupported && connectionStatus === "connected" && (
|
||||
<div className="space-y-2">
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue