fix(ci): gate PR review and triage on write permission #5191
+188
−72
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,41 +20,118 @@ permissions: | |
| pull-requests: 'write' | ||
|
|
||
| jobs: | ||
| authorize: | ||
| # Gate PR and /triage-comment triggers on the principal having write+ | ||
| # permission: the PR author for PR events, the commenter for /triage | ||
| # comments. Replaces the old eligibility checks based on same-repo PRs and | ||
| # comment author_association, so fork PRs by trusted authors are triaged. | ||
| # `issues` and `workflow_dispatch` triggers do not need this gate. | ||
| if: |- | ||
| github.repository == 'QwenLM/qwen-code' && | ||
| (github.event_name == 'pull_request_target' || | ||
| (github.event_name == 'issue_comment' && | ||
| startsWith(github.event.comment.body, '@qwen-code /triage'))) | ||
| runs-on: 'ubuntu-latest' | ||
| timeout-minutes: 5 | ||
| permissions: | ||
| contents: 'read' | ||
| outputs: | ||
| should_run: '${{ steps.perm.outputs.should_run }}' | ||
| steps: | ||
| - name: 'Check principal write permission' | ||
| id: 'perm' | ||
| env: | ||
| # CI_BOT_PAT (not GITHUB_TOKEN): reading a user's collaborator | ||
| # permission requires write/maintain/admin access, which the | ||
| # GITHUB_TOKEN with contents:read does not have. Safe here — this job | ||
| # runs no agent, checks out nothing, and processes no untrusted PR | ||
| # content; it only reads event metadata and calls one read API. | ||
| GH_TOKEN: '${{ secrets.CI_BOT_PAT }}' | ||
| EVENT_NAME: '${{ github.event_name }}' | ||
| PR_AUTHOR: '${{ github.event.pull_request.user.login }}' | ||
| COMMENT_USER: '${{ github.event.comment.user.login }}' | ||
| run: |- | ||
| set -euo pipefail | ||
| case "$EVENT_NAME" in | ||
| pull_request_target) principal="$PR_AUTHOR" ;; | ||
| issue_comment) principal="$COMMENT_USER" ;; | ||
| *) principal="" ;; | ||
| esac | ||
| if [ -z "$principal" ]; then | ||
| echo "No principal resolved for ${EVENT_NAME}; denying." >> "$GITHUB_STEP_SUMMARY" | ||
| echo "should_run=false" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| # Fail closed: any API error or non-write permission denies the run. | ||
| api_error_file="$(mktemp)" | ||
| if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${principal}/permission" --jq '.permission' 2>"$api_error_file")"; then | ||
| api_error="$(cat "$api_error_file")" | ||
| rm -f "$api_error_file" | ||
| api_error="${api_error:-unknown error}" | ||
| api_error="${api_error//$'\r'/ }" | ||
| api_error="${api_error//$'\n'/ }" | ||
| echo "::error::Permission API call failed for ${principal}: ${api_error}" | ||
| echo "Failed to check permission for ${principal} (API error: ${api_error}); denying." >> "$GITHUB_STEP_SUMMARY" | ||
| echo "should_run=false" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
|
Collaborator
There was a problem hiding this comment. [Suggestion] Same pattern as the review workflow — Consider:
— qwen3.7-max via Qwen Code /review |
||
| fi | ||
| rm -f "$api_error_file" | ||
| case "$permission" in | ||
| admin|maintain|write) | ||
| echo "should_run=true" >> "$GITHUB_OUTPUT" | ||
| ;; | ||
| *) | ||
| echo "Denying triage: ${principal} permission is '${permission}' (needs write)." >> "$GITHUB_STEP_SUMMARY" | ||
| echo "should_run=false" >> "$GITHUB_OUTPUT" | ||
| ;; | ||
| esac | ||
|
|
||
| triage: | ||
| needs: ['authorize'] | ||
| timeout-minutes: 30 | ||
| concurrency: | ||
| # GitHub evaluates concurrency before the job `if`, but after `needs`. | ||
| # Keep non-runnable PR/comment triggers out of the shared per-number | ||
| # group so they cannot cancel or replace an authorized run. | ||
| group: >- | ||
| ${{ | ||
| ( | ||
| (github.event_name == 'pull_request_target' && | ||
| (github.event.pull_request.draft == true || | ||
| needs.authorize.outputs.should_run != 'true')) || | ||
| (github.event_name == 'issue_comment' && | ||
| needs.authorize.outputs.should_run != 'true') | ||
| ) && | ||
| format('{0}-run-{1}', github.workflow, github.run_id) || | ||
| format('{0}-{1}', github.workflow, github.event.issue.number || github.event.pull_request.number || github.event.inputs.number) | ||
| }} | ||
| cancel-in-progress: >- | ||
| ${{ | ||
| github.event_name == 'issues' || | ||
| github.event_name == 'workflow_dispatch' || | ||
| (((github.event_name == 'pull_request_target' && | ||
| github.event.pull_request.draft == false) || | ||
| (github.event_name == 'issue_comment' && | ||
| startsWith(github.event.comment.body, '@qwen-code /triage'))) && | ||
| needs.authorize.outputs.should_run == 'true') | ||
| }} | ||
| runs-on: 'ubuntu-latest' | ||
| # startsWith (not contains) prevents false triggers from comments that | ||
| # mention the phrase in quoted text or mid-sentence descriptions. | ||
| # always() so the job still evaluates when the upstream `authorize` job is | ||
| # skipped (issues / workflow_dispatch paths, which need no permission gate). | ||
| if: >- | ||
| always() && | ||
| github.repository == 'QwenLM/qwen-code' && ( | ||
| github.event_name == 'issues' || | ||
| github.event_name == 'workflow_dispatch' || | ||
| ( | ||
| ((github.event_name == 'pull_request_target' && | ||
| github.event.pull_request.draft == false) || | ||
| (github.event_name == 'issue_comment' && | ||
| startsWith(github.event.comment.body, '@qwen-code /triage'))) && | ||
| needs.authorize.outputs.should_run == 'true' | ||
| ) | ||
| ) | ||
| steps: | ||
| - name: 'Checkout repo' | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] When the authorize job denies a
/reviewcommand (non-write user or API error), thisack-review-requestjob is skipped because itsifrequiresneeds.authorize.outputs.should_review == 'true'and it lacksalways(). No PR comment, reaction, or error message is posted — the user who typed/reviewgets zero feedback about why nothing happened.Consider adding a lightweight fallback job that posts a brief comment when authorize denies:
— qwen3.7-max via Qwen Code /review