Skip to content

Commit 6455d2d

Browse files
authored
fix(ci): make AI review safe + functional on fork PRs (aws#6087)
The review job runs under pull_request_target (trusted context with the Bedrock role + secrets). It was checking out the fork's head SHA, which actions/checkout now refuses ('pwn request' guard) — so the workflow failed on every fork PR (~90% of PRs), e.g. run 29965762436 on aws#6083. Rework to the safe diff-only pattern (mirrors aws/*-agentcore pr-security-review): - Check out the BASE repo, never fork head code — untrusted PR code is never executed in the privileged job. - Fetch the PR diff via the API into /tmp/pr.diff as read-only ground truth. - Remove Bash from allowedTools; Claude reads the diff + uses Read/Grep/Glob against the trusted base tree for context. Skip cleanly on empty diffs. Does NOT use allow-unsafe-pr-checkout, which would expose secrets to fork code.
1 parent 8c3096f commit 6455d2d

1 file changed

Lines changed: 39 additions & 4 deletions

File tree

‎.github/workflows/ai-code-review.yml‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,29 +71,62 @@ jobs:
7171
runs-on: ubuntu-latest
7272
needs: [wait-for-approval]
7373
steps:
74+
# SECURITY: this job runs in the trusted pull_request_target context (it
75+
# holds the Bedrock role + secrets). We therefore check out the BASE repo
76+
# at the PR's base branch — never the fork's head code — so untrusted PR
77+
# code is never executed here ("pwn request" prevention). The PR contents
78+
# are pulled in as a read-only diff below, not as an executable tree.
7479
- uses: actions/checkout@v4
7580
with:
76-
ref: ${{ github.event.pull_request.head.sha }}
81+
ref: ${{ github.event.pull_request.base.sha }}
7782
fetch-depth: 1
7883

84+
# Fetch the PR diff via the API (does not execute any fork code) and store
85+
# it as a static file. This is the ground truth Claude reviews.
86+
- name: Fetch PR diff
87+
id: diff
88+
env:
89+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
90+
PR_NUMBER: ${{ github.event.pull_request.number }}
91+
REPO: ${{ github.repository }}
92+
run: |
93+
set -euo pipefail
94+
gh api "repos/$REPO/pulls/$PR_NUMBER" \
95+
-H "Accept: application/vnd.github.v3.diff" > /tmp/pr.diff
96+
BYTES=$(wc -c < /tmp/pr.diff)
97+
echo "bytes=$BYTES" >> "$GITHUB_OUTPUT"
98+
echo "PR diff: $BYTES bytes"
99+
79100
- name: Configure AWS Credentials
101+
if: steps.diff.outputs.bytes != '0'
80102
uses: aws-actions/configure-aws-credentials@v4
81103
with:
82104
role-to-assume: ${{ secrets.CODE_REVIEW_ROLE }}
83105
aws-region: us-west-2
84106

85107
- uses: anthropics/claude-code-action@v1
108+
if: steps.diff.outputs.bytes != '0'
86109
with:
87110
github_token: ${{ secrets.GITHUB_TOKEN }}
88111
use_bedrock: "true"
89112
track_progress: true
113+
# Bash is intentionally NOT allowed. The PR diff at /tmp/pr.diff is the
114+
# only ground truth; the model reads it and uses Read/Grep/Glob against
115+
# the trusted base checkout for context. It must not execute commands
116+
# (which could run untrusted PR content) nor re-run git.
90117
claude_args: |
91118
--model us.anthropic.claude-opus-4-8
92-
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"
119+
--allowedTools "Read Grep Glob mcp__github_inline_comment__create_inline_comment"
93120
prompt: |
94121
REPO: ${{ github.repository }}
95122
PR NUMBER: ${{ github.event.pull_request.number }}
96123
124+
The complete PR diff is at `/tmp/pr.diff` — read it first with the
125+
Read tool. That file is the ground truth for what this PR changes; do
126+
not run git or any shell commands. For context (callers of changed
127+
functions, existing patterns, project conventions), use Read/Grep/Glob
128+
against the checked-out base repository.
129+
97130
Review this pull request for the SageMaker Python SDK. Focus on:
98131
- Correctness: bugs, incorrect API/argument usage, breaking changes
99132
to public interfaces, backward-incompatibility for SDK consumers
@@ -102,5 +135,7 @@ jobs:
102135
- Performance considerations
103136
- Missing or inadequate tests for changed behavior
104137
105-
Post specific issues as inline comments. Skip nits and style the
106-
linters already enforce. If the PR looks clean, say so briefly.
138+
Post specific issues as inline comments via the
139+
mcp__github_inline_comment__create_inline_comment tool. Skip nits and
140+
style the linters already enforce. If the PR looks clean, say so
141+
briefly.

0 commit comments

Comments
 (0)