Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/core/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
? {
Expand Down
27 changes: 27 additions & 0 deletions test/core/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
}

Expand Down Expand Up @@ -105,6 +109,29 @@ function awaitAbortIterator(
} as AsyncIterableIterator<unknown>;
}

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([]);
Comment thread
chrisleekr marked this conversation as resolved.
});
});

describe("executeAgent: cancellation", () => {
beforeEach(() => {
lastQueryCall = undefined;
Expand Down
Loading