diff --git a/.github/workflows/ace-editor.lock.yml b/.github/workflows/ace-editor.lock.yml index 6756c8705d0..90b6f9ff116 100644 --- a/.github/workflows/ace-editor.lock.yml +++ b/.github/workflows/ace-editor.lock.yml @@ -65,9 +65,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/archie.lock.yml b/.github/workflows/archie.lock.yml index 051c4e6d598..fb80cdc6f71 100644 --- a/.github/workflows/archie.lock.yml +++ b/.github/workflows/archie.lock.yml @@ -81,9 +81,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/brave.lock.yml b/.github/workflows/brave.lock.yml index afa23f63b8e..1358334ff2d 100644 --- a/.github/workflows/brave.lock.yml +++ b/.github/workflows/brave.lock.yml @@ -71,9 +71,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/changeset.lock.yml b/.github/workflows/changeset.lock.yml index f1ab8c1feb3..eb1cca9aac4 100644 --- a/.github/workflows/changeset.lock.yml +++ b/.github/workflows/changeset.lock.yml @@ -88,9 +88,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: "" diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index e44b6c121f5..d688d2b3396 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -83,9 +83,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: comment_id: ${{ steps.add-comment.outputs.comment-id }} comment_repo: ${{ steps.add-comment.outputs.comment-repo }} diff --git a/.github/workflows/craft.lock.yml b/.github/workflows/craft.lock.yml index 13ed115bfa7..55119e07c96 100644 --- a/.github/workflows/craft.lock.yml +++ b/.github/workflows/craft.lock.yml @@ -67,9 +67,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/dev.lock.yml b/.github/workflows/dev.lock.yml index ee4cdc65463..59725d5efe4 100644 --- a/.github/workflows/dev.lock.yml +++ b/.github/workflows/dev.lock.yml @@ -89,7 +89,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: comment_id: ${{ steps.add-comment.outputs.comment-id }} comment_repo: ${{ steps.add-comment.outputs.comment-repo }} diff --git a/.github/workflows/grumpy-reviewer.lock.yml b/.github/workflows/grumpy-reviewer.lock.yml index e5217f41456..2a6286cc75a 100644 --- a/.github/workflows/grumpy-reviewer.lock.yml +++ b/.github/workflows/grumpy-reviewer.lock.yml @@ -82,7 +82,6 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write pull-requests: write outputs: diff --git a/.github/workflows/mergefest.lock.yml b/.github/workflows/mergefest.lock.yml index 3a14bd5dd0d..a8b65568c4f 100644 --- a/.github/workflows/mergefest.lock.yml +++ b/.github/workflows/mergefest.lock.yml @@ -67,9 +67,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/pdf-summary.lock.yml b/.github/workflows/pdf-summary.lock.yml index 7eb3ec2ecdd..ec0e5ddfffb 100644 --- a/.github/workflows/pdf-summary.lock.yml +++ b/.github/workflows/pdf-summary.lock.yml @@ -95,9 +95,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/plan.lock.yml b/.github/workflows/plan.lock.yml index 2d06e4e8559..de09ed34c01 100644 --- a/.github/workflows/plan.lock.yml +++ b/.github/workflows/plan.lock.yml @@ -72,7 +72,6 @@ jobs: contents: read discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/poem-bot.lock.yml b/.github/workflows/poem-bot.lock.yml index 40d3095a64e..e401f410ac9 100644 --- a/.github/workflows/poem-bot.lock.yml +++ b/.github/workflows/poem-bot.lock.yml @@ -88,9 +88,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/pr-nitpick-reviewer.lock.yml b/.github/workflows/pr-nitpick-reviewer.lock.yml index 86c4be38f31..19041c249d8 100644 --- a/.github/workflows/pr-nitpick-reviewer.lock.yml +++ b/.github/workflows/pr-nitpick-reviewer.lock.yml @@ -77,7 +77,6 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write pull-requests: write outputs: diff --git a/.github/workflows/security-review.lock.yml b/.github/workflows/security-review.lock.yml index 2f84086ec36..6e1d1291998 100644 --- a/.github/workflows/security-review.lock.yml +++ b/.github/workflows/security-review.lock.yml @@ -79,7 +79,6 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write pull-requests: write outputs: diff --git a/.github/workflows/smoke-agent-all-merged.lock.yml b/.github/workflows/smoke-agent-all-merged.lock.yml index 7743805dd3e..81249bd4825 100644 --- a/.github/workflows/smoke-agent-all-merged.lock.yml +++ b/.github/workflows/smoke-agent-all-merged.lock.yml @@ -89,9 +89,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-agent-all-none.lock.yml b/.github/workflows/smoke-agent-all-none.lock.yml index 0aad235151f..13b9da636ad 100644 --- a/.github/workflows/smoke-agent-all-none.lock.yml +++ b/.github/workflows/smoke-agent-all-none.lock.yml @@ -89,9 +89,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-agent-public-approved.lock.yml b/.github/workflows/smoke-agent-public-approved.lock.yml index f3551c138e3..025042bea67 100644 --- a/.github/workflows/smoke-agent-public-approved.lock.yml +++ b/.github/workflows/smoke-agent-public-approved.lock.yml @@ -91,9 +91,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-agent-public-none.lock.yml b/.github/workflows/smoke-agent-public-none.lock.yml index 5e40aa1d6c7..b9607f24825 100644 --- a/.github/workflows/smoke-agent-public-none.lock.yml +++ b/.github/workflows/smoke-agent-public-none.lock.yml @@ -89,9 +89,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-agent-scoped-approved.lock.yml b/.github/workflows/smoke-agent-scoped-approved.lock.yml index acb01dd34d2..5dc8a665d01 100644 --- a/.github/workflows/smoke-agent-scoped-approved.lock.yml +++ b/.github/workflows/smoke-agent-scoped-approved.lock.yml @@ -90,9 +90,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 9720c5a6ff2..66c79d80107 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -104,9 +104,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index e7d0d80289b..b7392818a52 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -100,9 +100,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-copilot-arm.lock.yml b/.github/workflows/smoke-copilot-arm.lock.yml index 57a0f01e5f2..bb182a5a6d7 100644 --- a/.github/workflows/smoke-copilot-arm.lock.yml +++ b/.github/workflows/smoke-copilot-arm.lock.yml @@ -100,9 +100,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index cca41299cb9..9be54f8d768 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -95,9 +95,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: comment_id: ${{ steps.add-comment.outputs.comment-id }} comment_repo: ${{ steps.add-comment.outputs.comment-repo }} diff --git a/.github/workflows/smoke-create-cross-repo-pr.lock.yml b/.github/workflows/smoke-create-cross-repo-pr.lock.yml index 78b8877bcbc..e8a001e528c 100644 --- a/.github/workflows/smoke-create-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-create-cross-repo-pr.lock.yml @@ -89,9 +89,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-gemini.lock.yml b/.github/workflows/smoke-gemini.lock.yml index 27cfdc9571f..00c87c7c336 100644 --- a/.github/workflows/smoke-gemini.lock.yml +++ b/.github/workflows/smoke-gemini.lock.yml @@ -93,9 +93,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-multi-pr.lock.yml b/.github/workflows/smoke-multi-pr.lock.yml index b99c1b4140f..22c205bc806 100644 --- a/.github/workflows/smoke-multi-pr.lock.yml +++ b/.github/workflows/smoke-multi-pr.lock.yml @@ -89,9 +89,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-project.lock.yml b/.github/workflows/smoke-project.lock.yml index 6a99e4cc23e..f48bf7928a3 100644 --- a/.github/workflows/smoke-project.lock.yml +++ b/.github/workflows/smoke-project.lock.yml @@ -90,9 +90,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-service-ports.lock.yml b/.github/workflows/smoke-service-ports.lock.yml index 4565acb0813..929c7d1ca37 100644 --- a/.github/workflows/smoke-service-ports.lock.yml +++ b/.github/workflows/smoke-service-ports.lock.yml @@ -80,9 +80,6 @@ jobs: permissions: actions: read contents: read - discussions: write - issues: write - pull-requests: write outputs: comment_id: ${{ steps.add-comment.outputs.comment-id }} comment_repo: ${{ steps.add-comment.outputs.comment-repo }} diff --git a/.github/workflows/smoke-temporary-id.lock.yml b/.github/workflows/smoke-temporary-id.lock.yml index e43cb991678..883a749f99a 100644 --- a/.github/workflows/smoke-temporary-id.lock.yml +++ b/.github/workflows/smoke-temporary-id.lock.yml @@ -88,9 +88,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-test-tools.lock.yml b/.github/workflows/smoke-test-tools.lock.yml index 3f5bb05600e..45cf8d7c230 100644 --- a/.github/workflows/smoke-test-tools.lock.yml +++ b/.github/workflows/smoke-test-tools.lock.yml @@ -93,9 +93,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/smoke-update-cross-repo-pr.lock.yml b/.github/workflows/smoke-update-cross-repo-pr.lock.yml index 5bb15df20c4..d63bc681d5c 100644 --- a/.github/workflows/smoke-update-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-update-cross-repo-pr.lock.yml @@ -91,9 +91,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/tidy.lock.yml b/.github/workflows/tidy.lock.yml index c398b18b37c..7dd56871208 100644 --- a/.github/workflows/tidy.lock.yml +++ b/.github/workflows/tidy.lock.yml @@ -87,9 +87,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/unbloat-docs.lock.yml b/.github/workflows/unbloat-docs.lock.yml index 00c3bd2263e..cec7056366e 100644 --- a/.github/workflows/unbloat-docs.lock.yml +++ b/.github/workflows/unbloat-docs.lock.yml @@ -86,9 +86,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: ${{ steps.add-comment.outputs.comment-id }} diff --git a/.github/workflows/workflow-generator.lock.yml b/.github/workflows/workflow-generator.lock.yml index 7601d35e006..d1cb46b664b 100644 --- a/.github/workflows/workflow-generator.lock.yml +++ b/.github/workflows/workflow-generator.lock.yml @@ -71,9 +71,7 @@ jobs: permissions: actions: read contents: read - discussions: write issues: write - pull-requests: write outputs: body: ${{ steps.sanitized.outputs.body }} comment_id: "" diff --git a/docs/adr/26535-event-scoped-activation-permission-derivation.md b/docs/adr/26535-event-scoped-activation-permission-derivation.md new file mode 100644 index 00000000000..49fdbdf33e7 --- /dev/null +++ b/docs/adr/26535-event-scoped-activation-permission-derivation.md @@ -0,0 +1,79 @@ +# ADR-26535: Event-Scoped Activation Permission Derivation + +**Date**: 2026-04-16 +**Status**: Draft +**Deciders**: pelikhan, Copilot + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `gh aw compile` command generates a lock file that includes an activation job with a `permissions` block and a GitHub App token minting step. Before this change, whenever a compiled workflow configured `reaction` or `status-comment` triggers, the compiler unconditionally granted `issues: write`, `pull-requests: write`, and `discussions: write` — even if the workflow only triggers on `issues` events and never touches pull requests or discussions. This violated the principle of least privilege and caused compiled lock files to request far broader scopes than any workflow operation actually required. The `on:` section in gh-aw workflow frontmatter is a superset of real GitHub event names: it also contains metadata trigger fields (`reaction`, `status-comment`, `command`, etc.) that are interpreted by the framework, not forwarded to GitHub. Distinguishing real event names from metadata fields is therefore a prerequisite to computing accurate permissions. + +### Decision + +We will derive activation job permissions by parsing the `on:` section YAML at compile time, filtering out known metadata trigger fields, and granting only the write scopes required by the real GitHub event types that are configured. `issues: write` is granted only when `issues`, `issue_comment`, or `pull_request` events are present (since reactions and status comments on issues/PRs use the Issues REST API). `pull-requests: write` is granted only when `pull_request_review_comment` events are present. `discussions: write` is granted only when `discussion` or `discussion_comment` events are present. A fallback to the previous broad-grant behavior is preserved for synthetic or test `WorkflowData` instances where the `on:` section is empty. + +### Alternatives Considered + +#### Alternative 1: Retain Broad Permission Grants + +Always grant `issues: write`, `pull-requests: write`, and `discussions: write` whenever reactions or status comments are configured. This is the pre-existing behavior and is trivially correct (it never under-grants). It was rejected because it violates the principle of least privilege: a workflow that only triggers on issues should not carry `pull-requests: write` or `discussions: write` in its lock file, as those scopes could be exploited or trigger unexpected behavior. + +#### Alternative 2: Explicit Permission Declaration in Workflow Frontmatter + +Require workflow authors to declare which GitHub API scopes they need (e.g., `permissions: issues: write`) directly in the workflow markdown. This would be explicit and flexible, but it places the burden of correct permission reasoning on workflow authors — who are often non-engineers — and creates a class of misconfiguration bugs where a workflow runs with too few permissions because the author forgot to declare a scope. Automated derivation at compile time is more reliable. + +#### Alternative 3: Runtime Permission Escalation + +Request a minimal token at activation time and escalate permissions lazily when an operation fails due to insufficient scope. This approach is more dynamic but requires network round-trips to detect permission failures and complicates error handling and auditability. Compile-time derivation is simpler to reason about and does not require runtime infrastructure changes. + +### Consequences + +#### Positive +- Compiled lock files satisfy least-privilege: a workflow that only reacts to issues will no longer carry `pull-requests: write` or `discussions: write` scopes. +- The permission derivation logic is centralized in two new functions (`addActivationInteractionPermissionsMap`, `activationEventSet`), making future changes to permission rules easier to locate and audit. +- The metadata field allowlist (`activationMetadataTriggerFields`) is explicit and visible, replacing implicit substring-matching heuristics. +- Regression tests directly verify least-privilege behavior for issue-only and PR-review-comment-only trigger configurations. + +#### Negative +- The compiler now has a dependency on YAML parsing of the `on:` section at permission-derivation time, which adds a new failure mode: a malformed `on:` section will silently fall back to an empty event set and grant no permissions for reactions/status comments (though this is unlikely in practice since the same section is validated earlier in the compile pipeline). +- The fallback to broad permissions for empty `on:` sections means unit tests that use synthetic `WorkflowData` without a populated `on:` field will continue to get over-broad permissions, masking potential regressions in tests that don't exercise the full compile pipeline. + +#### Neutral +- The `activationMetadataTriggerFields` allowlist must be kept in sync with the set of metadata keys that gh-aw recognizes in the `on:` block. Adding a new metadata key without updating this list will cause it to be treated as a real GitHub event and potentially produce incorrect permission grants. +- The change affects both the activation job `permissions` block and the GitHub App token minting permissions — both call-sites now share the same derivation logic, which is a desirable consistency improvement. + +--- + +## 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). + +### Activation Permission Derivation + +1. Implementations **MUST** derive activation job write permissions from the set of real GitHub event types present in the `on:` section, not from the presence of `reaction` or `status-comment` configuration alone. +2. Implementations **MUST NOT** grant `pull-requests: write` in the activation job unless `pull_request_review_comment` is among the configured trigger events. +3. Implementations **MUST NOT** grant `discussions: write` in the activation job unless `discussion` or `discussion_comment` is among the configured trigger events. +4. Implementations **MUST NOT** grant `issues: write` solely for reaction/status-comment purposes unless `issues`, `issue_comment`, or `pull_request` is among the configured trigger events. +5. Implementations **MUST** apply the same permission derivation logic to both the activation job `permissions` block and the GitHub App token minting permissions. + +### Metadata Field Filtering + +1. Implementations **MUST** maintain an explicit allowlist of gh-aw metadata trigger fields (e.g., `reaction`, `status-comment`, `command`, `slash_command`, `label_command`, `stop-after`, `github-token`, `github-app`) and **MUST** exclude them from the derived event set. +2. Implementations **MUST NOT** treat an unrecognized key in the `on:` map as a metadata field; unknown keys **SHOULD** be treated as real GitHub event names to avoid silent under-granting. + +### Fallback Behavior + +1. When the `on:` section is absent or empty at permission derivation time, implementations **SHOULD** fall back to granting the full set of broad permissions (`issues: write`, `pull-requests: write`, `discussions: write`) and **MUST** emit a diagnostic log message explaining that the broad fallback is in use. +2. Implementations **MUST NOT** silently grant zero permissions for reactions or status comments when the `on:` section is malformed; a parse failure **SHOULD** be logged and the empty event set **SHOULD** result in no reaction/status-comment-related permissions being added. + +### 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/24488870005) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 0f4d83fdf80..e498389930a 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -1886,9 +1886,31 @@ "examples": ["eyes", "rocket", "+1", 1, -1, "none"] }, "status-comment": { - "type": "boolean", - "description": "Whether to post status comments (started/completed) on the triggering item. When true, adds a comment with workflow run link and updates it on completion. When false or not specified, no status comments are posted. Automatically enabled for slash_command and label_command triggers \u2014 manual configuration is only needed for other trigger types.", - "examples": [true, false] + "oneOf": [ + { + "type": "boolean" + }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "issues": { + "type": "boolean", + "description": "Whether status comments are allowed for issue triggers (issues, issue_comment)." + }, + "pull-requests": { + "type": "boolean", + "description": "Whether status comments are allowed for pull request triggers (pull_request, pull_request_review_comment)." + }, + "discussions": { + "type": "boolean", + "description": "Whether status comments are allowed for discussion and discussion_comment triggers." + } + } + } + ], + "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 }] }, "github-token": { "type": "string", diff --git a/pkg/workflow/activation_permissions_scope_test.go b/pkg/workflow/activation_permissions_scope_test.go new file mode 100644 index 00000000000..f5418e296f0 --- /dev/null +++ b/pkg/workflow/activation_permissions_scope_test.go @@ -0,0 +1,259 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActivationPermissionsIssueOnlyReactionAndStatusComment(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-issues-only") + testFile := filepath.Join(tmpDir, "issue-only.md") + testContent := `--- +on: + reaction: eyes + status-comment: true + issues: + types: [opened] +engine: copilot +safe-outputs: + add-comment: +--- + +# Issue-only activation permissions +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "issues: write", "activation job should include issues: write for issue trigger reactions/comments") + assert.NotContains(t, activationJobSection, "pull-requests: write", "activation job should not include pull-requests: write for issue-only triggers") + assert.NotContains(t, activationJobSection, "discussions: write", "activation job should not include discussions: write for issue-only triggers") +} + +func TestActivationPermissionsPRReviewReactionOnly(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-pr-review") + testFile := filepath.Join(tmpDir, "pr-review-reaction.md") + testContent := `--- +on: + reaction: eyes + status-comment: false + pull_request_review_comment: + types: [created] +engine: copilot +--- + +# PR review reaction permissions +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "pull-requests: write", "activation job should include pull-requests: write for PR review comment reactions") + assert.NotContains(t, activationJobSection, "issues: write", "activation job should not include issues: write for PR review comment reactions without status comments") + assert.NotContains(t, activationJobSection, "discussions: write", "activation job should not include discussions: write for PR review comment reactions") +} + +func TestActivationPermissionsStatusCommentDiscussionsDisabled(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-status-comment-discussions-disabled") + testFile := filepath.Join(tmpDir, "status-comment-discussions-disabled.md") + testContent := `--- +on: + reaction: none + status-comment: + discussions: false + discussion: + types: [created] +engine: copilot +--- + +# Status comment discussions disabled +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.NotContains(t, activationJobSection, "discussions: write", "activation job should not include discussions: write when status-comment.discussions is false") + assert.Contains(t, activationJobSection, "Add comment with workflow run link", "activation job should still include status comment step when enabled") + assert.Contains(t, activationJobSection, "github.event_name == 'issues'", "status comment condition should still include issue events") + assert.Contains(t, activationJobSection, "github.event_name == 'issue_comment'", "status comment condition should still include issue_comment events") + assert.NotContains(t, activationJobSection, "github.event_name == 'discussion'", "status comment condition should not include discussion events when status-comment.discussions is false") + assert.NotContains(t, activationJobSection, "github.event_name == 'discussion_comment'", "status comment condition should not include discussion_comment events when status-comment.discussions is false") +} + +func TestAddActivationInteractionPermissionsMapFallsBackOnInvalidOnYAML(t *testing.T) { + permsMap := map[PermissionScope]PermissionLevel{} + + addActivationInteractionPermissionsMap(permsMap, "on: [", true, true, true, true, true) + + assert.Equal(t, PermissionWrite, permsMap[PermissionIssues], "fallback should include issues:write") + assert.Equal(t, PermissionWrite, permsMap[PermissionPullRequests], "fallback should include pull-requests:write") + assert.Equal(t, PermissionWrite, permsMap[PermissionDiscussions], "fallback should include discussions:write when enabled") +} + +func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentDiscussionsToggle(t *testing.T) { + permsMap := map[PermissionScope]PermissionLevel{} + + addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", false, true, true, true, false) + + assert.Equal(t, PermissionWrite, permsMap[PermissionIssues], "fallback should include issues:write for status comments") + _, hasPullRequests := permsMap[PermissionPullRequests] + assert.False(t, hasPullRequests, "fallback should omit pull-requests:write when only status comments are enabled") + _, hasDiscussions := permsMap[PermissionDiscussions] + assert.False(t, hasDiscussions, "fallback should omit discussions:write when status-comment.discussions is false and reactions are disabled") +} + +func TestActivationPermissionsStatusCommentIssuesDisabled(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-status-comment-issues-disabled") + testFile := filepath.Join(tmpDir, "status-comment-issues-disabled.md") + testContent := `--- +on: + reaction: none + status-comment: + issues: false + issues: + types: [opened] + discussion: + types: [created] +engine: copilot +--- + +# Status comment issues disabled +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "discussions: write", "activation job should include discussions: write for discussion status comments") + assert.NotContains(t, activationJobSection, "issues: write", "activation job should not include issues: write when status-comment.issues is false and reactions are disabled") + assert.NotContains(t, activationJobSection, "pull-requests: write", "activation job should not include pull-requests: write when status-comment.issues is false and reactions are disabled") + assert.Contains(t, activationJobSection, "github.event_name == 'discussion'", "status comment condition should include discussion events when status-comment.issues is false") + assert.NotContains(t, activationJobSection, "github.event_name == 'issues'", "status comment condition should not include issue events when status-comment.issues is false") + assert.NotContains(t, activationJobSection, "github.event_name == 'issue_comment'", "status comment condition should not include issue_comment events when status-comment.issues is false") +} + +func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentIssuesToggle(t *testing.T) { + permsMap := map[PermissionScope]PermissionLevel{} + + addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", false, true, false, false, true) + + _, hasIssues := permsMap[PermissionIssues] + _, hasPullRequests := permsMap[PermissionPullRequests] + assert.False(t, hasIssues, "fallback should omit issues:write when status-comment.issues is false and reactions are disabled") + assert.False(t, hasPullRequests, "fallback should omit pull-requests:write when status-comment.issues is false and reactions are disabled") + assert.Equal(t, PermissionWrite, permsMap[PermissionDiscussions], "fallback should include discussions:write when status-comment.discussions is true") +} + +func TestStatusCommentObjectRejectsAllTargetsDisabled(t *testing.T) { + tmpDir := testutil.TempDir(t, "status-comment-object-all-disabled") + testFile := filepath.Join(tmpDir, "status-comment-object-all-disabled.md") + testContent := `--- +on: + status-comment: + issues: false + pull-requests: false + discussions: false + issues: + types: [opened] +engine: copilot +--- + +# Invalid status comment object +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.Error(t, err, "compilation should fail when status-comment object disables all targets") + assert.Contains(t, err.Error(), "status-comment object requires at least one target to be enabled", "error should explain invalid status-comment object configuration") +} + +func TestActivationPermissionsStatusCommentPullRequestsDisabled(t *testing.T) { + tmpDir := testutil.TempDir(t, "activation-perms-status-comment-pull-requests-disabled") + testFile := filepath.Join(tmpDir, "status-comment-pull-requests-disabled.md") + testContent := `--- +on: + reaction: none + status-comment: + pull-requests: false + issues: + types: [opened] + pull_request: + types: [opened] +engine: copilot +--- + +# Status comment pull-requests disabled +` + + err := os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err, "failed to write test workflow") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(testFile) + require.NoError(t, err, "failed to compile workflow") + + lockContent, err := os.ReadFile(stringutil.MarkdownToLockFile(testFile)) + require.NoError(t, err, "failed to read generated lock file") + + activationJobSection := extractJobSection(string(lockContent), string(constants.ActivationJobName)) + assert.Contains(t, activationJobSection, "issues: write", "activation job should include issues: write for issue status comments") + assert.NotContains(t, activationJobSection, "pull-requests: write", "activation job should not include pull-requests: write when reactions are disabled") + assert.Contains(t, activationJobSection, "github.event_name == 'issues'", "status comment condition should include issue events") + assert.Contains(t, activationJobSection, "github.event_name == 'issue_comment'", "status comment condition should include issue_comment events") + assert.NotContains(t, activationJobSection, "github.event_name == 'pull_request_review_comment'", "status comment condition should not include pull_request_review_comment when status-comment.pull-requests is false") +} + +func TestAddActivationInteractionPermissionsMapFallbackRespectsStatusCommentPullRequestsToggle(t *testing.T) { + permsMap := map[PermissionScope]PermissionLevel{} + + addActivationInteractionPermissionsMap(permsMap, "name: no-on-key", false, true, false, false, true) + + _, hasIssues := permsMap[PermissionIssues] + _, hasPullRequests := permsMap[PermissionPullRequests] + assert.False(t, hasIssues, "fallback should omit issues:write when status-comment.issues and status-comment.pull-requests are false and reactions are disabled") + assert.False(t, hasPullRequests, "fallback should omit pull-requests:write when reactions are disabled") + assert.Equal(t, PermissionWrite, permsMap[PermissionDiscussions], "fallback should include discussions:write when status-comment.discussions is true") +} diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index 03158d6a8d6..07a7d70aa50 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/pkg/workflow/compiler_activation_job.go @@ -10,10 +10,22 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/stringutil" + "github.com/goccy/go-yaml" ) var compilerActivationJobLog = logger.New("workflow:compiler_activation_job") +var activationMetadataTriggerFields = map[string]struct{}{ + "reaction": {}, + "status-comment": {}, + "command": {}, + "slash_command": {}, + "label_command": {}, + "stop-after": {}, + "github-token": {}, + "github-app": {}, +} + // buildActivationJob creates the activation job that handles timestamp checking, reactions, and locking. // This job depends on the pre-activation job if it exists, and runs before the main agent job. func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreated bool, workflowRunRepoSafety string, lockFilename string) (*Job, error) { @@ -106,6 +118,9 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate // inserted right after generate_aw_info for fast user feedback. hasReaction := data.AIReaction != "" && data.AIReaction != "none" hasStatusComment := data.StatusComment != nil && *data.StatusComment + statusCommentIncludesIssues := shouldIncludeIssueStatusComments(data) + statusCommentIncludesPullRequests := shouldIncludePullRequestStatusComments(data) + statusCommentIncludesDiscussions := shouldIncludeDiscussionStatusComments(data) hasLabelCommand := len(data.LabelCommand) > 0 // shouldRemoveLabel is true when label-command is active AND remove_label is not disabled shouldRemoveLabel := hasLabelCommand && data.LabelCommandRemoveLabel @@ -125,11 +140,15 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate // Build the combined permissions needed for all activation steps. // For label removal we only add the scopes required by the enabled events. appPerms := NewPermissions() - if hasReaction || hasStatusComment { - appPerms.Set(PermissionIssues, PermissionWrite) - appPerms.Set(PermissionPullRequests, PermissionWrite) - appPerms.Set(PermissionDiscussions, PermissionWrite) - } + addActivationInteractionPermissions( + appPerms, + data.On, + hasReaction, + hasStatusComment, + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) if shouldRemoveLabel { if slices.Contains(filteredLabelEvents, "issues") || slices.Contains(filteredLabelEvents, "pull_request") { appPerms.Set(PermissionIssues, PermissionWrite) @@ -291,11 +310,15 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate // Add comment with workflow run link if status comments are explicitly enabled if data.StatusComment != nil && *data.StatusComment { - reactionCondition := BuildReactionCondition() + statusCommentCondition := BuildStatusCommentCondition( + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) steps = append(steps, " - name: Add comment with workflow run link\n") steps = append(steps, " id: add-comment\n") - steps = append(steps, fmt.Sprintf(" if: %s\n", RenderCondition(reactionCondition))) + steps = append(steps, fmt.Sprintf(" if: %s\n", RenderCondition(statusCommentCondition))) steps = append(steps, fmt.Sprintf(" uses: %s\n", getCachedActionPin("actions/github-script", data))) // Add environment variables @@ -554,20 +577,15 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate permsMap[PermissionActions] = PermissionRead } - if hasReaction { - permsMap[PermissionDiscussions] = PermissionWrite - permsMap[PermissionIssues] = PermissionWrite - permsMap[PermissionPullRequests] = PermissionWrite - } - - // Add write permissions if status comments are enabled (even without a reaction). - // Status comments post to issues, PRs, and discussions, so write access is required. - // Assigning write to the map is safe here - it does not downgrade existing permissions. - if hasStatusComment { - permsMap[PermissionDiscussions] = PermissionWrite - permsMap[PermissionIssues] = PermissionWrite - permsMap[PermissionPullRequests] = PermissionWrite - } + addActivationInteractionPermissionsMap( + permsMap, + data.On, + hasReaction, + hasStatusComment, + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) // Add issues:write permission if lock-for-agent is enabled (even without reaction) if data.LockForAgent { @@ -621,6 +639,197 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate return job, nil } +func addActivationInteractionPermissions( + perms *Permissions, + onSection string, + hasReaction bool, + hasStatusComment bool, + statusCommentIncludesIssues bool, + statusCommentIncludesPullRequests bool, + statusCommentIncludesDiscussions bool, +) { + if perms == nil { + return + } + permsMap := make(map[PermissionScope]PermissionLevel) + addActivationInteractionPermissionsMap( + permsMap, + onSection, + hasReaction, + hasStatusComment, + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) + for scope, level := range permsMap { + perms.Set(scope, level) + } +} + +func addActivationInteractionPermissionsMap( + permsMap map[PermissionScope]PermissionLevel, + onSection string, + hasReaction bool, + hasStatusComment bool, + statusCommentIncludesIssues bool, + statusCommentIncludesPullRequests bool, + statusCommentIncludesDiscussions bool, +) { + if !hasReaction && !hasStatusComment { + return + } + + // Fallback for unit tests or synthetic WorkflowData instances that do not populate the "on" section. + // Real compiled workflows always have a populated trigger section. + if onSection == "" { + compilerActivationJobLog.Print("Empty on section while computing activation permissions; using broad fallback permissions") + addBroadActivationInteractionPermissions( + permsMap, + hasReaction, + hasStatusComment, + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) + return + } + + eventSet, eventSetParsed := activationEventSet(onSection) + if !eventSetParsed { + compilerActivationJobLog.Print("Unable to parse activation trigger events while computing permissions; using broad fallback permissions") + addBroadActivationInteractionPermissions( + permsMap, + hasReaction, + hasStatusComment, + statusCommentIncludesIssues, + statusCommentIncludesPullRequests, + statusCommentIncludesDiscussions, + ) + return + } + + hasIssuesEvent := eventSet["issues"] + hasIssueCommentEvent := eventSet["issue_comment"] + hasPullRequestEvent := eventSet["pull_request"] + hasPullRequestReviewCommentEvent := eventSet["pull_request_review_comment"] + hasDiscussionEvent := eventSet["discussion"] + hasDiscussionCommentEvent := eventSet["discussion_comment"] + + if hasReaction { + // Reactions on issues, issue comments, and pull requests all use issues endpoints. + if hasIssuesEvent || hasIssueCommentEvent || hasPullRequestEvent { + permsMap[PermissionIssues] = PermissionWrite + } + // Reactions on PR review comments use pull request review comment endpoints. + if hasPullRequestReviewCommentEvent { + permsMap[PermissionPullRequests] = PermissionWrite + } + // Reactions on discussions use GraphQL discussion APIs. + if hasDiscussionEvent || hasDiscussionCommentEvent { + permsMap[PermissionDiscussions] = PermissionWrite + } + } + + if hasStatusComment { + // Status comments for issue and pull request related events use issue comment endpoints. + if (statusCommentIncludesIssues && (hasIssuesEvent || hasIssueCommentEvent)) || + (statusCommentIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent)) { + permsMap[PermissionIssues] = PermissionWrite + } + // Status comments for discussions use discussion comment APIs and can be disabled via frontmatter. + if statusCommentIncludesDiscussions && (hasDiscussionEvent || hasDiscussionCommentEvent) { + permsMap[PermissionDiscussions] = PermissionWrite + } + } +} + +func addBroadActivationInteractionPermissions( + permsMap map[PermissionScope]PermissionLevel, + hasReaction bool, + hasStatusComment bool, + statusCommentIncludesIssues bool, + statusCommentIncludesPullRequests bool, + statusCommentIncludesDiscussions bool, +) { + if !hasReaction && !hasStatusComment { + return + } + + if hasReaction || statusCommentIncludesIssues || statusCommentIncludesPullRequests { + permsMap[PermissionIssues] = PermissionWrite + } + if hasReaction { + permsMap[PermissionPullRequests] = PermissionWrite + } + if hasReaction || statusCommentIncludesDiscussions { + permsMap[PermissionDiscussions] = PermissionWrite + } +} + +func shouldIncludeIssueStatusComments(data *WorkflowData) bool { + if data == nil || data.StatusCommentIssues == nil { + return true + } + return *data.StatusCommentIssues +} + +func shouldIncludePullRequestStatusComments(data *WorkflowData) bool { + if data == nil || data.StatusCommentPullRequests == nil { + return true + } + return *data.StatusCommentPullRequests +} + +func shouldIncludeDiscussionStatusComments(data *WorkflowData) bool { + if data == nil || data.StatusCommentDiscussions == nil { + return true + } + return *data.StatusCommentDiscussions +} + +func activationEventSet(onSection string) (map[string]bool, bool) { + events := make(map[string]bool) + var onData map[string]any + if err := yaml.Unmarshal([]byte(onSection), &onData); err != nil { + compilerActivationJobLog.Printf("Failed to parse on section for activation permission scoping: %v", err) + return events, false + } + + onValue, hasOn := onData["on"] + if !hasOn { + compilerActivationJobLog.Print("No top-level on key found while parsing activation permission events") + return events, false + } + + switch v := onValue.(type) { + case string: + events[v] = true + case []any: + for _, item := range v { + if eventName, ok := item.(string); ok { + events[eventName] = true + } + } + case map[string]any: + for eventName := range v { + if isActivationMetadataTriggerField(eventName) { + continue + } + events[eventName] = true + } + default: + compilerActivationJobLog.Printf("Unsupported on section type for activation permission scoping: %T", onValue) + return events, false + } + + return events, true +} + +func isActivationMetadataTriggerField(eventName string) bool { + _, isMetadataField := activationMetadataTriggerFields[eventName] + return isMetadataField +} + // generatePromptInActivationJob generates the prompt creation steps and adds them to the activation job // This creates the prompt.txt file that will be uploaded as an artifact and downloaded by the agent job // beforeActivationJobs is the list of custom job names that run before (i.e., are dependencies of) activation. diff --git a/pkg/workflow/compiler_safe_outputs.go b/pkg/workflow/compiler_safe_outputs.go index 40f549d0930..ebc092ebee9 100644 --- a/pkg/workflow/compiler_safe_outputs.go +++ b/pkg/workflow/compiler_safe_outputs.go @@ -2,6 +2,7 @@ package workflow import ( "encoding/json" + "errors" "fmt" "maps" "path/filepath" @@ -67,8 +68,50 @@ func (c *Compiler) parseOnSection(frontmatter map[string]any, workflowData *Work if statusCommentBool, ok := statusCommentValue.(bool); ok { workflowData.StatusComment = &statusCommentBool compilerSafeOutputsLog.Printf("status-comment set to: %v", statusCommentBool) + } else if statusCommentMap, ok := statusCommentValue.(map[string]any); ok { + statusCommentIssues := true + if issuesValue, hasIssues := statusCommentMap["issues"]; hasIssues { + issuesBool, ok := issuesValue.(bool) + if !ok { + return fmt.Errorf("status-comment.issues must be a boolean value, got %T", issuesValue) + } + statusCommentIssues = issuesBool + } + + statusCommentPullRequests := true + if pullRequestsValue, hasPullRequests := statusCommentMap["pull-requests"]; hasPullRequests { + pullRequestsBool, ok := pullRequestsValue.(bool) + if !ok { + return fmt.Errorf("status-comment.pull-requests must be a boolean value, got %T", pullRequestsValue) + } + statusCommentPullRequests = pullRequestsBool + } + + statusCommentDiscussions := true + if discussionsValue, hasDiscussions := statusCommentMap["discussions"]; hasDiscussions { + discussionsBool, ok := discussionsValue.(bool) + if !ok { + return fmt.Errorf("status-comment.discussions must be a boolean value, got %T", discussionsValue) + } + statusCommentDiscussions = discussionsBool + } + + statusCommentEnabled := true + workflowData.StatusComment = &statusCommentEnabled + workflowData.StatusCommentIssues = &statusCommentIssues + workflowData.StatusCommentPullRequests = &statusCommentPullRequests + workflowData.StatusCommentDiscussions = &statusCommentDiscussions + if !statusCommentIssues && !statusCommentPullRequests && !statusCommentDiscussions { + return errors.New("status-comment object requires at least one target to be enabled (issues, pull-requests, or discussions)") + } + compilerSafeOutputsLog.Printf( + "status-comment object set: issues=%v pullRequests=%v discussions=%v", + statusCommentIssues, + statusCommentPullRequests, + statusCommentDiscussions, + ) } else { - return fmt.Errorf("status-comment must be a boolean value, got %T", statusCommentValue) + return fmt.Errorf("status-comment must be a boolean or object value, got %T", statusCommentValue) } } diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 9dbc2a0b68d..a3f683f657e 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -436,6 +436,9 @@ type WorkflowData struct { LabelCommandRemoveLabel bool // whether to automatically remove the triggering label (default: true) AIReaction string // AI reaction type like "eyes", "heart", etc. StatusComment *bool // whether to post status comments (default: true when ai-reaction is set, false otherwise) + StatusCommentIssues *bool // whether status comments are allowed on issues/issue_comment triggers (default: true) + StatusCommentPullRequests *bool // whether status comments are allowed on pull_request/pull_request_review_comment triggers (default: true) + StatusCommentDiscussions *bool // whether status comments are allowed on discussion/discussion_comment triggers (default: true) ActivationGitHubToken string // custom github token from on.github-token for reactions/comments ActivationGitHubApp *GitHubAppConfig // github app config from on.github-app for minting activation tokens TopLevelGitHubApp *GitHubAppConfig // top-level github-app fallback for all nested github-app token minting operations diff --git a/pkg/workflow/expression_builder.go b/pkg/workflow/expression_builder.go index f293deed838..2a07fc1b08d 100644 --- a/pkg/workflow/expression_builder.go +++ b/pkg/workflow/expression_builder.go @@ -65,24 +65,53 @@ func BuildAnd(left ConditionNode, right ConditionNode) ConditionNode { // BuildReactionCondition creates a condition tree for the add_reaction job func BuildReactionCondition() ConditionNode { expressionBuilderLog.Print("Building reaction condition for multiple event types") - // Build a list of event types that should trigger reactions using the new expression nodes + return buildReactionLikeCondition(true, true, true) +} + +// BuildStatusCommentCondition creates a condition tree for activation status comments. +// When includeIssues is false, issues and issue_comment events are excluded. +// When includePullRequests is false, pull_request and pull_request_review_comment events are excluded. +// When includeDiscussions is false, discussion and discussion_comment events are excluded. +func BuildStatusCommentCondition(includeIssues bool, includePullRequests bool, includeDiscussions bool) ConditionNode { + expressionBuilderLog.Printf( + "Building status comment condition: includeIssues=%t includePullRequests=%t includeDiscussions=%t", + includeIssues, + includePullRequests, + includeDiscussions, + ) + return buildReactionLikeCondition(includeIssues, includePullRequests, includeDiscussions) +} + +func buildReactionLikeCondition(includeIssues bool, includePullRequests bool, includeDiscussions bool) ConditionNode { + // Build a list of event types that should trigger reactions/status-comments using expression nodes. var terms []ConditionNode - terms = append(terms, BuildEventTypeEquals("issues")) - terms = append(terms, BuildEventTypeEquals("issue_comment")) - terms = append(terms, BuildEventTypeEquals("pull_request_review_comment")) - terms = append(terms, BuildEventTypeEquals("discussion")) - terms = append(terms, BuildEventTypeEquals("discussion_comment")) + if includeIssues { + terms = append(terms, BuildEventTypeEquals("issues")) + terms = append(terms, BuildEventTypeEquals("issue_comment")) + } + if includePullRequests { + terms = append(terms, BuildEventTypeEquals("pull_request_review_comment")) + } + if includeDiscussions { + terms = append(terms, BuildEventTypeEquals("discussion")) + terms = append(terms, BuildEventTypeEquals("discussion_comment")) + } // For pull_request events, we need to ensure it's not from a forked repository // since forked repositories have read-only permissions and cannot add reactions - pullRequestCondition := &AndNode{ - Left: BuildEventTypeEquals("pull_request"), - Right: BuildNotFromFork(), + if includePullRequests { + pullRequestCondition := &AndNode{ + Left: BuildEventTypeEquals("pull_request"), + Right: BuildNotFromFork(), + } + terms = append(terms, pullRequestCondition) } - terms = append(terms, pullRequestCondition) expressionBuilderLog.Printf("Created disjunction with %d event type terms", len(terms)) + if len(terms) == 0 { + return BuildBooleanLiteral(false) + } // Use DisjunctionNode to avoid deep nesting return &DisjunctionNode{Terms: terms} diff --git a/pkg/workflow/task_and_reaction_permissions_test.go b/pkg/workflow/task_and_reaction_permissions_test.go index afd12488147..4be471fb5f1 100644 --- a/pkg/workflow/task_and_reaction_permissions_test.go +++ b/pkg/workflow/task_and_reaction_permissions_test.go @@ -98,14 +98,14 @@ The activation job references text output: "${{ steps.sanitized.outputs.text }}" } // Test 5: Verify activation job has required permissions for reactions - if !strings.Contains(activationJobSection, "discussions: write") { - t.Error("Activation job should have discussions: write permission") - } if !strings.Contains(activationJobSection, "issues: write") { t.Error("Activation job should have issues: write permission") } - if !strings.Contains(activationJobSection, "pull-requests: write") { - t.Error("Activation job should have pull-requests: write permission") + if strings.Contains(activationJobSection, "pull-requests: write") { + t.Error("Activation job should not have pull-requests: write permission for issues-only trigger") + } + if strings.Contains(activationJobSection, "discussions: write") { + t.Error("Activation job should not have discussions: write permission for issues-only trigger") } // Test 6: Verify reaction step is in activation job (moved from pre-activation)