diff --git a/base-action/src/setup-claude-code-settings.ts b/base-action/src/setup-claude-code-settings.ts index 0fe6841..a5109ae 100644 --- a/base-action/src/setup-claude-code-settings.ts +++ b/base-action/src/setup-claude-code-settings.ts @@ -4,6 +4,7 @@ import { readFile } from "fs/promises"; export async function setupClaudeCodeSettings( settingsInput?: string, + mcpJsonChanged = false, homeDir?: string, ) { const home = homeDir ?? homedir(); @@ -59,9 +60,16 @@ export async function setupClaudeCodeSettings( console.log(`Merged settings with input settings`); } - // Always set enableAllProjectMcpServers to true - settings.enableAllProjectMcpServers = true; - console.log(`Updated settings with enableAllProjectMcpServers: true`); + if (!mcpJsonChanged) { + // Only auto-approve project MCP servers if .mcp.json hasn't changed in this PR. + // If .mcp.json changed, the checked-out version may differ from the base branch. + settings.enableAllProjectMcpServers = true; + console.log(`Updated settings with enableAllProjectMcpServers: true`); + } else { + console.log( + `Skipping enableAllProjectMcpServers=true because .mcp.json changed in this PR`, + ); + } await $`echo ${JSON.stringify(settings, null, 2)} > ${settingsPath}`.quiet(); console.log(`Settings saved successfully`); diff --git a/base-action/test/setup-claude-code-settings.test.ts b/base-action/test/setup-claude-code-settings.test.ts index defe251..1438931 100644 --- a/base-action/test/setup-claude-code-settings.test.ts +++ b/base-action/test/setup-claude-code-settings.test.ts @@ -28,7 +28,7 @@ describe("setupClaudeCodeSettings", () => { }); test("should always set enableAllProjectMcpServers to true when no input", async () => { - await setupClaudeCodeSettings(undefined, testHomeDir); + await setupClaudeCodeSettings(undefined, false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -42,7 +42,7 @@ describe("setupClaudeCodeSettings", () => { env: { API_KEY: "test-key" }, }); - await setupClaudeCodeSettings(inputSettings, testHomeDir); + await setupClaudeCodeSettings(inputSettings, false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -69,7 +69,7 @@ describe("setupClaudeCodeSettings", () => { await writeFile(testSettingsPath, JSON.stringify(testSettings, null, 2)); - await setupClaudeCodeSettings(testSettingsPath, testHomeDir); + await setupClaudeCodeSettings(testSettingsPath, false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -79,13 +79,13 @@ describe("setupClaudeCodeSettings", () => { expect(settings.permissions).toEqual(testSettings.permissions); }); - test("should override enableAllProjectMcpServers even if false in input", async () => { + test("should override enableAllProjectMcpServers even if false in input when mcpJsonChanged is false", async () => { const inputSettings = JSON.stringify({ enableAllProjectMcpServers: false, model: "test-model", }); - await setupClaudeCodeSettings(inputSettings, testHomeDir); + await setupClaudeCodeSettings(inputSettings, false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -94,20 +94,49 @@ describe("setupClaudeCodeSettings", () => { expect(settings.model).toBe("test-model"); }); + test("should not set enableAllProjectMcpServers to true when mcpJsonChanged is true", async () => { + const inputSettings = JSON.stringify({ + model: "test-model", + }); + + await setupClaudeCodeSettings(inputSettings, true, testHomeDir); + + const settingsContent = await readFile(settingsPath, "utf-8"); + const settings = JSON.parse(settingsContent); + + expect(settings.enableAllProjectMcpServers).toBeUndefined(); + expect(settings.model).toBe("test-model"); + }); + + test("should not override enableAllProjectMcpServers when mcpJsonChanged is true and input sets it false", async () => { + const inputSettings = JSON.stringify({ + enableAllProjectMcpServers: false, + model: "test-model", + }); + + await setupClaudeCodeSettings(inputSettings, true, testHomeDir); + + const settingsContent = await readFile(settingsPath, "utf-8"); + const settings = JSON.parse(settingsContent); + + expect(settings.enableAllProjectMcpServers).toBe(false); + expect(settings.model).toBe("test-model"); + }); + test("should throw error for invalid JSON string", async () => { expect(() => - setupClaudeCodeSettings("{ invalid json", testHomeDir), + setupClaudeCodeSettings("{ invalid json", false, testHomeDir), ).toThrow(); }); test("should throw error for non-existent file path", async () => { expect(() => - setupClaudeCodeSettings("/non/existent/file.json", testHomeDir), + setupClaudeCodeSettings("/non/existent/file.json", false, testHomeDir), ).toThrow(); }); test("should handle empty string input", async () => { - await setupClaudeCodeSettings("", testHomeDir); + await setupClaudeCodeSettings("", false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -116,7 +145,7 @@ describe("setupClaudeCodeSettings", () => { }); test("should handle whitespace-only input", async () => { - await setupClaudeCodeSettings(" \n\t ", testHomeDir); + await setupClaudeCodeSettings(" \n\t ", false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); @@ -128,6 +157,7 @@ describe("setupClaudeCodeSettings", () => { // First, create some existing settings await setupClaudeCodeSettings( JSON.stringify({ existingKey: "existingValue" }), + false, testHomeDir, ); @@ -137,7 +167,7 @@ describe("setupClaudeCodeSettings", () => { model: "claude-opus-4-1-20250805", }); - await setupClaudeCodeSettings(newSettings, testHomeDir); + await setupClaudeCodeSettings(newSettings, false, testHomeDir); const settingsContent = await readFile(settingsPath, "utf-8"); const settings = JSON.parse(settingsContent); diff --git a/src/entrypoints/run.ts b/src/entrypoints/run.ts index a5de513..c9a62e3 100644 --- a/src/entrypoints/run.ts +++ b/src/entrypoints/run.ts @@ -217,7 +217,35 @@ async function run() { validateEnvironmentVariables(); - await setupClaudeCodeSettings(process.env.INPUT_SETTINGS); + // Check if .mcp.json changed in this PR to guard against RCE via malicious MCP servers. + // If .mcp.json was modified, skip auto-approving project MCP servers so attacker-injected + // servers are not trusted automatically. + let mcpJsonChanged = false; + if (isEntityContext(context) && context.isPR) { + try { + const { data: changedFiles } = await octokit.rest.pulls.listFiles({ + owner: context.repository.owner, + repo: context.repository.repo, + pull_number: context.entityNumber, + per_page: 100, + }); + mcpJsonChanged = changedFiles.some( + (f) => + f.filename === ".mcp.json" || f.filename.endsWith("/.mcp.json"), + ); + if (mcpJsonChanged) { + console.log( + ".mcp.json changed in this PR — disabling auto-approval of project MCP servers", + ); + } + } catch (e) { + console.log( + `Could not check PR changed files: ${e}. Defaulting to mcpJsonChanged=false.`, + ); + } + } + + await setupClaudeCodeSettings(process.env.INPUT_SETTINGS, mcpJsonChanged); await installPlugins( process.env.INPUT_PLUGIN_MARKETPLACES,