diff --git a/.github/workflows/example-failure-category-filter.lock.yml b/.github/workflows/example-failure-category-filter.lock.yml index 71dd9bd5b32..d7dfd78d53f 100644 --- a/.github/workflows/example-failure-category-filter.lock.yml +++ b/.github/workflows/example-failure-category-filter.lock.yml @@ -1208,7 +1208,7 @@ jobs: GH_AW_DAILY_AI_CREDITS_THRESHOLD: ${{ needs.activation.outputs.daily_ai_credits_threshold }} GH_AW_GROUP_REPORTS: "false" GH_AW_FAILURE_REPORT_AS_ISSUE: "true" - GH_AW_FAILURE_CATEGORIES_FILTER: '["agent_failure","missing_safe_outputs","missing_tool","missing_data"]' + GH_AW_FAILURE_CATEGORIES_FILTER: "[\"agent_failure\",\"missing_safe_outputs\",\"missing_tool\",\"missing_data\"]" GH_AW_MISSING_TOOL_REPORT_AS_FAILURE: "true" GH_AW_MISSING_DATA_REPORT_AS_FAILURE: "true" GH_AW_TIMEOUT_MINUTES: "20" diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index 2ca7a7be6b5..ea3d96fb9b4 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -404,14 +404,14 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa if len(data.SafeOutputs.ReportFailureAsIssueCategories) > 0 { categoriesJSON, err := json.Marshal(data.SafeOutputs.ReportFailureAsIssueCategories) if err == nil { - agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_FAILURE_CATEGORIES_FILTER: '%s'\n", categoriesJSON)) + agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_FAILURE_CATEGORIES_FILTER: %q\n", string(categoriesJSON))) } } // If excluded categories filter is configured, pass it as JSON if len(data.SafeOutputs.ReportFailureAsIssueExcludedCategories) > 0 { excludedCategoriesJSON, err := json.Marshal(data.SafeOutputs.ReportFailureAsIssueExcludedCategories) if err == nil { - agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER: '%s'\n", excludedCategoriesJSON)) + agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER: %q\n", string(excludedCategoriesJSON))) } } } diff --git a/pkg/workflow/notify_comment_test.go b/pkg/workflow/notify_comment_test.go index 49baa287984..d4fd92f96a8 100644 --- a/pkg/workflow/notify_comment_test.go +++ b/pkg/workflow/notify_comment_test.go @@ -1158,6 +1158,70 @@ func TestConclusionJobNonWorkflowCallNoArtifactPrefix(t *testing.T) { } } +// TestConclusionJobCategoriesFilterQuoting verifies that category names containing +// special characters (particularly single quotes) are safely embedded as double-quoted +// YAML scalars, preventing YAML injection via the GH_AW_FAILURE_CATEGORIES_FILTER and +// GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER env vars. +func TestConclusionJobCategoriesFilterQuoting(t *testing.T) { + t.Run("included categories with single quote", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + ReportFailureAsIssue: true, + ReportFailureAsIssueCategories: []string{"it's-a-category", "normal-category"}, + }, + } + + job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{}) + if err != nil { + t.Fatalf("Failed to build conclusion job: %v", err) + } + if job == nil { + t.Fatal("Expected conclusion job to be created") + } + + jobYAML := strings.Join(job.Steps, "") + // Must use double-quoted YAML scalar (produced by %q), not single-quoted + if !strings.Contains(jobYAML, `GH_AW_FAILURE_CATEGORIES_FILTER: "[\"it's-a-category\",\"normal-category\"]"`) { + t.Errorf("Expected GH_AW_FAILURE_CATEGORIES_FILTER to be a safe double-quoted YAML scalar.\nGenerated YAML:\n%s", jobYAML) + } + if strings.Contains(jobYAML, "GH_AW_FAILURE_CATEGORIES_FILTER: '") { + t.Error("GH_AW_FAILURE_CATEGORIES_FILTER must not use single-quoted YAML scalar (injection risk)") + } + }) + + t.Run("excluded categories with single quote", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test Workflow", + SafeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + ReportFailureAsIssue: true, + ReportFailureAsIssueExcludedCategories: []string{"it's-excluded", "other-excluded"}, + }, + } + + job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{}) + if err != nil { + t.Fatalf("Failed to build conclusion job: %v", err) + } + if job == nil { + t.Fatal("Expected conclusion job to be created") + } + + jobYAML := strings.Join(job.Steps, "") + // Must use double-quoted YAML scalar (produced by %q), not single-quoted + if !strings.Contains(jobYAML, `GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER: "[\"it's-excluded\",\"other-excluded\"]"`) { + t.Errorf("Expected GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER to be a safe double-quoted YAML scalar.\nGenerated YAML:\n%s", jobYAML) + } + if strings.Contains(jobYAML, "GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER: '") { + t.Error("GH_AW_FAILURE_EXCLUDED_CATEGORIES_FILTER must not use single-quoted YAML scalar (injection risk)") + } + }) +} + func TestConclusionJobIncludesUsageArtifactSteps(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{