diff --git a/pkg/scanner/rules.go b/pkg/scanner/rules.go index d521e60..9b0d995 100644 --- a/pkg/scanner/rules.go +++ b/pkg/scanner/rules.go @@ -17,20 +17,21 @@ type ExecutionAnalysis struct { // MitigationAnalysis captures defensive controls detected on a workflow/job. type MitigationAnalysis struct { - LabelGated bool - EnvironmentGated bool - MaintainerCheck bool - ForkGuard bool - ActorGuard bool // Job if: restricts execution to specific bot actor(s) - ActorGuardHuman bool // Job if: restricts to specific human actor(s) (weaker) - NeedsGate bool // Job depends on upstream job with environment/fork gate - TokenBlanked bool - PathIsolated bool // Fork code checked out to subdirectory, no direct execution - 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 + LabelGated bool + EnvironmentGated bool + MaintainerCheck bool + ForkGuard bool + ActorGuard bool // Job if: restricts execution to specific bot actor(s) + ActorGuardHuman bool // Job if: restricts to specific human actor(s) (weaker) + NeedsGate bool // Job depends on upstream job with environment/fork gate + TokenBlanked bool + PathIsolated bool // Fork code checked out to subdirectory, no direct execution + 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 + AuthorAssociationGate bool // Job if: restricts execution by author_association (trusted commenters/contributors) + Details []string } // Build tools that definitely execute code from the working directory. @@ -85,13 +86,24 @@ var readOnlyCommands = []string{ "echo", "printf", } -// CheckPwnRequest detects pull_request_target workflows that checkout PR head -// code with post-checkout execution analysis (FG-001). +// CheckPwnRequest detects pull_request_target and issue_comment workflows that +// checkout PR head code with post-checkout execution analysis (FG-001). Both are +// privileged triggers: issue_comment ChatOps bots (e.g. `gh pr checkout +// ${{ 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 { + if !wf.On.PullRequestTarget && !wf.On.IssueComment { 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" + } + var findings []Finding for _, job := range wf.Jobs { // Skip jobs scoped to pull_request only — they never run in @@ -213,13 +225,20 @@ func CheckPwnRequest(wf *Workflow) []Finding { mitigated = true } + // Author-association gate: only trusted commenters/contributors can + // trigger — an arbitrary external commenter can't reach the checkout. + if mitigation.AuthorAssociationGate && !mitigated { + severity = downgradeBy(severity, 2) + mitigated = true + } + // Path isolation adjusts confidence, not severity if mitigation.PathIsolated && confidence == ConfidenceConfirmed { confidence = ConfidencePatternOnly mitigated = true } - msg := fmt.Sprintf("Pwn Request: pull_request_target with fork checkout [%s]", confidence) + msg := fmt.Sprintf("Pwn Request: %s with fork checkout [%s]", triggerLabel, confidence) if execResult.Detail != "" { msg += " — " + execResult.Detail } @@ -229,8 +248,8 @@ func CheckPwnRequest(wf *Workflow) []Finding { permDesc := describePermissions(wf.Permissions, job.Permissions) details := fmt.Sprintf( - "Trigger: pull_request_target, Checkout ref: %s, Permissions: %s, Execution: %s", - checkoutRef, permDesc, confidence, + "Trigger: %s, Checkout ref: %s, Permissions: %s, Execution: %s", + triggerLabel, checkoutRef, permDesc, confidence, ) findings = append(findings, Finding{ @@ -927,6 +946,14 @@ func analyzeMitigations(wf *Workflow, job Job, checkoutIdx int, postCheckoutStep m.ActorGuardHuman = true m.Details = append(m.Details, fmt.Sprintf("job if: restricts to specific actor(s) (%s)", truncate(job.If, 80))) } + // author_association gate — common on issue_comment ChatOps: only + // trusted commenters (MEMBER/OWNER/COLLABORATOR) or known contributors + // can trigger. Strong for issue_comment (author is checked at fire time, + // no re-run TOCTOU). + if hasAuthorAssociationGuard(job) { + m.AuthorAssociationGate = true + m.Details = append(m.Details, "job if: restricts execution by author_association") + } } // 3. Check environment protection @@ -1710,6 +1737,9 @@ func refPointsToPRHead(ref string) bool { "github.event.pull_request.head.sha", "github.event.pull_request.head.ref", "github.head_ref", + // issue_comment context: a checkout ref built from the PR number, e.g. + // `refs/pull/${{ github.event.issue.number }}/head`. + "github.event.issue.number", } for _, d := range dangerous { if strings.Contains(ref, d) { diff --git a/pkg/scanner/rules_fixtures_test.go b/pkg/scanner/rules_fixtures_test.go index a3e0ab3..a060b8d 100644 --- a/pkg/scanner/rules_fixtures_test.go +++ b/pkg/scanner/rules_fixtures_test.go @@ -144,6 +144,40 @@ func TestCheckScriptInjection_Safe(t *testing.T) { } } +// 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) { + wf := loadFixture(t, "pwn-request-issue-comment.yaml") + findings := CheckPwnRequest(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-001 finding for issue_comment ChatOps, got %d: %v", len(findings), findings) + } + f := findings[0] + if f.Severity != SeverityCritical { + t.Errorf("expected critical for ungated issue_comment checkout+exec, got %s", f.Severity) + } + if !strings.Contains(f.Message, "issue_comment") { + t.Errorf("expected issue_comment in message, got: %s", f.Message) + } +} + +// The same ChatOps gated by an author_association allowlist (only trusted +// commenters can trigger) must be downgraded and marked mitigated. +func TestCheckPwnRequest_IssueCommentAuthorGuarded(t *testing.T) { + wf := loadFixture(t, "pwn-request-issue-comment-guarded.yaml") + findings := CheckPwnRequest(wf) + if len(findings) != 1 { + t.Fatalf("expected 1 FG-001 finding, got %d: %v", len(findings), findings) + } + f := findings[0] + if f.Severity == SeverityCritical || f.Severity == SeverityHigh { + t.Errorf("expected downgraded severity for author-association-gated ChatOps, got %s", f.Severity) + } + if !strings.Contains(f.Message, "mitigated") { + t.Errorf("expected mitigated note, got: %s", f.Message) + } +} + // 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. diff --git a/test/fixtures/pwn-request-issue-comment-guarded.yaml b/test/fixtures/pwn-request-issue-comment-guarded.yaml new file mode 100644 index 0000000..0dc049a --- /dev/null +++ b/test/fixtures/pwn-request-issue-comment-guarded.yaml @@ -0,0 +1,19 @@ +name: PR Command Guarded +on: + issue_comment: + types: [created] +jobs: + run-command: + if: > + github.event.issue.pull_request && + contains(github.event.comment.body, '/test') && + (github.event.comment.author_association == 'OWNER' || + github.event.comment.author_association == 'MEMBER' || + github.event.comment.author_association == 'COLLABORATOR') + runs-on: ubuntu-latest + steps: + - name: checkout pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: gh pr checkout ${{ github.event.issue.number }} + - run: npm install diff --git a/test/fixtures/pwn-request-issue-comment.yaml b/test/fixtures/pwn-request-issue-comment.yaml new file mode 100644 index 0000000..536ec7c --- /dev/null +++ b/test/fixtures/pwn-request-issue-comment.yaml @@ -0,0 +1,15 @@ +name: PR Command +on: + issue_comment: + types: [created] +jobs: + run-command: + if: github.event.issue.pull_request && contains(github.event.comment.body, '/test') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: checkout pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: gh pr checkout ${{ github.event.issue.number }} + - run: npm install