Skip to content
Merged
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
80 changes: 80 additions & 0 deletions docs/adr/0002-explicit-opt-in-allow-workflows-permission.md
Original file line number Diff line number Diff line change
@@ -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

Comment on lines +1 to +6
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ADR is numbered 0002, but docs/adr/0002-allow-secrets-in-step-env-bindings-under-strict-mode.md already exists. Having two ADR-0002 documents will cause ambiguity and broken references. Please renumber this new ADR (and its title/header) to the next available number (e.g., ADR-0003) to keep the sequence unique.

Copilot uses AI. Check for mistakes.
---

## 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.*
14 changes: 14 additions & 0 deletions docs/src/content/docs/reference/frontmatter-full.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
21 changes: 21 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 @@ -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:
Expand Down
12 changes: 12 additions & 0 deletions pkg/parser/schemas/main_workflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment on lines +208 to +212
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new validation is inserted after the network firewall validations, splitting the safe-outputs validation block (target/max/allowed-domains) into two separate sections. For maintainability and discoverability, consider keeping all safe-outputs validations together (e.g., moving this check up near the other safe-outputs validations).

Copilot uses AI. Check for mistakes.

// Validate labels configuration
log.Printf("Validating labels")
if err := validateLabels(workflowData); err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/create_pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
Loading
Loading