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
25 changes: 23 additions & 2 deletions pkg/scanner/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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),
})
}
}
Expand Down
51 changes: 46 additions & 5 deletions pkg/scanner/rules_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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")
}
}

Expand Down
12 changes: 12 additions & 0 deletions test/fixtures/script-injection-prtitle.yaml
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions test/fixtures/tag-pinned-branch.yaml
Original file line number Diff line number Diff line change
@@ -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