From 9ddce40de8c1ab71fb6303a125fdad0968dc1312 Mon Sep 17 00:00:00 2001 From: kashyap murali Date: Wed, 18 Mar 2026 09:00:18 -0700 Subject: [PATCH] Restore .claude/ and .mcp.json from PR base branch before CLI runs (#1066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Restore .claude/ and .mcp.json from PR base branch before CLI runs The CLI's non-interactive mode trusts cwd: it reads .mcp.json and .claude/settings{,.local}.json from the working directory and acts on them before any tool-permission gating — executing hooks, setting env vars (NODE_OPTIONS, LD_PRELOAD), running apiKeyHelper shell commands, and auto-approving MCP servers. When this action checks out a PR head, these files are attacker-controlled. Rather than enumerate dangerous keys, replace the entire .claude/ tree and .mcp.json with the versions from the PR base branch (which a maintainer has reviewed). Paths absent on base are deleted. Uses local git state, so no TOCTOU against the GitHub API. * Read PR base ref from payload for config restore in agent mode Agent mode's branchInfo.baseBranch defaults to "main" (or env/input override) instead of the PR's actual target branch — it doesn't query prData.baseRefName like tag mode does. This meant a PR targeting develop would get .claude/ restored from main. Fix by reading pull_request.base.ref directly from the webhook payload for pull_request, pull_request_review, and pull_request_review_comment events. For issue_comment on a PR (no base.ref in payload), fall back to the mode-provided value — tag mode's value is correct (from GraphQL); agent mode on issue_comment is an edge case that at worst restores from the wrong trusted branch, which is still secure. The payload value passes through validateBranchName for defense-in-depth (GitHub enforces valid branch names server-side, but we validate anyway). * Extend restored paths to .gitmodules, .ripgreprc, .claude.json .gitmodules defines submodule URLs and paths; path-confusion attacks against git submodule operations can write into .git/hooks. .ripgreprc can set --pre (arbitrary command on each file) if RIPGREP_CONFIG_PATH points at it. .claude.json is cheap defense-in-depth. Documented why .git/ is excluded (not trackable in commits, and restoring it would undo the PR checkout), along with .gitconfig (git never reads it from cwd) and shell rc files (sourced from $HOME, not cwd — checkout cannot reach $HOME). --- src/entrypoints/run.ts | 34 ++++++++++- src/github/operations/restore-config.ts | 79 +++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/github/operations/restore-config.ts diff --git a/src/entrypoints/run.ts b/src/entrypoints/run.ts index b7078ea..1858642 100644 --- a/src/entrypoints/run.ts +++ b/src/entrypoints/run.ts @@ -15,12 +15,20 @@ import { setupGitHubToken, WorkflowValidationSkipError } from "../github/token"; import { checkWritePermissions } from "../github/validation/permissions"; import { createOctokit } from "../github/api/client"; import type { Octokits } from "../github/api/client"; -import { parseGitHubContext, isEntityContext } from "../github/context"; +import { + parseGitHubContext, + isEntityContext, + isPullRequestEvent, + isPullRequestReviewEvent, + isPullRequestReviewCommentEvent, +} from "../github/context"; import type { GitHubContext } from "../github/context"; import { detectMode } from "../modes/detector"; import { prepareTagMode } from "../modes/tag"; import { prepareAgentMode } from "../modes/agent"; import { checkContainsTrigger } from "../github/validation/trigger"; +import { restoreConfigFromBase } from "../github/operations/restore-config"; +import { validateBranchName } from "../github/operations/branch"; import { collectActionInputsPresence } from "./collect-inputs"; import { updateCommentLink } from "./update-comment-link"; import { formatTurnsFromData } from "./format-turns"; @@ -217,6 +225,30 @@ async function run() { validateEnvironmentVariables(); + // On PRs, .claude/ and .mcp.json in the checkout are attacker-controlled. + // Restore them from the base branch before the CLI reads them. + // + // We read pull_request.base.ref from the payload directly because agent + // mode's branchInfo.baseBranch defaults to "main" rather than the PR's + // actual target (agent/index.ts). For issue_comment on a PR the payload + // lacks base.ref, so we fall back to the mode-provided value — tag mode + // fetches it from GraphQL; agent mode on issue_comment is an edge case + // that at worst restores from the wrong trusted branch (still secure). + if (isEntityContext(context) && context.isPR) { + let restoreBase = baseBranch; + if ( + isPullRequestEvent(context) || + isPullRequestReviewEvent(context) || + isPullRequestReviewCommentEvent(context) + ) { + restoreBase = context.payload.pull_request.base.ref; + validateBranchName(restoreBase); + } + if (restoreBase) { + restoreConfigFromBase(restoreBase); + } + } + await setupClaudeCodeSettings(process.env.INPUT_SETTINGS); await installPlugins( diff --git a/src/github/operations/restore-config.ts b/src/github/operations/restore-config.ts new file mode 100644 index 0000000..4e12739 --- /dev/null +++ b/src/github/operations/restore-config.ts @@ -0,0 +1,79 @@ +import { execFileSync } from "child_process"; +import { rmSync } from "fs"; + +// Paths that are both PR-controllable and read from cwd at CLI startup. +// +// Deliberately excluded from the CLI's broader auto-edit blocklist: +// .git/ — not tracked by git; PR commits cannot place files there. +// Restoring it would also undo the PR checkout entirely. +// .gitconfig — git reads ~/.gitconfig and .git/config, never cwd/.gitconfig. +// .bashrc etc. — shells source these from $HOME; checkout cannot reach $HOME. +// .vscode/.idea— IDE config; nothing in the CLI's startup path reads them. +const SENSITIVE_PATHS = [ + ".claude", + ".mcp.json", + ".claude.json", + ".gitmodules", + ".ripgreprc", +]; + +/** + * Restores security-sensitive config paths from the PR base branch. + * + * The CLI's non-interactive mode trusts cwd: it reads `.mcp.json`, + * `.claude/settings.json`, and `.claude/settings.local.json` from the working + * directory and acts on them before any tool-permission gating — executing + * hooks (including SessionStart), setting env vars (NODE_OPTIONS, LD_PRELOAD, + * PATH), running apiKeyHelper/awsAuthRefresh shell commands, and auto-approving + * MCP servers. When this action checks out a PR head, all of these are + * attacker-controlled. + * + * Rather than enumerate every dangerous key, this replaces the entire `.claude/` + * tree and `.mcp.json` with the versions from the PR base branch, which a + * maintainer has reviewed and merged. Paths absent on base are deleted. + * + * Known limitation: if a PR legitimately modifies `.claude/` and the CLI later + * commits with `git add -A`, the revert will be included in that commit. This + * is a narrow UX tradeoff for closing the RCE surface. + * + * @param baseBranch - PR base branch name. Must be pre-validated (branch.ts + * calls validateBranchName on it before returning). + */ +export function restoreConfigFromBase(baseBranch: string): void { + console.log( + `Restoring ${SENSITIVE_PATHS.join(", ")} from origin/${baseBranch} (PR head is untrusted)`, + ); + + // Fetch base first — if this fails we haven't touched the workspace and the + // caller sees a clean error. + execFileSync("git", ["fetch", "origin", baseBranch, "--depth=1"], { + stdio: "inherit", + }); + + // Delete PR-controlled versions. If the restore below fails for a given path, + // that path stays deleted — the safe fallback (no attacker-controlled config). + // A bare `git checkout` alone wouldn't remove files the PR added, so nuke first. + for (const p of SENSITIVE_PATHS) { + rmSync(p, { recursive: true, force: true }); + } + + for (const p of SENSITIVE_PATHS) { + try { + execFileSync("git", ["checkout", `origin/${baseBranch}`, "--", p], { + stdio: "pipe", + }); + } catch { + // Path doesn't exist on base — it stays deleted. + } + } + + // `git checkout -- ` stages the restored files. Unstage so the + // revert doesn't silently leak into commits the CLI makes later. + try { + execFileSync("git", ["reset", "--", ...SENSITIVE_PATHS], { + stdio: "pipe", + }); + } catch { + // Nothing was staged, or paths don't exist on HEAD — either is fine. + } +}