* feat(web): catalog vs read-only session launch model + apply --header (#1481, #1483) Per specification/v2_catalog_launch_config.md, `mcp-inspector --web` now selects the backend's server-list source and a session-wide writability, instead of seeding an inert `initialMcpConfig` the UI never read: - `--catalog <path>` (and `MCP_CATALOG_PATH`) → active WRITABLE catalog (seed/CRUD/watch), like the default catalog. - `--config <path>` → READ-ONLY session file: served in the UI but never written, seeded, or plaintext-secret-migrated — safe to point at a foreign config (e.g. claude_desktop_config.json). - ad-hoc `--server-url`/command (+ `--header`) → one server held IN MEMORY (no file written); `--header` is lifted onto the entry so the seeded connection actually uses it (closes the #1483 warn-only no-op). - nothing → default catalog (writable), unchanged. Backend (`createRemoteApp`): new `writable` + `initialServers` options. GET /api/servers branches three ways (in-memory / read-only file with no seed+no migrate / writable catalog); the four mutating routes return 403 when read-only; `/api/config` carries `writable`; the file watcher is skipped for in-memory sessions. `--catalog`/`--config` are mutually exclusive and neither combines with an ad-hoc target or `--header`; `--header` requires an ad-hoc HTTP/SSE server (clear errors otherwise — no silent no-op). UI: `useServerListWritable` reads `writable` from /api/config; the Servers screen hides add/edit/clone/remove/reorder/settings and shows a read-only banner when false. All new props default to writable=true (back-compatible). Docs: spec gaps G2/G3/G4 updated; launcher README documents the flags. Verified end-to-end against the built launcher: --config (writable:false, file byte-stable, mutations 403), ad-hoc --header (in-memory, header lifted), and --catalog (writable:true, POST persists). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(web): address review — Mantine <Code> in read-only banner, clarify watcher comment - Replace raw <code> with Mantine <Code> in the read-only-session Alert (repo convention: prefer Mantine primitives over raw HTML). - Add a comment near `watchable` clarifying that a read-only `--config` file is still watched on purpose (read-only blocks UI writes, not external observation; `writable` does not gate watching). Deferred (per reviewer): consolidating useSandboxUrl + useServerListWritable into one useConfig hook to avoid the duplicate /api/config fetch — optional follow-up. The redundant ad-hoc header type-guard is kept as defensive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
12b8eb11fd
commit
91f02fd6db
3 changed files with 1281 additions and 157 deletions
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<Response>;
|
||||
}): 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<void> {
|
||||
await new Promise<void>((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<RoHarness> {
|
||||
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<RoHarness> {
|
||||
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<string, string> }).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);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue