Skip to content

ci(release): skip CI and the approval gate on version-bump PRs#2318

Merged
rishigupta1599 merged 2 commits into
masterfrom
ci/skip-checks-on-release-prs
Jun 25, 2026
Merged

ci(release): skip CI and the approval gate on version-bump PRs#2318
rishigupta1599 merged 2 commits into
masterfrom
ci/skip-checks-on-release-prs

Conversation

@rishigupta1599

Copy link
Copy Markdown
Contributor

Problem

The Create Release PR workflow opens a release/X.Y.Z PR that only bumps version files and already posts passing check runs for every required context. But on the last real release PR (#2317) the full CI suite still got involved:

  • peter-evans/create-pull-request defaults the commit author to ${{ github.actor }} — the human who clicked Run workflow — so the commit looked user-authored (committer was still github-actions[bot]).
  • That defeated GitHub's GITHUB_TOKEN recursion guard, so the on: pull_request CI workflows were queued and parked in action_required → the unwanted "Approve and run workflows" prompt.
  • Once approved, the full ~20-min suite ran redundantly and left the PR BLOCKED until it finished.

Confirmed on #2317: attempt 1 conclusion = action_required (triggered by github-actions[bot]), attempt 2 triggered by a human clicking approve.

Fix

  1. Fully attribute the release commit to github-actions[bot] (both author and committer) in version-bump.yml, so GitHub's recursion guard suppresses the pull_request runs entirely. No approval gate; the synthetic check runs satisfy branch protection on their own.
  2. Defense-in-depth: add a !startsWith(github.head_ref, 'release/') guard to every PR-triggered CI job (test, windows, typecheck, lint, semgrep, executable-check). If a run is ever triggered/approved on a release PR anyway, the heavy jobs skip instead of running.

Resulting release flow (3 manual clicks)

  1. Run Create Release PR (pick bump type).
  2. Review + merge the version-only PR (now mergeable immediately, no approval prompt).
  3. Publish the auto-created draft GitHub Release → npm publish + executable builds run automatically.

Notes

  • draft-release.yml is intentionally untouched (it's part of the release flow, runs only on merged "Release …" PRs).
  • "Claude Code Review" / CodeQL have no in-repo workflow to guard (the review check is posted synthetically already).

🤖 Generated with Claude Code

The Create Release PR workflow opens a `release/X.Y.Z` PR that only bumps
version files and posts passing check runs for every required context, so the
full CI suite is redundant on these PRs. Two problems remained:

1. create-pull-request defaults the commit *author* to `github.actor` (the
   human who ran the workflow), so the PR commit looked user-authored. That
   defeated GitHub's GITHUB_TOKEN recursion guard, so the on:pull_request CI
   workflows were queued and parked in `action_required` — surfacing the
   unwanted "Approve and run workflows" prompt and, once approved, running the
   full ~20-min suite redundantly.

2. Even if approved, that CI is pointless for a version-only PR.

Fixes:
- Fully attribute the release commit to github-actions[bot] (author + committer)
  so the recursion guard suppresses the pull_request runs entirely — no approval
  gate; the synthetic checks satisfy branch protection on their own.
- Add a `!startsWith(github.head_ref, 'release/')` guard to every PR-triggered
  CI job (test, windows, typecheck, lint, semgrep, executable-check) as
  defense-in-depth: if a run is ever triggered/approved on a release PR, the
  heavy jobs skip instead of running.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rishigupta1599 rishigupta1599 requested a review from a team as a code owner June 24, 2026 15:32
The previous commit keyed the CI skip on `github.head_ref` (the branch name),
which any PR author controls. A contributor could open a PR from a branch named
`release/anything` and have every required check (Lint, Typecheck, Test,
Semgrep, executable verify) skip — a required-status-check bypass.

Re-gate the skip on identity that an attacker cannot forge: the PR must be
opened by github-actions[bot], from this same repository (not a fork), on a
release/* branch. Verified the skip fires only for the genuine automated release
PR; fork PRs, same-repo non-bot PRs, and normal PRs all run CI as before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rishigupta1599

Copy link
Copy Markdown
Contributor Author

Claude Code PR Review

PR: #2318Head: 3647f9fReviewers: stack:code-reviewer

Summary

Modifies seven GitHub Actions workflows so the automated Create Release PR flow (a github-actions[bot] version-bump PR) skips redundant CI and avoids the "Approve and run workflows" gate, while branch protection stays satisfied via the synthetic check runs the release workflow already posts. The CI-skip is gated on PR author identity (github-actions[bot] + same-repo + release/*), not the contributor-controllable branch name.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass Uses secrets.GITHUB_TOKEN; only the static bot identity string added.
High Security Authentication/authorization checks present Pass Skip gated on pull_request.user.login == 'github-actions[bot]' + same-repo — not forgeable by an external contributor.
High Security Input validation and sanitization Pass Branch name is no longer the security boundary; identity is. New static strings carry no injection surface.
High Security No IDOR — resource ownership validated N/A No resource access.
High Security No SQL injection (parameterized queries) N/A No DB.
High Correctness Logic is correct, handles edge cases Pass Verified across legit-release / fork-PR / same-repo-non-bot / normal-PR / push / schedule / dependabot scenarios (simulation + live fork test).
High Correctness Error handling is explicit, no swallowed exceptions N/A Declarative YAML.
High Correctness No race conditions or concurrency issues N/A No new concurrency.
Medium Testing New code has corresponding tests Pass Validated end-to-end on a fork: release PR MERGEABLE/CLEAN, 20/20 required checks green, 0 CI runs triggered.
Medium Testing Error paths and edge cases tested Pass Gate boolean simulated across 6 scenarios; attack cases all run CI.
Medium Testing Existing tests still pass (no regressions) Pass Non-release PRs unaffected — guard's first clause is false for them.
Medium Performance No N+1 queries or unbounded data fetching N/A
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass Matches the existing dependabot if: style; consistent guard across all six files.
Medium Quality Changes are focused (single concern) Pass One concern: skip CI/approval-gate on the automated release PR.
Low Quality Meaningful names, no dead code Pass
Low Quality Comments explain why, not what Pass Thorough rationale comments; one comment overstates the recursion-guard mechanism (Finding 2).
Low Quality No unnecessary dependencies added Pass No new deps.

Findings

  • File: .github/workflows/version-bump.yml:132-140 (synthetic contexts list)

  • Severity: Low (reviewer flagged High — downgraded; refuted by evidence)

  • Reviewer: stack:code-reviewer

  • Issue: Reviewer warned the synthetic context list might omit required checks (Regression, Build & verify executable, Test @percy/monitoring, Test @percy/cli-doctor), which would hang the release PR forever in "Expected".

  • Assessment: Refuted. master's required_status_checks is exactly the 20 contexts the workflow posts (Lint, Typecheck, Build, the 17 Test @percy/*, semgrep/ci, Claude Code Review). The four names cited are not required. Confirmed empirically: a fork configured with those exact 20 required contexts produced a MERGEABLE/CLEAN release PR (20/20 green, 0 CI runs). Not a blocker. The existing comment already instructs maintainers to keep the list in sync if required checks change.

  • File: .github/workflows/version-bump.yml:100-105 (comment)

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: The comment attributes CI suppression to the bot commit-author identity; the reviewer argues GitHub's recursion guard is purely GITHUB_TOKEN-based and author-independent.

  • Suggestion: Soften the comment to present the bot attribution as audit-clarity. (Caveat: observed behavior on the real broken release PR Release 1.32.3 #2317 — a GITHUB_TOKEN push with a human commit author did queue pull_request runs in action_required, while the bot-authored fork run queued none — so author attribution does appear to matter here. The mechanism is debatable; either way the fix is validated and this is a comment-only nit.)

  • File: .github/workflows/Semgrep.yml:41-45

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: The Semgrep release-gate omits the explicit github.event_name == 'pull_request' first clause that the other five files use. It's safe today (on push/schedule pull_request.user.login is empty → condition runs Semgrep), but relies on implicit empty-string behavior.

  • Suggestion: Optional — add github.event_name == 'pull_request' && to the inner negated group for parity and self-documentation. No behavior change.

  • File: all six guarded workflows (if: blocks)

  • Severity: Low (style)

  • Reviewer: stack:code-reviewer

  • Issue: ${{ }} wrapping inside a >- folded if: scalar is valid but unconventional (bare expressions are the common style for if:).

  • Suggestion: Optional cosmetic change; functionally correct as-is.

  • File: .github/workflows/version-bump.yml:139 (github-script SHA interpolation)

  • Severity: Low — out of scope (pre-existing line, not in this diff)

  • Reviewer: stack:code-reviewer

  • Issue: head_sha is interpolated into inline JS rather than passed via env:. SHA is a trusted 40-hex value, so risk is negligible.

  • Suggestion: Pre-existing; consider passing via env: in a future cleanup.

  • File: .github/workflows/test.yml / windows.yml (test, regression jobs)

  • Severity: Low (informational)

  • Reviewer: stack:code-reviewer

  • Issue: These jobs needs: [build], so they'd auto-skip when build skips even without their own if:.

  • Assessment: The explicit guards are intentional/defensive and make the skip self-documenting per-job. No change needed.

Security / forgeability: Reviewer independently confirmed the gate is sound — the three chained predicates (user.login == github-actions[bot] + same-repo + release/*) cannot all be satisfied by an untrusted contributor, so the CI-skip is not forgeable. This matches the hardening made after the automated commit-review flagged the original branch-name-only gate.


Verdict: PASS — Logic and security model are correct and validated end-to-end. The sole High finding is refuted by the repo's actual branch-protection config and a live fork test; only optional Low nits remain.

@rishigupta1599 rishigupta1599 merged commit 58b0dff into master Jun 25, 2026
47 checks passed
@rishigupta1599 rishigupta1599 deleted the ci/skip-checks-on-release-prs branch June 25, 2026 04:59
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