diff --git a/.changeset/patch-prevent-workflow-run-fork-execution.md b/.changeset/patch-prevent-workflow-run-fork-execution.md new file mode 100644 index 00000000000..f3b0343a615 --- /dev/null +++ b/.changeset/patch-prevent-workflow-run-fork-execution.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Prevent workflow_run triggers from executing in forked repositories diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index 95e54b43df0..b398cfa1b31 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -72,7 +72,8 @@ jobs: needs: pre_activation if: > ((needs.pre_activation.outputs.activated == 'true') && (github.event.workflow_run.conclusion == 'failure')) && - ((github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id)) + ((github.event_name != 'workflow_run') || ((github.event.workflow_run.repository.id == github.repository_id) && + (!(github.event.workflow_run.repository.fork)))) runs-on: ubuntu-slim permissions: contents: read diff --git a/.github/workflows/dev-hawk.lock.yml b/.github/workflows/dev-hawk.lock.yml index e6523497e90..1d23e49d5da 100644 --- a/.github/workflows/dev-hawk.lock.yml +++ b/.github/workflows/dev-hawk.lock.yml @@ -60,7 +60,8 @@ jobs: needs: pre_activation if: > ((needs.pre_activation.outputs.activated == 'true') && (github.event.workflow_run.event == 'workflow_dispatch')) && - ((github.event_name != 'workflow_run') || (github.event.workflow_run.repository.id == github.repository_id)) + ((github.event_name != 'workflow_run') || ((github.event.workflow_run.repository.id == github.repository_id) && + (!(github.event.workflow_run.repository.fork)))) runs-on: ubuntu-slim permissions: contents: read diff --git a/docs/src/content/docs/guides/security.md b/docs/src/content/docs/guides/security.md index 07bed6030db..b44a0db70b7 100644 --- a/docs/src/content/docs/guides/security.md +++ b/docs/src/content/docs/guides/security.md @@ -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: @@ -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:** diff --git a/docs/src/content/docs/reference/triggers.md b/docs/src/content/docs/reference/triggers.md index 5ae8d39f3be..07bb4775d13 100644 --- a/docs/src/content/docs/reference/triggers.md +++ b/docs/src/content/docs/reference/triggers.md @@ -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. diff --git a/pkg/workflow/role_checks.go b/pkg/workflow/role_checks.go index 8eca1fb35f4..7381ff5b3ed 100644 --- a/pkg/workflow/role_checks.go +++ b/pkg/workflow/role_checks.go @@ -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( @@ -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()) diff --git a/pkg/workflow/workflow_run_repo_safety_test.go b/pkg/workflow/workflow_run_repo_safety_test.go index 9bfa225c18b..25ae014e50e 100644 --- a/pkg/workflow/workflow_run_repo_safety_test.go +++ b/pkg/workflow/workflow_run_repo_safety_test.go @@ -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 @@ -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) + } +}