-
Notifications
You must be signed in to change notification settings - Fork 676
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
Add codeql suppression #8134
Add codeql suppression #8134
Conversation
@@ -253,7 +253,7 @@ public static ProcessOutput Run( | |||
psi.Arguments = string.Join(" ", | |||
arguments.Where(a => a != null).Select(QuoteSingleArgument)); | |||
} else { | |||
psi.Arguments = string.Join(" ", arguments.Where(a => a != null)); | |||
psi.Arguments = string.Join(" ", arguments.Where(a => a != null)); // CodeQL [SM00406] Code ql is complaining, but this is a utility class that is called with many different file names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
complaining that we dont verify arguments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah so the actual violation is https://liquid.microsoft.com/Web/Object/Read/ScanningToolWarnings/Requirements/CodeQL.SM00406#Zguide. The problem is that we can't possibly validate arguments here, it's a generic helper class to run a command line, we just join the arguments and run. Just red tape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so alternatively we look at all the usage points to find a set of strings.. but there will still be some dynamic ones. maybe arg templates safer.
Quality Gate passedIssues Measures |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #8090
CodeQL doesn't allow you to suppress an warning on an entire file or class, it's per line only.
So even though line 251 already has a suppression, the violation at https://liquid.microsoft.com/codeql/issues/38ae4090-21dd-4617-94cf-6f5f77966286 is now occurring on line 256, so we need another suppression.
That's what this change is.