Skip to content
Closed
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
2 changes: 1 addition & 1 deletion base-action/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ async function run() {

await setupClaudeCodeSettings(
process.env.INPUT_SETTINGS,
undefined, // homeDir
false, // mcpJsonChanged — standalone base-action has no PR context to check
);

// Install Claude Code plugins if specified
Expand Down
17 changes: 14 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,
) {
const home = homeDir ?? homedir();
Expand Down Expand Up @@ -59,9 +60,19 @@ 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 {
// .mcp.json is untrusted in this PR — strip any enableAllProjectMcpServers
// that came from user input so the protection cannot be bypassed.
delete settings.enableAllProjectMcpServers;
console.log(
`Removing enableAllProjectMcpServers because .mcp.json changed in this PR`,
);
}

await $`echo ${JSON.stringify(settings, null, 2)} > ${settingsPath}`.quiet();
console.log(`Settings saved successfully`);
Expand Down
65 changes: 55 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,64 @@ 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 remove 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).toBeUndefined();
expect(settings.model).toBe("test-model");
});

test("should remove enableAllProjectMcpServers when mcpJsonChanged is true even if input sets it true", async () => {
const inputSettings = JSON.stringify({
enableAllProjectMcpServers: true,
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 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 +160,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 +172,7 @@ describe("setupClaudeCodeSettings", () => {
// First, create some existing settings
await setupClaudeCodeSettings(
JSON.stringify({ existingKey: "existingValue" }),
false,
testHomeDir,
);

Expand All @@ -137,7 +182,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).`,
);
mcpJsonChanged = true;
}
}

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

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