From 89a46cad2a426392586af3b16f928aabaee5fcaa Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Wed, 3 Jun 2026 18:25:51 +1000 Subject: [PATCH 1/2] fix(security): set strictMcpConfig to block cloned-PR .mcp.json auto-load settingSources:[] (issue #191) gates settings.json/CLAUDE.md but not a project .mcp.json, which the CLI auto-discovers from cwd by default. With cwd = the cloned PR tree under bypassPermissions, a hostile .mcp.json could register a stdio MCP server whose command runs on connect, the same RCE primitive class as #191. strictMcpConfig:true emits --strict-mcp-config, restricting MCP loading to the explicitly-injected mcpServers. Closes #196 Co-Authored-By: Claude Opus 4.8 --- src/core/executor.ts | 12 ++++++++++++ test/core/executor.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) 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..62cb820 100644 --- a/test/core/executor.test.ts +++ b/test/core/executor.test.ts @@ -105,6 +105,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; From 3d625f69e087ffcf313d873316d705d0b0e931a7 Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Wed, 3 Jun 2026 18:49:38 +1000 Subject: [PATCH 2/2] test: declare security-pin fields on the QueryCall options interface Make the #196/#191 assertions type-visible instead of relying on the captured object's runtime shape. Co-Authored-By: Claude Opus 4.8 --- test/core/executor.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/core/executor.test.ts b/test/core/executor.test.ts index 62cb820..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; }; }