Sequester issue_comment triggered untrusted checkout from other triggers #18838
Conversation
* issue_comment triggered untrusted checkouts present a security risk but mitigating the risk cannot be done wholly in the workflow relying on the event and those mitigations cannot be detected by CodeQL so these triggers should be moved to separate alerts with level warning
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| string issueCommentTriggers() { | ||
| result = ["issue_comment"] |
Check warning
Code scanning / CodeQL
Singleton set literal Warning
| not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) and | ||
| not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout")) | ||
| select poisonable, checkout, poisonable, | ||
| "Potential execution of untrusted code on a privileged workflow ($@)", event, event.getName() |
Check warning
Code scanning / CodeQL
Alert message style violation Warning
| inPrivilegedContext(checkout, event) and | ||
| event.getName() = issueCommentTriggers() and | ||
| not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) | ||
| select checkout, "Potential execution of untrusted code on a privileged workflow ($@)", event, |
Check warning
Code scanning / CodeQL
Alert message style violation Warning
| @@ -0,0 +1 @@ | |||
| Security/CWE-829/UntrustedCheckoutIssueCommentCritical.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
| @@ -0,0 +1 @@ | |||
| Security/CWE-829/UntrustedCheckoutIssueCommentHigh.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
issue_commenttriggered untrusted checkouts present a security risk but mitigating the risk cannot be done wholly in the workflow relying on the event and those mitigations cannot be detected by CodeQL so these triggers should be moved to separate alerts with levelwarning. See https://github.blog/security/application-security/how-to-secure-your-github-actions-workflows-with-codeql/#issueoops-security-pitfalls-with-issue_comment-trigger for more details.I removed the
issue_commenttrigger from the untrusted checkout high and critical and created new alerts with mitigation advice more suited towardsissue_comment. I think it's important to warn developers about the risks of this workflow trigger, but understand it may not be possible for projects that rely heavily on IssueOps