diff --git a/pkg/scanner/rules.go b/pkg/scanner/rules.go index c951a09..54ee790 100644 --- a/pkg/scanner/rules.go +++ b/pkg/scanner/rules.go @@ -92,17 +92,24 @@ var readOnlyCommands = []string{ // ${{ github.event.issue.number }}` on a `/command`) run fork code in the base // context just like pull_request_target. func CheckPwnRequest(wf *Workflow) []Finding { - if !wf.On.PullRequestTarget && !wf.On.IssueComment { + if !wf.On.PullRequestTarget && !wf.On.IssueComment && !wf.On.WorkflowRun { return nil } - triggerLabel := "pull_request_target" - switch { - case wf.On.PullRequestTarget && wf.On.IssueComment: - triggerLabel = "pull_request_target/issue_comment" - case wf.On.IssueComment: - triggerLabel = "issue_comment" + // All three are privileged triggers that run in the base-repo context with + // secrets; workflow_run additionally can check out the triggering PR's head + // (github.event.workflow_run.head_sha). + var trigs []string + if wf.On.PullRequestTarget { + trigs = append(trigs, "pull_request_target") + } + if wf.On.IssueComment { + trigs = append(trigs, "issue_comment") } + if wf.On.WorkflowRun { + trigs = append(trigs, "workflow_run") + } + triggerLabel := strings.Join(trigs, "/") var findings []Finding for _, job := range wf.Jobs { @@ -825,10 +832,12 @@ func CheckForkPRCodeExec(wf *Workflow) []Finding { continue } - severity := SeverityMedium - if postCheckoutAccessesSecrets(postCheckoutSteps) { - severity = SeverityHigh - } + // On `pull_request` (not `_target`), fork PRs run with a read-only token + // and NO repository secrets — secret references resolve to empty. So this + // is arbitrary code execution on an ephemeral runner with nothing to + // steal: low severity. (Self-hosted-runner host risk is FG-009; cache + // poisoning is FG-010; secret-bearing pwns are FG-001 on _target.) + severity := SeverityLow confidence := ConfidenceConfirmed if !execResult.Confirmed { @@ -844,7 +853,7 @@ func CheckForkPRCodeExec(wf *Workflow) []Finding { File: wf.Path, Line: checkoutLine, Message: msg, - Details: "Trigger: pull_request (read-only token from forks, but arbitrary code execution on runner)", + Details: "Trigger: pull_request — fork PRs get a read-only token and no secrets, so this is code execution on an ephemeral runner with nothing to exfiltrate (cache-poisoning aside).", }) } return findings @@ -1747,6 +1756,9 @@ func refPointsToPRHead(ref string) bool { // issue_comment context: a checkout ref built from the PR number, e.g. // `refs/pull/${{ github.event.issue.number }}/head`. "github.event.issue.number", + // workflow_run context: the triggering (fork) PR's head. + "github.event.workflow_run.head_sha", + "github.event.workflow_run.head_branch", } for _, d := range dangerous { if strings.Contains(ref, d) { diff --git a/pkg/scanner/rules_fixtures_test.go b/pkg/scanner/rules_fixtures_test.go index 9530fdb..dd15f91 100644 --- a/pkg/scanner/rules_fixtures_test.go +++ b/pkg/scanner/rules_fixtures_test.go @@ -144,6 +144,23 @@ func TestCheckScriptInjection_Safe(t *testing.T) { } } +// FG-001 must cover workflow_run: a workflow that checks out the triggering PR's +// head (github.event.workflow_run.head_sha) and runs it is a pwn request. +func TestCheckPwnRequest_WorkflowRun(t *testing.T) { + wf := loadFixture(t, "pwn-request-workflow-run.yaml") + findings := CheckPwnRequest(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-001 finding for workflow_run checkout+exec, got %d: %v", len(findings), findings) + } + f := findings[0] + if f.Severity != SeverityCritical { + t.Errorf("expected critical for ungated workflow_run head checkout+exec, got %s", f.Severity) + } + if !strings.Contains(f.Message, "workflow_run") { + t.Errorf("expected workflow_run in message, got: %s", f.Message) + } +} + // FG-001 must now cover issue_comment ChatOps: an ungated `/command` job that // `gh pr checkout`s the PR and runs it is a pwn request, same as pull_request_target. func TestCheckPwnRequest_IssueComment(t *testing.T) { @@ -409,8 +426,9 @@ func TestCheckForkPRCodeExec(t *testing.T) { if f.RuleID != "FG-006" { t.Errorf("expected rule FG-006, got %s", f.RuleID) } - if f.Severity != SeverityMedium { - t.Errorf("expected medium severity (no secrets), got %s", f.Severity) + // pull_request fork exec: low (read-only token, no fork secrets, ephemeral runner). + if f.Severity != SeverityLow { + t.Errorf("expected low severity for pull_request fork exec, got %s", f.Severity) } } @@ -422,8 +440,10 @@ func TestCheckForkPRCodeExec_WithSecrets(t *testing.T) { t.Fatalf("expected 1 finding, got %d", len(findings)) } f := findings[0] - if f.Severity != SeverityHigh { - t.Errorf("expected high severity (secrets on build step), got %s", f.Severity) + // Even with secret refs, on pull_request those resolve to empty for fork PRs, + // so this stays low — the tuning that stops FG-006 over-flagging as high. + if f.Severity != SeverityLow { + t.Errorf("expected low severity (secrets empty for fork PRs on pull_request), got %s", f.Severity) } } diff --git a/test/fixtures/pwn-request-workflow-run.yaml b/test/fixtures/pwn-request-workflow-run.yaml new file mode 100644 index 0000000..e8d707d --- /dev/null +++ b/test/fixtures/pwn-request-workflow-run.yaml @@ -0,0 +1,13 @@ +name: Process Build +on: + workflow_run: + workflows: ["Build"] + types: [completed] +jobs: + process: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.workflow_run.head_sha }} + - run: npm install