From 2d4422219fd6d8ebec829132f0bd0ffc9b3dfd9d Mon Sep 17 00:00:00 2001 From: Michael Denyer Date: Fri, 12 Jun 2026 20:54:14 +0100 Subject: [PATCH] fix: post dogfood PR review comments on fork PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pr-review.yml dogfood workflow failed on every fork-originated PR: GITHUB_TOKEN is read-only in pull_request runs from forks, so the sticky-comment upsert died with HTTP 403 (observed on #555). The analysis ran fine — only the comment write failed. Split into the standard two-workflow pattern: - pr-review.yml runs unprivileged (contents: read only) with comment: false, and uploads the rendered report plus PR number as an artifact. - pr-review-comment.yml (workflow_run, pull-requests: write) downloads the artifact and upserts the sticky comment. It never checks out PR code and treats the artifact as untrusted: the PR number must be numeric and the named PR's head must match the triggering run's head_sha before anything is posted. action.yml gains a comment-file output (path to the rendered markdown) so any caller can implement the same pattern; existing consumers are unaffected (comment defaults to true and posts exactly as before). docs/GITHUB_ACTION.md documents the output and replaces the pull_request_target suggestion in the fork-PRs note with this safer pattern. --- .github/workflows/pr-review-comment.yml | 65 +++++++++++++++++++++++++ .github/workflows/pr-review.yml | 20 +++++++- action.yml | 11 +++++ docs/GITHUB_ACTION.md | 26 ++++++++-- 4 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/pr-review-comment.yml diff --git a/.github/workflows/pr-review-comment.yml b/.github/workflows/pr-review-comment.yml new file mode 100644 index 00000000..d4f2b130 --- /dev/null +++ b/.github/workflows/pr-review-comment.yml @@ -0,0 +1,65 @@ +# Posts the sticky PR comment rendered by pr-review.yml. Split into a +# separate workflow_run-triggered workflow because GITHUB_TOKEN is read-only +# in pull_request runs from forks (observed on #555), so the unprivileged +# run cannot comment. This workflow never checks out or executes PR code, +# and it treats the artifact as untrusted input: the PR number it names is +# only honored if that PR's head is the exact commit the triggering run +# analyzed. +name: PR Review Comment + +on: + workflow_run: + workflows: ["PR Review"] + types: [completed] + +permissions: + pull-requests: write + +jobs: + comment: + runs-on: ubuntu-latest + if: github.event.workflow_run.conclusion == 'success' + steps: + - name: Download report artifact + uses: actions/download-artifact@v4 + with: + name: crg-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Upsert sticky PR comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + pr_number=$(head -n 1 pr-number.txt | tr -d '[:space:]') + case "${pr_number}" in + ''|*[!0-9]*) + echo "Missing or non-numeric PR number in artifact." >&2 + exit 1 + ;; + esac + # The artifact comes from an unprivileged (possibly fork) run. + # Only trust its PR number if that PR's head is the commit the + # triggering run actually analyzed. + actual_sha=$(gh api \ + "repos/${GITHUB_REPOSITORY}/pulls/${pr_number}" --jq .head.sha) + if [ "${actual_sha}" != "${HEAD_SHA}" ]; then + echo "PR ${pr_number} head (${actual_sha}) does not match" \ + "analyzed commit (${HEAD_SHA}); refusing to comment." >&2 + exit 1 + fi + marker='' + comment_id=$(gh api \ + "repos/${GITHUB_REPOSITORY}/issues/${pr_number}/comments" \ + --paginate \ + --jq ".[] | select(.body | contains(\"${marker}\")) | .id" | head -n 1) + if [ -n "${comment_id}" ]; then + gh api --method PATCH --silent \ + "repos/${GITHUB_REPOSITORY}/issues/comments/${comment_id}" \ + -F body=@crg-comment.md + else + gh api --method POST --silent \ + "repos/${GITHUB_REPOSITORY}/issues/${pr_number}/comments" \ + -F body=@crg-comment.md + fi diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index dde491b6..96c04901 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -1,4 +1,8 @@ # Dogfoods the local composite action (action.yml at the repo root) on PRs. +# Runs unprivileged: GITHUB_TOKEN is read-only in pull_request runs from +# forks, so the sticky comment is not posted here. Instead the rendered +# report is uploaded as an artifact and posted by pr-review-comment.yml +# (workflow_run, which has write permissions). name: PR Review on: @@ -7,7 +11,6 @@ on: permissions: contents: read - pull-requests: write jobs: review: @@ -15,7 +18,22 @@ jobs: steps: - uses: actions/checkout@v4 - name: Run code-review-graph review + id: review uses: ./ with: github-token: ${{ secrets.GITHUB_TOKEN }} + comment: "false" fail-on-risk: none + - name: Stage report artifact + env: + COMMENT_FILE: ${{ steps.review.outputs.comment-file }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + mkdir crg-report + cp "${COMMENT_FILE}" crg-report/crg-comment.md + echo "${PR_NUMBER}" > crg-report/pr-number.txt + - name: Upload report artifact + uses: actions/upload-artifact@v4 + with: + name: crg-report + path: crg-report/ diff --git a/action.yml b/action.yml index 79718127..f363c52a 100644 --- a/action.yml +++ b/action.yml @@ -32,6 +32,15 @@ inputs: required: false default: "3.12" +outputs: + comment-file: + description: >- + Runner-local path to the rendered markdown report. Useful with + `comment: false` to publish the report another way — e.g. upload it + as an artifact for a privileged workflow_run workflow to post on + fork PRs (see docs/GITHUB_ACTION.md, "Fork PRs"). + value: ${{ steps.render.outputs.comment-file }} + runs: using: "composite" steps: @@ -87,11 +96,13 @@ runs: > "${RUNNER_TEMP}/crg-report.json" - name: Render markdown report + id: render shell: bash run: | python "${GITHUB_ACTION_PATH}/scripts/render_pr_comment.py" \ --input "${RUNNER_TEMP}/crg-report.json" \ --output "${RUNNER_TEMP}/crg-comment.md" + echo "comment-file=${RUNNER_TEMP}/crg-comment.md" >> "${GITHUB_OUTPUT}" - name: Upsert sticky PR comment if: ${{ inputs.comment == 'true' && github.event_name == 'pull_request' }} diff --git a/docs/GITHUB_ACTION.md b/docs/GITHUB_ACTION.md index 53b66f0c..9e0fa988 100644 --- a/docs/GITHUB_ACTION.md +++ b/docs/GITHUB_ACTION.md @@ -78,6 +78,12 @@ security-sensitive names, caller count). The action maps it to levels: | high | 0.70 – 0.84 | | critical | ≥ 0.85 | +## Outputs + +| Output | Description | +|--------|-------------| +| `comment-file` | Runner-local path to the rendered markdown report. Useful with `comment: false` to publish the report another way — e.g. upload it as an artifact for a privileged `workflow_run` workflow to post on fork PRs (see "Fork PRs" below). | + ## What the comment contains - **Overall risk** score and level, with counts of changed functions, @@ -139,16 +145,26 @@ database) with `actions/cache`: - **Pinning**: when consuming the action from another repository, pin `uses:` to a release tag or commit SHA rather than `@main`. - **Fork PRs**: `pull_request` runs from forks receive a read-only - `GITHUB_TOKEN`, so the comment step will fail for fork PRs unless you use - `pull_request_target` — which checks out trusted base-branch workflow - code; understand [the security implications](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/) - before switching, or set `comment: false` for fork PRs. + `GITHUB_TOKEN`, so the comment step fails for fork PRs. The recommended + pattern: run the action with `comment: false`, upload the rendered report + (the `comment-file` output) as an artifact, and post it from a separate + `workflow_run`-triggered workflow with `pull-requests: write` — see + [`pr-review.yml`](../.github/workflows/pr-review.yml) and + [`pr-review-comment.yml`](../.github/workflows/pr-review-comment.yml) for + a reference implementation, which also verifies the artifact's PR number + against the triggering run's head SHA before posting. Avoid + `pull_request_target` with a checkout of PR code — it runs untrusted code + with a privileged token + ([details](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/)). ## Dogfooding This repository runs the action on its own PRs via [`.github/workflows/pr-review.yml`](../.github/workflows/pr-review.yml), -which `uses: ./` (the local `action.yml`). +which `uses: ./` (the local `action.yml`). The review run is unprivileged +(`comment: false` + artifact upload) and the sticky comment is posted by +[`pr-review-comment.yml`](../.github/workflows/pr-review-comment.yml), so +fork PRs get review comments too. ## Rendering script