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
70 changes: 50 additions & 20 deletions pkg/scanner/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
34 changes: 34 additions & 0 deletions pkg/scanner/rules_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/pwn-request-issue-comment-guarded.yaml
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions test/fixtures/pwn-request-issue-comment.yaml
Original file line number Diff line number Diff line change
@@ -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