-
Notifications
You must be signed in to change notification settings - Fork 0
fix(impl-merge): use ADMIN_TOKEN PAT so --admin can bypass main ruleset #5523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,10 +178,20 @@ jobs: | |
| - name: Merge PR to main (with retry) | ||
| if: steps.check.outputs.should_run == 'true' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| # ADMIN_TOKEN: PAT with admin scope from a repo-admin user, used so | ||
| # that `gh pr merge --admin` can bypass the main-branch ruleset | ||
| # (required-status-checks). Falls back to GITHUB_TOKEN if not set so | ||
| # the workflow still runs and fails with a clear ruleset error | ||
| # instead of an opaque auth error. | ||
| GH_TOKEN: ${{ secrets.ADMIN_TOKEN || secrets.GITHUB_TOKEN }} | ||
| PR_NUM: ${{ steps.check.outputs.pr_number }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| HAS_ADMIN_TOKEN: ${{ secrets.ADMIN_TOKEN != '' }} | ||
| run: | | ||
| if [ "$HAS_ADMIN_TOKEN" != "true" ]; then | ||
| echo "::warning::ADMIN_TOKEN secret is not set — merge will fail if main ruleset enforces required status checks. Add a fine-grained PAT with Contents:Write + Pull requests:Write + Administration:Read+Write as repo secret ADMIN_TOKEN." | ||
| fi | ||
|
Comment on lines
+181
to
+193
|
||
|
|
||
| MAX_ATTEMPTS=5 | ||
|
|
||
| for attempt in $(seq 1 $MAX_ATTEMPTS); do | ||
|
|
@@ -197,6 +207,9 @@ jobs: | |
| # downstream CI workflows (Run Linting / Run Tests / Run Frontend | ||
| # Tests), so impl PRs never get those checks. The pipeline already | ||
| # gates merge behind the AI quality review threshold. | ||
| # | ||
| # Bypass only works if the token has admin role. GITHUB_TOKEN is | ||
| # only `write`, so a repo-admin PAT is required (ADMIN_TOKEN). | ||
| if gh pr merge "$PR_NUM" \ | ||
| --repo "$REPOSITORY" \ | ||
| --squash \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a repo-admin PAT here significantly increases the blast radius of this workflow: any PR that matches the current conditions (label
ai-approved+ branch nameimplementation/*) could be merged tomainwhile bypassing required checks. Consider adding an explicit trust gate before usingADMIN_TOKEN(e.g., require the PR author to be your automation bot and/or assert the PR is not cross-repo and the head repo is the same asgithub.repository), and fail fast if the gate isn’t met.