Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
14 changes: 11 additions & 3 deletions base-action/src/setup-claude-code-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { readFile } from "fs/promises";

export async function setupClaudeCodeSettings(
settingsInput?: string,
mcpJsonChanged = false,
homeDir?: string,
Comment thread
km-anthropic marked this conversation as resolved.
) {
const home = homeDir ?? homedir();
Expand Down Expand Up @@ -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`,
);
}
Comment thread
km-anthropic marked this conversation as resolved.

await $`echo ${JSON.stringify(settings, null, 2)} > ${settingsPath}`.quiet();
console.log(`Settings saved successfully`);
Expand Down
50 changes: 40 additions & 10 deletions base-action/test/setup-claude-code-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -128,6 +157,7 @@ describe("setupClaudeCodeSettings", () => {
// First, create some existing settings
await setupClaudeCodeSettings(
JSON.stringify({ existingKey: "existingValue" }),
false,
testHomeDir,
);

Expand All @@ -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);
Expand Down
34 changes: 33 additions & 1 deletion src/entrypoints/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,39 @@ 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 changedFiles = await octokit.rest.paginate(
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=true (fail-closed).`,
);
Comment thread
km-anthropic marked this conversation as resolved.
mcpJsonChanged = true;
}
}

await setupClaudeCodeSettings(process.env.INPUT_SETTINGS, mcpJsonChanged);

await installPlugins(
process.env.INPUT_PLUGIN_MARKETPLACES,
Expand Down
Loading