From 2d4c3341bb4ba151019802685aff89889a19e326 Mon Sep 17 00:00:00 2001 From: Christopher Lusk Date: Fri, 17 Jul 2026 10:03:16 -0400 Subject: [PATCH] fix: credit no-token exec (FG-001) and author-association guards (FG-016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more mitigation-aware downgrades, from prt-hunt triage false positives: - FG-001 Pwn Request: when the job executing fork code has empty permissions (`permissions: {}` — no GITHUB_TOKEN) AND references no secrets, fork code runs but there's nothing to exfiltrate, so it isn't a secret-stealing pwn request. Downgrade to info. (Self-hosted host risk, if any, is still covered by FG-009.) Jobs with a scoped token (e.g. pull-requests: write) are unaffected and stay elevated. - FG-016 Local Action After Untrusted Checkout: when the job's `if:` restricts execution to trusted authors (MEMBER/OWNER/COLLABORATOR) or excludes forks, an external fork can't reach the local action. Downgrade to info. Also teach hasAuthorAssociationGuard the `contains(fromJSON('["MEMBER",...]'), author_association)` allowlist idiom. Verified on the source repos: ionos-cloud/cluster-api-provider-proxmox FG-001 (permissions:{} + pre-screen) and redwoodjs/agent-ci FG-016 (author-association gate) drop critical -> info, while real findings (Dev-Card scoped-token pwn, Letta self-hosted, msviderok compromised action) stay critical. FG-022 was left unchanged: it correctly flags a real compromised action (actions-cool/issues-helper, May 2026 campaign) even when SHA-pinned. Co-Authored-By: Claude Opus 4.8 --- pkg/scanner/rules.go | 57 ++++++++++++++++++- pkg/scanner/rules_fixtures_test.go | 28 +++++++++ .../fixtures/local-action-author-guarded.yaml | 13 +++++ test/fixtures/pwn-request-no-token.yaml | 13 +++++ 4 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/local-action-author-guarded.yaml create mode 100644 test/fixtures/pwn-request-no-token.yaml diff --git a/pkg/scanner/rules.go b/pkg/scanner/rules.go index e90a784..d521e60 100644 --- a/pkg/scanner/rules.go +++ b/pkg/scanner/rules.go @@ -29,6 +29,7 @@ type MitigationAnalysis struct { TrustedRefIsolated bool // Fork checkout to subdir, all executed code from trusted ref PermissionGateJob bool // Upstream job verifies collaborator permissions via API BuildBeforeCheckout bool // Binary compiled from base branch BEFORE PR code checkout + NoCredentialExec bool // Executing job has empty permissions (no token) AND references no secrets Details []string } @@ -197,6 +198,15 @@ func CheckPwnRequest(wf *Workflow) []Finding { "pathspec-checkout isolation: fork content limited to '%s/', executed scripts from trusted ref", runPathspec)) } + // No-credential execution: the executing job has empty permissions (no + // GITHUB_TOKEN) and references no secrets — fork code runs but there is + // nothing to exfiltrate, so it isn't a secret-stealing pwn request. + if mitigation.NoCredentialExec { + severity = SeverityInfo + confidence = ConfidencePatternOnly + mitigated = true + } + // Permission gate job: upstream job verifies collaborator access — internal threat only if mitigation.PermissionGateJob && !mitigated { severity = downgradeBy(severity, 2) @@ -1040,9 +1050,30 @@ func analyzeMitigations(wf *Workflow, job Job, checkoutIdx int, postCheckoutStep m.Details = append(m.Details, "build-before-checkout: binary compiled from base branch before PR checkout") } + // 11. No-credential execution: the job running fork code has empty permissions + // (no GITHUB_TOKEN) AND references no secrets. Fork code still executes, but + // there is no token or secret to steal — so this is not a secret-exfil pwn + // request. (Self-hosted-runner host risk, if any, is covered by FG-009.) + if jobHasNoToken(wf, job) && !postCheckoutAccessesSecrets(postCheckoutSteps) { + m.NoCredentialExec = true + m.Details = append(m.Details, "executing job has empty permissions (no GITHUB_TOKEN) and references no secrets") + } + return m } +// jobHasNoToken reports whether the effective permissions of the job (its own +// block, or the workflow-level block if the job doesn't set one) explicitly +// grant no token access — i.e. `permissions: {}`. This removes the GITHUB_TOKEN +// entirely, so fork code cannot abuse it. +func jobHasNoToken(wf *Workflow, job Job) bool { + perm := job.Permissions + if !perm.Set { + perm = wf.Permissions + } + return perm.Set && !perm.WriteAll && !perm.ReadAll && len(perm.Scopes) == 0 +} + // detectBuildBeforeCheckout detects the defense-in-depth pattern where a binary // is compiled from the trusted base branch before the PR's code is checked out. // The attacker cannot control the binary's code because it was built from base. @@ -2585,17 +2616,29 @@ func CheckLocalActionUntrustedCheckout(wf *Workflow) []Finding { continue } + // Author-association / fork guard: if the job restricts execution to + // trusted authors (MEMBER/OWNER/COLLABORATOR) or excludes forks, an + // arbitrary fork PR cannot reach the local action. Downgrade to info + // (residual: a maintainer-applied label plus a synchronize TOCTOU could + // still run, but not an unauthenticated external fork). + severity := SeverityCritical + guardNote := "" + if hasAuthorAssociationGuard(job) || containsForkGuard(job.If) { + severity = SeverityInfo + guardNote = " (mitigated: job restricts execution to trusted authors / non-forks)" + } + for _, step := range job.Steps[checkoutIdx+1:] { if strings.HasPrefix(step.Uses, "./") { findings = append(findings, Finding{ RuleID: "FG-016", - Severity: SeverityCritical, + Severity: severity, File: wf.Path, Line: step.Line, Message: fmt.Sprintf( "Local Action After Untrusted Checkout: job '%s' uses local action '%s' "+ - "after checking out fork code — attacker controls action.yml", - jobName, step.Uses), + "after checking out fork code — attacker controls action.yml%s", + jobName, step.Uses, guardNote), Details: "Local composite actions (uses: ./) are loaded from the checked-out " + "code. When preceded by a checkout of fork/PR head code, the attacker controls " + "the entire action definition including action.yml, scripts, and Dockerfiles. " + @@ -4318,6 +4361,14 @@ func hasAuthorAssociationGuard(job Job) bool { return true } } + // Allowlist idiom: contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), author_association) + if strings.Contains(g, "fromjson") { + for _, role := range []string{"'member'", "'owner'", "'collaborator'", `"member"`, `"owner"`, `"collaborator"`} { + if strings.Contains(g, role) { + return true + } + } + } return false } diff --git a/pkg/scanner/rules_fixtures_test.go b/pkg/scanner/rules_fixtures_test.go index f18cdc1..a3e0ab3 100644 --- a/pkg/scanner/rules_fixtures_test.go +++ b/pkg/scanner/rules_fixtures_test.go @@ -144,6 +144,20 @@ func TestCheckScriptInjection_Safe(t *testing.T) { } } +// FG-001 must downgrade to info when the executing job has empty permissions +// (no GITHUB_TOKEN) and references no secrets — fork code runs but there is +// nothing to exfiltrate. Regression for ionos-cloud/cluster-api-provider-proxmox. +func TestCheckPwnRequest_NoTokenExec(t *testing.T) { + wf := loadFixture(t, "pwn-request-no-token.yaml") + findings := CheckPwnRequest(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-001 finding, got %d: %v", len(findings), findings) + } + if findings[0].Severity != SeverityInfo { + t.Errorf("expected info severity for no-token/no-secret exec job, got %s", findings[0].Severity) + } +} + // 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) { @@ -1177,6 +1191,20 @@ func TestCheckLocalActionUntrustedCheckout_RunCheckout(t *testing.T) { } } +// FG-016 must downgrade to info when the job restricts execution to trusted +// authors (MEMBER/OWNER/COLLABORATOR via the contains(fromJSON([...])) idiom) — +// an external fork can't reach the local action. Regression for redwoodjs/agent-ci. +func TestCheckLocalActionUntrustedCheckout_AuthorGuarded(t *testing.T) { + wf := loadFixture(t, "local-action-author-guarded.yaml") + findings := CheckLocalActionUntrustedCheckout(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-016 finding, got %d: %v", len(findings), findings) + } + if findings[0].Severity != SeverityInfo { + t.Errorf("expected info severity for author-association-guarded job, got %s", findings[0].Severity) + } +} + // --- FG-027 TOCTOU Label Gate tests --- // Vulnerable pattern: pull_request_target with synchronize, job gated on an diff --git a/test/fixtures/local-action-author-guarded.yaml b/test/fixtures/local-action-author-guarded.yaml new file mode 100644 index 0000000..15c7c68 --- /dev/null +++ b/test/fixtures/local-action-author-guarded.yaml @@ -0,0 +1,13 @@ +name: Guarded Local Action +on: + pull_request_target: + types: [opened, labeled, synchronize] +jobs: + local-action: + if: contains(fromJSON('["MEMBER", "OWNER", "COLLABORATOR"]'), github.event.pull_request.author_association) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + - uses: ./.github/actions/smoke diff --git a/test/fixtures/pwn-request-no-token.yaml b/test/fixtures/pwn-request-no-token.yaml new file mode 100644 index 0000000..5827e34 --- /dev/null +++ b/test/fixtures/pwn-request-no-token.yaml @@ -0,0 +1,13 @@ +name: No Token Pwn +on: + pull_request_target: + types: [opened, synchronize] +jobs: + test: + runs-on: ubuntu-latest + permissions: {} + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + - run: npm install && npm test