diff --git a/src/core/executor.ts b/src/core/executor.ts index a723367..a7b28eb 100644 --- a/src/core/executor.ts +++ b/src/core/executor.ts @@ -276,6 +276,18 @@ export async function executeAgent({ // Any new SDK option must not flip this back to a permissive value. See #191. settingSources: [], mcpServers, + // settingSources:[] gates settings.json / CLAUDE.md / hooks, but NOT a + // project `.mcp.json`: the CLI auto-discovers MCP servers by default (it is + // a distinct item from settings/memory in the `--bare` discovery list), so + // a hostile `.mcp.json` committed in the cloned PR tree at `cwd` would + // register a stdio MCP server whose `command` runs on connect under + // bypassPermissions, the same RCE primitive class as #191. The SDK maps + // this option to the CLI `--strict-mcp-config` flag, which restricts MCP + // loading to the `--mcp-config` set (our `mcpServers`), ignoring project + // `.mcp.json`. Verified against the agent-sdk arg construction + the CLI + // reference. See #196. The TS `strictMcpConfig` docstring only mentions + // validation strictness, but the flag it emits also suppresses discovery. + strictMcpConfig: true, systemPrompt: useCacheableLayout && promptParts !== undefined ? { diff --git a/test/core/executor.test.ts b/test/core/executor.test.ts index 45c5bde..ada4719 100644 --- a/test/core/executor.test.ts +++ b/test/core/executor.test.ts @@ -23,6 +23,10 @@ interface QueryCall { abortController?: AbortController; stderr?: (chunk: string) => void; systemPrompt?: unknown; + // Security pins asserted by the #196 / #191 regression tests. + strictMcpConfig?: boolean; + settingSources?: unknown[]; + mcpServers?: unknown; }; } @@ -105,6 +109,29 @@ function awaitAbortIterator( } as AsyncIterableIterator; } +describe("executeAgent: MCP config hardening (#196)", () => { + beforeEach(() => { + lastQueryCall = undefined; + nextIterator = emptyIterator; + }); + + it("sets strictMcpConfig so a cloned-PR .mcp.json is not auto-loaded, keeping the injected mcpServers", async () => { + // settingSources:[] does NOT gate project `.mcp.json` MCP discovery; the + // SDK maps strictMcpConfig -> the CLI `--strict-mcp-config` flag, which + // restricts MCP loading to the explicit `mcpServers` (--mcp-config) set. + const mcpServers = { + fake: { type: "stdio", command: "node", args: [] }, + } as unknown as McpServerConfig; + await executeAgent(baseParams({ mcpServers })); + + expect(lastQueryCall?.options.strictMcpConfig).toBe(true); + // The legitimately-injected servers must still be forwarded (criterion 2). + expect(lastQueryCall?.options.mcpServers).toBe(mcpServers); + // The #191 sibling pin stays in place. + expect(lastQueryCall?.options.settingSources).toEqual([]); + }); +}); + describe("executeAgent: cancellation", () => { beforeEach(() => { lastQueryCall = undefined;