diff --git a/client/src/App.tsx b/client/src/App.tsx index d6680c35..fecd9839 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -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("debug"); const [notifications, setNotifications] = useState([]); - const [stdErrNotifications, setStdErrNotifications] = useState< - StdErrNotification[] - >([]); const [roots, setRoots] = useState([]); const [env, setEnv] = useState>({}); @@ -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 = () => ( { setOauthScope={setOauthScope} onConnect={connectMcpServer} onDisconnect={disconnectMcpServer} - stdErrNotifications={stdErrNotifications} logLevel={logLevel} sendLogLevelRequest={sendLogLevelRequest} loggingSupported={!!serverCapabilities?.logging || false} - clearStdErrNotifications={clearStdErrNotifications} />

Click the link to authorize in your browser. After @@ -354,7 +373,21 @@ export const OAuthFlowProgress = ({ authState.authorizationUrl && ( diff --git a/client/src/components/Sidebar.tsx b/client/src/components/Sidebar.tsx index a41e4bfc..a44c6e29 100644 --- a/client/src/components/Sidebar.tsx +++ b/client/src/components/Sidebar.tsx @@ -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" /> @@ -760,36 +756,6 @@ const Sidebar = ({ )} - - {stdErrNotifications.length > 0 && ( - <> -

-
-

- Error output from MCP server -

- -
-
- {stdErrNotifications.map((notification, index) => ( -
- {notification.params.content} -
- ))} -
-
- - )} diff --git a/client/src/components/__tests__/Sidebar.test.tsx b/client/src/components/__tests__/Sidebar.test.tsx index e892a7f8..8f693717 100644 --- a/client/src/components/__tests__/Sidebar.test.tsx +++ b/client/src/components/__tests__/Sidebar.test.tsx @@ -31,7 +31,7 @@ Object.defineProperty(navigator, "clipboard", { // Setup fake timers jest.useFakeTimers(); -describe("Sidebar Environment Variables", () => { +describe("Sidebar", () => { const defaultProps = { connectionStatus: "disconnected" as const, transportType: "stdio" as const, @@ -69,595 +69,329 @@ describe("Sidebar Environment Variables", () => { ); }; - const openEnvVarsSection = () => { - const button = screen.getByTestId("env-vars-button"); - fireEvent.click(button); - }; - beforeEach(() => { jest.clearAllMocks(); jest.clearAllTimers(); }); - describe("Basic Operations", () => { - it("should add a new environment variable", () => { - const setEnv = jest.fn(); - renderSidebar({ env: {}, setEnv }); + describe("Command and arguments", () => { + it("should trim whitespace from command input on blur", () => { + const setCommand = jest.fn(); + renderSidebar({ command: " node server.js ", setCommand }); - openEnvVarsSection(); + const commandInput = screen.getByLabelText("Command"); - const addButton = screen.getByText("Add Environment Variable"); - fireEvent.click(addButton); - - expect(setEnv).toHaveBeenCalledWith({ "": "" }); + fireEvent.blur(commandInput); + expect(setCommand).toHaveBeenLastCalledWith("node server.js"); }); - it("should remove an environment variable", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); + it("should handle whitespace-only command input on blur", () => { + const setCommand = jest.fn(); + renderSidebar({ command: " ", setCommand }); - openEnvVarsSection(); + const commandInput = screen.getByLabelText("Command"); - const removeButton = screen.getByRole("button", { name: "Γ—" }); - fireEvent.click(removeButton); - - expect(setEnv).toHaveBeenCalledWith({}); + fireEvent.blur(commandInput); + expect(setCommand).toHaveBeenLastCalledWith(""); }); - it("should update environment variable value", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); + it("should not affect command without surrounding whitespace", () => { + const setCommand = jest.fn(); + renderSidebar({ command: "node", setCommand }); - openEnvVarsSection(); + const commandInput = screen.getByLabelText("Command"); - const valueInput = screen.getByDisplayValue("test_value"); - fireEvent.change(valueInput, { target: { value: "new_value" } }); - - expect(setEnv).toHaveBeenCalledWith({ TEST_KEY: "new_value" }); - }); - - it("should toggle value visibility", () => { - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv }); - - openEnvVarsSection(); - - const valueInput = screen.getByDisplayValue("test_value"); - expect(valueInput).toHaveProperty("type", "password"); - - const toggleButton = screen.getByRole("button", { name: /show value/i }); - fireEvent.click(toggleButton); - - expect(valueInput).toHaveProperty("type", "text"); + fireEvent.blur(commandInput); + expect(setCommand).toHaveBeenLastCalledWith("node"); }); }); - describe("Authentication", () => { - const openAuthSection = () => { - const button = screen.getByTestId("auth-button"); + describe("Environment Variables", () => { + const openEnvVarsSection = () => { + const button = screen.getByTestId("env-vars-button"); fireEvent.click(button); }; - it("should update bearer token", () => { - const setBearerToken = jest.fn(); - renderSidebar({ - bearerToken: "", - setBearerToken, - transportType: "sse", // Set transport type to SSE + describe("Basic Operations", () => { + it("should add a new environment variable", () => { + const setEnv = jest.fn(); + renderSidebar({ env: {}, setEnv }); + + openEnvVarsSection(); + + const addButton = screen.getByText("Add Environment Variable"); + fireEvent.click(addButton); + + expect(setEnv).toHaveBeenCalledWith({ "": "" }); }); - openAuthSection(); + it("should remove an environment variable", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); - const tokenInput = screen.getByTestId("bearer-token-input"); - fireEvent.change(tokenInput, { target: { value: "new_token" } }); + openEnvVarsSection(); - expect(setBearerToken).toHaveBeenCalledWith("new_token"); - }); + const removeButton = screen.getByRole("button", { name: "Γ—" }); + fireEvent.click(removeButton); - it("should update header name", () => { - const setHeaderName = jest.fn(); - renderSidebar({ - headerName: "Authorization", - setHeaderName, - transportType: "sse", + expect(setEnv).toHaveBeenCalledWith({}); }); - openAuthSection(); + it("should update environment variable value", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); - const headerInput = screen.getByTestId("header-input"); - fireEvent.change(headerInput, { target: { value: "X-Custom-Auth" } }); + openEnvVarsSection(); - expect(setHeaderName).toHaveBeenCalledWith("X-Custom-Auth"); - }); + const valueInput = screen.getByDisplayValue("test_value"); + fireEvent.change(valueInput, { target: { value: "new_value" } }); - it("should clear bearer token", () => { - const setBearerToken = jest.fn(); - renderSidebar({ - bearerToken: "existing_token", - setBearerToken, - transportType: "sse", // Set transport type to SSE + expect(setEnv).toHaveBeenCalledWith({ TEST_KEY: "new_value" }); }); - openAuthSection(); + it("should toggle value visibility", () => { + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv }); - const tokenInput = screen.getByTestId("bearer-token-input"); - fireEvent.change(tokenInput, { target: { value: "" } }); + openEnvVarsSection(); - expect(setBearerToken).toHaveBeenCalledWith(""); - }); + const valueInput = screen.getByDisplayValue("test_value"); + expect(valueInput).toHaveProperty("type", "password"); - it("should properly render bearer token input", () => { - const { rerender } = renderSidebar({ - bearerToken: "existing_token", - transportType: "sse", // Set transport type to SSE - }); + const toggleButton = screen.getByRole("button", { + name: /show value/i, + }); + fireEvent.click(toggleButton); - openAuthSection(); - - // Token input should be a password field - const tokenInput = screen.getByTestId("bearer-token-input"); - expect(tokenInput).toHaveProperty("type", "password"); - - // Update the token - fireEvent.change(tokenInput, { target: { value: "new_token" } }); - - // Rerender with updated token - rerender( - - - , - ); - - // Token input should still exist after update - expect(screen.getByTestId("bearer-token-input")).toBeInTheDocument(); - }); - - it("should maintain token visibility state after update", () => { - const { rerender } = renderSidebar({ - bearerToken: "existing_token", - transportType: "sse", // Set transport type to SSE - }); - - openAuthSection(); - - // Token input should be a password field - const tokenInput = screen.getByTestId("bearer-token-input"); - expect(tokenInput).toHaveProperty("type", "password"); - - // Update the token - fireEvent.change(tokenInput, { target: { value: "new_token" } }); - - // Rerender with updated token - rerender( - - - , - ); - - // Token input should still exist after update - expect(screen.getByTestId("bearer-token-input")).toBeInTheDocument(); - }); - - it("should maintain header name when toggling auth section", () => { - renderSidebar({ - headerName: "X-API-Key", - transportType: "sse", - }); - - // Open auth section - openAuthSection(); - - // Verify header name is displayed - const headerInput = screen.getByTestId("header-input"); - expect(headerInput).toHaveValue("X-API-Key"); - - // Close auth section - const authButton = screen.getByTestId("auth-button"); - fireEvent.click(authButton); - - // Reopen auth section - fireEvent.click(authButton); - - // Verify header name is still preserved - expect(screen.getByTestId("header-input")).toHaveValue("X-API-Key"); - }); - - it("should display default header name when not specified", () => { - renderSidebar({ - headerName: undefined, - transportType: "sse", - }); - - openAuthSection(); - - const headerInput = screen.getByTestId("header-input"); - expect(headerInput).toHaveAttribute("placeholder", "Authorization"); - }); - }); - - describe("Key Editing", () => { - it("should maintain order when editing first key", () => { - const setEnv = jest.fn(); - const initialEnv = { - FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - THIRD_KEY: "third_value", - }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const firstKeyInput = screen.getByDisplayValue("FIRST_KEY"); - fireEvent.change(firstKeyInput, { target: { value: "NEW_FIRST_KEY" } }); - - expect(setEnv).toHaveBeenCalledWith({ - NEW_FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - THIRD_KEY: "third_value", + expect(valueInput).toHaveProperty("type", "text"); }); }); - it("should maintain order when editing middle key", () => { - const setEnv = jest.fn(); - const initialEnv = { - FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - THIRD_KEY: "third_value", - }; - renderSidebar({ env: initialEnv, setEnv }); + describe("Key Editing", () => { + it("should maintain order when editing first key", () => { + const setEnv = jest.fn(); + const initialEnv = { + FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + THIRD_KEY: "third_value", + }; + renderSidebar({ env: initialEnv, setEnv }); - openEnvVarsSection(); + openEnvVarsSection(); - const middleKeyInput = screen.getByDisplayValue("SECOND_KEY"); - fireEvent.change(middleKeyInput, { target: { value: "NEW_SECOND_KEY" } }); + const firstKeyInput = screen.getByDisplayValue("FIRST_KEY"); + fireEvent.change(firstKeyInput, { target: { value: "NEW_FIRST_KEY" } }); - expect(setEnv).toHaveBeenCalledWith({ - FIRST_KEY: "first_value", - NEW_SECOND_KEY: "second_value", - THIRD_KEY: "third_value", - }); - }); - - it("should maintain order when editing last key", () => { - const setEnv = jest.fn(); - const initialEnv = { - FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - THIRD_KEY: "third_value", - }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const lastKeyInput = screen.getByDisplayValue("THIRD_KEY"); - fireEvent.change(lastKeyInput, { target: { value: "NEW_THIRD_KEY" } }); - - expect(setEnv).toHaveBeenCalledWith({ - FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - NEW_THIRD_KEY: "third_value", - }); - }); - - it("should maintain order during key editing", () => { - const setEnv = jest.fn(); - const initialEnv = { - KEY1: "value1", - KEY2: "value2", - }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - // Type "NEW_" one character at a time - const key1Input = screen.getByDisplayValue("KEY1"); - "NEW_".split("").forEach((char) => { - fireEvent.change(key1Input, { - target: { value: char + "KEY1".slice(1) }, + expect(setEnv).toHaveBeenCalledWith({ + NEW_FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + THIRD_KEY: "third_value", }); }); - // Verify the last setEnv call maintains the order - const lastCall = setEnv.mock.calls[ - setEnv.mock.calls.length - 1 - ][0] as Record; - const entries = Object.entries(lastCall); + it("should maintain order when editing middle key", () => { + const setEnv = jest.fn(); + const initialEnv = { + FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + THIRD_KEY: "third_value", + }; + renderSidebar({ env: initialEnv, setEnv }); - // The values should stay with their original keys - expect(entries[0][1]).toBe("value1"); // First entry should still have value1 - expect(entries[1][1]).toBe("value2"); // Second entry should still have value2 - }); - }); + openEnvVarsSection(); - describe("Multiple Operations", () => { - it("should maintain state after multiple key edits", () => { - const setEnv = jest.fn(); - const initialEnv = { - FIRST_KEY: "first_value", - SECOND_KEY: "second_value", - }; - const { rerender } = renderSidebar({ env: initialEnv, setEnv }); + const middleKeyInput = screen.getByDisplayValue("SECOND_KEY"); + fireEvent.change(middleKeyInput, { + target: { value: "NEW_SECOND_KEY" }, + }); - openEnvVarsSection(); + expect(setEnv).toHaveBeenCalledWith({ + FIRST_KEY: "first_value", + NEW_SECOND_KEY: "second_value", + THIRD_KEY: "third_value", + }); + }); - // First key edit - const firstKeyInput = screen.getByDisplayValue("FIRST_KEY"); - fireEvent.change(firstKeyInput, { target: { value: "NEW_FIRST_KEY" } }); + it("should maintain order when editing last key", () => { + const setEnv = jest.fn(); + const initialEnv = { + FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + THIRD_KEY: "third_value", + }; + renderSidebar({ env: initialEnv, setEnv }); - // Get the updated env from the first setEnv call - const updatedEnv = setEnv.mock.calls[0][0] as Record; + openEnvVarsSection(); - // Rerender with the updated env - rerender( - - - , - ); + const lastKeyInput = screen.getByDisplayValue("THIRD_KEY"); + fireEvent.change(lastKeyInput, { target: { value: "NEW_THIRD_KEY" } }); - // Second key edit - const secondKeyInput = screen.getByDisplayValue("SECOND_KEY"); - fireEvent.change(secondKeyInput, { target: { value: "NEW_SECOND_KEY" } }); + expect(setEnv).toHaveBeenCalledWith({ + FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + NEW_THIRD_KEY: "third_value", + }); + }); - // Verify the final state matches what we expect - expect(setEnv).toHaveBeenLastCalledWith({ - NEW_FIRST_KEY: "first_value", - NEW_SECOND_KEY: "second_value", + it("should maintain order during key editing", () => { + const setEnv = jest.fn(); + const initialEnv = { + KEY1: "value1", + KEY2: "value2", + }; + renderSidebar({ env: initialEnv, setEnv }); + + openEnvVarsSection(); + + // Type "NEW_" one character at a time + const key1Input = screen.getByDisplayValue("KEY1"); + "NEW_".split("").forEach((char) => { + fireEvent.change(key1Input, { + target: { value: char + "KEY1".slice(1) }, + }); + }); + + // Verify the last setEnv call maintains the order + const lastCall = setEnv.mock.calls[ + setEnv.mock.calls.length - 1 + ][0] as Record; + const entries = Object.entries(lastCall); + + // The values should stay with their original keys + expect(entries[0][1]).toBe("value1"); // First entry should still have value1 + expect(entries[1][1]).toBe("value2"); // Second entry should still have value2 }); }); - it("should maintain visibility state after key edit", () => { - const initialEnv = { TEST_KEY: "test_value" }; - const { rerender } = renderSidebar({ env: initialEnv }); + describe("Multiple Operations", () => { + it("should maintain state after multiple key edits", () => { + const setEnv = jest.fn(); + const initialEnv = { + FIRST_KEY: "first_value", + SECOND_KEY: "second_value", + }; + const { rerender } = renderSidebar({ env: initialEnv, setEnv }); - openEnvVarsSection(); + openEnvVarsSection(); - // Show the value - const toggleButton = screen.getByRole("button", { name: /show value/i }); - fireEvent.click(toggleButton); + // First key edit + const firstKeyInput = screen.getByDisplayValue("FIRST_KEY"); + fireEvent.change(firstKeyInput, { target: { value: "NEW_FIRST_KEY" } }); - const valueInput = screen.getByDisplayValue("test_value"); - expect(valueInput).toHaveProperty("type", "text"); + // Get the updated env from the first setEnv call + const updatedEnv = setEnv.mock.calls[0][0] as Record; - // Edit the key - const keyInput = screen.getByDisplayValue("TEST_KEY"); - fireEvent.change(keyInput, { target: { value: "NEW_KEY" } }); + // Rerender with the updated env + rerender( + + + , + ); - // Rerender with updated env - rerender( - - - , - ); + // Second key edit + const secondKeyInput = screen.getByDisplayValue("SECOND_KEY"); + fireEvent.change(secondKeyInput, { + target: { value: "NEW_SECOND_KEY" }, + }); - // Value should still be visible - const updatedValueInput = screen.getByDisplayValue("test_value"); - expect(updatedValueInput).toHaveProperty("type", "text"); + // Verify the final state matches what we expect + expect(setEnv).toHaveBeenLastCalledWith({ + NEW_FIRST_KEY: "first_value", + NEW_SECOND_KEY: "second_value", + }); + }); + + it("should maintain visibility state after key edit", () => { + const initialEnv = { TEST_KEY: "test_value" }; + const { rerender } = renderSidebar({ env: initialEnv }); + + openEnvVarsSection(); + + // Show the value + const toggleButton = screen.getByRole("button", { + name: /show value/i, + }); + fireEvent.click(toggleButton); + + const valueInput = screen.getByDisplayValue("test_value"); + expect(valueInput).toHaveProperty("type", "text"); + + // Edit the key + const keyInput = screen.getByDisplayValue("TEST_KEY"); + fireEvent.change(keyInput, { target: { value: "NEW_KEY" } }); + + // Rerender with updated env + rerender( + + + , + ); + + // Value should still be visible + const updatedValueInput = screen.getByDisplayValue("test_value"); + expect(updatedValueInput).toHaveProperty("type", "text"); + }); + }); + + describe("Edge Cases", () => { + it("should handle empty key", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); + + openEnvVarsSection(); + + const keyInput = screen.getByDisplayValue("TEST_KEY"); + fireEvent.change(keyInput, { target: { value: "" } }); + + expect(setEnv).toHaveBeenCalledWith({ "": "test_value" }); + }); + + it("should handle special characters in key", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); + + openEnvVarsSection(); + + const keyInput = screen.getByDisplayValue("TEST_KEY"); + fireEvent.change(keyInput, { target: { value: "TEST-KEY@123" } }); + + expect(setEnv).toHaveBeenCalledWith({ "TEST-KEY@123": "test_value" }); + }); + + it("should handle unicode characters in key", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); + + openEnvVarsSection(); + + const keyInput = screen.getByDisplayValue("TEST_KEY"); + fireEvent.change(keyInput, { target: { value: "TEST_πŸ”‘" } }); + + expect(setEnv).toHaveBeenCalledWith({ "TEST_πŸ”‘": "test_value" }); + }); + + it("should handle a very long key name", () => { + const setEnv = jest.fn(); + const initialEnv = { TEST_KEY: "test_value" }; + renderSidebar({ env: initialEnv, setEnv }); + + openEnvVarsSection(); + + const keyInput = screen.getByDisplayValue("TEST_KEY"); + const longKey = "A".repeat(100); + fireEvent.change(keyInput, { target: { value: longKey } }); + + expect(setEnv).toHaveBeenCalledWith({ [longKey]: "test_value" }); + }); }); }); - describe("Edge Cases", () => { - it("should handle empty key", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const keyInput = screen.getByDisplayValue("TEST_KEY"); - fireEvent.change(keyInput, { target: { value: "" } }); - - expect(setEnv).toHaveBeenCalledWith({ "": "test_value" }); - }); - - it("should handle special characters in key", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const keyInput = screen.getByDisplayValue("TEST_KEY"); - fireEvent.change(keyInput, { target: { value: "TEST-KEY@123" } }); - - expect(setEnv).toHaveBeenCalledWith({ "TEST-KEY@123": "test_value" }); - }); - - it("should handle unicode characters", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const keyInput = screen.getByDisplayValue("TEST_KEY"); - fireEvent.change(keyInput, { target: { value: "TEST_πŸ”‘" } }); - - expect(setEnv).toHaveBeenCalledWith({ "TEST_πŸ”‘": "test_value" }); - }); - - it("should handle very long key names", () => { - const setEnv = jest.fn(); - const initialEnv = { TEST_KEY: "test_value" }; - renderSidebar({ env: initialEnv, setEnv }); - - openEnvVarsSection(); - - const keyInput = screen.getByDisplayValue("TEST_KEY"); - const longKey = "A".repeat(100); - fireEvent.change(keyInput, { target: { value: longKey } }); - - expect(setEnv).toHaveBeenCalledWith({ [longKey]: "test_value" }); - }); - }); - - describe("Configuration Operations", () => { - const openConfigSection = () => { - const button = screen.getByTestId("config-button"); - fireEvent.click(button); - }; - - it("should update MCP server request timeout", () => { - const setConfig = jest.fn(); - renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); - - openConfigSection(); - - const timeoutInput = screen.getByTestId( - "MCP_SERVER_REQUEST_TIMEOUT-input", - ); - fireEvent.change(timeoutInput, { target: { value: "5000" } }); - - expect(setConfig).toHaveBeenCalledWith( - expect.objectContaining({ - MCP_SERVER_REQUEST_TIMEOUT: { - label: "Request Timeout", - description: "Timeout for requests to the MCP server (ms)", - value: 5000, - is_session_item: false, - }, - }), - ); - }); - - it("should update MCP server proxy address", () => { - const setConfig = jest.fn(); - renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); - - openConfigSection(); - - const proxyAddressInput = screen.getByTestId( - "MCP_PROXY_FULL_ADDRESS-input", - ); - fireEvent.change(proxyAddressInput, { - target: { value: "http://localhost:8080" }, - }); - - expect(setConfig).toHaveBeenCalledWith( - expect.objectContaining({ - MCP_PROXY_FULL_ADDRESS: { - label: "Inspector Proxy Address", - description: - "Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577", - value: "http://localhost:8080", - is_session_item: false, - }, - }), - ); - }); - - it("should update max total timeout", () => { - const setConfig = jest.fn(); - renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); - - openConfigSection(); - - const maxTotalTimeoutInput = screen.getByTestId( - "MCP_REQUEST_MAX_TOTAL_TIMEOUT-input", - ); - fireEvent.change(maxTotalTimeoutInput, { - target: { value: "10000" }, - }); - - expect(setConfig).toHaveBeenCalledWith( - expect.objectContaining({ - MCP_REQUEST_MAX_TOTAL_TIMEOUT: { - label: "Maximum Total Timeout", - description: - "Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications)", - value: 10000, - is_session_item: false, - }, - }), - ); - }); - - it("should handle invalid timeout values entered by user", () => { - const setConfig = jest.fn(); - renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); - - openConfigSection(); - - const timeoutInput = screen.getByTestId( - "MCP_SERVER_REQUEST_TIMEOUT-input", - ); - fireEvent.change(timeoutInput, { target: { value: "abc1" } }); - - expect(setConfig).toHaveBeenCalledWith( - expect.objectContaining({ - MCP_SERVER_REQUEST_TIMEOUT: { - label: "Request Timeout", - description: "Timeout for requests to the MCP server (ms)", - value: 0, - is_session_item: false, - }, - }), - ); - }); - - it("should maintain configuration state after multiple updates", () => { - const setConfig = jest.fn(); - const { rerender } = renderSidebar({ - config: DEFAULT_INSPECTOR_CONFIG, - setConfig, - }); - - openConfigSection(); - // First update - const timeoutInput = screen.getByTestId( - "MCP_SERVER_REQUEST_TIMEOUT-input", - ); - fireEvent.change(timeoutInput, { target: { value: "5000" } }); - - // Get the updated config from the first setConfig call - const updatedConfig = setConfig.mock.calls[0][0] as InspectorConfig; - - // Rerender with the updated config - rerender( - - - , - ); - - // Second update - const updatedTimeoutInput = screen.getByTestId( - "MCP_SERVER_REQUEST_TIMEOUT-input", - ); - fireEvent.change(updatedTimeoutInput, { target: { value: "3000" } }); - - // Verify the final state matches what we expect - expect(setConfig).toHaveBeenLastCalledWith( - expect.objectContaining({ - MCP_SERVER_REQUEST_TIMEOUT: { - label: "Request Timeout", - description: "Timeout for requests to the MCP server (ms)", - value: 3000, - is_session_item: false, - }, - }), - ); - }); - }); - - describe("Copy Configuration Features", () => { - beforeEach(() => { - jest.clearAllMocks(); - jest.clearAllTimers(); - }); - + describe("Copy Server Features", () => { const getCopyButtons = () => { return { serverEntry: screen.getByRole("button", { name: /server entry/i }), @@ -878,4 +612,307 @@ describe("Sidebar Environment Variables", () => { expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig); }); }); + + describe("Authentication", () => { + const openAuthSection = () => { + const button = screen.getByTestId("auth-button"); + fireEvent.click(button); + }; + + it("should update bearer token", () => { + const setBearerToken = jest.fn(); + renderSidebar({ + bearerToken: "", + setBearerToken, + transportType: "sse", // Set transport type to SSE + }); + + openAuthSection(); + + const tokenInput = screen.getByTestId("bearer-token-input"); + fireEvent.change(tokenInput, { target: { value: "new_token" } }); + + expect(setBearerToken).toHaveBeenCalledWith("new_token"); + }); + + it("should update header name", () => { + const setHeaderName = jest.fn(); + renderSidebar({ + headerName: "Authorization", + setHeaderName, + transportType: "sse", + }); + + openAuthSection(); + + const headerInput = screen.getByTestId("header-input"); + fireEvent.change(headerInput, { target: { value: "X-Custom-Auth" } }); + + expect(setHeaderName).toHaveBeenCalledWith("X-Custom-Auth"); + }); + + it("should clear bearer token", () => { + const setBearerToken = jest.fn(); + renderSidebar({ + bearerToken: "existing_token", + setBearerToken, + transportType: "sse", // Set transport type to SSE + }); + + openAuthSection(); + + const tokenInput = screen.getByTestId("bearer-token-input"); + fireEvent.change(tokenInput, { target: { value: "" } }); + + expect(setBearerToken).toHaveBeenCalledWith(""); + }); + + it("should properly render bearer token input", () => { + const { rerender } = renderSidebar({ + bearerToken: "existing_token", + transportType: "sse", // Set transport type to SSE + }); + + openAuthSection(); + + // Token input should be a password field + const tokenInput = screen.getByTestId("bearer-token-input"); + expect(tokenInput).toHaveProperty("type", "password"); + + // Update the token + fireEvent.change(tokenInput, { target: { value: "new_token" } }); + + // Rerender with updated token + rerender( + + + , + ); + + // Token input should still exist after update + expect(screen.getByTestId("bearer-token-input")).toBeInTheDocument(); + }); + + it("should maintain token visibility state after update", () => { + const { rerender } = renderSidebar({ + bearerToken: "existing_token", + transportType: "sse", // Set transport type to SSE + }); + + openAuthSection(); + + // Token input should be a password field + const tokenInput = screen.getByTestId("bearer-token-input"); + expect(tokenInput).toHaveProperty("type", "password"); + + // Update the token + fireEvent.change(tokenInput, { target: { value: "new_token" } }); + + // Rerender with updated token + rerender( + + + , + ); + + // Token input should still exist after update + expect(screen.getByTestId("bearer-token-input")).toBeInTheDocument(); + }); + + it("should maintain header name when toggling auth section", () => { + renderSidebar({ + headerName: "X-API-Key", + transportType: "sse", + }); + + // Open auth section + openAuthSection(); + + // Verify header name is displayed + const headerInput = screen.getByTestId("header-input"); + expect(headerInput).toHaveValue("X-API-Key"); + + // Close auth section + const authButton = screen.getByTestId("auth-button"); + fireEvent.click(authButton); + + // Reopen auth section + fireEvent.click(authButton); + + // Verify header name is still preserved + expect(screen.getByTestId("header-input")).toHaveValue("X-API-Key"); + }); + + it("should display default header name when not specified", () => { + renderSidebar({ + headerName: undefined, + transportType: "sse", + }); + + openAuthSection(); + + const headerInput = screen.getByTestId("header-input"); + expect(headerInput).toHaveAttribute("placeholder", "Authorization"); + }); + }); + + describe("Configuration", () => { + const openConfigSection = () => { + const button = screen.getByTestId("config-button"); + fireEvent.click(button); + }; + + it("should update MCP server request timeout", () => { + const setConfig = jest.fn(); + renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); + + openConfigSection(); + + const timeoutInput = screen.getByTestId( + "MCP_SERVER_REQUEST_TIMEOUT-input", + ); + fireEvent.change(timeoutInput, { target: { value: "5000" } }); + + expect(setConfig).toHaveBeenCalledWith( + expect.objectContaining({ + MCP_SERVER_REQUEST_TIMEOUT: { + label: "Request Timeout", + description: "Timeout for requests to the MCP server (ms)", + value: 5000, + is_session_item: false, + }, + }), + ); + }); + + it("should update MCP server proxy address", () => { + const setConfig = jest.fn(); + renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); + + openConfigSection(); + + const proxyAddressInput = screen.getByTestId( + "MCP_PROXY_FULL_ADDRESS-input", + ); + fireEvent.change(proxyAddressInput, { + target: { value: "http://localhost:8080" }, + }); + + expect(setConfig).toHaveBeenCalledWith( + expect.objectContaining({ + MCP_PROXY_FULL_ADDRESS: { + label: "Inspector Proxy Address", + description: + "Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577", + value: "http://localhost:8080", + is_session_item: false, + }, + }), + ); + }); + + it("should update max total timeout", () => { + const setConfig = jest.fn(); + renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); + + openConfigSection(); + + const maxTotalTimeoutInput = screen.getByTestId( + "MCP_REQUEST_MAX_TOTAL_TIMEOUT-input", + ); + fireEvent.change(maxTotalTimeoutInput, { + target: { value: "10000" }, + }); + + expect(setConfig).toHaveBeenCalledWith( + expect.objectContaining({ + MCP_REQUEST_MAX_TOTAL_TIMEOUT: { + label: "Maximum Total Timeout", + description: + "Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications)", + value: 10000, + is_session_item: false, + }, + }), + ); + }); + + it("should handle invalid timeout values entered by user", () => { + const setConfig = jest.fn(); + renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); + + openConfigSection(); + + const timeoutInput = screen.getByTestId( + "MCP_SERVER_REQUEST_TIMEOUT-input", + ); + fireEvent.change(timeoutInput, { target: { value: "abc1" } }); + + expect(setConfig).toHaveBeenCalledWith( + expect.objectContaining({ + MCP_SERVER_REQUEST_TIMEOUT: { + label: "Request Timeout", + description: "Timeout for requests to the MCP server (ms)", + value: 0, + is_session_item: false, + }, + }), + ); + }); + + it("should maintain configuration state after multiple updates", () => { + const setConfig = jest.fn(); + const { rerender } = renderSidebar({ + config: DEFAULT_INSPECTOR_CONFIG, + setConfig, + }); + + openConfigSection(); + // First update + const timeoutInput = screen.getByTestId( + "MCP_SERVER_REQUEST_TIMEOUT-input", + ); + fireEvent.change(timeoutInput, { target: { value: "5000" } }); + + // Get the updated config from the first setConfig call + const updatedConfig = setConfig.mock.calls[0][0] as InspectorConfig; + + // Rerender with the updated config + rerender( + + + , + ); + + // Second update + const updatedTimeoutInput = screen.getByTestId( + "MCP_SERVER_REQUEST_TIMEOUT-input", + ); + fireEvent.change(updatedTimeoutInput, { target: { value: "3000" } }); + + // Verify the final state matches what we expect + expect(setConfig).toHaveBeenLastCalledWith( + expect.objectContaining({ + MCP_SERVER_REQUEST_TIMEOUT: { + label: "Request Timeout", + description: "Timeout for requests to the MCP server (ms)", + value: 3000, + is_session_item: false, + }, + }), + ); + }); + }); }); diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 78d52a59..c1505398 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -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; } diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index d3690f31..8c44d51b 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -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 = diff --git a/client/src/lib/notificationTypes.ts b/client/src/lib/notificationTypes.ts index 8627ccc6..a956452a 100644 --- a/client/src/lib/notificationTypes.ts +++ b/client/src/lib/notificationTypes.ts @@ -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; export type Notification = z.infer; diff --git a/client/src/utils/__tests__/urlValidation.test.ts b/client/src/utils/__tests__/urlValidation.test.ts new file mode 100644 index 00000000..9ee6a454 --- /dev/null +++ b/client/src/utils/__tests__/urlValidation.test.ts @@ -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,"), + ).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", + ); + }); + }); +}); diff --git a/client/src/utils/urlValidation.ts b/client/src/utils/urlValidation.ts new file mode 100644 index 00000000..70bc458b --- /dev/null +++ b/client/src/utils/urlValidation.ts @@ -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}`); + } +} diff --git a/server/src/index.ts b/server/src/index.ts index 0a0f7bcc..a30c1845 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -181,7 +181,7 @@ const createTransport = async (req: express.Request): Promise => { 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, + }, }, }); }