Update CodeQLReader.java to handle current CodeQL CLI Output.

[ncouraud]

This involves moving rules detection from the rules array that is on the tool.driver object to parsing the rules run from executed CodeQL query packs in the tool.extensions Array.

I think this might break LGTM generated SARIF, though I believe that is no longer supported. This should work for SARIF generated by CodeQL using the latest CLI bits.

Tested using CodeQL 2.12.2

[davewichers]

Thanks for the contribution. Can you provide some test results that include these new rule types so we can test your changes? You can just email to: dave.wichers@owasp.org.

Also, please fix this compilation error:

Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project benchmarkutils-maven-plugin: Compilation failure 10641 Error: /home/runner/work/BenchmarkUtils/BenchmarkUtils/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/CodeQLReader.java:[99,67] error: cannot find symbol 10642 Error: symbol: variable rulesUsed 10643 Error: location: class CodeQLReader

[davewichers]

@ncouraud - Are you going to address the issues I've pointed out and provide us with test data to verify the changes?

[ncouraud]

@davewichers - apologies, have gotten a bit sidetracked lately. Java is not my forte, but I think the NullReference that I created should be fixed. I'm working on forking & reproducing the run I have against the BenchmarkJava Suite. I'll include at least a link to that Actions run, if not the results shortly.

[ncouraud]

SARIF Output for Java/JS should be here - that should feed into the benchmark util nicely: https://github.com/ncouraud/BenchmarkJava/actions/runs/4992456768

[davewichers]

@darkspirit510 - Sascha - You OK with these changes? I agree I should merge?

[darkspirit510]

@ncouraud Do you have access to an old result file? If yes, could you add a simplified test for both old and new result file? If not, could you at least add a test for the new file format? If in doubt, send me a result file to github@darkspirit510.de and I'll create a test for you 😉

@davewichers the code change looks fine, but since someone (@ncouraud) touched the code, there should be a test, if possible.

[davewichers]

@ncouraud - Can you address @darkspirit510's comment? Or provide him with the results file(s) so he can create test cases?

[ncouraud]

I don't have the original output that the CodeQLReader class was authored against, apologies.

That said, I've authored a test that emulates the others, along with a mocked SARIF file that should give a base to model against.

[ncouraud]

Actually - based on a chat with some colleagues - I think there is a couple more edge cases I would like to handle here - I'll fix that and add some more tests- please hold off on merging this for now. Apologies!

[ncouraud]

ok I reworked it to be a more conservative change - I think that I actually prefer this. It turns out that there are multiple paths to get rules into the tool, and the rules can show up in either place.

@darkspirit510 - I added tests to cover both cases in a single test suite - happy to factor it out if need be.

One additional thing that I did not handle, but could if you'd like me to - is deduping the resultant rules array when pulling from both potential locations. I don't know if this is possible today, but I can't rule it out.

[davewichers]

@darkspirit510 - Let me know what you think about this with these latest updates.