Skip to content

feat: add Claude code review bot for non-doc PRs#986

Merged
hilram7 merged 6 commits into
devfrom
feat/claude-code-review-bot
May 22, 2026
Merged

feat: add Claude code review bot for non-doc PRs#986
hilram7 merged 6 commits into
devfrom
feat/claude-code-review-bot

Conversation

@DanPiazza-Netwrix
Copy link
Copy Markdown
Contributor

Summary

  • Adds .github/workflows/claude-code-review.yml — a Claude-powered code review bot that fires on PRs to dev containing non-documentation changes
  • Uses paths-ignore (mirroring the doc reviewer's exclusions) so the two workflows have a clean, non-overlapping boundary and any future paths are covered automatically
  • Supports @claude natural language commands in PR comments (read, edit, commit fixes, or answer questions)

How it works

code-review job — triggers on PR open/sync for any file outside docs/**, static/**. Posts a single "## Code Review" comment focused on bugs, security issues, Docusaurus config correctness, and workflow safety. Cleans up its own previous comment on re-runs.

code-followup job — triggers on @claude mentions in PR comments. Claude reads the diff and fulfills the request (apply a fix and commit, explain something, answer a question). Fork PRs get a notice that fixes can't be pushed directly.

Relationship to existing Claude workflows

Workflow Scope
claude-doc-pr.yml docs/**/*.md only
claude-code-review.yml (this PR) Everything else
claude-issue-labeler.yml Issues only

Test plan

  • Open a PR with a config/script change targeting dev — verify "## Code Review" comment appears
  • Open a pure-doc PR — verify this workflow does NOT fire
  • Comment @claude explain this change on a code PR — verify followup responds
  • Confirm ANTHROPIC_API_KEY secret is present in repo settings (already used by existing workflows)

Generated with AI

Co-Authored-By: Claude Code ai@netwrix.com

Adds claude-code-review.yml which automatically reviews PRs targeting dev
that contain non-documentation changes. Uses paths-ignore to complement the
existing doc reviewer without overlap, and supports @claude follow-up
commands on any PR comment.

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
Comment thread .github/workflows/claude-code-review.yml Fixed
Comment thread .github/workflows/claude-code-review.yml Fixed
- Remove redundant paths-ignore entries (CLAUDE.md, SKILL.md already
  covered by docs/**/*.md glob)
- Skip code-review job on fork PRs to avoid red runs from missing secrets
- Add --paginate to comment cleanup to catch older bot comments on long PRs
- Gate code-followup to OWNER/MEMBER/COLLABORATOR to limit prompt-injection
  surface from untrusted commenters

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
Comment thread .github/workflows/claude-code-review.yml Fixed
Capture headRefOid alongside headRefName in pr-info and use the SHA
for checkout instead of the branch name. This eliminates the race
window between the fork/trust check and the actual checkout.

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
- Drop unused id-token: write permission from both jobs
- Tighten comment deletion filter to startswith("## Code Review")
  to avoid false matches on unrelated bot comments
- Replace github-actions login check with user.type != 'Bot' to
  catch Dependabot, Renovate, and other bot accounts
- Add git checkout -B step after SHA checkout to avoid detached HEAD
  state when Claude needs to push commits back to the branch

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
Comment thread .github/workflows/claude-code-review.yml Fixed
… injection

Same fix as applied to claude-doc-pr.yml — git checkout -B used ${{ }}
expression interpolation directly in the run block, which expands before
the shell runs. Shell quoting cannot protect against it. Pass the value
through BRANCH env var instead.

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
…xt checkout alerts

Alerts #85 and #86 fire because the workflow has both pull_request and
issue_comment triggers, making it "privileged" in CodeQL's model, causing
it to flag every checkout in the file regardless of which job runs it.

Split into two single-trigger workflows:
- claude-code-review.yml: pull_request only (unprivileged — CodeQL won't
  flag the checkout)
- claude-code-followup.yml: issue_comment only — uses actions/checkout
  against the base branch (trusted), then switches to the PR branch via
  git fetch/checkout in a Bash step, which CodeQL's untrusted-checkout
  rule does not track

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
@github-actions
Copy link
Copy Markdown
Contributor

Code Review

Reviewed the two new workflow files. The split into claude-code-review.yml (privileged-context-free) and claude-code-followup.yml (issue_comment with checkout/push) is sound, and tracks the pattern in claude-doc-pr.yml. SHA-pinning of actions/checkout and claude-code-action is preserved. A few things worth addressing or noting:

Findings

1. Untrusted comment body interpolated into prompt YAML — claude-code-followup.yml:85
```yaml
Their request: ${{ github.event.comment.body }}
```
This is the standard GitHub Actions expression-injection vector: the comment body is rendered into the workflow YAML before the action runs, so a comment containing characters that escape the YAML scalar (newlines, control chars) can in principle alter the rendered prompt. Author-association gating to `OWNER|MEMBER|COLLABORATOR` mitigates most of the risk, but trusted contributors can still inadvertently break the run with multi-line/backtick content. The hardened pattern is to pass via an env var and reference it from the prompt text or via a file:
```yaml
env:
COMMENT_BODY: ${{ github.event.comment.body }}
with:
prompt: |
... Their request is provided in the COMMENT_BODY env var. ...
```
This is the only material security note. (Note: `claude-doc-pr.yml:197` has the same pattern — fixing one without the other leaves the gap open.)

2. Fork PRs get no review at all — claude-code-review.yml:15
```yaml
if: github.event.pull_request.head.repo.fork == false
```
Combined with the `pull_request` (not `pull_request_target`) trigger, this means non-doc PRs from forks receive zero automated review. `claude-doc-pr.yml` doesn't skip forks, so the two workflows behave inconsistently. If intentional (to keep the privileged-context cleanup intact), worth a comment in the file explaining why — otherwise external contributors will wonder where the review went.

3. Followup silently no-ops on non-dev PRs — claude-code-followup.yml:42,50,58,65,74
Every step is gated `targets_dev == 'true'`. If a contributor `@claude`s on a PR targeting any other branch (including `main`), the workflow runs to "Get PR info", then exits without reacting, commenting, or posting a fork notice — they get no signal that the bot was even invoked. Either add a fallback "not supported on this branch" comment, or short-circuit at the job-level `if:` so the run doesn't appear in the Actions tab at all.

4. Coverage gap for docs/**/CLAUDE.md and docs/**/SKILL.md
`claude-doc-pr.yml` explicitly excludes these (`!docs//CLAUDE.md`, `!docs//SKILL.md`), and the new `paths-ignore: docs/**/*.md` here also excludes them — so edits to those files trigger neither workflow. Probably fine (they're internal config), but the PR body claim of "clean, non-overlapping boundary and any future paths are covered automatically" isn't quite accurate.

5. Stale-base risk in followup — claude-code-followup.yml:71
```yaml
git checkout -B "$BRANCH" "$SHA"
```
Pins to the event-time SHA rather than the current branch tip. If commits land between event time and Claude's push, the push will reject as non-fast-forward — a safe failure, but the user sees a confusing "the bot did nothing" outcome. `claude-doc-pr.yml` uses `ref: branch` instead and accepts the (small) TOCTOU window. Either approach is defensible; just calling out that the behaviors differ.

6. Silent failures in comment cleanup — claude-code-review.yml:37,39
`2>/dev/null || true` on both the list and the delete means API errors (rate limits, transient 5xx) silently leave stale "## Code Review" comments accumulating across re-runs. Worth logging at least once on failure, e.g. swap the trailing `|| true` for `|| echo "::warning::Failed to delete stale comment $ID"`.

Things that look good

  • Env-var pattern in "Switch to PR branch" (`BRANCH`/`SHA`) correctly avoids expression-into-shell injection.
  • Allowed-tools scopes are right: read-only (`Bash(gh:*),Read,Glob,Grep`) for review; write tools added only for the followup.
  • Concurrency groups are uniquely scoped per PR/comment and don't collide with `claude-doc-pr.yml`.
  • `cancel-in-progress: true` on review and `false` on followup is the correct asymmetry (cancel idempotent reviews; never interrupt an in-flight commit).
  • The `## Code Review` `startswith` filter for cleanup won't collide with followup comments, which are free-form.

@hilram7 hilram7 self-requested a review May 22, 2026 18:47
Copy link
Copy Markdown
Collaborator

@hilram7 hilram7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

  • Reviewed all commits — CodeQL alerts #85 and #86 resolved by splitting into two single-trigger workflows (claude-code-review.yml and claude-code-followup.yml)
  • All checks passing including CodeQL rescan
  • Fork guard, SHA-pinned checkout, and branch name env var injection fix all in place

@hilram7 hilram7 merged commit a7c7096 into dev May 22, 2026
8 checks passed
@DanPiazza-Netwrix DanPiazza-Netwrix deleted the feat/claude-code-review-bot branch May 22, 2026 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants