Skip to content
Open
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
85 changes: 85 additions & 0 deletions pkg/scanner/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,19 @@ func CheckPwnRequest(wf *Workflow) []Finding {
mitigated = true
}

// Environment gate: the executing job is bound to a deployment
// environment, which may carry required-reviewer protection rules — a
// fork PR would then pause for manual approval by a trusted team before
// any step runs. Protection rules aren't visible in the YAML, so this is
// a one-level downgrade, not a suppression (an environment with no rules
// gates nothing).
if job.Environment != "" && !mitigated {
severity = downgradeBy(severity, 1)
mitigation.Details = append(mitigation.Details, fmt.Sprintf(
"job gated on environment '%s' (may require deployment approval)", job.Environment))
mitigated = true
}

// Path isolation adjusts confidence, not severity
if mitigation.PathIsolated && confidence == ConfidenceConfirmed {
confidence = ConfidencePatternOnly
Expand Down Expand Up @@ -2110,13 +2123,27 @@ func CheckOIDCMisconfiguration(wf *Workflow) []Finding {
severity = SeverityCritical
}

// Environment gate: a job bound to a deployment environment
// (`environment:`) has its secrets AND its execution scoped to that
// environment, which may carry required-reviewer protection rules —
// in which case a fork PR pauses for manual approval by a trusted team
// before any step (or OIDC request) runs. Protection rules aren't
// visible in the workflow YAML, so this is a one-level downgrade, not a
// suppression (an environment with no rules gates nothing).
envNote := ""
if job.Environment != "" {
severity = downgradeBy(severity, 1)
envNote = fmt.Sprintf(" (job gated on environment '%s' — may require deployment approval)", job.Environment)
}

msg := fmt.Sprintf("OIDC Misconfiguration: id-token:write on pull_request_target workflow (job '%s')", jobName)
if len(cloudActions) > 0 {
msg += fmt.Sprintf(" with cloud auth (%s)", strings.Join(cloudActions, ", "))
}
if checkoutsFork {
msg += " — attacker can mint cloud credentials via fork PR"
}
msg += envNote

findings = append(findings, Finding{
RuleID: "FG-008",
Expand Down Expand Up @@ -2713,6 +2740,47 @@ func localActionUnderTrustedCheckout(job Job, uses string) bool {
return false
}

// localActionStagedBeforeForkCheckout reports whether a parent-escaping local
// action (`uses: ./../<dir>/...`) resolves to a sibling directory that a run
// step populated BEFORE the untrusted (fork) checkout at forkCheckoutIdx.
//
// This recognizes GitHub's recommended pull_request_target hardening idiom:
// check out the trusted base ref, copy the scripts/actions you intend to
// execute into a directory OUTSIDE the workspace (e.g. `../base-branch/`), then
// check out the fork PR head into the workspace. `actions/checkout` only writes
// within GITHUB_WORKSPACE, so a `..`-escaping path can never be populated by the
// fork checkout; its contents are whatever the trusted pre-checkout run steps
// staged there.
func localActionStagedBeforeForkCheckout(job Job, uses string, forkCheckoutIdx int) bool {
p := strings.TrimPrefix(uses, "./")
// Only credit paths that escape the workspace via `..` — an in-workspace
// `./.github/...` path IS overwritten by the fork checkout and stays untrusted.
if !strings.HasPrefix(p, "../") {
return false
}
seg := ""
for _, s := range strings.Split(p, "/") {
if s == "" || s == "." || s == ".." {
continue
}
seg = s
break
}
if seg == "" || forkCheckoutIdx < 0 {
return false
}
// Require a run step BEFORE the fork checkout that references the sibling
// dir — the trusted staging (`mkdir -p ../base/... && cp ... ../base/...`).
// Requiring it to precede the untrusted checkout guarantees the staged
// content came from the trusted tree, not the fork.
for i := 0; i < forkCheckoutIdx && i < len(job.Steps); i++ {
if run := job.Steps[i].Run; run != "" && strings.Contains(run, seg) {
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 @@ -2759,6 +2827,13 @@ func CheckLocalActionUntrustedCheckout(wf *Workflow) []Finding {
if hasAuthorAssociationGuard(job) || containsForkGuard(job.If) {
severity = SeverityInfo
guardNote = " (mitigated: job restricts execution to trusted authors / non-forks)"
} else if postCheckout := job.Steps[checkoutIdx+1:]; (jobHasNoToken(wf, job) || jobHasReadOnlyToken(wf, job)) && !postCheckoutAccessesSecrets(postCheckout) {
// Read-only (or empty) token + no secrets referenced: the fork-controlled
// action.yml still runs, but the token can't push/comment and there is
// nothing to exfiltrate. This is normal `pull_request` CI (build/lint the
// fork). Mirrors FG-001's ReadOnlyToken/NoCredentialExec mitigation.
severity = SeverityInfo
guardNote = " (mitigated: read-only token, no secrets in scope)"
}

for _, step := range job.Steps[checkoutIdx+1:] {
Expand All @@ -2769,6 +2844,16 @@ func CheckLocalActionUntrustedCheckout(wf *Workflow) []Finding {
if localActionUnderTrustedCheckout(job, step.Uses) {
continue
}
// The local action lives in a parent-escaping sibling dir
// (`./../<dir>/...`) that a run step staged from trusted code
// BEFORE the untrusted checkout. `actions/checkout` writes only
// inside the workspace, so a `..`-escaping path is never populated
// by the fork checkout — its contents come from the trusted
// pre-checkout staging (GitHub's recommended "copy trusted scripts
// out of the checkout" hardening idiom).
if localActionStagedBeforeForkCheckout(job, step.Uses, checkoutIdx) {
continue
}
findings = append(findings, Finding{
RuleID: "FG-016",
Severity: severity,
Expand Down
66 changes: 66 additions & 0 deletions pkg/scanner/rules_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,26 @@ func TestCheckPwnRequest_NoTokenExec(t *testing.T) {
}
}

// FG-001 must downgrade one level when the executing job is bound to a
// deployment environment (which may require reviewer approval before a fork PR
// runs). The environment fixture has id-token:write (not read-only), so the
// baseline is critical and the env gate drops it to high. Regression for
// cilium/cilium build-images-base.yaml.
func TestCheckPwnRequest_EnvironmentGated(t *testing.T) {
wf := loadFixture(t, "oidc-prt-environment-gated.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 != SeverityHigh {
t.Errorf("expected high severity (critical downgraded by environment gate), got %s", f.Severity)
}
if !strings.Contains(f.Message, "environment") {
t.Errorf("expected environment mitigation note, got: %s", f.Message)
}
}

// 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) {
Expand Down Expand Up @@ -687,6 +707,25 @@ func TestCheckOIDC_PushOnly(t *testing.T) {
}
}

// A PRT job with id-token:write + fork checkout that is bound to a deployment
// environment downgrades one level (critical -> high): the environment may carry
// required-reviewer protection gating the OIDC request behind manual approval.
// Regression for cilium/cilium build-images-base.yaml.
func TestCheckOIDC_EnvironmentGated(t *testing.T) {
wf := loadFixture(t, "oidc-prt-environment-gated.yaml")
findings := CheckOIDCMisconfiguration(wf)

if len(findings) != 1 {
t.Fatalf("expected 1 finding, got %d", len(findings))
}
if findings[0].Severity != SeverityHigh {
t.Errorf("expected high (PRT + fork checkout downgraded by environment gate), got %s", findings[0].Severity)
}
if !strings.Contains(findings[0].Message, "environment") {
t.Error("expected message to mention the environment gate")
}
}

// --- FG-009 Self-Hosted Runner tests ---

func TestCheckSelfHosted_PR(t *testing.T) {
Expand Down Expand Up @@ -1284,6 +1323,33 @@ func TestCheckLocalActionUntrustedCheckout_AuthorGuarded(t *testing.T) {
}
}

// FG-016 must downgrade to info when the executing job has a read-only token
// (permissions: read-all) and references no secrets — fork code runs but the
// token can't write and there is nothing to exfiltrate (normal pull_request CI).
// Regression for cilium/cilium lint-build-commits.yaml.
func TestCheckLocalActionUntrustedCheckout_ReadOnlyToken(t *testing.T) {
wf := loadFixture(t, "local-action-readonly-token.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 read-only-token job, got %s", findings[0].Severity)
}
}

// FG-016 must not fire when the local action lives in a parent-escaping sibling
// dir (`./../base-branch/...`) that a run step staged from trusted code BEFORE
// the untrusted checkout — actions/checkout can't write to a `..`-escaping path,
// so its contents are trusted. Regression for cilium/cilium build-images-base.yaml.
func TestCheckLocalActionUntrustedCheckout_StagedSiblingDir(t *testing.T) {
wf := loadFixture(t, "local-action-staged-sibling-dir.yaml")
findings := CheckLocalActionUntrustedCheckout(wf)
if len(findings) != 0 {
t.Fatalf("expected 0 findings for trusted staged sibling-dir action, got %d: %v", len(findings), findings)
}
}

// --- FG-027 TOCTOU Label Gate tests ---

// Vulnerable pattern: pull_request_target with synchronize, job gated on an
Expand Down
22 changes: 22 additions & 0 deletions test/fixtures/local-action-readonly-token.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Build Commits
# Plain pull_request (fork code runs) with a read-only token and no secrets:
# normal CI. The workflow_run trigger is what makes FG-016 consider the workflow,
# but read-all + no secrets means the fork-controlled local action has nothing to
# steal and a token that can't write.
on:
pull_request: {}
workflow_run:
workflows: ["Cache Cleaner"]
branches: [main]
types: [completed]
permissions: read-all
jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
persist-credentials: false
- uses: ./.github/actions/disk-cleanup
- run: make build
31 changes: 31 additions & 0 deletions test/fixtures/local-action-staged-sibling-dir.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Base Image Release Build
# GitHub's recommended pull_request_target hardening: check out the trusted base
# ref, copy the actions/scripts you intend to run into a directory OUTSIDE the
# workspace, THEN check out the fork PR head. The `./../base-branch/...` action is
# staged from trusted code before the untrusted checkout, so it is not attacker-
# controlled — actions/checkout can't write to a `..`-escaping sibling dir.
on:
pull_request_target:
types: [opened, synchronize, reopened]
permissions:
contents: read
jobs:
build-and-push:
runs-on: ubuntu-latest
steps:
- name: Checkout base branch (trusted)
uses: actions/checkout@v4
with:
ref: ${{ github.base_ref }}
persist-credentials: false
- name: Copy trusted action out of the workspace
run: |
mkdir -p ../base-branch/.github/actions
cp -r .github/actions/cosign ../base-branch/.github/actions/
- name: Checkout PR head (NOT TRUSTED)
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
persist-credentials: false
- name: Sign with trusted (staged) action
uses: ./../base-branch/.github/actions/cosign
23 changes: 23 additions & 0 deletions test/fixtures/oidc-prt-environment-gated.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Base Image Release Build
# pull_request_target with id-token:write AND a fork-head checkout, but the job is
# bound to a deployment environment. If that environment carries required-reviewer
# protection, a fork PR pauses for manual approval before any step (or OIDC token
# request) runs. Protection rules aren't visible in YAML, so FG-008/FG-001 apply a
# one-level downgrade (critical -> high), not a suppression.
on:
pull_request_target:
types: [opened, synchronize, reopened]
permissions:
contents: read
id-token: write
jobs:
build-and-push:
runs-on: ubuntu-latest
environment:
name: release-base-images
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}
persist-credentials: false
- run: make build