Compare commits
3 Commits
main
...
ashwin/dis
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
72a5802bde | ||
|
|
f4ac33b9a4 | ||
|
|
19a4f34b17 |
@ -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
|
||||
|
||||
@ -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,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`);
|
||||
|
||||
@ -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,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);
|
||||
@ -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);
|
||||
@ -128,6 +172,7 @@ describe("setupClaudeCodeSettings", () => {
|
||||
// First, create some existing settings
|
||||
await setupClaudeCodeSettings(
|
||||
JSON.stringify({ existingKey: "existingValue" }),
|
||||
false,
|
||||
testHomeDir,
|
||||
);
|
||||
|
||||
@ -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);
|
||||
|
||||
@ -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,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user