From bd2d5dff71205462fa04f7f04dc6e954840aaf9f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 03:35:51 +0000 Subject: [PATCH 1/3] Initial plan From 765304fe6e13da50a44fb5b9629c11ae973607f2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 03:55:43 +0000 Subject: [PATCH 2/3] feat: implement bot-actor isolation for issue_comment + github-app workflows (#concurrency) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b2acf6df-c3c1-43a4-9e4f-880d4df1f0c7 --- pkg/workflow/compiler.go | 18 ++ pkg/workflow/concurrency.go | 76 +++++++- pkg/workflow/concurrency_test.go | 299 +++++++++++++++++++++++++++++++ 3 files changed, 386 insertions(+), 7 deletions(-) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 6ccde4b7f00..c86bd3589f8 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -224,6 +224,24 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath } } + // Warn when the user has specified custom workflow-level concurrency with cancel-in-progress: true + // AND the workflow has the bot self-cancel risk combination (issue_comment triggers + GitHub App + // safe-outputs). In this case the auto-generated bot-actor isolation cannot be applied because the + // user's concurrency expression is preserved as-is. The user must add the bot-actor isolation + // themselves (e.g. prepend `contains(github.actor, '[bot]') && github.run_id ||` to their group key). + if workflowData.Concurrency != "" && + strings.Contains(workflowData.Concurrency, "cancel-in-progress: true") && + hasBotSelfCancelRisk(workflowData) { + fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", + "Custom workflow-level concurrency with cancel-in-progress: true may cause self-cancellation.\n"+ + "safe-outputs.github-app can post comments that re-trigger this workflow via issue_comment,\n"+ + "and those passive bot-authored runs can collide with the primary run's concurrency group.\n"+ + "Add `contains(github.actor, '[bot]') && github.run_id ||` at the start of your concurrency\n"+ + "group expression to route bot-triggered runs to a unique key and prevent self-cancellation.\n"+ + "See: https://gh.io/gh-aw/reference/concurrency for details.")) + c.IncrementWarningCount() + } + // Emit warning for sandbox.agent: false (disables agent sandbox firewall) if isAgentSandboxDisabled(workflowData) { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("⚠️ WARNING: Agent sandbox disabled (sandbox.agent: false). This removes firewall protection. The AI agent will have direct network access without firewall filtering. The MCP gateway remains enabled. Only use this for testing or in controlled environments where you trust the AI agent completely.")) diff --git a/pkg/workflow/concurrency.go b/pkg/workflow/concurrency.go index e0e273668da..4889cfda5cc 100644 --- a/pkg/workflow/concurrency.go +++ b/pkg/workflow/concurrency.go @@ -205,6 +205,48 @@ func entityConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumb return "${{ " + strings.Join(parts, " || ") + " }}" } +// botIsolatedConcurrencyKey builds a ${{ ... }} concurrency-group expression that +// routes GitHub App bot-triggered runs to a unique github.run_id key, preventing +// passive bot-authored comment events from colliding with the primary run's group. +// When contains(github.actor, '[bot]') is true, the expression short-circuits to +// github.run_id so that bot-triggered runs never share a group with human runs. +func botIsolatedConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumber bool) string { + parts := make([]string, 0, len(primaryParts)+len(tailParts)+2) + // Prepend the bot-actor isolation check: bot runs always get a unique key + parts = append(parts, "contains(github.actor, '[bot]') && github.run_id") + parts = append(parts, primaryParts...) + if hasItemNumber { + parts = append(parts, "inputs.item_number") + } + parts = append(parts, tailParts...) + return "${{ " + strings.Join(parts, " || ") + " }}" +} + +// hasIssueCommentTrigger returns true when the workflow has any trigger that uses +// issue_comment events: explicit issue_comment, slash_command, or command triggers. +// These are the only triggers where a GitHub App-authored comment on an issue can +// re-enter the same workflow and match the primary run's workflow-level concurrency +// group, creating the self-cancellation risk described in the issue. +func hasIssueCommentTrigger(workflowData *WorkflowData) bool { + on := workflowData.On + return strings.Contains(on, "issue_comment") || + strings.Contains(on, "slash_command") || + len(workflowData.Command) > 0 +} + +// hasBotSelfCancelRisk returns true when the workflow's auto-generated concurrency +// configuration can be collided with by a passive GitHub App bot-authored event. +// The dangerous combination is: issue_comment triggers + GitHub App safe-outputs. +// When this combination is present, App-authored comments posted by safe-outputs can +// re-trigger the workflow and resolve to the same workflow-level concurrency group +// as the originating run, causing cancel-in-progress to cancel the original run. +func hasBotSelfCancelRisk(workflowData *WorkflowData) bool { + if workflowData.SafeOutputs == nil || workflowData.SafeOutputs.GitHubApp == nil { + return false + } + return hasIssueCommentTrigger(workflowData) +} + // buildConcurrencyGroupKeys builds an array of keys for the concurrency group func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool) []string { keys := []string{"gh-aw", "${{ github.workflow }}"} @@ -214,16 +256,38 @@ func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool // use distinct groups and don't cancel each other. hasItemNumber := workflowData.HasDispatchItemNumber + // Detect whether the workflow is at risk of bot-self-cancellation. + // When safe-outputs uses a GitHub App token AND the workflow has issue_comment triggers, + // App-authored comments can re-trigger the workflow and collide with the primary run's + // concurrency group. When this risk is present we use botIsolatedConcurrencyKey so that + // bot-triggered runs always resolve to a unique github.run_id key instead. + botRisk := hasBotSelfCancelRisk(workflowData) + if botRisk { + concurrencyLog.Print("Bot self-cancel risk detected: applying bot-actor isolation to concurrency group key") + } + + // entityKey selects the correct concurrency key builder based on bot risk. + entityKey := func(primaryParts []string, tailParts []string) string { + if botRisk { + return botIsolatedConcurrencyKey(primaryParts, tailParts, hasItemNumber) + } + return entityConcurrencyKey(primaryParts, tailParts, hasItemNumber) + } + if isCommandTrigger || isSlashCommandWorkflow(workflowData.On) { // For command/slash_command workflows: use issue/PR number; fall back to run_id when // neither is available (e.g. manual workflow_dispatch of the outer workflow). - keys = append(keys, "${{ github.event.issue.number || github.event.pull_request.number || github.run_id }}") + // When bot risk is detected, prepend the bot-actor isolation check. + if botRisk { + keys = append(keys, "${{ contains(github.actor, '[bot]') && github.run_id || github.event.issue.number || github.event.pull_request.number || github.run_id }}") + } else { + keys = append(keys, "${{ github.event.issue.number || github.event.pull_request.number || github.run_id }}") + } } else if isPullRequestWorkflow(workflowData.On) && isIssueWorkflow(workflowData.On) { // Mixed workflows with both issue and PR triggers - keys = append(keys, entityConcurrencyKey( + keys = append(keys, entityKey( []string{"github.event.issue.number", "github.event.pull_request.number"}, []string{"github.run_id"}, - hasItemNumber, )) } else if isPullRequestWorkflow(workflowData.On) && isDiscussionWorkflow(workflowData.On) { // Mixed workflows with PR and discussion triggers @@ -234,10 +298,9 @@ func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool )) } else if isIssueWorkflow(workflowData.On) && isDiscussionWorkflow(workflowData.On) { // Mixed workflows with issue and discussion triggers - keys = append(keys, entityConcurrencyKey( + keys = append(keys, entityKey( []string{"github.event.issue.number", "github.event.discussion.number"}, []string{"github.run_id"}, - hasItemNumber, )) } else if isPullRequestWorkflow(workflowData.On) { // PR workflows: use PR number, fall back to ref then run_id @@ -249,10 +312,9 @@ func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool } else if isIssueWorkflow(workflowData.On) { // Issue workflows: run_id is the fallback when no issue context is available // (e.g. when a mixed-trigger workflow is started via workflow_dispatch). - keys = append(keys, entityConcurrencyKey( + keys = append(keys, entityKey( []string{"github.event.issue.number"}, []string{"github.run_id"}, - hasItemNumber, )) } else if isDiscussionWorkflow(workflowData.On) { // Discussion workflows: run_id is the fallback when no discussion context is available. diff --git a/pkg/workflow/concurrency_test.go b/pkg/workflow/concurrency_test.go index 3e949b77f3e..9996d9fc792 100644 --- a/pkg/workflow/concurrency_test.go +++ b/pkg/workflow/concurrency_test.go @@ -1276,3 +1276,302 @@ func TestHasSpecialTriggers(t *testing.T) { }) } } + +// TestHasBotSelfCancelRisk tests detection of the dangerous combination that can +// cause passive GitHub App bot-authored comment events to self-cancel a workflow run. +func TestHasBotSelfCancelRisk(t *testing.T) { + appConfig := &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + } + + tests := []struct { + name string + workflowData *WorkflowData + expected bool + description string + }{ + { + name: "slash_command with github-app is at risk", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + expected: true, + description: "slash_command workflows with GitHub App safe-outputs can self-cancel", + }, + { + name: "issue_comment with github-app is at risk", + workflowData: &WorkflowData{ + On: `on: + issue_comment: + types: [created]`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + expected: true, + description: "issue_comment workflows with GitHub App safe-outputs can self-cancel", + }, + { + name: "command trigger with github-app is at risk", + workflowData: &WorkflowData{ + On: `on:`, + Command: []string{"test-bot"}, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + expected: true, + description: "command trigger workflows with GitHub App safe-outputs can self-cancel", + }, + { + name: "slash_command without github-app is NOT at risk", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{}, + }, + expected: false, + description: "slash_command workflows without GitHub App safe-outputs are not at risk", + }, + { + name: "issue-only trigger with github-app is NOT at risk", + workflowData: &WorkflowData{ + On: `on: + issues: + types: [opened]`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + expected: false, + description: "issues: trigger without issue_comment does not re-trigger on app-posted comments", + }, + { + name: "PR trigger with github-app is NOT at risk", + workflowData: &WorkflowData{ + On: `on: + pull_request: + types: [opened]`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + expected: false, + description: "PR-only workflows without issue_comment do not re-trigger on app-posted comments", + }, + { + name: "no safe-outputs is NOT at risk", + workflowData: &WorkflowData{ + On: `on: + issue_comment: + types: [created]`, + SafeOutputs: nil, + }, + expected: false, + description: "workflows without safe-outputs do not post App-authored comments", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasBotSelfCancelRisk(tt.workflowData) + if result != tt.expected { + t.Errorf("hasBotSelfCancelRisk() for %q = %v, want %v: %s", tt.name, result, tt.expected, tt.description) + } + }) + } +} + +// TestBotActorIsolationInConcurrencyKeys tests that the bot-actor isolation prefix +// is correctly applied to concurrency group keys when the dangerous combination +// (issue_comment triggers + GitHub App safe-outputs) is detected. +func TestBotActorIsolationInConcurrencyKeys(t *testing.T) { + appConfig := &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + } + const botPrefix = "contains(github.actor, '[bot]') && github.run_id" + + tests := []struct { + name string + workflowData *WorkflowData + isAliasTrigger bool + expectBotKey bool + description string + }{ + { + name: "slash_command with github-app gets bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expectBotKey: true, + description: "slash_command + github-app should include bot-actor isolation", + }, + { + name: "issue_comment with github-app gets bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + issue_comment: + types: [created]`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expectBotKey: true, + description: "issue_comment + github-app should include bot-actor isolation", + }, + { + name: "command trigger with github-app gets bot-actor isolation", + workflowData: &WorkflowData{ + On: `on:`, + Command: []string{"test-bot"}, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: true, + expectBotKey: true, + description: "command trigger + github-app should include bot-actor isolation", + }, + { + name: "slash_command without github-app does NOT get bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{}, + }, + isAliasTrigger: false, + expectBotKey: false, + description: "slash_command without github-app should not include bot-actor isolation", + }, + { + name: "issue_comment without safe-outputs does NOT get bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + issue_comment: + types: [created]`, + SafeOutputs: nil, + }, + isAliasTrigger: false, + expectBotKey: false, + description: "issue_comment without safe-outputs should not include bot-actor isolation", + }, + { + name: "issues-only with github-app does NOT get bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + issues: + types: [opened]`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expectBotKey: false, + description: "issues-only trigger + github-app does not re-trigger on app-posted comments", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keys := buildConcurrencyGroupKeys(tt.workflowData, tt.isAliasTrigger) + + hasBotKey := false + for _, key := range keys { + if strings.Contains(key, botPrefix) { + hasBotKey = true + break + } + } + + if tt.expectBotKey && !hasBotKey { + t.Errorf("buildConcurrencyGroupKeys() for %q: expected bot-actor isolation prefix %q in keys %v", tt.name, botPrefix, keys) + } + if !tt.expectBotKey && hasBotKey { + t.Errorf("buildConcurrencyGroupKeys() for %q: unexpected bot-actor isolation prefix %q in keys %v", tt.name, botPrefix, keys) + } + }) + } +} + +// TestGenerateConcurrencyConfigWithBotIsolation tests that the full concurrency +// config string is correctly generated with bot-actor isolation when applicable. +func TestGenerateConcurrencyConfigWithBotIsolation(t *testing.T) { + appConfig := &GitHubAppConfig{ + AppID: "${{ vars.APP_ID }}", + PrivateKey: "${{ secrets.APP_PRIVATE_KEY }}", + } + + tests := []struct { + name string + workflowData *WorkflowData + isAliasTrigger bool + expected string + description string + }{ + { + name: "slash_command with github-app uses bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expected: `concurrency: + group: "gh-aw-${{ github.workflow }}-${{ contains(github.actor, '[bot]') && github.run_id || github.event.issue.number || github.event.pull_request.number || github.run_id }}"`, + description: "slash_command + github-app generates bot-isolated concurrency group", + }, + { + name: "issue_comment with github-app uses bot-actor isolation", + workflowData: &WorkflowData{ + On: `on: + issue_comment: + types: [created] + workflow_dispatch:`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expected: `concurrency: + group: "gh-aw-${{ github.workflow }}-${{ contains(github.actor, '[bot]') && github.run_id || github.event.issue.number || github.run_id }}"`, + description: "issue_comment + github-app generates bot-isolated concurrency group", + }, + { + name: "slash_command without github-app uses standard concurrency key", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + SafeOutputs: &SafeOutputsConfig{}, + }, + isAliasTrigger: false, + expected: `concurrency: + group: "gh-aw-${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number || github.run_id }}"`, + description: "slash_command without github-app uses standard concurrency (backward compat)", + }, + { + name: "Custom concurrency is preserved unchanged even with bot risk", + workflowData: &WorkflowData{ + On: `on: + slash_command: + name: test`, + Concurrency: `concurrency: + group: "custom-group-${{ github.event.issue.number }}" + cancel-in-progress: true`, + SafeOutputs: &SafeOutputsConfig{GitHubApp: appConfig}, + }, + isAliasTrigger: false, + expected: `concurrency: + group: "custom-group-${{ github.event.issue.number }}" + cancel-in-progress: true`, + description: "Custom concurrency is always preserved as-is (user must add bot isolation themselves)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GenerateConcurrencyConfig(tt.workflowData, tt.isAliasTrigger) + if result != tt.expected { + t.Errorf("GenerateConcurrencyConfig() for %q\nExpected:\n%s\nGot:\n%s", + tt.description, tt.expected, result) + } + }) + } +} From 6a19e010403dc65bc79195847859170704ff586f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 03:58:16 +0000 Subject: [PATCH 3/3] fix: add clarifying comment to entityKey closure explaining hasItemNumber capture Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b2acf6df-c3c1-43a4-9e4f-880d4df1f0c7 --- pkg/workflow/concurrency.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/workflow/concurrency.go b/pkg/workflow/concurrency.go index 4889cfda5cc..fae0ae51202 100644 --- a/pkg/workflow/concurrency.go +++ b/pkg/workflow/concurrency.go @@ -267,6 +267,8 @@ func buildConcurrencyGroupKeys(workflowData *WorkflowData, isCommandTrigger bool } // entityKey selects the correct concurrency key builder based on bot risk. + // It captures both botRisk and hasItemNumber from the outer scope, so call + // sites only need to supply the entity-specific parts. entityKey := func(primaryParts []string, tailParts []string) string { if botRisk { return botIsolatedConcurrencyKey(primaryParts, tailParts, hasItemNumber)