From c7db5f02c2b0ae0123a2af34fbc79721aac90a57 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Tue, 16 Jun 2026 19:18:00 +0800 Subject: [PATCH 1/3] fix(ci): gate PR review and triage on write permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the per-path author_association checks (and the same-repo-only fork check in triage) with a dedicated `authorize` job that resolves the triggering principal — the PR author for automatic PR events, the commenter for /review and /triage commands, the requester for review_requested — and verifies it has admin/maintain/write permission via the collaborators API. This lets fork PRs by write-permission authors qualify for automatic review and triage (author_association reports such authors as CONTRIBUTOR from a fork, so they were previously skipped), and tightens command triggers to real write permission instead of the author_association proxy. The gate fails closed: any API error or non-write permission denies the run. The authorize job uses CI_BOT_PAT because reading collaborator permission requires write/maintain/admin access; it runs no agent, checks out nothing, and processes no untrusted PR content, so holding the PAT there is safe. External actors without write permission still cannot trigger the model-calling jobs. --- .github/workflows/qwen-code-pr-review.yml | 134 +++++++++++++--------- .github/workflows/qwen-triage.yml | 90 ++++++++++++--- 2 files changed, 156 insertions(+), 68 deletions(-) diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index 6500ad2ccc9..de66638cc1f 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -46,32 +46,27 @@ concurrency: jobs: ack-review-request: # KEEP IN SYNC with review-pr.if (explicit-trigger branches). + # Authorization is delegated to the `authorize` job (write+ permission); + # this `if` only matches the /review command shape. + needs: ['authorize'] if: |- - (github.event_name == 'issue_comment' && + needs.authorize.outputs.should_review == 'true' && + ((github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.issue.state == 'open' && (github.event.comment.body == '@qwen-code /review' || startsWith(github.event.comment.body, '@qwen-code /review ') || - startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) || + startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n')))) || (github.event_name == 'pull_request_review_comment' && github.event.pull_request.state == 'open' && (github.event.comment.body == '@qwen-code /review' || startsWith(github.event.comment.body, '@qwen-code /review ') || - startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) || + startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n')))) || (github.event_name == 'pull_request_review' && github.event.pull_request.state == 'open' && (github.event.review.body == '@qwen-code /review' || startsWith(github.event.review.body, '@qwen-code /review ') || - startsWith(github.event.review.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.review.author_association == 'OWNER' || - github.event.review.author_association == 'MEMBER' || - github.event.review.author_association == 'COLLABORATOR')) + startsWith(github.event.review.body, format('@qwen-code /review{0}', '\n'))))) concurrency: group: 'qwen-pr-ack-${{ github.event.issue.number || github.event.pull_request.number }}' cancel-in-progress: false @@ -128,15 +123,14 @@ jobs: echo "bot_login=qwen-code-ci-bot" >> "$GITHUB_OUTPUT" delay-automatic-review: + needs: ['authorize'] if: |- github.event_name == 'pull_request_target' && (github.event.action == 'opened' || github.event.action == 'synchronize') && github.event.pull_request.state == 'open' && !github.event.pull_request.draft && - (github.event.pull_request.author_association == 'OWNER' || - github.event.pull_request.author_association == 'MEMBER' || - github.event.pull_request.author_association == 'COLLABORATOR') + needs.authorize.outputs.should_review == 'true' runs-on: 'ubuntu-latest' # Configured in repo settings with a 30-minute wait timer. environment: @@ -170,30 +164,75 @@ jobs: fi echo "should_review=true" >> "$GITHUB_OUTPUT" - authorize-review-request: - needs: ['review-config'] + authorize: + # Single source of truth for "may this trigger spend review compute". + # The principal whose permission decides eligibility is the PR author + # (automatic PR events), the commenter (comment/review command events), or + # the requester (review_requested). They must have write+ permission. + # This replaces the per-path author_association checks, which are + # unreliable for fork PRs (a write user pushing from a fork is reported as + # CONTRIBUTOR, not MEMBER), so fork PRs by trusted authors now qualify. + # Only run for PR-target events and /review command comments — not every + # unrelated comment — to avoid spawning a job per comment. The downstream + # `if`s still do the exact /review body match; this prefix is just a filter. if: |- - github.event_name == 'pull_request_target' && - github.event.action == 'review_requested' && - github.event.requested_reviewer.login == needs.review-config.outputs.bot_login && - github.event.pull_request.state == 'open' && - !github.event.pull_request.draft + github.event_name == 'pull_request_target' || + ((github.event_name == 'issue_comment' || + github.event_name == 'pull_request_review_comment') && + startsWith(github.event.comment.body, '@qwen-code /review')) || + (github.event_name == 'pull_request_review' && + startsWith(github.event.review.body, '@qwen-code /review')) runs-on: 'ubuntu-latest' + timeout-minutes: 5 permissions: contents: 'read' outputs: - should_review: '${{ steps.sender_permission.outputs.should_review }}' + should_review: '${{ steps.principal_permission.outputs.should_review }}' steps: - - name: 'Check requester permission' - id: 'sender_permission' + - name: 'Check principal write permission' + id: 'principal_permission' env: - GH_TOKEN: '${{ secrets.GITHUB_TOKEN }}' - REQUESTER: '${{ github.event.sender.login }}' + # 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_ACTION: '${{ github.event.action }}' + PR_AUTHOR: '${{ github.event.pull_request.user.login }}' + COMMENT_USER: '${{ github.event.comment.user.login }}' + REVIEW_USER: '${{ github.event.review.user.login }}' + SENDER: '${{ github.event.sender.login }}' run: |- set -euo pipefail - if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${REQUESTER}/permission" --jq '.permission')"; then - echo "Failed to check permission for ${REQUESTER}." >&2 - echo "Failed to check permission for ${REQUESTER}." >> "$GITHUB_STEP_SUMMARY" + # Select the principal whose permission gates this trigger. + case "$EVENT_NAME" in + pull_request_target) + if [ "$PR_ACTION" = "review_requested" ]; then + principal="$SENDER" + else + principal="$PR_AUTHOR" + fi + ;; + issue_comment|pull_request_review_comment) + principal="$COMMENT_USER" + ;; + pull_request_review) + principal="$REVIEW_USER" + ;; + *) + principal="" + ;; + esac + if [ -z "$principal" ]; then + echo "No principal resolved for ${EVENT_NAME}; denying." >> "$GITHUB_STEP_SUMMARY" + echo "should_review=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + # Fail closed: any API error or non-write permission denies the run. + if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${principal}/permission" --jq '.permission' 2>/dev/null)"; then + echo "Failed to check permission for ${principal}; denying." >> "$GITHUB_STEP_SUMMARY" echo "should_review=false" >> "$GITHUB_OUTPUT" exit 0 fi @@ -202,18 +241,18 @@ jobs: echo "should_review=true" >> "$GITHUB_OUTPUT" ;; *) - echo "Skipping requested review: ${REQUESTER} lacks write permission or permission check failed." >> "$GITHUB_STEP_SUMMARY" + echo "Denying review: ${principal} permission is '${permission}' (needs write)." >> "$GITHUB_STEP_SUMMARY" echo "should_review=false" >> "$GITHUB_OUTPUT" ;; esac review-pr: - needs: - ['review-config', 'delay-automatic-review', 'authorize-review-request'] - # pull_request_target routing: - # - review_requested uses authorize-review-request and skips delay + needs: ['review-config', 'delay-automatic-review', 'authorize'] + # pull_request_target routing (every path additionally gated by the + # `authorize` job = the principal has write+ permission): + # - review_requested checks the requester and skips delay # - opened/synchronize uses delay-automatic-review - # - reopened/ready_for_review runs immediately for trusted PR authors + # - reopened/ready_for_review runs immediately # KEEP IN SYNC with ack-review-request.if (explicit-trigger branches). if: |- always() && @@ -221,41 +260,32 @@ jobs: (github.event_name == 'pull_request_target' && github.event.pull_request.state == 'open' && !github.event.pull_request.draft && + needs.authorize.outputs.should_review == 'true' && ((github.event.action == 'review_requested' && - github.event.requested_reviewer.login == needs.review-config.outputs.bot_login && - needs.authorize-review-request.outputs.should_review == 'true') || + github.event.requested_reviewer.login == needs.review-config.outputs.bot_login) || (github.event.action != 'review_requested' && ((github.event.action != 'opened' && github.event.action != 'synchronize') || - needs.delay-automatic-review.outputs.should_review == 'true') && - (github.event.pull_request.author_association == 'OWNER' || - github.event.pull_request.author_association == 'MEMBER' || - github.event.pull_request.author_association == 'COLLABORATOR')))) || + needs.delay-automatic-review.outputs.should_review == 'true')))) || (github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.issue.state == 'open' && (github.event.comment.body == '@qwen-code /review' || startsWith(github.event.comment.body, '@qwen-code /review ') || startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) || + needs.authorize.outputs.should_review == 'true') || (github.event_name == 'pull_request_review_comment' && github.event.pull_request.state == 'open' && (github.event.comment.body == '@qwen-code /review' || startsWith(github.event.comment.body, '@qwen-code /review ') || startsWith(github.event.comment.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) || + needs.authorize.outputs.should_review == 'true') || (github.event_name == 'pull_request_review' && github.event.pull_request.state == 'open' && (github.event.review.body == '@qwen-code /review' || startsWith(github.event.review.body, '@qwen-code /review ') || startsWith(github.event.review.body, format('@qwen-code /review{0}', '\n'))) && - (github.event.review.author_association == 'OWNER' || - github.event.review.author_association == 'MEMBER' || - github.event.review.author_association == 'COLLABORATOR'))) + needs.authorize.outputs.should_review == 'true')) timeout-minutes: 90 runs-on: ['self-hosted', 'linux', 'x64', 'ecs-qwen'] permissions: diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index 259607c64b2..9731b65cb1f 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -20,41 +20,99 @@ 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 same-repo-only fork check and the comment + # author_association check, 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. + if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${principal}/permission" --jq '.permission' 2>/dev/null)"; then + echo "Failed to check permission for ${principal}; denying." >> "$GITHUB_STEP_SUMMARY" + echo "should_run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + 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: group: '${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number || github.event.inputs.number }}' - # Repeat the maintainer /triage check here intentionally: GitHub - # evaluates concurrency before the job `if`, so this controls - # cancellation, not job eligibility. Other job gates below can diverge. + # GitHub evaluates concurrency before the job `if`, and it cannot call + # the permission API, so this only controls cancellation (a coarse event + # match), not eligibility. The real write-permission gate is the + # `authorize` job below. cancel-in-progress: >- ${{ github.event_name == 'issues' || (github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && - startsWith(github.event.comment.body, '@qwen-code /triage') && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) + startsWith(github.event.comment.body, '@qwen-code /triage')) }} 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 == 'pull_request_target' && - github.event.pull_request.head.repo.full_name == github.repository && - github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || - (github.event_name == 'issue_comment' && - startsWith(github.event.comment.body, '@qwen-code /triage') && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR')) + ( + ((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' From c710c0d6531b2cdf378f54d9c880c2c406d9e7c4 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Tue, 16 Jun 2026 20:19:23 +0800 Subject: [PATCH 2/3] fix(ci): isolate unauthorized triage triggers --- .github/workflows/qwen-triage.yml | 33 ++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index 9731b65cb1f..bf91e82e5f9 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -23,8 +23,8 @@ 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 same-repo-only fork check and the comment - # author_association check, so fork PRs by trusted authors are triaged. + # 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' && @@ -82,19 +82,30 @@ jobs: needs: ['authorize'] timeout-minutes: 30 concurrency: - group: '${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number || github.event.inputs.number }}' - # GitHub evaluates concurrency before the job `if`, and it cannot call - # the permission API, so this only controls cancellation (a coarse event - # match), not eligibility. The real write-permission gate is the - # `authorize` job below. + # 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 == 'pull_request_target' && - github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || - (github.event_name == 'issue_comment' && - startsWith(github.event.comment.body, '@qwen-code /triage')) + (((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 From d7d16ef321b89f880d4da936e0ee8d7d858cc4a5 Mon Sep 17 00:00:00 2001 From: yiliang114 <1204183885@qq.com> Date: Tue, 16 Jun 2026 20:40:56 +0800 Subject: [PATCH 3/3] fix(ci): report permission gate API failures --- .github/workflows/qwen-code-pr-review.yml | 25 +++++++++++++++-------- .github/workflows/qwen-triage.yml | 12 +++++++++-- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index de66638cc1f..c4b3281ea0d 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -176,12 +176,13 @@ jobs: # unrelated comment — to avoid spawning a job per comment. The downstream # `if`s still do the exact /review body match; this prefix is just a filter. if: |- - github.event_name == 'pull_request_target' || - ((github.event_name == 'issue_comment' || - github.event_name == 'pull_request_review_comment') && - startsWith(github.event.comment.body, '@qwen-code /review')) || - (github.event_name == 'pull_request_review' && - startsWith(github.event.review.body, '@qwen-code /review')) + github.repository == 'QwenLM/qwen-code' && + (github.event_name == 'pull_request_target' || + ((github.event_name == 'issue_comment' || + github.event_name == 'pull_request_review_comment') && + startsWith(github.event.comment.body, '@qwen-code /review')) || + (github.event_name == 'pull_request_review' && + startsWith(github.event.review.body, '@qwen-code /review'))) runs-on: 'ubuntu-latest' timeout-minutes: 5 permissions: @@ -231,11 +232,19 @@ jobs: exit 0 fi # Fail closed: any API error or non-write permission denies the run. - if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${principal}/permission" --jq '.permission' 2>/dev/null)"; then - echo "Failed to check permission for ${principal}; denying." >> "$GITHUB_STEP_SUMMARY" + 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_review=false" >> "$GITHUB_OUTPUT" exit 0 fi + rm -f "$api_error_file" case "$permission" in admin|maintain|write) echo "should_review=true" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index bf91e82e5f9..be5fcfbfcd6 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -63,11 +63,19 @@ jobs: exit 0 fi # Fail closed: any API error or non-write permission denies the run. - if ! permission="$(gh api "repos/${GITHUB_REPOSITORY}/collaborators/${principal}/permission" --jq '.permission' 2>/dev/null)"; then - echo "Failed to check permission for ${principal}; denying." >> "$GITHUB_STEP_SUMMARY" + 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 fi + rm -f "$api_error_file" case "$permission" in admin|maintain|write) echo "should_run=true" >> "$GITHUB_OUTPUT"