Skip to content
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

Ruby: Use the new dataflow API for checked in queries #14124

Merged
merged 51 commits into from
Sep 13, 2023

Conversation

alexrford
Copy link
Contributor

@alexrford alexrford commented Sep 3, 2023

Uses the DataFlow::ConfigSig module signature instead of the DataFlow::Configuration class.

The general naming convention is SomeQueryFlow for the flow module for the SomeQuery.ql query.

In most cases the old configuration has been left in place and deprecated. For cases where the configuration was defined in the query file itself (i.e. within src), I've just deleted the old configuration as it's not part of a CodeQL library.

Any barrier guard predicates have been dropped. I don't think we have any cases where this makes a difference, unless the barrier guard abstract classes are extended externally (i.e. not within this repo).

There are some very tiny changes to flow steps in some places, reflected in updated .expected output. These seem to be benign or minor improvements as far as I can tell.

Two queries, rb/http-to-file-access and rb/tainted-format-string share a *Query.qll file with JS. I've reverted the changes to these queries for now - I think it'll be slightly cleaner to do the Ruby and JS changes for these in a separate PR.

@alexrford alexrford marked this pull request as ready for review September 5, 2023 14:04
@alexrford alexrford requested a review from a team as a code owner September 5, 2023 14:04
@alexrford
Copy link
Contributor Author

Worth noting - this increases cache sizes significantly. This seems to be expected and has also occurred in other languages that have made this change.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for doing this. Some minor comments.

ruby/ql/lib/codeql/ruby/security/CodeInjectionQuery.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll Outdated Show resolved Hide resolved
ruby/ql/src/queries/security/cwe-094/CodeInjection.ql Outdated Show resolved Hide resolved
ruby/ql/src/queries/security/cwe-611/Xxe.ql Outdated Show resolved Hide resolved
@alexrford
Copy link
Contributor Author

@hvitved thanks for the review - updated this branch.

hvitved
hvitved previously approved these changes Sep 13, 2023
@alexrford alexrford merged commit 79c305c into github:main Sep 13, 2023
19 of 21 checks passed
@alexrford alexrford deleted the rb/dataflow-query-refactor branch September 13, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants