inspector/core/mcp/remote/node/server.ts
Cliff Hall 02182e3b6c
test(core): backfill coverage on inspectorClient + remote server (#1317)
* test(core): backfill coverage on inspectorClient + remote server

Resolves #1310. Drops the two TODO(#1310) exclusions from
clients/web/vite.config.ts (coverage.exclude); both files now pass the
per-file gate (lines >=90, statements >=85, functions >=80, branches >=50).

core/mcp/remote/node/server.ts (88.15 / 78.37 -> 97.29 / 100):
- Extract createTokenAuthProvider to its own file
  (core/mcp/remote/node/tokenAuthProvider.ts) so the 7 no-op
  OAuthClientProvider stubs can be unit-tested directly.
- Add focused integration tests covering the remaining gaps:
  * createTransportNode failure path on POST /api/mcp/connect
  * /api/mcp/send when transport is dead
  * /api/log non-JSON body (catch fallback)
  * forwardLogEvent unknown-level / no-messages / string-first-message
    fallbacks (lines 232, 240-242, 257-265 in the original)
  * /api/fetch missing-url + upstream-throw paths
  * /api/storage POST invalid storeId + invalid JSON body
  * /api/storage GET when the on-disk store is unparseable

core/mcp/inspectorClient.ts (73.48 / 72.09 -> 91.16 / 89.92):
Added a single "Coverage backfill (#1310)" describe block to the existing
integration test, plus a few unit-style cases:
- OAuth getters / setOAuthConfig / authenticate when oauthManager is unset.
- Simple session, roots, subscription, and instructions accessors.
- subscribe / unsubscribe error paths against a server without the
  subscribe capability; happy path against one with it.
- getPrompt + readResourceFromTemplate happy path + expansion-failure.
- setLoggingLevel "Server does not support logging" guard.
- getAppRendererClient (null before connect, cached proxy after).
- OAuth + stdio: authenticate() throws via getServerUrl()'s stdio check.
- listChanged notifications: tools/resources/prompts handlers via the
  fixture add_tool / add_resource / add_prompt tools.
- Misc tiny branches: re-connect no-op, elicit form mode, getTaskCapabilities
  on a tasks-less server, receiver-task TTL cleanup + cancel via the
  private surface (the focused-unit pass the issue describes).
- defensive `if (!this.client) throw` guards: one nullify-and-call test
  exercises 16 of them via a single instance, plus the connect() entry
  guard.

The receiver-task internals test uses (instance as unknown as
{ private fields... }) to drive createReceiverTask / cancelReceiverTask
/ TTL cleanup directly; per the issue, refactoring private helpers to
be testable from a unit test was authorized.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(core): address #1317 PR review feedback

- Rename the misnamed "listChanged notifications" describe in
  inspectorClient.test.ts to "capability detection after connect", since
  the test asserts only on getCapabilities() round-tripping and never
  triggers or observes a notification dispatch. Real dispatch coverage
  lives in the adjacent "list_changed + resourceUpdated notifications"
  block. Kept the resources/prompts fixtures with a note explaining why
  they are required for the server to advertise the matching capabilities.
- Expand the comment on the 250ms wait in remote-transport.test.ts'
  /api/mcp/send dead-transport test so it is clear the >= 500 assertion
  is intentionally permissive across both the dead-transport short-
  circuit and the downstream send-failure branches.
- Add a "keep this in sync" pointer on the nullify() helper so a future
  cleanup that removes any of the defensive `if (!this.client) throw`
  guards in inspectorClient.ts is reminded to drop the matching call here
  rather than silently under-covering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 20:43:55 -04:00

596 lines
18 KiB
TypeScript

/**
* Hono-based remote server for MCP transports.
* Hosts /api/config, /api/mcp/connect, send, events, disconnect, /api/fetch, /api/log, /api/storage/:storeId.
*/
import { randomBytes, timingSafeEqual } from "node:crypto";
import type pino from "pino";
import {
getDefaultStorageDir,
getStoreFilePath,
validateStoreId,
readStoreFile,
writeStoreFile,
deleteStoreFile,
parseStore,
serializeStore,
} from "../../../storage/store-io.js";
import type { LogEvent } from "pino";
import { Hono } from "hono";
import type { Context, Env, Next } from "hono";
import { streamSSE } from "hono/streaming";
import { createTransportNode } from "../../node/transport.js";
import type { RemoteConnectRequest, RemoteSendRequest } from "../types.js";
import type { MCPServerConfig } from "../../types.js";
import { RemoteSession } from "./remote-session.js";
import { createTokenAuthProvider } from "./tokenAuthProvider.js";
import { API_SERVER_ENV_VARS } from "../constants.js";
/**
* Shape of the initial config returned by GET /api/config (defaults for client).
*/
export interface InitialConfigPayload {
defaultCommand?: string;
defaultArgs?: string[];
defaultTransport?: string;
defaultServerUrl?: string;
defaultHeaders?: Record<string, string>;
defaultCwd?: string;
defaultEnvironment: Record<string, string>;
}
export interface RemoteServerOptions {
/** Optional auth token. If not provided, uses API_SERVER_ENV_VARS.AUTH_TOKEN env var or generates one. Ignored when dangerouslyOmitAuth is true. */
authToken?: string;
/**
* When true, do not require x-mcp-remote-auth on API routes.
* Origin validation (allowedOrigins) still applies.
* Set via DANGEROUSLY_OMIT_AUTH env var; not recommended for any exposed deployment.
*/
dangerouslyOmitAuth?: boolean;
/** Optional: validate Origin header against allowed origins (for CORS) */
allowedOrigins?: string[];
/** Optional pino file logger. When set, /api/log forwards received events to it. */
logger?: pino.Logger;
/** Optional storage directory for /api/storage/:storeId. Default: ~/.mcp-inspector/storage */
storageDir?: string;
/** Optional sandbox URL for MCP Apps tab. When set, GET /api/config includes sandboxUrl. */
sandboxUrl?: string;
/** Initial config for GET /api/config. Caller must pass this (e.g. from webServerConfigToInitialPayload(config)). */
initialConfig: InitialConfigPayload;
}
export interface CreateRemoteAppResult {
/** The Hono app */
app: Hono;
/** The auth token (from options, env var, or generated). Returned so caller can embed in client. */
authToken: string;
}
/**
* Hono middleware for origin validation (CORS and DNS rebinding protection).
* Validates Origin header against allowedOrigins if provided.
*/
function createOriginMiddleware(allowedOrigins?: string[]) {
return async (c: Context, next: Next) => {
// If no allowedOrigins configured, skip validation (allow all)
if (!allowedOrigins || allowedOrigins.length === 0) {
await next();
return;
}
const origin = c.req.header("origin");
// Handle CORS preflight requests
if (c.req.method === "OPTIONS") {
if (origin && allowedOrigins.includes(origin)) {
c.header("Access-Control-Allow-Origin", origin);
c.header("Access-Control-Allow-Methods", "GET, POST, DELETE, OPTIONS");
c.header(
"Access-Control-Allow-Headers",
"Content-Type, x-mcp-remote-auth",
);
c.header("Access-Control-Max-Age", "86400"); // 24 hours
return c.body(null, 204);
}
// Invalid origin for preflight - return 403
return c.json(
{
error: "Forbidden",
message:
"Invalid origin. Request blocked to prevent DNS rebinding attacks.",
},
403,
);
}
// For actual requests, validate origin if present
if (origin) {
if (!allowedOrigins.includes(origin)) {
return c.json(
{
error: "Forbidden",
message:
"Invalid origin. Request blocked to prevent DNS rebinding attacks. Configure allowed origins via allowedOrigins option.",
},
403,
);
}
// Set CORS header for allowed origin
c.header("Access-Control-Allow-Origin", origin);
}
// If no origin header (same-origin or non-browser client), allow request
await next();
};
}
/**
* Hono middleware for auth token validation.
* Expects Bearer token format: x-mcp-remote-auth: Bearer <token>
*/
function createAuthMiddleware(authToken: string) {
return async (c: Context, next: Next) => {
const authHeader = c.req.header("x-mcp-remote-auth");
if (!authHeader || !authHeader.startsWith("Bearer ")) {
return c.json(
{
error: "Unauthorized",
message:
"Authentication required. Use the x-mcp-remote-auth header with Bearer token.",
},
401,
);
}
const providedToken = authHeader.substring(7); // Remove 'Bearer ' prefix
const expectedToken = authToken;
// Convert to buffers for timing-safe comparison
const providedBuffer = Buffer.from(providedToken);
const expectedBuffer = Buffer.from(expectedToken);
// Check length first to prevent timing attacks
if (providedBuffer.length !== expectedBuffer.length) {
return c.json(
{
error: "Unauthorized",
message:
"Authentication required. Use the x-mcp-remote-auth header with Bearer token.",
},
401,
);
}
// Perform timing-safe comparison
if (!timingSafeEqual(providedBuffer, expectedBuffer)) {
return c.json(
{
error: "Unauthorized",
message:
"Authentication required. Use the x-mcp-remote-auth header with Bearer token.",
},
401,
);
}
await next();
};
}
function forwardLogEvent(
logger: pino.Logger,
logEvent: Partial<LogEvent>,
): void {
const levelLabel = (logEvent?.level?.label ?? "info").toLowerCase();
const method = (logger as unknown as Record<string, unknown>)[levelLabel];
if (typeof method !== "function") return;
const bindings = Object.assign(
{},
...(Array.isArray(logEvent.bindings) ? logEvent.bindings : []),
);
const messages = Array.isArray(logEvent.messages) ? logEvent.messages : [];
if (messages.length === 0) {
(method as (obj: object) => void).call(logger, bindings);
return;
}
const first = messages[0];
if (typeof first === "object" && first !== null && !Array.isArray(first)) {
const obj = { ...bindings, ...(first as Record<string, unknown>) };
const msg = messages[1];
const args = messages.slice(2);
(method as (obj: object, msg?: unknown, ...args: unknown[]) => void).call(
logger,
obj,
msg,
...args,
);
} else {
const msg = messages[0];
const args = messages.slice(1);
(method as (obj: object, msg?: unknown, ...args: unknown[]) => void).call(
logger,
bindings,
msg,
...args,
);
}
}
export function createRemoteApp(
options: RemoteServerOptions,
): CreateRemoteAppResult {
const dangerouslyOmitAuth = !!options.dangerouslyOmitAuth;
// Determine auth token when auth is enabled: options > env var > generate
const authToken = dangerouslyOmitAuth
? ""
: options.authToken ||
process.env[API_SERVER_ENV_VARS.AUTH_TOKEN] ||
randomBytes(32).toString("hex");
const app = new Hono<Env>();
const sessions = new Map<string, RemoteSession>();
const { logger: fileLogger, allowedOrigins } = options;
const storageDir = options.storageDir ?? getDefaultStorageDir();
// Apply origin validation middleware first (before auth)
// This prevents DNS rebinding attacks by validating Origin header
app.use("*", createOriginMiddleware(allowedOrigins));
// Apply auth middleware unless dangerously omitted
if (!dangerouslyOmitAuth) {
app.use("*", createAuthMiddleware(authToken));
}
app.get("/api/config", (c) => {
const payload = options.sandboxUrl
? { ...options.initialConfig, sandboxUrl: options.sandboxUrl }
: options.initialConfig;
return c.json(payload);
});
app.post("/api/mcp/connect", async (c) => {
let body: RemoteConnectRequest;
try {
body = (await c.req.json()) as RemoteConnectRequest;
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
const config = body.config as MCPServerConfig;
if (!config) {
return c.json({ error: "Missing config" }, 400);
}
const sessionId = crypto.randomUUID();
const session = new RemoteSession(sessionId);
let transport: Awaited<ReturnType<typeof createTransportNode>>["transport"];
try {
// Create authProvider from tokens if provided
const authProvider = createTokenAuthProvider(body.oauthTokens);
const result = createTransportNode(config, {
pipeStderr: true,
onStderr: (entry) => session.onStderr(entry),
onFetchRequest: (entry) => session.onFetchRequest(entry),
authProvider,
});
transport = result.transport;
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
return c.json({ error: `Failed to create transport: ${msg}` }, 500);
}
session.setTransport(transport);
transport.onmessage = (msg) => session.onMessage(msg);
// Track if transport closes/errors during start - this matches local behavior
// If transport.start() throws, we catch it. If it resolves but transport closes immediately,
// we detect that too (process failure after spawn).
let transportFailed = false;
let transportError: string | null = null;
const originalOnclose = transport.onclose;
const originalOnerror = transport.onerror;
// Set up error handlers BEFORE calling start() so we catch failures during start
transport.onerror = (err) => {
transportFailed = true;
transportError = err instanceof Error ? err.message : String(err);
originalOnerror?.(err);
};
transport.onclose = () => {
const session = sessions.get(sessionId);
if (session) {
// Mark transport as dead but don't delete session yet
// We'll notify client via SSE and cleanup when client disconnects
const errorMsg =
transportError || "Transport closed - process may have exited";
session.markTransportDead(errorMsg);
// If no client connected, can cleanup immediately
if (!session.hasEventConsumer()) {
sessions.delete(sessionId);
}
} else {
// Session not created yet - failed during start
transportFailed = true;
transportError =
transportError ||
"Transport closed during start - process may have failed";
}
originalOnclose?.();
};
try {
// transport.start() should throw if process fails to start
// If it resolves, the process should be running
await transport.start();
// Check if transport failed during start (onerror/onclose fired synchronously)
if (transportFailed) {
const errorMsg = transportError || "Transport failed during start";
return c.json({ error: `Failed to start transport: ${errorMsg}` }, 500);
}
} catch (err) {
// transport.start() threw - this is the expected failure path
const msg = err instanceof Error ? err.message : String(err);
// Preserve 401 only when the transport/SDK reports it (no message guessing)
const status =
(err as { code?: number; status?: number }).code ??
(err as { code?: number; status?: number }).status;
const is401 = status === 401;
return c.json(
{ error: `Failed to start transport: ${msg}` },
is401 ? 401 : 500,
);
}
// Transport started successfully - add to sessions
sessions.set(sessionId, session);
return c.json({ sessionId });
});
app.post("/api/mcp/send", async (c) => {
let body: RemoteSendRequest & { sessionId?: string };
try {
body = (await c.req.json()) as RemoteSendRequest & { sessionId?: string };
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
const { sessionId, message, relatedRequestId } = body;
if (!sessionId || !message) {
return c.json({ error: "Missing sessionId or message" }, 400);
}
const session = sessions.get(sessionId);
if (!session) {
return c.json({ error: "Session not found" }, 404);
}
// Check if transport is dead - return error immediately (matches local behavior)
if (session.isTransportDead()) {
const errorMsg = session.getTransportError() || "Transport closed";
return c.json({ error: errorMsg }, 500);
}
try {
await session.transport.send(message, {
relatedRequestId: relatedRequestId as string | number | undefined,
});
return c.json({ ok: true });
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
// Preserve 401 only when the transport/SDK reports it (no message guessing)
const status =
(err as { code?: number; status?: number }).code ??
(err as { code?: number; status?: number }).status;
const is401 = status === 401;
return c.json({ error: msg }, is401 ? 401 : 500);
}
});
app.get("/api/mcp/events", async (c) => {
const sessionId = c.req.query("sessionId");
if (!sessionId) {
return c.json({ error: "Missing sessionId query" }, 400);
}
const session = sessions.get(sessionId);
if (!session) {
return c.json({ error: "Session not found" }, 404);
}
return streamSSE(c, async (stream) => {
session.setEventConsumer((event) => {
const data = JSON.stringify(event);
void stream.writeSSE({
event: event.type,
data,
});
});
stream.onAbort(() => {
// Client disconnected - clear event consumer
const shouldCleanup = session.clearEventConsumer();
stream.close();
// If transport is dead and no client connected, cleanup session
if (shouldCleanup || session.isTransportDead()) {
sessions.delete(sessionId);
}
});
// Keep the stream open until the client disconnects. Hono's streamSSE
// closes the stream when this callback returns, so we must not return
// until the connection is aborted.
await new Promise<void>((resolve) => {
stream.onAbort(() => {
// Cleanup happens in onAbort handler above
resolve();
});
});
});
});
app.post("/api/mcp/disconnect", async (c) => {
let body: { sessionId?: string };
try {
body = (await c.req.json()) as { sessionId?: string };
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
const sessionId = body.sessionId;
if (!sessionId) {
return c.json({ error: "Missing sessionId" }, 400);
}
const session = sessions.get(sessionId);
if (session) {
session.clearEventConsumer();
await session.transport.close();
sessions.delete(sessionId);
}
return c.json({ ok: true });
});
app.post("/api/fetch", async (c) => {
let body: {
url: string;
method?: string;
headers?: Record<string, string>;
body?: string;
};
try {
body = (await c.req.json()) as typeof body;
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
const { url, method = "GET", headers = {}, body: reqBody } = body;
if (!url) {
return c.json({ error: "Missing url" }, 400);
}
try {
const res = await fetch(url, {
method,
headers: new Headers(headers),
body: reqBody,
});
const resHeaders: Record<string, string> = {};
res.headers.forEach((v, k) => {
resHeaders[k] = v;
});
const contentType = res.headers.get("content-type");
const isStream =
contentType?.includes("text/event-stream") ||
contentType?.includes("application/x-ndjson");
let resBody: string | undefined;
if (!isStream && res.body) {
resBody = await res.text();
}
return c.json({
ok: res.ok,
status: res.status,
statusText: res.statusText,
headers: resHeaders,
body: resBody,
});
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
return c.json({ error: msg }, 500);
}
});
app.post("/api/log", async (c) => {
const body = (await c.req.json().catch(() => ({}))) as Partial<LogEvent>;
if (fileLogger) {
forwardLogEvent(fileLogger, body);
}
return c.json({ ok: true });
});
app.get("/api/storage/:storeId", async (c) => {
const storeId = c.req.param("storeId");
if (!storeId || !validateStoreId(storeId)) {
return c.json({ error: "Invalid storeId" }, 400);
}
const filePath = getStoreFilePath(storageDir, storeId);
try {
const raw = await readStoreFile(filePath);
if (raw === null) {
return c.json({}, 200);
}
const store = parseStore(raw);
return c.json(store);
} catch (error) {
const msg = error instanceof Error ? error.message : String(error);
return c.json({ error: `Failed to read store: ${msg}` }, 500);
}
});
app.post("/api/storage/:storeId", async (c) => {
const storeId = c.req.param("storeId");
if (!storeId || !validateStoreId(storeId)) {
return c.json({ error: "Invalid storeId" }, 400);
}
let body: unknown;
try {
body = await c.req.json();
} catch {
return c.json({ error: "Invalid JSON body" }, 400);
}
const filePath = getStoreFilePath(storageDir, storeId);
try {
const jsonData = serializeStore(body);
await writeStoreFile(filePath, jsonData);
return c.json({ ok: true });
} catch (error) {
const msg = error instanceof Error ? error.message : String(error);
return c.json({ error: `Failed to write store: ${msg}` }, 500);
}
});
app.delete("/api/storage/:storeId", async (c) => {
const storeId = c.req.param("storeId");
if (!storeId || !validateStoreId(storeId)) {
return c.json({ error: "Invalid storeId" }, 400);
}
const filePath = getStoreFilePath(storageDir, storeId);
try {
await deleteStoreFile(filePath);
return c.json({ ok: true });
} catch (error) {
const msg = error instanceof Error ? error.message : String(error);
return c.json({ error: `Failed to delete store: ${msg}` }, 500);
}
});
return { app, authToken };
}