feat: add check-branch-protection option to push-to-pull-request-branch#28365
feat: add check-branch-protection option to push-to-pull-request-branch#28365
check-branch-protection option to push-to-pull-request-branch#28365Conversation
Add `check-branch-protection: false` option to the `push-to-pull-request-branch` safe output configuration. When set to false, the branch protection API pre-flight check is skipped, avoiding the need for `administration: read` permission. The GitHub platform still enforces branch protection at push time regardless of this setting, so disabling the pre-flight check does not reduce actual security. Changes: - Add `CheckBranchProtection *bool` field to PushToPullRequestBranchConfig - Parse `check-branch-protection` in parsePushToPullRequestBranchConfig - Wire `check_branch_protection` into the handler registry builder - Update JS handler to skip branch protection API call when false - Update JSON schema with the new property - Update documentation with new option and explanation - Add tests for explicit false and default (omitted) cases Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3b381d54-f8fa-4ebf-bb33-3089685d2ba8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3b381d54-f8fa-4ebf-bb33-3089685d2ba8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
check-branch-protection option to push-to-pull-request-branch
| // Unexpected errors (5xx, network failures, etc.) – fail closed to | ||
| // avoid bypassing branch protection due to transient API issues. | ||
| const msg = `Cannot verify branch protection rules for "${branchName}": ${getErrorMessage(protectionError)}. Push blocked to prevent accidental writes to protected branches.`; | ||
| if (checkBranchProtection) { |
There was a problem hiding this comment.
@copilot refactor branch protection code to helper file
There was a problem hiding this comment.
Done in 96ab603. Extracted the entire branch security block into checkBranchPushable(githubClient, owner, repo, branchName, checkBranchProtection) in pr_helpers.cjs. It returns null when the push is safe, or an error string to block it. Added 6 unit tests covering all branches (404, 403, 5xx, protected, default branch, and disabled check). The call site in push_to_pull_request_branch.cjs is now a single await checkBranchPushable(...) call.
…in pr_helpers.cjs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4c3a23d2-247b-4629-8a38-133eacf7905f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile |
Ran Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot when check branch protection is enabled, the job that uses this code must automatically add the "administration: read" permission for github-app based tokens |
…ection is enabled Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fe6a4ce5-4817-4bdc-8675-9e181eb1d449 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 320deca. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Adds a check-branch-protection opt-out for the push-to-pull-request-branch safe-output, and (when enabled/default) ensures GitHub App tokens include administration: read so the branch protection pre-flight API call can succeed.
Changes:
- Introduces
check-branch-protection(defaulttrue) in the Go config/parser + handler config wiring. - Refactors the JS branch safety checks into a reusable
checkBranchPushable(...)helper and makes the protection check skippable. - Updates permissions computation, schema, docs, and adds/updates tests to cover the new behavior.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/safe_outputs_permissions.go |
Adds defaulting helper and auto-adds administration: read when branch protection checks are enabled. |
pkg/workflow/safe_outputs_permissions_test.go |
Updates/extends permission expectation coverage for the new option and default behavior. |
pkg/workflow/push_to_pull_request_branch.go |
Adds CheckBranchProtection *bool to config and parses check-branch-protection from frontmatter. |
pkg/workflow/compiler_safe_outputs_handlers.go |
Emits check_branch_protection into handler config when explicitly set. |
pkg/workflow/push_to_pull_request_branch_test.go |
Verifies lock/handler config includes the flag when set to false and omits it by default. |
actions/setup/js/push_to_pull_request_branch.cjs |
Reads check_branch_protection (default true) and delegates pushability checks to helper. |
actions/setup/js/pr_helpers.cjs |
Adds checkBranchPushable(...) helper encapsulating default-branch + protection checks. |
actions/setup/js/pr_helpers.test.cjs |
Adds unit tests for checkBranchPushable(...). |
pkg/parser/schemas/main_workflow_schema.json |
Adds check-branch-protection to the workflow schema for push-to-pull-request-branch. |
docs/src/content/docs/reference/safe-outputs-pull-requests.md |
Documents the new option and its trade-offs. |
docs/src/content/docs/reference/frontmatter-full.md |
Adds the new option to the full frontmatter reference. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 1
| describe("checkBranchPushable", () => { | ||
| const { checkBranchPushable } = require("./pr_helpers.cjs"); | ||
|
|
||
| const mockCore = { info: vi.fn(), warning: vi.fn(), error: vi.fn() }; | ||
| beforeEach(() => { | ||
| global.core = mockCore; | ||
| vi.clearAllMocks(); | ||
| }); |
There was a problem hiding this comment.
The new checkBranchPushable tests mutate global.core in beforeEach but never restore the previous value. This can leak global state into other test suites and cause order-dependent failures. Capture the original global.core before overriding it and restore it in an afterEach (or delete it if it was originally undefined).
🧪 Test Quality Sentinel ReportTest Quality Score: 75/100
Test Classification DetailsView all 11 test classifications
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (149 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 What the draft ADR coversThe generated draft captures the following decision from the PR diff: Decision: Make the branch protection pre-flight check in Alternatives analyzed:
Key consequences noted:
📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24917291168
|
Without
administration: read, the branch protection pre-flight check inpush-to-pull-request-branchsilently falls through on every run (403 → warn → continue), making the check useless noise. This PR adds acheck-branch-protection: falseopt-out to skip the API call entirely—GitHub still enforces protection at push time. When the check is enabled (the default),administration: readis now automatically added to GitHub App tokens so the API call succeeds.Changes
push_to_pull_request_branch.go): newCheckBranchProtection *boolfield; parsed fromcheck-branch-protectionfrontmatter keycompiler_safe_outputs_handlers.go): wirescheck_branch_protectionintoGH_AW_SAFE_OUTPUTS_HANDLER_CONFIGviaAddBoolPtrpush_to_pull_request_branch.cjs): readsconfig.check_branch_protection !== false(defaultstrue); delegates to the newcheckBranchPushablehelper; default-branch guard still runs unconditionallypr_helpers.cjs): newcheckBranchPushable(githubClient, owner, repo, branchName, checkBranchProtection)function encapsulates the full branch security block; returnsnullwhen safe to push or an error string to block the pushsafe_outputs_permissions.go):ComputePermissionsForSafeOutputsnow automatically addsadministration: readwhencheck-branch-protectionis enabled; omitted whencheck-branch-protection: falseis set (GitHub App-only scope — no effect on standardGITHUB_TOKEN)main_workflow_schema.json): addscheck-branch-protectionboolean property to thepush-to-pull-request-branchobject schemasafe-outputs-pull-requests.md): documents the option and explains the trade-offUsage
Omitting the field keeps current behaviour (check enabled,
administration: readautomatically added to GitHub App tokens).