Skip to content

Drop unnecessary secrets: inherit and lock codegen merge to App actor - #73

Merged
ptr727 merged 1 commit into
developfrom
pr71-copilot-followups
May 11, 2026
Merged

Drop unnecessary secrets: inherit and lock codegen merge to App actor#73
ptr727 merged 1 commit into
developfrom
pr71-copilot-followups

Conversation

@ptr727

@ptr727 ptr727 commented May 11, 2026

Copy link
Copy Markdown
Owner

Summary

Two additional Copilot findings on PR #71 (develop → main release) that PR #72 was supposed to carry alongside the behaviour → behavior fix. Auto-merge on PR #72 fired before the second commit on its branch landed, so PR #72's squash captured only the spelling fix — these two land here separately.

1. secrets: inherit removed from get-version job in build-pypilibrary-task.yml

The job calls get-version-task.yml which declares no required secrets. secrets: inherit was widening the secret blast radius for no benefit. (Same pattern exists in build-nugetlibrary-task.yml, untouched here per AGENTS.md "Workflow YAML Conventions" — "existing workflows are migrated opportunistically when they're being touched for other reasons". Easy follow-up PR later.)

2. merge-codegen if: gate now requires App-actor too

The current gate checks PR author/branch/base but not the event actor. A maintainer pushing extra commits to the App's codegen branch fires a synchronize event the job would happily auto-merge — folding human changes into a release through the App PR. Restored github.actor == 'ptr727-codegen[bot]' (which the pre-consolidation merge-codegen-app job carried in PR #70-era code) alongside the existing PR author check. Comment expanded to explain why both checks matter.

+ # Both the PR author AND the event actor must be the App: the author
+ # check stops human-opened PRs that happen to target the `codegen`
+ # branch from auto-merging; the actor check stops a maintainer
+ # pushing extra commits to the App's `codegen` branch (a
+ # `synchronize` event the human triggered) from auto-merging
+ # unintended changes through the App PR.
  if: >-
    github.event.pull_request.user.login == 'ptr727-codegen[bot]' &&
+   github.actor == 'ptr727-codegen[bot]' &&
    github.event.pull_request.head.ref == 'codegen' &&
    github.event.pull_request.base.ref == 'main' &&
    github.event.pull_request.head.repo.full_name == github.repository

Test plan

Two additional Copilot findings on PR #71 that PR #72 was supposed to
carry along with the behaviour->behavior spelling fix; auto-merge on
PR #72 fired before this second commit landed, so re-applying here as
a separate PR (PR #72 was already squashed).

1. build-pypilibrary-task.yml: the `get-version` job called
   `get-version-task.yml` with `secrets: inherit`, but that task
   workflow does not declare any required secrets. Drop the inherit
   so the job runs without secrets in scope — smaller blast radius.
   (build-nugetlibrary-task.yml has the same pattern; unmodified
   here per AGENTS.md opportunistic-pin scope rule.)

2. merge-bot-pull-request.yml `merge-codegen`: the `if:` gate
   checked PR author/branch/base but not the event actor. A
   maintainer pushing extra commits to the App's `codegen` branch
   would fire a `synchronize` event that this job would happily
   auto-merge — folding human changes into a release via the App
   PR. Restore the `github.actor == 'ptr727-codegen[bot]'` check
   (which the pre-consolidation `merge-codegen-app` job carried)
   alongside the existing PR author check. Comment expanded to
   explain why both checks matter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 20:37

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

Reduces GitHub Actions secret exposure and tightens the merge-bot’s auto-merge guardrails for codegen PRs to prevent unintended human-triggered updates from being merged via the App’s PR.

Changes:

  • Removed secrets: inherit from the PyPI build workflow’s get-version reusable-workflow call (no required secrets).
  • Hardened merge-codegen auto-merge gating by requiring both PR author and triggering actor to be ptr727-codegen[bot], with expanded rationale in comments.

Reviewed changes

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

File Description
.github/workflows/merge-bot-pull-request.yml Tightens merge-codegen if: condition to require bot actor as well as bot PR author.
.github/workflows/build-pypilibrary-task.yml Drops unnecessary secrets: inherit on the get-version job to reduce secret blast radius.

@ptr727
ptr727 enabled auto-merge (squash) May 11, 2026 20:42
@ptr727
ptr727 merged commit b67737e into develop May 11, 2026
25 checks passed
@ptr727
ptr727 deleted the pr71-copilot-followups branch July 7, 2026 21:05
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