Actions: Fix dominates() false positive in reusable workflows#21986
Actions: Fix dominates() false positive in reusable workflows#21986JarLob wants to merge 3 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the CWE-829 “Untrusted Checkout (Critical)” query logic and fixtures to better recognize permission checks (including across reusable workflows) and to validate expected results.
Changes:
- Extend
ControlCheckdominance logic to account for checks that occur in the caller of a reusable workflow. - Add new GitHub Actions workflow fixtures covering permission-check patterns (and a “missing needs” negative case).
- Update the expected test results to include the new fixtures/edges.
Show a summary per file
| File | Description |
|---|---|
| actions/ql/lib/codeql/actions/security/ControlChecks.qll | Adds reusable-workflow caller coverage to control-check dominance logic. |
| actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected | Updates expected edges/results for new workflow fixtures. |
| actions/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout_permissions_check.yml | Adds fixture where collaborator permission check precedes checkout. |
| actions/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout_permission_check_reusable.yml | Adds fixture where permission check precedes a reusable workflow call. |
| actions/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout_no_needs.yml | Adds negative fixture where the permission-check job exists but is not required by the checkout job. |
| actions/ql/test/query-tests/Security/CWE-829/.github/workflows/external/TestOrg/TestRepo/.github/workflows/build.yml | Adds external reusable workflow fixture used by the tests. |
| actions/ql/lib/change-notes/2026-06-15-permission_check.md | Adds changelog entry for the reusable-workflow support fix. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 1
Sorry, something went wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
redsun82
left a comment
redsun82
left a comment
There was a problem hiding this comment.
👋 thanks for this contribution, I think this is going in the right direction! Currently this is relaxing the check a little too widely though, if I got it right. It seems one caller having the correct permission check can hide other callers being unsafe. Could you double check that, and add it to the tests after fixing if I got that right?
Sorry, something went wrong.
Reviewers
Copilot code review Copilot Copilot left review comments
+1 more reviewer
Requested changes must be addressed to merge this pull request.
No description provided.