Restore .claude/ and .mcp.json from PR base branch before CLI runs

[km-anthropic]

Problem

The CLI's non-interactive mode trusts the working directory: it reads .mcp.json, .claude/settings.json, and .claude/settings.local.json from cwd and acts on them before any tool-permission gating. This includes:

• hooks (SessionStart, PreToolUse) — arbitrary shell commands
• env — NODE_OPTIONS, LD_PRELOAD, PATH
• apiKeyHelper / awsAuthRefresh — shell commands for credential fetching
• .mcp.json with enableAllProjectMcpServers — MCP server init spawns processes

When this action checks out a PR head, all of these files are attacker-controlled. A PR author without write access can achieve RCE in the workflow runner and exfiltrate secrets before the CLI reaches turn 1.

#942 addresses the .mcp.json case specifically. This PR addresses the root cause.

Fix

Before the CLI runs, restore .claude/ and .mcp.json from the PR base branch:

1. git fetch origin <base> --depth=1
2. rm -rf .claude .mcp.json
3. git checkout origin/<base> -- .claude .mcp.json (per path; errors = path absent on base → stays deleted)
4. git reset -- .claude .mcp.json (unstage so the revert doesn't leak into CLI commits)

The base branch version is trusted because a maintainer reviewed and merged it. The PR author's delta is discarded. Reviewed config on base still works normally.

	#942 (file-check)	This PR
Coverage	.mcp.json only	.claude/ + .mcp.json — all startup-executed config
TOCTOU	API check races against checkout SHA	Local git, no race
Legitimate base config	Disabled when .mcp.json touched	Still works — only PR delta neutralized
Maintenance	File-pattern list to sync with CLI	One mechanism

Scope

• Only runs on PRs. Pushes to protected branches, workflow_dispatch, etc. are unaffected — workspace is already a trusted ref.
• CLAUDE.md is not covered. It's prompt injection (gated by tool permissions), not pre-turn-1 RCE.

Known limitation

If a PR legitimately modifies .claude/ and the CLI later runs git add -A, the revert will be included in that commit. Workaround: merge the config change first with human review, then test. Documented in the function docstring.

Implementation notes

• baseBranch comes from prData.baseRefName (GitHub GraphQL) and passes through validateBranchName() in branch.ts before being returned — blocks option injection (--upload-pack), shell metachars, path traversal.
• execFileSync with array args throughout — no shell interpolation.
• Fetch happens before rm, so fetch failure leaves the workspace untouched.

[claude]

🟡 restoreConfigFromBase requires its parameter to be pre-validated (per its docstring), and validateBranchName() is called on the pull_request event path (line 245) but not on the issue_comment fallback path where restoreBase = baseBranch from agent mode. Adding validateBranchName(restoreBase) before line 248 would close this defense-in-depth gap, though practical risk is near-zero since the fallback value comes from admin-controlled sources.

Extended reasoning...

The Bug

When the event is issue_comment on a PR, the code at lines 237-250 takes the fallback path where restoreBase is set to baseBranch (from the mode prepare function) rather than reading context.payload.pull_request.base.ref. In agent mode, this baseBranch value comes from process.env.BASE_BRANCH || context.inputs.baseBranch || "main" (agent/index.ts:88-89) and is never passed through validateBranchName().

The Inconsistency

The restoreConfigFromBase docstring at restore-config.ts:25-26 explicitly states: "Must be pre-validated (branch.ts calls validateBranchName on it before returning)." The PR event path correctly calls validateBranchName(restoreBase) at line 245, but the issue_comment fallback skips this validation entirely. For tag mode this is fine because setupBranch validates baseBranch at branch.ts:176, but agent mode does not validate its baseBranch anywhere in its pipeline.

Step-by-Step Proof

1. An issue_comment event fires on a PR in a repo using agent mode.
2. prepareAgentMode runs and computes baseBranch = process.env.BASE_BRANCH || context.inputs.baseBranch || "main" (agent/index.ts:88-89).
3. At run.ts:237, isEntityContext(context) && context.isPR is true, so we enter the block.
4. At line 238, restoreBase = baseBranch (the unvalidated value from agent mode).
5. The isPullRequestEvent / isPullRequestReviewEvent / isPullRequestReviewCommentEvent checks all fail (this is an issue_comment event), so the inner block with validateBranchName at line 245 is skipped.
6. At line 248, restoreConfigFromBase(restoreBase) is called with the unvalidated baseBranch.

Practical Impact

The practical risk is effectively zero. The BASE_BRANCH env var is set in the workflow YAML (which runs from the default branch, not the PR branch), context.inputs.baseBranch resolves to the same process.env.BASE_BRANCH (context.ts:150), and the hardcoded default is "main". None of these are attacker-controlled. Additionally, execFileSync with array args prevents shell injection. However, validateBranchName also blocks leading dashes (preventing git option injection like --upload-pack=...), so including it would be a worthwhile defense-in-depth measure.

Suggested Fix

Add validateBranchName(restoreBase) before the restoreConfigFromBase(restoreBase) call at line 248, or move the validation outside the event-type if block so it applies to all paths uniformly:

if (restoreBase) {
  validateBranchName(restoreBase);
  restoreConfigFromBase(restoreBase);
}