Skip to content

Commit 9ddce40

Browse files
authored
Restore .claude/ and .mcp.json from PR base branch before CLI runs (#1066)
* 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).
1 parent 1b422b3 commit 9ddce40

File tree

2 files changed

+112
-1
lines changed

2 files changed

+112
-1
lines changed

src/entrypoints/run.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,20 @@ import { setupGitHubToken, WorkflowValidationSkipError } from "../github/token";
1515
import { checkWritePermissions } from "../github/validation/permissions";
1616
import { createOctokit } from "../github/api/client";
1717
import type { Octokits } from "../github/api/client";
18-
import { parseGitHubContext, isEntityContext } from "../github/context";
18+
import {
19+
parseGitHubContext,
20+
isEntityContext,
21+
isPullRequestEvent,
22+
isPullRequestReviewEvent,
23+
isPullRequestReviewCommentEvent,
24+
} from "../github/context";
1925
import type { GitHubContext } from "../github/context";
2026
import { detectMode } from "../modes/detector";
2127
import { prepareTagMode } from "../modes/tag";
2228
import { prepareAgentMode } from "../modes/agent";
2329
import { checkContainsTrigger } from "../github/validation/trigger";
30+
import { restoreConfigFromBase } from "../github/operations/restore-config";
31+
import { validateBranchName } from "../github/operations/branch";
2432
import { collectActionInputsPresence } from "./collect-inputs";
2533
import { updateCommentLink } from "./update-comment-link";
2634
import { formatTurnsFromData } from "./format-turns";
@@ -217,6 +225,30 @@ async function run() {
217225

218226
validateEnvironmentVariables();
219227

228+
// On PRs, .claude/ and .mcp.json in the checkout are attacker-controlled.
229+
// Restore them from the base branch before the CLI reads them.
230+
//
231+
// We read pull_request.base.ref from the payload directly because agent
232+
// mode's branchInfo.baseBranch defaults to "main" rather than the PR's
233+
// actual target (agent/index.ts). For issue_comment on a PR the payload
234+
// lacks base.ref, so we fall back to the mode-provided value — tag mode
235+
// fetches it from GraphQL; agent mode on issue_comment is an edge case
236+
// that at worst restores from the wrong trusted branch (still secure).
237+
if (isEntityContext(context) && context.isPR) {
238+
let restoreBase = baseBranch;
239+
if (
240+
isPullRequestEvent(context) ||
241+
isPullRequestReviewEvent(context) ||
242+
isPullRequestReviewCommentEvent(context)
243+
) {
244+
restoreBase = context.payload.pull_request.base.ref;
245+
validateBranchName(restoreBase);
246+
}
247+
if (restoreBase) {
248+
restoreConfigFromBase(restoreBase);
249+
}
250+
}
251+
220252
await setupClaudeCodeSettings(process.env.INPUT_SETTINGS);
221253

222254
await installPlugins(
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { execFileSync } from "child_process";
2+
import { rmSync } from "fs";
3+
4+
// Paths that are both PR-controllable and read from cwd at CLI startup.
5+
//
6+
// Deliberately excluded from the CLI's broader auto-edit blocklist:
7+
// .git/ — not tracked by git; PR commits cannot place files there.
8+
// Restoring it would also undo the PR checkout entirely.
9+
// .gitconfig — git reads ~/.gitconfig and .git/config, never cwd/.gitconfig.
10+
// .bashrc etc. — shells source these from $HOME; checkout cannot reach $HOME.
11+
// .vscode/.idea— IDE config; nothing in the CLI's startup path reads them.
12+
const SENSITIVE_PATHS = [
13+
".claude",
14+
".mcp.json",
15+
".claude.json",
16+
".gitmodules",
17+
".ripgreprc",
18+
];
19+
20+
/**
21+
* Restores security-sensitive config paths from the PR base branch.
22+
*
23+
* The CLI's non-interactive mode trusts cwd: it reads `.mcp.json`,
24+
* `.claude/settings.json`, and `.claude/settings.local.json` from the working
25+
* directory and acts on them before any tool-permission gating — executing
26+
* hooks (including SessionStart), setting env vars (NODE_OPTIONS, LD_PRELOAD,
27+
* PATH), running apiKeyHelper/awsAuthRefresh shell commands, and auto-approving
28+
* MCP servers. When this action checks out a PR head, all of these are
29+
* attacker-controlled.
30+
*
31+
* Rather than enumerate every dangerous key, this replaces the entire `.claude/`
32+
* tree and `.mcp.json` with the versions from the PR base branch, which a
33+
* maintainer has reviewed and merged. Paths absent on base are deleted.
34+
*
35+
* Known limitation: if a PR legitimately modifies `.claude/` and the CLI later
36+
* commits with `git add -A`, the revert will be included in that commit. This
37+
* is a narrow UX tradeoff for closing the RCE surface.
38+
*
39+
* @param baseBranch - PR base branch name. Must be pre-validated (branch.ts
40+
* calls validateBranchName on it before returning).
41+
*/
42+
export function restoreConfigFromBase(baseBranch: string): void {
43+
console.log(
44+
`Restoring ${SENSITIVE_PATHS.join(", ")} from origin/${baseBranch} (PR head is untrusted)`,
45+
);
46+
47+
// Fetch base first — if this fails we haven't touched the workspace and the
48+
// caller sees a clean error.
49+
execFileSync("git", ["fetch", "origin", baseBranch, "--depth=1"], {
50+
stdio: "inherit",
51+
});
52+
53+
// Delete PR-controlled versions. If the restore below fails for a given path,
54+
// that path stays deleted — the safe fallback (no attacker-controlled config).
55+
// A bare `git checkout` alone wouldn't remove files the PR added, so nuke first.
56+
for (const p of SENSITIVE_PATHS) {
57+
rmSync(p, { recursive: true, force: true });
58+
}
59+
60+
for (const p of SENSITIVE_PATHS) {
61+
try {
62+
execFileSync("git", ["checkout", `origin/${baseBranch}`, "--", p], {
63+
stdio: "pipe",
64+
});
65+
} catch {
66+
// Path doesn't exist on base — it stays deleted.
67+
}
68+
}
69+
70+
// `git checkout <ref> -- <path>` stages the restored files. Unstage so the
71+
// revert doesn't silently leak into commits the CLI makes later.
72+
try {
73+
execFileSync("git", ["reset", "--", ...SENSITIVE_PATHS], {
74+
stdio: "pipe",
75+
});
76+
} catch {
77+
// Nothing was staged, or paths don't exist on HEAD — either is fine.
78+
}
79+
}

0 commit comments

Comments
 (0)