From 07a7b0beb5a1875e90e607bba53a9c5837bebb5e Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 20:07:16 -0400 Subject: [PATCH 01/11] fix(web): accept CRLF-delimited SSE frames on the servers events stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/api/servers/events` reader in `useServers` searched for `\n\n` and consumed exactly two characters as the separator. SSE permits CRLF line endings, so a `event: change\r\n\r\n` frame stayed buffered forever: `isSseDataFrame()` was never reached and no background refresh fired, leaving the server list stale until a manual refresh or an LF-delimited event arrived. Frame boundaries are now located with a CRLF-aware matcher that reports the separator's real length, and frame lines split on either terminator. A bare CR is deliberately not accepted as a terminator: it cannot be distinguished from the first half of a CRLF straddling a chunk boundary, so a buffer ending in `…\r\n\r` would read as a completed frame and the `\n` arriving next would open a bogus one. Two regressions cover it, both verified failing against the old parser. Each uses a referentially stable `fetchFn` and holds the frame behind a promise released after the disk write — with an inline `fetchFn` the hook re-subscribes every render and that re-subscription's own mount refresh picks the edit up regardless, so the test would pass unfixed. Closes #2006 Signed-off-by: cliffhall --- .../src/test/core/react/useServers.test.tsx | 146 ++++++++++++++++++ core/react/useServers.ts | 44 +++++- 2 files changed, 183 insertions(+), 7 deletions(-) diff --git a/clients/web/src/test/core/react/useServers.test.tsx b/clients/web/src/test/core/react/useServers.test.tsx index bb95402d..e7c2fbf3 100644 --- a/clients/web/src/test/core/react/useServers.test.tsx +++ b/clients/web/src/test/core/react/useServers.test.tsx @@ -969,6 +969,152 @@ describe("useServers", () => { ); }); + it("parses CRLF-delimited SSE frames as change notifications (#2006)", async () => { + // SSE permits CRLF line endings, and this hook is a hand-rolled parser + // (EventSource can't send `x-mcp-remote-auth`). Matching only `\n\n` left + // a CRLF frame buffered forever, so an external mcp.json edit never + // reached the list. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { seed: { type: "stdio", command: "s" } }, + }), + ); + + // The frame is held back until the test has mutated the file, so the + // resulting list can only come from a refresh this frame triggered. + let releaseFrame: (() => void) | undefined; + const framed = new Promise((r) => { + releaseFrame = r; + }); + let reads = 0; + const encoder = new TextEncoder(); + // Referentially stable: an inline arrow re-subscribes on every render, and + // the re-subscription's own mount refresh would pick the edit up on its + // own — passing whether or not the frame was ever parsed. + const fetchFn: typeof fetch = async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/api/servers/events")) { + const body = { + getReader: () => ({ + read: async () => { + reads += 1; + if (reads === 1) { + await framed; + return { + done: false, + value: encoder.encode("event: change\r\n\r\n"), + }; + } + return { done: true, value: undefined }; + }, + }), + }; + return { ok: true, body } as unknown as Response; + } + return h.fetchFn(url, init); + }; + const { result } = renderHook(() => + useServers({ baseUrl: "http://test.local", fetchFn }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.servers.map((s) => s.id)).toEqual(["seed"]); + + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { aftercrlf: { type: "stdio", command: "x" } }, + }), + ); + releaseFrame?.(); + + await waitFor( + () => { + expect(result.current.servers.map((s) => s.id)).toEqual(["aftercrlf"]); + }, + { timeout: 3000 }, + ); + }); + + it("does not split a CRLF frame separator straddling two chunks (#2006)", async () => { + // The bare-CR terminator is deliberately not accepted: a chunk ending in + // `…\r\n\r` would otherwise read as a completed frame, and the `\n` + // arriving next would start a bogus one. Deliver exactly that split and + // assert the frame is recognized once — only after both halves land. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { seed: { type: "stdio", command: "s" } }, + }), + ); + + let releaseFirstHalf: (() => void) | undefined; + const firstHalf = new Promise((r) => { + releaseFirstHalf = r; + }); + let releaseSecondHalf: (() => void) | undefined; + const secondHalf = new Promise((r) => { + releaseSecondHalf = r; + }); + let listGets = 0; + let reads = 0; + const encoder = new TextEncoder(); + const fetchFn: typeof fetch = async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/api/servers/events")) { + const body = { + getReader: () => ({ + read: async () => { + reads += 1; + if (reads === 1) { + await firstHalf; + return { + done: false, + value: encoder.encode("event: change\r\n\r"), + }; + } + if (reads === 2) { + await secondHalf; + return { done: false, value: encoder.encode("\n") }; + } + return { done: true, value: undefined }; + }, + }), + }; + return { ok: true, body } as unknown as Response; + } + if (url.endsWith("/api/servers")) listGets += 1; + return h.fetchFn(url, init); + }; + const { result } = renderHook(() => + useServers({ baseUrl: "http://test.local", fetchFn }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(listGets).toBe(1); + + // Half a separator is not a frame: no refresh yet. + releaseFirstHalf?.(); + await waitFor(() => expect(reads).toBeGreaterThanOrEqual(2)); + await new Promise((r) => setTimeout(r, 100)); + expect(listGets).toBe(1); + + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { aftersplit: { type: "stdio", command: "x" } }, + }), + ); + releaseSecondHalf?.(); + + await waitFor( + () => { + expect(result.current.servers.map((s) => s.id)).toEqual(["aftersplit"]); + }, + { timeout: 3000 }, + ); + expect(listGets).toBe(2); + }); + it("ignores the backend's inert priming comment frame (#1858)", async () => { // The backend opens every SSE stream with a `:` comment so a streaming // fetch() resolves on Firefox at all. That frame carries no event/data diff --git a/core/react/useServers.ts b/core/react/useServers.ts index 3a2a5002..c9713344 100644 --- a/core/react/useServers.ts +++ b/core/react/useServers.ts @@ -90,10 +90,38 @@ async function readErrorMessage(res: Response): Promise { */ function isSseDataFrame(frame: string): boolean { return frame - .split("\n") + .split(SSE_LINE_SEPARATOR) .some((line) => line.startsWith("event:") || line.startsWith("data:")); } +/** + * SSE line terminators. The spec permits CRLF, LF, and a bare CR; we accept + * the first two. A bare CR is deliberately excluded because it can't be told + * apart from the first half of a CRLF that straddles a chunk boundary — a + * buffer ending in `…\r\n\r` would look like a completed frame and the `\n` + * arriving in the next chunk would be mis-read as the start of another. No + * SSE producer in the wild emits bare-CR frames, so the ambiguity buys + * nothing. + * + * A frame ends at a blank line, i.e. two terminators back to back. + */ +const SSE_LINE_SEPARATOR = /\r\n|\n/; +const SSE_FRAME_SEPARATOR = /(?:\r\n|\n){2}/; + +/** + * Locates the next frame boundary in `buffer`. Returns the frame's end offset + * and the length of the blank-line separator that terminated it (2–4 chars, + * depending on which terminators the server used), or `null` when the buffer + * holds no complete frame yet. + */ +function findSseFrameEnd( + buffer: string, +): { end: number; separatorLength: number } | null { + const match = SSE_FRAME_SEPARATOR.exec(buffer); + if (!match) return null; + return { end: match.index, separatorLength: match[0].length }; +} + export function useServers(opts: UseServersOptions): UseServersResult { const { baseUrl, authToken, fetchFn } = opts; const doFetch = fetchFn ?? globalThis.fetch; @@ -163,7 +191,9 @@ export function useServers(opts: UseServersOptions): UseServersResult { const reader = res.body.getReader(); const decoder = new TextDecoder(); let buffer = ""; - // SSE frames are separated by a blank line (`\n\n`). We don't parse + // SSE frames are separated by a blank line — LF- or CRLF-terminated, + // both of which the spec permits and `findSseFrameEnd` accepts (#2006). + // We don't parse // the event type or data — `refreshInternal()` re-fetches the // canonical state regardless — so we just count frames and fire a // single background refresh per decode chunk. Two `change` @@ -183,12 +213,12 @@ export function useServers(opts: UseServersOptions): UseServersResult { if (done) break; buffer += decoder.decode(value, { stream: true }); let sawFrame = false; - let frameEnd = buffer.indexOf("\n\n"); - while (frameEnd !== -1) { - const frame = buffer.slice(0, frameEnd); - buffer = buffer.slice(frameEnd + 2); + let boundary = findSseFrameEnd(buffer); + while (boundary !== null) { + const frame = buffer.slice(0, boundary.end); + buffer = buffer.slice(boundary.end + boundary.separatorLength); if (isSseDataFrame(frame)) sawFrame = true; - frameEnd = buffer.indexOf("\n\n"); + boundary = findSseFrameEnd(buffer); } if (sawFrame) void refreshInternal(true); } From 364d8c9dbc01de49b2c7d28366332e00dd514d23 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 20:23:25 -0400 Subject: [PATCH 02/11] feat(auth): per-server authorization and token URL overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two optional per-server fields — `oauth.authorizationUrl` and `oauth.tokenUrl` — that override the endpoints authorization-server metadata discovery resolved, so a server can be pointed at a development or staging authorization server without changing what it advertises. SDK v2 routes both endpoints through the discovered metadata document and neither through OAuthClientProvider, so the override is applied by wrapping the client's base fetch and patching the metadata document in flight (core/auth/endpointOverrides.ts). Wrapping the base fetch — rather than only the auth fetch — also covers the discovery the SDK runs from inside the transport on the 401/refresh path. The fields round-trip through mcp.json, the /api/servers routes, the runner (CLI/TUI), and the Server Settings → Authorization panel, where a value that is not an absolute http(s) URL is flagged inline; at runtime such a value is dropped with a single warning rather than failing the connection. Signed-off-by: cliffhall --- AGENTS.md | 8 +- README.md | 2 +- clients/web/src/App.tsx | 6 + .../ServerSettingsForm.test.tsx | 76 +++++ .../ServerSettingsForm/ServerSettingsForm.tsx | 52 ++++ .../ServerSettingsModal.test.tsx | 46 +++ .../ServerSettingsModal.tsx | 5 + .../test/core/auth/endpointOverrides.test.ts | 276 ++++++++++++++++++ .../web/src/test/core/client/runner.test.ts | 40 +++ ...torClient-oauth-endpoint-overrides.test.ts | 143 +++++++++ .../src/test/core/mcp/oauthManager.test.ts | 24 ++ .../web/src/test/core/mcp/serverList.test.ts | 61 ++++ .../mcp/remote/servers-route.test.ts | 69 +++++ core/auth/endpointOverrides.ts | 246 ++++++++++++++++ core/auth/index.ts | 10 + core/client/runner.ts | 10 +- core/mcp/inspectorClient.ts | 14 + core/mcp/oauthManager.ts | 18 ++ core/mcp/remote/node/server.ts | 31 +- core/mcp/serverList.ts | 36 +++ core/mcp/types.ts | 39 +++ docs/mcp-server-configuration.md | 6 +- 22 files changed, 1212 insertions(+), 6 deletions(-) create mode 100644 clients/web/src/test/core/auth/endpointOverrides.test.ts create mode 100644 clients/web/src/test/core/mcp/inspectorClient-oauth-endpoint-overrides.test.ts create mode 100644 core/auth/endpointOverrides.ts diff --git a/AGENTS.md b/AGENTS.md index d07e84f4..3deca03b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,13 @@ v2/main/ │ │ # providers.redirectToAuthorization, the only │ │ # seam that sees the SDK-built authorize URL; │ │ # authorization request only, never the token -│ │ # request — #2018) +│ │ # request — #2018; +│ │ # endpointOverrides.ts per-server +│ │ # authorization/token URL overrides — a fetch +│ │ # wrapper that rewrites the discovered AS +│ │ # metadata document, the one seam SDK v2 routes +│ │ # BOTH endpoints through (neither reaches the +│ │ # OAuthClientProvider) — #1906) │ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation) │ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer, │ │ │ # runner-interactive-oauth loopback callback flow) diff --git a/README.md b/README.md index cb57dd35..a9fab0c1 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ inspector/ │ ├── tui/ # TUI client (Ink + React, tsup bundle) │ └── launcher/ # Shared launcher — provides the `mcp-inspector` bin, dispatches to web/cli/tui ├── core/ # Shared code consumed via the `@inspector/core` alias (no package.json) -│ ├── auth/ # OAuth: providers, discovery, storage, mid-session recovery (browser/node/remote backends) +│ ├── auth/ # OAuth: providers, discovery, storage, endpoint overrides, mid-session recovery (browser/node/remote backends) │ ├── client/ # Install-level client config (`client.json`): browser-safe parse/validate + Node load/save, remote backend, secrets │ ├── json/ # JSON + parameter/argument conversion utilities, and the nullable-union │ │ # schema collapse shared by the web and TUI form builders diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 72d8bd1f..5837f27f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -64,6 +64,7 @@ import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSu import { cleanRoots, oauthAuthorizationParamsFromSettings, + oauthEndpointOverridesFromSettings, serializeMcpConfig, } from "@inspector/core/mcp/serverList.js"; import type { ClientConfig } from "@inspector/core/client/types.js"; @@ -2315,12 +2316,16 @@ function App() { const serverAuthorizationParams = savedSettings ? oauthAuthorizationParamsFromSettings(savedSettings) : undefined; + const serverEndpointOverrides = savedSettings + ? oauthEndpointOverridesFromSettings(savedSettings) + : undefined; const oauthFromServer = savedSettings && (savedSettings.oauthClientId || savedSettings.oauthClientSecret || savedSettings.oauthScopes || serverAuthorizationParams || + serverEndpointOverrides || savedSettings.enterpriseManaged) ? { ...(savedSettings.oauthClientId && { @@ -2335,6 +2340,7 @@ function App() { ...(serverAuthorizationParams && { authorizationParams: serverAuthorizationParams, }), + ...serverEndpointOverrides, ...(savedSettings.enterpriseManaged && { enterpriseManaged: true, }), diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index d238228a..ac26d2eb 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -701,6 +701,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); @@ -966,6 +968,76 @@ describe("ServerSettingsForm", () => { expect(screen.getByText(/state, scope are set/)).toBeInTheDocument(); }); + // #1906 — the endpoint overrides ride the same `onOAuthChange` callback as + // the rest of the Authorization section. + it("invokes onOAuthChange when an endpoint override is typed", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + await user.type( + screen.getByRole("textbox", { name: /Authorization URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ authorizationUrl: "h" }), + ); + + onOAuthChange.mockClear(); + await user.type( + screen.getByRole("textbox", { name: /Token URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "h" }), + ); + }); + + it("flags an endpoint override that is not an absolute http(s) URL", () => { + renderWithMantine( + , + ); + expect( + screen.getByText('"/authorize" is not an absolute URL.'), + ).toBeInTheDocument(); + expect(screen.queryByText(/is not an http\(s\) URL/)).toBeNull(); + }); + + it("clears an endpoint override through its clear button", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + const clearButtons = screen.getAllByRole("button", { name: /clear/i }); + await user.click(clearButtons[clearButtons.length - 1]); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "" }), + ); + }); + it("invokes onOAuthChange with the chosen insufficient-scope policy (SEP-2350)", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); @@ -1007,6 +1079,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: true, }); }); @@ -1112,6 +1186,8 @@ describe("ServerSettingsForm", () => { clientSecret: "z", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 276c9cf9..887555c3 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -32,6 +32,7 @@ import { authorizationParamKeyError, isReservedAuthorizationParam, } from "@inspector/core/auth/authorizationParams.js"; +import { oauthEndpointUrlError } from "@inspector/core/auth/endpointOverrides.js"; import { ADVERTISABLE_EXTENSIONS } from "@inspector/core/mcp/extensions.js"; import type { Root } from "@modelcontextprotocol/client"; @@ -478,6 +479,8 @@ export function ServerSettingsForm({ clientSecret: settings.oauthClientSecret ?? "", scopes: settings.oauthScopes ?? "", authorizationParams, + authorizationUrl: settings.oauthAuthorizationUrl ?? "", + tokenUrl: settings.oauthTokenUrl ?? "", enterpriseManaged: settings.enterpriseManaged ?? false, onInsufficientScope: settings.oauthOnInsufficientScope, }; @@ -863,6 +866,55 @@ export function ServerSettingsForm({ + Add Parameter + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthAuthorizationUrl ? ( + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: "", + }) + } + /> + ) : null + } + /> + + onOAuthChange({ + ...currentOAuth(), + tokenUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthTokenUrl ? ( + + onOAuthChange({ ...currentOAuth(), tokenUrl: "" }) + } + /> + ) : null + } + />