From 7939fc953b57a702c9ce11d59e9478b42ff5da07 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:02:52 +0000 Subject: [PATCH 1/5] Initial plan From 61bc3c307513d3070df512f991ed8db1e3bb03e1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:14:42 +0000 Subject: [PATCH 2/5] Refactor safe-output target parsing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/config_helpers.go | 10 +-- pkg/workflow/config_parsing_helpers_test.go | 84 ++++++++++++++++++++- pkg/workflow/create_code_scanning_alert.go | 11 +-- pkg/workflow/create_pr_review_comment.go | 18 ++--- pkg/workflow/dispatch_workflow.go | 9 ++- pkg/workflow/link_sub_issue.go | 9 ++- pkg/workflow/push_to_pull_request_branch.go | 18 ++--- pkg/workflow/reply_to_pr_review_comment.go | 27 ++----- pkg/workflow/safe_outputs_parser.go | 47 +++++++++--- pkg/workflow/submit_pr_review.go | 15 ++-- pkg/workflow/update_project.go | 10 +-- 11 files changed, 176 insertions(+), 82 deletions(-) diff --git a/pkg/workflow/config_helpers.go b/pkg/workflow/config_helpers.go index 3b953a44d4e..ae292ce84ae 100644 --- a/pkg/workflow/config_helpers.go +++ b/pkg/workflow/config_helpers.go @@ -128,13 +128,11 @@ func extractStringFromMap(m map[string]any, key string, debugLog *logger.Logger) // Returns an error (indicated by the second return value being true) if the value is "*" (wildcard), // which is not allowed for safe output target repositories. func parseTargetRepoWithValidation(configMap map[string]any) (string, bool) { - targetRepoSlug := extractStringFromMap(configMap, "target-repo", configHelpersLog) - // Validate that target-repo is not "*" - only definite strings are allowed - if targetRepoSlug == "*" { - configHelpersLog.Print("Invalid target-repo: wildcard '*' is not allowed") - return "", true // Return true to indicate validation error + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, configHelpersLog, safeOutputTargetConfigOptions{}) + if isInvalid { + return "", true } - return targetRepoSlug, false + return targetConfig.TargetRepoSlug, false } // NOTE: parseExpiresFromConfig and parseRelativeTimeSpec have been moved to time_delta.go diff --git a/pkg/workflow/config_parsing_helpers_test.go b/pkg/workflow/config_parsing_helpers_test.go index dba17faaaaa..08ae245adb4 100644 --- a/pkg/workflow/config_parsing_helpers_test.go +++ b/pkg/workflow/config_parsing_helpers_test.go @@ -410,7 +410,8 @@ func TestParsePRReviewCommentsConfigWithHelpers(t *testing.T) { compiler := &Compiler{} outputMap := map[string]any{ "create-pull-request-review-comment": map[string]any{ - "target-repo": "company/codebase", + "target-repo": "company/codebase", + "allowed-repos": []any{"company/codebase", "company/other"}, }, } @@ -422,6 +423,9 @@ func TestParsePRReviewCommentsConfigWithHelpers(t *testing.T) { if result.TargetRepoSlug != "company/codebase" { t.Errorf("expected target-repo 'company/codebase', got %q", result.TargetRepoSlug) } + if len(result.AllowedRepos) != 2 || result.AllowedRepos[0] != "company/codebase" || result.AllowedRepos[1] != "company/other" { + t.Errorf("expected parsed allowed-repos, got %v", result.AllowedRepos) + } } // Test wildcard target-repo is now allowed for all create/close handlers @@ -565,6 +569,84 @@ func TestParseTargetRepoWithValidation(t *testing.T) { } } +func TestParseSafeOutputTargetConfigOptions(t *testing.T) { + t.Run("parses target repo and allowed repos without target", func(t *testing.T) { + config, isInvalid := parseSafeOutputTargetConfig(map[string]any{ + "target": "17", + "target-repo": "owner/repo", + "allowed-repos": []any{"owner/repo", "owner/other"}, + }, nil, safeOutputTargetConfigOptions{ + parseAllowedRepos: true, + }) + + if isInvalid { + t.Fatal("expected config to be valid") + } + if config.Target != "" { + t.Fatalf("expected target to be ignored, got %q", config.Target) + } + if config.TargetRepoSlug != "owner/repo" { + t.Fatalf("expected target-repo owner/repo, got %q", config.TargetRepoSlug) + } + if len(config.AllowedRepos) != 2 || config.AllowedRepos[0] != "owner/repo" || config.AllowedRepos[1] != "owner/other" { + t.Fatalf("expected parsed allowed-repos, got %v", config.AllowedRepos) + } + }) + + t.Run("rejects wildcard target repo by default", func(t *testing.T) { + _, isInvalid := parseSafeOutputTargetConfig(map[string]any{ + "target-repo": "*", + }, nil, safeOutputTargetConfigOptions{}) + + if !isInvalid { + t.Fatal("expected wildcard target-repo to be invalid") + } + }) + + t.Run("allows wildcard target repo when configured", func(t *testing.T) { + config, isInvalid := parseSafeOutputTargetConfig(map[string]any{ + "target-repo": "*", + }, nil, safeOutputTargetConfigOptions{ + allowTargetRepoWildcard: true, + }) + + if isInvalid { + t.Fatal("expected wildcard target-repo to be valid") + } + if config.TargetRepoSlug != "*" { + t.Fatalf("expected wildcard target-repo, got %q", config.TargetRepoSlug) + } + }) + + t.Run("parses expression allowed repos only when configured", func(t *testing.T) { + expr := "${{ inputs['allowed-repos'] }}" + baseConfig := map[string]any{ + "allowed-repos": expr, + } + + config, isInvalid := parseSafeOutputTargetConfig(baseConfig, nil, safeOutputTargetConfigOptions{ + parseAllowedRepos: true, + }) + if isInvalid { + t.Fatal("expected config to be valid") + } + if config.AllowedRepos != nil { + t.Fatalf("expected expression allowed-repos to be ignored without expression support, got %v", config.AllowedRepos) + } + + config, isInvalid = parseSafeOutputTargetConfig(baseConfig, nil, safeOutputTargetConfigOptions{ + parseAllowedRepos: true, + allowAllowedReposExpression: true, + }) + if isInvalid { + t.Fatal("expected config to be valid") + } + if len(config.AllowedRepos) != 1 || config.AllowedRepos[0] != expr { + t.Fatalf("expected expression allowed-repos, got %v", config.AllowedRepos) + } + }) +} + func TestParseBoolFromConfig(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/create_code_scanning_alert.go b/pkg/workflow/create_code_scanning_alert.go index 09586f87fa2..cf71eedd4cf 100644 --- a/pkg/workflow/create_code_scanning_alert.go +++ b/pkg/workflow/create_code_scanning_alert.go @@ -38,14 +38,15 @@ func (c *Compiler) parseCodeScanningAlertsConfig(outputMap map[string]any) *Crea } } - // Parse target-repo - securityReportsConfig.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", createCodeScanningAlertLog) + targetConfig, _ := parseSafeOutputTargetConfig(configMap, createCodeScanningAlertLog, safeOutputTargetConfigOptions{ + allowTargetRepoWildcard: true, + parseAllowedRepos: true, + }) + securityReportsConfig.TargetRepoSlug = targetConfig.TargetRepoSlug if securityReportsConfig.TargetRepoSlug != "" { createCodeScanningAlertLog.Printf("Target repo for code scanning alerts: %s", securityReportsConfig.TargetRepoSlug) } - - // Parse allowed-repos - securityReportsConfig.AllowedRepos = ParseStringArrayFromConfig(configMap, "allowed-repos", createCodeScanningAlertLog) + securityReportsConfig.AllowedRepos = targetConfig.AllowedRepos if len(securityReportsConfig.AllowedRepos) > 0 { createCodeScanningAlertLog.Printf("Allowed repos for cross-repo alerts: %d configured", len(securityReportsConfig.AllowedRepos)) } diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index f84a62134ce..7108c4f2499 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -39,19 +39,17 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any } } - // Parse target - if target, exists := configMap["target"]; exists { - if targetStr, ok := target.(string); ok { - prReviewCommentsConfig.Target = targetStr - } - } - - // Parse target-repo using shared helper with validation - targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap) + // Parse target config (target, target-repo, allowed-repos) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, createPRReviewCommentLog, safeOutputTargetConfigOptions{ + parseTarget: true, + parseAllowedRepos: true, + }) if isInvalid { return nil // Invalid configuration, return nil to cause validation error } - prReviewCommentsConfig.TargetRepoSlug = targetRepoSlug + prReviewCommentsConfig.Target = targetConfig.Target + prReviewCommentsConfig.TargetRepoSlug = targetConfig.TargetRepoSlug + prReviewCommentsConfig.AllowedRepos = targetConfig.AllowedRepos if commitId, exists := configMap["commit-id"]; exists { if commitIdStr, ok := commitId.(string); ok { diff --git a/pkg/workflow/dispatch_workflow.go b/pkg/workflow/dispatch_workflow.go index 869fc9886ab..c8df2b81ee8 100644 --- a/pkg/workflow/dispatch_workflow.go +++ b/pkg/workflow/dispatch_workflow.go @@ -64,8 +64,13 @@ func (c *Compiler) parseDispatchWorkflowConfig(outputMap map[string]any) *Dispat } // Parse target-repo (optional cross-repo dispatch target) - dispatchWorkflowConfig.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", dispatchWorkflowLog) - dispatchWorkflowConfig.AllowedRepos = ParseStringArrayOrExprFromConfig(configMap, "allowed-repos", dispatchWorkflowLog) + targetConfig, _ := parseSafeOutputTargetConfig(configMap, dispatchWorkflowLog, safeOutputTargetConfigOptions{ + allowTargetRepoWildcard: true, + parseAllowedRepos: true, + allowAllowedReposExpression: true, + }) + dispatchWorkflowConfig.TargetRepoSlug = targetConfig.TargetRepoSlug + dispatchWorkflowConfig.AllowedRepos = targetConfig.AllowedRepos dispatchWorkflowConfig.AllowedRefs = ParseStringArrayOrExprFromConfig(configMap, "allowed-refs", dispatchWorkflowLog) if dispatchWorkflowConfig.AllowedRefs == nil { dispatchWorkflowConfig.AllowedRefs = []string{defaultDispatchWorkflowAllowedRef} diff --git a/pkg/workflow/link_sub_issue.go b/pkg/workflow/link_sub_issue.go index 65573db8fd3..a2c1a2b2643 100644 --- a/pkg/workflow/link_sub_issue.go +++ b/pkg/workflow/link_sub_issue.go @@ -26,13 +26,16 @@ func (c *Compiler) parseLinkSubIssueConfig(outputMap map[string]any) *LinkSubIss linkSubIssueLog.Print("Found link-sub-issue config map") // Parse target config (target-repo) with validation - targetConfig, isInvalid := ParseTargetConfig(configMap) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, linkSubIssueLog, safeOutputTargetConfigOptions{ + parseTarget: true, + allowTargetRepoWildcard: true, + parseAllowedRepos: true, + allowAllowedReposExpression: true, + }) if isInvalid { return nil // Invalid configuration (e.g., wildcard target-repo), return nil to cause validation error } linkSubIssueConfig.SafeOutputTargetConfig = targetConfig - // Override AllowedRepos with expression-aware parsing (supports GitHub Actions expressions) - linkSubIssueConfig.AllowedRepos = ParseStringArrayOrExprFromConfig(configMap, "allowed-repos", linkSubIssueLog) // Parse common base fields with default max of 5 c.parseBaseSafeOutputConfig(configMap, &linkSubIssueConfig.BaseSafeOutputConfig, 5) diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 78bfb96b5aa..2fc75bc8bbe 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -100,13 +100,6 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } if configMap, ok := configData.(map[string]any); ok { - // Parse target (optional, similar to add-comment) - if target, exists := configMap["target"]; exists { - if targetStr, ok := target.(string); ok { - pushToBranchConfig.Target = targetStr - } - } - // Parse if-no-changes (optional, defaults to "warn") if ifNoChanges, exists := configMap["if-no-changes"]; exists { if ifNoChangesStr, ok := ifNoChanges.(string); ok { @@ -159,7 +152,14 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } // Parse target-repo for cross-repository push - pushToBranchConfig.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", pushToPullRequestBranchLog) + targetConfig, _ := parseSafeOutputTargetConfig(configMap, pushToPullRequestBranchLog, safeOutputTargetConfigOptions{ + parseTarget: true, + allowTargetRepoWildcard: true, + parseAllowedRepos: true, + allowAllowedReposExpression: true, + }) + pushToBranchConfig.Target = targetConfig.Target + pushToBranchConfig.TargetRepoSlug = targetConfig.TargetRepoSlug pushToBranchConfig.HeadRepoSlug = extractStringFromMap(configMap, "head-repo", pushToPullRequestBranchLog) pushToBranchConfig.HeadGitHubToken = extractStringFromMap(configMap, "head-github-token", pushToPullRequestBranchLog) @@ -179,7 +179,7 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) pushToBranchConfig.BaseBranch = extractStringFromMap(configMap, "base-branch", pushToPullRequestBranchLog) // Parse allowed-repos for cross-repository push (expression-aware) - pushToBranchConfig.AllowedRepos = ParseStringArrayOrExprFromConfig(configMap, "allowed-repos", pushToPullRequestBranchLog) + pushToBranchConfig.AllowedRepos = targetConfig.AllowedRepos // Parse protected-files: supports string enum OR object form {policy, exclude}. exclude := preprocessProtectedFilesField(configMap, pushToPullRequestBranchLog) diff --git a/pkg/workflow/reply_to_pr_review_comment.go b/pkg/workflow/reply_to_pr_review_comment.go index 86e480012f0..35e6d57510a 100644 --- a/pkg/workflow/reply_to_pr_review_comment.go +++ b/pkg/workflow/reply_to_pr_review_comment.go @@ -27,30 +27,15 @@ func (c *Compiler) parseReplyToPullRequestReviewCommentConfig(outputMap map[stri // Parse common base fields with default max of 10 c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 10) - // Parse target - if target, exists := configMap["target"]; exists { - if targetStr, ok := target.(string); ok { - config.Target = targetStr - } - } - - // Parse target-repo using shared helper with validation - targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap) + // Parse target config (target, target-repo, allowed-repos) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, replyToPRReviewCommentLog, safeOutputTargetConfigOptions{ + parseTarget: true, + parseAllowedRepos: true, + }) if isInvalid { return nil // Invalid configuration, return nil to cause validation error } - config.TargetRepoSlug = targetRepoSlug - - // Parse allowed-repos - if allowedRepos, exists := configMap["allowed-repos"]; exists { - if repos, ok := allowedRepos.([]any); ok { - for _, repo := range repos { - if repoStr, ok := repo.(string); ok { - config.AllowedRepos = append(config.AllowedRepos, repoStr) - } - } - } - } + config.SafeOutputTargetConfig = targetConfig // Parse footer as templatable bool if err := preprocessBoolFieldAsString(configMap, "footer", replyToPRReviewCommentLog); err != nil { diff --git a/pkg/workflow/safe_outputs_parser.go b/pkg/workflow/safe_outputs_parser.go index 431540d4ad6..23dcb03780a 100644 --- a/pkg/workflow/safe_outputs_parser.go +++ b/pkg/workflow/safe_outputs_parser.go @@ -12,6 +12,13 @@ type SafeOutputTargetConfig struct { AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that operations can target (additionally to the target-repo) } +type safeOutputTargetConfigOptions struct { + parseTarget bool + allowTargetRepoWildcard bool + parseAllowedRepos bool + allowAllowedReposExpression bool +} + // SafeOutputAllowedLabelsConfig contains the shared allowed-labels field for safe output configurations. // Embed this in safe output config structs that restrict which labels may be used. type SafeOutputAllowedLabelsConfig struct { @@ -56,22 +63,42 @@ type ListJobConfig struct { // Returns the parsed SafeOutputTargetConfig and a boolean indicating if there was a validation error. // target-repo accepts "*" (wildcard) to indicate that any repository can be targeted. func ParseTargetConfig(configMap map[string]any) (SafeOutputTargetConfig, bool) { - safeOutputParserLog.Print("Parsing target config from map") + return parseSafeOutputTargetConfig(configMap, safeOutputParserLog, safeOutputTargetConfigOptions{ + parseTarget: true, + allowTargetRepoWildcard: true, + parseAllowedRepos: true, + }) +} + +func parseSafeOutputTargetConfig(configMap map[string]any, debugLog *logger.Logger, opts safeOutputTargetConfigOptions) (SafeOutputTargetConfig, bool) { + if debugLog != nil { + debugLog.Print("Parsing target config from map") + } + config := SafeOutputTargetConfig{} - // Parse target - if target, exists := configMap["target"]; exists { - if targetStr, ok := target.(string); ok { - config.Target = targetStr - safeOutputParserLog.Printf("Target set to: %s", targetStr) + if opts.parseTarget { + config.Target = extractStringFromMap(configMap, "target", debugLog) + if config.Target != "" && debugLog != nil { + debugLog.Printf("Target set to: %s", config.Target) } } - // Parse target-repo; wildcard "*" is allowed and means "any repository" - config.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", safeOutputParserLog) + config.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", debugLog) + if config.TargetRepoSlug == "*" && !opts.allowTargetRepoWildcard { + if debugLog != nil { + debugLog.Print("Invalid target-repo: wildcard '*' is not allowed") + } + return SafeOutputTargetConfig{}, true + } - // Parse allowed-repos - config.AllowedRepos = ParseStringArrayFromConfig(configMap, "allowed-repos", safeOutputParserLog) + if opts.parseAllowedRepos { + if opts.allowAllowedReposExpression { + config.AllowedRepos = ParseStringArrayOrExprFromConfig(configMap, "allowed-repos", debugLog) + } else { + config.AllowedRepos = ParseStringArrayFromConfig(configMap, "allowed-repos", debugLog) + } + } return config, false } diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index edc07bff36f..ce428578186 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -40,19 +40,14 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 1) // Parse target config (target, target-repo, allowed-repos) - // Uses parseTargetRepoWithValidation to disallow wildcard "*" for target-repo - if target, exists := configMap["target"]; exists { - if targetStr, ok := target.(string); ok { - config.Target = targetStr - } - } - - targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, submitPRReviewLog, safeOutputTargetConfigOptions{ + parseTarget: true, + parseAllowedRepos: true, + }) if isInvalid { return nil // Invalid configuration, return nil to cause validation error } - config.TargetRepoSlug = targetRepoSlug - config.AllowedRepos = ParseStringArrayFromConfig(configMap, "allowed-repos", submitPRReviewLog) + config.SafeOutputTargetConfig = targetConfig // Parse footer configuration (string: "always"/"none"/"if-body", or bool for backward compat) if footer, exists := configMap["footer"]; exists { diff --git a/pkg/workflow/update_project.go b/pkg/workflow/update_project.go index d0541bb64f8..e3db3ad4de2 100644 --- a/pkg/workflow/update_project.go +++ b/pkg/workflow/update_project.go @@ -51,14 +51,14 @@ func (c *Compiler) parseUpdateProjectConfig(outputMap map[string]any) *UpdatePro } // Parse target-repo for cross-repo content resolution (no wildcard allowed) - targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, updateProjectLog, safeOutputTargetConfigOptions{ + parseAllowedRepos: true, + }) if isInvalid { return nil } - updateProjectConfig.TargetRepoSlug = targetRepoSlug - - // Parse allowed-repos for cross-repo content resolution - updateProjectConfig.AllowedRepos = ParseStringArrayFromConfig(configMap, "allowed-repos", updateProjectLog) + updateProjectConfig.TargetRepoSlug = targetConfig.TargetRepoSlug + updateProjectConfig.AllowedRepos = targetConfig.AllowedRepos // Parse views if specified updateProjectConfig.Views = parseProjectViews(configMap, updateProjectLog) From bd49d40e8fe0285282fbdfce62508d7f17d3d27e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:21:28 +0000 Subject: [PATCH 3/5] Address target parser review feedback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/config_helpers.go | 4 +++- pkg/workflow/config_parsing_helpers_test.go | 19 ++++++++++++++++++- pkg/workflow/create_code_scanning_alert.go | 6 +++++- pkg/workflow/create_pr_review_comment.go | 1 + pkg/workflow/dispatch_workflow.go | 6 +++++- pkg/workflow/link_sub_issue.go | 1 + pkg/workflow/push_to_pull_request_branch.go | 6 +++++- pkg/workflow/reply_to_pr_review_comment.go | 1 + .../safe_outputs_cross_repo_config_test.go | 11 +++++++++++ pkg/workflow/safe_outputs_parser.go | 14 +++++++++----- pkg/workflow/submit_pr_review.go | 1 + pkg/workflow/update_project.go | 1 + 12 files changed, 61 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/config_helpers.go b/pkg/workflow/config_helpers.go index ae292ce84ae..0ab1eb01d6e 100644 --- a/pkg/workflow/config_helpers.go +++ b/pkg/workflow/config_helpers.go @@ -128,7 +128,9 @@ func extractStringFromMap(m map[string]any, key string, debugLog *logger.Logger) // Returns an error (indicated by the second return value being true) if the value is "*" (wildcard), // which is not allowed for safe output target repositories. func parseTargetRepoWithValidation(configMap map[string]any) (string, bool) { - targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, configHelpersLog, safeOutputTargetConfigOptions{}) + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, configHelpersLog, safeOutputTargetConfigOptions{ + parseTargetRepo: true, + }) if isInvalid { return "", true } diff --git a/pkg/workflow/config_parsing_helpers_test.go b/pkg/workflow/config_parsing_helpers_test.go index 08ae245adb4..afcd3b3e1f2 100644 --- a/pkg/workflow/config_parsing_helpers_test.go +++ b/pkg/workflow/config_parsing_helpers_test.go @@ -570,12 +570,26 @@ func TestParseTargetRepoWithValidation(t *testing.T) { } func TestParseSafeOutputTargetConfigOptions(t *testing.T) { + t.Run("ignores target repo without option", func(t *testing.T) { + config, isInvalid := parseSafeOutputTargetConfig(map[string]any{ + "target-repo": "owner/repo", + }, nil, safeOutputTargetConfigOptions{}) + + if isInvalid { + t.Fatal("expected config to be valid") + } + if config.TargetRepoSlug != "" { + t.Fatalf("expected target-repo to be ignored, got %q", config.TargetRepoSlug) + } + }) + t.Run("parses target repo and allowed repos without target", func(t *testing.T) { config, isInvalid := parseSafeOutputTargetConfig(map[string]any{ "target": "17", "target-repo": "owner/repo", "allowed-repos": []any{"owner/repo", "owner/other"}, }, nil, safeOutputTargetConfigOptions{ + parseTargetRepo: true, parseAllowedRepos: true, }) @@ -596,7 +610,9 @@ func TestParseSafeOutputTargetConfigOptions(t *testing.T) { t.Run("rejects wildcard target repo by default", func(t *testing.T) { _, isInvalid := parseSafeOutputTargetConfig(map[string]any{ "target-repo": "*", - }, nil, safeOutputTargetConfigOptions{}) + }, nil, safeOutputTargetConfigOptions{ + parseTargetRepo: true, + }) if !isInvalid { t.Fatal("expected wildcard target-repo to be invalid") @@ -607,6 +623,7 @@ func TestParseSafeOutputTargetConfigOptions(t *testing.T) { config, isInvalid := parseSafeOutputTargetConfig(map[string]any{ "target-repo": "*", }, nil, safeOutputTargetConfigOptions{ + parseTargetRepo: true, allowTargetRepoWildcard: true, }) diff --git a/pkg/workflow/create_code_scanning_alert.go b/pkg/workflow/create_code_scanning_alert.go index cf71eedd4cf..3dda00d4407 100644 --- a/pkg/workflow/create_code_scanning_alert.go +++ b/pkg/workflow/create_code_scanning_alert.go @@ -38,10 +38,14 @@ func (c *Compiler) parseCodeScanningAlertsConfig(outputMap map[string]any) *Crea } } - targetConfig, _ := parseSafeOutputTargetConfig(configMap, createCodeScanningAlertLog, safeOutputTargetConfigOptions{ + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, createCodeScanningAlertLog, safeOutputTargetConfigOptions{ + parseTargetRepo: true, allowTargetRepoWildcard: true, parseAllowedRepos: true, }) + if isInvalid { + return nil + } securityReportsConfig.TargetRepoSlug = targetConfig.TargetRepoSlug if securityReportsConfig.TargetRepoSlug != "" { createCodeScanningAlertLog.Printf("Target repo for code scanning alerts: %s", securityReportsConfig.TargetRepoSlug) diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index 7108c4f2499..c64c6511dd4 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -42,6 +42,7 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any // Parse target config (target, target-repo, allowed-repos) targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, createPRReviewCommentLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, parseAllowedRepos: true, }) if isInvalid { diff --git a/pkg/workflow/dispatch_workflow.go b/pkg/workflow/dispatch_workflow.go index c8df2b81ee8..cb2a4184d34 100644 --- a/pkg/workflow/dispatch_workflow.go +++ b/pkg/workflow/dispatch_workflow.go @@ -64,11 +64,15 @@ func (c *Compiler) parseDispatchWorkflowConfig(outputMap map[string]any) *Dispat } // Parse target-repo (optional cross-repo dispatch target) - targetConfig, _ := parseSafeOutputTargetConfig(configMap, dispatchWorkflowLog, safeOutputTargetConfigOptions{ + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, dispatchWorkflowLog, safeOutputTargetConfigOptions{ + parseTargetRepo: true, allowTargetRepoWildcard: true, parseAllowedRepos: true, allowAllowedReposExpression: true, }) + if isInvalid { + return nil + } dispatchWorkflowConfig.TargetRepoSlug = targetConfig.TargetRepoSlug dispatchWorkflowConfig.AllowedRepos = targetConfig.AllowedRepos dispatchWorkflowConfig.AllowedRefs = ParseStringArrayOrExprFromConfig(configMap, "allowed-refs", dispatchWorkflowLog) diff --git a/pkg/workflow/link_sub_issue.go b/pkg/workflow/link_sub_issue.go index a2c1a2b2643..3f2a2ce839e 100644 --- a/pkg/workflow/link_sub_issue.go +++ b/pkg/workflow/link_sub_issue.go @@ -28,6 +28,7 @@ func (c *Compiler) parseLinkSubIssueConfig(outputMap map[string]any) *LinkSubIss // Parse target config (target-repo) with validation targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, linkSubIssueLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, allowTargetRepoWildcard: true, parseAllowedRepos: true, allowAllowedReposExpression: true, diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index 2fc75bc8bbe..3f664fa1923 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -152,12 +152,16 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) } // Parse target-repo for cross-repository push - targetConfig, _ := parseSafeOutputTargetConfig(configMap, pushToPullRequestBranchLog, safeOutputTargetConfigOptions{ + targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, pushToPullRequestBranchLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, allowTargetRepoWildcard: true, parseAllowedRepos: true, allowAllowedReposExpression: true, }) + if isInvalid { + return nil + } pushToBranchConfig.Target = targetConfig.Target pushToBranchConfig.TargetRepoSlug = targetConfig.TargetRepoSlug pushToBranchConfig.HeadRepoSlug = extractStringFromMap(configMap, "head-repo", pushToPullRequestBranchLog) diff --git a/pkg/workflow/reply_to_pr_review_comment.go b/pkg/workflow/reply_to_pr_review_comment.go index 35e6d57510a..ddd6efef234 100644 --- a/pkg/workflow/reply_to_pr_review_comment.go +++ b/pkg/workflow/reply_to_pr_review_comment.go @@ -30,6 +30,7 @@ func (c *Compiler) parseReplyToPullRequestReviewCommentConfig(outputMap map[stri // Parse target config (target, target-repo, allowed-repos) targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, replyToPRReviewCommentLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, parseAllowedRepos: true, }) if isInvalid { diff --git a/pkg/workflow/safe_outputs_cross_repo_config_test.go b/pkg/workflow/safe_outputs_cross_repo_config_test.go index 51775e39efe..91568c5bbc9 100644 --- a/pkg/workflow/safe_outputs_cross_repo_config_test.go +++ b/pkg/workflow/safe_outputs_cross_repo_config_test.go @@ -149,6 +149,17 @@ func TestCreateCodeScanningAlertConfigTargetRepo(t *testing.T) { expectedToken: "", expectedDriver: "My Scanner", }, + { + name: "wildcard target-repo remains allowed", + configMap: map[string]any{ + "create-code-scanning-alert": map[string]any{ + "target-repo": "*", + }, + }, + expectedRepo: "*", + expectedRepos: nil, + expectedToken: "", + }, { name: "no cross-repo config", configMap: map[string]any{ diff --git a/pkg/workflow/safe_outputs_parser.go b/pkg/workflow/safe_outputs_parser.go index 23dcb03780a..c42a7763312 100644 --- a/pkg/workflow/safe_outputs_parser.go +++ b/pkg/workflow/safe_outputs_parser.go @@ -14,6 +14,7 @@ type SafeOutputTargetConfig struct { type safeOutputTargetConfigOptions struct { parseTarget bool + parseTargetRepo bool allowTargetRepoWildcard bool parseAllowedRepos bool allowAllowedReposExpression bool @@ -65,6 +66,7 @@ type ListJobConfig struct { func ParseTargetConfig(configMap map[string]any) (SafeOutputTargetConfig, bool) { return parseSafeOutputTargetConfig(configMap, safeOutputParserLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, allowTargetRepoWildcard: true, parseAllowedRepos: true, }) @@ -84,12 +86,14 @@ func parseSafeOutputTargetConfig(configMap map[string]any, debugLog *logger.Logg } } - config.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", debugLog) - if config.TargetRepoSlug == "*" && !opts.allowTargetRepoWildcard { - if debugLog != nil { - debugLog.Print("Invalid target-repo: wildcard '*' is not allowed") + if opts.parseTargetRepo { + config.TargetRepoSlug = extractStringFromMap(configMap, "target-repo", debugLog) + if config.TargetRepoSlug == "*" && !opts.allowTargetRepoWildcard { + if debugLog != nil { + debugLog.Print("Invalid target-repo: wildcard '*' is not allowed") + } + return SafeOutputTargetConfig{}, true } - return SafeOutputTargetConfig{}, true } if opts.parseAllowedRepos { diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index ce428578186..78d4d733606 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -42,6 +42,7 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) // Parse target config (target, target-repo, allowed-repos) targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, submitPRReviewLog, safeOutputTargetConfigOptions{ parseTarget: true, + parseTargetRepo: true, parseAllowedRepos: true, }) if isInvalid { diff --git a/pkg/workflow/update_project.go b/pkg/workflow/update_project.go index e3db3ad4de2..185562e1b1f 100644 --- a/pkg/workflow/update_project.go +++ b/pkg/workflow/update_project.go @@ -52,6 +52,7 @@ func (c *Compiler) parseUpdateProjectConfig(outputMap map[string]any) *UpdatePro // Parse target-repo for cross-repo content resolution (no wildcard allowed) targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, updateProjectLog, safeOutputTargetConfigOptions{ + parseTargetRepo: true, parseAllowedRepos: true, }) if isInvalid { From a0aebb4d76f6cbbdfe4b3893e3f625358cb322cb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:39:13 +0000 Subject: [PATCH 4/5] fix: preserve PR review comment allowed repos expressions Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/approve_workflow_run_test.go | 2 +- pkg/workflow/config_parsing_helpers_test.go | 23 +++++++++++++++++++++ pkg/workflow/create_pr_review_comment.go | 7 ++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/approve_workflow_run_test.go b/pkg/workflow/approve_workflow_run_test.go index 036527ce9eb..25cf9206bca 100644 --- a/pkg/workflow/approve_workflow_run_test.go +++ b/pkg/workflow/approve_workflow_run_test.go @@ -235,7 +235,7 @@ func TestApproveWorkflowRunAllowedPullRequestsExpressionEmitsJSONArray(t *testin runtimeConfig := strings.ReplaceAll(string(configJSON), wrappedExpression, `""`) var config map[string]map[string]any require.NoError(t, json.Unmarshal([]byte(runtimeConfig), &config)) - assert.Equal(t, "", config["approve_workflow_run"]["allowed_pull_requests"]) + assert.Empty(t, config["approve_workflow_run"]["allowed_pull_requests"]) }) } diff --git a/pkg/workflow/config_parsing_helpers_test.go b/pkg/workflow/config_parsing_helpers_test.go index afcd3b3e1f2..85a643c090c 100644 --- a/pkg/workflow/config_parsing_helpers_test.go +++ b/pkg/workflow/config_parsing_helpers_test.go @@ -428,6 +428,29 @@ func TestParsePRReviewCommentsConfigWithHelpers(t *testing.T) { } } +func TestParsePRReviewCommentsConfigPreservesAllowedReposExpression(t *testing.T) { + compiler := &Compiler{} + expr := "${{ inputs['allowed-repos'] }}" + outputMap := map[string]any{ + "create-pull-request-review-comment": map[string]any{ + "target-repo": "${{ inputs.target_repo }}", + "allowed-repos": expr, + }, + } + + result := compiler.parsePullRequestReviewCommentsConfig(outputMap) + if result == nil { + t.Fatal("expected non-nil result") + } + + if result.TargetRepoSlug != "${{ inputs.target_repo }}" { + t.Errorf("expected target-repo expression, got %q", result.TargetRepoSlug) + } + if len(result.AllowedRepos) != 1 || result.AllowedRepos[0] != expr { + t.Errorf("expected allowed-repos expression to be preserved, got %v", result.AllowedRepos) + } +} + // Test wildcard target-repo is now allowed for all create/close handlers func TestParseIssuesConfigWithWildcardTargetRepo(t *testing.T) { diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index c64c6511dd4..f9e1c6f2eb3 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -41,9 +41,10 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any // Parse target config (target, target-repo, allowed-repos) targetConfig, isInvalid := parseSafeOutputTargetConfig(configMap, createPRReviewCommentLog, safeOutputTargetConfigOptions{ - parseTarget: true, - parseTargetRepo: true, - parseAllowedRepos: true, + parseTarget: true, + parseTargetRepo: true, + parseAllowedRepos: true, + allowAllowedReposExpression: true, }) if isInvalid { return nil // Invalid configuration, return nil to cause validation error From 619a31159ee8357b654c249b9fc854534b613e2e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:42:46 +0000 Subject: [PATCH 5/5] fix: clarify link sub-issue invalid config comment Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/link_sub_issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/link_sub_issue.go b/pkg/workflow/link_sub_issue.go index 3f2a2ce839e..161ff741e69 100644 --- a/pkg/workflow/link_sub_issue.go +++ b/pkg/workflow/link_sub_issue.go @@ -34,7 +34,7 @@ func (c *Compiler) parseLinkSubIssueConfig(outputMap map[string]any) *LinkSubIss allowAllowedReposExpression: true, }) if isInvalid { - return nil // Invalid configuration (e.g., wildcard target-repo), return nil to cause validation error + return nil // Invalid configuration, return nil to cause validation error } linkSubIssueConfig.SafeOutputTargetConfig = targetConfig