Skip to content
3 changes: 2 additions & 1 deletion .github/workflows/ci-doctor.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion .github/workflows/dev-hawk.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion .github/workflows/smoke-detector.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 26 additions & 4 deletions pkg/workflow/compiler_jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error {
needsPermissionCheck := c.needsRoleCheck(data, frontmatter)
hasStopTime := data.StopTime != ""

// Determine if we need to add workflow_run repository safety check
// Add the check if the agentic workflow declares a workflow_run trigger
// This prevents cross-repository workflow_run attacks
var workflowRunRepoSafety string
if c.hasWorkflowRunTrigger(frontmatter) {
workflowRunRepoSafety = c.buildWorkflowRunRepoSafetyCondition()
}

// Build pre-activation job if needed (combines membership checks, stop-time validation, and command position check)
var preActivationJobCreated bool
hasCommandTrigger := data.Command != ""
Expand All @@ -61,7 +69,7 @@ func (c *Compiler) buildJobs(data *WorkflowData, markdownPath string) error {
var activationJobCreated bool

if c.isActivationJobNeeded() {
activationJob, err := c.buildActivationJob(data, preActivationJobCreated)
activationJob, err := c.buildActivationJob(data, preActivationJobCreated, workflowRunRepoSafety)
if err != nil {
return fmt.Errorf("failed to build activation job: %w", err)
}
Expand Down Expand Up @@ -448,9 +456,13 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec
"activated": activatedExpression,
}

// Pre-activation job uses the user's original if condition (data.If)
// The workflow_run safety check is NOT applied here - it's only on the activation job
jobIfCondition := data.If

job := &Job{
Name: constants.PreActivationJobName,
If: data.If, // Use the existing condition (which may include alias checks)
If: jobIfCondition,
RunsOn: c.formatSafeOutputsRunsOn(data.SafeOutputs),
Permissions: permissions,
Steps: steps,
Expand All @@ -461,7 +473,8 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec
}

// buildActivationJob creates the preamble activation job that acts as a barrier for runtime conditions
func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreated bool) (*Job, error) {
// The workflow_run repository safety check is applied exclusively to this job
func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreated bool, workflowRunRepoSafety string) (*Job, error) {
outputs := map[string]string{}
var steps []string

Expand Down Expand Up @@ -551,10 +564,16 @@ func (c *Compiler) buildActivationJob(data *WorkflowData, preActivationJobCreate
activationCondition = activatedExpr.Render()
}
} else {
// No pre-activation check needed
// No pre-activation check needed, use user's if condition
activationCondition = data.If
}

// Apply workflow_run repository safety check exclusively to activation job
// This check is combined with any existing activation condition
if workflowRunRepoSafety != "" {
activationCondition = c.combineJobIfConditions(activationCondition, workflowRunRepoSafety)
}

// Set permissions - add reaction permissions if reaction is configured and not "none"
var permissions string
if data.AIReaction != "" && data.AIReaction != "none" {
Expand Down Expand Up @@ -594,6 +613,9 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) (
if activationJobCreated {
jobCondition = "" // Main job depends on activation job, so no need for inline condition
}

// Note: workflow_run repository safety check is applied exclusively to activation job

// Permission checks are now handled by the separate check_membership job
// No role checks needed in the main job

Expand Down
3 changes: 3 additions & 0 deletions pkg/workflow/compiler_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ func (c *Compiler) generateYAML(data *WorkflowData, markdownPath string) (string
yaml.WriteString(fmt.Sprintf("name: \"%s\"\n", data.Name))
yaml.WriteString(data.On + "\n\n")

// Note: GitHub Actions doesn't support workflow-level if conditions
// The workflow_run safety check is added to individual jobs instead

// Add permissions if present
if data.Permissions != "" {
yaml.WriteString(data.Permissions + "\n\n")
Expand Down
70 changes: 70 additions & 0 deletions pkg/workflow/role_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,73 @@ func (c *Compiler) hasSafeEventsOnly(data *WorkflowData, frontmatter map[string]
// For command workflows, they are not considered "safe only"
return false
}

// hasWorkflowRunTrigger checks if the agentic workflow's frontmatter declares a workflow_run trigger
func (c *Compiler) hasWorkflowRunTrigger(frontmatter map[string]any) bool {
if frontmatter == nil {
return false
}

// Check the "on" section in frontmatter
if onValue, exists := frontmatter["on"]; exists {
// Handle map format (most common)
if onMap, ok := onValue.(map[string]any); ok {
_, hasWorkflowRun := onMap["workflow_run"]
return hasWorkflowRun
}
// Handle string format
if onStr, ok := onValue.(string); ok {
return onStr == "workflow_run"
}
}

return false
}

// buildWorkflowRunRepoSafetyCondition generates the if condition to ensure workflow_run is from same repo
// The condition uses: (event_name != 'workflow_run') OR (repository IDs match)
// This allows all non-workflow_run events, but requires repository match for workflow_run events
func (c *Compiler) buildWorkflowRunRepoSafetyCondition() string {
// Check that event is NOT workflow_run
eventNotWorkflowRun := BuildNotEquals(
BuildPropertyAccess("github.event_name"),
BuildStringLiteral("workflow_run"),
)

// Check that repository IDs match
repoIDCheck := BuildEquals(
BuildPropertyAccess("github.event.workflow_run.repository.id"),
BuildPropertyAccess("github.repository_id"),
)

// Combine with OR: allow if NOT workflow_run OR repository matches
combinedCheck := buildOr(eventNotWorkflowRun, repoIDCheck)

// Wrap in ${{ }} for GitHub Actions
return fmt.Sprintf("${{ %s }}", combinedCheck.Render())
}

// combineJobIfConditions combines an existing if condition with workflow_run repository safety check
// Returns the combined condition, or just one of them if the other is empty
func (c *Compiler) combineJobIfConditions(existingCondition, workflowRunRepoSafety string) string {
// If no workflow_run safety check needed, return existing condition
if workflowRunRepoSafety == "" {
return existingCondition
}

// If no existing condition, return just the workflow_run safety check
if existingCondition == "" {
return workflowRunRepoSafety
}

// Both conditions present - combine them with AND
// Strip ${{ }} wrapper from existingCondition if present
unwrappedExisting := stripExpressionWrapper(existingCondition)
unwrappedSafety := stripExpressionWrapper(workflowRunRepoSafety)

existingNode := &ExpressionNode{Expression: unwrappedExisting}
safetyNode := &ExpressionNode{Expression: unwrappedSafety}

combinedExpr := buildAnd(existingNode, safetyNode)
return combinedExpr.Render()
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combined expression is returned without the ${{ }} wrapper. While this works for long expressions that get converted to multiline format (which doesn't need the wrapper), short combined expressions will be invalid when rendered as single-line if conditions. The function should re-wrap the result: return fmt.Sprintf(\"${{ %s }}\", combinedExpr.Render())

Suggested change
return combinedExpr.Render()
return fmt.Sprintf("${{ %s }}", combinedExpr.Render())

Copilot uses AI. Check for mistakes.
}
16 changes: 13 additions & 3 deletions pkg/workflow/task_job_if_condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,19 @@ Check the failed workflow and provide analysis.`
t.Error("Expected activation job to be present in generated workflow")
}

// Test 2: Verify activation job has the if condition
if !strings.Contains(lockContentStr, "if: ${{ github.event.workflow_run.conclusion == 'failure' }}") {
t.Error("Expected activation job to have the if condition")
// Test 2: Verify activation job has the user's if condition (may be combined with other conditions)
if !strings.Contains(lockContentStr, "github.event.workflow_run.conclusion == 'failure'") {
t.Error("Expected activation job to include the user's if condition")
}

// Test 2b: Verify activation job has the workflow_run event_name check (using !=)
if !strings.Contains(lockContentStr, "github.event_name != 'workflow_run'") {
t.Error("Expected activation job to include the event_name check with != operator")
}

// Test 2c: Verify activation job has the workflow_run repository safety check
if !strings.Contains(lockContentStr, "github.event.workflow_run.repository.id == github.repository_id") {
t.Error("Expected activation job to include the workflow_run repository safety check")
}

// Test 3: Verify activation job has steps (specifically the dummy step)
Expand Down
Loading
Loading