Prune results of Ruby query from SARIF #1344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edoardopirovano
force-pushed
the
edoardo/prune-ruby
branch
from
955dd50
to
9157344
Compare
aeisenberg
reviewed
Nov 4, 2022
src/upload-lib.ts
Outdated
| run.tool?.driver?.semanticVersion === "2.11.2" | ||
| ) { | ||
| // Version 2.11.2 of the CodeQL CLI had many false positives in the | ||
| // rb/weak-cryptographic-algorithm query which we prune here. |
There was a problem hiding this comment.
Do we have a public facing issue for this? Maybe add a link to it here.
There was a problem hiding this comment.
We do indeed. Added a link.
aeisenberg
reviewed
Nov 4, 2022
src/upload-lib.ts
Outdated
| @@ -396,6 +396,8 @@ async function uploadFiles( | |||
| environment | |||
| ); | |||
|
|
|||
| sarif = pruneInvalidResults(sarif, logger); | |||
There was a problem hiding this comment.
Do we want to gate this with an undocumented environment variable? So that individual users can turn this back on if they really, really want to?
We can consider also feature flagging this, but that feels like something too heavy-weight.
There was a problem hiding this comment.
Sure. This code is hopefully very temporary but it can't hurt to have an escape hatch if a user needs to disable it for some reason.
edoardopirovano
force-pushed
the
edoardo/prune-ruby
branch
from
9157344
to
862a512
Compare
aeisenberg
approved these changes
Nov 4, 2022
There was a problem hiding this comment.
Code LGTM. @aibaars, thanks for verifying this is working on code scanning.
This was referenced Nov 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a mitigation for the fact that the
rb/weak-cryptographic-algorithmquery was released in version 2.11.2 of CodeQL with a large number of false positives relating to hashing algorithms. This was tweaked in github/codeql#11119 for 2.11.3, but we'd like to filter the false positives out while we wait for that to be released.Merge / deployment checklist