-
Notifications
You must be signed in to change notification settings - Fork 401
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
Fix CodeQL issues #2228
Comments
@nabijaczleweli Do you have any interest to look into these ? |
the page linked doesn't work (the popups are broken and the direct code links all link to nothing), and the summaries mostly range from "so what?" to "idiotic", so not really, no |
@nabijaczleweli Thanks. Understandable. I think that scan is out of date which is why lot of links do not work, and if you upload a PR now that would not be out of date on the PR run. We only have a few of us and and if we not able to make this bot pass, then it is better to disable it (and continue drive blind). I suspect once we make it green, it might be more useful on a per PR bases |
@LaszloGombos given that you decided to merge this prior to fixing the issues the scanner had already detected as I had mentioned we should do prior to merging #1987 so we would have a clean state to start from this is now your responsibility to fix... |
@johannbg I might not fully understand how this work, but what do you mean by "clean slate" ? It seems to me that this works differently than shellcheck. There is no mechanism for inline suppression. Newly uploaded code passes the check with green (see #2229) and now we have a tool to check new PRs and GitHub's infrastructure to manage the reported issues. I agree with @nabijaczleweli in that it should not be a goal to resolve all issues as a lot of them is just noise. It seem to "dismiss noise", the expectation is NOT to annotate the source code, but simply manage it in the https://github.com/dracutdevs/dracut/security/code-scanning dashboard. Once you mark an issue "Dismiss" in the dashboard, it is gone without any code changes. I already marked quite a few of them not applicable and intent to resolve some of them but not all. Link to the Dismiss'd issues - https://github.com/dracutdevs/dracut/security/code-scanning?query=is%3Aclosed+branch%3Amaster I could certainly mark all of the issues "closed" for "clean slate", which is more or less the state we would be in if we just staring at that open PR without landing it and hoping that somebody would reopen a stale PR. We need to triage and decide which one we care to fix - ideally before writing a lot of code. The best way to triage is to land the PR. This is a good bug (and good time) to discuss which one is safe to close and which one we expect to resolve. |
https://github.com/dracutdevs/dracut/runs/8493550424https://github.com/dracutdevs/dracut/security/code-scanning
Security Alerts:
10 high
Other Alerts:
3 warnings
17 notes
Would be good to resolve at least the 10 high alerts
The text was updated successfully, but these errors were encountered: