Skip to content

Commit a34dbb5

Browse files
cliffhallclaude
andcommitted
ci(claude): address review — broaden config strip, refuse fork @claude, harden existing job
Addresses review on #1339 (plus the analogous finding from modelcontextprotocol/servers#4222 which applies here too). In the new claude-fork-review job: - Strip step now covers the full sensitive-path set the action's own restoreConfigFromBase handles (.claude/, .husky/, .mcp.json, .claude.json, .gitmodules, .ripgreprc, CLAUDE.md, CLAUDE.local.md) rather than only .mcp.json. Block comment names .claude/settings.json hooks as the primary threat (arbitrary shell on SessionStart/PreToolUse, runs before --allowedTools applies) and frames the step as defense-in-depth that survives future action versions narrowing restoreConfigFromBase. - Adds a concurrency group keyed on PR number (cancel-in-progress: false) so rapid label→unlabel→relabel cycles don't spawn parallel runs or leave half-finished reviews. - Drops `issues: read` from permissions — label removal on a PR is covered by pull-requests: write alone. - Rewords the "only outbound HTTP" comment to "only outbound HTTP Claude can direct" — the runner itself talks to api.anthropic.com / api.github.com / the Actions cache; the claim is about Claude's tool surface, not the runner. In the existing first-party claude job: - Refuses @claude mentions on fork PRs (steps.pr.outputs.repo != github.repository) with a comment pointing at the claude-review label. Closes the back door where a maintainer @claude'ing a fork PR would have Claude read untrusted fork content with unrestricted Bash and ANTHROPIC_API_KEY in scope. Checkout-PR-branch and run-claude steps short-circuit on cross-repo PRs. - Pins the action to SHA 12310e4417c3473095c957cb311b3cf59a38d659 (v1.0.99) for symmetry with the fork-review job. The AJV-schema-drift crash family applies to both paths. CONTRIBUTING.md: documents the claude-review label flow and the diff-scoped nature of the automated review so external contributors understand what they're getting and maintainers remain responsible for whole-file audits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9f3bf09 commit a34dbb5

2 files changed

Lines changed: 78 additions & 28 deletions

File tree

.github/workflows/claude.yml

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,30 @@ jobs:
6969
core.setOutput('sha', pr.data.head.sha);
7070
core.setOutput('repo', pr.data.head.repo.full_name);
7171
72+
# Refuse @claude mentions on fork PRs from this (broader) job. Even with
73+
# the maintainer collaborator gate above, a maintainer @claude'ing a fork
74+
# PR would have Claude read untrusted fork content with `Bash` and the
75+
# ANTHROPIC_API_KEY in scope — that's an exfiltration path. Fork PRs must
76+
# go through the hardened `claude-fork-review` job via the `claude-review`
77+
# label instead. This step posts a comment with that pointer and the
78+
# downstream checkout / run-claude steps short-circuit on cross-repo PRs.
79+
- name: Refuse @claude on fork PRs (point to claude-review label)
80+
if: steps.pr.outcome == 'success' && steps.pr.outputs.repo != github.repository
81+
uses: actions/github-script@v8
82+
with:
83+
script: |
84+
const prNumber = context.eventName === 'issue_comment'
85+
? context.issue.number
86+
: context.payload.pull_request.number;
87+
await github.rest.issues.createComment({
88+
owner: context.repo.owner,
89+
repo: context.repo.repo,
90+
issue_number: prNumber,
91+
body: '`@claude` mentions are not supported on fork PRs from this workflow path — Claude would read untrusted fork content with broader tooling than is safe. To get a Claude review on this PR, a maintainer can apply the `claude-review` label, which runs the hardened, sandboxed review job.'
92+
});
93+
7294
- name: Checkout PR branch
73-
if: steps.pr.outcome == 'success'
95+
if: steps.pr.outcome == 'success' && steps.pr.outputs.repo == github.repository
7496
uses: actions/checkout@v6
7597
with:
7698
ref: ${{ steps.pr.outputs.sha }}
@@ -85,7 +107,15 @@ jobs:
85107

86108
- name: Run Claude Code
87109
id: claude
88-
uses: anthropics/claude-code-action@v1
110+
# Skip on fork PRs — they go through claude-fork-review via the label.
111+
if: steps.pr.outcome != 'success' || steps.pr.outputs.repo == github.repository
112+
# Pinned to v1.0.99 for the same AJV-schema-drift / version-regression
113+
# reasons documented on the fork-review job below. SHA pinning applies
114+
# symmetrically: the first-party path also reads PR diffs (and on a
115+
# @claude mention of an authored-on-fork PR by a collaborator, reads
116+
# the proposed code), so a broken action version here breaks reviews
117+
# the same way it would on the fork-review path.
118+
uses: anthropics/claude-code-action@12310e4417c3473095c957cb311b3cf59a38d659 # v1.0.99
89119
with:
90120
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
91121

@@ -115,23 +145,29 @@ jobs:
115145
# - Mitigations: (a) no Bash glob, no Edit, no WebFetch — Claude can
116146
# only post inline comments, read PR metadata via narrow `gh`
117147
# commands, and query the MCP docs server at modelcontextprotocol.io
118-
# (the only network egress; the fork's `.mcp.json` is removed and
119-
# replaced with a base-repo-controlled config — see the "Prepare
120-
# trusted MCP config" step); (b) no fork code is ever executed (no
121-
# install, build, or test steps); (c) the GITHUB_TOKEN is scoped to
122-
# pull-requests:write only; (d) the ANTHROPIC_API_KEY exists in the
123-
# runner env but isn't reachable through Claude's allowed tool
124-
# surface.
148+
# (the only outbound HTTP Claude can direct; the fork's `.mcp.json`,
149+
# `.claude/`, `CLAUDE.md`, `.husky/`, etc. are all stripped after
150+
# checkout — see the "Strip fork-supplied CLI config" step); (b) no
151+
# fork code is ever executed (no install, build, or test steps);
152+
# (c) the GITHUB_TOKEN is scoped to pull-requests:write only;
153+
# (d) the ANTHROPIC_API_KEY exists in the runner env but isn't
154+
# reachable through Claude's allowed tool surface.
125155
# ---------------------------------------------------------------------------
126156
claude-fork-review:
127157
if: |
128158
github.event_name == 'pull_request_target' &&
129159
github.event.label.name == 'claude-review'
130160
runs-on: ubuntu-latest
161+
# Coalesce rapid label→unlabel→relabel cycles per PR. cancel-in-progress
162+
# is false so an in-flight review finishes (and removes the label) rather
163+
# than being aborted partway, which would otherwise leave the PR in a
164+
# half-reviewed state.
165+
concurrency:
166+
group: claude-fork-review-${{ github.event.pull_request.number }}
167+
cancel-in-progress: false
131168
permissions:
132169
contents: read
133170
pull-requests: write
134-
issues: read
135171
steps:
136172
# Check out the FORK head explicitly. We use a separate, least-privileged
137173
# token here and disable credential persistence so nothing fork-side can
@@ -144,29 +180,39 @@ jobs:
144180
fetch-depth: 1
145181
persist-credentials: false
146182

147-
# Prepare a trusted, base-repo-controlled MCP config for the review run.
183+
# Strip ALL fork-supplied Claude/CLI config from cwd before launching the
184+
# action, then write a trusted MCP config under $RUNNER_TEMP.
185+
#
186+
# The most dangerous of these is `.claude/settings.json`: Claude Code
187+
# reads it from cwd at startup and will execute `SessionStart` /
188+
# `PreToolUse` hooks declared in it BEFORE any --allowedTools allowlist
189+
# is applied. A fork shipping a `.claude/settings.json` with a hook
190+
# would get arbitrary shell on the runner, bypassing every tool
191+
# restriction on this job. `.mcp.json` is the analogous threat for MCP
192+
# servers (auto-discovered from cwd, would expose Claude to attacker-
193+
# controlled servers as an exfiltration channel). The remaining paths
194+
# (CLAUDE.md, CLAUDE.local.md, .claude.json, .husky/, .gitmodules,
195+
# .ripgreprc) are the rest of the config-discovery surface that the
196+
# action treats as sensitive.
148197
#
149-
# Why we `rm -f .mcp.json`:
150-
# Claude Code auto-discovers a project-level `.mcp.json` from the
151-
# working directory in addition to anything passed via `--mcp-config`.
152-
# The fork's checkout above may contain a `.mcp.json` that has been
153-
# modified to point at attacker-controlled MCP servers, which would
154-
# become an exfiltration channel the moment Claude connected to it
155-
# (an injection in the diff could coerce tool calls that leak review
156-
# context). Deleting the fork's copy guarantees the only MCP servers
157-
# in scope for this run are the ones in our trusted, inline config
158-
# below.
198+
# `anthropics/claude-code-action` itself restores all of these from the
199+
# base branch via `restoreConfigFromBase` before launching the CLI, so
200+
# in v1.0.99 the fork-review job is already safe on this axis. This
201+
# step is defense-in-depth: it ensures the security model survives
202+
# future action versions that might narrow or remove that behavior —
203+
# exactly the regression class we SHA-pinned to defend against.
159204
#
160-
# The trusted config is written under $RUNNER_TEMP (outside the fork
161-
# checkout, so the fork cannot shadow it) and exposes only the
205+
# The trusted MCP config is written under $RUNNER_TEMP (outside the
206+
# fork checkout, so the fork cannot shadow it) and exposes only the
162207
# read-only MCP docs server at https://modelcontextprotocol.io/mcp.
163208
# Combined with no WebFetch and no unrestricted Bash in --allowedTools,
164-
# this means the only outbound HTTP this job can make is to
165-
# modelcontextprotocol.io — useful for protocol lookups while
166-
# reviewing, with no other network egress.
167-
- name: Prepare trusted MCP config
209+
# the only outbound HTTP Claude can direct is to modelcontextprotocol.io
210+
# — useful for protocol lookups while reviewing, with no other network
211+
# egress reachable through Claude's tool surface.
212+
- name: Strip fork-supplied CLI config + write trusted MCP config
168213
run: |
169-
rm -f .mcp.json
214+
rm -rf .claude .husky
215+
rm -f .mcp.json .claude.json .gitmodules .ripgreprc CLAUDE.md CLAUDE.local.md
170216
mkdir -p "$RUNNER_TEMP/claude-fork-review"
171217
printf '%s\n' '{"mcpServers":{"mcp-docs":{"type":"http","url":"https://modelcontextprotocol.io/mcp"}}}' > "$RUNNER_TEMP/claude-fork-review/mcp.json"
172218
echo "FORK_REVIEW_MCP_CONFIG=$RUNNER_TEMP/claude-fork-review/mcp.json" >> "$GITHUB_ENV"

CONTRIBUTING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ We're actively developing **Inspector V2** to address architectural and UX impro
3030
7. Submit a pull request
3131
8. PRs will be reviewed by maintainers
3232

33+
### Automated Claude review on fork PRs
34+
35+
Maintainers can apply the `claude-review` label to a fork PR to trigger an automated review by Claude. The job runs in a hardened, sandboxed mode: Claude sees the PR diff and can post inline comments, but cannot install, build, run, or check out the rest of the codebase. As a result, Claude's review is **diff-scoped** — it sees the hunks under change but not surrounding file context — so maintainers remain responsible for whole-file audits and broader architectural review. The label is removed automatically after each run; re-apply it to trigger a fresh review.
36+
3337
## Code of Conduct
3438

3539
This project follows our [Code of Conduct](CODE_OF_CONDUCT.md). Please read it before contributing.

0 commit comments

Comments
 (0)