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
5 changes: 5 additions & 0 deletions .changeset/patch-prevent-workflow-run-fork-execution.md

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/ci-doctor.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/dev-hawk.lock.yml

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

17 changes: 11 additions & 6 deletions docs/src/content/docs/guides/security.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ The compiler generates conditions using repository ID comparison (`github.event.

#### workflow_run Trigger Security

Workflows triggered by `workflow_run` events include automatic protections against cross-repository attacks:
Workflows triggered by `workflow_run` events include automatic protections against cross-repository attacks and fork execution:

**Automatic repository validation:**
**Automatic repository and fork validation:**

The compiler automatically injects a repository ID check into the activation job for all workflows using `workflow_run` triggers:
The compiler automatically injects a repository ID check and fork detection into the activation job for all workflows using `workflow_run` triggers:

```yaml wrap
on:
Expand All @@ -110,16 +110,21 @@ on:
types: [completed]
```

This generates a safety condition that prevents execution if the triggering workflow_run is from a different repository:
This generates a safety condition that prevents execution if the triggering workflow_run is from a different repository or from a forked repository:

```yaml wrap
if: >
(user_condition) &&
((github.event_name != 'workflow_run') ||
(github.event.workflow_run.repository.id == github.repository_id))
((github.event.workflow_run.repository.id == github.repository_id) &&
(!github.event.workflow_run.repository.fork)))
```

The safety check combines with user-specified conditions using AND logic and protects all downstream jobs through job dependencies.
The safety check:
- Prevents cross-repository attacks (repository ID mismatch)
- Prevents execution when triggered from forked repositories
- Combines with user-specified conditions using AND logic
- Protects all downstream jobs through job dependencies

**Branch restriction validation:**

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/triggers.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ on:

Workflows with `workflow_run` triggers include automatic security protections:

**Automatic repository validation:** The compiler automatically injects a repository ID check to prevent cross-repository attacks. This safety condition ensures workflows only execute when triggered by workflow runs from the same repository.
**Automatic repository and fork validation:** The compiler automatically injects repository ID and fork checks to prevent cross-repository attacks and fork execution. This safety condition ensures workflows only execute when triggered by workflow runs from the same repository and not from forked repositories.

**Branch restrictions required:** Include `branches` to limit which branch workflows can trigger the event. Without branch restrictions, the compiler emits warnings (or errors in strict mode). This prevents unexpected execution for workflow runs on all branches.

Expand Down
18 changes: 13 additions & 5 deletions pkg/workflow/role_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ func (c *Compiler) hasWorkflowRunTrigger(frontmatter map[string]any) bool {
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
// buildWorkflowRunRepoSafetyCondition generates the if condition to ensure workflow_run is from same repo and not a fork
// The condition uses: (event_name != 'workflow_run') OR (repository IDs match AND not from fork)
// This allows all non-workflow_run events, but requires repository match and fork check for workflow_run events
func (c *Compiler) buildWorkflowRunRepoSafetyCondition() string {
// Check that event is NOT workflow_run
eventNotWorkflowRun := BuildNotEquals(
Expand All @@ -218,8 +218,16 @@ func (c *Compiler) buildWorkflowRunRepoSafetyCondition() string {
BuildPropertyAccess("github.repository_id"),
)

// Combine with OR: allow if NOT workflow_run OR repository matches
combinedCheck := buildOr(eventNotWorkflowRun, repoIDCheck)
// Check that the triggering repository is NOT a fork
notFromForkCheck := &NotNode{
Child: BuildPropertyAccess("github.event.workflow_run.repository.fork"),
}

// Combine repository ID check AND not-from-fork check
repoSafetyCheck := buildAnd(repoIDCheck, notFromForkCheck)

// Combine with OR: allow if NOT workflow_run OR (repository matches AND not fork)
combinedCheck := buildOr(eventNotWorkflowRun, repoSafetyCheck)

// Wrap in ${{ }} for GitHub Actions
return fmt.Sprintf("${{ %s }}", combinedCheck.Render())
Expand Down
87 changes: 87 additions & 0 deletions pkg/workflow/workflow_run_repo_safety_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,19 @@ This workflow runs when CI workflows fail to help diagnose issues.`
if !strings.Contains(lockContentStr, "||") {
t.Error("Expected safety condition to use || operator")
}

// Verify the fork check is present
forkCondition := "github.event.workflow_run.repository.fork"
if !strings.Contains(lockContentStr, forkCondition) {
t.Errorf("Expected fork check to be present in lock file")
t.Logf("Lock file content:\n%s", lockContentStr)
}

// Verify the NOT operator is used for the fork check
if !strings.Contains(lockContentStr, "!(github.event.workflow_run.repository.fork)") {
t.Errorf("Expected NOT operator on fork check")
t.Logf("Lock file content:\n%s", lockContentStr)
}
}

// TestNoWorkflowRunRepoSafetyForPushTrigger tests that push triggers don't get the safety check
Expand Down Expand Up @@ -281,3 +294,77 @@ Do something on push.`
t.Logf("Lock file content:\n%s", lockContentStr)
}
}

// TestWorkflowRunForkCheckPresent verifies that the fork check is present in workflow_run workflows
func TestWorkflowRunForkCheckPresent(t *testing.T) {
workflowContent := `---
on:
workflow_run:
workflows: ["CI"]
types: [completed]
branches: [main]
---

# Test Workflow

Test workflow with workflow_run trigger.`

// Create a temporary directory for the test
tmpDir, err := os.MkdirTemp("", "workflow-run-fork-check-test*")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

// Write the test workflow file
workflowFile := filepath.Join(tmpDir, "test-workflow.md")
err = os.WriteFile(workflowFile, []byte(workflowContent), 0644)
if err != nil {
t.Fatalf("Failed to write workflow file: %v", err)
}

// Compile the workflow
compiler := NewCompiler(false, "", "test")

err = compiler.CompileWorkflow(workflowFile)
if err != nil {
t.Fatalf("Failed to compile workflow: %v", err)
}

// Read the generated lock file
lockFile := strings.TrimSuffix(workflowFile, ".md") + ".lock.yml"
lockContent, err := os.ReadFile(lockFile)
if err != nil {
t.Fatalf("Failed to read lock file: %v", err)
}

lockContentStr := string(lockContent)

// Verify the fork check is present
forkCheck := "github.event.workflow_run.repository.fork"
if !strings.Contains(lockContentStr, forkCheck) {
t.Errorf("Expected fork check to be present in compiled workflow")
t.Logf("Lock file content:\n%s", lockContentStr)
}

// Verify the NOT operator is applied to the fork check
notForkCheck := "!(github.event.workflow_run.repository.fork)"
if !strings.Contains(lockContentStr, notForkCheck) {
t.Errorf("Expected NOT operator on fork check")
t.Logf("Lock file content:\n%s", lockContentStr)
}

// Verify the complete safety condition structure
// Should have: (repo.id == repository_id) && (!repo.fork)
repoIDCheck := "github.event.workflow_run.repository.id == github.repository_id"
if !strings.Contains(lockContentStr, repoIDCheck) {
t.Errorf("Expected repository ID check to be present")
t.Logf("Lock file content:\n%s", lockContentStr)
}

// Verify AND operator combines the checks
if !strings.Contains(lockContentStr, "&&") {
t.Errorf("Expected AND operator to combine repository ID and fork checks")
t.Logf("Lock file content:\n%s", lockContentStr)
}
}
Loading