Merge branch 'main' into main
This commit is contained in:
commit
d758e8be93
11 changed files with 819 additions and 601 deletions
|
|
@ -34,7 +34,6 @@ import {
|
|||
useDraggablePane,
|
||||
useDraggableSidebar,
|
||||
} from "./lib/hooks/useDraggablePane";
|
||||
import { StdErrNotification } from "./lib/notificationTypes";
|
||||
|
||||
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||
import { Button } from "@/components/ui/button";
|
||||
|
|
@ -106,9 +105,6 @@ const App = () => {
|
|||
>(getInitialTransportType);
|
||||
const [logLevel, setLogLevel] = useState<LoggingLevel>("debug");
|
||||
const [notifications, setNotifications] = useState<ServerNotification[]>([]);
|
||||
const [stdErrNotifications, setStdErrNotifications] = useState<
|
||||
StdErrNotification[]
|
||||
>([]);
|
||||
const [roots, setRoots] = useState<Root[]>([]);
|
||||
const [env, setEnv] = useState<Record<string, string>>({});
|
||||
|
||||
|
|
@ -224,12 +220,6 @@ const App = () => {
|
|||
onNotification: (notification) => {
|
||||
setNotifications((prev) => [...prev, notification as ServerNotification]);
|
||||
},
|
||||
onStdErrNotification: (notification) => {
|
||||
setStdErrNotifications((prev) => [
|
||||
...prev,
|
||||
notification as StdErrNotification,
|
||||
]);
|
||||
},
|
||||
onPendingRequest: (request, resolve, reject) => {
|
||||
setPendingSampleRequests((prev) => [
|
||||
...prev,
|
||||
|
|
@ -757,10 +747,6 @@ const App = () => {
|
|||
setLogLevel(level);
|
||||
};
|
||||
|
||||
const clearStdErrNotifications = () => {
|
||||
setStdErrNotifications([]);
|
||||
};
|
||||
|
||||
const AuthDebuggerWrapper = () => (
|
||||
<TabsContent value="auth">
|
||||
<AuthDebugger
|
||||
|
|
@ -829,11 +815,9 @@ const App = () => {
|
|||
setOauthScope={setOauthScope}
|
||||
onConnect={connectMcpServer}
|
||||
onDisconnect={disconnectMcpServer}
|
||||
stdErrNotifications={stdErrNotifications}
|
||||
logLevel={logLevel}
|
||||
sendLogLevelRequest={sendLogLevelRequest}
|
||||
loggingSupported={!!serverCapabilities?.logging || false}
|
||||
clearStdErrNotifications={clearStdErrNotifications}
|
||||
/>
|
||||
<div
|
||||
onMouseDown={handleSidebarDragStart}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { AuthDebuggerState, EMPTY_DEBUGGER_STATE } from "../lib/auth-types";
|
|||
import { OAuthFlowProgress } from "./OAuthFlowProgress";
|
||||
import { OAuthStateMachine } from "../lib/oauth-state-machine";
|
||||
import { SESSION_KEYS } from "../lib/constants";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
|
||||
export interface AuthDebuggerProps {
|
||||
serverUrl: string;
|
||||
|
|
@ -163,6 +164,23 @@ const AuthDebugger = ({
|
|||
currentState.oauthStep === "authorization_code" &&
|
||||
currentState.authorizationUrl
|
||||
) {
|
||||
// Validate the URL before redirecting
|
||||
try {
|
||||
validateRedirectUrl(currentState.authorizationUrl);
|
||||
} catch (error) {
|
||||
updateAuthState({
|
||||
...currentState,
|
||||
isInitiatingAuth: false,
|
||||
latestError:
|
||||
error instanceof Error ? error : new Error(String(error)),
|
||||
statusMessage: {
|
||||
type: "error",
|
||||
message: `Invalid authorization URL: ${error instanceof Error ? error.message : String(error)}`,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Store the current auth state before redirecting
|
||||
sessionStorage.setItem(
|
||||
SESSION_KEYS.AUTH_DEBUGGER_STATE,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import { Button } from "./ui/button";
|
|||
import { DebugInspectorOAuthClientProvider } from "@/lib/auth";
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { OAuthClientInformation } from "@modelcontextprotocol/sdk/shared/auth.js";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
import { useToast } from "@/lib/hooks/useToast";
|
||||
|
||||
interface OAuthStepProps {
|
||||
label: string;
|
||||
|
|
@ -71,6 +73,7 @@ export const OAuthFlowProgress = ({
|
|||
updateAuthState,
|
||||
proceedToNextStep,
|
||||
}: OAuthFlowProgressProps) => {
|
||||
const { toast } = useToast();
|
||||
const provider = useMemo(
|
||||
() => new DebugInspectorOAuthClientProvider(serverUrl),
|
||||
[serverUrl],
|
||||
|
|
@ -239,16 +242,32 @@ export const OAuthFlowProgress = ({
|
|||
<p className="text-xs break-all">
|
||||
{authState.authorizationUrl}
|
||||
</p>
|
||||
<a
|
||||
href={authState.authorizationUrl}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
<button
|
||||
onClick={() => {
|
||||
try {
|
||||
validateRedirectUrl(authState.authorizationUrl!);
|
||||
window.open(
|
||||
authState.authorizationUrl!,
|
||||
"_blank",
|
||||
"noopener noreferrer",
|
||||
);
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Invalid URL",
|
||||
description:
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: "The authorization URL is not valid",
|
||||
variant: "destructive",
|
||||
});
|
||||
}
|
||||
}}
|
||||
className="flex items-center text-blue-500 hover:text-blue-700"
|
||||
aria-label="Open authorization URL in new tab"
|
||||
title="Open authorization URL"
|
||||
>
|
||||
<ExternalLink className="h-4 w-4" />
|
||||
</a>
|
||||
</button>
|
||||
</div>
|
||||
<p className="text-xs text-muted-foreground mt-2">
|
||||
Click the link to authorize in your browser. After
|
||||
|
|
@ -354,7 +373,21 @@ export const OAuthFlowProgress = ({
|
|||
authState.authorizationUrl && (
|
||||
<Button
|
||||
variant="outline"
|
||||
onClick={() => window.open(authState.authorizationUrl!, "_blank")}
|
||||
onClick={() => {
|
||||
try {
|
||||
validateRedirectUrl(authState.authorizationUrl!);
|
||||
window.open(authState.authorizationUrl!, "_blank");
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Invalid URL",
|
||||
description:
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: "The authorization URL is not valid",
|
||||
variant: "destructive",
|
||||
});
|
||||
}
|
||||
}}
|
||||
>
|
||||
Open in New Tab
|
||||
</Button>
|
||||
|
|
|
|||
|
|
@ -24,7 +24,6 @@ import {
|
|||
SelectTrigger,
|
||||
SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
import { StdErrNotification } from "@/lib/notificationTypes";
|
||||
import {
|
||||
LoggingLevel,
|
||||
LoggingLevelSchema,
|
||||
|
|
@ -62,8 +61,6 @@ interface SidebarProps {
|
|||
setOauthScope: (scope: string) => void;
|
||||
onConnect: () => void;
|
||||
onDisconnect: () => void;
|
||||
stdErrNotifications: StdErrNotification[];
|
||||
clearStdErrNotifications: () => void;
|
||||
logLevel: LoggingLevel;
|
||||
sendLogLevelRequest: (level: LoggingLevel) => void;
|
||||
loggingSupported: boolean;
|
||||
|
|
@ -93,8 +90,6 @@ const Sidebar = ({
|
|||
setOauthScope,
|
||||
onConnect,
|
||||
onDisconnect,
|
||||
stdErrNotifications,
|
||||
clearStdErrNotifications,
|
||||
logLevel,
|
||||
sendLogLevelRequest,
|
||||
loggingSupported,
|
||||
|
|
@ -268,6 +263,7 @@ const Sidebar = ({
|
|||
placeholder="Command"
|
||||
value={command}
|
||||
onChange={(e) => setCommand(e.target.value)}
|
||||
onBlur={(e) => setCommand(e.target.value.trim())}
|
||||
className="font-mono"
|
||||
/>
|
||||
</div>
|
||||
|
|
@ -760,36 +756,6 @@ const Sidebar = ({
|
|||
</Select>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{stdErrNotifications.length > 0 && (
|
||||
<>
|
||||
<div className="mt-4 border-t border-gray-200 pt-4">
|
||||
<div className="flex justify-between items-center">
|
||||
<h3 className="text-sm font-medium">
|
||||
Error output from MCP server
|
||||
</h3>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={clearStdErrNotifications}
|
||||
className="h-8 px-2"
|
||||
>
|
||||
Clear
|
||||
</Button>
|
||||
</div>
|
||||
<div className="mt-2 max-h-80 overflow-y-auto">
|
||||
{stdErrNotifications.map((notification, index) => (
|
||||
<div
|
||||
key={index}
|
||||
className="text-sm text-red-500 font-mono py-2 border-b border-gray-200 last:border-b-0"
|
||||
>
|
||||
{notification.params.content}
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -11,6 +11,7 @@ import {
|
|||
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
|
||||
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
|
||||
import { generateOAuthState } from "@/utils/oauthUtils";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
|
||||
/**
|
||||
* Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
|
||||
|
|
@ -182,12 +183,8 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
|
|||
}
|
||||
|
||||
redirectToAuthorization(authorizationUrl: URL) {
|
||||
if (
|
||||
authorizationUrl.protocol !== "http:" &&
|
||||
authorizationUrl.protocol !== "https:"
|
||||
) {
|
||||
throw new Error("Authorization URL must be HTTP or HTTPS");
|
||||
}
|
||||
// Validate the URL using the shared utility
|
||||
validateRedirectUrl(authorizationUrl.href);
|
||||
window.location.href = authorizationUrl.href;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,7 @@ import { useEffect, useState } from "react";
|
|||
import { useToast } from "@/lib/hooks/useToast";
|
||||
import { z } from "zod";
|
||||
import { ConnectionStatus } from "../constants";
|
||||
import { Notification, StdErrNotificationSchema } from "../notificationTypes";
|
||||
import { Notification } from "../notificationTypes";
|
||||
import {
|
||||
auth,
|
||||
discoverOAuthProtectedResourceMetadata,
|
||||
|
|
@ -92,7 +92,6 @@ export function useConnection({
|
|||
oauthScope,
|
||||
config,
|
||||
onNotification,
|
||||
onStdErrNotification,
|
||||
onPendingRequest,
|
||||
onElicitationRequest,
|
||||
getRoots,
|
||||
|
|
@ -505,13 +504,6 @@ export function useConnection({
|
|||
};
|
||||
}
|
||||
|
||||
if (onStdErrNotification) {
|
||||
client.setNotificationHandler(
|
||||
StdErrNotificationSchema,
|
||||
onStdErrNotification,
|
||||
);
|
||||
}
|
||||
|
||||
let capabilities;
|
||||
try {
|
||||
const transport =
|
||||
|
|
|
|||
|
|
@ -5,18 +5,8 @@ import {
|
|||
} from "@modelcontextprotocol/sdk/types.js";
|
||||
import { z } from "zod";
|
||||
|
||||
export const StdErrNotificationSchema = BaseNotificationSchema.extend({
|
||||
method: z.literal("notifications/stderr"),
|
||||
params: z.object({
|
||||
content: z.string(),
|
||||
}),
|
||||
});
|
||||
|
||||
export const NotificationSchema = ClientNotificationSchema.or(
|
||||
StdErrNotificationSchema,
|
||||
)
|
||||
.or(ServerNotificationSchema)
|
||||
.or(BaseNotificationSchema);
|
||||
ServerNotificationSchema,
|
||||
).or(BaseNotificationSchema);
|
||||
|
||||
export type StdErrNotification = z.infer<typeof StdErrNotificationSchema>;
|
||||
export type Notification = z.infer<typeof NotificationSchema>;
|
||||
|
|
|
|||
135
client/src/utils/__tests__/urlValidation.test.ts
Normal file
135
client/src/utils/__tests__/urlValidation.test.ts
Normal file
|
|
@ -0,0 +1,135 @@
|
|||
import { validateRedirectUrl } from "../urlValidation";
|
||||
|
||||
describe("validateRedirectUrl", () => {
|
||||
describe("valid URLs", () => {
|
||||
it("should allow HTTP URLs", () => {
|
||||
expect(() => validateRedirectUrl("http://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow HTTPS URLs", () => {
|
||||
expect(() => validateRedirectUrl("https://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with ports", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("https://example.com:8080"),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with paths", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("https://example.com/path/to/auth"),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with query parameters", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("https://example.com?param=value"),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid URLs - XSS vectors", () => {
|
||||
it("should block javascript: protocol", () => {
|
||||
expect(() => validateRedirectUrl("javascript:alert('XSS')")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
|
||||
it("should block javascript: with encoded characters", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("javascript:alert%28%27XSS%27%29"),
|
||||
).toThrow("Authorization URL must be HTTP or HTTPS");
|
||||
});
|
||||
|
||||
it("should block data: protocol", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("data:text/html,<script>alert('XSS')</script>"),
|
||||
).toThrow("Authorization URL must be HTTP or HTTPS");
|
||||
});
|
||||
|
||||
it("should block vbscript: protocol", () => {
|
||||
expect(() => validateRedirectUrl("vbscript:msgbox")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
|
||||
it("should block file: protocol", () => {
|
||||
expect(() => validateRedirectUrl("file:///etc/passwd")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
|
||||
it("should block about: protocol", () => {
|
||||
expect(() => validateRedirectUrl("about:blank")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
|
||||
it("should block custom protocols", () => {
|
||||
expect(() => validateRedirectUrl("custom://example")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("should handle malformed URLs", () => {
|
||||
expect(() => validateRedirectUrl("not a url")).toThrow(
|
||||
"Invalid URL: not a url",
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle empty string", () => {
|
||||
expect(() => validateRedirectUrl("")).toThrow("Invalid URL: ");
|
||||
});
|
||||
|
||||
it("should handle URLs with unicode characters", () => {
|
||||
expect(() => validateRedirectUrl("https://例え.jp")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should handle URLs with case variations", () => {
|
||||
expect(() => validateRedirectUrl("HTTPS://EXAMPLE.COM")).not.toThrow();
|
||||
expect(() => validateRedirectUrl("HtTpS://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should handle protocol-relative URLs as invalid", () => {
|
||||
expect(() => validateRedirectUrl("//example.com")).toThrow(
|
||||
"Invalid URL: //example.com",
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle URLs with authentication", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("https://user:pass@example.com"),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("security considerations", () => {
|
||||
it("should not be fooled by whitespace", () => {
|
||||
expect(() => validateRedirectUrl(" javascript:alert('XSS')")).toThrow();
|
||||
expect(() => validateRedirectUrl("javascript:alert('XSS') ")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle null bytes", () => {
|
||||
expect(() =>
|
||||
validateRedirectUrl("java\x00script:alert('XSS')"),
|
||||
).toThrow();
|
||||
});
|
||||
|
||||
it("should handle tab characters", () => {
|
||||
expect(() => validateRedirectUrl("java\tscript:alert('XSS')")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle newlines", () => {
|
||||
expect(() => validateRedirectUrl("java\nscript:alert('XSS')")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle mixed case protocols", () => {
|
||||
expect(() => validateRedirectUrl("JaVaScRiPt:alert('XSS')")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
24
client/src/utils/urlValidation.ts
Normal file
24
client/src/utils/urlValidation.ts
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
/**
|
||||
* Validates that a URL is safe for redirection.
|
||||
* Only allows HTTP and HTTPS protocols to prevent XSS attacks.
|
||||
*
|
||||
* @param url - The URL string to validate
|
||||
* @throws Error if the URL has an unsafe protocol
|
||||
*/
|
||||
export function validateRedirectUrl(url: string): void {
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
if (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:") {
|
||||
throw new Error("Authorization URL must be HTTP or HTTPS");
|
||||
}
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof Error &&
|
||||
error.message === "Authorization URL must be HTTP or HTTPS"
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
// If URL parsing fails, it's also invalid
|
||||
throw new Error(`Invalid URL: ${url}`);
|
||||
}
|
||||
}
|
||||
|
|
@ -181,7 +181,7 @@ const createTransport = async (req: express.Request): Promise<Transport> => {
|
|||
const transportType = query.transportType as string;
|
||||
|
||||
if (transportType === "stdio") {
|
||||
const command = query.command as string;
|
||||
const command = (query.command as string).trim();
|
||||
const origArgs = shellParseArgs(query.args as string) as string[];
|
||||
const queryEnv = query.env ? JSON.parse(query.env as string) : {};
|
||||
const env = { ...defaultEnvironment, ...process.env, ...queryEnv };
|
||||
|
|
@ -401,24 +401,66 @@ app.get(
|
|||
|
||||
(serverTransport as StdioClientTransport).stderr!.on("data", (chunk) => {
|
||||
if (chunk.toString().includes("MODULE_NOT_FOUND")) {
|
||||
// Server command not found, remove transports
|
||||
const message = "Command not found, transports removed";
|
||||
webAppTransport.send({
|
||||
jsonrpc: "2.0",
|
||||
method: "notifications/stderr",
|
||||
method: "notifications/message",
|
||||
params: {
|
||||
content: "Command not found, transports removed",
|
||||
level: "emergency",
|
||||
logger: "proxy",
|
||||
data: {
|
||||
message,
|
||||
},
|
||||
},
|
||||
});
|
||||
webAppTransport.close();
|
||||
serverTransport.close();
|
||||
webAppTransports.delete(webAppTransport.sessionId);
|
||||
serverTransports.delete(webAppTransport.sessionId);
|
||||
console.error("Command not found, transports removed");
|
||||
console.error(message);
|
||||
} else {
|
||||
// Inspect message and attempt to assign a RFC 5424 Syslog Protocol level
|
||||
let level;
|
||||
let message = chunk.toString().trim();
|
||||
let ucMsg = chunk.toString().toUpperCase();
|
||||
if (ucMsg.includes("DEBUG")) {
|
||||
level = "debug";
|
||||
} else if (ucMsg.includes("INFO")) {
|
||||
level = "info";
|
||||
} else if (ucMsg.includes("NOTICE")) {
|
||||
level = "notice";
|
||||
} else if (ucMsg.includes("WARN")) {
|
||||
level = "warning";
|
||||
} else if (ucMsg.includes("ERROR")) {
|
||||
level = "error";
|
||||
} else if (ucMsg.includes("CRITICAL")) {
|
||||
level = "critical";
|
||||
} else if (ucMsg.includes("ALERT")) {
|
||||
level = "alert";
|
||||
} else if (ucMsg.includes("EMERGENCY")) {
|
||||
level = "emergency";
|
||||
} else if (ucMsg.includes("SIGINT")) {
|
||||
message = "SIGINT received. Server shutdown.";
|
||||
level = "emergency";
|
||||
} else if (ucMsg.includes("SIGHUP")) {
|
||||
message = "SIGHUP received. Server shutdown.";
|
||||
level = "emergency";
|
||||
} else if (ucMsg.includes("SIGTERM")) {
|
||||
message = "SIGTERM received. Server shutdown.";
|
||||
level = "emergency";
|
||||
} else {
|
||||
level = "info";
|
||||
}
|
||||
webAppTransport.send({
|
||||
jsonrpc: "2.0",
|
||||
method: "notifications/stderr",
|
||||
method: "notifications/message",
|
||||
params: {
|
||||
content: chunk.toString(),
|
||||
level,
|
||||
logger: "stdio",
|
||||
data: {
|
||||
message,
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue