From 73d079d93d1590caab530310038cc4272dbf2777 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 01:30:55 +0000 Subject: [PATCH] fix: privilege-separate re-request-review to support fork PRs Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- .github/workflows/auto_assign_prs.yml | 75 ++++----------- .github/workflows/re_request_review.yml | 120 ++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 56 deletions(-) create mode 100644 .github/workflows/re_request_review.yml diff --git a/.github/workflows/auto_assign_prs.yml b/.github/workflows/auto_assign_prs.yml index bbcb3a9f4..5dd3fb31a 100644 --- a/.github/workflows/auto_assign_prs.yml +++ b/.github/workflows/auto_assign_prs.yml @@ -6,8 +6,12 @@ name: "Auto Assign Author" on: pull_request_target: types: [opened, reopened, ready_for_review] - # Watch for submitted reviews so we can re-request a second CODEOWNERS - # review once only one maintainer has approved. + # Watch for submitted reviews so we can record the PR number for the + # privileged re-request-review workflow (see re_request_review.yml). + # NOTE: For fork-originated PRs the GITHUB_TOKEN on pull_request_review is + # read-only even when pull-requests: write is declared, so the actual + # requestReviewers API call is delegated to a workflow_run workflow that + # receives a write token regardless of fork status. pull_request_review: types: [submitted] @@ -29,61 +33,20 @@ jobs: # `.github/CODEOWNERS` automatically requests review from the # velero-io/maintainer team, but that request is cleared as soon as a # single member of the team submits a review. Since we require a minimum - # of 2 reviewers (see `number_of_reviewers` in auto-assignees.yml), this - # re-requests a review from the maintainer team whenever a PR still has - # fewer than the required number of approvals, so a second CODEOWNERS - # reviewer gets pinged. - re-request-review: + # of 2 reviewers (see `number_of_reviewers` in auto-assignees.yml), record + # the PR number here so the privileged re_request_review.yml workflow can + # safely re-request a review from the maintainer team. + record-pr-number: if: github.repository == 'velero-io/velero' && github.event_name == 'pull_request_review' && github.event.review.state == 'approved' runs-on: ubuntu-latest steps: - - name: Re-request review from maintainers if more approvals are needed - uses: actions/github-script@v9 + - name: Save PR number to artifact + run: | + mkdir -p /tmp/pr + # Write only the numeric PR number; the privileged workflow validates this. + printf '%s' '${{ github.event.pull_request.number }}' > /tmp/pr/number + - name: Upload PR number artifact + uses: actions/upload-artifact@v4 with: - script: | - const requiredApprovals = 2; - const maintainerTeam = 'maintainer'; - const { owner, repo } = context.repo; - const pull_number = context.payload.pull_request.number; - - const { data: reviews } = await github.rest.pulls.listReviews({ - owner, - repo, - pull_number, - }); - - // Count distinct users whose most recent review is an approval. - // The Reviews API does not guarantee chronological order, so - // sort by submission time before folding into the map. - const sortedReviews = [...reviews].sort( - (a, b) => new Date(a.submitted_at) - new Date(b.submitted_at) - ); - const latestReviewByUser = new Map(); - for (const review of sortedReviews) { - latestReviewByUser.set(review.user.login, review.state); - } - const approvedReviewers = [...latestReviewByUser.entries()].filter( - ([, state]) => state === 'APPROVED' - ); - - if (approvedReviewers.length >= requiredApprovals) { - console.log( - `PR already has ${approvedReviewers.length} approvals, no need to re-request review.` - ); - return; - } - - console.log( - `PR has ${approvedReviewers.length}/${requiredApprovals} approvals, re-requesting review from @${owner}/${maintainerTeam}.` - ); - - try { - await github.rest.pulls.requestReviewers({ - owner, - repo, - pull_number, - team_reviewers: [maintainerTeam], - }); - } catch (error) { - core.warning(`Failed to re-request review from maintainers: ${error.message}`); - } + name: pr-number + path: /tmp/pr/number diff --git a/.github/workflows/re_request_review.yml b/.github/workflows/re_request_review.yml new file mode 100644 index 000000000..f7b7a837b --- /dev/null +++ b/.github/workflows/re_request_review.yml @@ -0,0 +1,120 @@ +--- +name: "Re-request Maintainer Review" + +# This workflow runs with a write token even for fork-originated PRs. +# It is triggered by the completion of the unprivileged "Auto Assign Author" +# workflow (see auto_assign_prs.yml), which only records the PR number as an +# artifact to avoid executing any PR-controlled code in a privileged context. +# +# Security boundary: artifact content is treated as untrusted input and +# validated strictly before being used. No PR code is checked out or executed. +on: + workflow_run: + workflows: ["Auto Assign Author"] + types: [completed] + +# Least-privilege: only the permissions needed to download the triggering +# run's artifact, read PR/review data, and request reviewers. +permissions: + actions: read + pull-requests: write + +jobs: + re-request-review: + # Only run when the triggering workflow succeeded on a pull_request_review + # event (recorded by the record-pr-number job). + if: > + github.event.workflow_run.event == 'pull_request_review' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + steps: + - name: Download PR number artifact + uses: actions/download-artifact@v4 + with: + name: pr-number + path: ${{ runner.temp }}/pr + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + + - name: Re-request review from maintainers if more approvals are needed + uses: actions/github-script@v9 + with: + script: | + const fs = require('fs'); + const path = require('path'); + + const requiredApprovals = 2; + const maintainerTeam = 'maintainer'; + const { owner, repo } = context.repo; + + // Read and strictly validate the artifact content. + // Treat the artifact as untrusted: accept only a plain integer. + const artifactPath = path.join(process.env.RUNNER_TEMP, 'pr', 'number'); + const raw = fs.readFileSync(artifactPath, 'utf8').trim(); + if (!/^\d+$/.test(raw)) { + core.setFailed(`Artifact contained an invalid PR number: ${JSON.stringify(raw)}`); + return; + } + const pull_number = parseInt(raw, 10); + console.log(`Processing PR #${pull_number}`); + + // Fetch current PR state to confirm it still exists and is open. + let pr; + try { + ({ data: pr } = await github.rest.pulls.get({ owner, repo, pull_number })); + } catch (error) { + core.setFailed(`Failed to fetch PR #${pull_number}: ${error.message}`); + return; + } + if (pr.state !== 'open') { + console.log(`PR #${pull_number} is ${pr.state}, skipping.`); + return; + } + + // Fetch all reviews with pagination. + const reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, + repo, + pull_number, + per_page: 100, + }); + + // Count distinct users whose most recent review is an approval. + // The Reviews API does not guarantee chronological order, so + // sort by submission time before folding into the map. + const sortedReviews = [...reviews].sort( + (a, b) => new Date(a.submitted_at) - new Date(b.submitted_at) + ); + const latestReviewByUser = new Map(); + for (const review of sortedReviews) { + latestReviewByUser.set(review.user.login, review.state); + } + const approvedReviewers = [...latestReviewByUser.entries()].filter( + ([, state]) => state === 'APPROVED' + ); + + if (approvedReviewers.length >= requiredApprovals) { + console.log( + `PR #${pull_number} already has ${approvedReviewers.length} approvals, no need to re-request review.` + ); + return; + } + + console.log( + `PR #${pull_number} has ${approvedReviewers.length}/${requiredApprovals} approvals, re-requesting review from @${owner}/${maintainerTeam}.` + ); + + // This call requires pull-requests: write, which is available here + // even for fork-originated PRs because workflow_run always runs in + // the context of the base repository with the repository's token. + try { + await github.rest.pulls.requestReviewers({ + owner, + repo, + pull_number, + team_reviewers: [maintainerTeam], + }); + console.log(`Successfully requested review from @${owner}/${maintainerTeam}.`); + } catch (error) { + core.setFailed(`Failed to re-request review from maintainers: ${error.message}`); + }