diff --git a/src/entrypoints/run.ts b/src/entrypoints/run.ts index ddeb5f288..5bfeaa371 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 000000000..4e12739bf --- /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. + } +}