From c463b913e72b6d959ad5b129b7496d1ede816eb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 22:22:52 +0000 Subject: [PATCH 1/6] Initial plan From eaa83238c7871722646aee3c17dd89647c438413 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 22:39:24 +0000 Subject: [PATCH 2/6] feat: add check-branch-protection option to push-to-pull-request-branch 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> --- .../setup/js/push_to_pull_request_branch.cjs | 64 +++++++------ .../docs/reference/frontmatter-full.md | 7 ++ .../reference/safe-outputs-pull-requests.md | 3 + pkg/parser/schemas/main_workflow_schema.json | 93 +++++++++++++------ .../compiler_safe_outputs_handlers.go | 1 + pkg/workflow/push_to_pull_request_branch.go | 8 ++ .../push_to_pull_request_branch_test.go | 87 +++++++++++++++++ 7 files changed, 208 insertions(+), 55 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 643cb90b95a..8ecc0b26ff9 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -57,6 +57,7 @@ async function main(config = {}) { const ifNoChanges = config.if_no_changes || "warn"; const ignoreMissingBranchFailure = config.ignore_missing_branch_failure === true; const fallbackAsPullRequest = config.fallback_as_pull_request !== false; + const checkBranchProtection = config.check_branch_protection !== false; const commitTitleSuffix = config.commit_title_suffix || ""; const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; const maxCount = config.max || 0; // 0 means no limit @@ -93,6 +94,7 @@ async function main(config = {}) { core.info(`If no changes: ${ifNoChanges}`); core.info(`Ignore missing branch failure: ${ignoreMissingBranchFailure}`); core.info(`Fallback as pull request: ${fallbackAsPullRequest}`); + core.info(`Check branch protection: ${checkBranchProtection}`); if (commitTitleSuffix) { core.info(`Commit title suffix: ${commitTitleSuffix}`); } @@ -426,38 +428,42 @@ async function main(config = {}) { } // Check whether the branch has protection rules - let isBranchProtected = false; - try { - await githubClient.rest.repos.getBranchProtection({ - owner: repoParts.owner, - repo: repoParts.repo, - branch: branchName, - }); - // Successful response means branch protection rules exist - isBranchProtected = true; - } catch (protectionError) { - const protectionStatus = protectionError && typeof protectionError === "object" && "status" in protectionError ? protectionError.status : undefined; - if (protectionStatus === 404) { - // 404 means no protection rules – safe to proceed - core.info(`Branch "${branchName}" has no protection rules`); - } else if (protectionStatus === 403) { - // 403 means the token lacks permission to read branch protection rules. - // The GitHub platform will still enforce branch protection at push time, - // so warn and allow the push to proceed. - core.warning(`Could not check branch protection rules for "${branchName}" (insufficient permissions): ${getErrorMessage(protectionError)}`); - } else { - // 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) { + let isBranchProtected = false; + try { + await githubClient.rest.repos.getBranchProtection({ + owner: repoParts.owner, + repo: repoParts.repo, + branch: branchName, + }); + // Successful response means branch protection rules exist + isBranchProtected = true; + } catch (protectionError) { + const protectionStatus = protectionError && typeof protectionError === "object" && "status" in protectionError ? protectionError.status : undefined; + if (protectionStatus === 404) { + // 404 means no protection rules – safe to proceed + core.info(`Branch "${branchName}" has no protection rules`); + } else if (protectionStatus === 403) { + // 403 means the token lacks permission to read branch protection rules. + // The GitHub platform will still enforce branch protection at push time, + // so warn and allow the push to proceed. + core.warning(`Could not check branch protection rules for "${branchName}" (insufficient permissions): ${getErrorMessage(protectionError)}`); + } else { + // 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.`; + core.error(msg); + return { success: false, error: msg }; + } + } + + if (isBranchProtected) { + const msg = `Cannot push to branch "${branchName}": this branch has protection rules. Agents must not push directly to protected branches.`; core.error(msg); return { success: false, error: msg }; } - } - - if (isBranchProtected) { - const msg = `Cannot push to branch "${branchName}": this branch has protection rules. Agents must not push directly to protected branches.`; - core.error(msg); - return { success: false, error: msg }; + } else { + core.info(`Branch protection check skipped (check-branch-protection: false)`); } } diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index e3405f2e3f2..a2696ec89cd 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -4843,6 +4843,13 @@ safe-outputs: # (optional) allow-workflows: true + # When false, skips the branch protection API pre-flight check before pushing. Set + # to false to avoid requiring administration: read permission. The GitHub platform + # will still enforce branch protection at push time. Default is true (check + # enabled). + # (optional) + check-branch-protection: true + # Enable AI agents to minimize (hide) comments on issues or pull requests based on # relevance, spam detection, or moderation rules. # (optional) diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 9006f76e546..4348afccde5 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -262,6 +262,7 @@ safe-outputs: github-token-for-extra-empty-commit: ${{ secrets.CI_TOKEN }} # optional token to push empty commit triggering CI fallback-as-pull-request: true # on non-fast-forward failure, create fallback PR to original PR branch (default: true) ignore-missing-branch-failure: false # treat deleted/missing branch errors as skipped instead of failed (default: false) + check-branch-protection: true # set to false to skip the branch protection pre-flight check (default: true) protected-files: fallback-to-issue # create review issue if protected files modified ``` @@ -315,6 +316,8 @@ When `fallback-as-pull-request` is enabled (default), non-fast-forward push fail When `ignore-missing-branch-failure: true` is set, push failures caused by a deleted or missing PR branch return `skipped: true` instead of a hard failure. This is useful when the PR branch may have been deleted before the safe-output job runs (for example, on auto-merged PRs). Without this flag, a missing branch is a terminal error. +When `check-branch-protection: false` is set, the branch protection API pre-flight check is skipped. By default (`true`), the handler calls `GET /repos/{owner}/{repo}/branches/{branch}/protection` before pushing to detect whether the target branch is protected. This API call requires `administration: read`. If the token lacks that permission, the check logs a warning and continues (the GitHub platform still enforces protection at push time). Set `check-branch-protection: false` to suppress the warning and avoid the API call entirely. + ## Protected Files Both `create-pull-request` and `push-to-pull-request-branch` enforce protected file protection by default. Patches that modify package manifests, agent instruction files, or repository security configuration are refused unless you explicitly configure a policy. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index f67edfabfd4..0ed1a8ad692 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -553,7 +553,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names \u2014 any of these labels will trigger the workflow.", + "description": "Array of label names — any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -574,7 +574,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names \u2014 any of these labels will trigger the workflow.", + "description": "Array of label names — any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -1706,7 +1706,7 @@ "oneOf": [ { "type": "null", - "description": "Bare key with no value \u2014 equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." + "description": "Bare key with no value — equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." }, { "type": "boolean", @@ -1780,12 +1780,12 @@ "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." }, "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", "oneOf": [ { "type": "string", "enum": ["admin", "maintainer", "maintain", "write", "triage", "read", "all"], - "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (\u26a0\ufe0f disables permission checking entirely - use with caution)" + "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (⚠️ disables permission checking entirely - use with caution)" }, { "type": "array", @@ -1859,7 +1859,22 @@ ], "default": "eyes", "description": "AI reaction to add/remove on triggering item. Scalar form accepts one of: +1, -1, laugh, confused, heart, hooray, rocket, eyes, none. Object form implies enabled reactions and supports optional `issues`, `pull-requests`, and `discussions` fields to control trigger groups independently; use `type` to choose the reaction emoji (defaults to `eyes` when omitted). Use 'none' to disable reactions.", - "examples": ["eyes", "rocket", "+1", 1, -1, "none", { "type": "rocket", "pull-requests": false }, { "issues": false, "discussions": true }] + "examples": [ + "eyes", + "rocket", + "+1", + 1, + -1, + "none", + { + "type": "rocket", + "pull-requests": false + }, + { + "issues": false, + "discussions": true + } + ] }, "status-comment": { "oneOf": [ @@ -1886,7 +1901,24 @@ } ], "description": "Whether to post status comments (started/completed) on the triggering item. Boolean form enables/disables status comments globally. Object form implies enabled status comments and supports optional `issues`, `pull-requests`, and `discussions` fields to control trigger groups independently. Automatically enabled for slash_command and label_command triggers when not explicitly configured.", - "examples": [true, false, { "issues": false }, { "pull-requests": false }, { "discussions": false }, { "issues": false, "pull-requests": true, "discussions": true }] + "examples": [ + true, + false, + { + "issues": false + }, + { + "pull-requests": false + }, + { + "discussions": false + }, + { + "issues": false, + "pull-requests": true, + "discussions": true + } + ] }, "github-token": { "type": "string", @@ -2881,7 +2913,7 @@ }, "network": { "$comment": "Strict mode requirements: When strict=true, the 'network' field must be present (not null/undefined) and cannot contain standalone wildcard '*' in allowed domains (but patterns like '*.example.com' ARE allowed). This is validated in Go code (pkg/workflow/strict_mode_validation.go) via validateStrictNetwork().", - "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' \u2014 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", + "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' — 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", "examples": [ "defaults", { @@ -3363,7 +3395,7 @@ [ { "name": "Verify Post-Steps Execution", - "run": "echo \"\u2705 Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" + "run": "echo \"✅ Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" }, { "name": "Upload Test Results", @@ -5887,7 +5919,9 @@ }, "exclude": { "type": "array", - "items": { "type": "string" }, + "items": { + "type": "string" + }, "description": "List of filenames or path prefixes to remove from the default protected-file set. Items are matched by basename (e.g. \"AGENTS.md\") or path prefix (e.g. \".agents/\"). Use this to allow the agent to modify specific files that are otherwise blocked by default.", "examples": [["AGENTS.md"], ["AGENTS.md", ".agents/"]] } @@ -5903,7 +5937,7 @@ "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "preserve-branch-name": { "type": "boolean", @@ -6163,7 +6197,7 @@ "oneOf": [ { "type": "object", - "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only \u2014 threads on other PRs cannot be resolved.", + "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only — threads on other PRs cannot be resolved.", "properties": { "max": { "description": "Maximum number of review threads to resolve (default: 10) Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", @@ -7177,7 +7211,9 @@ }, "exclude": { "type": "array", - "items": { "type": "string" }, + "items": { + "type": "string" + }, "description": "List of filenames or path prefixes to remove from the default protected-file set. Items are matched by basename (e.g. \"AGENTS.md\") or path prefix (e.g. \".agents/\"). Use this to allow the agent to modify specific files that are otherwise blocked by default.", "examples": [["AGENTS.md"], ["AGENTS.md", ".agents/"]] } @@ -7193,7 +7229,7 @@ "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "excluded-files": { "type": "array", @@ -7213,6 +7249,11 @@ "description": "When true, adds workflows: write to the GitHub App token permissions. Required when allowed-files targets .github/workflows/ paths. Requires safe-outputs.github-app to be configured because the workflows permission is a GitHub App-only permission and cannot be granted via GITHUB_TOKEN.", "default": false, "examples": [true, false] + }, + "check-branch-protection": { + "type": "boolean", + "description": "When false, skips the branch protection API pre-flight check before pushing. Set to false to avoid requiring administration: read permission. The GitHub platform will still enforce branch protection at push time. Default is true (check enabled).", + "default": true } }, "additionalProperties": false @@ -8188,7 +8229,7 @@ }, "scripts": { "type": "object", - "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function \u2014 the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", + "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", "patternProperties": { "^[a-zA-Z_][a-zA-Z0-9_-]*$": { "type": "object", @@ -8246,7 +8287,7 @@ }, "script": { "type": "string", - "description": "JavaScript handler body. Write only the code that runs inside the handler for each item \u2014 the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." + "description": "JavaScript handler body. Write only the code that runs inside the handler for each item — the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." } }, "required": ["script"], @@ -8281,8 +8322,8 @@ }, "staged-title": { "type": "string", - "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '\ud83c\udfad Preview: {operation}'", - "examples": ["\ud83c\udfad Preview: {operation}", "## Staged Mode: {operation}"] + "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '🎭 Preview: {operation}'", + "examples": ["🎭 Preview: {operation}", "## Staged Mode: {operation}"] }, "staged-description": { "type": "string", @@ -8296,18 +8337,18 @@ }, "run-success": { "type": "string", - "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.'", - "examples": ["\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.", "\u2705 [{workflow_name}]({run_url}) finished."] + "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '✅ Agentic [{workflow_name}]({run_url}) completed successfully.'", + "examples": ["✅ Agentic [{workflow_name}]({run_url}) completed successfully.", "✅ [{workflow_name}]({run_url}) finished."] }, "run-failure": { "type": "string", - "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", - "examples": ["\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "\u274c [{workflow_name}]({run_url}) {status}."] + "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", + "examples": ["❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "❌ [{workflow_name}]({run_url}) {status}."] }, "detection-failure": { "type": "string", - "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", - "examples": ["\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "\u26a0\ufe0f Detection job failed in [{workflow_name}]({run_url})."] + "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", + "examples": ["⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "⚠️ Detection job failed in [{workflow_name}]({run_url})."] }, "agent-failure-issue": { "type": "string", @@ -9068,7 +9109,7 @@ }, "github-token": { "type": "string", - "description": "GitHub token expression to authenticate APM with private package repositories. Uses cascading fallback (GH_AW_PLUGINS_TOKEN \u2192 GH_AW_GITHUB_TOKEN \u2192 GITHUB_TOKEN) when not specified. Takes effect unless github-app is also configured (which takes precedence).", + "description": "GitHub token expression to authenticate APM with private package repositories. Uses cascading fallback (GH_AW_PLUGINS_TOKEN → GH_AW_GITHUB_TOKEN → GITHUB_TOKEN) when not specified. Takes effect unless github-app is also configured (which takes precedence).", "examples": ["${{ secrets.MY_TOKEN }}", "${{ secrets.GH_AW_GITHUB_TOKEN }}"] } }, @@ -9815,7 +9856,7 @@ }, "auth": { "type": "array", - "description": "Authentication bindings \u2014 maps logical roles (e.g. 'api-key') to GitHub Actions secret names", + "description": "Authentication bindings — maps logical roles (e.g. 'api-key') to GitHub Actions secret names", "items": { "type": "object", "properties": { diff --git a/pkg/workflow/compiler_safe_outputs_handlers.go b/pkg/workflow/compiler_safe_outputs_handlers.go index d8b45725950..0991a98efa4 100644 --- a/pkg/workflow/compiler_safe_outputs_handlers.go +++ b/pkg/workflow/compiler_safe_outputs_handlers.go @@ -444,6 +444,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("excluded_files", c.ExcludedFiles). AddIfNotEmpty("patch_format", c.PatchFormat). AddBoolPtr("fallback_as_pull_request", c.FallbackAsPullRequest). + AddBoolPtr("check_branch_protection", c.CheckBranchProtection). Build() }, "update_pull_request": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 4530a332a20..5003973f1ea 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -28,6 +28,7 @@ type PushToPullRequestBranchConfig struct { PatchFormat string `yaml:"patch-format,omitempty"` // Transport format for packaging changes: "am" (default, uses git format-patch) or "bundle" (uses git bundle, preserves merge topology and per-commit metadata). FallbackAsPullRequest *bool `yaml:"fallback-as-pull-request,omitempty"` // When true (default), creates a fallback pull request if direct push fails due to diverged/non-fast-forward branch. When false, fallback is disabled and pull-requests: write is not requested. AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured. + CheckBranchProtection *bool `yaml:"check-branch-protection,omitempty"` // When false, skips the branch protection API pre-flight check. Default is true (check enabled). Set to false to avoid needing administration: read permission. } // buildCheckoutRepository generates a checkout step with optional target repository and custom token @@ -188,6 +189,13 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } } + // Parse check-branch-protection: when false, skips the branch protection API pre-flight check + if checkBranchProtection, exists := configMap["check-branch-protection"]; exists { + if checkBranchProtectionBool, ok := checkBranchProtection.(bool); ok { + pushToBranchConfig.CheckBranchProtection = &checkBranchProtectionBool + } + } + // Parse common base fields with default max of 0 (no limit) c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0) } diff --git a/pkg/workflow/push_to_pull_request_branch_test.go b/pkg/workflow/push_to_pull_request_branch_test.go index 998c006ac11..3b0a8e6c2d3 100644 --- a/pkg/workflow/push_to_pull_request_branch_test.go +++ b/pkg/workflow/push_to_pull_request_branch_test.go @@ -1094,3 +1094,90 @@ This test verifies that the aw-*.patch artifact is downloaded in the safe_output t.Errorf("Expected condition to check for 'push_to_pull_request_branch' in output_types") } } + +func TestPushToPullRequestBranchCheckBranchProtectionFalse(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + + testMarkdown := `--- +on: + pull_request: + types: [opened, synchronize] +safe-outputs: + push-to-pull-request-branch: + check-branch-protection: false +--- + +# Test Push to Branch with check-branch-protection: false + +This workflow skips the branch protection pre-flight check. +` + + mdFile := filepath.Join(tmpDir, "test-push-to-pull-request-branch-no-protection-check.md") + if err := os.WriteFile(mdFile, []byte(testMarkdown), 0644); err != nil { + t.Fatalf("Failed to write test markdown file: %v", err) + } + + compiler := NewCompiler() + + if err := compiler.CompileWorkflow(mdFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockFile := stringutil.MarkdownToLockFile(mdFile) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // Verify check_branch_protection: false is passed to the handler config + pushCfg := extractPushToPullRequestBranchHandlerConfig(t, lockContent) + + checkBranchProtection, exists := pushCfg["check_branch_protection"] + if !exists { + t.Fatalf("Handler config should contain check_branch_protection key when set to false") + } + if checkBranchProtection != false { + t.Errorf("check_branch_protection should be false, got %v", checkBranchProtection) + } +} + +func TestPushToPullRequestBranchCheckBranchProtectionDefaultOmitted(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + + testMarkdown := `--- +on: + pull_request: + types: [opened, synchronize] +safe-outputs: + push-to-pull-request-branch: +--- + +# Test Push to Branch default check-branch-protection + +By default, check-branch-protection is not set (JS handler defaults to true). +` + + mdFile := filepath.Join(tmpDir, "test-push-to-pull-request-branch-default-protection.md") + if err := os.WriteFile(mdFile, []byte(testMarkdown), 0644); err != nil { + t.Fatalf("Failed to write test markdown file: %v", err) + } + + compiler := NewCompiler() + + if err := compiler.CompileWorkflow(mdFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockFile := stringutil.MarkdownToLockFile(mdFile) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + // When not specified, check_branch_protection should not be in the handler config + // (the JS handler defaults to true via !== false) + pushCfg := extractPushToPullRequestBranchHandlerConfig(t, lockContent) + if _, exists := pushCfg["check_branch_protection"]; exists { + t.Errorf("Handler config should NOT contain check_branch_protection when not explicitly set") + } +} From 952537e2ef0615eceff30f0099ae76f74b903ccb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 22:43:13 +0000 Subject: [PATCH 3/6] fix: use targeted schema edit to avoid unrelated Unicode normalization 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> --- pkg/parser/schemas/main_workflow_schema.json | 88 ++++++-------------- 1 file changed, 26 insertions(+), 62 deletions(-) diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 0ed1a8ad692..d62a38fba97 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -553,7 +553,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names — any of these labels will trigger the workflow.", + "description": "Array of label names \u2014 any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -574,7 +574,7 @@ { "type": "array", "minItems": 1, - "description": "Array of label names — any of these labels will trigger the workflow.", + "description": "Array of label names \u2014 any of these labels will trigger the workflow.", "items": { "type": "string", "minLength": 1, @@ -1706,7 +1706,7 @@ "oneOf": [ { "type": "null", - "description": "Bare key with no value — equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." + "description": "Bare key with no value \u2014 equivalent to true. Skips workflow execution if any CI checks on the target branch are currently failing." }, { "type": "boolean", @@ -1780,12 +1780,12 @@ "description": "Skip workflow execution for specific GitHub users. Useful for preventing workflows from running for specific accounts (e.g., bots, specific team members)." }, "roles": { - "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (⚠️ security consideration).", + "description": "Repository access roles required to trigger agentic workflows. Defaults to ['admin', 'maintainer', 'write'] for security. Use 'all' to allow any authenticated user (\u26a0\ufe0f security consideration).", "oneOf": [ { "type": "string", "enum": ["admin", "maintainer", "maintain", "write", "triage", "read", "all"], - "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (⚠️ disables permission checking entirely - use with caution)" + "description": "Single repository permission level that can trigger the workflow. Use 'all' to allow any authenticated user (\u26a0\ufe0f disables permission checking entirely - use with caution)" }, { "type": "array", @@ -1859,22 +1859,7 @@ ], "default": "eyes", "description": "AI reaction to add/remove on triggering item. Scalar form accepts one of: +1, -1, laugh, confused, heart, hooray, rocket, eyes, none. Object form implies enabled reactions and supports optional `issues`, `pull-requests`, and `discussions` fields to control trigger groups independently; use `type` to choose the reaction emoji (defaults to `eyes` when omitted). Use 'none' to disable reactions.", - "examples": [ - "eyes", - "rocket", - "+1", - 1, - -1, - "none", - { - "type": "rocket", - "pull-requests": false - }, - { - "issues": false, - "discussions": true - } - ] + "examples": ["eyes", "rocket", "+1", 1, -1, "none", { "type": "rocket", "pull-requests": false }, { "issues": false, "discussions": true }] }, "status-comment": { "oneOf": [ @@ -1901,24 +1886,7 @@ } ], "description": "Whether to post status comments (started/completed) on the triggering item. Boolean form enables/disables status comments globally. Object form implies enabled status comments and supports optional `issues`, `pull-requests`, and `discussions` fields to control trigger groups independently. Automatically enabled for slash_command and label_command triggers when not explicitly configured.", - "examples": [ - true, - false, - { - "issues": false - }, - { - "pull-requests": false - }, - { - "discussions": false - }, - { - "issues": false, - "pull-requests": true, - "discussions": true - } - ] + "examples": [true, false, { "issues": false }, { "pull-requests": false }, { "discussions": false }, { "issues": false, "pull-requests": true, "discussions": true }] }, "github-token": { "type": "string", @@ -2913,7 +2881,7 @@ }, "network": { "$comment": "Strict mode requirements: When strict=true, the 'network' field must be present (not null/undefined) and cannot contain standalone wildcard '*' in allowed domains (but patterns like '*.example.com' ARE allowed). This is validated in Go code (pkg/workflow/strict_mode_validation.go) via validateStrictNetwork().", - "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' — 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", + "description": "Network access control for AI engines using ecosystem identifiers and domain allowlists. Supports wildcard patterns like '*.example.com' to match any subdomain. Controls web fetch and search capabilities. IMPORTANT: For workflows that build/install/test code, always include the language ecosystem identifier alongside 'defaults' \u2014 'defaults' alone only covers basic infrastructure, not package registries. Key ecosystem identifiers by runtime: 'dotnet' (.NET/NuGet), 'python' (pip/PyPI), 'node' (npm/yarn), 'go' (go modules), 'java' (Maven/Gradle), 'ruby' (Bundler), 'rust' (Cargo), 'swift' (Swift PM). Example: a .NET project needs network: { allowed: [defaults, dotnet] }.", "examples": [ "defaults", { @@ -3395,7 +3363,7 @@ [ { "name": "Verify Post-Steps Execution", - "run": "echo \"✅ Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" + "run": "echo \"\u2705 Post-steps are executing correctly\"\necho \"This step runs after the AI agent completes\"\n" }, { "name": "Upload Test Results", @@ -5919,9 +5887,7 @@ }, "exclude": { "type": "array", - "items": { - "type": "string" - }, + "items": { "type": "string" }, "description": "List of filenames or path prefixes to remove from the default protected-file set. Items are matched by basename (e.g. \"AGENTS.md\") or path prefix (e.g. \".agents/\"). Use this to allow the agent to modify specific files that are otherwise blocked by default.", "examples": [["AGENTS.md"], ["AGENTS.md", ".agents/"]] } @@ -5937,7 +5903,7 @@ "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "preserve-branch-name": { "type": "boolean", @@ -6197,7 +6163,7 @@ "oneOf": [ { "type": "object", - "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only — threads on other PRs cannot be resolved.", + "description": "Configuration for resolving review threads on pull requests. Resolution is scoped to the triggering PR only \u2014 threads on other PRs cannot be resolved.", "properties": { "max": { "description": "Maximum number of review threads to resolve (default: 10) Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", @@ -7211,9 +7177,7 @@ }, "exclude": { "type": "array", - "items": { - "type": "string" - }, + "items": { "type": "string" }, "description": "List of filenames or path prefixes to remove from the default protected-file set. Items are matched by basename (e.g. \"AGENTS.md\") or path prefix (e.g. \".agents/\"). Use this to allow the agent to modify specific files that are otherwise blocked by default.", "examples": [["AGENTS.md"], ["AGENTS.md", ".agents/"]] } @@ -7229,7 +7193,7 @@ "items": { "type": "string" }, - "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern — files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." + "description": "Exclusive allowlist of glob patterns. When set, every file in the patch must match at least one pattern \u2014 files outside the list are always refused, including normal source files. This is a restriction, not an exception: setting allowed-files: [\".github/workflows/*\"] blocks all other files. To allow multiple sets of files, list all patterns explicitly. Acts independently of the protected-files policy; both checks must pass. To modify a protected file, it must both match allowed-files and be permitted by protected-files (e.g. protected-files: allowed). Supports * (any characters except /) and ** (any characters including /)." }, "excluded-files": { "type": "array", @@ -8229,7 +8193,7 @@ }, "scripts": { "type": "object", - "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function — the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", + "description": "Inline JavaScript script handlers that run inside the consolidated safe-outputs job handler loop. Unlike 'jobs' (which create separate GitHub Actions jobs), scripts execute in-process alongside the built-in handlers. Users write only the body of the main function \u2014 the compiler wraps it with 'async function main(config = {}) { ... }' and 'module.exports = { main };' automatically. Script names containing dashes will be automatically normalized to underscores (e.g., 'post-slack-message' becomes 'post_slack_message').", "patternProperties": { "^[a-zA-Z_][a-zA-Z0-9_-]*$": { "type": "object", @@ -8287,7 +8251,7 @@ }, "script": { "type": "string", - "description": "JavaScript handler body. Write only the code that runs inside the handler for each item — the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." + "description": "JavaScript handler body. Write only the code that runs inside the handler for each item \u2014 the compiler generates the full outer wrapper including config input destructuring (`const { channel, message } = config;`) and the handler function (`return async function handleX(item, resolvedTemporaryIds) { ... }`). The body has access to `item` (runtime message with input values), `resolvedTemporaryIds` (map of temporary IDs), and config-destructured local variables for each declared input." } }, "required": ["script"], @@ -8322,8 +8286,8 @@ }, "staged-title": { "type": "string", - "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '🎭 Preview: {operation}'", - "examples": ["🎭 Preview: {operation}", "## Staged Mode: {operation}"] + "description": "Custom title template for staged mode preview. Available placeholders: {operation}. Example: '\ud83c\udfad Preview: {operation}'", + "examples": ["\ud83c\udfad Preview: {operation}", "## Staged Mode: {operation}"] }, "staged-description": { "type": "string", @@ -8337,18 +8301,18 @@ }, "run-success": { "type": "string", - "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '✅ Agentic [{workflow_name}]({run_url}) completed successfully.'", - "examples": ["✅ Agentic [{workflow_name}]({run_url}) completed successfully.", "✅ [{workflow_name}]({run_url}) finished."] + "description": "Custom message template for successful workflow completion. Available placeholders: {workflow_name}, {run_url}. Default: '\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.'", + "examples": ["\u2705 Agentic [{workflow_name}]({run_url}) completed successfully.", "\u2705 [{workflow_name}]({run_url}) finished."] }, "run-failure": { "type": "string", - "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", - "examples": ["❌ Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "❌ [{workflow_name}]({run_url}) {status}."] + "description": "Custom message template for failed workflow. Available placeholders: {workflow_name}, {run_url}, {status}. Default: '\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.'", + "examples": ["\u274c Agentic [{workflow_name}]({run_url}) {status} and wasn't able to produce a result.", "\u274c [{workflow_name}]({run_url}) {status}."] }, "detection-failure": { "type": "string", - "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", - "examples": ["⚠️ Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "⚠️ Detection job failed in [{workflow_name}]({run_url})."] + "description": "Custom message template for detection job failure. Available placeholders: {workflow_name}, {run_url}. Default: '\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.'", + "examples": ["\u26a0\ufe0f Security scanning failed for [{workflow_name}]({run_url}). Review the logs for details.", "\u26a0\ufe0f Detection job failed in [{workflow_name}]({run_url})."] }, "agent-failure-issue": { "type": "string", @@ -9109,7 +9073,7 @@ }, "github-token": { "type": "string", - "description": "GitHub token expression to authenticate APM with private package repositories. Uses cascading fallback (GH_AW_PLUGINS_TOKEN → GH_AW_GITHUB_TOKEN → GITHUB_TOKEN) when not specified. Takes effect unless github-app is also configured (which takes precedence).", + "description": "GitHub token expression to authenticate APM with private package repositories. Uses cascading fallback (GH_AW_PLUGINS_TOKEN \u2192 GH_AW_GITHUB_TOKEN \u2192 GITHUB_TOKEN) when not specified. Takes effect unless github-app is also configured (which takes precedence).", "examples": ["${{ secrets.MY_TOKEN }}", "${{ secrets.GH_AW_GITHUB_TOKEN }}"] } }, @@ -9856,7 +9820,7 @@ }, "auth": { "type": "array", - "description": "Authentication bindings — maps logical roles (e.g. 'api-key') to GitHub Actions secret names", + "description": "Authentication bindings \u2014 maps logical roles (e.g. 'api-key') to GitHub Actions secret names", "items": { "type": "object", "properties": { From 96ab603eec1eba10f2e453483ab9c6c76ca1cd5a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 23:03:11 +0000 Subject: [PATCH 4/6] refactor: move branch protection check to checkBranchPushable helper 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> --- actions/setup/js/pr_helpers.cjs | 70 ++++++++++++++++++- actions/setup/js/pr_helpers.test.cjs | 57 +++++++++++++++ .../setup/js/push_to_pull_request_branch.cjs | 61 ++-------------- 3 files changed, 131 insertions(+), 57 deletions(-) diff --git a/actions/setup/js/pr_helpers.cjs b/actions/setup/js/pr_helpers.cjs index 53a772352f0..414c0bf92cf 100644 --- a/actions/setup/js/pr_helpers.cjs +++ b/actions/setup/js/pr_helpers.cjs @@ -1,5 +1,7 @@ // @ts-check +const { getErrorMessage } = require("./error_helpers.cjs"); + /** * Detect if a pull request is from a fork repository. * @@ -110,4 +112,70 @@ function buildBranchInstruction(effectiveBaseBranch, resolvedDefaultBranch) { return `IMPORTANT: Create your branch from the '${effectiveBaseBranch}' branch${notClause}.`; } -module.exports = { detectForkPR, getPullRequestNumber, resolvePullRequestRepo, buildBranchInstruction }; +/** + * Check whether a branch is safe to push to. + * + * Performs two security checks: + * 1. Ensures the branch is not the repository's default branch. + * 2. When `checkBranchProtection` is true, queries the branch protection API to + * verify the branch has no protection rules. + * + * Returns null when the push is safe to proceed, or a string error message that + * should be surfaced as a hard failure when the push must be blocked. + * + * @param {any} githubClient - Octokit REST client + * @param {string} owner - Repository owner + * @param {string} repo - Repository name + * @param {string} branchName - Target branch to validate + * @param {boolean} checkBranchProtection - Whether to call the branch protection API + * @returns {Promise} Error message if push is blocked, null if safe + */ +async function checkBranchPushable(githubClient, owner, repo, branchName, checkBranchProtection) { + // Check whether the branch is the repository default branch + let defaultBranch = null; + try { + const { data: repoData } = await githubClient.rest.repos.get({ owner, repo }); + defaultBranch = repoData.default_branch; + } catch (repoError) { + core.warning(`Could not check repository default branch: ${getErrorMessage(repoError)}`); + } + + if (defaultBranch && branchName === defaultBranch) { + return `Cannot push to branch "${branchName}": this is the repository's default branch. Agents must not push directly to the default branch.`; + } + + // Check whether the branch has protection rules + if (checkBranchProtection) { + let isBranchProtected = false; + try { + await githubClient.rest.repos.getBranchProtection({ owner, repo, branch: branchName }); + // Successful response means branch protection rules exist + isBranchProtected = true; + } catch (protectionError) { + const protectionStatus = protectionError && typeof protectionError === "object" && "status" in protectionError ? protectionError.status : undefined; + if (protectionStatus === 404) { + // 404 means no protection rules – safe to proceed + core.info(`Branch "${branchName}" has no protection rules`); + } else if (protectionStatus === 403) { + // 403 means the token lacks permission to read branch protection rules. + // The GitHub platform will still enforce branch protection at push time, + // so warn and allow the push to proceed. + core.warning(`Could not check branch protection rules for "${branchName}" (insufficient permissions): ${getErrorMessage(protectionError)}`); + } else { + // Unexpected errors (5xx, network failures, etc.) – fail closed to + // avoid bypassing branch protection due to transient API issues. + return `Cannot verify branch protection rules for "${branchName}": ${getErrorMessage(protectionError)}. Push blocked to prevent accidental writes to protected branches.`; + } + } + + if (isBranchProtected) { + return `Cannot push to branch "${branchName}": this branch has protection rules. Agents must not push directly to protected branches.`; + } + } else { + core.info(`Branch protection check skipped (check-branch-protection: false)`); + } + + return null; +} + +module.exports = { detectForkPR, getPullRequestNumber, resolvePullRequestRepo, buildBranchInstruction, checkBranchPushable }; diff --git a/actions/setup/js/pr_helpers.test.cjs b/actions/setup/js/pr_helpers.test.cjs index 352958c4a72..74c080184dc 100644 --- a/actions/setup/js/pr_helpers.test.cjs +++ b/actions/setup/js/pr_helpers.test.cjs @@ -351,3 +351,60 @@ describe("buildBranchInstruction", () => { expect(instruction).not.toContain("NOT from"); }); }); + +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(); + }); + + const makeClient = (defaultBranch, protectionStatus) => ({ + rest: { + repos: { + get: vi.fn().mockResolvedValue({ data: { default_branch: defaultBranch } }), + getBranchProtection: protectionStatus === null ? vi.fn().mockResolvedValue({}) : vi.fn().mockRejectedValue(Object.assign(new Error("error"), { status: protectionStatus })), + }, + }, + }); + + it("returns null when branch is not default and has no protection rules (404)", async () => { + const client = makeClient("main", 404); + const result = await checkBranchPushable(client, "owner", "repo", "feature", true); + expect(result).toBeNull(); + }); + + it("blocks push when branch is the default branch", async () => { + const client = makeClient("main", 404); + const result = await checkBranchPushable(client, "owner", "repo", "main", true); + expect(result).toContain("default branch"); + }); + + it("blocks push when branch has protection rules", async () => { + const client = makeClient("main", null); // null status = successful getBranchProtection response + const result = await checkBranchPushable(client, "owner", "repo", "feature", true); + expect(result).toContain("protection rules"); + }); + + it("returns null and skips protection check when checkBranchProtection is false", async () => { + const client = makeClient("main", null); + const result = await checkBranchPushable(client, "owner", "repo", "feature", false); + expect(result).toBeNull(); + expect(client.rest.repos.getBranchProtection).not.toHaveBeenCalled(); + }); + + it("returns error on unexpected protection check failure (5xx)", async () => { + const client = makeClient("main", 500); + const result = await checkBranchPushable(client, "owner", "repo", "feature", true); + expect(result).toContain("Cannot verify branch protection rules"); + }); + + it("returns null and warns on 403 (insufficient permissions)", async () => { + const client = makeClient("main", 403); + const result = await checkBranchPushable(client, "owner", "repo", "feature", true); + expect(result).toBeNull(); + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("insufficient permissions")); + }); +}); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 8ecc0b26ff9..41f19229181 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -10,7 +10,7 @@ const { updateActivationCommentWithCommit, updateActivationComment } = require(" const { getErrorMessage } = require("./error_helpers.cjs"); const { normalizeBranchName } = require("./normalize_branch_name.cjs"); const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); -const { detectForkPR } = require("./pr_helpers.cjs"); +const { detectForkPR, checkBranchPushable } = require("./pr_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { checkFileProtection } = require("./manifest_file_helpers.cjs"); @@ -409,61 +409,10 @@ async function main(config = {}) { // This prevents agents from pushing directly to branches that should only receive // changes through reviewed pull requests. { - // Check whether the branch is the repository default branch - let defaultBranch = null; - try { - const { data: repoData } = await githubClient.rest.repos.get({ - owner: repoParts.owner, - repo: repoParts.repo, - }); - defaultBranch = repoData.default_branch; - } catch (repoError) { - core.warning(`Could not check repository default branch: ${getErrorMessage(repoError)}`); - } - - if (defaultBranch && branchName === defaultBranch) { - const msg = `Cannot push to branch "${branchName}": this is the repository's default branch. Agents must not push directly to the default branch.`; - core.error(msg); - return { success: false, error: msg }; - } - - // Check whether the branch has protection rules - if (checkBranchProtection) { - let isBranchProtected = false; - try { - await githubClient.rest.repos.getBranchProtection({ - owner: repoParts.owner, - repo: repoParts.repo, - branch: branchName, - }); - // Successful response means branch protection rules exist - isBranchProtected = true; - } catch (protectionError) { - const protectionStatus = protectionError && typeof protectionError === "object" && "status" in protectionError ? protectionError.status : undefined; - if (protectionStatus === 404) { - // 404 means no protection rules – safe to proceed - core.info(`Branch "${branchName}" has no protection rules`); - } else if (protectionStatus === 403) { - // 403 means the token lacks permission to read branch protection rules. - // The GitHub platform will still enforce branch protection at push time, - // so warn and allow the push to proceed. - core.warning(`Could not check branch protection rules for "${branchName}" (insufficient permissions): ${getErrorMessage(protectionError)}`); - } else { - // 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.`; - core.error(msg); - return { success: false, error: msg }; - } - } - - if (isBranchProtected) { - const msg = `Cannot push to branch "${branchName}": this branch has protection rules. Agents must not push directly to protected branches.`; - core.error(msg); - return { success: false, error: msg }; - } - } else { - core.info(`Branch protection check skipped (check-branch-protection: false)`); + const blockReason = await checkBranchPushable(githubClient, repoParts.owner, repoParts.repo, branchName, checkBranchProtection); + if (blockReason) { + core.error(blockReason); + return { success: false, error: blockReason }; } } From 320decaaf86905ac3bd63e30b2416552c9f2497d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Apr 2026 23:48:18 +0000 Subject: [PATCH 5/6] feat: auto-add administration: read permission when check-branch-protection 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> --- pkg/workflow/safe_outputs_permissions.go | 15 ++++++++ pkg/workflow/safe_outputs_permissions_test.go | 37 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/safe_outputs_permissions.go b/pkg/workflow/safe_outputs_permissions.go index 89ea00a0447..6e75492f492 100644 --- a/pkg/workflow/safe_outputs_permissions.go +++ b/pkg/workflow/safe_outputs_permissions.go @@ -59,6 +59,14 @@ func getPushFallbackAsPullRequest(config *PushToPullRequestBranchConfig) bool { return *config.FallbackAsPullRequest } +// getCheckBranchProtection returns the effective check-branch-protection setting (defaults to true). +func getCheckBranchProtection(config *PushToPullRequestBranchConfig) bool { + if config == nil || config.CheckBranchProtection == nil { + return true // Default: check is enabled + } + return *config.CheckBranchProtection +} + // ComputePermissionsForSafeOutputs computes the minimal required permissions // based on the configured safe-outputs. This function is used by both the // consolidated safe outputs job and the conclusion job to ensure they only @@ -161,6 +169,13 @@ func ComputePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio safeOutputsPermissionsLog.Print("Adding workflows: write for push-to-pull-request-branch (allow-workflows: true)") permissions.Set(PermissionWorkflows, PermissionWrite) } + // Add administration: read when check-branch-protection is enabled (GitHub App-only permission) + // The branch protection API requires administration: read for GitHub App tokens. + // Standard GITHUB_TOKEN lacks this scope; set it so the minted app token includes it. + if getCheckBranchProtection(safeOutputs.PushToPullRequestBranch) { + safeOutputsPermissionsLog.Print("Adding administration: read for push-to-pull-request-branch (check-branch-protection enabled)") + permissions.Set(PermissionAdministration, PermissionRead) + } } if safeOutputs.UpdatePullRequests != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.UpdatePullRequests.Staged) { safeOutputsPermissionsLog.Print("Adding permissions for update-pull-request") diff --git a/pkg/workflow/safe_outputs_permissions_test.go b/pkg/workflow/safe_outputs_permissions_test.go index 0ee03e7c2f0..c07fa97274e 100644 --- a/pkg/workflow/safe_outputs_permissions_test.go +++ b/pkg/workflow/safe_outputs_permissions_test.go @@ -274,27 +274,56 @@ func TestComputePermissionsForSafeOutputs(t *testing.T) { }, }, { - name: "push-to-pull-request-branch default fallback - requires pull-requests write", + name: "push-to-pull-request-branch default fallback - requires pull-requests write and administration read", safeOutputs: &SafeOutputsConfig{ PushToPullRequestBranch: &PushToPullRequestBranchConfig{ BaseSafeOutputConfig: BaseSafeOutputConfig{}, }, }, + expected: map[PermissionScope]PermissionLevel{ + PermissionContents: PermissionWrite, + PermissionPullRequests: PermissionWrite, + PermissionAdministration: PermissionRead, + }, + }, + { + name: "push-to-pull-request-branch with fallback-as-pull-request false - no pull-requests permission but administration read", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{}, + FallbackAsPullRequest: boolPtr(false), + }, + }, + expected: map[PermissionScope]PermissionLevel{ + PermissionContents: PermissionWrite, + PermissionAdministration: PermissionRead, + }, + }, + { + name: "push-to-pull-request-branch with check-branch-protection false - no administration permission", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{}, + CheckBranchProtection: boolPtr(false), + }, + }, expected: map[PermissionScope]PermissionLevel{ PermissionContents: PermissionWrite, PermissionPullRequests: PermissionWrite, }, }, { - name: "push-to-pull-request-branch with fallback-as-pull-request false - no pull-requests permission", + name: "push-to-pull-request-branch with check-branch-protection explicit true - includes administration read", safeOutputs: &SafeOutputsConfig{ PushToPullRequestBranch: &PushToPullRequestBranchConfig{ BaseSafeOutputConfig: BaseSafeOutputConfig{}, - FallbackAsPullRequest: boolPtr(false), + CheckBranchProtection: boolPtr(true), }, }, expected: map[PermissionScope]PermissionLevel{ - PermissionContents: PermissionWrite, + PermissionContents: PermissionWrite, + PermissionPullRequests: PermissionWrite, + PermissionAdministration: PermissionRead, }, }, { From 79a67197b0b9845c1293ee686facea1644b935f2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 25 Apr 2026 00:03:57 +0000 Subject: [PATCH 6/6] docs(adr): add draft ADR-28365 for optional branch protection check Co-Authored-By: Claude Sonnet 4.6 --- ...n-check-for-push-to-pull-request-branch.md | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 docs/adr/28365-optional-branch-protection-check-for-push-to-pull-request-branch.md diff --git a/docs/adr/28365-optional-branch-protection-check-for-push-to-pull-request-branch.md b/docs/adr/28365-optional-branch-protection-check-for-push-to-pull-request-branch.md new file mode 100644 index 00000000000..392b3b55c1a --- /dev/null +++ b/docs/adr/28365-optional-branch-protection-check-for-push-to-pull-request-branch.md @@ -0,0 +1,89 @@ +# ADR-28365: Optional Branch Protection Pre-flight Check for `push-to-pull-request-branch` + +**Date**: 2026-04-25 +**Status**: Draft +**Deciders**: Unknown + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `push-to-pull-request-branch` safe output handler performs a pre-flight check against the GitHub branch protection API (`GET /repos/{owner}/{repo}/branches/{branch}/protection`) before pushing. This call requires `administration: read` permission, which is a GitHub App-only scope not available on standard `GITHUB_TOKEN`. Without this scope, the API returns HTTP 403, which the handler silently swallowed—logging a warning and continuing—making the check effectively useless noise. At the same time, automatically granting `administration: read` to all workflows that use this handler would increase every deployment's permission surface area, even for teams that don't need or want the protection check. + +### Decision + +We will make the branch protection pre-flight check opt-out via a new `check-branch-protection` boolean frontmatter key (default `true`). When the check is enabled (the default), the permission compiler automatically adds `administration: read` to the GitHub App token so the API call succeeds. When `check-branch-protection: false` is set, neither the API call nor the `administration: read` permission is added. The default branch guard (blocking pushes to the repository's default branch) runs unconditionally regardless of this setting, because it uses the repos API rather than the branch protection API. + +### Alternatives Considered + +#### Alternative 1: Always require `administration: read` with no opt-out + +All workflows using `push-to-pull-request-branch` would automatically receive `administration: read`. This eliminates the silent-failure problem and simplifies the implementation. It was rejected because it forces every team to grant a broad GitHub App scope even when they have no interest in the branch protection check and prefer minimal permission footprints. + +#### Alternative 2: Remove the branch protection pre-flight check entirely + +Eliminating the check avoids the permission problem without any new configuration surface. It was rejected because the check provides genuine defense-in-depth: it prevents agentic pushes to branches under review policies, catching misconfigurations before GitHub's push-time enforcement triggers a harder failure. The value of the check outweighs the complexity of making it configurable. + +#### Alternative 3: Silently ignore 403 and keep the existing behavior + +The current code already falls through on 403 with a warning, meaning the check is already a no-op for most GitHub App tokens. This was rejected because it perpetuates misleading log output and provides no security value while still requiring `administration: read` to appear in the schema. An explicit opt-out is more honest about what the check does and does not guarantee. + +### Consequences + +#### Positive +- The 403-swallow silent failure is eliminated: when the check is enabled, the token has the right scope and the check is meaningful. +- Users with strict permission policies can set `check-branch-protection: false` to avoid requesting `administration: read` entirely. +- The default branch guard (using the repos API) continues to run unconditionally, preserving the most critical safety check. +- Extracting the combined guard logic into a `checkBranchPushable` helper improves testability and isolation. + +#### Negative +- Enabling the default increases the permission footprint of all existing `push-to-pull-request-branch` deployments that use GitHub App tokens (they gain `administration: read` automatically on upgrade). +- The configuration surface for the handler grows by one field, adding a new concept callers must understand. +- The `administration: read` permission is a GitHub App-only scope with no effect on `GITHUB_TOKEN`; this asymmetry may confuse users who rely on `GITHUB_TOKEN`. + +#### Neutral +- The JSON schema for `push-to-pull-request-branch` gains a `check-branch-protection` boolean property, which may require documentation updates in tooling that consumes the schema. +- The permission computation in `ComputePermissionsForSafeOutputs` now branches on the effective `CheckBranchProtection` value, so permission calculations are no longer purely additive for this handler. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Branch Protection Check Configuration + +1. The `push-to-pull-request-branch` handler **MUST** treat `check-branch-protection` as an optional boolean key in workflow frontmatter, defaulting to `true` when absent. +2. When `check-branch-protection` is `true` (the default), the handler **MUST** call `GET /repos/{owner}/{repo}/branches/{branch}/protection` before executing the push. +3. When `check-branch-protection` is `false`, the handler **MUST NOT** call the branch protection API and **MUST NOT** request `administration: read` permission. +4. The handler **MUST** block the push and return an error when the branch protection API responds with a successful (2xx) status, indicating active protection rules exist. +5. The handler **MUST** permit the push when the branch protection API responds with HTTP 404 (no rules configured). +6. The handler **SHOULD** log a warning and permit the push when the branch protection API responds with HTTP 403 (insufficient permissions), because the GitHub platform still enforces protection at push time. +7. The handler **MUST** block the push when the branch protection API responds with any unexpected error (e.g., 5xx, network failure) to fail closed and prevent accidental writes to potentially protected branches. + +### Default Branch Guard + +1. The handler **MUST** check whether the target branch is the repository's default branch, regardless of the `check-branch-protection` setting. +2. The handler **MUST** block the push and return an error when the target branch equals the repository's default branch. +3. The handler **SHOULD** log a warning and continue when the repositories API call needed to resolve the default branch fails, rather than blocking the push due to an unrelated API error. + +### Permission Computation + +1. The permission compiler **MUST** automatically add `administration: read` to the computed GitHub App token permissions when `check-branch-protection` is `true` (or absent/defaulted). +2. The permission compiler **MUST NOT** add `administration: read` when `check-branch-protection` is explicitly `false`. +3. Implementations **SHOULD** document that `administration: read` is a GitHub App-only scope with no effect on standard `GITHUB_TOKEN`. + +### Schema + +1. The JSON schema for `push-to-pull-request-branch` **MUST** declare `check-branch-protection` as an optional boolean property with `default: true`. +2. The schema **MUST NOT** require `check-branch-protection` to be present. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24917291168) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*