Skip to content

Commit a903abf

Browse files
committed
fix(review): key fork-guard bypass on event name
GitHub documents the pull_request payload as empty for fork PRs, so !github.event.pull_request also admitted forks. Bypass only for non-pull_request events, the smoke run on push to main.
1 parent e0c44fe commit a903abf

2 files changed

Lines changed: 13 additions & 5 deletions

File tree

‎.github/workflows/claude-pr-review.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@ jobs:
4242
# Fork pull requests run without secrets, so the App token step would fail on an empty
4343
# private key before any review. Skip them rather than fail every outside contribution.
4444
# Compared by name, not head.repo.fork, which is also true for same-repo branches in a repo
45-
# that is itself a fork. No pull request means the smoke run on push to main, which must
46-
# still run.
45+
# that is itself a fork. A non-pull_request event is the smoke run on push to main, which
46+
# must still run; keyed on the event name because a fork's payload is documented as empty,
47+
# so testing for an absent payload would let forks through.
4748
if: >-
4849
github.event.pull_request.draft == false &&
49-
(!github.event.pull_request ||
50+
(github.event_name != 'pull_request' ||
5051
github.event.pull_request.head.repo.full_name == github.repository)
5152
runs-on: ubuntu-latest
5253
timeout-minutes: 15

‎tests/workflow-lint-test.sh‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,9 @@ fi
225225
# Comparing the head repo's full name to this repo is the question actually being asked.
226226
# - tests.yml runs this workflow on push to main, where there is no pull request and the head
227227
# repo expands to null. A bare comparison is then false and the smoke job silently skips on
228-
# main, so the guard must let a run without a pull request through.
228+
# main, so the guard must let a run without a pull request through -- keyed on the event
229+
# name, not on the payload being absent, because GitHub documents the pull_request payload
230+
# as empty for fork pull requests, and `!github.event.pull_request` would wave those in.
229231
job_gate=$(awk '/^ review:$/ { found = 1; next }
230232
found && /^ if:/ { print; in_if = 1; next }
231233
in_if && /^ / { print; next }
@@ -238,7 +240,12 @@ elif ! printf '%s\n' "$job_gate" | grep -qF 'github.event.pull_request.head.repo
238240
echo " and fail at the App token step:"
239241
printf '%s\n' "$job_gate" | sed 's/^/ /'
240242
failures=$((failures + 1))
241-
elif ! printf '%s\n' "$job_gate" | grep -qF '!github.event.pull_request ||'; then
243+
elif printf '%s\n' "$job_gate" | grep -qF '!github.event.pull_request'; then
244+
echo "FAIL the review job's no-PR bypass tests for an absent payload, which is also true of a"
245+
echo " fork pull request's documented empty payload, so forks reach the App token step:"
246+
printf '%s\n' "$job_gate" | sed 's/^/ /'
247+
failures=$((failures + 1))
248+
elif ! printf '%s\n' "$job_gate" | grep -qF "github.event_name != 'pull_request' ||"; then
242249
echo "FAIL the review job's fork guard is not bypassed when there is no pull request, so the"
243250
echo " smoke run on push to main skips instead of proving the workflow starts:"
244251
printf '%s\n' "$job_gate" | sed 's/^/ /'

0 commit comments

Comments
 (0)