Improper Access Control Rule Overview #
This rule detects improper access control vulnerabilities in GitHub Actions workflows that use label-based approval mechanisms. This is a critical security vulnerability (CVSS 9.3, CWE-285) that allows attackers to bypass label-based approval and execute malicious code.
Vulnerable Example:
name: PR Build
on:
pull_request_target:
types: [opened, synchronize] # Dangerous: synchronize allows code changes after approval
jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with:
ref: ${{ github.event.pull_request.head.ref }} # Mutable reference!
- run: npm test
Detection Output:
vulnerable.yaml:12:9: improper access control: checkout uses label-based approval with 'synchronize' event type and mutable ref. An attacker can modify code after label approval. Fix: 1) Change trigger types from 'synchronize' to 'labeled', 2) Use immutable 'github.event.pull_request.head.sha' instead of mutable 'head.ref'. See https://codeql.github.com/codeql-query-help/actions/actions-improper-access-control/ [improper-access-control]
12 π| - uses: actions/checkout@v4
Security Background #
Why is this dangerous? #
The vulnerability occurs when three conditions are met:
pull_request_targettrigger withsynchronizeevent type - Allows the workflow to trigger when new commits are pushed to the PR- Label-based approval check - Uses labels like “safe to test” to gate execution
- Mutable branch reference - Uses
head.refinstead ofhead.sha
Attack Scenario #
1. Attacker opens PR with benign code
βββ Workflow does NOT run (no "safe to test" label)
2. Maintainer reviews code and adds "safe to test" label
βββ Workflow runs with benign code β
3. Attacker pushes malicious commit to same PR
βββ "synchronize" event triggers workflow
βββ "safe to test" label is still present
βββ Workflow runs with MALICIOUS code! π¨
4. Mutable ref (head.ref) points to the NEW malicious code
βββ Attacker's code executes with access to secrets
Visual Timeline:
Time βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββΊ
PR opened Label added Malicious commit pushed
β β β
βΌ βΌ βΌ
[Benign code] [Approved! β] [Still has label! π¨]
[head.ref β malicious code]
Why head.ref vs head.sha matters
#
| Reference | Type | After new push | Security |
|---|---|---|---|
head.ref | Branch name | Points to NEW commit | β Mutable |
head.sha | Commit SHA | Points to SAME commit | β Immutable |
Using head.ref (branch name) means the checkout will always get the latest code, even after approval. Using head.sha (commit SHA) locks the checkout to the specific commit that was approved.
OWASP and CWE Mapping #
- CWE-285: Improper Authorization
- OWASP Top 10 CI/CD Security Risks:
- CICD-SEC-4: Poisoned Pipeline Execution (PPE)
Technical Detection Mechanism #
The rule performs three-step detection:
Step 1: Identify pull_request_target with synchronize
// In VisitWorkflowPre
for _, event := range workflow.On {
if webhookEvent, ok := event.(*ast.WebhookEvent); ok {
if webhookEvent.EventName() == "pull_request_target" {
for _, eventType := range webhookEvent.Types {
if eventType.Value == "synchronize" {
rule.hasSynchronizeType = true
}
}
}
}
}
Step 2: Find Checkout Actions with Mutable Refs
// In VisitStep
if action, ok := step.Exec.(*ast.ExecAction); ok {
if strings.HasPrefix(action.Uses.Value, "actions/checkout@") {
refInput := action.Inputs["ref"]
if strings.Contains(refInput.Value.Value, "head.ref") ||
strings.Contains(refInput.Value.Value, "github.head_ref") {
// Mutable reference detected!
}
}
}
Step 3: Check for Label-Based Conditions
// Check step's if condition
if step.If != nil {
if strings.Contains(step.If.Value, "github.event.pull_request.labels") {
// Label-based gating detected
}
}
Detection Logic Explanation #
Mutable Ref Patterns Detected #
The rule detects these dangerous mutable reference patterns:
${{ github.event.pull_request.head.ref }}- PR branch name (mutable)${{ github.head_ref }}- Shorthand for PR branch name (mutable)
Label-Based Condition Patterns Detected #
contains(github.event.pull_request.labels.*.name, 'safe to test')github.event.pull_request.labelsgithub.event.label
Safe Patterns #
The rule does NOT flag these patterns:
Safe Pattern 1: Use labeled event type only
on:
pull_request_target:
types: [labeled] # Only triggers when label is added - no subsequent pushes
jobs:
test:
steps:
- uses: actions/checkout@v4
if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with:
ref: ${{ github.event.pull_request.head.sha }} # Immutable!
Safe Pattern 2: Use immutable SHA reference
on:
pull_request_target:
types: [opened, synchronize]
jobs:
test:
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }} # Immutable!
Safe Pattern 3: Use pull_request trigger instead
on: pull_request # No secrets access, safe to run PR code
jobs:
test:
steps:
- uses: actions/checkout@v4 # Safe: no privileged context
Comparison with Untrusted Checkout Rule #
| Rule | Focus | Trigger |
|---|---|---|
| Untrusted Checkout | Any checkout of PR code in privileged context | All privileged triggers |
| Improper Access Control | Label approval bypass via synchronize + mutable ref | pull_request_target with synchronize |
The Improper Access Control rule is more specific - it focuses on the combination of label-based approval, synchronize events, and mutable references that creates a bypass vulnerability.
False Positives #
The rule has very few false positives because it requires ALL of:
pull_request_targettriggersynchronizein the event types- Checkout with mutable ref (
head.reforgithub.head_ref)
If any condition is not met, the rule will not flag the workflow.
References #
GitHub Documentation #
Security Research #
OWASP Resources #
Auto-Fix #
This rule supports automatic fixing. When you run sisakulint with the -fix on flag, it will automatically apply two fixes:
Fix 1: Replace mutable refs with immutable SHAs
ref: ${{ github.event.pull_request.head.ref }}becomesref: ${{ github.event.pull_request.head.sha }}ref: ${{ github.head_ref }}becomesref: ${{ github.event.pull_request.head.sha }}
Fix 2: Replace synchronize with labeled in event types
types: [opened, synchronize]becomestypes: [opened, labeled]
Example:
Before auto-fix:
on:
pull_request_target:
types: [opened, synchronize]
jobs:
test:
steps:
- uses: actions/checkout@v4
if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with:
ref: ${{ github.event.pull_request.head.ref }}
After running sisakulint -fix on:
on:
pull_request_target:
types: [opened, labeled]
jobs:
test:
steps:
- uses: actions/checkout@v4
if: contains(github.event.pull_request.labels.*.name, 'safe to test')
with:
ref: ${{ github.event.pull_request.head.sha }}
Note: After auto-fix, the workflow will only trigger when a label is added (not on subsequent pushes). This is the correct security behavior for label-gated workflows.
Remediation Steps #
When this rule triggers:
Use auto-fix for quick remediation
- Run
sisakulint -fix onto automatically fix the vulnerability - Review the changes to ensure they meet your workflow requirements
- Run
Change event types to
labeledonly- Remove
synchronizefrom the types array - Add
labeledif not already present - This ensures the workflow only runs when approval is explicitly granted
- Remove
Use immutable SHA references
- Replace
head.refwithhead.sha - This locks the checkout to the specific approved commit
- Replace
Consider using
pull_requesttrigger- If you don’t need secrets access, use
pull_requestinstead - This is the safest option for running untrusted PR code
- If you don’t need secrets access, use
Implement robust approval workflows
- Require label removal on new commits
- Use GitHub’s required reviews feature
- Consider using GitHub Apps for automated approval management
Additional Resources #
For more information on securing GitHub Actions workflows, see:
