From b73f9f31fe8f9d331e73109b65eb1ca4440a8a80 Mon Sep 17 00:00:00 2001 From: Guarzo Date: Sat, 1 Aug 2026 19:22:51 +0000 Subject: [PATCH 1/3] ci: post PR test comment via workflow_run to fix fork PR failures PRs from forks receive a read-only GITHUB_TOKEN for pull_request events regardless of the workflow's `permissions:` block, so the comment step in test.yml failed with 403 "Resource not accessible by integration" and took the whole job down with it. Every fork PR was red despite the test run itself succeeding. Split the reporting: test.yml now renders the comment body and uploads it as an artifact, and a new pr-comment.yml posts it via workflow_run, which runs in the base repo context with a writable token. The render step interpolates tool output from env vars in the shell rather than through Actions expression substitution, so tool output can no longer inject workflow syntax. pr-comment.yml never checks out or executes PR code, re-validates the PR number as numeric, and passes the body via body-path. Also mirrors the report to $GITHUB_STEP_SUMMARY so it stays visible if the follow-up workflow does not run, and fixes the empty "Updated:" timestamp (github.event.head_commit.timestamp is unset on pull_request events). --- .github/workflows/pr-comment.yml | 81 +++++++++++++ .github/workflows/test.yml | 190 ++++++++++++++++++------------- 2 files changed, 194 insertions(+), 77 deletions(-) create mode 100644 .github/workflows/pr-comment.yml diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml new file mode 100644 index 00000000..cfa96554 --- /dev/null +++ b/.github/workflows/pr-comment.yml @@ -0,0 +1,81 @@ +name: ๐Ÿ’ฌ PR Test Comment + +# Posts the test-results comment produced by the "๐Ÿงช Test Suite" workflow. +# +# Why this is a separate workflow: for pull_request events from forks GitHub +# issues a read-only GITHUB_TOKEN regardless of the `permissions:` block, so the +# test workflow itself cannot post a comment (403 "Resource not accessible by +# integration"). A workflow_run workflow runs in the base repository's context +# with a writable token, so it can. +# +# SECURITY: this workflow runs with write permissions and must never execute +# code from the pull request. Do not add actions/checkout of the PR head, and do +# not run build/test tooling here. The downloaded artifact is untrusted input: +# the comment body is passed to the API via `body-path` (never interpolated into +# a run: block or a ${{ }} expression) and the PR number is re-validated as +# numeric before use. + +on: + workflow_run: + # Must match the `name:` of .github/workflows/test.yml exactly. + workflows: ['๐Ÿงช Test Suite'] + types: [completed] + +permissions: + contents: read + pull-requests: write + issues: write + +jobs: + comment: + name: Post test results comment + runs-on: ubuntu-latest + if: github.event.workflow_run.event == 'pull_request' + steps: + - name: Download comment artifact + id: download + uses: actions/download-artifact@v4 + continue-on-error: true + with: + name: pr-comment + path: pr-comment + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Read PR number + id: pr + if: steps.download.outcome == 'success' + run: | + if [ ! -f pr-comment/pr-number ] || [ ! -f pr-comment/body.md ]; then + echo "Artifact incomplete; nothing to post." + echo "valid=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + number=$(tr -dc '0-9' < pr-comment/pr-number | head -c 10) + if [ -z "$number" ]; then + echo "PR number is not numeric; refusing to continue." + echo "valid=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + echo "number=$number" >> "$GITHUB_OUTPUT" + echo "valid=true" >> "$GITHUB_OUTPUT" + + - name: Find existing PR comment + id: find_comment + if: steps.pr.outputs.valid == 'true' + uses: peter-evans/find-comment@v3 + with: + issue-number: ${{ steps.pr.outputs.number }} + comment-author: 'github-actions[bot]' + body-includes: '## ๐Ÿงช Test Results Summary' + + - name: Create or update PR comment + if: steps.pr.outputs.valid == 'true' + uses: peter-evans/create-or-update-comment@v4 + with: + comment-id: ${{ steps.find_comment.outputs.comment-id }} + issue-number: ${{ steps.pr.outputs.number }} + edit-mode: replace + body-path: pr-comment/body.md diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ac9f87ab..bbee56c5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -253,81 +253,117 @@ jobs: fi continue-on-error: true - - name: Find existing PR comment - if: github.event_name == 'pull_request' - id: find_comment - uses: peter-evans/find-comment@v3 + # Fork PRs get a read-only GITHUB_TOKEN, so this workflow cannot post the + # comment itself. Instead the rendered body is uploaded as an artifact and + # posted by the privileged `pr-comment.yml` workflow via workflow_run. + - name: Render PR comment body + if: always() && github.event_name == 'pull_request' + env: + OVERALL_SCORE: ${{ steps.summary.outputs.overall_score }} + OVERALL_STATUS: ${{ steps.summary.outputs.overall_status }} + FORMAT_STATUS: ${{ steps.format.outputs.status }} + FORMAT_COUNT: ${{ steps.format.outputs.count }} + COMPILE_STATUS: ${{ steps.compile.outputs.status }} + COMPILE_WARNINGS: ${{ steps.compile.outputs.warnings }} + TESTS_STATUS: ${{ steps.tests.outputs.status }} + TESTS_FAILURES: ${{ steps.tests.outputs.failures }} + TESTS_TOTAL: ${{ steps.tests.outputs.total }} + TESTS_SUCCESS_RATE: ${{ steps.tests.outputs.success_rate }} + COVERAGE_STATUS: ${{ steps.coverage.outputs.status }} + COVERAGE_PERCENTAGE: ${{ steps.coverage.outputs.percentage }} + CREDO_STATUS: ${{ steps.credo.outputs.status }} + CREDO_TOTAL: ${{ steps.credo.outputs.total_issues }} + CREDO_HIGH: ${{ steps.credo.outputs.high_issues }} + CREDO_NORMAL: ${{ steps.credo.outputs.normal_issues }} + CREDO_LOW: ${{ steps.credo.outputs.low_issues }} + DIALYZER_STATUS: ${{ steps.dialyzer.outputs.status }} + DIALYZER_ERRORS: ${{ steps.dialyzer.outputs.errors }} + DIALYZER_WARNINGS: ${{ steps.dialyzer.outputs.warnings }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + mkdir -p pr-comment + # Validate the PR number so the trusted workflow can trust this file. + case "$PR_NUMBER" in + ''|*[!0-9]*) echo "Refusing to emit non-numeric PR number" >&2; exit 1 ;; + esac + printf '%s\n' "$PR_NUMBER" > pr-comment/pr-number + + # Values are interpolated by the shell from env, never by Actions + # expression substitution, so tool output cannot inject workflow syntax. + cat > pr-comment/body.md < + ๐Ÿ“ˆ Progress Toward Goals + + Target goals for the project: + - โœจ **Zero compilation warnings** (currently: ${COMPILE_WARNINGS}) + - โœจ **โ‰ค10 Credo issues** (currently: ${CREDO_TOTAL}) + - โœจ **Zero Dialyzer warnings** (currently: ${DIALYZER_WARNINGS}) + - โœจ **โ‰ฅ85% test coverage** (currently: ${COVERAGE_PERCENTAGE}%) + - โœ… **Zero test failures** (currently: ${TESTS_FAILURES}) + + + +
+ ๐Ÿ”ง Quick Actions + + To improve code quality: + \`\`\`bash + # Fix formatting issues + mix format + + # View detailed Credo analysis + mix credo --strict + + # Check Dialyzer warnings + mix dialyzer + + # Generate detailed coverage report + mix coveralls.html + \`\`\` + +
+ + --- + + ๐Ÿค– *Auto-generated by GitHub Actions* โ€ข Updated: $(date -u '+%Y-%m-%d %H:%M UTC') + + > **Note**: This comment will be updated automatically when new commits are pushed to this PR. + EOF + + # Mirror to the run summary so the report is visible even if the + # follow-up commenting workflow is unavailable. + cat pr-comment/body.md >> "$GITHUB_STEP_SUMMARY" + continue-on-error: true + + - name: Upload PR comment body + if: always() && github.event_name == 'pull_request' + uses: actions/upload-artifact@v4 with: - issue-number: ${{ github.event.pull_request.number }} - comment-author: 'github-actions[bot]' - body-includes: '## ๐Ÿงช Test Results Summary' - - - name: Create or update PR comment - if: github.event_name == 'pull_request' - uses: peter-evans/create-or-update-comment@v4 - with: - comment-id: ${{ steps.find_comment.outputs.comment-id }} - issue-number: ${{ github.event.pull_request.number }} - edit-mode: replace - body: | - ## ๐Ÿงช Test Results Summary - - **Overall Quality Score: ${{ steps.summary.outputs.overall_score }}%** ${{ steps.summary.outputs.overall_status }} - - ### ๐Ÿ“Š Metrics Dashboard - - | Category | Status | Count | Details | - |----------|---------|-------|---------| - | ๐Ÿ“ **Code Formatting** | ${{ steps.format.outputs.status }} | ${{ steps.format.outputs.count }} issues | `mix format --check-formatted` | - | ๐Ÿ”จ **Compilation** | ${{ steps.compile.outputs.status }} | ${{ steps.compile.outputs.warnings }} warnings | `mix compile` | - | ๐Ÿงช **Tests** | ${{ steps.tests.outputs.status }} | ${{ steps.tests.outputs.failures }}/${{ steps.tests.outputs.total }} failed | Success rate: ${{ steps.tests.outputs.success_rate }}% | - | ๐Ÿ“Š **Coverage** | ${{ steps.coverage.outputs.status }} | ${{ steps.coverage.outputs.percentage }}% | `mix coveralls` | - | ๐ŸŽฏ **Credo** | ${{ steps.credo.outputs.status }} | ${{ steps.credo.outputs.total_issues }} issues | High: ${{ steps.credo.outputs.high_issues }}, Normal: ${{ steps.credo.outputs.normal_issues }}, Low: ${{ steps.credo.outputs.low_issues }} | - | ๐Ÿ” **Dialyzer** | ${{ steps.dialyzer.outputs.status }} | ${{ steps.dialyzer.outputs.errors }} errors, ${{ steps.dialyzer.outputs.warnings }} warnings | `mix dialyzer` | - - ### ๐ŸŽฏ Quality Gates - - Based on the project's quality thresholds: - - **Compilation Warnings**: ${{ steps.compile.outputs.warnings }}/148 (limit: 148) - - **Credo Issues**: ${{ steps.credo.outputs.total_issues }}/87 (limit: 87) - - **Dialyzer Warnings**: ${{ steps.dialyzer.outputs.warnings }}/161 (limit: 161) - - **Test Coverage**: ${{ steps.coverage.outputs.percentage }}%/50% (minimum: 50%) - - **Test Failures**: ${{ steps.tests.outputs.failures }}/0 (limit: 0) - -
- ๐Ÿ“ˆ Progress Toward Goals - - Target goals for the project: - - โœจ **Zero compilation warnings** (currently: ${{ steps.compile.outputs.warnings }}) - - โœจ **โ‰ค10 Credo issues** (currently: ${{ steps.credo.outputs.total_issues }}) - - โœจ **Zero Dialyzer warnings** (currently: ${{ steps.dialyzer.outputs.warnings }}) - - โœจ **โ‰ฅ85% test coverage** (currently: ${{ steps.coverage.outputs.percentage }}%) - - โœ… **Zero test failures** (currently: ${{ steps.tests.outputs.failures }}) - -
- -
- ๐Ÿ”ง Quick Actions - - To improve code quality: - ```bash - # Fix formatting issues - mix format - - # View detailed Credo analysis - mix credo --strict - - # Check Dialyzer warnings - mix dialyzer - - # Generate detailed coverage report - mix coveralls.html - ``` - -
- - --- - - ๐Ÿค– *Auto-generated by GitHub Actions* โ€ข Updated: ${{ github.event.head_commit.timestamp }} - - > **Note**: This comment will be updated automatically when new commits are pushed to this PR. + name: pr-comment + path: pr-comment/ + retention-days: 1 + continue-on-error: true From 6d6029b7bf4156e029ec1cf6b2906bd232e25886 Mon Sep 17 00:00:00 2001 From: Guarzo Date: Sat, 1 Aug 2026 19:39:03 +0000 Subject: [PATCH 2/3] ci: pin artifact actions to Node 24 majors The upload/download-artifact steps added in the previous commit were pinned to @v4, which declares `runs.using: node20` and would emit the same Node 20 deprecation warning this repo is trying to clear. Verified against upstream action.yml rather than release notes, which are misleading here: download-artifact v5 and v6 are still node20 despite v6 being labeled a breaking "supports Node v24" release. node24 begins at download-artifact v7 and upload-artifact v6. upload@v4 -> v7, download@v4 -> v8 (current majors, both node24). Checked that every input we pass still exists, and that the intervening breaking changes do not apply: v5's path change affects downloads by artifact-id (we download by name), v7 upload's `archive` defaults to true (we upload a directory), and v8 download's `digest-mismatch: error` default is desirable. --- .github/workflows/pr-comment.yml | 2 +- .github/workflows/test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index cfa96554..78c2e10b 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -34,7 +34,7 @@ jobs: steps: - name: Download comment artifact id: download - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 continue-on-error: true with: name: pr-comment diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bbee56c5..e9e22f6c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -361,7 +361,7 @@ jobs: - name: Upload PR comment body if: always() && github.event_name == 'pull_request' - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: pr-comment path: pr-comment/ From 2c5164229ebc035e6ade8242c741f682da3e8fbe Mon Sep 17 00:00:00 2001 From: Guarzo Date: Sat, 8 Aug 2026 17:05:42 -0400 Subject: [PATCH 3/3] review: address pre-submit checklist findings Corrections from the upstream pre-submit review pass. --- .github/workflows/pr-comment.yml | 40 +++++++++++++++++++++++++++++++- .github/workflows/test.yml | 5 ++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 78c2e10b..5b3e83a5 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -14,6 +14,12 @@ name: ๐Ÿ’ฌ PR Test Comment # the comment body is passed to the API via `body-path` (never interpolated into # a run: block or a ${{ }} expression) and the PR number is re-validated as # numeric before use. +# +# A numeric PR number is not necessarily the *right* PR number: the artifact is +# produced by a job that has already run fork-authored code, so it could name +# any issue or PR in the base repository. The "Verify PR matches triggering run" +# step below closes that by requiring the claimed PR's head SHA to equal the +# head SHA of the run that triggered this workflow. on: workflow_run: @@ -23,7 +29,15 @@ on: permissions: contents: read - pull-requests: write + # `actions: read` is required: download-artifact with `run-id:` reads the + # artifacts of a *different* workflow run, which the default token scope does + # not cover. Without it the download 403s and the workflow silently posts + # nothing. + actions: read + # Read-only PR access is only used to verify the artifact's PR number against + # the triggering run's head SHA. The comment itself is an *issue* comment, so + # `issues: write` is the scope that grants posting. + pull-requests: read issues: write jobs: @@ -62,6 +76,30 @@ jobs: echo "number=$number" >> "$GITHUB_OUTPUT" echo "valid=true" >> "$GITHUB_OUTPUT" + - name: Verify PR matches triggering run + if: steps.pr.outputs.valid == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + EXPECTED_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + set -euo pipefail + + # The artifact comes from a job that ran untrusted code, so the PR + # number it claims is only a hint. Accept it only if that PR's head + # commit is the exact commit the triggering run tested; otherwise a + # fork could steer this privileged comment at any PR or issue. + actual_sha=$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq .head.sha) + + if [ "$actual_sha" != "$EXPECTED_SHA" ]; then + echo "PR #$PR_NUMBER head is $actual_sha but the triggering run tested $EXPECTED_SHA." >&2 + echo "Refusing to comment on a PR the artifact does not belong to." >&2 + exit 1 + fi + + echo "PR #$PR_NUMBER head matches the triggering run ($EXPECTED_SHA)." + - name: Find existing PR comment id: find_comment if: steps.pr.outputs.valid == 'true' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e9e22f6c..2edb6b6b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,10 +6,11 @@ on: push: branches: [main, develop] +# This workflow executes fork-authored code (mix deps.get / compile / test), so +# it holds no write scopes. The test-results comment is posted by the privileged +# pr-comment.yml workflow via workflow_run. permissions: contents: read - pull-requests: write - issues: write env: MIX_ENV: test