Skip to content

ci: publish fork-PR test reports via workflow_run#4643

Merged
jbogard merged 2 commits into
mainfrom
ci/fork-safe-test-reporting
Jun 12, 2026
Merged

ci: publish fork-PR test reports via workflow_run#4643
jbogard merged 2 commits into
mainfrom
ci/fork-safe-test-reporting

Conversation

@jbogard

@jbogard jbogard commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Fixes the fork-PR CI failures tracked in #4642 — and does it the right way, so fork PRs get real inline test reports instead of silently swallowed ones.

Problem

For pull_request runs from a fork, GitHub forcibly downgrades GITHUB_TOKEN to read-only (untrusted code must not get write access to the base repo) and withholds secrets. Two steps in ci.yml therefore failed on every fork PR even when all tests passed:

  • dorny/test-reporter calls the Checks API to publish results, which needs checks: write. With a read-only token it gets 403 Resource not accessible by integration and (default fail-on-error: true) fails the job.
  • Azure Login via OIDC uses secrets.AZURE_*, which are empty on fork PRs, so login fails. This step only exists to sign/publish packages — a main-only path.

Fix — split building from reporting (workflow_run pattern)

ci.yml (runs on the PR, read-only token, no secrets):

  • Replaces the Report Test Results steps with Upload Test Results — the .trx files are uploaded as test-results-Linux / test-results-Windows artifacts.
  • Gates Azure Login via OIDC and Sign packages to refs/heads/main (matching the existing Push to MyGet guard).
  • Drops checks: write from permissions — the build no longer touches the Checks API.

test-report.yml (new, triggered by workflow_run when CI completes):

  • Runs in the base-repository context with checks: write, so it has a writable token even for fork PRs.
  • Downloads the test-results-* artifacts and publishes one inline check per platform (Test Results (Linux), Test Results (Windows)).
  • fail-on-empty: false so a build that fails before producing .trx doesn't add a spurious red report.

#4642 checklist

  • Port both stop-gap fixes to main (superseded here by the proper fix; Azure/signing gating carried over verbatim).
  • Adopt the workflow_run pattern for test reporting (the "optional, fuller fix").
  • Confirm no other ci.yml step assumes secrets or a writable token on PRs — Azure Login, Sign packages, and Push to MyGet are all main-only; the artifact uploads work fine with a read-only fork token.

Note on rollout

workflow_run only fires for the copy of test-report.yml on the default branch, so reports for fork PRs start working once this lands on main. Until then, PR #4634 keeps its continue-on-error stop-gap.

🤖 Generated with Claude Code

GitHub forces GITHUB_TOKEN to read-only for pull_request runs from forks,
so dorny/test-reporter's Checks API call (POST .../check-runs) returned
403 "Resource not accessible by integration" and failed every fork PR,
even when all tests passed. Secrets are likewise absent on fork PRs, so
the Azure OIDC login failed in the build-windows job.

Split reporting out of the build:

- ci.yml now runs with a read-only token and only uploads the .trx files
  as test-results-<platform> artifacts. Azure login and package signing
  are gated to refs/heads/main (publish-only, matching Push to MyGet).
  checks:write is dropped — the build no longer touches the Checks API.
- test-report.yml is triggered by workflow_run when CI completes. It runs
  in the base-repository context with checks:write, downloads the
  test-results-* artifacts, and publishes one inline check per platform.
  This restores real test reports for fork PRs instead of swallowing them.

Closes #4642

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the repository’s CI for forked pull requests by moving test report publication into a separate workflow_run workflow (base-repo context with writable token) while keeping the PR CI workflow limited to building/testing and uploading .trx artifacts.

Changes:

  • Replace inline dorny/test-reporter steps in ci.yml with .trx artifact uploads (test-results-Linux / test-results-Windows).
  • Add test-report.yml triggered via workflow_run to download those artifacts and publish per-platform check runs with checks: write.
  • Restrict Azure login/signing to main only (so fork PRs don’t fail due to missing secrets).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/test-report.yml New workflow_run workflow that publishes test reports from uploaded .trx artifacts using checks: write.
.github/workflows/ci.yml Uploads test results as artifacts instead of publishing checks directly; removes checks: write from CI workflow permissions and ensures main-only publishing steps don’t run on PRs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yml Outdated
Comment on lines +13 to +16
# This workflow runs untrusted code from fork PRs, so it keeps a read-only token
# and never needs secrets. Test results are published by a separate workflow
# (test-report.yml, triggered via workflow_run) that runs in the base-repo
# context with checks:write. See issue #4642.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — reworded. The header comment now scopes the "no secrets / read-only" claim to PR (and especially fork) runs, and explicitly notes the main-only signing/publishing steps in build-windows are where secrets/OIDC are used. (commit 8c86567)

Comment thread .github/workflows/ci.yml
Comment on lines 17 to 19
permissions:
id-token: write
contents: read

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by scoping id-token: write to the build-windows job only (the workflow default is now contents: read), so the Linux build job and any future job carry no OIDC. I kept signing/publishing in build-windows rather than splitting into a dedicated job because that path only runs on main and is untested-by-CI; restructuring it risks silently breaking releases for a marginal gain. Worth noting the fork escalation specifically isn't reachable: GitHub forces the token read-only for fork pull_request runs and will not honor id-token: write, so forks cannot mint an OIDC token here at all. The residual surface is trusted same-repo collaborators, and the Azure federated credential should be subject-constrained on the Azure side regardless. (commit 8c86567)

Per review feedback on #4643:
- The workflow-level header comment claimed it 'never needs secrets', which
  is inaccurate for the main-only signing/publishing steps. Reword to scope
  the no-secrets/read-only claim to PR (and fork) runs.
- id-token:write was granted workflow-wide, so it was active on the Linux
  build job and any future job. OIDC is only used by the main-only Azure
  login that code-signs packages, which lives in build-windows. Move the
  grant to that job so the default is contents:read and the Linux job
  carries no OIDC. (Fork PRs can't mint OIDC tokens regardless — GitHub
  forces the token read-only — so the residual surface is trusted same-repo
  actors.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jbogard jbogard merged commit fed6e64 into main Jun 12, 2026
6 checks passed
@jbogard jbogard deleted the ci/fork-safe-test-reporting branch June 12, 2026 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants