C#: Diff-informed queries: phase 3 (non-trivial locations) #20074
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables diff-informed mode on C# queries that select locations other than dataflow source or sink by adding the observeDiffInformedIncrementalMode() predicate and custom location overrides where needed. This is the final step in mass-enabling diff-informed queries across all languages.
- Adds
observeDiffInformedIncrementalMode()predicate to enable diff-informed mode on data flow configurations - Implements custom location selection logic via
getASelectedSinkLocation()for queries that report locations different from sink locations - Distinguishes between primary configurations (enabled with
any()) and secondary configurations (disabled withnone())
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| HardcodedConnectionString.ql | Enables diff-informed mode and adds location override to select call locations instead of sink locations |
| ThreadUnsafeICryptoTransformLambda.ql | Enables diff-informed mode for parallel crypto transform usage detection |
| UnsafeDeserializationQuery.qll | Enables diff-informed mode for primary configs, disables for secondary configs used in unsafe deserialization queries |
| ExternalAPIsQuery.qll | Enables diff-informed mode for external API data flow tracking |
| ConditionalBypassQuery.qll | Enables diff-informed mode and adds location override to select both sink and sensitive method call locations |
There was a problem hiding this comment.
LGTM under the conditions that
(1) DCA looks good
(2) The discussion started here is resolved.
| @@ -78,6 +78,8 @@ private module RemoteSourceToExternalApiConfig implements DataFlow::ConfigSig { | |||
| predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } | |||
|
|
|||
| predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } | |||
|
|
|||
| predicate observeDiffInformedIncrementalMode() { any() } | |||
There was a problem hiding this comment.
This will also affect the output of cs/count-untrusted-data-external-api (if run in diff informed mode). However, this query is not included in any query suite, so this should be ok.
There was a problem hiding this comment.
Since the same objection as in #19957 (comment) applies, I'll remove this commit.
40aac5e to
218fcbb
Compare
|
Will need to run DCA again; Looks like there was a problem with actions yesterday. |
|
The last DCA looks good. @d10c : If the DCA run was triggered with the right arguments then feel free to merge, otherwise please pull the latest DCA changes (as they contain an update to the C# suite) and run DCA again. |
|
Btw, what does |
Excellent question - I have tried to start a thread in slack on, how exactly the option works as I don't understand it from reading the documentation. The reason I only provided |
This PR enables diff-informed mode on queries that select a location other than dataflow source or sink. This entails adding a non-trivial location override that returns the locations that are actually selected.
Prior work includes PRs like #19663, #19759, and #19817. This PR uses the same patch script as those PRs to find candidate queries to convert to diff-enabled. This is the final step in mass-enabling diff-informed queries on all the languages.
Commit-by-commit reviewing is recommended.