René's URL Explorer Experiment

Title: Java: make all code-scanning queries diff-informed by jbj · Pull Request #17846 · github/codeql · GitHub

Open Graph Title: Java: make all code-scanning queries diff-informed by jbj · Pull Request #17846 · github/codeql

X Title: Java: make all code-scanning queries diff-informed by jbj · Pull Request #17846 · github/codeql

Description: With these changes, all queries in the code-scanning suite will be diff-informed. This PR is a draft for now because I haven't found a good way to test all these changes. But before I go further, I'd like a review from @aschackmull on whether the approach taken here is what we want at all. #17190 made all the straightforward data-flow queries diff-informed. To cover all the remaining ones, I introduced two new concepts into the data-flow library: The idea that a source or sink has selected locations, which are locations that will eventually be associated with this source/sink in the query. This was necessary because many queries select not the exact source or sink but something closely related to it. has{Source,Sink}InDiffRange, encapsulating the recurring pattern of checking whether a given data-flow configuration has sources or sinks in the diff range at all. This pattern came up a lot in queries where a secondary data-flow configuration was made diff-informed, such as a configuration for finding sinks for the main data-flow configuration. I put these new predicates on DataFlow::Global, but I don't feel too confident about that. Maybe they should be defaults on DataFlow::Configuration instead, intended to be called but not overridden. Or maybe there should be a helper module in DataFlow that deals only with this matter, like DataFlow::DiffInformed::hasSinkInRange. Another pattern that came up was where we have queries that exclude results found by another query, using the latter query as a library. I believe it's fine to make these queries diff-informed for the same reason that the QL optimiser can add context to a negation: only results in the diff range are interesting to subtract. I found it necessary to introduce action at a distance in several places, moving knowledge about callers into callees. In the future I think some of this coupling can be removed by collapsing FooQuery.qll and Foo.ql into a single file, but for now the action at a distance is supposed to be discoverable and maintainable in one of the following ways: Code comments explaining the reverse dependencies. Assuming a convention that implementing any getASelected{Source,Sink}Location is a way to communicate that this is what the query does. Parameterised modules (like in XSS.qll). Abstract classes (like in the regex queries). I recommend reviewing this PR one commit at a time. Pull Request checklist All query authors A change note is added if necessary. See the documentation in this repository. All new queries have appropriate .qhelp. See the documentation in this repository. QL tests are added if necessary. See Testing custom queries in the GitHub documentation. New and changed queries have correct query metadata. See the documentation in this repository. Internal query authors only Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required). Changes are validated at scale (internal access required). Adding a new query? Consider also adding the query to autofix.

Open Graph Description: With these changes, all queries in the code-scanning suite will be diff-informed. This PR is a draft for now because I haven't found a good way to test all these changes. But before I go furthe...

X Description: With these changes, all queries in the code-scanning suite will be diff-informed. This PR is a draft for now because I haven't found a good way to test all these changes. But before I go fu...

Opengraph URL: https://github.com/github/codeql/pull/17846

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width