From 135386db6ee28d7b577aa98f3d0238d28d396673 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 14:19:25 +0000 Subject: [PATCH] feat: add allow-workflows field for GitHub App workflows:write permission on safe-outputs (#25776) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/68aaa733-6049-46fd-9e04-278b12b3a09b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...licit-opt-in-allow-workflows-permission.md | 80 +++++ .../docs/reference/frontmatter-full.md | 14 + .../reference/safe-outputs-pull-requests.md | 21 ++ pkg/parser/schemas/main_workflow_schema.json | 12 + pkg/workflow/compiler.go | 6 + pkg/workflow/create_pull_request.go | 1 + pkg/workflow/push_to_pull_request_branch.go | 8 + .../safe_outputs_allow_workflows_test.go | 336 ++++++++++++++++++ pkg/workflow/safe_outputs_permissions.go | 10 + pkg/workflow/safe_outputs_validation.go | 49 +++ 10 files changed, 537 insertions(+) create mode 100644 docs/adr/0002-explicit-opt-in-allow-workflows-permission.md create mode 100644 pkg/workflow/safe_outputs_allow_workflows_test.go diff --git a/docs/adr/0002-explicit-opt-in-allow-workflows-permission.md b/docs/adr/0002-explicit-opt-in-allow-workflows-permission.md new file mode 100644 index 00000000000..e5e2bcc1cf7 --- /dev/null +++ b/docs/adr/0002-explicit-opt-in-allow-workflows-permission.md @@ -0,0 +1,80 @@ +# ADR-0002: Explicit Opt-In for GitHub App workflows:write Permission via allow-workflows Field + +**Date**: 2026-04-11 +**Status**: Draft +**Deciders**: pelikhan, Copilot + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The gh-aw safe-outputs system mints scoped GitHub App tokens to push changes to pull request branches and create pull requests. When the `allowed-files` configuration targets `.github/workflows/` paths, GitHub requires the `workflows:write` permission on the minted token — a permission only available to GitHub Apps, not to `GITHUB_TOKEN`. The compiler's `ComputePermissionsForSafeOutputs` function previously had no knowledge of this requirement, leaving users unable to push workflow files through safe-outputs without resorting to fragile post-compile `sed` injection as a workaround. The question was: should the compiler auto-infer `workflows:write` from `allowed-files` patterns, or require an explicit opt-in field? + +### Decision + +We will add an explicit boolean field `allow-workflows` on both `create-pull-request` and `push-to-pull-request-branch` safe-outputs configurations. When set to `true`, the compiler adds `workflows: write` to the GitHub App token permissions computed by `ComputePermissionsForSafeOutputs`. The field is intentionally not auto-inferred from `allowed-files` patterns, keeping the elevated permission visible and auditable in the workflow source. Compile-time validation enforces that `safe-outputs.github-app` is configured (with non-empty `app-id` and `private-key`) whenever `allow-workflows: true` is present, because `workflows:write` cannot be granted via `GITHUB_TOKEN`. + +### Alternatives Considered + +#### Alternative 1: Auto-infer workflows:write from allowed-files patterns + +Detect at compile time whether any `allowed-files` pattern matches a `.github/workflows/` path (e.g., `strings.HasPrefix(pattern, ".github/workflows/")`), and automatically add `workflows: write` to the token permissions when such a pattern is found. This eliminates a configuration step for the user. It was rejected because it makes an elevated, GitHub App-only permission appear silently: a reviewer reading a workflow file would have no indication that `workflows:write` is being requested unless they also inspected the `allowed-files` list and understood the compiler's inference rules. Explicit opt-in makes the elevated permission a first-class, auditable fact in the workflow source. + +#### Alternative 2: Grant workflows:write globally for all safe-outputs operations + +Always include `workflows: write` in the safe-outputs GitHub App token, regardless of whether any workflow files are being pushed. This simplifies the permission model by eliminating per-handler configuration. It was rejected because it violates the principle of least privilege: the vast majority of safe-outputs deployments do not push workflow files, and granting `workflows:write` to those tokens unnecessarily expands the blast radius of a token compromise. Scoped permissions per handler are a deliberate security property of the safe-outputs system. + +#### Alternative 3: Keep the post-compile sed-injection workaround as the documented path + +Document the existing workaround (injecting `permission-workflows: write` via `sed` in a post-compile step) as the official solution for users who need to push workflow files. This requires no compiler changes. It was rejected because sed injection is inherently fragile, version-sensitive, and bypasses compile-time safety checks. It also produces compiled output that diverges from what the compiler would generate from the source, breaking the compiler's reproducibility guarantee. + +### Consequences + +#### Positive +- The elevated `workflows:write` permission is explicit and visible in the workflow source — security reviewers can see it at a glance without needing to understand compiler inference rules. +- Compile-time validation prevents misconfiguration: a workflow with `allow-workflows: true` but no GitHub App configured fails at compile time with a clear error message, rather than silently generating a broken workflow. +- The implementation is consistent with the existing staged-mode pattern: `allow-workflows: true` has no effect when the handler is in staged mode, since staged mode does not mint real tokens. +- Eliminates the fragile post-compile `sed` workaround that was the only previous path for pushing workflow files. + +#### Negative +- Users who need to push workflow files must add an extra field (`allow-workflows: true`) to their configuration, even when the need is obvious from their `allowed-files` patterns. This is a deliberate UX trade-off for auditability. +- The `validateSafeOutputsAllowWorkflows` validation function must be called explicitly from `validateWorkflowData` in `compiler.go`. If future validation functions are not wired up in the same way, they will be silently skipped. + +#### Neutral +- The `allow-workflows` field is defined separately on each handler (`create-pull-request` and `push-to-pull-request-branch`) rather than as a single top-level safe-outputs flag. This allows per-handler control but means users targeting both handlers must set the field in both places. +- The JSON schema is updated for both handler schemas, keeping schema-based tooling (IDE validation, linting) in sync with the Go implementation. + +--- + +## 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). + +### Permission Computation + +1. Implementations **MUST** add `workflows: write` to the computed GitHub App token permissions for a safe-outputs handler if and only if that handler's `allow-workflows` field is `true` and the handler is not in staged mode. +2. Implementations **MUST NOT** add `workflows: write` to token permissions when `allow-workflows` is absent or `false`. +3. Implementations **MUST NOT** add `workflows: write` to token permissions when the handler is in staged mode (i.e., `Staged: true`), even if `allow-workflows: true` is set. +4. Implementations **MUST NOT** infer the need for `workflows: write` from `allowed-files` patterns or any other implicit signal — the only authoritative source is the explicit `allow-workflows` field. + +### Compile-Time Validation + +1. Implementations **MUST** validate at compile time that `safe-outputs.github-app` is configured with a non-empty `app-id` and non-empty `private-key` whenever any safe-outputs handler has `allow-workflows: true`. +2. Implementations **MUST** produce a compile error if `allow-workflows: true` is present without a valid GitHub App configuration. +3. Implementations **SHOULD** include the handler name(s) in the compile error message to help users identify which handler(s) triggered the validation failure. +4. Implementations **SHOULD** include a configuration example in the compile error message showing how to add a GitHub App configuration. + +### Schema and Documentation + +1. Implementations **MUST** declare `allow-workflows` as an optional boolean field (default: `false`) in the JSON schema for both the `create-pull-request` and `push-to-pull-request-branch` handler objects. +2. Implementations **SHOULD** document that `allow-workflows` requires a GitHub App and cannot be used with `GITHUB_TOKEN`. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Specifically: the `workflows: write` permission is added to GitHub App token permissions when and only when an active (non-staged) safe-outputs handler has `allow-workflows: true`, and compilation fails with a clear error when `allow-workflows: true` is set without a valid GitHub App configuration. 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/24280835716) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 6818b18c7f7..db465c44571 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -3925,6 +3925,13 @@ safe-outputs: # (optional) staged: true + # 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. + # (optional) + allow-workflows: true + # Option 2: Enable pull request creation with default configuration create-pull-request: null @@ -4979,6 +4986,13 @@ safe-outputs: # (optional) patch-format: "am" + # 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. + # (optional) + allow-workflows: 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 07643a2b53d..494646ed9c3 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -208,6 +208,27 @@ Patterns support `*` (any characters except `/`) and `**` (any characters includ > [!WARNING] > `allowed-files` should enumerate exactly the files the workflow legitimately manages. Overly broad patterns (e.g., `**`) disable all protection. +### Allowing Workflow File Changes with `allow-workflows` + +When `allowed-files` targets `.github/workflows/` paths, pushing to those paths requires the GitHub Actions `workflows` permission. This is a **GitHub App-only permission** — it cannot be granted via `GITHUB_TOKEN`. + +Set `allow-workflows: true` on `create-pull-request` or `push-to-pull-request-branch` to add `workflows: write` to the minted GitHub App token. A `safe-outputs.github-app` configuration is required; the compiler will error if `allow-workflows: true` is set without one. + +```yaml wrap +safe-outputs: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + create-pull-request: + allow-workflows: true + allowed-files: + - ".github/workflows/*.lock.yml" + protected-files: allowed +``` + +> [!NOTE] +> `allow-workflows` is intentionally explicit rather than auto-inferred from `allowed-files` patterns. This makes the elevated permission visible and auditable in the workflow source. + ### Protected Files Protection covers three categories: diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index f89395c29ed..b7bb6926e3d 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5767,6 +5767,12 @@ "type": "boolean", "description": "If true, emit step summary messages instead of making GitHub API calls for this specific output type (preview mode)", "examples": [true, false] + }, + "allow-workflows": { + "type": "boolean", + "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] } }, "additionalProperties": false, @@ -6921,6 +6927,12 @@ "enum": ["am", "bundle"], "default": "am", "description": "Transport format for packaging changes. \"am\" (default) uses git format-patch/git am. \"bundle\" uses git bundle, which preserves merge commit topology, per-commit authorship, and merge-resolution-only content." + }, + "allow-workflows": { + "type": "boolean", + "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] } }, "additionalProperties": false diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index fc6f3414b33..a90d1773638 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -205,6 +205,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath return formatCompilerError(markdownPath, "error", err.Error(), err) } + // Validate safe-outputs allow-workflows requires GitHub App + log.Printf("Validating safe-outputs allow-workflows") + if err := validateSafeOutputsAllowWorkflows(workflowData.SafeOutputs); err != nil { + return formatCompilerError(markdownPath, "error", err.Error(), err) + } + // Validate labels configuration log.Printf("Validating labels") if err := validateLabels(workflowData); err != nil { diff --git a/pkg/workflow/create_pull_request.go b/pkg/workflow/create_pull_request.go index 770c96350e8..e9680b2f225 100644 --- a/pkg/workflow/create_pull_request.go +++ b/pkg/workflow/create_pull_request.go @@ -39,6 +39,7 @@ type CreatePullRequestsConfig struct { ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. PreserveBranchName bool `yaml:"preserve-branch-name,omitempty"` // When true, skips the random salt suffix on agent-specified branch names. Invalid characters are still replaced for security; casing is always preserved. Useful when CI enforces branch naming conventions (e.g. Jira keys in uppercase). 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). + AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured. } // parsePullRequestsConfig handles only create-pull-request (singular) configuration diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index a655ea4d87e..85738d1a499 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -24,6 +24,7 @@ type PushToPullRequestBranchConfig struct { AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for push. Checked independently of protected-files; both checks must pass. ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. 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). + AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured. } // buildCheckoutRepository generates a checkout step with optional target repository and custom token @@ -160,6 +161,13 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } } + // Parse allow-workflows: when true, adds workflows: write to the GitHub App token + if allowWorkflows, exists := configMap["allow-workflows"]; exists { + if allowWorkflowsBool, ok := allowWorkflows.(bool); ok { + pushToBranchConfig.AllowWorkflows = allowWorkflowsBool + } + } + // Parse common base fields with default max of 0 (no limit) c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0) } diff --git a/pkg/workflow/safe_outputs_allow_workflows_test.go b/pkg/workflow/safe_outputs_allow_workflows_test.go new file mode 100644 index 00000000000..8f7ccf10a46 --- /dev/null +++ b/pkg/workflow/safe_outputs_allow_workflows_test.go @@ -0,0 +1,336 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestAllowWorkflowsPermission tests that allow-workflows: true on create-pull-request +// adds workflows: write to the computed safe-output permissions. +func TestAllowWorkflowsPermission(t *testing.T) { + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectWorkflow bool + }{ + { + name: "create-pull-request with allow-workflows true adds workflows write", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + AllowWorkflows: true, + }, + }, + expectWorkflow: true, + }, + { + name: "create-pull-request without allow-workflows does not add workflows write", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{}, + }, + expectWorkflow: false, + }, + { + name: "push-to-pull-request-branch with allow-workflows true adds workflows write", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + AllowWorkflows: true, + }, + }, + expectWorkflow: true, + }, + { + name: "push-to-pull-request-branch without allow-workflows does not add workflows write", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{}, + }, + expectWorkflow: false, + }, + { + name: "both handlers with allow-workflows true", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + AllowWorkflows: true, + }, + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + AllowWorkflows: true, + }, + }, + expectWorkflow: true, + }, + { + name: "staged create-pull-request with allow-workflows does not add workflows write", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Staged: true}, + AllowWorkflows: true, + }, + }, + expectWorkflow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + permissions := ComputePermissionsForSafeOutputs(tt.safeOutputs) + require.NotNil(t, permissions, "Permissions should not be nil") + + level, ok := permissions.GetExplicit(PermissionWorkflows) + if tt.expectWorkflow { + assert.True(t, ok, "workflows permission should be present") + assert.Equal(t, PermissionWrite, level, "workflows should be write") + } else { + assert.False(t, ok, "workflows permission should not be present") + } + }) + } +} + +// TestAllowWorkflowsValidationRequiresGitHubApp tests that allow-workflows: true +// without a GitHub App configuration produces a validation error. +func TestAllowWorkflowsValidationRequiresGitHubApp(t *testing.T) { + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectError bool + }{ + { + name: "nil safe outputs - no error", + safeOutputs: nil, + expectError: false, + }, + { + name: "allow-workflows without github-app - error", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + AllowWorkflows: true, + }, + }, + expectError: true, + }, + { + name: "allow-workflows with github-app - no error", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + AllowWorkflows: true, + }, + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + }, + expectError: false, + }, + { + name: "push-to-pr-branch allow-workflows without github-app - error", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + AllowWorkflows: true, + }, + }, + expectError: true, + }, + { + name: "push-to-pr-branch allow-workflows with github-app - no error", + safeOutputs: &SafeOutputsConfig{ + PushToPullRequestBranch: &PushToPullRequestBranchConfig{ + AllowWorkflows: true, + }, + GitHubApp: &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + }, + }, + expectError: false, + }, + { + name: "no allow-workflows - no error even without github-app", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{}, + }, + expectError: false, + }, + { + name: "allow-workflows with empty github-app config - error", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + AllowWorkflows: true, + }, + GitHubApp: &GitHubAppConfig{}, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateSafeOutputsAllowWorkflows(tt.safeOutputs) + if tt.expectError { + require.Error(t, err, "Expected validation error") + assert.Contains(t, err.Error(), "allow-workflows", "Error should mention allow-workflows") + assert.Contains(t, err.Error(), "requires a GitHub App", "Error should mention GitHub App requirement") + assert.Contains(t, err.Error(), "github-app:", "Error should include configuration example") + } else { + assert.NoError(t, err, "Expected no validation error") + } + }) + } +} + +// TestAllowWorkflowsParsing tests that allow-workflows is correctly parsed from frontmatter. +func TestAllowWorkflowsParsing(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + markdown := `--- +on: issues +permissions: + contents: read +safe-outputs: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + create-pull-request: + allow-workflows: true + allowed-files: + - ".github/workflows/*.lock.yml" +--- + +# Test Workflow + +Test workflow with allow-workflows on create-pull-request. +` + + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + err := os.WriteFile(testFile, []byte(markdown), 0644) + require.NoError(t, err, "Failed to write test file") + + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse markdown content") + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.CreatePullRequests, "CreatePullRequests should not be nil") + + assert.True(t, workflowData.SafeOutputs.CreatePullRequests.AllowWorkflows, "AllowWorkflows should be true") + assert.Equal(t, []string{".github/workflows/*.lock.yml"}, workflowData.SafeOutputs.CreatePullRequests.AllowedFiles, "AllowedFiles should be parsed") +} + +// TestAllowWorkflowsParsingPushToPullRequestBranch tests parsing for push-to-pull-request-branch. +func TestAllowWorkflowsParsingPushToPullRequestBranch(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + markdown := `--- +on: pull_request +permissions: + contents: read +safe-outputs: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + push-to-pull-request-branch: + allow-workflows: true + allowed-files: + - ".github/workflows/*.lock.yml" +--- + +# Test Workflow + +Test workflow with allow-workflows on push-to-pull-request-branch. +` + + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + err := os.WriteFile(testFile, []byte(markdown), 0644) + require.NoError(t, err, "Failed to write test file") + + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse markdown content") + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + require.NotNil(t, workflowData.SafeOutputs.PushToPullRequestBranch, "PushToPullRequestBranch should not be nil") + + assert.True(t, workflowData.SafeOutputs.PushToPullRequestBranch.AllowWorkflows, "AllowWorkflows should be true") +} + +// TestAllowWorkflowsAppTokenPermission tests that when allow-workflows is true +// and a GitHub App is configured, the compiled output includes permission-workflows: write. +func TestAllowWorkflowsAppTokenPermission(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + markdown := `--- +on: issues +permissions: + contents: read +safe-outputs: + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + create-pull-request: + allow-workflows: true + allowed-files: + - ".github/workflows/*.lock.yml" +--- + +# Test Workflow + +Test workflow checking permission-workflows: write in GitHub App token. +` + + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "test.md") + err := os.WriteFile(testFile, []byte(markdown), 0644) + require.NoError(t, err, "Failed to write test file") + + workflowData, err := compiler.ParseWorkflowFile(testFile) + require.NoError(t, err, "Failed to parse workflow") + require.NotNil(t, workflowData.SafeOutputs, "SafeOutputs should not be nil") + + job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, "main", testFile) + require.NoError(t, err, "Failed to build safe_outputs job") + require.NotNil(t, job, "Job should not be nil") + + stepsStr := strings.Join(job.Steps, "") + assert.Contains(t, stepsStr, "permission-workflows: write", "GitHub App token should include workflows write permission") +} + +// TestAllowWorkflowsCompileErrorWithoutGitHubApp tests that compiling a workflow +// with allow-workflows: true but no GitHub App produces a compile error. +func TestAllowWorkflowsCompileErrorWithoutGitHubApp(t *testing.T) { + compiler := NewCompilerWithVersion("1.0.0") + + markdown := `--- +on: issues +permissions: + contents: read +safe-outputs: + create-pull-request: + allow-workflows: true + allowed-files: + - ".github/workflows/*.lock.yml" +--- + +# Test Workflow + +Test workflow with allow-workflows but no GitHub App. +` + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + mdPath := filepath.Join(workflowsDir, "test.md") + err := os.WriteFile(mdPath, []byte(markdown), 0644) + require.NoError(t, err, "Failed to write test file") + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(tmpDir)) + defer func() { _ = os.Chdir(origDir) }() + + err = compiler.CompileWorkflow(mdPath) + require.Error(t, err, "Compilation should fail without GitHub App") + assert.Contains(t, err.Error(), "allow-workflows", "Error should mention allow-workflows") +} diff --git a/pkg/workflow/safe_outputs_permissions.go b/pkg/workflow/safe_outputs_permissions.go index 7df2a668b2a..14c0c7b5b6e 100644 --- a/pkg/workflow/safe_outputs_permissions.go +++ b/pkg/workflow/safe_outputs_permissions.go @@ -130,10 +130,20 @@ func ComputePermissionsForSafeOutputs(safeOutputs *SafeOutputsConfig) *Permissio safeOutputsPermissionsLog.Print("Adding permissions for create-pull-request") permissions.Merge(NewPermissionsContentsWritePRWrite()) } + // Add workflows: write when allow-workflows is true (GitHub App-only permission) + if safeOutputs.CreatePullRequests.AllowWorkflows { + safeOutputsPermissionsLog.Print("Adding workflows: write for create-pull-request (allow-workflows: true)") + permissions.Set(PermissionWorkflows, PermissionWrite) + } } if safeOutputs.PushToPullRequestBranch != nil && !isHandlerStaged(safeOutputs.Staged, safeOutputs.PushToPullRequestBranch.Staged) { safeOutputsPermissionsLog.Print("Adding permissions for push-to-pull-request-branch") permissions.Merge(NewPermissionsContentsWritePRWrite()) + // Add workflows: write when allow-workflows is true (GitHub App-only permission) + if safeOutputs.PushToPullRequestBranch.AllowWorkflows { + safeOutputsPermissionsLog.Print("Adding workflows: write for push-to-pull-request-branch (allow-workflows: true)") + permissions.Set(PermissionWorkflows, PermissionWrite) + } } 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_validation.go b/pkg/workflow/safe_outputs_validation.go index 75b0e6fd618..8f119a6d5c8 100644 --- a/pkg/workflow/safe_outputs_validation.go +++ b/pkg/workflow/safe_outputs_validation.go @@ -307,3 +307,52 @@ func validateSafeOutputsMax(config *SafeOutputsConfig) error { safeOutputsMaxValidationLog.Print("Safe-outputs max fields validation passed") return nil } + +var safeOutputsAllowWorkflowsValidationLog = newValidationLogger("safe_outputs_allow_workflows") + +// validateSafeOutputsAllowWorkflows validates that allow-workflows: true requires +// a GitHub App to be configured in safe-outputs.github-app. The workflows permission +// is a GitHub App-only permission and cannot be granted via GITHUB_TOKEN. +func validateSafeOutputsAllowWorkflows(safeOutputs *SafeOutputsConfig) error { + if safeOutputs == nil { + return nil + } + + hasAllowWorkflows := false + var handlers []string + + if safeOutputs.CreatePullRequests != nil && safeOutputs.CreatePullRequests.AllowWorkflows { + hasAllowWorkflows = true + handlers = append(handlers, "create-pull-request") + } + if safeOutputs.PushToPullRequestBranch != nil && safeOutputs.PushToPullRequestBranch.AllowWorkflows { + hasAllowWorkflows = true + handlers = append(handlers, "push-to-pull-request-branch") + } + + if !hasAllowWorkflows { + return nil + } + + safeOutputsAllowWorkflowsValidationLog.Printf("allow-workflows: true found on: %s", strings.Join(handlers, ", ")) + + // Check if GitHub App is configured with required fields + if safeOutputs.GitHubApp == nil || safeOutputs.GitHubApp.AppID == "" || safeOutputs.GitHubApp.PrivateKey == "" { + safeOutputsAllowWorkflowsValidationLog.Print("allow-workflows requires github-app but none configured") + return fmt.Errorf( + "safe-outputs.%s.allow-workflows: requires a GitHub App to be configured.\n"+ + "The workflows permission is a GitHub App-only permission and cannot be granted via GITHUB_TOKEN.\n\n"+ + "Add a GitHub App configuration to safe-outputs:\n\n"+ + "safe-outputs:\n"+ + " github-app:\n"+ + " app-id: ${{ vars.APP_ID }}\n"+ + " private-key: ${{ secrets.APP_PRIVATE_KEY }}\n"+ + " %s:\n"+ + " allow-workflows: true", + handlers[0], handlers[0], + ) + } + + safeOutputsAllowWorkflowsValidationLog.Print("allow-workflows validation passed") + return nil +}