Close remaining gaps in .mcp.json change protection

When mcpJsonChanged is true, delete enableAllProjectMcpServers from the
merged settings object so user-provided INPUT_SETTINGS cannot re-enable
it and bypass the check. Also fix the stale positional arg in the
standalone base-action call site and add test coverage for the bypass case.
This commit is contained in:
Kashyap Murali 2026-03-17 12:24:27 -07:00
parent f4ac33b9a4
commit 72a5802bde
No known key found for this signature in database
3 changed files with 22 additions and 4 deletions

View File

@ -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

View File

@ -66,8 +66,11 @@ export async function setupClaudeCodeSettings(
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(
`Skipping enableAllProjectMcpServers=true because .mcp.json changed in this PR`,
`Removing enableAllProjectMcpServers because .mcp.json changed in this PR`,
);
}

View File

@ -108,7 +108,7 @@ describe("setupClaudeCodeSettings", () => {
expect(settings.model).toBe("test-model");
});
test("should not override enableAllProjectMcpServers when mcpJsonChanged is true and input sets it false", async () => {
test("should remove enableAllProjectMcpServers when mcpJsonChanged is true and input sets it false", async () => {
const inputSettings = JSON.stringify({
enableAllProjectMcpServers: false,
model: "test-model",
@ -119,7 +119,22 @@ describe("setupClaudeCodeSettings", () => {
const settingsContent = await readFile(settingsPath, "utf-8");
const settings = JSON.parse(settingsContent);
expect(settings.enableAllProjectMcpServers).toBe(false);
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");
});