diff --git a/pkg/scanner/rules.go b/pkg/scanner/rules.go index 016fab0..e90a784 100644 --- a/pkg/scanner/rules.go +++ b/pkg/scanner/rules.go @@ -568,6 +568,13 @@ func CheckScriptInjection(wf *Workflow) []Finding { if isGHALoggingOnly(step.Run, expr) { severity = SeverityInfo detail = " (logging only — echo/printf context, not directly exploitable)" + } else if isDispatchInputExpr(expr) { + // workflow_dispatch/workflow_call inputs are populated only via + // a write-gated manual dispatch or an internal caller — never + // directly from an untrusted trigger. Real injection, but low + // external reach; drop a tier from high. + severity = SeverityMedium + detail = " (workflow_dispatch/workflow_call input — requires trigger access; verify reusable-workflow callers don't forward untrusted data)" } findings = append(findings, Finding{ @@ -609,6 +616,14 @@ func isGHALoggingOnly(run string, expr string) bool { return true } +// isDispatchInputExpr reports whether a dangerous expression is a +// workflow_dispatch / workflow_call input. These contexts are only populated by +// a manual dispatch (which requires write access) or an internal caller, so the +// injection is not reachable from an untrusted trigger. +func isDispatchInputExpr(expr string) bool { + return expr == "inputs." || expr == "github.event.inputs." +} + var shaPattern = regexp.MustCompile(`^[a-f0-9]{40}$`) // CheckTagPinning detects third-party actions referenced by tag instead of @@ -628,9 +643,15 @@ func CheckTagPinning(wf *Workflow) []Finding { continue // SHA-pinned, safe } + // Supply-chain hygiene, not an acute exploit: a moving ref lets the + // action change under you, but nothing is directly exploitable today. + // Cap at medium. Branch refs (main/master/dev) move on every push and + // are worse than a version tag, so they keep the ceiling and a note; + // first-party actions/* are GitHub-controlled and stay info. severity := SeverityMedium + note := "" if ref == "main" || ref == "master" || ref == "dev" { - severity = SeverityHigh + note = " — mutable branch ref" } if strings.HasPrefix(action, "actions/") { severity = SeverityInfo @@ -641,7 +662,7 @@ func CheckTagPinning(wf *Workflow) []Finding { Severity: severity, File: wf.Path, Line: step.Line, - Message: fmt.Sprintf("Tag Pinning: %s@%s (use SHA instead)", action, ref), + Message: fmt.Sprintf("Tag Pinning: %s@%s (use SHA instead)%s", action, ref, note), }) } } diff --git a/pkg/scanner/rules_fixtures_test.go b/pkg/scanner/rules_fixtures_test.go index f41b2a1..f18cdc1 100644 --- a/pkg/scanner/rules_fixtures_test.go +++ b/pkg/scanner/rules_fixtures_test.go @@ -144,6 +144,19 @@ func TestCheckScriptInjection_Safe(t *testing.T) { } } +// Attacker-controllable expressions (PR title on pull_request_target, used in a +// non-echo command) must stay high — the dispatch-input downgrade must not touch them. +func TestCheckScriptInjection_PRTitleStaysHigh(t *testing.T) { + wf := loadFixture(t, "script-injection-prtitle.yaml") + findings := CheckScriptInjection(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-002 finding, got %d: %v", len(findings), findings) + } + if findings[0].Severity != SeverityHigh { + t.Errorf("expected high severity for pull_request.title injection, got %s", findings[0].Severity) + } +} + func TestCheckTagPinning(t *testing.T) { wf := loadFixture(t, "tag-pinned.yaml") findings := CheckTagPinning(wf) @@ -170,6 +183,30 @@ func TestCheckTagPinning(t *testing.T) { } } +// Branch-pinned third-party actions (@main) are supply-chain hygiene, not an +// acute exploit — capped at medium (down from high), with a mutable-branch note. +// First-party actions/* stay info regardless of ref. +func TestCheckTagPinning_BranchRefIsMedium(t *testing.T) { + wf := loadFixture(t, "tag-pinned-branch.yaml") + findings := CheckTagPinning(wf) + + var branchPin *Finding + for i := range findings { + if strings.Contains(findings[i].Message, "some-org/some-action") { + branchPin = &findings[i] + } + } + if branchPin == nil { + t.Fatalf("expected a finding for some-org/some-action@main, got %v", findings) + } + if branchPin.Severity != SeverityMedium { + t.Errorf("expected medium severity for third-party branch pin, got %s", branchPin.Severity) + } + if !strings.Contains(branchPin.Message, "mutable branch ref") { + t.Errorf("expected mutable-branch note in message, got: %s", branchPin.Message) + } +} + func TestCheckBroadPermissions(t *testing.T) { wf := loadFixture(t, "broad-perms.yaml") findings := CheckBroadPermissions(wf) @@ -914,18 +951,22 @@ func TestCheckScriptInjection_DispatchInputs(t *testing.T) { if len(findings) < 2 { t.Fatalf("expected at least 2 injection findings for dispatch inputs, got %d", len(findings)) } - // At least one should be high (the deploy.sh line), others may be info (echo lines) - hasHigh := false + // Dispatch/call inputs are write-gated, so the executed (non-echo) deploy.sh + // line is medium, not high; the echo lines stay info. Nothing should be high. + hasMedium := false for _, f := range findings { if f.RuleID != "FG-002" { t.Errorf("expected FG-002, got %s", f.RuleID) } if f.Severity == SeverityHigh { - hasHigh = true + t.Errorf("dispatch-input injection should not be high, got high for: %s", f.Message) + } + if f.Severity == SeverityMedium { + hasMedium = true } } - if !hasHigh { - t.Errorf("expected at least one high severity finding (deploy.sh context)") + if !hasMedium { + t.Errorf("expected the executed dispatch-input line to be medium") } } diff --git a/test/fixtures/script-injection-prtitle.yaml b/test/fixtures/script-injection-prtitle.yaml new file mode 100644 index 0000000..671c1f1 --- /dev/null +++ b/test/fixtures/script-injection-prtitle.yaml @@ -0,0 +1,12 @@ +name: PR Title Injection +on: + pull_request_target: + types: [opened] +jobs: + label: + runs-on: ubuntu-latest + steps: + - name: Use title + run: | + TITLE="${{ github.event.pull_request.title }}" + grep "$TITLE" data.txt diff --git a/test/fixtures/tag-pinned-branch.yaml b/test/fixtures/tag-pinned-branch.yaml new file mode 100644 index 0000000..5c8f523 --- /dev/null +++ b/test/fixtures/tag-pinned-branch.yaml @@ -0,0 +1,8 @@ +name: Branch Pinned Action +on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: some-org/some-action@main