Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions pkg/workflow/agent_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion pkg/workflow/expression_extraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
62 changes: 62 additions & 0 deletions pkg/workflow/workflow_run_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `---
Expand Down