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
36 changes: 24 additions & 12 deletions pkg/scanner/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
28 changes: 24 additions & 4 deletions pkg/scanner/rules_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/pwn-request-workflow-run.yaml
Original file line number Diff line number Diff line change
@@ -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