Skip to content

Auto-merge clean inter-branch forward merges - #36875

Merged
kubaflo merged 8 commits into
mainfrom
pureween-automerge-inter-branch-merge-prs
Jul 31, 2026
Merged

Auto-merge clean inter-branch forward merges#36875
kubaflo merged 8 commits into
mainfrom
pureween-automerge-inter-branch-merge-prs

Conversation

@PureWeen

@PureWeen PureWeen commented Jul 28, 2026

Copy link
Copy Markdown
Member

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

Auto-approve and enable GitHub native auto-merge for immutable snapshots of these exact forward-merge targets:

main    => net11.0
net11.0 => release/11.0.1xx-preview7
net11.0 => release/11.0.1xx-rc1
net11.0 => release/11.0.1xx-rc2

Immutable snapshot design

Each caller workflow checks for its generated merge PR before invoking Arcade:

no open merge PR  -> run Arcade and create a fresh snapshot
open merge PR     -> leave its branch unchanged while CI runs

The checks use exact head/base pairs and require the PR authoring App to be github-actions with isCrossRepository == false. The release workflow resolves the current MergeToBranch from github-merge-flow-release-11.jsonc on net11.0, and passes configuration_file_branch: net11.0 to Arcade so the gate and merge implementation use the same source of truth.

Workflow runs are serialized with distinct concurrency groups so simultaneous push/schedule/manual runs cannot both pass the check and create or update the same PR. Because scheduled workflows start from the default branch, the release schedule uses a schedule-only job to dispatch merge-net11-to-release.yml at ref net11.0; the dispatched run is not a schedule event and cannot recurse.

During rollout, the schedule job first compares the parsed safety-critical sections (concurrency, CheckForOpenMergePullRequest, and Merge) between the workflow on main and net11.0. It skips the dispatch until the full immutable-snapshot gate has propagated, while allowing unrelated branch-specific workflow differences. Fetch or parse failures fail visibly instead of dispatching an unknown definition.

The read-only snapshot checks use read-only GITHUB_TOKEN permissions. Only the reusable Arcade merge job retains content and pull-request write access.

New source commits that arrive while a merge PR is open wait for the next generated PR. Once the current PR merges or closes, the next push or daily schedule creates a fresh snapshot containing the remaining commits.

This intentionally stops using Arcade's existing "fast-forward the open merge PR" behavior. The generated PR head does not change during CI, so source-branch pushes do not invalidate its approval or restart CI.

Policy Service rule

The rule runs only on Opened; Synchronize is not accepted.

It requires:

  • event sender github-actions[bot]
  • event sender is also the PR author
  • exact target branch
  • exact, fully anchored generated title

Human conflict-resolution pushes do not trigger reapproval. Bot-attributed /rebase synchronization also does not trigger the policy, closing the review-bypass path identified in the adversarial review.

Review behavior and accepted limitation

MAUI protection intentionally remains:

dismiss_stale_reviews_on_push: true
require_last_push_approval: false
required_approving_review_count: 1

This preserves the repository's human-review workflow: a maintainer who pushes a fix to another person's PR can provide the subsequent approval without requiring a third reviewer. This PR does not modify repository rulesets or add a bypass.

The generated merge PR remains safe under these settings. Policy Service approves only the initial Opened snapshot, the caller workflow refuses to invoke Arcade while the exact bot-authored PR remains open, and any out-of-band head push dismisses the approval with no automatic reapproval path.

Ordinary target-branch advancement does not require the generated PR to update because the required status-check rules use strict_required_status_checks_policy: false. If the immutable head remains conflict-free, its approval remains valid, and required checks pass, auto-merge can complete against the advanced base. In the narrower case where base activity actually changes the reviewed diff or merge base, GitHub can dismiss the approval and safely stall the PR. That fail-closed limitation is accepted for this automation.

Exact branch and title allow-list

net11.0                    + ^[automated] Merge branch 'main' => 'net11.0'$
release/...-preview7       + ^[automated] Merge branch 'net11.0' => 'release/...-preview7'$
release/...-rc1            + ^[automated] Merge branch 'net11.0' => 'release/...-rc1'$
release/...-rc2            + ^[automated] Merge branch 'net11.0' => 'release/...-rc2'$

Each entry in the file contains the full literal branch and anchored regex. Targets outside this allow-list remain manual.

Merge behavior

- enableAutoMerge:
    mergeMethod: merge

This always creates a true merge commit, never squash or rebase. Arcade relies on merge ancestry to determine what remains to flow.

GitHub completes auto-merge only when the PR has no merge conflict and required checks pass.

Required checks

ruleset checks Policy Service bypass
MAUI required CI checks maui-pr none
MAUI device and UI test checks maui-pr-devicetests, maui-pr-uitests pull requests only

A failing or pending maui-pr blocks the merge. Device/UI checks do not.

MAUI protection has no Policy Service bypass. It requires one ordinary approval; Policy Service supplies it for the exact authenticated Opened events above.

CODEOWNERS

PR #36890 removes the invalid CODEOWNERS file. Require review from Code Owners is disabled in MAUI protection; the ordinary one-approval requirement remains.

Accepted trust boundary

An initial Opened event authorizes on exact title/base plus github-actions[bot] as both sender and PR author. A collaborator with repository push access could deliberately create a same-repository workflow and matching PR. Real maui-pr from Azure Pipelines integration 9426 must still pass, but there is no additional human review under the intentionally ordinary one-review policy.

This tradeoff is accepted for these exact forward-merge target pairs. The immutable-snapshot gate prevents later human content from being reapproved through synchronization. No new App is installed and no bypass is added to MAUI protection or the maui-pr ruleset.

Verification

  • Both caller workflows pass actionlint.
  • All three changed YAML files parse successfully.
  • The live target resolver returns release/11.0.1xx-preview7.
  • Exact App/head/base/same-repository queries identify [automated] Merge branch 'main' => 'net11.0' #36886 (main => net11.0) and [automated] Merge branch 'net11.0' => 'release/11.0.1xx-preview7' #36880 (net11.0 => release/11.0.1xx-preview7).
  • The semantic rollout check rejects the current old net11.0 workflow and accepts matching safety-critical sections.
  • Official GitHub documentation confirms that workflow_dispatch events created with GITHUB_TOKEN start workflow runs; the dispatched event skips the schedule-only job.
  • The Policy Service file parses and GitOps schema validation runs on every update.
  • The live MAUI protection settings and effective non-strict required-status-check rules were reverified on 2026-07-31.

Issues Fixed

None; infrastructure automation.

The inter-branch merge flow opens "[automated] Merge branch ... => ..." PRs but
deliberately never merges them, so every clean forward merge still waits on a
human. Add a Policy Service rule that auto-approves those PRs and enables
auto-merge, matching what dotnet/roslyn and dotnet/vscode-csharp already do.

Scoped to the two flows this repo runs, and uses a merge commit because the
merge flow requires one rather than a squash or rebase. Conflicted PRs are
unaffected and still need a human.
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:28 — with GitHub Actions Inactive
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36875

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36875"

@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:29 — with GitHub Actions Inactive
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:33 — with GitHub Actions Inactive
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:35 — with GitHub Actions Inactive
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:36 — with GitHub Actions Inactive
@github-actions github-actions Bot added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Jul 28, 2026
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 28, 2026 13:37 — with GitHub Actions Inactive
@PureWeen PureWeen changed the title Auto-approve and auto-merge automated inter-branch merge PRs [DO NOT MERGE] Auto-merge for inter-branch merge PRs — security review says this approach is unsafe Jul 28, 2026
Require both the exact generated title and the exact net11.0 target branch.
Do not auto-merge the net11.0 => release flow. The Policy Service still
enables native auto-merge with mergeMethod=merge, while maui-pr remains a
required check in a separate ruleset with no app bypass.
@PureWeen PureWeen changed the title [DO NOT MERGE] Auto-merge for inter-branch merge PRs — security review says this approach is unsafe Auto-merge clean main to net11.0 forward merges Jul 28, 2026
Cover both main => net11.0 and net11.0 => release/* automated merge PRs.
Re-approve and re-arm auto-merge when Arcade synchronizes the merge branch so
the organization latest-push approval rule is satisfied. Human conflict
resolution pushes do not match because their activity sender is not
GitHub Actions.
@PureWeen PureWeen changed the title Auto-merge clean main to net11.0 forward merges Auto-merge clean inter-branch forward merges Jul 28, 2026
@PureWeen
PureWeen marked this pull request as ready for review July 28, 2026 18:58
Copilot AI review requested due to automatic review settings July 28, 2026 18:58
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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

Enables Policy Service automation to auto-approve and turn on GitHub native auto-merge for narrowly-scoped, automated inter-branch forward-merge PRs (main → net11.0 and net11.0 → release/*), so clean merges can land without manual intervention while still requiring required CI checks.

Changes:

  • Add a Policy Service rule that matches github-actions[bot] PR activity on Opened/Synchronize events for the targeted forward-merge PR title patterns.
  • Automatically approves matching PRs and enables GitHub auto-merge using the merge method (merge commit).
Show a summary per file
File Description
.github/policies/resourceManagement.yml Adds a Policy Service rule to auto-approve and enable auto-merge for the specified automated forward-merge PRs.

Copilot's findings

Comments suppressed due to low confidence (1)

.github/policies/resourceManagement.yml:741

  • For the net11.0→release/* flow, using titleContains without anchoring means a title that merely includes this substring anywhere would match. If the intent is a strict title prefix match (as described), consider using a regex anchored at the start so only the intended automated merge titles are eligible for auto-approval/auto-merge.
        - titleContains:
            pattern: "[automated] Merge branch 'net11.0' => 'release/"
            isRegex: False
  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread .github/policies/resourceManagement.yml Outdated
Comment on lines +736 to +738
- titleContains:
pattern: "[automated] Merge branch 'main' => 'net11.0'"
isRegex: False

@kubaflo kubaflo 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.

Round 1 — adversarial 4-model security review (Claude Opus 4.8 + GPT‑5.5 + Gemini 3.1 Pro + GPT‑5.6 Sol). Findings independently verified against the repo's own workflows + branch rules.

The intent — auto‑approving/merging the mechanical [automated] Merge branch … forward‑merges — is reasonable, and the Opened path is well gated. But the Synchronize arm has a real review‑bypass, and I want to be transparent: three of the four models (and my own first pass) initially cleared this as "bot‑sender gate is airtight." Sol found the hole, and I verified it end‑to‑end — it's genuine.

❌ HIGH — the Synchronize arm auto‑approves on bot activity, not PR provenance → unreviewed content can auto‑merge into net11.0/release

The rule fires when the activity sender is github-actions[bot] and the action is Opened or Synchronize (resourceManagement.yml:721-746). isActivitySender checks who triggered the event, not who authored the PR or where the head comes from — and a bot‑attributed Synchronize is trivially reachable for human/fork content:

  1. A contributor opens a PR from a fork → base net11.0, with a title that merely contains "[automated] Merge branch 'main' => 'net11.0'" (titleContains, isRegex:False = substring, author‑controlled) and arbitrary content. The Opened event's sender is the human, so the rule correctly does not fire here.
  2. /rebase on that PR runs .github/workflows/rebase.yml, which gh pr update-branch --rebasees it using secrets.GITHUB_TOKEN (perms contents/pull-requests: write). The push is therefore attributed to github-actions[bot], so the resulting Synchronize event's sender is the bot.
  3. The rule now matches → approvePullRequest + enableAutoMerge on the human‑/fork‑authored content.

This is a real, demonstrated pattern, not hypothetical: #36448 is a fork PR (author == headRepositoryOwner) that was force‑pushed by github-actions[bot] after a /rebase. And the usual dismiss_stale_reviews protection does not save it — I checked the ruleset (net11.0 requires 1 approval with dismiss_stale_reviews_on_push: true), but the bot Synchronize is precisely what re‑creates a fresh approval, and with the bot as last‑pusher it also satisfies require_last_push_approval. So the stale‑dismissal I (and GPT/Opus) leaned on is bypassed by the very event that re‑approves.

The barrier is that a write‑access user must run /rebase (rebase.yml verifies commenter write access). But /rebase is a routine maintainer courtesy — a maintainer rebasing a magic‑titled PR to "help it along" would silently auto‑approve+merge unreviewed (possibly fork‑authored) code into net11.0. That defeats the required‑review control for a shipping branch.

Fix: don't grant approval/auto‑merge on Synchronize off the bot‑sender alone. Either restrict to Opened by the codeflow identity, or validate real provenance before approving — exact head repository (must be dotnet/maui), exact expected head branch, exact base branch, and commit ancestry — ideally via a dedicated workflow with its own App identity rather than isActivitySender: github-actions[bot].

⚠️ MEDIUM — the net11.0 → release/* arm has no targetsBranch guard

The second arm matches on the title substring "[automated] Merge branch 'net11.0' => 'release/" alone (:739-741), with no targetsBranch (unlike the main→net11.0 arm, which correctly ands targetsBranch: net11.0). The title is author‑controlled and doesn't prove the destination, so an eligible (bot‑attributed) Synchronize on such a PR could auto‑merge into an unintended base — including main. Combined with the Synchronize issue above, this widens the blast radius to release branches. Fix: require the exact configured release base + expected head branch, and anchor the full title (isRegex with ^…$).


Verdict: REQUEST_CHANGES

The Opened‑by‑bot flow is fine; the problem is auto‑approving Synchronize off isActivitySender: github-actions[bot], which the /rebase workflow makes reachable for human/fork content (and which dismiss_stale_reviews does not contain). Gate the approval on real PR provenance, and add the missing targetsBranch/anchored‑title on the release arm, before enabling this.

Reviewed at head 7238fa0c252. Verified: rebase.yml pushes via GITHUB_TOKEN (bot‑attributed), #36448 shows the fork‑content/bot‑push split, the rule includes isAction: Synchronize, and the net11.0 ruleset's dismiss_stale_reviews_on_push is re‑satisfied by the bot sync. Credit to the Sol pass for catching what the other three models (and my first read) missed.

🤖 AI-assisted adversarial review (Opus 4.8 + GPT‑5.5 + Gemini 3.1 Pro + Sol; orchestrator independently verified rebase.yml, PR #36448 provenance, the branch ruleset, and the rule's Synchronize arm). Not a maintainer approval.

Require GitHub Actions to be both the activity sender and PR author before
automatically approving an Opened or Synchronize event. This excludes human
and fork PRs updated by the /rebase workflow. Also anchor both generated titles
and require exact target branches; release auto-merge fails closed when the
release train advances until the policy is updated.
Copilot AI review requested due to automatic review settings July 28, 2026 21:49
@PureWeen

Copy link
Copy Markdown
Member Author

Addressed the adversarial review at head 5a23ca304cac:

  • Synchronize now requires github-actions[bot] to be both the activity sender and PR author. Arcade-generated PRs satisfy both; human/fork PRs updated by /rebase do not.
  • Both titles are fully anchored regexes.
  • Both arms require exact target branches. Release is pinned to release/11.0.1xx-preview7 and intentionally fails closed when the train advances; the policy must be updated with the configured release target.
  • Human conflict-resolution pushes do not match and receive no fresh approval.

GitOps schema validation is being re-run on the updated rule.

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.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new

Enumerate the remaining .NET 11 release milestones as exact target/title pairs.
This preserves the fail-closed target validation while avoiding a policy update
when the train advances from preview7 through rc1 and rc2.
Copilot AI review requested due to automatic review settings July 28, 2026 22:04
@PureWeen

Copy link
Copy Markdown
Member Author

Reworked the design at head 7ea1094ac546 to remove Synchronize entirely.

  • Both MAUI caller workflows now check for their open generated merge PR before invoking Arcade.
  • If one is open, the workflow leaves its branch immutable while CI runs; later source commits wait for the next PR.
  • Workflow-level concurrency serializes push/schedule/manual runs and avoids duplicate-creation races.
  • Policy Service now approves only the original bot-authored Opened event.
  • Human pushes and /rebase synchronization receive no fresh approval, so the residual HIGH review-bypass is closed without a separate provenance pipeline.

The live gates currently detect #36886 and #36880 and correctly choose the skip path. Both workflows pass actionlint.

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.

Copilot's findings

Comments suppressed due to low confidence (2)

.github/workflows/merge-main-to-net11.yml:50

  • The open-PR check can be tripped by a non-bot PR (including a fork PR) that happens to use the same head branch name, which would block the workflow from creating the real automated merge PR. Filter the query to only treat PRs authored by github-actions[bot] as merge PRs.
            --head 'merge/main-to-net11.0' \
            --base 'net11.0' \
            --json number \
            --jq '.[0].number // empty')"

.github/workflows/merge-net11-to-release.yml:44

  • The open-PR check matches any open PR whose headRefName starts with merge/net11.0-to-release/. A fork PR (or any non-bot PR) could use the same branch name and inadvertently block automation. Filter to PRs authored by github-actions[bot] so only the generated merge PRs can suppress new snapshots.
            --state open \
            --json number,headRefName \
            --jq '[.[] | select(.headRefName | startswith("merge/net11.0-to-release/"))][0].number // empty')"
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new

Use exact bot-authored same-repository PR queries, resolve the active release target from net11.0, and dispatch scheduled release merges from the required branch.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1b2b2dd6-bf5f-4179-8a8f-c4c85b9bce26
Copilot AI review requested due to automatic review settings July 29, 2026 20:52
@PureWeen

Copy link
Copy Markdown
Member Author

Applied the latest adversarial consensus at 2467b16f404 (Claude Opus 4.8, GPT-5.6 Sol, and Claude Sonnet 5; three independent reviewers).

  • Snapshot detection now requires the github-actions App, exact head/base pairs, and isCrossRepository == false in both workflows.
  • The release workflow resolves the active MergeToBranch from net11.0 and queries only that target, so old release PRs cannot block the next train branch.
  • The default-branch schedule now dispatches one run at ref net11.0; normal jobs skip the schedule invocation, avoiding recursion.
  • Policy Service remains Opened-only with exact allow-listed target/title pairs and merge commits.

Remaining prerequisite: repository ruleset MAUI protection is still dismiss_stale_reviews_on_push: true / require_last_push_approval: false. Before enabling this automation, it needs to be aligned to false / true; I verified the live API state and documented the prerequisite in the PR body without mutating the ruleset.

Validation: actionlint is clean, all changed YAML parses, the live resolver returns release/11.0.1xx-preview7, and the exact queries return #36886 and #36880. GitOps schema validation is expected on the pushed commit.

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.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment on lines +64 to +65
$config = $configText | ConvertFrom-Json
$mergeToBranch = $config.'merge-flow-configurations'.'net11.0'.MergeToBranch
Keep the release gate and Arcade on the same net11.0 configuration, defer scheduled dispatch until the safety-critical workflow sections have propagated, and scope read-only checks to read permissions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1b2b2dd6-bf5f-4179-8a8f-c4c85b9bce26
Copilot AI review requested due to automatic review settings July 29, 2026 21:32

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adversarial follow-up — fixes pushed at be5efbbfe48

No unresolved code findings remain after three independent reviewers with adversarial consensus and self-correction passes.

Applied findings:

  • 3/3 reviewers — rollout safety: scheduled dispatch now fails closed until net11.0 contains the same parsed safety-critical workflow sections as main; it cannot invoke the old ungated definition during propagation.
  • 2/3 reviewers — config consistency: Arcade now reads github-merge-flow-release-11.jsonc from net11.0, matching the target resolver and eliminating release-cut branch divergence.
  • 2/3 after dispute — least privilege: both read-only PR-check jobs now use contents: read and pull-requests: read.

The final semantic guard was re-reviewed after two self-corrections (whole-file equality was too restrictive; a text marker was too weak). It compares the complete parsed concurrency, CheckForOpenMergePullRequest, and Merge sections, allowing unrelated branch-specific edits while rejecting any safety-gate drift.

Validation coverage: actionlint, YAML parsing for all changed files, live target resolution, exact live PR queries, and old-vs-new semantic rollout discrimination. The existing JSONC parsing concern was reproduced with the exact live config and did not fail.

The separately documented MAUI protection ruleset prerequisite remains unchanged and must still be completed before enabling unattended auto-merge.

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.

Copilot's findings

Comments suppressed due to low confidence (2)

.github/workflows/merge-net11-to-release.yml:126

  • ConvertFrom-Json is being run on github-merge-flow-release-11.jsonc, which contains // JSONC comments (e.g., the first line of the file). PowerShell's JSON parser will fail on comment syntax, so this step will throw even when gh api succeeds. Strip the comment lines before calling ConvertFrom-Json (or switch to a JSONC-capable parser).

          $config = $configText | ConvertFrom-Json
          $mergeToBranch = $config.'merge-flow-configurations'.'net11.0'.MergeToBranch

.github/policies/resourceManagement.yml:728

  • The isActivitySender predicates are mutually exclusive here: the first requires the sender to be github-actions[bot] and not the PR author (issueAuthor: False), while the second requires the sender to be the PR author (issueAuthor: True). Because all if entries are AND'ed, this rule will never match and will never auto-approve the intended PRs. Combine this into a single predicate with issueAuthor: True.
      - isActivitySender:
          user: github-actions[bot]
          issueAuthor: False
      - isActivitySender:
          issueAuthor: True
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new

@kubaflo kubaflo 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.

Round 3 — the HIGH review-bypass is closed ✅ (2 design/ops residuals for discussion)

Re-reviewed the reworked design (7ea10942467b16be5efbb) with an independent adversarial pair (Claude Opus 4.8 + GPT‑5.6‑sol), and verified the branch-protection facts myself via the GitHub API.

✅ The residual HIGH from the last round is closed

The prior bypass was: the auto-approve fired on Synchronize, so a force-push onto an already-approved bot merge PR rode straight into net11.0 without human review. The rework closes it, confirmed on both the policy and the live rulesets:

  • Synchronize is goneresourceManagement.yml now gates on isAction: Opened only, so a later push / /rebase never re-triggers approval (grep -c Synchronize on the policy = 0).
  • Approval is dismissed on push — I confirmed via gh api repos/dotnet/maui/rules/branches/<b> that net11.0, release/11.0.1xx-preview7, rc1, and rc2 all aggregate their active rulesets to required_approving_review_count: 1 + dismiss_stale_reviews_on_push: true (preview7/rc also add require_last_push_approval: true). So a force-push to the unprotected merge/* head dismisses the bot's stale approval → 0 < 1 → auto-merge stalls with no re-approval path (Opened-only policy).
  • The workflow-side hardening (--app github-actions + isCrossRepository == false gate filter, MergeToBranch regex validation, least-privilege permissions: blocks, the YAML.safe_load rollout-parity check, configuration_file_branch pin) all checked out clean — no ref/flag/GITHUB_OUTPUT injection, and the gate fails closed (set -eo pipefail → job fails → Merge skipped). workflow_dispatch can't inject a source (arcade keys config by GITHUB_REF_NAME; the --ref net11.0 dispatch target is a hardcoded protected literal).

Nice iteration — the core bypass is genuinely gone.

🟡 Two residuals worth a maintainer decision (non-blocking)

Neither reopens the HIGH; surfacing them because the two models split on the first one (sol: READY; opus: worth flagging), and both are design/operational rather than code defects.

1 — Auto-approve is keyed on (bot author + exact title + base), but not on the merge/* head or the legitimate workflow. I confirmed the real merge PRs (#36886, #36880) are authored by github-actions[bot] via GITHUB_TOKEN, which is exactly what the policy matches. The gap: any github-actions[bot]-authored PR whose title matches ^\[automated\] Merge branch …$ and targets the branch gets auto-approved regardless of its head branch or content — the policy never checks that the head is the canonical merge/main-to-net11.0 etc. So a write-collaborator who can get the Actions token to open a look-alike PR from an arbitrary head would obtain a human-review bypass (the merge still has to pass maui-pr + device/ui + cla, so it's not unreviewed code injection — hence I'm treating it as a discussion item, not a blocker; I couldn't confirm whether a pushed workflow can actually obtain pull-requests: write here, which is the linchpin of exploitability). Cheap mitigation: add a head-branch constraint to the policy (e.g. require the head to match ^merge/(main-to-net11\.0|net11\.0-to-<release>)$) so approval is bound to the canonical snapshot branches, not just to bot-authorship + title.

2 — The whole safety model rests on external branch protection that this PR neither pins nor documents. dismiss_stale_reviews_on_push: true + ≥1 required review on all four targets is what makes the Opened-only design safe; if those rulesets are ever relaxed, the original HIGH silently returns with zero signal in this code. Worth a comment in the workflow/policy documenting that these settings are load-bearing (and ideally a note in the PR description).

Verdict

The blocking security concern is resolved and the workflow hardening is clean. Leaving this as a comment (not a block) — the two residuals above are a design/ops call for you to make; #1 in particular would be a small, worthwhile tightening.

Multi-model adversarial review (Claude Opus 4.8 + GPT‑5.6‑sol); branch-protection + PR-authorship claims independently verified against the GitHub API.

@PureWeen
PureWeen requested a review from kubaflo July 30, 2026 19:05
@PureWeen

Copy link
Copy Markdown
Member Author

@kubaflo Thanks for the Round 3 follow-up. On the two residuals:

  1. Head/source provenance: I could not find a Policy Service schema primitive for source repository or head branch in the available rule vocabulary. I am not adding an invented predicate. The supported bound here remains exact github-actions[bot] App/sender-as-author, exact anchored title, and exact base branch. The PR body explicitly records the accepted write-collaborator trust boundary: a collaborator able to run a same-repository write-capable workflow could deliberately create a matching PR, while required CI still applies.

  2. Load-bearing ruleset: This is already called out under Required ruleset prerequisite. I rechecked the live MAUI protection ruleset: dismiss_stale_reviews_on_push is still true and require_last_push_approval is still false. It must be changed to false / true before this automation is enabled or the PR is merged.

The Copilot JSONC inline is not actionable: I reran the exact PowerShell ConvertFrom-Json path against the live commented github-merge-flow-release-11.jsonc on net11.0; it parsed successfully and resolved release/11.0.1xx-preview7.

Please re-review current head be5efbbfe48953c76a2f1e1b5c0a585ed7e3f2ba and approve if these documented design decisions and prerequisite satisfy the remaining concerns.

@kubaflo kubaflo 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.

Round 4 — re‑review at be5efbb (no code change since r3; responding to your follow‑up). Thanks Shane — both residuals are now resolved on the review side. I'm not going to unilaterally approve, for one operational reason spelled out below, but there are no blocking code findings from me.

1. Head/source provenance — accepted

Agreed there's no Policy Service schema primitive for source repo / head ref, and inventing a predicate would be worse. The bound you've kept — exact github-actions[bot] App author + exact anchored title + exact base — plus the write‑collaborator trust boundary now recorded in the PR body, plus required CI still applying, is a reasonable envelope. Closing this as documented‑and‑accepted.

2. Load‑bearing ruleset — this is the whole ballgame, and it's currently NOT satisfied

This is the one thing I want to underline rather than wave through, because I independently traced why require_last_push_approval is load‑bearing here (not just that it's "recommended"):

  • dismiss_stale_reviews_on_push = true alone does not close the escalation. The failure path is: bot PR gets the auto‑approval → someone with write pushes a malicious commit onto the bot branch → dismiss_stale dismisses the approval → but the auto‑approve workflow re‑fires on the new push and re‑approves the now‑malicious head → auto‑merge proceeds. The only thing that breaks that loop is require_last_push_approval = true, which forbids the actor who produced the last push from being the sole approver — so a bot self‑approval after a push can't satisfy the gate.
  • Per your own check, the live MAUI protection ruleset is currently dismiss_stale_reviews_on_push = true (good) and require_last_push_approval = false (not yet safe).

So as configured today, enabling/merging this automation would leave the bypass loop above open. This isn't a code defect — the workflow is correct given the prerequisite — it's a deployment gate:

Do not enable/merge until require_last_push_approval is flipped to true on the target ruleset, and ideally confirm dismiss_stale_reviews_on_push stays true.

JSONC parse — non‑actionable, confirmed

Consistent with my r3 note that the commented github-merge-flow-release-11.jsonc parses fine; your rerun of the exact ConvertFrom-Json path resolving release/11.0.1xx-preview7 matches. Nothing to change.

Why COMMENT and not APPROVE

With the two residuals documented/accepted and CI clean for a workflow‑only change (license/cla pass, maui-pr correctly path‑skipped), I have no blocking code findings — from a pure code‑review standpoint this is LGTM. I'm withholding an approval only because (a) this is a review‑bypass automation whose safety is currently unmet pending the require_last_push_approval flip, and (b) merging a bypass mechanism is a human security‑boundary call, not something I'll rubber‑stamp while the load‑bearing prerequisite is false. Once the ruleset shows require_last_push_approval = true, this is good to go from my side.

@kubaflo — recommend: confirm the ruleset flip is in place, then approve. Until then this is LGTM‑conditional, not merge‑ready.

🤖 Adversarial ensemble (orchestrator re‑assessment; no new code since r3). r3 posture: HIGH review‑bypass closed, 2 non‑blocking residuals — both now resolved/accepted; the sole remaining item is the external ruleset prerequisite, which is currently unmet.

@PureWeen

Copy link
Copy Markdown
Member Author

@kubaflo Thanks — agreed this remains an external deployment gate, and I will not merge or enable it while the live ruleset is dismiss_stale_reviews_on_push: true / require_last_push_approval: false.

One correction to the escalation trace: the Policy Service rule is strictly isAction: Opened; it does not fire again on a later push/Synchronize. With dismiss_stale_reviews_on_push: true, a malicious head push would dismiss the original approval and there is no automatic reapproval path.

The documented prerequisite intentionally changes the pair to:

dismiss_stale_reviews_on_push: false
require_last_push_approval: true

That combination is needed because ordinary base movement can also change the PR diff and dismiss the one-time Opened approval. Turning stale dismissal off preserves that approval across base movement, while require_last_push_approval: true still blocks any later head push until someone other than the last pusher approves it. This is the same division GitHub documents for the two settings.

So we agree on the current outcome: code review is clean, but the live true/false state is not the required false/true state and the PR must remain unmerged until the ruleset owner makes and verifies that change.

@kubaflo kubaflo 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.

Round 5 — re‑review at be5efbb (no code change; response to your correction).

You're right — my r4 escalation trace was wrong, and I verified it

I pulled the rule at be5efbb (.github/policies/resourceManagement.yml, the [Inter-branch merge] block) and confirmed the trigger is:

- isActivitySender: { user: github-actions[bot], issueAuthor: False }
- isActivitySender: { issueAuthor: True }
- isAction: { action: Opened }

So the approvePullRequest action fires once, on Opened only — it does not re‑evaluate on Synchronize. My r4 claim that "the auto‑approve workflow re‑fires on the new push and re‑approves the malicious head" is incorrect; there is no automatic reapproval path. Thanks for the correction — withdrawing that mechanism.

Re‑grounding the actual load‑bearing requirement

The conclusion (stay unmerged until the ruleset is verified) survives, but for a sharper reason than I gave. Tracing the three configurations against a write‑collaborator head push after the one‑time approval:

dismiss_stale / require_last_push_approval Head push by collaborator Base movement Net
true / false (live now) approval dismissed, no re‑fire → blocked approval also dismissed → feature stalls safe but non‑functional
false / true (documented target) approval preserved, but most‑recent‑push needs a non‑pusher approval → blocked approval preserved → merges safe + functional
false / false (the trap) approval not dismissed and pre‑push approval still counts → auto‑merges malicious head preserved exploitable

The real gate: turning dismiss_stale_reviews_on_push off is required to make the feature survive base movement — and the moment it's off, require_last_push_approval must be on, or a head push lands in the false/false exploitable cell. So the prerequisite isn't just "flip two flags," it's "these two must move together to false/true; a partial flip to false/false is worse than today."

Spoofing the opener isn't a path — isActivitySender: github-actions[bot] can't be satisfied by a human — so the only residual is the post‑open head push, which require_last_push_approval: true closes. Agreed on all of that.

Agreement + disposition

We're aligned: code review is clean, and the PR must stay unmerged until the ruleset owner sets and verifies dismiss_stale_reviews_on_push: false / require_last_push_approval: true. I'm keeping this a COMMENT rather than approving because (a) the required maui-pr check is skipping (undetermined CI — I don't post approvals on undetermined CI), and (b) the safety prerequisite is an external ruleset change I can't verify from here. No blocking code findings from me.

@kubaflo — once the ruleset shows false/true and is verified live, this is good to approve. Until then: LGTM‑conditional, not merge‑ready.

🤖 Adversarial ensemble (orchestrator re‑assessment; no new code since r3). r4 mechanism error conceded and verified against the policy config; residual gate re‑derived (the false/false trap). Outcome unchanged.

@PureWeen

Copy link
Copy Markdown
Member Author

@kubaflo Thanks for Round 5. We revisited the proposed ruleset prerequisite against the repository-wide human-review behavior it affects. MAUI protection is intentionally restored to dismiss_stale=true, require_last_push=false, approvals 1. The proposed false/true pair cannot be scoped to bot PRs and would require a third reviewer whenever a maintainer pushes a fix, so we are withdrawing it as a prerequisite.

The security invariant remains intact: Policy Service approves only the exact bot-authored Opened event; the caller workflow refuses to invoke Arcade while the exact bot PR remains open; and any out-of-band head push dismisses approval with no automatic reapproval path.

We also verified strict_required_status_checks_policy=false. Ordinary base advancement remains mergeable when the immutable head is conflict-free, approval remains valid, and required checks pass. Only an actual reviewed-diff/merge-base change may dismiss approval and safely stall the PR; that fail-closed limitation is accepted and is now documented in the PR body.

maui-pr is expected to path-skip for this workflow/policy-only PR; GitOps schema validation and CLA are green. No code changed since be5efbb. Please re-review the corrected deployment decision and dismiss the prior changes-requested review if satisfied.

@kubaflo
kubaflo merged commit fc1536c into main Jul 31, 2026
6 of 7 checks passed
@kubaflo
kubaflo deleted the pureween-automerge-inter-branch-merge-prs branch July 31, 2026 16:41
@github-actions github-actions Bot added this to the .NET 10 SR10 milestone Jul 31, 2026
kubaflo pushed a commit that referenced this pull request Aug 1, 2026
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

### Description of Change

Remove the redundant `isActivitySender: { issueAuthor: True }` predicate
from the inter-branch merge Policy Service rule.

The end-to-end retest after #36875 merged created #36989 with the exact
expected `github-actions[bot]` author, title, head, and base, but Policy
Service did not approve it or enable auto-merge. The additional
issue-author predicate is the unique difference from the proven policy
used by `dotnet/vscode-csharp`, which successfully auto-approves and
enables merge-commit auto-merge for equivalent bot-opened inter-branch
merge PRs.

For an `Opened` event, requiring the activity sender to be
`github-actions[bot]` already binds the event to the bot that opened the
PR. The exact target branch and fully anchored title checks remain
unchanged, and `Synchronize` remains excluded.

### Verification

- The branch contains exactly one commit relative to `main`.
- `.github/policies/resourceManagement.yml` parses as YAML.
- `git diff --check` passes.
- The PR diff is exactly one file with two deletions.
- The inter-branch rule retains the exact bot sender, `Opened` action,
target branches, anchored titles, approval, and merge-method auto-merge
actions.
- Compared against the live known-good `dotnet/vscode-csharp` Policy
Service rule and PR #9595, where `dotnet-policy-service` approved and
enabled `MERGE` auto-merge for a `github-actions[bot]` inter-branch
merge PR.

### Issues Fixed

Follow-up to #36875. Replaces #36990, whose branch retained the original
squash-merged PR history.

Co-authored-by: PureWeen <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2b2dd6-bf5f-4179-8a8f-c4c85b9bce26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants