Actions: only count association checks for events that populate the checked field - #22391
Merged
jketema merged 1 commit intoAug 20, 2026
Merged
Conversation
…ulate the checked field Same problem as EventActorIfCheck had. A condition like 'github.event.pull_request.author_association != NONE' on a workflow triggered by issues events is always true since github.event.pull_request is not populated there, but it still counted as a protective check for those events. Bind the matched context prefix in the characteristic predicate and require contextTriggerDataModel to populate it for the event, following the pattern from github#22368. Existing TOCTOU alerts on vacuous gates migrate to the untrusted-checkout critical query, since a vacuous gate is no protection rather than raceable protection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4084febb-f9c7-44df-baf3-c8be8e9932a7
Contributor
There was a problem hiding this comment.
Pull request overview
Updates association-based control checks to ignore event payload fields unavailable for the triggering event.
Changes:
- Maps association checks to compatible event payload contexts.
- Adds valid and vacuous association-check test cases.
- Updates affected query expectations and release notes.
Show a summary per file
| File | Description |
|---|---|
ControlChecks.qll |
Adds event-aware association checks. |
association_check_wrong_event.yml |
Tests valid and vacuous gates. |
CodeInjectionCritical.expected |
Updates critical-alert expectations. |
CodeInjectionMedium.expected |
Updates medium-alert expectations. |
UntrustedCheckoutCritical.expected |
Records newly detected critical alerts. |
UntrustedCheckoutTOCTOUCritical.expected |
Removes reclassified TOCTOU alerts. |
UntrustedCheckoutTOCTOUHigh.expected |
Removes reclassified high alerts. |
2026-08-19-association-if-check-event-validity.md |
Documents the analysis change. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22388
A workflow gated by
if: github.event.pull_request.author_association != 'NONE'but triggered byissuesorissue_commentisn't actually protected, since those payloads don't have a top-levelpull_requestand the condition is always true for them. Anyone who opens an issue or comments reaches the job.AssociationIfCheckstill counted that gate as protection and suppressed alerts on these workflows, which is the same false negative that #22368 fixed for actor checks. This PR applies the same fix to association checks, using thecontextTriggerDataModelmapping to decide which events actually populate the field a check reads, so a check only counts as protection for those events.Some existing test results move around as a result. A few of the untrusted-checkout testcases happen to use this exact vacuous gate on issue_comment workflows, so their alerts move from the TOCTOU queries over to
actions/untrusted-checkout/critical. I think that's the right outcome, since a gate that never gates isn't something you can win a race against, and the TOCTOU suggestion of pinning the reviewed SHA wouldn't have helped those workflows. The testcases that use validcomment.andissue.author_associationgates are unchanged, and I've added new ones covering a vacuous check next to a valid one.