Skip to content

ci: add Claudius PR Review workflow #3347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lklimek wants to merge 6 commits into
base: v3.1-dev
Choose a base branch
from
+36 −0
Open

ci: add Claudius PR Review workflow #3347

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9979307
ci: add Claudius PR Review workflow
lklimek Mar 16, 2026
5b5d153
Merge branch 'v3.1-dev' into chore/claudius-review-workflow
lklimek Mar 16, 2026
242ef6d
ci: switch to OAuth token and skip draft PRs
lklimek Mar 16, 2026
e8443b5
ci: restrict /claudius:review to trusted commenters
lklimek Mar 16, 2026
ae8808d
ci: change comment trigger to /review
lklimek Mar 16, 2026
e77497f
ci: set CLAUDE_CODE_EFFORT_LEVEL=max for review workflow
lklimek Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/claudius-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: "Claudius PR Review"

on:
pull_request_target:
types:
- labeled
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 31, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: Missing ready_for_review trigger type

If a draft PR is labeled claudius-review and later marked ready for review (without new commits), the review won't trigger. Adding ready_for_review to the types list covers this gap without the security issues of synchronize.

source: ['codex-general']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [SUGGESTION] line 6: Missing ready_for_review trigger type
  If a draft PR is labeled `claudius-review` and later marked ready for review (without new commits), the review won't trigger. Adding `ready_for_review` to the types list covers this gap without the security issues of `synchronize`.

- synchronize
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 31, 2026

Choose a reason for hiding this comment

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

🔴 Blocking: TOCTOU: synchronize on pull_request_target allows fork authors to re-trigger with secrets

pull_request_target + synchronize is a well-known attack vector. After a maintainer adds claudius-review (one-time trusted action), the label persists. Any push to the PR head — including from a fork — fires synchronize, the label check passes, and the workflow executes with ANTHROPIC_API_KEY and GITHUB_TOKEN.

Since the review action reads PR code, the fork author can push prompt-injection payloads (e.g. in CLAUDE.md) that run in the privileged context.

Fix: Remove synchronize from the pull_request_target types. If re-review on push is desired, require the label to be removed and re-added (maintainer-only action).

source: ['claude-security', 'codex-security', 'claude-general', 'codex-general']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [BLOCKING] line 7: TOCTOU: synchronize on pull_request_target allows fork authors to re-trigger with secrets
  `pull_request_target` + `synchronize` is a well-known attack vector. After a maintainer adds `claudius-review` (one-time trusted action), the label persists. Any push to the PR head — including from a fork — fires `synchronize`, the label check passes, and the workflow executes with `ANTHROPIC_API_KEY` and `GITHUB_TOKEN`.

Since the review action reads PR code, the fork author can push prompt-injection payloads (e.g. in `CLAUDE.md`) that run in the privileged context.

**Fix:** Remove `synchronize` from the `pull_request_target` types. If re-review on push is desired, require the label to be removed and re-added (maintainer-only action).

issue_comment:
Comment on lines +3 to +8
types:
- created

concurrency:
group: claudius-review-${{ github.event.pull_request.number || github.event.issue.number }}
cancel-in-progress: true

permissions:
contents: read
pull-requests: write
issues: write

jobs:
claudius-review:
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 31, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: Missing timeout-minutes

No timeout-minutes is set. GitHub defaults to 360 minutes (6 hours), which is excessive for an AI review action. Consider adding timeout-minutes: 15.

source: ['claude-general']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [SUGGESTION] line 22: Missing timeout-minutes
  No `timeout-minutes` is set. GitHub defaults to 360 minutes (6 hours), which is excessive for an AI review action. Consider adding `timeout-minutes: 15`.

name: Claudius PR Review
runs-on: ubuntu-24.04
env:
CLAUDE_CODE_EFFORT_LEVEL: max
if: |
(github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'claudius-review') && github.event.pull_request.draft == false) ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '/review') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association))
Comment on lines +27 to +29
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: labeled trigger re-runs review when unrelated labels are added

The condition checks contains(github.event.pull_request.labels.*.name, 'claudius-review') for all pull_request_target events, without distinguishing between labeled and synchronize actions. When the labeled event fires, github.event.pull_request.labels contains all current labels on the PR, not just the newly added one. So if claudius-review was added earlier and someone later adds an unrelated label (e.g., bug), the labeled event fires, the condition is satisfied, and the review workflow runs again unnecessarily.

The fix is to check github.event.label.name == 'claudius-review' specifically for the labeled action, while keeping the contains() check for synchronize events.

Suggested change
if: |
(github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'claudius-review') && github.event.pull_request.draft == false) ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '/review') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association))
if: |
(github.event_name == 'pull_request_target' && github.event.pull_request.draft == false && (
(github.event.action == 'labeled' && github.event.label.name == 'claudius-review') ||
(github.event.action == 'synchronize' && contains(github.event.pull_request.labels.*.name, 'claudius-review'))
)) ||
(github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '/review') && contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association))

source: ['codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [SUGGESTION] lines 27-29: `labeled` trigger re-runs review when unrelated labels are added
  The condition checks `contains(github.event.pull_request.labels.*.name, 'claudius-review')` for all `pull_request_target` events, without distinguishing between `labeled` and `synchronize` actions. When the `labeled` event fires, `github.event.pull_request.labels` contains *all* current labels on the PR, not just the newly added one. So if `claudius-review` was added earlier and someone later adds an unrelated label (e.g., `bug`), the `labeled` event fires, the condition is satisfied, and the review workflow runs again unnecessarily.

The fix is to check `github.event.label.name == 'claudius-review'` specifically for the `labeled` action, while keeping the `contains()` check for `synchronize` events.

steps:
- uses: lklimek/claudius-review-action@main
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 31, 2026

Choose a reason for hiding this comment

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

🔴 Blocking: Third-party action pinned to mutable @main with secret access

lklimek/claudius-review-action@main is from a personal GitHub account, pinned to a mutable branch ref, and receives ANTHROPIC_API_KEY and GITHUB_TOKEN. If the action repo is compromised or the owner pushes a malicious update, secrets are exfiltrated.

Fix: Pin to a specific commit SHA with a version comment.

source: ['claude-general', 'codex-general', 'codex-security']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [BLOCKING] line 29: Third-party action pinned to mutable @main with secret access
  `lklimek/claudius-review-action@main` is from a personal GitHub account, pinned to a mutable branch ref, and receives `ANTHROPIC_API_KEY` and `GITHUB_TOKEN`. If the action repo is compromised or the owner pushes a malicious update, secrets are exfiltrated.

**Fix:** Pin to a specific commit SHA with a version comment.

with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
Comment on lines +31 to +34
Copy link
Copy Markdown
Collaborator

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

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

🔴 Blocking: Action pinned to mutable branch, not commit SHA

lklimek/claudius-review-action@main references a mutable branch. This action receives CLAUDE_CODE_OAUTH_TOKEN and GITHUB_TOKEN and runs in a pull_request_target context, which grants access to base-repo secrets even for fork PRs. If the upstream action repository is compromised or force-pushed, an attacker gains access to these credentials and can write to any PR in this repo.

GitHub's security hardening guide recommends pinning third-party actions to a full-length commit SHA. This is especially critical here because: (1) the action is from a personal account, not a verified org, (2) it runs in a pull_request_target context with elevated trust, and (3) it receives secrets. This repo already follows SHA-pinning in milestone.yml (actions/github-script@f28e40c7...), making @main an outlier.

Suggested change
- uses: lklimek/claudius-review-action@main
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}
- uses: lklimek/claudius-review-action@<full-40-char-commit-sha> # pin to audited commit

source: ['claude', 'codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `.github/workflows/claudius-review.yml`:
- [BLOCKING] lines 31-34: Action pinned to mutable branch, not commit SHA
  `lklimek/claudius-review-action@main` references a mutable branch. This action receives `CLAUDE_CODE_OAUTH_TOKEN` and `GITHUB_TOKEN` and runs in a `pull_request_target` context, which grants access to base-repo secrets even for fork PRs. If the upstream action repository is compromised or force-pushed, an attacker gains access to these credentials and can write to any PR in this repo.

GitHub's security hardening guide recommends pinning third-party actions to a full-length commit SHA. This is especially critical here because: (1) the action is from a personal account, not a verified org, (2) it runs in a `pull_request_target` context with elevated trust, and (3) it receives secrets. This repo already follows SHA-pinning in `milestone.yml` (`actions/github-script@f28e40c7...`), making `@main` an outlier.

claude_model: "opus"
trigger_label: "claudius-review"
Loading