Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions actions/setup/js/push_branch_safety.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// @ts-check
const { getErrorMessage } = require("./error_helpers.cjs");

/**
* Validate that a target branch is safe for agent pushes.
* Blocks pushes to default/protected branches and fails closed on unexpected
* branch protection lookup errors.
*
* @param {import("@actions/github-script").AsyncFunctionArguments["github"]} githubClient
* @param {string} owner
* @param {string} repo
* @param {string} branchName
* @param {boolean} checkBranchProtection
* @returns {Promise<{ success: true } | { success: false, error: string }>}
*/
async function validatePushBranchSafety(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) {
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 };
}

if (checkBranchProtection) {
// Check whether the branch has protection rules
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.
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 pre-flight check disabled for "${branchName}" by config (check-branch-protection: false)`);
}

return { success: true };
}

module.exports = { validatePushBranchSafety };
65 changes: 8 additions & 57 deletions actions/setup/js/push_to_pull_request_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const { checkFileProtection } = require("./manifest_file_helpers.cjs");
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { renderTemplateFromFile, buildProtectedFileList } = require("./messages_core.cjs");
const { getGitAuthEnv } = require("./git_helpers.cjs");
const { validatePushBranchSafety } = require("./push_branch_safety.cjs");

/**
* @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction
Expand Down Expand Up @@ -56,6 +57,7 @@ async function main(config = {}) {
const envLabels = config.labels ? (Array.isArray(config.labels) ? config.labels : config.labels.split(",")).map(label => String(label).trim()).filter(label => label) : [];
const ifNoChanges = config.if_no_changes || "warn";
const ignoreMissingBranchFailure = config.ignore_missing_branch_failure === true;
const checkBranchProtection = config.check_branch_protection !== false;
const fallbackAsPullRequest = config.fallback_as_pull_request !== false;
const commitTitleSuffix = config.commit_title_suffix || "";
const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024;
Expand Down Expand Up @@ -92,6 +94,7 @@ async function main(config = {}) {
}
core.info(`If no changes: ${ifNoChanges}`);
core.info(`Ignore missing branch failure: ${ignoreMissingBranchFailure}`);
core.info(`Check branch protection: ${checkBranchProtection}`);
core.info(`Fallback as pull request: ${fallbackAsPullRequest}`);
if (commitTitleSuffix) {
core.info(`Commit title suffix: ${commitTitleSuffix}`);
Expand Down Expand Up @@ -350,63 +353,11 @@ async function main(config = {}) {
core.info(`PR title: ${prTitle}`);
core.info(`PR labels: ${prLabels.join(", ")}`);

// SECURITY: Block pushing to the repository's default branch or any branch with
// protection rules. PR head branches must never be default or protected branches.
// 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
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 };
}
// SECURITY: Block pushing to the repository's default branch or any branch
// with protection rules. PR head branches must never be default/protected.
const branchSafetyResult = await validatePushBranchSafety(githubClient, repoParts.owner, repoParts.repo, branchName, checkBranchProtection);
if (!branchSafetyResult.success) {
return branchSafetyResult;
}

// Validate title prefix if specified
Expand Down
14 changes: 14 additions & 0 deletions actions/setup/js/push_to_pull_request_branch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,20 @@ index 0000000..abc1234
// Should not attempt any git operations
expect(mockExec.exec).not.toHaveBeenCalled();
});

it("should skip branch protection API check when check_branch_protection is false", async () => {
mockGithub.rest.repos.getBranchProtection.mockRejectedValue(Object.assign(new Error("Internal Server Error"), { status: 500 }));
const patchPath = createPatchFile();
mockExec.getExecOutput.mockResolvedValue({ exitCode: 0, stdout: "abc123\n", stderr: "" });

const module = await loadModule();
const handler = await module.main({ check_branch_protection: false });
const result = await handler({ patch_path: patchPath }, {});

expect(result.success).toBe(true);
expect(mockGithub.rest.repos.getBranchProtection).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Branch protection pre-flight check disabled"));
});
});

// ──────────────────────────────────────────────────────
Expand Down
6 changes: 6 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -4720,6 +4720,12 @@ safe-outputs:
# (optional)
ignore-missing-branch-failure: true

# When true (default), perform a pre-flight branch protection API check before
# pushing. Set to false to skip the API check (for example, when the token does
# not have administration: read); GitHub still enforces protection at push time.
# (optional)
check-branch-protection: true

# Optional suffix to append to generated commit titles (e.g., ' [skip ci]' to
# prevent triggering CI on the commit)
# (optional)
Expand Down
3 changes: 3 additions & 0 deletions docs/src/content/docs/reference/safe-outputs-pull-requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 # pre-flight branch protection API check (default: true)
protected-files: fallback-to-issue # create review issue if protected files modified
```

Expand Down Expand Up @@ -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 handler skips the branch-protection pre-flight API call (which otherwise requires `administration: read` on GitHub App tokens). GitHub still enforces branch protection at push time.

## 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.
Expand Down
5 changes: 5 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -7106,6 +7106,11 @@
"description": "When true, treat deleted/missing pull request branch errors as a skipped push instead of a hard failure. Useful when the PR branch may be deleted before safe outputs run.",
"default": false
},
"check-branch-protection": {
"type": "boolean",
"description": "When true (default), perform a pre-flight branch protection API check before pushing. Set to false to skip the API check (for example, when the token does not have administration: read); GitHub still enforces protection at push time.",
"default": true
},
"commit-title-suffix": {
"type": "string",
"description": "Optional suffix to append to generated commit titles (e.g., ' [skip ci]' to prevent triggering CI on the commit)"
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/compiler_safe_outputs_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ var handlerRegistry = map[string]handlerBuilder{
AddStringSlice("labels", c.Labels).
AddIfNotEmpty("if_no_changes", c.IfNoChanges).
AddIfTrue("ignore_missing_branch_failure", c.IgnoreMissingBranchFailure).
AddBoolPtr("check_branch_protection", c.CheckBranchProtection).
AddIfNotEmpty("commit_title_suffix", c.CommitTitleSuffix).
AddDefault("max_patch_size", maxPatchSize).
AddIfNotEmpty("target-repo", c.TargetRepoSlug).
Expand Down
8 changes: 8 additions & 0 deletions pkg/workflow/push_to_pull_request_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type PushToPullRequestBranchConfig struct {
Labels []string `yaml:"labels,omitempty"` // Required labels for pull request validation
IfNoChanges string `yaml:"if-no-changes,omitempty"` // Behavior when no changes to push: "warn", "error", or "ignore" (default: "warn")
IgnoreMissingBranchFailure bool `yaml:"ignore-missing-branch-failure,omitempty"` // When true, missing/deleted target branches are treated as skipped instead of hard failures.
CheckBranchProtection *bool `yaml:"check-branch-protection,omitempty"` // When false, skip branch protection pre-flight API check (default: true).
CommitTitleSuffix string `yaml:"commit-title-suffix,omitempty"` // Optional suffix to append to generated commit titles
GithubTokenForExtraEmptyCommit string `yaml:"github-token-for-extra-empty-commit,omitempty"` // Token used to push an empty commit to trigger CI events. Use a PAT or "app" for GitHub App auth.
TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository push to pull request branch
Expand Down Expand Up @@ -122,6 +123,13 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any)
}
}

// Parse check-branch-protection (optional, defaults to true in handler)
if checkBranchProtection, exists := configMap["check-branch-protection"]; exists {
if checkBranchProtectionBool, ok := checkBranchProtection.(bool); ok {
pushToBranchConfig.CheckBranchProtection = &checkBranchProtectionBool
}
}

// Parse title-prefix using shared helper
pushToBranchConfig.TitlePrefix = parseTitlePrefixFromConfig(configMap)

Expand Down
45 changes: 45 additions & 0 deletions pkg/workflow/push_to_pull_request_branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,51 @@ safe-outputs:
}
}

func TestPushToPullRequestBranchCheckBranchProtectionDisabled(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 PR Branch Branch-Protection Check Disabled
`

mdFile := filepath.Join(tmpDir, "test-push-to-pull-request-branch-check-protection-disabled.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)
}

pushConfig := extractPushToPullRequestBranchHandlerConfig(t, lockContent)
checkBranchProtection, exists := pushConfig["check_branch_protection"]
if !exists {
t.Fatalf("Expected check_branch_protection in handler config when check-branch-protection is explicitly set")
}
checkBranchProtectionBool, isBool := checkBranchProtection.(bool)
if !isBool {
t.Fatalf("Expected check_branch_protection to be a bool, got %#v", checkBranchProtection)
}
if checkBranchProtectionBool {
t.Fatalf("Expected check_branch_protection=false, got true")
}
}

func TestPushToPullRequestBranchFallbackAsPullRequestDisabled(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-*")

Expand Down