Secure release-note checks for fork pull requests by NatElkins · Pull Request #20081 · dotnet/fsharp

NatElkins · 2026-07-23T13:41:53Z

Summary

avoid checking out fork pull request heads from the privileged pull_request_target workflow
read trusted release-note configuration from the base commit and inspect changed-file metadata through the GitHub API
read candidate release-note Markdown as inert text at the exact pull request head
discard stale results and avoid posting empty comments
reduce token permissions and consolidate comment updates on actions/github-script@v9

GitHub backported safer actions/checkout defaults to floating supported major tags on July 20, 2026. The current actions/checkout@v2 step now rejects the fork checkout before the release-note logic runs, as seen in https://github.com/dotnet/fsharp/actions/runs/30010753188/job/89217916202?pr=20031.

Using allow-unsafe-pr-checkout: true would restore the old behavior but would preserve the privileged fork-checkout risk. This change keeps fork contributions working without opting out of that protection.

Validation

actionlint .github/workflows/check_release_notes.yml
replayed the shell step against PR Add an experimental flag-gated in-process compile path for hot reload sessions #20031 at its exact head and found the changed compiler release-note file
replayed against PR Add Roslyn-format EnC CustomDebugInformation codec and portable PDB method CDI emission #20018 with NO_RELEASE_NOTES and produced the expected opt-out result with exit code 0
replayed with a stale head SHA and confirmed it exits successfully without emitting comment output

Expected check state

The check_release_notes result on this PR is expected to remain red. The pull_request_target event loads its workflow definition from the base branch, so this PR runs the old workflow from main and fails before it can see the fix. The PR cannot validate its own replacement through this event. After this change merges, subsequent pull request events will use the fixed workflow.

Refresh status (2026-07-24)

Current head: abb68322f3, with dotnet/fsharp main 1dc395ad34 merged.
All security-review threads are addressed and resolved. The PR is ready for review.
Validation remains: actionlint, live release-note and opt-out replays, and stale-head rejection.
This PR's own check_release_notes result is expected to stay red because pull_request_target loads the old workflow from main. That exception is explained in the Expected check state section above.

NatElkins · 2026-07-23T14:10:50Z

@T-Gro I dug into why this suddenly started failing across the hot reload PRs.

GitHub announced safer defaults for pull_request_target on June 18 and moved enforcement for backported actions/checkout versions to July 20, 2026: https://github.blog/changelog/2026-06-18-safer-pull_request_target-defaults-for-github-actions-checkout/

This workflow itself had not recently changed. Its last repository change was June 8, but it uses the floating actions/checkout@v2 tag, so it automatically picked up the backported protection. Since pull_request_target runs with the base repository token, checkout now refuses to fetch a fork PR head by default. That is why older fork PRs passed and the current ones all started failing before the release-note logic runs. One example is https://github.com/dotnet/fsharp/actions/runs/30010753188/job/89217916202?pr=20031.

I also verified that NO_RELEASE_NOTES is not causing this. I replayed the updated shell logic against both a PR with release notes and a PR carrying that label, and both completed successfully.

I do not think contributors should stop using forks, and I do not think allow-unsafe-pr-checkout: true is the right fix. This PR keeps the normal fork workflow, reads the trusted configuration from the base commit, and uses the GitHub API to inspect the changed-file list and release-note Markdown at the exact PR head without checking out or executing fork content. It also discards stale results and avoids posting an empty comment.

I validated it with actionlint, a live replay against #20031, an opt-out replay against #20018, and a stale-head replay. I would appreciate your review since you last worked on this workflow.

T-Gro · 2026-07-24T08:41:02Z

The explanation makes sense. Let me verify any security risks (should be fewer, just need to check against common exploits).
Will merge ASAP, thanks for addressing this @NatElkins 👍

…k-checkout

github-actions · 2026-07-28T21:54:51Z

🔍 Tooling Safety Check — Affects-Build-Infra
Affects-Build-Infra: modifies .github/workflows/check_release_notes.yml

Generated by PR Tooling Safety Check · opus46 12.1M · ◷

The check_release_notes workflow (rewritten in #20081) reduced the token to pull-requests: read. issues.createComment (POST) on a pull request requires pull-requests: write, so brand-new PRs that have no existing bot comment -- for example dependency updates opened by dotnet-maestro (#20134) -- failed with 403 'Resource not accessible by integration'. Only PRs that already had a comment from before #20081 passed, because the updateComment (PATCH) path works with issues: write. The security fix in #20081 was removing the untrusted fork-head checkout; that protection is unchanged. No untrusted code runs and the comment body is passed via an environment variable, so restoring pull-requests: write does not reintroduce the fork-checkout risk. Also wrap the comment logic in try/catch so posting the informational comment can never fail the release-notes gate, which is enforced by the previous step's exit code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…t 403 Rewriting the release-notes check in #20081 reduced the token to pull-requests: read, but posting a NEW PR comment (issues.createComment POST) requires pull-requests: write. Brand-new PRs with no existing bot comment (like #20130 and dependency PRs) therefore fail with 403 'Resource not accessible by integration', while PRs with a pre-existing comment pass via the updateComment PATCH path (allowed by issues: write). Restore pull-requests: write (the fork-head checkout removed in #20081 stays removed, so no untrusted code runs and the comment body is passed via env var) and wrap the comment write in try/catch so the best-effort informational comment can never fail the release-notes gate. Supersedes the abandoned PR #20136. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ing the comment non-fatal The check_release_notes job posts an informational comment on the PR. Creating a PR comment requires pull-requests: write, but #20081 reduced the token to pull-requests: read. Because the job runs via pull_request_target (GitHub executes the workflow from the default branch), this turned the required check red with HTTP 403 'Resource not accessible by integration' on any PR that needed to create (not update) the comment - e.g. the Maestro/darc PR #20133 targeting release/dev18.0 - even though release-notes validation itself passed. Restore pull-requests: write so the comment can actually be posted, and additionally guard the comment step with continue-on-error plus a try/catch that downgrades any failure to a warning. The real gate (exit 1 when release notes are genuinely missing) is unchanged, and no PR content is evaluated as JavaScript. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ing the comment non-fatal (#20198) check_release_notes runs via pull_request_target, so GitHub executes the workflow from the default branch (main). Creating the informational PR comment requires pull-requests: write, but #20081 reduced the token to read, turning the check red with HTTP 403 on any PR that had to create (not update) the comment - e.g. Maestro/darc PR #20133. Restore pull-requests: write so the comment posts, and guard the comment step with continue-on-error plus try/catch so posting can never fail the release-notes verdict. Supersedes #20200.

PR #20081 (Secure release-note checks for fork pull requests) downgraded the check_release_notes workflow permissions from 'pull-requests: write' to 'pull-requests: read' while keeping 'issues: write'. Commenting on a pull request via GitHub Actions requires 'pull-requests: write' (issues: write alone is insufficient for PR conversation comments), so the final 'Create or update comment' step began failing with 'Resource not accessible by integration' (HTTP 403). PR #20135 is the first codeflow PR to run the new workflow and surfaced the regression. Restore 'pull-requests: write' at both workflow and job level while keeping the rest of the #20081 hardening (contents: read, explicit env, stale-head guards). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Secure release-note checks for fork pull requests

62153e1

github-project-automation Bot added this to F# Compiler and Tooling Jul 23, 2026

github-project-automation Bot moved this to New in F# Compiler and Tooling Jul 23, 2026

NatElkins marked this pull request as ready for review July 23, 2026 14:13

NatElkins requested a review from a team as a code owner July 23, 2026 14:13

github-actions Bot added the ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure label Jul 23, 2026

This comment has been minimized.

Sign in to view

T-Gro reviewed Jul 24, 2026

View reviewed changes

Comment thread .github/workflows/check_release_notes.yml

Comment thread .github/workflows/check_release_notes.yml

Comment thread .github/workflows/check_release_notes.yml Outdated

NatElkins added 2 commits July 24, 2026 10:00

Address release-note workflow review feedback

428cec4

Merge remote-tracking branch 'origin/main' into fix/release-notes-for…

873c7b3

…k-checkout

NatElkins mentioned this pull request Jul 24, 2026

Parser: recover on unfinished abstract members #20070

Open

Merge current main

abb6832

This comment has been minimized.

Sign in to view

T-Gro approved these changes Aug 3, 2026

View reviewed changes

github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 3, 2026

T-Gro merged commit 647548b into dotnet:main Aug 3, 2026
3 of 4 checks passed

github-project-automation Bot moved this from In Progress to Done in F# Compiler and Tooling Aug 3, 2026

This was referenced Aug 3, 2026

[release/11.0.1xx-preview7] Source code updates from dotnet/fsharp dotnet/dotnet#8111

Closed

[main] Source code updates from dotnet/fsharp dotnet/dotnet#8116

Open

T-Gro mentioned this pull request Aug 4, 2026

Restore pull-requests: write so release-note comments can post #20136

Closed

dotnet-maestro Bot mentioned this pull request Aug 4, 2026

[release/11.0.1xx-preview7] Source code updates from dotnet/fsharp dotnet/dotnet#8126

Open

T-Gro mentioned this pull request Aug 4, 2026

[release/10.0.4xx] Update dependencies from dotnet/arcade #20134

Open

Navigation Menu

Uh oh!

Secure release-note checks for fork pull requests - #20081

Secure release-note checks for fork pull requests#20081
T-Gro merged 4 commits into
dotnet:mainfrom
NatElkins:fix/release-notes-fork-checkout

NatElkins commented Jul 23, 2026 •

edited

Loading

Uh oh!

NatElkins commented Jul 23, 2026

Uh oh!

This comment has been minimized.

T-Gro commented Jul 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

github-actions Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

NatElkins commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Expected check state

Refresh status (2026-07-24)

Uh oh!

NatElkins commented Jul 23, 2026

Uh oh!

This comment has been minimized.

T-Gro commented Jul 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

github-actions Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NatElkins commented Jul 23, 2026 •

edited

Loading