diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index 638240a8bb1..7b524a1d831 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -231,8 +231,8 @@ func (c *Compiler) validateBareModeSupport(frontmatter map[string]any, engine Co } } -// validateWorkflowRunBranches validates that workflow_run triggers include branch restrictions -// This is a security best practice to avoid running on all branches +// validateWorkflowRunBranches validates workflow_run trigger requirements. +// It enforces required workflows and branch restrictions guidance. func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markdownPath string) error { // Fast path: skip expensive YAML parsing when the On field cannot possibly contain // a workflow_run trigger (including when it is empty). This avoids yaml.Unmarshal @@ -241,7 +241,7 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd return nil } - agentValidationLog.Print("Validating workflow_run triggers for branch restrictions") + agentValidationLog.Print("Validating workflow_run trigger requirements") // Parse the On field as YAML to check for workflow_run // The On field is a YAML string that starts with "on:" key @@ -272,13 +272,29 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd return nil } - // Check if workflow_run has branches field workflowRunMap, isMap := workflowRunVal.(map[string]any) if !isMap { // workflow_run is not a map (unusual), skip validation return nil } + // workflow_run requires workflows to be present and non-empty. + workflowsVal, hasWorkflows := workflowRunMap["workflows"] + if !hasWorkflows || !hasNonEmptyWorkflowRunWorkflows(workflowsVal) { + message := `workflow_run trigger must include a non-empty workflows field. + +GitHub Actions requires on.workflow_run.workflows to reference at least one workflow. +Without it, the compiled workflow is invalid and will be rejected. + +Suggested fix: +on: + workflow_run: + workflows: ["your-workflow"] + types: [completed]` + return formatCompilerError(markdownPath, "error", message, nil) + } + + // Check if workflow_run has branches field _, hasBranches := workflowRunMap["branches"] if hasBranches { // Has branch restrictions, validation passed @@ -313,3 +329,36 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd return nil } + +// hasNonEmptyWorkflowRunWorkflows returns true when workflow_run.workflows +// includes at least one non-empty workflow name. +// +// Supported types: +// - string: valid when non-empty after trimming whitespace +// - []string: valid when any item is non-empty after trimming whitespace +// - []any: valid when any string item is non-empty after trimming whitespace +// +// For all other types, it returns false. +func hasNonEmptyWorkflowRunWorkflows(v any) bool { + switch workflows := v.(type) { + case string: + return strings.TrimSpace(workflows) != "" + case []string: + for _, workflow := range workflows { + if strings.TrimSpace(workflow) != "" { + return true + } + } + return false + case []any: + for _, workflow := range workflows { + s, ok := workflow.(string) + if ok && strings.TrimSpace(s) != "" { + return true + } + } + return false + default: + return false + } +} diff --git a/pkg/workflow/expression_extraction_test.go b/pkg/workflow/expression_extraction_test.go index b2256fef7a4..ade8d34a4bb 100644 --- a/pkg/workflow/expression_extraction_test.go +++ b/pkg/workflow/expression_extraction_test.go @@ -651,7 +651,7 @@ func TestExtractTerminalSubExpressions(t *testing.T) { want: []string{"steps.foo.outputs.bar"}, }, { - name: "simple expression (no operators) — single qualifying token is returned", + name: "simple expression (no operators) — single qualifying token is returned", // Note: ExtractExpressions guards the call with !simpleIdentifierRegex, // so this case never arises in production; but the function is correct regardless. content: "steps.sanitized.outputs.text", diff --git a/pkg/workflow/workflow_run_validation_test.go b/pkg/workflow/workflow_run_validation_test.go index 5b46f9d591b..37a3dfa2750 100644 --- a/pkg/workflow/workflow_run_validation_test.go +++ b/pkg/workflow/workflow_run_validation_test.go @@ -115,6 +115,68 @@ Test workflow content.`, expectWarning: false, warningCount: 0, }, + { + name: "workflow_run without workflows - should error", + frontmatter: `--- +strict: false +on: + workflow_run: + types: [completed] +tools: + github: false +--- + +# Workflow Run Without Workflows +Test workflow content.`, + filename: "workflow-run-no-workflows.md", + strictMode: false, + expectError: true, + expectWarning: false, + errorContains: "workflow_run trigger must include a non-empty workflows field", + warningCount: 0, + }, + { + name: "workflow_run with empty workflows - strict mode - should error", + frontmatter: `--- +on: + workflow_run: + workflows: [] + types: [completed] +tools: + github: + toolsets: [repos] +--- + +# Workflow Run Empty Workflows +Test workflow content.`, + filename: "workflow-run-empty-workflows.md", + strictMode: true, + expectError: true, + expectWarning: false, + errorContains: "workflow_run trigger must include a non-empty workflows field", + warningCount: 0, + }, + { + name: "workflow_run with whitespace workflows - non-strict mode - should error", + frontmatter: `--- +strict: false +on: + workflow_run: + workflows: [" ", ""] + types: [completed] +tools: + github: false +--- + +# Workflow Run Whitespace Workflows +Test workflow content.`, + filename: "workflow-run-whitespace-workflows.md", + strictMode: false, + expectError: true, + expectWarning: false, + errorContains: "workflow_run trigger must include a non-empty workflows field", + warningCount: 0, + }, { name: "no workflow_run - should pass", frontmatter: `---