diff --git a/.github/workflows/ready-for-review-label.yml b/.github/workflows/ready-for-review-label.yml index f3739959b..e4b9880d4 100644 --- a/.github/workflows/ready-for-review-label.yml +++ b/.github/workflows/ready-for-review-label.yml @@ -1,11 +1,16 @@ -name: Ready-for-review label +name: Ready-for-review (collect) -# When a pull-request changes state (closed, converted to draft) or a review is submitted/dismissed, -# run the `manage-ready-for-review.yml` workflow, to remove the -# "ready-for-review" label from a PR if its no longer needed. +# Stage 1 of a two-stage pipeline that removes the "ready-for-review" label from +# a pull request once it is no longer awaiting review. # -# The label removal workflow lives in hyperlight-dev's `.github` repository. To apply this workflow -# to another repository, copy this file to that repo. +# Pull-request and review events originating from FORKED repositories only ever +# receive a read-only GITHUB_TOKEN, so a workflow triggered directly by them +# cannot modify labels. +# This stage therefore performs no privileged work: it +# simply records the pull-request number and hands it to the privileged +# "Ready-for-review (manage)" workflow, which is triggered via `workflow_run` +# and runs with a read-write token. See +# ready-for-review-manage.yml. on: pull_request: types: [closed, converted_to_draft] @@ -14,16 +19,28 @@ on: # Serialise runs per pull request so that concurrent events cannot race. concurrency: - group: ready-for-review-${{ github.event.pull_request.number }} + group: ready-for-review-collect-${{ github.event.pull_request.number }} cancel-in-progress: false - permissions: contents: read - pull-requests: write - issues: write + jobs: - manage-label: - # Shared workflow in the org-wide `.github` repository, pinned to a commit SHA - uses: hyperlight-dev/.github/.github/workflows/manage-ready-for-review.yml@e5e471d927d3a72676ab48433da8a99d15a32ad7 - with: - pr-number: ${{ github.event.pull_request.number }} + collect: + runs-on: ubuntu-latest + steps: + - name: Record the pull-request number + env: + # Always a GitHub-provided integer; present on both the pull_request + # and pull_request_review event payloads. + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + mkdir -p pr + printf '%s' "$PR_NUMBER" > pr/pr_number + + - name: Upload the pull-request number + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ready-for-review-pr-number + path: pr/ + retention-days: 1 diff --git a/.github/workflows/ready-for-review-manage.yml b/.github/workflows/ready-for-review-manage.yml new file mode 100644 index 000000000..a1eb48a44 --- /dev/null +++ b/.github/workflows/ready-for-review-manage.yml @@ -0,0 +1,73 @@ +name: Ready-for-review (manage) + +# Stage 2 of the two-stage "ready-for-review" label pipeline. It is triggered +# when "Ready-for-review (collect)" completes and runs in the context of the +# base repository's default branch, so it receives a read-write GITHUB_TOKEN +# even when the originating event came from a fork. This is what lets us remove +# the label from pull requests opened from forks. See ready-for-review-label.yml. +# +# SECURITY: the collect stage can run from untrusted fork code, so the artifact +# it produces is UNTRUSTED. We therefore: +# * download it into a temporary directory, +# * never check out or execute any pull-request code, and +# * strictly validate the pull-request number (digits only) before use. +# The downstream reusable workflow only ever *removes* the label, and only when +# the PR genuinely meets a removal condition, so the worst a spoofed-but-valid +# number could achieve is removing a single, manually re-addable label from a PR +# that already qualified for removal. +on: + workflow_run: + workflows: ["Ready-for-review (collect)"] + types: [completed] + +# Best-effort per-source-branch serialisation. The real PR number is only known +# after the artifact is read, and the reusable workflow already tolerates a +# concurrent label removal (HTTP 404), so this is belt-and-braces. +concurrency: + group: ready-for-review-manage-${{ github.event.workflow_run.head_repository.full_name }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: false + +permissions: + actions: read # download the artifact from the collect run + contents: read + pull-requests: write # remove the label + issues: write # labels are managed through the issues API + +jobs: + read-pr: + runs-on: ubuntu-latest + # Only act on a collect run that actually succeeded. + if: ${{ github.event.workflow_run.conclusion == 'success' }} + outputs: + pr_number: ${{ steps.read.outputs.pr_number }} + steps: + - name: Download the pull-request number + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: ready-for-review-pr-number + path: ${{ runner.temp }}/pr # temp dir, never the workspace + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Read and validate the pull-request number + id: read + env: + ARTIFACT_DIR: ${{ runner.temp }}/pr + run: | + set -euo pipefail + raw="$(cat "${ARTIFACT_DIR}/pr_number")" + # Untrusted input from a potentially fork-triggered run: digits only. + if [[ ! "$raw" =~ ^[0-9]+$ ]]; then + echo "Refusing to act on non-numeric PR number" >&2 + printf 'Value was: %q\n' "$raw" >&2 + exit 1 + fi + echo "pr_number=${raw}" >> "$GITHUB_OUTPUT" + + manage-label: + needs: read-pr + # Shared workflow in the org-wide `.github` repository, pinned to a commit SHA. + uses: hyperlight-dev/.github/.github/workflows/manage-ready-for-review.yml@e5e471d927d3a72676ab48433da8a99d15a32ad7 + with: + # fromJSON turns the validated digit-string into a numeric input. + pr-number: ${{ fromJSON(needs.read-pr.outputs.pr_number) }} diff --git a/docs/github-labels.md b/docs/github-labels.md index 19508c78a..810d02a9f 100644 --- a/docs/github-labels.md +++ b/docs/github-labels.md @@ -12,7 +12,7 @@ We use GitHub labels to categorize PRs. Before a PR can be merged, it must be as We use the **ready-for-review** label to signal that a PR is waiting for a (re-)review: - **Add** `ready-for-review` when you open a PR that is ready for review, or when a PR is ready for re-review (for example, once you have addressed requested changes and re-requested review). -- The label is **removed automatically** by the [`Ready-for-review label`](../.github/workflows/ready-for-review-label.yml) workflow once the PR is no longer awaiting that review, specifically when any of the following become true: +- The label is **removed automatically** by the ready-for-review label automation (the [`collect`](../.github/workflows/ready-for-review-label.yml) and [`manage`](../.github/workflows/ready-for-review-manage.yml) workflows) once the PR is no longer awaiting that review, specifically when any of the following become true: - the PR is closed or merged, - the PR is converted to a draft, - the PR has two or more approvals, or