Harden MCP add against abort, local shadow, and bad URLs

Live Streamable HTTP transports now pass the dispose signal through
requestInit so OAuth auth() refreshes abort on quit. The add row is
hidden while local mcpServers shadow global settings. HTTP URLs must
be real http(s) hosts without userinfo, and persisted as href.

Author: TheGreatAxios

src/mcp/add-server.test.ts

@@ -3,7 +3,11 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
-import { createGlobalSettingsWriter, persistGlobalHTTPMCPServer } from "./add-server.js";
+import {
+  createGlobalSettingsWriter,
+  isAbsoluteHTTPURL,
+  persistGlobalHTTPMCPServer,
+} from "./add-server.js";
 import { isReadOnlyMcpTool, mcpToolName } from "./tool-name.js";
 
 const dirs: string[] = [];
@@ -59,6 +63,17 @@ describe("persistGlobalHTTPMCPServer", () => {
       ok: false,
       reason: "invalid-url",
     });
+    expect(await persistGlobalHTTPMCPServer(writer, "other", "relative/path")).toEqual({
+      ok: false,
+      reason: "invalid-url",
+    });
+    expect(await persistGlobalHTTPMCPServer(writer, "other", "https://.")).toEqual({
+      ok: false,
+      reason: "invalid-url",
+    });
+    expect(await persistGlobalHTTPMCPServer(writer, "other", "https://user:pass@host/mcp")).toEqual(
+      { ok: false, reason: "invalid-url" },
+    );
     expect(await persistGlobalHTTPMCPServer(writer, "linear", "https://new.test/mcp")).toEqual({
       ok: false,
       reason: "duplicate",
@@ -95,6 +110,32 @@ describe("persistGlobalHTTPMCPServer", () => {
     expect(await readFile(path, "utf8")).toBe(original);
   });
 
+  test("rejects degenerate and credential-bearing HTTP URLs", () => {
+    expect(isAbsoluteHTTPURL("relative/path")).toBe(false);
+    expect(isAbsoluteHTTPURL("ftp://example.test/mcp")).toBe(false);
+    expect(isAbsoluteHTTPURL("https://.")).toBe(false);
+    expect(isAbsoluteHTTPURL("https:example.com")).toBe(false);
+    expect(isAbsoluteHTTPURL("https:////evil.com")).toBe(false);
+    expect(isAbsoluteHTTPURL("https://user:pass@host/mcp")).toBe(false);
+    expect(isAbsoluteHTTPURL("https://mcp.linear.app/mcp")).toBe(true);
+  });
+
+  test("persists the normalized href rather than the typed URL", async () => {
+    const path = await settingsPath();
+    await Bun.write(path, JSON.stringify({ providers: {} }));
+    const writer = createGlobalSettingsWriter(path);
+
+    expect(
+      await persistGlobalHTTPMCPServer(writer, "linear", "https://CUSTOM.example:443/mcp"),
+    ).toMatchObject({
+      ok: true,
+      server: { name: "linear", type: "http", url: "https://custom.example/mcp" },
+    });
+    expect(JSON.parse(await readFile(path, "utf8")).mcpServers).toEqual([
+      { name: "linear", type: "http", url: "https://custom.example/mcp" },
+    ]);
+  });
+
   test("serializes a delayed MCP add with hook and plugin mutation", async () => {
     const path = await settingsPath();
     await writeFile(path, JSON.stringify({ providers: {}, showPromptCost: true }));

src/mcp/add-server.ts

@@ -86,15 +86,29 @@ export function validateMCPServerName(value: string): string | null {
   return null;
 }
 
-export function isAbsoluteHTTPURL(value: string): boolean {
+export function parseAbsoluteHTTPURL(value: string): URL | null {
+  const trimmed = value.trim();
+  const scheme = trimmed.match(/^(https?):\/\//i);
+  if (scheme === null) return null;
+  const afterScheme = trimmed.slice(scheme[0].length);
+  if (afterScheme.startsWith("/")) return null;
   try {
-    const url = new URL(value);
-    return (url.protocol === "http:" || url.protocol === "https:") && url.origin !== "null";
+    const url = new URL(trimmed);
+    if (url.protocol !== "http:" && url.protocol !== "https:") return null;
+    if (url.username !== "" || url.password !== "") return null;
+    if (url.hostname.length === 0 || url.hostname === "." || url.hostname.startsWith(".")) {
+      return null;
+    }
+    return url;
   } catch {
-    return false;
+    return null;
   }
 }
 
+export function isAbsoluteHTTPURL(value: string): boolean {
+  return parseAbsoluteHTTPURL(value) !== null;
+}
+
 export async function persistGlobalHTTPMCPServer(
   writer: GlobalSettingsWriter,
   rawName: string,
@@ -103,12 +117,12 @@ export async function persistGlobalHTTPMCPServer(
   isNameActive: (name: string) => boolean = () => false,
 ): Promise<PersistMCPServerResult> {
   const name = rawName.trim();
-  const url = rawURL.trim();
+  const parsedURL = parseAbsoluteHTTPURL(rawURL);
   if (validateMCPServerName(name) !== null) return { ok: false, reason: "invalid-name" };
-  if (!isAbsoluteHTTPURL(url)) return { ok: false, reason: "invalid-url" };
+  if (parsedURL === null) return { ok: false, reason: "invalid-url" };
   if (source === "local") return { ok: false, reason: "local-shadow" };
 
-  const server: MCPServerConfig = { name, type: "http", url };
+  const server: MCPServerConfig = { name, type: "http", url: parsedURL.href };
   let active = false;
   let duplicate = false;
   const settings = await writer.update((base) => {

src/mcp/client-auth-policy.test.ts

@@ -16,6 +16,24 @@ let toolDiscoveryAborts = 0;
 let blockTokenExchange = false;
 let tokenExchangeSignals: (AbortSignal | null | undefined)[] = [];
 let tokenExchangeAborts = 0;
+let lastTransportAuth: (() => Promise<void>) | undefined;
+let tokenRefreshSignals: (AbortSignal | null | undefined)[] = [];
+let tokenRefreshAborts = 0;
+
+function hangUntilAbort(
+  signal: AbortSignal | null | undefined,
+  onAbort: () => void,
+  fallback: string,
+): Promise<void> {
+  return new Promise((_resolve, reject) => {
+    const fail = (): void => {
+      onAbort();
+      reject(signal?.reason ?? new Error(fallback));
+    };
+    if (signal?.aborted === true) fail();
+    else signal?.addEventListener("abort", fail, { once: true });
+  });
+}
 
 const authProvider = { resetAuthorization: async () => undefined };
 
@@ -45,6 +63,11 @@ await withMockedModule(
         }
         return { tools: [] };
       }
+      async callTool(): Promise<{ content: [] }> {
+        if (lastTransportAuth === undefined) throw new Error("no live HTTP transport");
+        await lastTransportAuth();
+        return { content: [] };
+      }
       async close(): Promise<void> {
         clientCloses += 1;
       }
@@ -62,21 +85,32 @@ await withMockedModule(
         private readonly options?: { requestInit?: RequestInit },
       ) {
         transportOptions.push(options);
+        lastTransportAuth = () => this.auth();
       }
       async finishAuth(): Promise<void> {
         const signal = this.options?.requestInit?.signal;
         tokenExchangeSignals.push(signal);
         if (blockTokenExchange) {
-          await new Promise<void>((_resolve, reject) => {
-            const onAbort = (): void => {
+          await hangUntilAbort(
+            signal,
+            () => {
               tokenExchangeAborts += 1;
-              reject(signal?.reason ?? new Error("token exchange aborted"));
-            };
-            if (signal?.aborted === true) onAbort();
-            else signal?.addEventListener("abort", onAbort, { once: true });
-          });
+            },
+            "token exchange aborted",
+          );
         }
       }
+      async auth(): Promise<void> {
+        const signal = this.options?.requestInit?.signal;
+        tokenRefreshSignals.push(signal);
+        await hangUntilAbort(
+          signal,
+          () => {
+            tokenRefreshAborts += 1;
+          },
+          "token refresh aborted",
+        );
+      }
       get sessionId(): string | undefined {
         return undefined;
       }
@@ -133,6 +167,9 @@ describe("HTTP MCP auth policy", () => {
     blockTokenExchange = false;
     tokenExchangeSignals = [];
     tokenExchangeAborts = 0;
+    lastTransportAuth = undefined;
+    tokenRefreshSignals = [];
+    tokenRefreshAborts = 0;
   });
 
   test("built-in anonymous Exa treats 401 as a normal failure without OAuth machinery", async () => {
@@ -212,6 +249,24 @@ describe("HTTP MCP auth policy", () => {
     expect(callbackCloses).toBe(1);
   });
 
+  test("aborts a live-transport auth refresh that ignores the transport abort controller", async () => {
+    const abort = new AbortController();
+    const result = await connectMCPServer(
+      { name: "linear", type: "http", url: "https://mcp.linear.app/mcp" },
+      { signal: abort.signal },
+    );
+    expect(result.ok).toBe(true);
+    if (!result.ok) return;
+    expect(transportOptions).toEqual([{ authProvider, requestInit: { signal: abort.signal } }]);
+
+    const call = result.client.call("ping", {}, abort.signal);
+    while (tokenRefreshSignals.length === 0) await Promise.resolve();
+    expect(tokenRefreshSignals[0]).toBe(abort.signal);
+    abort.abort(new Error("toolset disposed"));
+    await expect(call).rejects.toThrow("toolset disposed");
+    expect(tokenRefreshAborts).toBe(1);
+  });
+
   test("ordinary HTTP creates endpoint-scoped OAuth and passes it to transport", async () => {
     const result = await connectMCPServer({
       name: "exa",

src/mcp/client.ts

@@ -72,14 +72,25 @@ function isRecoverableAuthError(err: unknown): boolean {
   return err instanceof UnauthorizedError || err instanceof OAuthError;
 }
 
+function streamableHTTPTransportOptions(
+  authProvider: CorbitsOAuthProvider | undefined,
+  signal: AbortSignal | undefined,
+) {
+  if (authProvider === undefined && signal === undefined) return undefined;
+  return {
+    ...(authProvider === undefined ? {} : { authProvider }),
+    ...(signal === undefined ? {} : { requestInit: { signal } }),
+  };
+}
+
 async function completeInteractiveAuth(context: HTTPAuthContext): Promise<void> {
   if (!context.interactive)
     throw new Error("Authorization required but no interactive handler is available.");
   const code = await context.callback.waitForCode(context.signal ?? new AbortController().signal);
-  await new StreamableHTTPClientTransport(context.url, {
-    authProvider: context.authProvider,
-    ...(context.signal === undefined ? {} : { requestInit: { signal: context.signal } }),
-  }).finishAuth(code);
+  await new StreamableHTTPClientTransport(
+    context.url,
+    streamableHTTPTransportOptions(context.authProvider, context.signal),
+  ).finishAuth(code);
 }
 
 /**
@@ -218,7 +229,11 @@ async function connectHttp(
     let authContext: HTTPAuthContext | undefined;
     let makeTransport: () => Transport;
     if (config.oauth === false) {
-      makeTransport = () => new StreamableHTTPClientTransport(url) as unknown as Transport;
+      makeTransport = () =>
+        new StreamableHTTPClientTransport(
+          url,
+          streamableHTTPTransportOptions(undefined, options.signal),
+        ) as unknown as Transport;
     } else {
       callback = await startCallbackServer(config.name);
       const authProvider = await createOAuthProvider({
@@ -229,7 +244,10 @@ async function connectHttp(
         onAuthorizationState: callback.expectState,
       });
       makeTransport = () =>
-        new StreamableHTTPClientTransport(url, { authProvider }) as unknown as Transport;
+        new StreamableHTTPClientTransport(
+          url,
+          streamableHTTPTransportOptions(authProvider, options.signal),
+        ) as unknown as Transport;
       authContext = {
         url,
         authProvider,

src/tui/command-surfaces.test.ts

@@ -596,6 +596,37 @@ describe("mcp surface", () => {
     });
   });
 
+  test("hides the add row while local MCP settings shadow global", async () => {
+    await withShell((shell) => {
+      openCommandSurface(shell, "mcp", {
+        notify: () => {},
+        mcp: {
+          list: () => entries,
+          openAuthURL: () => {},
+          mcpServersSource: "local",
+          addServer: async () => ({ ok: true, message: "should not run" }),
+        },
+      });
+      expect(shell.overlayItems).not.toContain("Add MCP server — Alt+A");
+      expect(shell.overlayItems.at(-1)).toBe("Close mcp");
+      expect(runOverlayAction(shell, altKey("a"))).toBe(false);
+    });
+  });
+
+  test("empty MCP list uses a placeholder distinct from close", async () => {
+    await withShell((shell) => {
+      openCommandSurface(shell, "mcp", {
+        notify: () => {},
+        mcp: { list: () => [], openAuthURL: () => {} },
+      });
+      expect(shell.overlayItems).toEqual([
+        "No MCP servers configured",
+        "Add MCP server — Alt+A",
+        "Close mcp",
+      ]);
+    });
+  });
+
   test("the visible add row opens the same add-server flow", async () => {
     await withShell((shell) => {
       openCommandSurface(shell, "mcp", {