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
37 changes: 37 additions & 0 deletions pkg/scanner/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,13 @@ func isTrustedRef(ref string) bool {
if strings.Contains(lower, "pull_request.base.") {
return true
}
// github.workflow_sha = the commit of the workflow file (the base/trusted ref
// on pull_request_target), and github.sha = the base commit on
// pull_request_target. Both check out trusted code, unlike ...head.sha (fork).
// "github.sha" does not appear as a substring of "...head.sha", so this is safe.
if strings.Contains(lower, "workflow_sha") || strings.Contains(lower, "github.sha") {
return true
}
return false
}

Expand Down Expand Up @@ -2610,6 +2617,30 @@ func CheckCurlPipeBash(wf *Workflow) []Finding {
return findings
}

// localActionUnderTrustedCheckout reports whether a `uses: ./<dir>/...` local
// action resolves to a directory that a checkout of a trusted ref populated
// (e.g. `actions/checkout` at `github.workflow_sha` into `path: <dir>`), so the
// action definition comes from trusted code, not the fork.
func localActionUnderTrustedCheckout(job Job, uses string) bool {
p := strings.TrimPrefix(uses, "./")
seg := p
if i := strings.Index(p, "/"); i != -1 {
seg = p[:i]
}
if seg == "" || seg == "." {
return false
}
for _, step := range job.Steps {
if !isCheckoutAction(step.Uses) {
continue
}
if normalizePathspec(step.With["path"]) == seg && isTrustedRef(step.With["ref"]) {
return true
}
}
return false
}

// CheckLocalActionUntrustedCheckout detects local action usage after fork checkout (FG-016).
func CheckLocalActionUntrustedCheckout(wf *Workflow) []Finding {
if !wf.On.PullRequestTarget && !wf.On.IssueComment && !wf.On.WorkflowRun {
Expand Down Expand Up @@ -2660,6 +2691,12 @@ func CheckLocalActionUntrustedCheckout(wf *Workflow) []Finding {

for _, step := range job.Steps[checkoutIdx+1:] {
if strings.HasPrefix(step.Uses, "./") {
// The local action lives under a subdir populated by a checkout of
// a trusted ref (e.g. actions checked out at github.workflow_sha
// into path/), so its definition is not attacker-controlled.
if localActionUnderTrustedCheckout(job, step.Uses) {
continue
}
findings = append(findings, Finding{
RuleID: "FG-016",
Severity: severity,
Expand Down
11 changes: 11 additions & 0 deletions pkg/scanner/rules_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,17 @@ func TestCheckLocalActionUntrustedCheckout_TrustedRefIsolated(t *testing.T) {
}
}

// A local action loaded from a subdir populated by a checkout of the trusted
// github.workflow_sha (not the fork) is not attacker-controlled. Regression for
// leanprover-community/mathlib4's PR_summary.yml.
func TestCheckLocalActionUntrustedCheckout_WorkflowShaDir(t *testing.T) {
wf := loadFixture(t, "local-action-workflow-sha.yaml")
findings := CheckLocalActionUntrustedCheckout(wf)
if len(findings) != 0 {
t.Fatalf("expected 0 findings for local action under trusted workflow_sha checkout, got %d: %v", len(findings), findings)
}
}

// A run block that fetches the PR head to a named ref but never checks it out
// leaves fork code off the working tree; a local action loaded afterward comes
// from the base clone, not the fork. No FG-016 finding.
Expand Down
23 changes: 23 additions & 0 deletions pkg/scanner/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,29 @@ func TestCheckLifecycleInstallBeforeCredentialedOperation_SelfUpdatePlusDeps(t *
}
}

func TestIsTrustedRef(t *testing.T) {
trusted := []string{
"main", "master", "${{ github.base_ref }}",
"${{ github.event.pull_request.base.sha }}",
"${{ github.workflow_sha }}", "${{ github.sha }}",
}
for _, r := range trusted {
if !isTrustedRef(r) {
t.Errorf("expected %q to be trusted", r)
}
}
untrusted := []string{
"${{ github.event.pull_request.head.sha }}",
"${{ github.event.pull_request.head.ref }}",
"${{ github.head_ref }}",
}
for _, r := range untrusted {
if isTrustedRef(r) {
t.Errorf("expected %q to NOT be trusted (fork head)", r)
}
}
}

// --- AllRules completeness ---

func TestAllRules_IncludesNewRules(t *testing.T) {
Expand Down
20 changes: 20 additions & 0 deletions test/fixtures/local-action-workflow-sha.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: PR Summary
on: pull_request_target
permissions:
contents: read
pull-requests: write
jobs:
summary:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
path: pr-branch
persist-credentials: false
- uses: actions/checkout@v4
with:
ref: ${{ github.workflow_sha }}
sparse-checkout: .github/actions
path: workflow-actions
- uses: ./workflow-actions/.github/actions/get-ci