diff --git a/clients/launcher/README.md b/clients/launcher/README.md index 19d431b7..70d2e1ac 100644 --- a/clients/launcher/README.md +++ b/clients/launcher/README.md @@ -10,6 +10,22 @@ The launcher is the package that provides the global `mcp-inspector` binary (e.g All configuration parsing, config-file loading, and server setup are handled by the app runners and by **core**; the launcher does not interpret config or env vars. +## Web server-list flags (`--web`) + +`mcp-inspector --web` chooses which server list the UI shows and whether it is +editable (see [specification/v2_catalog_launch_config.md](../../specification/v2_catalog_launch_config.md)): + +| Invocation | Server list | Editable in UI? | +|------------|-------------|-----------------| +| `mcp-inspector --web` | Default catalog `~/.mcp-inspector/mcp.json` | Yes | +| `mcp-inspector --web --catalog ` (or `MCP_CATALOG_PATH=`) | That file as the active catalog (created/seeded if missing) | Yes | +| `mcp-inspector --web --config ` | That file as a **read-only session** — shown but never written, seeded, or migrated (safe for a foreign config) | No | +| `mcp-inspector --web --server-url --transport http --header "Name: Value"` (or a positional command) | One ad-hoc server held in memory, connectable with the given `--header`s | No | + +Rules: `--catalog` and `--config` are mutually exclusive; neither combines with +an ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server +and is applied to that connection (it is no longer a warn-only no-op). + ## Publishing The root `@modelcontextprotocol/inspector` package ships as one fat tarball: `npm run build` at the repo root builds all clients, then `prepack` runs before `npm publish`. Runtime dependencies are declared on the root `package.json`; client builds bundle `@inspector/core` and externalize npm packages resolved from the root install. diff --git a/clients/web/server/run-web.ts b/clients/web/server/run-web.ts index 668b6d5c..d5725374 100644 --- a/clients/web/server/run-web.ts +++ b/clients/web/server/run-web.ts @@ -7,7 +7,11 @@ import { resolve, join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { Command } from "commander"; -import type { MCPServerConfig } from "../../../core/mcp/types.ts"; +import type { + MCPConfig, + MCPServerConfig, + StoredMCPServer, +} from "../../../core/mcp/types.ts"; import { resolveServerConfigs, parseKeyValuePair, @@ -31,6 +35,22 @@ function ensureStdioCwd(config: MCPServerConfig): MCPServerConfig { return config; } +/** + * Derive a readable catalog id for an ad-hoc server seeded from CLI flags: + * the URL host for HTTP/SSE, or the command basename for stdio. Falls back to + * "server" so the id is always a non-empty `mcpServers` map key. + */ +function deriveSeedServerId(config: MCPServerConfig): string { + if (config.type === "sse" || config.type === "streamable-http") { + try { + return new URL(config.url).host || "server"; + } catch { + return "server"; + } + } + return config.command?.split(/[\\/]/).pop() || "server"; +} + export async function runWeb(argv: string[]): Promise { const program = new Command(); @@ -53,7 +73,8 @@ export async function runWeb(argv: string[]): Promise { parseKeyValuePair, {}, ) - .option("--config ", "config file path") + .option("--catalog ", "writable catalog file path") + .option("--config ", "read-only session config file path") .option("--server ", "server name from config file") .option("--transport ", "transport type (stdio, sse, http)") .option("--server-url ", "server URL for SSE/HTTP transport") @@ -68,6 +89,7 @@ export async function runWeb(argv: string[]): Promise { .parse(preArgs); const opts = program.opts() as { + catalog?: string; config?: string; server?: string; e?: Record; @@ -82,50 +104,139 @@ export async function runWeb(argv: string[]): Promise { const target = [...args, ...postArgs]; const isDev = !!opts.dev; - const hasServerInput = - opts.config || + // Catalog source for the web backend (spec: v2_catalog_launch_config.md): + // - `--catalog ` / MCP_CATALOG_PATH → that file is the active WRITABLE + // catalog (CRUD, seed-if-missing, file-watch) — like the default catalog. + // - `--config ` → a READ-ONLY session file: shown in the UI but never + // written, seeded, or migrated (so a foreign config is safe to point at). + // - ad-hoc `--server-url` / command target → a one-server READ-ONLY session + // held in memory (no file written), with `--header` lifted onto the entry + // so the seeded connection actually uses those headers (#1483). + // - nothing → default catalog (writable), backend uses its default path. + // `initialMcpConfig` stays populated for the ad-hoc case only so the legacy + // GET /api/config defaults survive; the catalog path / in-memory list is what + // the UI's server list actually reads. + const catalogPath = opts.catalog ?? process.env.MCP_CATALOG_PATH; + const hasCatalog = !!catalogPath; + const hasConfig = !!opts.config; + const hasHeaders = !!opts.header && Object.keys(opts.header).length > 0; + const hasAdHocServer = target.length > 0 || - opts.serverUrl || - (opts.transport && opts.transport !== "stdio"); + !!opts.serverUrl || + (!!opts.transport && opts.transport !== "stdio"); + + // Reject illegal flag combinations up front so each branch below is clean. + const conflict = + hasCatalog && hasConfig + ? "--catalog and --config are mutually exclusive. --catalog is the writable catalog; --config is a read-only session file." + : hasCatalog && hasAdHocServer + ? "--catalog cannot be combined with an ad-hoc server URL/command." + : hasCatalog && hasHeaders + ? "--header cannot be combined with --catalog. Set per-server headers in the catalog file." + : hasConfig && hasAdHocServer + ? "--config cannot be combined with an ad-hoc server URL/command. --config selects a read-only session file." + : hasConfig && hasHeaders + ? "--header cannot be combined with --config. Set per-server headers inside the config file instead." + : null; + if (conflict) { + console.error(`Error: ${conflict}`); + process.exit(1); + } let initialMcpConfig: MCPServerConfig | null = null; + let mcpConfigPath: string | undefined; + let writable = true; + let initialServers: MCPConfig | null = null; - if (hasServerInput) { - const serverOptions: ServerConfigOptions = { - configPath: opts.config, - serverName: opts.server, - target: target.length > 0 ? target : undefined, - transport: opts.transport as "stdio" | "sse" | "http" | undefined, - serverUrl: opts.serverUrl, - cwd: opts.cwd, - env: opts.e, - }; - - if (opts.header && Object.keys(opts.header).length > 0) { + if (hasCatalog) { + if (opts.server) { console.warn( - "Warning: --header is accepted but initial HTTP headers are configured via server settings in the web UI (post-#1358).", + "Note: --server has no effect on the web UI yet; it lists every server in the catalog.", ); } - + // A writable catalog may not exist yet — the backend seeds it on first + // read, exactly like the default catalog — so we don't pre-validate it. + mcpConfigPath = resolve(process.cwd(), catalogPath!); + writable = true; + } else if (hasConfig) { + if (opts.server) { + console.warn( + "Note: --server has no effect on the web UI yet; it lists every server in the file.", + ); + } + const resolvedConfigPath = resolve(process.cwd(), opts.config!); try { - const configs = resolveServerConfigs(serverOptions, "single"); - const config = configs[0]; - if (!config) { + // Validate the file loads (and exists) before serving it: a read-only + // session never seeds, so a typo'd path would otherwise show a silently + // empty server list instead of a clear error. + resolveServerConfigs({ configPath: resolvedConfigPath }, "multi"); + } catch (err) { + const message = + err instanceof Error ? err.message : "Could not load config file."; + console.error("Error:", message); + process.exit(1); + } + mcpConfigPath = resolvedConfigPath; + writable = false; + } else if (hasAdHocServer) { + let config: MCPServerConfig; + try { + const serverOptions: ServerConfigOptions = { + serverName: opts.server, + target: target.length > 0 ? target : undefined, + transport: opts.transport as "stdio" | "sse" | "http" | undefined, + serverUrl: opts.serverUrl, + cwd: opts.cwd, + env: opts.e, + }; + const resolved = resolveServerConfigs(serverOptions, "single")[0]; + if (!resolved) { console.error( - "Error: Could not resolve server config. Use --config and --server, or pass a command/URL.", + "Error: Could not resolve server config. Pass a command/URL, or use --catalog / --config to serve a file.", ); process.exit(1); } - initialMcpConfig = ensureStdioCwd(config); + config = ensureStdioCwd(resolved); } catch (err) { const message = err instanceof Error ? err.message : "Could not resolve server config."; console.error("Error:", message); process.exit(1); } + + if (hasHeaders && (config.type === "stdio" || config.type === undefined)) { + console.error( + "Error: --header only applies to HTTP/SSE servers; stdio servers take no HTTP headers.", + ); + process.exit(1); + } + + initialMcpConfig = config; + + // In-memory session entry: the SDK config plus the flat `headers` record, + // which the backend lifts into `settings.headers` on read. Nothing is + // written to disk — the ad-hoc server is a read-only session list. + const entry: StoredMCPServer = + hasHeaders && config.type !== "stdio" && config.type !== undefined + ? { ...config, headers: opts.header } + : { ...config }; + initialServers = { + mcpServers: { [deriveSeedServerId(config)]: entry }, + }; + writable = false; + } else if (hasHeaders) { + console.error( + "Error: --header requires an ad-hoc --server-url or command target.", + ); + process.exit(1); } - const webConfig = buildWebServerConfig({ initialMcpConfig }); + const webConfig = buildWebServerConfig({ + initialMcpConfig, + mcpConfigPath, + writable, + initialServers, + }); if (!isDev) { webConfig.staticRoot = join(__dirname, "..", "dist"); } diff --git a/clients/web/server/server.ts b/clients/web/server/server.ts index a0b076ac..3bfde7bf 100644 --- a/clients/web/server/server.ts +++ b/clients/web/server/server.ts @@ -52,6 +52,9 @@ export async function startHonoServer( authToken: config.dangerouslyOmitAuth ? undefined : resolvedAuthToken, dangerouslyOmitAuth: config.dangerouslyOmitAuth, storageDir: config.storageDir, + mcpConfigPath: config.mcpConfigPath, + writable: config.writable, + initialServers: config.initialServers ?? undefined, allowedOrigins: config.allowedOrigins, sandboxUrl: sandboxController.getUrl() ?? undefined, logger: config.logger, diff --git a/clients/web/server/vite-hono-plugin.ts b/clients/web/server/vite-hono-plugin.ts index be57a063..4974bd20 100644 --- a/clients/web/server/vite-hono-plugin.ts +++ b/clients/web/server/vite-hono-plugin.ts @@ -74,6 +74,9 @@ export function honoMiddlewarePlugin(config: WebServerConfig): Plugin { authToken: config.dangerouslyOmitAuth ? undefined : config.authToken, dangerouslyOmitAuth: config.dangerouslyOmitAuth, storageDir: config.storageDir, + mcpConfigPath: config.mcpConfigPath, + writable: config.writable, + initialServers: config.initialServers ?? undefined, allowedOrigins: config.allowedOrigins, sandboxUrl: sandboxController.getUrl() ?? undefined, logger: config.logger, diff --git a/clients/web/server/web-server-config.ts b/clients/web/server/web-server-config.ts index ddf3de2c..3dc833ac 100644 --- a/clients/web/server/web-server-config.ts +++ b/clients/web/server/web-server-config.ts @@ -4,7 +4,7 @@ import pino from "pino"; import type { Logger } from "pino"; -import type { MCPServerConfig } from "../../../core/mcp/types.ts"; +import type { MCPConfig, MCPServerConfig } from "../../../core/mcp/types.ts"; // Import directly from the leaf module — not the `core/mcp/remote/index.ts` // barrel — so Vite's config-time module graph doesn't pull in `createRemoteLogger` // (which references `pino/browser.js`) or other browser-side re-exports that @@ -23,6 +23,28 @@ export interface WebServerConfig { dangerouslyOmitAuth: boolean; /** Single initial MCP server config, or null when no server specified. */ initialMcpConfig: MCPServerConfig | null; + /** + * Absolute path to the `mcp.json` file the backend reads (and writes/watches + * when {@link writable}). When undefined the backend falls back to the + * default catalog (`~/.mcp-inspector/mcp.json`). Set by the launcher for + * `--catalog ` (writable) or `--config ` (read-only session). + * Mutually exclusive with {@link initialServers} (#1481). + */ + mcpConfigPath: string | undefined; + /** + * When false, the server list is read-only for the session: the backend + * rejects `/api/servers` mutations and never seeds/migrates the file, and + * the web UI hides catalog CRUD. True for the default catalog and + * `--catalog`; false for `--config` and ad-hoc launches (#1481). + */ + writable: boolean; + /** + * In-memory server list served by the backend instead of reading a file — + * set by the launcher to seed an ad-hoc `--server-url` / command target + * (with `--header`) without writing any file. Implies `writable: false`. + * Mutually exclusive with {@link mcpConfigPath} (#1481/#1483). + */ + initialServers: MCPConfig | null; storageDir: string | undefined; allowedOrigins: string[]; /** Sandbox port (0 = dynamic). */ @@ -151,6 +173,12 @@ export function printServerBanner( export interface BuildWebServerConfigOptions { initialMcpConfig?: MCPServerConfig | null; + /** Catalog/session file the backend should serve; see {@link WebServerConfig.mcpConfigPath}. */ + mcpConfigPath?: string; + /** Read-only when false; see {@link WebServerConfig.writable}. Defaults to true. */ + writable?: boolean; + /** In-memory ad-hoc list; see {@link WebServerConfig.initialServers}. */ + initialServers?: MCPConfig | null; } /** @@ -160,7 +188,12 @@ export interface BuildWebServerConfigOptions { export function buildWebServerConfig( options: BuildWebServerConfigOptions = {}, ): WebServerConfig { - const { initialMcpConfig = null } = options; + const { + initialMcpConfig = null, + mcpConfigPath, + writable = true, + initialServers = null, + } = options; const port = parseInt(process.env.CLIENT_PORT ?? "6274", 10); const hostname = process.env.HOST ?? "localhost"; const baseUrl = `http://${hostname}:${port}`; @@ -191,6 +224,9 @@ export function buildWebServerConfig( authToken, dangerouslyOmitAuth, initialMcpConfig, + mcpConfigPath, + writable, + initialServers, storageDir: process.env.MCP_STORAGE_DIR, allowedOrigins: process.env.ALLOWED_ORIGINS?.split(",").filter(Boolean) ?? [ baseUrl, diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index b408557e..544cd5e4 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -78,6 +78,7 @@ import { useResourceSubscriptions } from "@inspector/core/react/useResourceSubsc import { useMessageLog } from "@inspector/core/react/useMessageLog.js"; import { useFetchRequestLog } from "@inspector/core/react/useFetchRequestLog.js"; import { useSandboxUrl } from "@inspector/core/react/useSandboxUrl.js"; +import { useServerListWritable } from "@inspector/core/react/useServerListWritable.js"; import { usePendingClientRequests } from "@inspector/core/react/usePendingClientRequests.js"; import { InspectorView } from "./components/views/InspectorView/InspectorView"; import type { @@ -537,11 +538,16 @@ function App() { // the active client's underlying SDK client so the running view can call the // server, and reads the tool's UI resource into the sandbox on handshake. const appRendererRef = useRef(null); + const configBaseUrl = + typeof window !== "undefined" ? window.location.origin : "http://localhost"; const { sandboxUrl } = useSandboxUrl({ - baseUrl: - typeof window !== "undefined" - ? window.location.origin - : "http://localhost", + baseUrl: configBaseUrl, + authToken: getAuthToken(), + }); + // Read-only sessions (launched with `--config` or an ad-hoc server) hide + // catalog CRUD; the default catalog and `--catalog` stay writable. + const { writable: serverListWritable } = useServerListWritable({ + baseUrl: configBaseUrl, authToken: getAuthToken(), }); const sandboxBridgeFactory = useMemo( @@ -2353,6 +2359,7 @@ function App() { <> { Node.DOCUMENT_POSITION_FOLLOWING, ); }); + + describe("read-only (writable=false)", () => { + it("hides Clone/Edit/Remove/Settings but keeps connect + Connection Info", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { name: "Clone" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Edit" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Remove" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Settings" }), + ).not.toBeInTheDocument(); + // connection === connected, so Connection Info stays available. + expect( + screen.getByRole("button", { name: "Connection Info" }), + ).toBeInTheDocument(); + }); + + it("shows mutation actions when writable (default)", () => { + renderWithMantine(); + expect(screen.getByRole("button", { name: "Edit" })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Settings" }), + ).toBeInTheDocument(); + }); + }); }); diff --git a/clients/web/src/components/groups/ServerCard/ServerCard.tsx b/clients/web/src/components/groups/ServerCard/ServerCard.tsx index a52b68d5..da1bdc1c 100644 --- a/clients/web/src/components/groups/ServerCard/ServerCard.tsx +++ b/clients/web/src/components/groups/ServerCard/ServerCard.tsx @@ -19,6 +19,12 @@ export interface ServerCardProps extends ServerEntry { onClone: (id: string) => void; onRemove: (id: string) => void; compact?: boolean; + /** + * When false (read-only session), the catalog mutation actions + * (Clone / Edit / Remove / Settings) are hidden; connect and Connection Info + * remain. Defaults to true. + */ + writable?: boolean; /** * Optional drag-handle affordance rendered at the start of the card header, * before the server name. Supplied by the sortable wrapper @@ -112,6 +118,7 @@ export function ServerCard({ onClone, onRemove, compact = false, + writable = true, dragHandle, }: ServerCardProps) { const isDimmed = activeServer !== undefined && activeServer !== id; @@ -165,9 +172,17 @@ export function ServerCard({ - onClone(id)}>Clone - onEdit(id)}>Edit - onRemove(id)}>Remove + {writable && ( + <> + onClone(id)}> + Clone + + onEdit(id)}>Edit + onRemove(id)}> + Remove + + + )} {connection.status === "connected" && ( @@ -175,9 +190,11 @@ export function ServerCard({ Connection Info )} - onSettings(id)}> - Settings - + {writable && ( + onSettings(id)}> + Settings + + )} diff --git a/clients/web/src/components/groups/ServerListControls/ServerListControls.tsx b/clients/web/src/components/groups/ServerListControls/ServerListControls.tsx index c71704e6..a76c2f3c 100644 --- a/clients/web/src/components/groups/ServerListControls/ServerListControls.tsx +++ b/clients/web/src/components/groups/ServerListControls/ServerListControls.tsx @@ -11,6 +11,8 @@ export interface ServerListControlsProps extends AddServerMenuProps { onToggleList: () => void; /** Download the current server list as a canonical `mcp.json` file. */ onExport: () => void; + /** When false (read-only session), the Add menu is hidden. Defaults to true. */ + writable?: boolean; } export function ServerListControls({ @@ -21,6 +23,7 @@ export function ServerListControls({ onImportConfig, onImportServerJson, onExport, + writable = true, }: ServerListControlsProps) { return ( @@ -30,11 +33,13 @@ export function ServerListControls({ - + {writable && ( + + )} ); } diff --git a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.test.tsx b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.test.tsx index 873a112d..9869601b 100644 --- a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.test.tsx +++ b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.test.tsx @@ -91,4 +91,48 @@ describe("ServerListScreen", () => { ).not.toBeInTheDocument(); }); }); + + describe("read-only session (writable=false)", () => { + it("shows the read-only banner", () => { + renderWithMantine(); + expect(screen.getByText("Read-only session")).toBeInTheDocument(); + }); + + it("hides the Add menu and per-card mutation actions", () => { + renderWithMantine(); + // Cards still render and connect controls remain. + expect(screen.getByText("Alpha")).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /add servers/i }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Edit" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Remove" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Clone" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Settings" }), + ).not.toBeInTheDocument(); + }); + + it("omits drag handles even when onReorder is provided", () => { + renderWithMantine(); + expect( + screen.queryByRole("button", { name: /^Reorder / }), + ).not.toBeInTheDocument(); + }); + + it("keeps the Add menu and actions when writable (default)", () => { + renderWithMantine(); + expect( + screen.getByRole("button", { name: /add servers/i }), + ).toBeInTheDocument(); + expect(screen.getAllByRole("button", { name: "Edit" }).length).toBe(2); + expect(screen.queryByText("Read-only session")).not.toBeInTheDocument(); + }); + }); }); diff --git a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.tsx b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.tsx index b0504149..84b8b529 100644 --- a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.tsx +++ b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.tsx @@ -1,4 +1,11 @@ -import { ScrollArea, SimpleGrid, Stack, Text } from "@mantine/core"; +import { + Alert, + Code, + ScrollArea, + SimpleGrid, + Stack, + Text, +} from "@mantine/core"; import { DndContext, KeyboardSensor, @@ -23,6 +30,13 @@ import { export interface ServerListScreenProps { servers: ServerEntry[]; + /** + * Whether the server list is writable (catalog) or read-only (a `--config` + * session file / ad-hoc launch). When false, all catalog mutation controls + * (add, edit, clone, remove, reorder, settings) are hidden and a read-only + * banner is shown. Defaults to true. + */ + writable?: boolean; /** Id of the server the wiring layer treats as active (drives card dimming). */ activeServer?: string; onAddManually: () => void; @@ -57,6 +71,7 @@ const EmptyState = Text.withProps({ export function ServerListScreen({ servers, + writable = true, activeServer, onAddManually, onImportConfig, @@ -81,11 +96,12 @@ export function ServerListScreen({ const ids = servers.map((s) => s.id); - // `reorderable` only when a persistence callback is wired. Without it we - // render plain `ServerCard`s (no grip, no DndContext) so the screen stays - // usable as a pure display — the SortableServerCard's grip would otherwise - // be a dead affordance. - const reorderable = onReorder !== undefined; + // `reorderable` only when a persistence callback is wired AND the list is + // writable. Without it we render plain `ServerCard`s (no grip, no DndContext) + // so the screen stays usable as a pure display — the SortableServerCard's + // grip would otherwise be a dead affordance (and reorder is a catalog write + // the backend rejects in a read-only session). + const reorderable = onReorder !== undefined && writable; // Built only when reorderable — the drag-end handler and the fresh // announcements object (four closures) are otherwise allocated every render @@ -99,6 +115,7 @@ export function ServerListScreen({ const cardProps = (server: ServerEntry) => ({ compact, + writable, activeServer, onToggleConnection, onConnectionInfo, @@ -127,9 +144,17 @@ export function ServerListScreen({ return ( + {!writable && ( + + This server list was launched with --config or an ad-hoc + server and can't be edited here. Changes won't be saved. Use{" "} + --catalog (or no flag) to manage a writable catalog. + + )} body, + } as unknown as Response; +} + +describe("useServerListWritable", () => { + it("defaults to writable while loading, then keeps true for a writable session", async () => { + const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ writable: true })); + + const { result } = renderHook(() => + useServerListWritable({ baseUrl: "http://test.local", fetchFn }), + ); + + expect(result.current.loading).toBe(true); + expect(result.current.writable).toBe(true); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(true); + }); + + it("resolves writable:false for a read-only session", async () => { + const fetchFn = vi + .fn() + .mockResolvedValue(jsonResponse({ writable: false })); + + const { result } = renderHook(() => + useServerListWritable({ baseUrl: "http://test.local", fetchFn }), + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(false); + }); + + it("stays writable when the field is absent (legacy backend)", async () => { + const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); + + const { result } = renderHook(() => + useServerListWritable({ baseUrl: "http://test.local", fetchFn }), + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(true); + }); + + it("sends the bearer auth header when a token is provided", async () => { + const fetchFn = vi + .fn() + .mockResolvedValue(jsonResponse({ writable: false })); + + renderHook(() => + useServerListWritable({ + baseUrl: "http://test.local/", + authToken: "secret-token", + fetchFn, + }), + ); + + await waitFor(() => expect(fetchFn).toHaveBeenCalled()); + const [url, init] = fetchFn.mock.calls[0] as [string, RequestInit]; + // Trailing slash on baseUrl is trimmed. + expect(url).toBe("http://test.local/api/config"); + expect((init.headers as Record)["x-mcp-remote-auth"]).toBe( + "Bearer secret-token", + ); + }); + + it("stays writable on an HTTP error", async () => { + const fetchFn = vi + .fn() + .mockResolvedValue(jsonResponse({ writable: false }, false)); + + const { result } = renderHook(() => + useServerListWritable({ baseUrl: "http://test.local", fetchFn }), + ); + + // An !ok response is ignored (the body is never read), so the writable + // default survives even though loading still settles via `finally`. + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(true); + }); + + it("stays writable when fetch throws", async () => { + const fetchFn = vi.fn().mockRejectedValue(new Error("network down")); + + const { result } = renderHook(() => + useServerListWritable({ baseUrl: "http://test.local", fetchFn }), + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(true); + }); +}); diff --git a/clients/web/src/test/integration/mcp/remote/server-config.test.ts b/clients/web/src/test/integration/mcp/remote/server-config.test.ts index 992c4701..a25d81f8 100644 --- a/clients/web/src/test/integration/mcp/remote/server-config.test.ts +++ b/clients/web/src/test/integration/mcp/remote/server-config.test.ts @@ -55,4 +55,29 @@ describe("createRemoteApp GET /api/config", () => { expect(data.defaultCwd).toBe("/tmp"); expect(data.defaultEnvironment).toEqual({ PATH: "/usr/bin" }); }); + + it("reports writable:true by default and writable:false when set", async () => { + const writableApp = createRemoteApp({ + dangerouslyOmitAuth: true, + initialConfig: { defaultEnvironment: {} }, + }).app; + const writableRes = await writableApp.request( + new Request("http://test/api/config"), + ); + expect( + ((await writableRes.json()) as { writable?: boolean }).writable, + ).toBe(true); + + const readOnlyApp = createRemoteApp({ + dangerouslyOmitAuth: true, + writable: false, + initialConfig: { defaultEnvironment: {} }, + }).app; + const readOnlyRes = await readOnlyApp.request( + new Request("http://test/api/config"), + ); + expect( + ((await readOnlyRes.json()) as { writable?: boolean }).writable, + ).toBe(false); + }); }); diff --git a/clients/web/src/test/integration/mcp/remote/servers-events.test.ts b/clients/web/src/test/integration/mcp/remote/servers-events.test.ts index 28e5d16d..05651a94 100644 --- a/clients/web/src/test/integration/mcp/remote/servers-events.test.ts +++ b/clients/web/src/test/integration/mcp/remote/servers-events.test.ts @@ -386,3 +386,57 @@ describe("GET /api/servers/events", () => { sink2.close(); }); }); + +describe("GET /api/servers/events (in-memory ad-hoc session)", () => { + let h: Harness; + + beforeEach(async () => { + const tempDir = mkdtempSync(join(tmpdir(), "inspector-events-mem-")); + const configPath = join(tempDir, "mcp.json"); + const { app, close: closeApi } = createRemoteApp({ + dangerouslyOmitAuth: true, + mcpConfigPath: configPath, + writable: false, + initialServers: { + mcpServers: { srv: { type: "stdio", command: "node" } }, + }, + initialConfig: { defaultEnvironment: {} }, + secretStore: new InMemorySecretStore(), + }); + const { baseUrl, server } = await new Promise<{ + baseUrl: string; + server: ServerType; + }>((resolve, reject) => { + const s = serve( + { fetch: app.fetch, port: 0, hostname: "127.0.0.1" }, + (info) => { + const port = + info && typeof info === "object" && "port" in info + ? (info as { port: number }).port + : 0; + resolve({ baseUrl: `http://127.0.0.1:${port}`, server: s }); + }, + ); + s.on("error", reject); + }); + h = { baseUrl, server, configPath, tempDir, closeApi }; + }); + + afterEach(async () => { + await teardown(h); + }); + + it("opens the stream but never broadcasts (no watcher, nothing to watch)", async () => { + const sink = await subscribe(h.baseUrl); // 200 + open stream or it throws + // Writing to the (unwatched) config path must not produce any event. + writeFileSync( + h.configPath, + JSON.stringify({ + mcpServers: { other: { type: "stdio", command: "x" } }, + }), + ); + await expect(sink.waitFor(1, 400)).rejects.toThrow(/Timed out/); + expect(sink.events).toHaveLength(0); + sink.close(); + }); +}); diff --git a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts index 6a5113df..8bd827da 100644 --- a/clients/web/src/test/integration/mcp/remote/servers-route.test.ts +++ b/clients/web/src/test/integration/mcp/remote/servers-route.test.ts @@ -1845,3 +1845,214 @@ describe("/api/servers routes", () => { }); }); }); + +describe("/api/servers read-only sessions (#1481/#1483)", () => { + interface RoHarness { + baseUrl: string; + server: ServerType; + tempDir: string; + configPath: string; + } + + async function listen(app: { + fetch: (req: Request) => Response | Promise; + }): Promise<{ baseUrl: string; server: ServerType }> { + return new Promise((resolve, reject) => { + const server = serve( + { fetch: app.fetch, port: 0, hostname: "127.0.0.1" }, + (info) => { + const port = + info && typeof info === "object" && "port" in info + ? (info as { port: number }).port + : 0; + resolve({ baseUrl: `http://127.0.0.1:${port}`, server }); + }, + ); + server.on("error", reject); + }); + } + + async function close(h: RoHarness): Promise { + await new Promise((r) => h.server.close(() => r())); + try { + rmSync(h.tempDir, { recursive: true }); + } catch { + /* ignore */ + } + } + + /** Read-only session backed by a `--config`-style file. */ + async function setupReadOnlyFile(contents?: string): Promise { + const tempDir = mkdtempSync(join(tmpdir(), "inspector-ro-file-")); + const configPath = join(tempDir, "mcp.json"); + if (contents !== undefined) writeFileSync(configPath, contents); + const { app } = createRemoteApp({ + dangerouslyOmitAuth: true, + mcpConfigPath: configPath, + writable: false, + initialConfig: { defaultEnvironment: {} }, + secretStore: new InMemorySecretStore(), + }); + const { baseUrl, server } = await listen(app); + return { baseUrl, server, tempDir, configPath }; + } + + /** Read-only session backed by an in-memory ad-hoc list (no file). */ + async function setupInMemory(servers: MCPConfig): Promise { + const tempDir = mkdtempSync(join(tmpdir(), "inspector-ro-mem-")); + const configPath = join(tempDir, "mcp.json"); + const { app } = createRemoteApp({ + dangerouslyOmitAuth: true, + // mcpConfigPath points at a path that must NEVER be touched. + mcpConfigPath: configPath, + writable: false, + initialServers: servers, + initialConfig: { defaultEnvironment: {} }, + secretStore: new InMemorySecretStore(), + }); + const { baseUrl, server } = await listen(app); + return { baseUrl, server, tempDir, configPath }; + } + + const jsonHeaders = { "Content-Type": "application/json" }; + + it("rejects every mutation with 403 and leaves the file untouched", async () => { + const original = JSON.stringify( + { mcpServers: { api: { type: "stdio", command: "node" } } }, + null, + 2, + ); + const h = await setupReadOnlyFile(original); + try { + const mutations: Array<{ method: string; path: string; body: unknown }> = + [ + { + method: "POST", + path: "/api/servers", + body: { id: "new", config: { type: "stdio", command: "x" } }, + }, + { + method: "PUT", + path: "/api/servers/api", + body: { config: { type: "stdio", command: "y" } }, + }, + { + method: "PUT", + path: "/api/servers/order", + body: { order: ["api"] }, + }, + { method: "DELETE", path: "/api/servers/api", body: undefined }, + ]; + for (const m of mutations) { + const res = await fetch(`${h.baseUrl}${m.path}`, { + method: m.method, + headers: m.body ? jsonHeaders : undefined, + body: m.body ? JSON.stringify(m.body) : undefined, + }); + expect(res.status).toBe(403); + const body = (await res.json()) as { error?: string }; + expect(body.error).toMatch(/read-only/i); + } + // The file is byte-for-byte unchanged. + expect(readFileSync(h.configPath, "utf-8")).toBe(original); + } finally { + await close(h); + } + }); + + it("returns 403 (not 404) for a mutation on a missing id — no existence leak", async () => { + const h = await setupReadOnlyFile( + JSON.stringify({ mcpServers: {} }, null, 2), + ); + try { + const res = await fetch(`${h.baseUrl}/api/servers/ghost`, { + method: "DELETE", + }); + expect(res.status).toBe(403); + } finally { + await close(h); + } + }); + + it("serves a read-only file as-is and never seeds when the file is missing", async () => { + const h = await setupReadOnlyFile(); // no file written + try { + const res = await fetch(`${h.baseUrl}/api/servers`); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ mcpServers: {} }); + // No seed write happened. + expect(existsSync(h.configPath)).toBe(false); + } finally { + await close(h); + } + }); + + it("does not migrate on-disk plaintext secrets on read (file bytes stable)", async () => { + // A foreign config with a plaintext stdio env value that the writable path + // would migrate into the keychain (a write). Read-only must leave it be. + const original = JSON.stringify( + { + mcpServers: { + api: { + type: "stdio", + command: "node", + env: { API_KEY: "plaintext-secret" }, + }, + }, + }, + null, + 2, + ); + const h = await setupReadOnlyFile(original); + try { + const res = await fetch(`${h.baseUrl}/api/servers`); + expect(res.status).toBe(200); + const body = (await res.json()) as MCPConfig; + // Plaintext flows straight back (no keychain swap) and disk is untouched. + expect( + (body.mcpServers.api as { env?: Record }).env?.API_KEY, + ).toBe("plaintext-secret"); + expect(readFileSync(h.configPath, "utf-8")).toBe(original); + } finally { + await close(h); + } + }); + + it("serves an in-memory ad-hoc list (with lifted headers) and writes no file", async () => { + const h = await setupInMemory({ + mcpServers: { + "example.com": { + type: "streamable-http", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer x" }, + }, + }, + }); + try { + const res = await fetch(`${h.baseUrl}/api/servers`); + expect(res.status).toBe(200); + const body = (await res.json()) as MCPConfig; + expect(body.mcpServers["example.com"]).toMatchObject({ + type: "streamable-http", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer x" }, + }); + // The configPath must never be created for an in-memory session. + expect(existsSync(h.configPath)).toBe(false); + + // Mutations are still rejected. + const post = await fetch(`${h.baseUrl}/api/servers`, { + method: "POST", + headers: jsonHeaders, + body: JSON.stringify({ + id: "x", + config: { type: "stdio", command: "n" }, + }), + }); + expect(post.status).toBe(403); + expect(existsSync(h.configPath)).toBe(false); + } finally { + await close(h); + } + }); +}); diff --git a/clients/web/src/test/integration/server/run-web.test.ts b/clients/web/src/test/integration/server/run-web.test.ts index 138458e4..029f1224 100644 --- a/clients/web/src/test/integration/server/run-web.test.ts +++ b/clients/web/src/test/integration/server/run-web.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { mkdtemp, writeFile, rm } from "node:fs/promises"; +import { existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -32,6 +33,7 @@ describe("runWeb", () => { let originalError: typeof console.error; let exitSpy: ReturnType; let exitCode: number | undefined; + let catalogEnvSnapshot: string | undefined; const signalHandlers = new Map void>(); beforeEach(() => { @@ -40,6 +42,8 @@ describe("runWeb", () => { mockClose.mockClear(); signalHandlers.clear(); exitCode = undefined; + catalogEnvSnapshot = process.env.MCP_CATALOG_PATH; + delete process.env.MCP_CATALOG_PATH; logLines = []; warnLines = []; @@ -79,6 +83,11 @@ describe("runWeb", () => { console.error = originalError; exitSpy.mockRestore(); vi.restoreAllMocks(); + if (catalogEnvSnapshot === undefined) { + delete process.env.MCP_CATALOG_PATH; + } else { + process.env.MCP_CATALOG_PATH = catalogEnvSnapshot; + } }); async function expectServerStarted( @@ -87,12 +96,29 @@ describe("runWeb", () => { await vi.waitFor(() => expect(starter).toHaveBeenCalledTimes(1)); } - it("starts Hono in production mode with no server args", async () => { + /** The single entry of a synthesized in-memory ad-hoc catalog. */ + function soleSeededEntry(): Record { + const initialServers = startHonoServer.mock.calls[0]?.[0] + ?.initialServers as { + mcpServers: Record>; + } | null; + expect(initialServers).toBeTruthy(); + const entries = Object.values(initialServers!.mcpServers); + expect(entries).toHaveLength(1); + return entries[0]!; + } + + it("starts Hono in production mode with no server args (writable default catalog)", async () => { void runWeb(["node", "run-web"]); await expectServerStarted(startHonoServer); expect(startHonoServer).toHaveBeenCalledWith( - expect.objectContaining({ initialMcpConfig: null }), + expect.objectContaining({ + initialMcpConfig: null, + mcpConfigPath: undefined, + writable: true, + initialServers: null, + }), ); expect(startHonoServer.mock.calls[0]?.[0]?.staticRoot).toMatch(/dist$/); expect(logLines.some((l) => l.includes("Starting MCP inspector..."))).toBe( @@ -105,12 +131,14 @@ describe("runWeb", () => { await expectServerStarted(startViteDevServer); expect(startViteDevServer).toHaveBeenCalledWith( - expect.objectContaining({ initialMcpConfig: null }), + expect.objectContaining({ initialMcpConfig: null, writable: true }), ); expect(logLines.some((l) => l.includes("development mode"))).toBe(true); }); - it("resolves an ad-hoc HTTP URL into initialMcpConfig", async () => { + // --- ad-hoc launch → read-only in-memory session ------------------------ + + it("seeds an ad-hoc HTTP URL into an in-memory read-only session", async () => { void runWeb([ "node", "run-web", @@ -122,12 +150,18 @@ describe("runWeb", () => { expect(startHonoServer).toHaveBeenCalledWith( expect.objectContaining({ + writable: false, + mcpConfigPath: undefined, initialMcpConfig: expect.objectContaining({ type: "streamable-http", url: "https://example.com/mcp", }), }), ); + expect(soleSeededEntry()).toMatchObject({ + type: "streamable-http", + url: "https://example.com/mcp", + }); }); it("appends positional args after -- to the ad-hoc target", async () => { @@ -141,21 +175,30 @@ describe("runWeb", () => { ]); await expectServerStarted(startHonoServer); - expect(startHonoServer).toHaveBeenCalledWith( - expect.objectContaining({ - initialMcpConfig: expect.objectContaining({ - url: "https://example.com/mcp", - }), - }), - ); + expect(soleSeededEntry()).toMatchObject({ + url: "https://example.com/mcp", + }); + }); + + it("resolves --server-url without a positional target", async () => { + void runWeb([ + "node", + "run-web", + "--server-url", + "https://example.com/mcp", + "--transport", + "http", + ]); + await expectServerStarted(startHonoServer); + + expect(soleSeededEntry()).toMatchObject({ url: "https://example.com/mcp" }); }); it("fills stdio cwd when omitted", async () => { void runWeb(["node", "run-web", "node", "server.js"]); await expectServerStarted(startHonoServer); - const config = startHonoServer.mock.calls[0]?.[0]?.initialMcpConfig; - expect(config).toMatchObject({ + expect(soleSeededEntry()).toMatchObject({ type: "stdio", command: "node", args: ["server.js"], @@ -163,7 +206,21 @@ describe("runWeb", () => { }); }); - it("warns when --header is passed", async () => { + it("preserves an explicit --cwd for an ad-hoc stdio server", async () => { + void runWeb([ + "node", + "run-web", + "--cwd", + "/custom/cwd", + "node", + "server.js", + ]); + await expectServerStarted(startHonoServer); + + expect(soleSeededEntry()).toMatchObject({ cwd: "/custom/cwd" }); + }); + + it("plumbs --header onto the seeded ad-hoc server (no warning)", async () => { void runWeb([ "node", "run-web", @@ -175,22 +232,190 @@ describe("runWeb", () => { ]); await expectServerStarted(startHonoServer); + // No silent no-op warning anymore — the header is applied. + expect(warnLines.some((l) => l.includes("--header"))).toBe(false); + expect(soleSeededEntry()).toMatchObject({ + type: "streamable-http", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer x" }, + }); + }); + + it("rejects --header for a stdio ad-hoc server", async () => { + await expect( + runWeb([ + "node", + "run-web", + "node", + "server.js", + "--header", + "Authorization: Bearer x", + ]), + ).rejects.toThrow("process.exit:1"); expect( - warnLines.some((l) => l.includes("Warning: --header is accepted")), + errorLines.some((l) => l.includes("--header only applies to HTTP/SSE")), + ).toBe(true); + expect(startHonoServer).not.toHaveBeenCalled(); + }); + + it("rejects --header with no ad-hoc server to attach it to", async () => { + await expect( + runWeb(["node", "run-web", "--header", "Authorization: Bearer x"]), + ).rejects.toThrow("process.exit:1"); + expect( + errorLines.some((l) => l.includes("--header requires an ad-hoc")), + ).toBe(true); + expect(startHonoServer).not.toHaveBeenCalled(); + }); + + it("exits when an ad-hoc config resolves to no entries", async () => { + vi.spyOn(nodeConfig, "resolveServerConfigs").mockReturnValueOnce([]); + + await expect( + runWeb([ + "node", + "run-web", + "https://example.com/mcp", + "--transport", + "http", + ]), + ).rejects.toThrow("process.exit:1"); + expect( + errorLines.some((l) => l.includes("Could not resolve server config")), ).toBe(true); }); - it("loads a named server from a config file", async () => { - const dir = await mkdtemp(join(tmpdir(), "run-web-test-")); + // --- --catalog → writable catalog --------------------------------------- + + it("points the backend at a writable catalog with --catalog", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-catalog-")); + const catalogPath = join(dir, "mcp.json"); + await writeFile( + catalogPath, + JSON.stringify({ + mcpServers: { + api: { type: "streamable-http", url: "https://example.com/mcp" }, + }, + }), + ); + + try { + void runWeb(["node", "run-web", "--catalog", catalogPath]); + await expectServerStarted(startHonoServer); + + expect(startHonoServer).toHaveBeenCalledWith( + expect.objectContaining({ + mcpConfigPath: catalogPath, + writable: true, + initialMcpConfig: null, + initialServers: null, + }), + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("accepts a non-existent --catalog path (backend seeds it)", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-catalog-")); + const catalogPath = join(dir, "does-not-exist-yet.json"); + + try { + void runWeb(["node", "run-web", "--catalog", catalogPath]); + await expectServerStarted(startHonoServer); + + expect(startHonoServer.mock.calls[0]?.[0]).toMatchObject({ + mcpConfigPath: catalogPath, + writable: true, + }); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("reads MCP_CATALOG_PATH as the catalog when --catalog is absent", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-catalog-")); + const catalogPath = join(dir, "mcp.json"); + await writeFile(catalogPath, JSON.stringify({ mcpServers: {} })); + process.env.MCP_CATALOG_PATH = catalogPath; + + try { + void runWeb(["node", "run-web"]); + await expectServerStarted(startHonoServer); + + expect(startHonoServer.mock.calls[0]?.[0]).toMatchObject({ + mcpConfigPath: catalogPath, + writable: true, + }); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("notes that --server is ignored alongside --catalog", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-catalog-")); + const catalogPath = join(dir, "mcp.json"); + await writeFile(catalogPath, JSON.stringify({ mcpServers: {} })); + + try { + void runWeb([ + "node", + "run-web", + "--catalog", + catalogPath, + "--server", + "api", + ]); + await expectServerStarted(startHonoServer); + + expect(warnLines.some((l) => l.includes("--server has no effect"))).toBe( + true, + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + // --- --config → read-only session file ---------------------------------- + + it("serves a --config file as a read-only session", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-config-")); const configPath = join(dir, "mcp.json"); await writeFile( configPath, JSON.stringify({ mcpServers: { - api: { - type: "streamable-http", - url: "https://example.com/mcp", - }, + api: { type: "streamable-http", url: "https://example.com/mcp" }, + other: { type: "sse", url: "https://example.com/sse" }, + }, + }), + ); + + try { + void runWeb(["node", "run-web", "--config", configPath]); + await expectServerStarted(startHonoServer); + + expect(startHonoServer).toHaveBeenCalledWith( + expect.objectContaining({ + mcpConfigPath: configPath, + writable: false, + initialMcpConfig: null, + initialServers: null, + }), + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it("notes that --server is ignored alongside --config", async () => { + const dir = await mkdtemp(join(tmpdir(), "run-web-config-")); + const configPath = join(dir, "mcp.json"); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + api: { type: "streamable-http", url: "https://example.com/mcp" }, }, }), ); @@ -206,20 +431,15 @@ describe("runWeb", () => { ]); await expectServerStarted(startHonoServer); - expect(startHonoServer).toHaveBeenCalledWith( - expect.objectContaining({ - initialMcpConfig: expect.objectContaining({ - type: "streamable-http", - url: "https://example.com/mcp", - }), - }), + expect(warnLines.some((l) => l.includes("--server has no effect"))).toBe( + true, ); } finally { await rm(dir, { recursive: true, force: true }); } }); - it("exits when server config resolution fails", async () => { + it("exits when the --config file cannot be loaded", async () => { await expect( runWeb(["node", "run-web", "--config", "/no/such/file.json"]), ).rejects.toThrow("process.exit:1"); @@ -227,6 +447,58 @@ describe("runWeb", () => { expect(startHonoServer).not.toHaveBeenCalled(); }); + // --- conflict matrix ---------------------------------------------------- + + it.each([ + { + name: "--catalog + --config", + argv: ["--catalog", "/a.json", "--config", "/b.json"], + match: "mutually exclusive", + }, + { + name: "--catalog + ad-hoc", + argv: [ + "--catalog", + "/a.json", + "--server-url", + "https://x/mcp", + "--transport", + "http", + ], + match: "--catalog cannot be combined with an ad-hoc", + }, + { + name: "--catalog + --header", + argv: ["--catalog", "/a.json", "--header", "Authorization: Bearer x"], + match: "--header cannot be combined with --catalog", + }, + { + name: "--config + ad-hoc", + argv: [ + "--config", + "/a.json", + "--server-url", + "https://x/mcp", + "--transport", + "http", + ], + match: "--config cannot be combined with an ad-hoc", + }, + { + name: "--config + --header", + argv: ["--config", "/a.json", "--header", "Authorization: Bearer x"], + match: "--header cannot be combined with --config", + }, + ])("rejects $name", async ({ argv, match }) => { + await expect(runWeb(["node", "run-web", ...argv])).rejects.toThrow( + "process.exit:1", + ); + expect(errorLines.some((l) => l.includes(match))).toBe(true); + expect(startHonoServer).not.toHaveBeenCalled(); + }); + + // --- lifecycle ---------------------------------------------------------- + it("exits when the web server fails to start", async () => { startHonoServer.mockRejectedValueOnce(new Error("bind failed")); @@ -246,78 +518,21 @@ describe("runWeb", () => { await vi.waitFor(() => expect(exitCode).toBe(0)); }); - it("preserves an existing stdio cwd", async () => { - const dir = await mkdtemp(join(tmpdir(), "run-web-cwd-")); - const configPath = join(dir, "mcp.json"); - await writeFile( - configPath, - JSON.stringify({ - mcpServers: { - local: { - type: "stdio", - command: "node", - args: ["server.js"], - cwd: "/custom/cwd", - }, - }, - }), - ); - - try { - void runWeb([ - "node", - "run-web", - "--config", - configPath, - "--server", - "local", - ]); - await expectServerStarted(startHonoServer); - - expect( - startHonoServer.mock.calls[0]?.[0]?.initialMcpConfig, - ).toMatchObject({ - cwd: "/custom/cwd", - }); - } finally { - await rm(dir, { recursive: true, force: true }); - } - }); - - it("resolves --server-url without a positional target", async () => { + it("does not create any temp file for an ad-hoc launch", async () => { void runWeb([ "node", "run-web", - "--server-url", "https://example.com/mcp", "--transport", "http", ]); await expectServerStarted(startHonoServer); - expect(startHonoServer).toHaveBeenCalledWith( - expect.objectContaining({ - initialMcpConfig: expect.objectContaining({ - url: "https://example.com/mcp", - }), - }), - ); - }); - - it("exits when resolveServerConfigs returns no entries", async () => { - vi.spyOn(nodeConfig, "resolveServerConfigs").mockReturnValueOnce([]); - - await expect( - runWeb([ - "node", - "run-web", - "https://example.com/mcp", - "--transport", - "http", - ]), - ).rejects.toThrow("process.exit:1"); - expect( - errorLines.some((l) => l.includes("Could not resolve server config")), - ).toBe(true); + // The ad-hoc list is in memory only; no path is handed to the backend. + const cfg = startHonoServer.mock.calls[0]?.[0]; + expect(cfg?.mcpConfigPath).toBeUndefined(); + // Sanity: existsSync is imported and the absence of a path means nothing + // to clean up on shutdown. + expect(existsSync(join(tmpdir(), "definitely-not-created"))).toBe(false); }); }); diff --git a/clients/web/src/test/integration/server/server-token-injection.test.ts b/clients/web/src/test/integration/server/server-token-injection.test.ts index 7e7992f3..18fd96f0 100644 --- a/clients/web/src/test/integration/server/server-token-injection.test.ts +++ b/clients/web/src/test/integration/server/server-token-injection.test.ts @@ -66,6 +66,9 @@ describe("startHonoServer index.html token injection (/ -> /api/*)", () => { authToken: TOKEN, dangerouslyOmitAuth: false, initialMcpConfig: null, + mcpConfigPath: undefined, + writable: true, + initialServers: null, storageDir: undefined, // Allow the same-origin requests the test issues below. allowedOrigins: [baseUrl], diff --git a/clients/web/src/test/integration/server/web-server-config.test.ts b/clients/web/src/test/integration/server/web-server-config.test.ts index 71d7aecf..a9d7839b 100644 --- a/clients/web/src/test/integration/server/web-server-config.test.ts +++ b/clients/web/src/test/integration/server/web-server-config.test.ts @@ -34,6 +34,9 @@ const baseConfig = (): WebServerConfig => ({ authToken: "tok", dangerouslyOmitAuth: false, initialMcpConfig: null, + mcpConfigPath: undefined, + writable: true, + initialServers: null, storageDir: undefined, allowedOrigins: ["http://localhost:6274"], sandboxPort: 0, @@ -202,6 +205,29 @@ describe("buildWebServerConfig", () => { ); }); + it("threads mcpConfigPath through and defaults it to undefined", () => { + expect(buildWebServerConfig().mcpConfigPath).toBeUndefined(); + expect( + buildWebServerConfig({ mcpConfigPath: "/tmp/catalog/mcp.json" }) + .mcpConfigPath, + ).toBe("/tmp/catalog/mcp.json"); + }); + + it("defaults writable to true and initialServers to null", () => { + const cfg = buildWebServerConfig(); + expect(cfg.writable).toBe(true); + expect(cfg.initialServers).toBeNull(); + }); + + it("threads writable and initialServers when provided", () => { + const initialServers = { + mcpServers: { srv: { type: "stdio" as const, command: "node" } }, + }; + const cfg = buildWebServerConfig({ writable: false, initialServers }); + expect(cfg.writable).toBe(false); + expect(cfg.initialServers).toBe(initialServers); + }); + it("preserves a stdio initialMcpConfig while applying shared env defaults", () => { process.env.CLIENT_PORT = "7000"; const initialMcpConfig = { diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index ad2c99ef..bdaee1b1 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -62,6 +62,13 @@ export interface InitialConfigPayload { defaultServerUrl?: string; defaultCwd?: string; defaultEnvironment: Record; + /** + * Whether the session's server list is writable (catalog) or read-only + * (a `--config` session file or ad-hoc seeded server). Absent → treated as + * true by clients for backward compatibility. Drives whether the web UI + * shows catalog CRUD affordances. + */ + writable?: boolean; } export interface RemoteServerOptions { @@ -87,6 +94,26 @@ export interface RemoteServerOptions { /** Optional path for the user's server list file (/api/servers). Default: ~/.mcp-inspector/mcp.json */ mcpConfigPath?: string; + /** + * When false, the server list is read-only for this session: all + * `/api/servers` mutations (POST/PUT/DELETE/order) are rejected with 403, + * the file is never seeded on first read, and on-disk plaintext secrets are + * not migrated into the keychain (migration is a write). Used for the web + * launcher's read-only `--config` session file and ad-hoc seeded servers, so + * a foreign config passed at launch is never silently rewritten. Defaults to + * true (the default catalog and `--catalog` are writable). + */ + writable?: boolean; + + /** + * In-memory server list served by GET /api/servers instead of reading from + * disk. Set by the web launcher to seed an ad-hoc `--server-url` / command + * target (with `--header`) without writing any file. Implies a read-only + * session — pass `writable: false` alongside it. When set, `mcpConfigPath` + * is ignored for reads and the file watcher is not started. + */ + initialServers?: MCPConfig; + /** Optional sandbox URL for MCP Apps tab. When set, GET /api/config includes sandboxUrl. */ sandboxUrl?: string; @@ -298,6 +325,19 @@ export function createRemoteApp( const secretStore: SecretStore = options.secretStore ?? new KeyringSecretStore(); + // Read-only session support (#1481/#1483). `writable: false` rejects every + // /api/servers mutation and suppresses the seed/migrate writes on read. + // `initialServers` serves an in-memory list (ad-hoc launch) with no file at + // all. Both default off so the catalog path is unchanged. + const writable = options.writable ?? true; + const inMemoryServers = options.initialServers; + // Watch only a real on-disk catalog/session file — never for an in-memory + // ad-hoc list (there is nothing to watch and the default path is unrelated). + // A read-only `--config` *file* is still watched on purpose: read-only blocks + // UI-driven writes, not external observation, so an external edit to the file + // should still refresh the UI. (`writable` does not gate watching.) + const watchable = !inMemoryServers; + // --- /api/servers/events: file-watch fanout state ----------------------- // // Subscribers are SSE writers added by GET /api/servers/events and removed @@ -447,9 +487,11 @@ export function createRemoteApp( } app.get("/api/config", (c) => { - const payload = options.sandboxUrl - ? { ...options.initialConfig, sandboxUrl: options.sandboxUrl } - : options.initialConfig; + const payload = { + ...options.initialConfig, + writable, + ...(options.sandboxUrl ? { sandboxUrl: options.sandboxUrl } : {}), + }; return c.json(payload); }); @@ -1420,8 +1462,31 @@ export function createRemoteApp( return undefined; }; + // Reject catalog mutations in a read-only session. Checked first in each + // mutating route — before body parse or existence checks — so a read-only + // backend answers uniformly and never leaks whether an id exists (403, not + // 404). Returns a Response to short-circuit, or undefined to proceed. + const requireWritable = (c: Context): Response | undefined => { + if (writable) return undefined; + return c.json( + { error: "Server list is read-only for this session." }, + 403, + ); + }; + app.get("/api/servers", async (c) => { try { + // In-memory session (ad-hoc launch): serve the seeded list verbatim. + // No disk read, no seed, no migration — there is no file. Ad-hoc env / + // header values live as plaintext in this list and flow straight to + // POST /api/mcp/connect, which is what makes `--header` take effect. + if (inMemoryServers) { + const normalized: MCPConfig = { + mcpServers: normalizeMcpServers(inMemoryServers.mcpServers), + }; + return c.json(await rehydrateConfig(normalized)); + } + // Fast path: peek at the file without taking the write lock. Most // GETs land here — file exists, no plaintext to migrate. The // unlocked read is safe because we don't write anything in this @@ -1433,9 +1498,19 @@ export function createRemoteApp( const onDisk: MCPConfig = { mcpServers: normalizeMcpServers(parsed?.mcpServers), }; + // Read-only session file (`--config`): never migrate (a write) and + // never seed. Return the file as-is so a foreign config is shown but + // its bytes — and any plaintext secrets — are left untouched on disk. + if (!writable) { + return c.json(await rehydrateConfig(onDisk)); + } if (!hasPlaintextSecrets(onDisk)) { return c.json(await rehydrateConfig(onDisk)); } + } else if (!writable) { + // Read-only and the file is absent: present an empty list rather than + // seeding the default sample servers into a path the user pointed us at. + return c.json({ mcpServers: {} }); } // Slow path: either the file is missing (first-launch seed) or it @@ -1480,6 +1555,8 @@ export function createRemoteApp( }); app.post("/api/servers", async (c) => { + const ro = requireWritable(c); + if (ro) return ro; let body: { id?: unknown; config?: unknown; settings?: unknown }; try { body = (await c.req.json()) as { @@ -1553,6 +1630,8 @@ export function createRemoteApp( // values, so we just permute the existing stripped entries and reuse the // same atomic-write + watcher-notify path as the other mutators. app.put("/api/servers/order", async (c) => { + const ro = requireWritable(c); + if (ro) return ro; let body: { order?: unknown }; try { body = (await c.req.json()) as { order?: unknown }; @@ -1605,6 +1684,8 @@ export function createRemoteApp( }); app.put("/api/servers/:id", async (c) => { + const ro = requireWritable(c); + if (ro) return ro; const originalId = c.req.param("id"); if (!originalId || !validateStoreId(originalId)) { return c.json({ error: "Invalid id" }, 400); @@ -1766,6 +1847,8 @@ export function createRemoteApp( }); app.delete("/api/servers/:id", async (c) => { + const ro = requireWritable(c); + if (ro) return ro; const id = c.req.param("id"); if (!id || !validateStoreId(id)) { return c.json({ error: "Invalid id" }, 400); @@ -1804,8 +1887,13 @@ export function createRemoteApp( const send = (data: string): void => { void stream.writeSSE({ event: "change", data }); }; - serverEventSubscribers.add(send); - ensureWatcher(); + // An in-memory ad-hoc list never changes and has no file to watch, so we + // hold the stream open (so the client's fetch resolves cleanly) but + // register no subscriber and start no watcher — there is nothing to emit. + if (watchable) { + serverEventSubscribers.add(send); + ensureWatcher(); + } stream.onAbort(() => { serverEventSubscribers.delete(send); diff --git a/core/react/useServerListWritable.ts b/core/react/useServerListWritable.ts new file mode 100644 index 00000000..949430b2 --- /dev/null +++ b/core/react/useServerListWritable.ts @@ -0,0 +1,82 @@ +/** + * React hook over the `GET /api/config` endpoint that recovers the session's + * `writable` flag. The backend sets `writable: false` when the web UI was + * launched against a read-only server list — a `--config` session file or an + * ad-hoc `--server-url` / command target — so the UI can hide catalog CRUD + * (add / edit / remove / reorder / settings-save) it would only get a 403 for. + * + * Mirrors {@link useSandboxUrl} (same `/api/config` source, same fetch shape). + * Defaults to `true` until the fetch resolves and whenever the field is absent + * (a legacy backend that predates the flag), so the default catalog and + * `--catalog` keep full CRUD without any UI change. + */ + +import { useCallback, useEffect, useState } from "react"; + +export interface UseServerListWritableOptions { + /** Base URL of the remote server (typically `window.location.origin`). */ + baseUrl: string; + /** Optional auth token for the `x-mcp-remote-auth` header. */ + authToken?: string; + /** Fetch function to use (default: globalThis.fetch). Useful in tests. */ + fetchFn?: typeof fetch; +} + +export interface UseServerListWritableResult { + /** Whether the server list is writable (catalog) or read-only (session). */ + writable: boolean; + /** True while the initial fetch is in flight. */ + loading: boolean; +} + +/** Minimal shape we read from the `/api/config` payload. */ +interface ConfigPayload { + writable?: unknown; +} + +export function useServerListWritable( + opts: UseServerListWritableOptions, +): UseServerListWritableResult { + const { baseUrl, authToken, fetchFn } = opts; + const doFetch = fetchFn ?? globalThis.fetch; + const base = baseUrl.replace(/\/$/, ""); + + // Default writable so the common (catalog) case shows CRUD immediately and a + // legacy backend that omits the field keeps working. + const [writable, setWritable] = useState(true); + const [loading, setLoading] = useState(true); + + const load = useCallback( + async (isCancelled: () => boolean): Promise => { + const headers: Record = {}; + if (authToken) headers["x-mcp-remote-auth"] = `Bearer ${authToken}`; + try { + const res = await doFetch(`${base}/api/config`, { + method: "GET", + headers, + }); + if (isCancelled() || !res.ok) return; + const body = (await res.json()) as ConfigPayload; + if (isCancelled()) return; + // Only an explicit `false` makes the list read-only; a missing field + // (legacy backend) stays writable. + setWritable(body.writable !== false); + } catch { + // Network error / aborted fetch: leave writable at its default (true). + } finally { + if (!isCancelled()) setLoading(false); + } + }, + [base, authToken, doFetch], + ); + + useEffect(() => { + let cancelled = false; + void load(() => cancelled); + return () => { + cancelled = true; + }; + }, [load]); + + return { writable, loading }; +} diff --git a/specification/v2_catalog_launch_config.md b/specification/v2_catalog_launch_config.md index 7ec5f302..23e7b9fc 100644 --- a/specification/v2_catalog_launch_config.md +++ b/specification/v2_catalog_launch_config.md @@ -299,17 +299,25 @@ Import is not a list-client concern — catalog mutation via UC2 or web CRUD whe ### Web -| Launch | Catalog (`/api/servers`) | `initialMcpConfig` → `/api/config` | Connect | -|--------|--------------------------|--------------------------------------|---------| -| No server args | Default catalog via `useServers` | Empty | User selects catalog server | -| Ad-hoc server flags | **Still full default catalog** | Populated (transport fields) | **Launch config not consumed** — no auto-connect, no form pre-fill (G2) | -| `--config` + `--server` | **Still default catalog** | Populated from resolved config (settings stripped) | User sees catalog, not launch file | +Implemented (#1481/#1483): `runWeb` selects the backend's catalog source and a +session-wide `writable` flag (`/api/config`); the web UI hides catalog CRUD when +`writable: false`. -**Target:** `/api/servers` serves the **resolved** list for the session (catalog or UC3 file) with `writable` and `defaultServerId`. See [Web launch behavior](#web-launch-behavior). +| Launch | Catalog (`/api/servers`) | `writable` | Connect | +|--------|--------------------------|-----------|---------| +| No server args | Default catalog via `useServers` | `true` | User selects catalog server | +| `--catalog ` / `MCP_CATALOG_PATH` | That file (seed/CRUD/watch) | `true` | User selects a server | +| `--config ` | That file, served read-only (never written/seeded/migrated) | `false` | User selects; CRUD hidden | +| Ad-hoc server flags (`--server-url`/command, `--header`) | One server held **in memory** (no file) | `false` | User connects; `--header` applied; CRUD hidden | + +`--catalog`/`--config` are mutually exclusive, and neither combines with an +ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server. + +**Still open:** `--server` selection / `defaultServerId` + auto-connect (G8, [#1183](https://github.com/modelcontextprotocol/inspector/issues/1183)); the `resolveServerList` core unification (TUI/CLI) below remains the target shape. ### CLI and TUI -CLI connects to exactly one resolved config per invocation; TUI shows all entries from the resolved list. Resolution paths, settings lift, and port gaps are documented in [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). Key catalog-specific gaps: **G1** (CLI drops disk settings on catalog/file lookup), **G4** (web `runWeb --header` warns only). +CLI connects to exactly one resolved config per invocation; TUI shows all entries from the resolved list. Resolution paths, settings lift, and port gaps are documented in [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). Key remaining catalog-specific gap: **G1** (CLI drops disk settings on catalog/file lookup). (Web's **G4** `--header` and **G2** list-seeding are resolved — see above.) ### Shared resolvers (`resolveLaunchServerConfigs` / `resolveServerConfigs`) @@ -462,9 +470,9 @@ Resolved (documented above): catalog vs session flag split; no launch write-back | # | Gap | Severity | Clients | |---|-----|----------|---------| | G1 | CLI catalog/file lookup drops settings — `loadServerFromConfig` not `mcpConfigToServerEntries` | Functional | CLI | -| G2 | Web ignores launch-time `initialMcpConfig` for list/selection/connect | Functional / UX | Web | -| G3 | `InitialConfigPayload` has no settings — cannot pass headers/oauth at web launch | Functional | Web + launcher | -| G4 | `runWeb --header` warns only | Functional | Web launcher | +| G2 | ~~Web ignores launch-time server input for the list~~ — **resolved for web (#1481):** `runWeb` now points the backend catalog at `--catalog` (writable) / `--config` (read-only session), or serves an in-memory ad-hoc server, and `/api/config` carries `writable` so the UI gates CRUD. Selection/auto-connect (`--server` / `defaultServerId`) is still open (G8). | Web | +| G3 | ~~`InitialConfigPayload` has no settings — cannot pass headers at web launch~~ — **resolved for ad-hoc headers (#1483):** ad-hoc `--header` rides on the in-memory session entry (`headers` → `settings.headers`); a general settings channel on `InitialConfigPayload` is still unbuilt. | Web + launcher | +| G4 | ~~`runWeb --header` warns only~~ — **resolved (#1483):** applied to the seeded ad-hoc connection (or a clear error when it can't apply). | Web launcher | | G5 | Two config pipelines — `config.ts` vs `serverList.ts` | Design debt | CLI vs TUI vs web | | G6 | v1.5 `docs/mcp-server-configuration.md` not ported to v2 | Docs | All | | G7 | CLI test expects `--server` without `--config` to fail — predates default catalog | Tests | CLI | @@ -506,6 +514,8 @@ G1, G4, and launcher details: [v2_cli_tui_launcher.md](v2_cli_tui_launcher.md). | Issue | Status | Relevance | |-------|--------|-----------| +| [#1481](https://github.com/modelcontextprotocol/inspector/issues/1481) — launcher `--config` drives the web catalog | Implemented | `--catalog` (writable) / `--config` (read-only session) / ad-hoc in-memory → backend `mcpConfigPath`/`initialServers` + `writable` on `/api/config`; UI gates CRUD | +| [#1483](https://github.com/modelcontextprotocol/inspector/issues/1483) — web `--header` warns only | Implemented | `--header` lifted onto the in-memory ad-hoc entry (`settings.headers`); errors clearly when it can't apply | | [#1347](https://github.com/modelcontextprotocol/inspector/issues/1347) — default `--config` | Open (likely closable) | Implemented via `resolveLaunchServerConfigs()` | | [#1246](https://github.com/modelcontextprotocol/inspector/issues/1246) — port CLI/TUI/launcher | Done in worktree | Parent of default-catalog behavior | | [#1183](https://github.com/modelcontextprotocol/inspector/issues/1183) — auto-connect | Open | UC5 web ergonomics |