Repository navigation
Replace pull_request_target workflows with pull_request + workflow_run - #195536
Conversation
This resolves flutter#193185. GitHub is restricting `pull_request_target`, which runs with secrets and a write token on events from forks. Each affected workflow is split in two: * An unprivileged `pull_request` workflow with a read-only token and no secrets. Cheap gates (label name, base branch, bot author) are job-level `if:`s so non-matching events never allocate a runner. When it applies, it uploads a small artifact with context about the triggering event (PR number, and the cp channel). * A privileged `workflow_run` workflow that runs from the default branch with access to secrets. It downloads the artifact and does the actual work: commenting, labeling, reverting, cherry-picking, deleting branches. A PR can modify the `pull_request` workflow, so the artifact is untrusted. The privileged side ties the PR number to the triggering run's head SHA and repo. It then reads all other state from the GitHub API, including re-checking any gate whose failure would have made the trigger skip. Checks that end in a comment either way are done only on the privileged side. | Unprivileged (pull_request) | Privileged (workflow_run) | |---------------------------------|---------------------------| | revert-trigger.yml | revert.yml | | cp-trigger.yml | easy-cp.yml | | labeler-trigger.yml | labeler.yml | | delete-bot-branches-trigger.yml | delete-bot-branches.yml | | freeze.yml | freeze-comment.yml | | no-main-prs.yaml | no-main-prs-comment.yaml | freeze.yml and no-main-prs.yaml keep their names because they still provide the PR check. no-main-prs.yaml now gates on `base_ref == 'main'` at the job level, so it no longer uses a runner on every PR.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
These workflows only consume the triggering run's artifact as untrusted data (validated and bound to the PR's head SHA) and never check out or execute PR code.
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Before landing, there are two behavioral regressions caused by switching the triggers from pull_request_target to pull_request that we should address or explicitly plan for:
-
In flutter/flutter,
pull_requestworkflows for first-time contributors do not run automatically, they stop withconclusion: action_requireduntil a maintainer clicks "Approve and run" (see e.g. #195226). Becauselabeler.ymlgates ongithub.event.workflow_run.conclusion == 'success', first-time contributor PRs will no longer receiveteam-*or area labels when opened, which means they will not show up in team PR triage queries (docs/triage/README.md) where a maintainer would find them and approve the workflow run. Similarly, first-time contributors who accidentally targetmainor modify frozen Material/Cupertino files will not get theno-main-prsorfreezefailure/comment until someone manually approves their workflow runs. How should we address this? -
Because
on: pull_requestresolves workflows from the PR's own commit/merge ref (which is frozen once a PR merges):- Applying
cp: beta,cp: stable, orrevertto any PR merged _before- this PR lands will silently do nothing (cp-trigger.ymlandrevert-trigger.ymldo not exist in those PRs' commits, andeasy-cp.yml/revert.ymlonmasterno longer trigger onpull_request_target). Could we add aworkflow_dispatchtrigger (takingpr_number, andchannelforeasy-cp.yml) toeasy-cp.ymlandrevert.ymlas a fallback during the transition window? delete-bot-branches-trigger.ymlwon't exist on currentflutter-*-candidate*branches wheneasy-cp.ymlbranches offupstream/$RELEASE_BRANCH, so we will also want to cherry-pickdelete-bot-branches-trigger.ymlto the activebetaandstablecandidate branches if we want bot branches targeting those branches to be cleaned up.
- Applying
| # Safe because this only consumes an artifact from the triggering run as | ||
| # untrusted data (validated and bound to the PR's head SHA); no PR code is | ||
| # checked out or executed. | ||
| workflow_run: # zizmor: ignore[dangerous-triggers] |
There was a problem hiding this comment.
Once this lands, any PR that was merged before this commit will not have .github/workflows/cp-trigger.yml in its commit tree, so adding cp: beta or cp: stable to pre-existing merged PRs will not trip the Cherry-pick Trigger.
Consider adding a workflow_dispatch trigger (with pr_number and channel inputs) that feeds into the same job/steps so release engineers and authors have a fallback to trigger cherry-picks for PRs that landed prior to this change. (Same suggestion for revert.yml.)
| name: Delete Bot Branches Trigger | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
When easy-cp.yml creates a cherry-pick PR targeting a flutter-*-candidate* branch, the PR's head and base commits come from upstream/$RELEASE_BRANCH. Until delete-bot-branches-trigger.yml is present on the candidate branch, closing a bot PR targeting an existing flutter-*-candidate* branch won't trigger this workflow. We should cherry-pick delete-bot-branches-trigger.yml to the active beta/stable candidate branches after this lands.
| name: Record cherry-pick request | ||
| runs-on: ubuntu-latest | ||
| # Evaluated before a runner is allocated, so other labels cost nothing. | ||
| if: github.event.label.name == format('cp{0} beta', ':') || github.event.label.name == format('cp{0} stable', ':') |
There was a problem hiding this comment.
Nit: Consider adding github.repository == 'flutter/flutter' && to the job 'if:' condition here (and in revert-trigger.yml / delete-bot-branches-trigger.yml, matching labeler-trigger.yml, freeze.yml, and no-main-prs.yaml) so these triggers don't run on forks.
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Thanks for addressing the review feedback! This LGTM!
A few things under docs/ (and our cherry-pick skill script) should also be updated either here or in a follow-up:
- docs/contributing/Labeling-PRs.md: Currently says setting up a new repo only requires copying .github/workflows/labeler.yml. Should mention copying both labeler-trigger.yml and labeler.yml (and updating the repository check).
- docs/releases/Flutter-Cherrypick-Process.md: Should note the manual workflow_dispatch command (gh workflow run easy-cp.yml) for PRs merged before cp-trigger.yml landed, and that the two-step workflow now takes ~1-2 minutes instead of 30 seconds.
- docs/infra/GitHub-Action-Workflows.md: Should document the pull_request + workflow_run pattern now used in place of pull_request_target.
- docs/infra/Ci-Best-Practices.md: Should note the manual workflow_dispatch fallback (gh workflow run revert.yml) for reverting PRs merged before revert-trigger.yml landed.
- .agents/skills/flutter-cherry-pick/scripts/flutter_cp.dart: Triggers cherry-picks by adding the cp label, so it may also want a gh workflow run easy-cp.yml fallback for older merged PRs.
And we should definitely announce to the team and Discord so they are aware of process changes related to this.
…kflows Documents the pattern replacing pull_request_target, the workflow_dispatch fallbacks for reverting or cherry-picking PRs merged before the trigger workflows existed, and the two-file labeler setup. The cherry-pick skill now dispatches easy-cp.yml directly for PRs whose merge commit lacks cp-trigger.yml.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If both you and the PR author believe this PR qualifies for a test exemption, escalate to the tech lead for the changed area to review and apply the |
github-actions
Bot
added
c: contributor-productivity
eyebrowsoffire
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
…migration/phase0 Two conflicts in dev/bots: master's flutter#195536 adds runCommand's `checkExitCode` beside the branch's `acceptedExitCodes` (both kept: a check that is off accepts everything, an accepted code is never a failure), and its test beside ours. Rolls: Dart 3b174b7217fe (and the Dart packages DEPS pins), Skia b11315d0a096, pub -- deps.lock.bzl and pub.lock.bzl regenerated. master's new linux_riscv_host_engine.json (flutter#178712, GN) is noted by the converter as a platform the Bazel build has none for (configs.bazelrc); no other GN file changed. Checked: Bazel analysis of ci_host_debug_unopt (+tests) and the root suites under --config=prebuilt_engine; the converter's builder tests; `dart analyze` of the two dev/bots files.
flutter/flutter@09175cd...e36b370 2026-11-07 [email protected] Roll Fuchsia Linux SDK from KIc0XZbhLmvDpTluZ... to Sf2yvdTiFXAKkfLkD... (flutter/flutter#196160) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196155) 2026-11-07 [email protected] [iOS a11y] Use a real `UIScrollView` as a11y scrollable container (flutter/flutter#195731) 2026-11-07 [email protected] Add ParagraphStyle.hyphenCharacter to dart:ui (flutter/flutter#195345) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196143) 2026-11-06 [email protected] [Linux] Fix layer position when compositing without glBlitFramebuffer (flutter/flutter#196083) 2026-11-06 [email protected] Add support for riscv64 desktop linux engine (flutter/flutter#178712) 2026-11-06 [email protected] Add ResizeImagePolicy.cover (flutter/flutter#194665) 2026-11-06 [email protected] Roll Skia from f533d4034900 to b11315d0a096 (6 revisions) (flutter/flutter#196138) 2026-11-06 [email protected] iOS: Remove dead code/unused imports from FlutterPlatformViewsController (flutter/flutter#196084) 2026-11-06 [email protected] Replace pull_request_target workflows with pull_request + workflow_run (flutter/flutter#195536) 2026-11-06 [email protected] [master] Update Flutter DEPS to Dart 3b174b7217fe09b21da836ea623932b5f041b091 (flutter/flutter#196004) 2026-11-06 [email protected] Fix android_hardware_smoke_test retry mechanisms (flutter/flutter#195708) 2026-11-06 [email protected] Roll Skia from 452b9c1e5db6 to f533d4034900 (1 revision) (flutter/flutter#196129) 2026-11-06 [email protected] [flutter_tools] Use `ErrorHandlingFileSystem.deleteIfExists` for `IOSCoreDeviceControl` temp directory cleanup (flutter/flutter#195913) 2026-11-06 [email protected] Adds guideline tests to flutter create samples (flutter/flutter#195827) 2026-11-06 [email protected] refactor(flutter_tools): unify OptionDescriptor identity and addTo (flutter/flutter#196076) 2026-11-06 [email protected] Roll pub packages (flutter/flutter#196114) 2026-11-06 [email protected] Roll Skia from 5ce64e63a0e7 to 452b9c1e5db6 (5 revisions) (flutter/flutter#196111) 2026-11-06 [email protected] Add PlatformDispatcher.onAlwaysUse24HourFormatChanged callback (flutter/flutter#194670) 2026-11-06 [email protected] Roll Packages from 0478162 to 63881f9 (10 revisions) (flutter/flutter#196108) 2026-11-06 [email protected] Roll Skia from 793b8ac73d4b to 5ce64e63a0e7 (4 revisions) (flutter/flutter#196107) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Removes the remaining `pull_request_target` triggers in `.github/workflows/`: - Replaces `pull_request_label.yml` with `labeler-trigger.yml` (`pull_request`) and `labeler.yml` (`workflow_run`), matching flutter/flutter#195536. - Removes `remove_cicd.yml`, since `CICD` label management is now handled in Cocoon (matching flutter/flutter#189087). Part of flutter/flutter#193185 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
This resolves #193185.
GitHub is restricting
pull_request_target, which runs with secrets and a write token on events from forks. Each affected workflow is split in two:An unprivileged
pull_requestworkflow with a read-only token and no secrets. Cheap gates (label name, base branch, bot author) are job-levelif:s so non-matching events never allocate a runner. When it applies, it uploads a small artifact with context about the triggering event (PR number, and the cp channel).A privileged
workflow_runworkflow that runs from the default branch with access to secrets. It downloads the artifact and does the actual work: commenting, labeling, reverting, cherry-picking, deleting branches.A PR can modify the
pull_requestworkflow, so the artifact is untrusted. The privileged side ties the PR number to the triggering run's head SHA and repo. It then reads all other state from the GitHub API, including re-checking any gate whose failure would have made the trigger skip. Checks that end in a comment either way are done only on the privileged side.freeze.yml and no-main-prs.yaml keep their names because they still provide the PR check. no-main-prs.yaml now gates on
base_ref == 'main'at the job level, so it no longer uses a runner on every PR.