Skip to content

chore: rework backport checklist — post on merge, simplify to single workflow, harden auto-backport - #21736

Merged
mrodm merged 9 commits into
elastic:mainfrom
mrodm:update_workflows_auto_backport
Oct 1, 2026
Merged

mrodm merged 9 commits into
elastic:mainfrom
mrodm:update_workflows_auto_backport

Conversation

@mrodm

@mrodm mrodm commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Proposed commit message

Rework the two-workflow backport checklist pattern. The previous design
ran `backport-packages-detect.yml` on `pull_request` events (restricted,
fork-safe context) and used a `workflow_run` trigger in
`post-backport-checklist.yml` to post the checklist with write
permissions. This two-step indirection was introduced for fork safety but
added complexity and introduced a security concern when using artifacts
in forks with the `pull_request` event.

What changed:

  • backport-packages-detect.yml now triggers on push to main (post-merge) instead of pull_request. It owns the full pipeline: find the merged PR via the GitHub API, detect changed packages, and post/update the checklist comment. Push events always run in the base repository context, so the fork-safety concern disappears entirely — no artifact, no workflow_run indirection.

  • post-backport-checklist.yml is deleted. Its logic is absorbed into the detect workflow.

  • auto-backport.yml drops the push trigger. The checklist is now posted after merge, so there are no pre-checked branches at push time. The workflow is now purely issue_comment:edited driven. Additional hardening:

    • Job-level if: now also requires github.event.comment.user.login == 'github-actions[bot]' so only edits to the bot-posted checklist trigger the workflow.
    • cancel-in-progress set to false to prevent mid-cherry-pick cancellation when a user checks multiple boxes in quick succession. Runs are idempotent (deduplication via gh pr list), so queuing is safe.
  • The checklist comment now includes a cc @author @merger line so both the PR author and the person who merged it are notified. The merger is fetched from the full PR object (/pulls/{number}) since merged_by is absent from the list representation returned by /commits/{sha}/pulls.

  • Checklist comment intro text updated to reflect the post-merge workflow: removed the stale "updated on each push" and "on merge" wording.

Why:

Moving the trigger to push to main addresses the security concern around artifacts in forks with the pull_request event more cleanly than the workflow_run approach: it eliminates the attack surface entirely rather than patching around it. The checklist appears on the merged PR; authors check boxes after merge, which triggers auto-backport.yml via issue_comment:edited.

Author's Checklist

  • Workflows tested end-to-end: same-repo PR merge and issue_comment:edited trigger
  • YAML validated with yq for all modified workflow files
  • Checklist comment rendering verified (render-checklist unit tests pass)

How to test this PR locally

  1. Merge a PR to main on a fork — verify the detect workflow runs, finds the PR, detects packages, and posts a checklist comment with cc @author @merger.
  2. Check a box in the posted checklist comment — verify auto-backport.yml triggers and processes only that branch.
  3. Check two boxes in rapid succession — verify the second run queues rather than cancels the first, and both backport PRs are created.
  4. Merge a PR with the backport:sync-changelog label — verify no checklist is posted.
  5. Have a user without write access attempt to check a box — verify behaviour (expected: GitHub rejects the edit).

Example test runs:

Related issues


This PR was generated with the assistance of Claude (claude-sonnet-4-6).

mrodm and others added 9 commits September 28, 2026 16:09
…pattern

backport-packages-detect.yml now triggers on push to main instead of
pull_request. It owns the full pipeline: checkout (trusted main code),
build binary, find the merged PR via the GitHub API, detect packages,
post/update the checklist comment.

post-backport-checklist.yml is deleted — its logic is absorbed into the
detect workflow. The two-workflow fork-safety pattern (pull_request +
workflow_run) is no longer needed: push events always run in the base
repo context, so there is no untrusted-code execution risk to begin with.
This addresses the artifact-poisoning concern from the bug bounty report
(elastic#21660) more cleanly than the workflow_run indirection.

auto-backport.yml drops the push trigger. The checklist is now posted
after merge, so there are no pre-checked branches at push time — that
code path was a no-op. The workflow is now purely issue_comment:edited
driven. Concurrency group simplified to key on the issue number only.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Split "Find merged PR" into its own first step (pure API call, no
checkout). Checkout, Go setup, binary build, and package detection are
now all conditional on that step emitting a pr_number output.

Direct commits to main (bot pushes, etc.) that have no associated PR
exit after the single API call instead of running the full pipeline.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… the bot

Added a check on github.event.comment.user.login so the job only fires
when the comment being edited was originally created by github-actions[bot]
(i.e. it is our checklist comment). This prevents accidental triggers if
a user edits an unrelated comment that happens to contain the checklist
marker string.

Added a debug step that logs the comment ID, PR number, original comment
author, and editor at the start of every run.

Also updated the job-level if: comment to document the exact event field
each guard reads.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract pr_author (PR.user.login) and pr_merger (PR.merged_by.login)
from the GitHub API response in the find-pr step. Append a cc line to
the rendered checklist comment so both users are notified when the
checklist is posted.

When the author and merger are the same person only one mention is
appended to avoid a duplicate ping.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The /commits/{sha}/pulls endpoint returns a list-representation of PRs
that does not include merged_by. Add a second API call to
/repos/{owner}/{repo}/pulls/{number} to get the full PR object, which
does include the field.

Keep // empty as a defensive fallback for cases where merged_by is null
(e.g. PRs merged without a user context).

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
With cancel-in-progress: true, a second checkbox edit could cancel a run
mid-cherry-pick or mid-push, leaving a backport branch partially created
with no PR. Subsequent runs may then fail on that dirty branch state.

Switching to false queues runs instead. Runs are idempotent — each one
skips branches whose PR already exists via gh pr list deduplication — so
queuing is safe.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace "packages touched by this PR's current diff" with
  "packages changed by this PR" in the [!IMPORTANT] callout.
- Remove the stale "updated automatically on each push — manual edits
  will be overwritten" line; the comment is now posted once on merge.
- Replace "PRs will be created automatically on merge, or when you
  update this checklist after merge" with "A backport PR will be opened
  automatically for each branch you check."

Update the corresponding test assertion.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Command substitution strips the trailing newline from render-checklist
output, so a single \n before the cc line landed inside the blockquote.
Using \n\n produces a blank line that closes the [!TIP] callout before
the cc mentions are appended.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When merged_by is null (Dependabot merge, bot push, etc.) jq returns an
empty string for PR_MERGER. The previous if/else produced a trailing
bare @ (e.g. "cc @alice @") which is not a valid mention.

Build the MENTIONS string first, appending the merger only when it is
non-empty and differs from the author, then embed it in a single printf.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mrodm mrodm self-assigned this Sep 29, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

cc @mrodm

@mrodm
mrodm marked this pull request as ready for review September 29, 2026 16:13
@mrodm
mrodm requested a review from a team as a code owner September 29, 2026 16:14
Copilot AI balanced review requested due to automatic review settings September 29, 2026 16:14

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 review overview

🟢 Approval recommended

The consolidated workflow removes the untrusted artifact boundary while preserving package detection, authorization, and backport behavior.

Review effort: Balanced
Findings: None

What changed in this PR

Consolidates post-merge backport checklist generation into one safer workflow and hardens automatic backport execution.

Changes:

  • Posts checklists from trusted push events and removes artifact handoff.
  • Restricts auto-backports to authorized edits of bot-authored checklists.
  • Updates checklist messaging and notifications.
File Description
cmd/​backport/​backports/​checklist/​checklist.go Updates post-merge checklist guidance.
cmd/​backport/​backports/​checklist/​checklist_test.go Updates expected checklist text.
.github/​workflows/​post-backport-checklist.yml Removes the superseded consumer workflow.
.github/​workflows/​backport-packages-detect.yml Detects packages and posts checklists after merge.
.github/​workflows/​auto-backport.yml Hardens and serializes checklist-triggered backports.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mergify

mergify Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@mrodm
mrodm merged commit bd1e2a3 into elastic:main Oct 1, 2026
14 checks passed
@mrodm
mrodm deleted the update_workflows_auto_backport branch October 1, 2026 15:10
mrodm added a commit to mrodm/integrations that referenced this pull request Oct 1, 2026
The checklist comment is now posted after a PR is merged to main by
backport-packages-detect.yml (push trigger) instead of while the PR is
open. Update the developer workflow doc accordingly:

- describe the post-merge timing and the cc line for author and merger
- drop the removed post-backport-checklist.yml and "updated on each push"
  wording
- align the example comment with the current checklist template
- document who can trigger auto-backport, queued runs and de-duplication
- tell readers to tick branches after merge in the intro and step 3

Reflects elastic#21736.

Created by Claude.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.

3 participants