-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Actions: mass enable diff-informed data flow #19659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
An auto-generated patch that enables diff-informed data flow in the obvious cases. Builds on github#18346 and github/codeql-patch#88
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables diff-informed data flow by adding the observeDiffInformedIncrementalMode
p
8000
redicate in the DataFlow::ConfigSig
modules across various Action and security query files.
- Introduces
predicate observeDiffInformedIncrementalMode() { any() }
in reusable and composite workflow config modules. - Adds the same predicate to security query config modules for secret exfiltration, request forgery, and output clobbering.
- Builds on prior work to support incremental/diff-based analysis in obvious cases.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
actions/ql/src/Models/ReusableWorkflowsSummaries.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/src/Models/ReusableWorkflowsSources.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/src/Models/ReusableWorkflowsSinks.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/src/Models/CompositeActionsSummaries.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/src/Models/CompositeActionsSources.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/src/Models/CompositeActionsSinks.ql | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/lib/codeql/actions/security/SecretExfiltrationQuery.qll | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/lib/codeql/actions/security/RequestForgeryQuery.qll | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll | Added observeDiffInformedIncrementalMode predicate to enable diff-informed flow. |
Comments suppressed due to low confidence (2)
actions/ql/src/Models/CompositeActionsSummaries.ql:29
- Add a brief doc comment above this predicate to explain its role in enabling diff-informed incremental analysis for readers unfamiliar with the feature.
predicate observeDiffInformedIncrementalMode() { any() }
actions/ql/lib/codeql/actions/security/SecretExfiltrationQuery.qll:19
- No tests were added to validate diff-informed incremental mode. Consider adding unit or integration tests to ensure this predicate actually enables the intended incremental behavior.
predicate observeDiffInformedIncrementalMode() { any() }
It turns out that some of the generated changes in the PRs were not correct, e.g. because they should have also generated a getASelected{Source,Sink}Location() override but didn't (see Chuan-kai's comment here). So for now I'm putting them back in Draft until I make sure (via the patch script) that we are correctly handling all 3 documented query patterns, starting with the simplest one (both source and sink are used as location sources). If you have already started reviewing the PRs, thank you (also for your patience) and stay tuned for an update as to what has changed in the meantime! |
An auto-generated patch that enables diff-informed data flow in the obvious cases.
Builds on #18346 and https://github.com/github/codeql-patch/pull/88