9ddce40de8
* 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).
80 lines
3.1 KiB
TypeScript
80 lines
3.1 KiB
TypeScript
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 <ref> -- <path>` 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.
|
|
}
|
|
}
|