Skip to content

feat: composite action support#331

Merged
woodruffw merged 32 commits intomainfrom
ww/composite-actions
Dec 25, 2024
Merged

feat: composite action support#331
woodruffw merged 32 commits intomainfrom
ww/composite-actions

Conversation

@woodruffw
Copy link
Copy Markdown
Member

@woodruffw woodruffw commented Dec 19, 2024

Very WIP, just stitching things together.

Closes #173.

Some key limitations:

  • The ignore config doesn't currently support ignoring composite actions

@woodruffw
Copy link
Copy Markdown
Member Author

I've started working on this, but I'm now realizing that the trait layout here isn't going to be conducive to a separate ActionAudit, since Rust doesn't have a clean way to cast between traits.

Instead of keeping WorkflowAudit and ActionAudit separate, I think I'm going to rename Audit -> AuditCore and then make WorkflowAudit into a single Audit trait that can handle both workflows and actions. That'll avoid the trait casting pain.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
@woodruffw woodruffw self-assigned this Dec 21, 2024
@woodruffw woodruffw added the enhancement New feature or request label Dec 21, 2024
@woodruffw
Copy link
Copy Markdown
Member Author

There's still a decent amount of cleanup that needs to be done here, but zizmor can now audit actions! Here's what it produces for the vulnerable Ultralytics action (https://github.com/ultralytics/actions/blob/63d2cd98a098f45d5532e3b1ee7715bae66280c2/action.yml):

error[template-injection]: code injection via template expansion
  --> /Users/william/tmp/action.yml:70:7
   |
70 |       - name: Install Dependencies
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
71 |         # Note tomli required for codespell with pyproject.toml
...
74 |         #   pip install -q git+https://github.com/ultralytics/actions@main codespell tomli
75 | /       run: |
76 | |         packages="ultralytics-actions"
...  |
87 | |
88 | |         ultralytics-actions-info
   | |________________________________^ inputs.spelling may expand into attacker-controllable code
   |
   = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ inputs.github_username may expand into attacker-controllable code
    |
    = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ inputs.github_email may expand into attacker-controllable code
    |
    = note: audit confidence → Low

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ github.head_ref may expand into attacker-controllable code
    |
    = note: audit confidence → High

error[template-injection]: code injection via template expansion
   --> /Users/william/tmp/action.yml:211:7
    |
211 |       - name: Commit and Push Changes
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
212 |         if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && github.event.action != 'closed'
213 | /       run: |
214 | |         git config --global user.name "${{ inputs.github_username }}"
...   |
223 | |           echo "No changes to commit"
224 | |         fi
    | |__________^ github.ref may expand into attacker-controllable code
    |
    = note: audit confidence → High

5 findings: 0 unknown, 0 informational, 0 low, 0 medium, 5 high

Critically, it catches all three of the template injections (the last 3 findings) that made the action exploitable.

Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
@woodruffw woodruffw marked this pull request as ready for review December 23, 2024 19:25
@woodruffw woodruffw merged commit c28d44c into main Dec 25, 2024
@woodruffw woodruffw deleted the ww/composite-actions branch December 25, 2024 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: support auditing (composite) actions

1 participant