fix(mcp): block __proto__ server names, fix lint; add route and registry tests

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/mcp-sync.ts

@@ -74,6 +74,9 @@ export interface McpSyncResult {
 
 const isRecord = (v: unknown): v is Record<string, unknown> => typeof v === 'object' && v !== null && !Array.isArray(v);
 
+/** Names that would reach Object.prototype through a plain-object table (`out[name] = ...`). */
+const UNSAFE_NAMES = new Set(['__proto__', 'constructor', 'prototype']);
+
 function strMap(v: unknown): Record<string, string> | undefined {
   if (!isRecord(v)) return undefined;
   const out: Record<string, string> = {};
@@ -323,7 +326,7 @@ function parseTomlKey(src: string, start: number): [string, number] {
 
 /** Split a table header like `mcp_servers."my.srv".env` into dotted key parts. */
 function parseTomlHeader(line: string): string[] | null {
-  const m = /^\[([^\]\[].*)\]\s*(#.*)?$/.exec(line.trim());
+  const m = /^\[([^[\]].*)\]\s*(#.*)?$/.exec(line.trim());
   if (!m) return null;
   const body = m[1];
   const parts: string[] = [];
@@ -357,6 +360,7 @@ function parseCodexTables(text: string): Record<string, Record<string, TomlValue
       if (trimmed.startsWith('[[')) continue;
       const parts = parseTomlHeader(trimmed);
       if (parts && parts[0] === 'mcp_servers' && (parts.length === 2 || parts.length === 3)) {
+        if (UNSAFE_NAMES.has(parts[1])) continue;
         current = out[parts[1]] ??= {};
         sub = parts.length === 3 ? parts[2] : null;
         if (sub && !isRecord(current[sub])) current[sub] = {};
@@ -424,6 +428,7 @@ export function parseServers(format: McpFormat, text: string | null): McpServerM
   if (text === null || !text.trim()) return out;
   if (format === 'codex-toml') {
     for (const [name, table] of Object.entries(parseCodexTables(text))) {
+      if (UNSAFE_NAMES.has(name)) continue;
       const s = fromCodex(table);
       if (s) out[name] = s;
     }
@@ -436,6 +441,7 @@ export function parseServers(format: McpFormat, text: string | null): McpServerM
   if (table === undefined) return out;
   if (!isRecord(table)) throw new Error(`"${dialect.key}" is not an object`);
   for (const [name, raw] of Object.entries(table)) {
+    if (UNSAFE_NAMES.has(name)) continue;
     const s = dialect.from(raw);
     if (s) out[name] = s;
   }

test/mcp-sync-registry.test.ts

@@ -0,0 +1,44 @@
+// @vitest-environment node
+// The registry half of MCP sync: which CLIs declare an MCP config file, and that the schema
+// guards the path (sync writes to it) so a user clis.json cannot aim a write outside $HOME.
+
+import { describe, expect, it } from 'vitest';
+import { CliEntrySchema } from '../src/config/cli-registry/schema.js';
+import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
+import type { CliEntry } from '../src/config/cli-registry/types.js';
+
+const claude = () => structuredClone(STOCK_CLIS.find((e) => (e.id as string) === 'claude')!) as CliEntry;
+
+function withMcp(mcpConfig: unknown) {
+  const e = claude();
+  (e.capabilities as Record<string, unknown>).mcpConfig = mcpConfig;
+  return CliEntrySchema.safeParse(e);
+}
+
+describe('capabilities.mcpConfig', () => {
+  it('is declared by exactly the CLIs whose format is verified', () => {
+    const declared = STOCK_CLIS.filter((e) => e.capabilities.mcpConfig).map((e) => e.id as string);
+    expect(declared.sort()).toEqual(['antigravity', 'claude', 'codex', 'gemini', 'opencode']);
+  });
+
+  it('every stock declaration passes the schema, with a distinct file per CLI', () => {
+    for (const e of STOCK_CLIS) expect(CliEntrySchema.safeParse(e).success, e.id as string).toBe(true);
+    const paths = STOCK_CLIS.flatMap((e) => (e.capabilities.mcpConfig ? [e.capabilities.mcpConfig.path] : []));
+    expect(new Set(paths).size).toBe(paths.length);
+  });
+
+  it('accepts a home-relative path with a known format', () => {
+    expect(withMcp({ path: '.tool/mcp.json', format: 'claude-json' }).success).toBe(true);
+  });
+
+  it.each([
+    ['parent traversal', { path: '../evil.json', format: 'claude-json' }],
+    ['nested traversal', { path: '.a/../../evil.json', format: 'claude-json' }],
+    ['absolute path', { path: '/etc/cron.d/x', format: 'claude-json' }],
+    ['shell metacharacters', { path: '.a;rm -rf', format: 'claude-json' }],
+    ['unknown format', { path: '.a/mcp.json', format: 'yaml' }],
+    ['extra key', { path: '.a/mcp.json', format: 'claude-json', mode: 'rw' }],
+  ])('rejects %s', (_label, value) => {
+    expect(withMcp(value).success).toBe(false);
+  });
+});

test/mcp-sync.test.ts

@@ -134,6 +134,28 @@ describe('real CLI output (captured from `agy`/`gemini`/`codex mcp add`)', () =>
   });
 });
 
+describe('hostile config files', () => {
+  it('never lets a server name reach Object.prototype (toml and json)', () => {
+    const toml = parseServers(
+      'codex-toml',
+      '[mcp_servers.__proto__]\ncommand = "x"\npolluted = "yes"\n[mcp_servers.ok]\ncommand = "y"\n'
+    );
+    expect(Object.keys(toml)).toEqual(['ok']);
+    const json = parseServers(
+      'claude-json',
+      '{"mcpServers":{"__proto__":{"command":"x"},"constructor":{"command":"x"},"ok":{"command":"y"}}}'
+    );
+    expect(Object.keys(json)).toEqual(['ok']);
+    expect(({} as Record<string, unknown>).polluted).toBeUndefined();
+    expect(({} as Record<string, unknown>).command).toBeUndefined();
+  });
+
+  it('rejects a non-object server table instead of overwriting it', () => {
+    expect(() => parseServers('claude-json', '{"mcpServers":[]}')).toThrow();
+    expect(() => parseServers('claude-json', '[]')).toThrow();
+  });
+});
+
 describe('addServers', () => {
   it('preserves other keys and existing servers, appends codex tables without touching the rest', () => {
     const out = JSON.parse(

test/routes/mcp-sync-routes.test.ts

@@ -0,0 +1,116 @@
+/**
+ * @fileoverview Route tests for /api/mcp-sync. Only CLIs that are ENABLED in the registry take
+ * part; enabled agent CLIs with no known MCP config are reported as unsupported.
+ *
+ * ⚠️ test/setup.ts gives the whole FILE one temp HOME, so each test wipes the config files it
+ * creates. Port: N/A (app.inject()).
+ */
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { homedir } from 'node:os';
+import { dirname, join } from 'node:path';
+import { createRouteTestHarness } from './_route-test-utils.js';
+import { registerMcpSyncRoutes } from '../../src/web/routes/mcp-sync-routes.js';
+import { registryFilePath, reloadCliRegistry } from '../../src/config/cli-registry/registry.js';
+
+const home = () => homedir();
+const write = (rel: string, text: string) => {
+  const f = join(home(), rel);
+  mkdirSync(dirname(f), { recursive: true });
+  writeFileSync(f, text);
+};
+const disable = (...ids: string[]) => {
+  const file = registryFilePath();
+  mkdirSync(dirname(file), { recursive: true });
+  const clis = Object.fromEntries(ids.map((id) => [id, { enabled: false }]));
+  writeFileSync(file, JSON.stringify({ schemaVersion: 1, clis }), { mode: 0o600 });
+  reloadCliRegistry();
+};
+
+const CLAUDE = '.claude.json';
+const CODEX = '.codex/config.toml';
+const GEMINI = '.gemini/settings.json';
+
+beforeEach(() => {
+  rmSync(registryFilePath(), { force: true });
+  reloadCliRegistry();
+  for (const d of ['.claude.json', '.codex', '.gemini', '.config'])
+    rmSync(join(home(), d), { recursive: true, force: true });
+  write(CLAUDE, JSON.stringify({ mcpServers: { fs: { type: 'stdio', command: 'npx', args: ['-y', 'fs'] } } }));
+});
+afterEach(() => {
+  delete process.env.CODEMAN_MULTIUSER;
+  rmSync(registryFilePath(), { force: true });
+  reloadCliRegistry();
+});
+
+describe('/api/mcp-sync', () => {
+  it('GET previews without writing', async () => {
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes);
+    const res = await app.inject({ method: 'GET', url: '/api/mcp-sync' });
+    expect(res.statusCode).toBe(200);
+    const body = res.json();
+    expect(body.success).toBe(true);
+    expect(body.data.applied).toBe(false);
+    expect(body.data.targets.find((t: { id: string }) => t.id === 'codex').added).toEqual(['fs']);
+    expect(existsSync(join(home(), CODEX))).toBe(false);
+  });
+
+  it('POST adds the server to every enabled CLI', async () => {
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes);
+    const res = await app.inject({ method: 'POST', url: '/api/mcp-sync' });
+    expect(res.json().data.applied).toBe(true);
+    expect(readFileSync(join(home(), CODEX), 'utf8')).toContain('[mcp_servers.fs]');
+    expect(JSON.parse(readFileSync(join(home(), GEMINI), 'utf8')).mcpServers.fs.command).toBe('npx');
+  });
+
+  it('never touches a CLI that is disabled in the registry', async () => {
+    disable('codex');
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes);
+    const res = await app.inject({ method: 'POST', url: '/api/mcp-sync' });
+    const ids = res.json().data.targets.map((t: { id: string }) => t.id);
+    expect(ids).not.toContain('codex');
+    expect(ids).toContain('gemini');
+    expect(existsSync(join(home(), '.codex'))).toBe(false);
+    expect(existsSync(join(home(), GEMINI))).toBe(true);
+  });
+
+  it('lists enabled agent CLIs without MCP support, and omits disabled ones and the shell', async () => {
+    disable('pi');
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes);
+    const { unsupported } = (await app.inject({ method: 'GET', url: '/api/mcp-sync' })).json().data;
+    expect(unsupported).toContain('Grok');
+    expect(unsupported).not.toContain('Pi');
+    expect(unsupported.some((l: string) => /shell|terminal/i.test(l))).toBe(false);
+  });
+
+  it('never returns env values or headers', async () => {
+    write(
+      CLAUDE,
+      JSON.stringify({ mcpServers: { fs: { type: 'stdio', command: 'npx', env: { TOKEN: 'sekrit-value' } } } })
+    );
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes);
+    const res = await app.inject({ method: 'POST', url: '/api/mcp-sync' });
+    expect(res.body).not.toContain('sekrit-value');
+  });
+
+  it('multi-user: a non-admin is refused on both verbs and nothing is written', async () => {
+    process.env.CODEMAN_MULTIUSER = '1';
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes, {
+      authUser: { username: 'bob', role: 'user' },
+    });
+    for (const method of ['GET', 'POST'] as const) {
+      const res = await app.inject({ method, url: '/api/mcp-sync' });
+      expect(res.json().success, method).toBe(false);
+    }
+    expect(existsSync(join(home(), CODEX))).toBe(false);
+  });
+
+  it('multi-user: an admin is allowed', async () => {
+    process.env.CODEMAN_MULTIUSER = '1';
+    const { app } = await createRouteTestHarness(registerMcpSyncRoutes, {
+      authUser: { username: 'root', role: 'admin' },
+    });
+    expect((await app.inject({ method: 'GET', url: '/api/mcp-sync' })).json().success).toBe(true);
+  });
+});