[CRAFTING] Add AI code review with Claude Code Action and AWS Bedrock#2199
[CRAFTING] Add AI code review with Claude Code Action and AWS Bedrock#2199leonardomendix wants to merge 1 commit intomainfrom
Conversation
leonardomendix
force-pushed
the
crafting/enabling-claude-code-review-action
branch
3 times, most recently
from
67d9448 to
ea900c9
Compare
leonardomendix
force-pushed
the
crafting/enabling-claude-code-review-action
branch
from
ea900c9 to
405e603
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds a GitHub Actions workflow that integrates Claude Code Action (via AWS Bedrock) for automated AI code review on every internal PR, plus an interactive @claude trigger for follow-up questions.
Scope: CI-only change — no runtime/widget code is touched, so no semver bump or CHANGELOG.md entry is needed. ✅
Overall assessment
The approach is architecturally sound: OIDC for AWS credentials, pinned action hashes, fork-PR filtering on the auto-review job, and a single-source-of-truth prompt that delegates to .github/copilot-instructions.md. However, there are two security issues that should be addressed before merging, and a few smaller concerns.
🔴 Critical – Security
1. --dangerously-skip-permissions in both jobs
Both the auto-review and interactive jobs pass --dangerously-skip-permissions to Claude Code. This flag disables all permission prompts, allowing Claude to execute arbitrary shell commands, read/write arbitrary files, and perform network requests without confirmation. Combined with pull-requests: write and live AWS credentials in the runner environment, a prompt-injected payload inside a reviewed PR (e.g. a specially crafted comment or file) could exfiltrate secrets or corrupt the repository.
At minimum, remove this flag or replace it with an explicit allowlist via Claude Code's permission configuration. If the action genuinely needs it for read-only review operations, document the threat model and confirm the AWS IAM role is scoped to read-only Bedrock inference only.
2. interactive job has no fork/contributor guard
The auto-review job correctly checks github.event.pull_request.head.repo.full_name == 'mendix/web-widgets'. The interactive job has no such guard — any GitHub user (including external contributors from forks) can write @claude do X in a PR comment and trigger an unrestricted Claude Code run with AWS credentials. At minimum, add:
if: |
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') &&
github.event.issue.pull_request &&
github.event.comment.author_association != 'NONE' &&
github.event.comment.author_association != 'FIRST_TIME_CONTRIBUTOR') ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude') &&
github.event.comment.author_association != 'NONE')Or restrict to MEMBER / COLLABORATOR author associations only.
🟡 Moderate
3. Renovate bot exclusion is incomplete
The auto-review job skips PRs from uicontent, but the description says it also skips Renovate. Renovate's GitHub login is renovate[bot], not uicontent. Add it explicitly:
github.event.pull_request.user.login != 'renovate[bot]' &&
github.event.pull_request.user.login != 'uicontent' &&4. No job timeout
Neither job sets a timeout-minutes. A runaway Claude session could consume GitHub Actions minutes indefinitely. Add timeout-minutes: 15 (or whatever is reasonable) to each job.
🟢 Minor / Positive notes
- Pinned all three third-party action versions to full commit SHAs — excellent supply-chain hygiene. ✅
- OIDC (
id-token: write) instead of long-lived AWS keys — correct approach. ✅ concurrencyblock withcancel-in-progress: trueavoids redundant review runs on rapid pushes. ✅fetch-depth: 1is fine for a review-only workflow. ✅- Delegating the review prompt to
.github/copilot-instructions.mdkeeps a single source of truth. ✅ github.event.pull_request.draft == falseprevents reviews on in-progress work. ✅
Please address the two 🔴 security issues before merging.
| Review scope: | ||
| - Focus ONLY on changed files (the diff) | ||
| - Ignore dist/, lockfile changes, and generated files | ||
| - For Renovate/dependency-only PRs, check for breaking changes only |
There was a problem hiding this comment.
Security: --dangerously-skip-permissions should be removed.
This flag disables all Claude Code permission checks, allowing unrestricted shell execution, file I/O, and network access inside the runner. Combined with live AWS credentials and pull-requests: write, a prompt-injection attack embedded in a reviewed file could exfiltrate secrets.
For a review-only workflow no write-filesystem or shell-execution permissions are needed. Remove this flag entirely, or configure an explicit read-only allowlist in the Claude Code settings rather than bypassing all guards.
| interactive: | ||
| name: Claude Interactive | ||
| if: | | ||
| (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') && github.event.issue.pull_request) || |
There was a problem hiding this comment.
Security: interactive job has no contributor/fork guard.
Any GitHub user (including external contributors on fork PRs) can write @claude … in a comment and trigger a Claude Code run with your AWS credentials and pull-requests: write access. Unlike the auto-review job, there is no head.repo.full_name or author_association check here.
Restrict to trusted contributors, for example:
if: |
(github.event_name == 'issue_comment' &&
contains(github.event.comment.body, '@claude') &&
github.event.issue.pull_request &&
contains(fromJSON('\["MEMBER","COLLABORATOR","OWNER"]\\), github.event.comment.author_association)) ||
(github.event_name == 'pull_request_review_comment' &&
contains(github.event.comment.body, '@claude') &&
contains(fromJSON('\["MEMBER","COLLABORATOR","OWNER"]\\), github.event.comment.author_association))
| trigger_phrase: "@claude" | ||
| claude_args: | | ||
| --model eu.anthropic.claude-sonnet-4-6 | ||
| --dangerously-skip-permissions |
There was a problem hiding this comment.
Security: same --dangerously-skip-permissions concern as the auto-review job.
Remove or replace with a minimal allowlist. The interactive job poses a higher risk because its trigger conditions are currently unguarded (see the if: comment on this job).
| if: | | ||
| github.event_name == 'pull_request' && | ||
| github.event.pull_request.head.repo.full_name == 'mendix/web-widgets' && | ||
| github.event.pull_request.user.login != 'uicontent' && |
There was a problem hiding this comment.
Renovate bot not excluded.
The PR description says Renovate PRs are skipped, but only uicontent is filtered here. Renovate uses the login renovate[bot]. Add:
github.event.pull_request.user.login != 'renovate[bot]' &&
| github.event.pull_request.user.login != 'uicontent' && | ||
| github.event.pull_request.draft == false | ||
| runs-on: ubuntu-latest | ||
| permissions: |
There was a problem hiding this comment.
Missing job timeout.
Without a timeout-minutes, a runaway Claude session could consume GitHub Actions minutes indefinitely. Add:
runs-on: ubuntu-latest
timeout-minutes: 151 participant
Pull request type
No code changes (changes to documentation, CI, metadata, etc.)
Description