You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add CodeQL security analysis to GitHub Actions workflows #36957
It seems that a PR was opened by someone to add this before, but somehow it was closed. In many cases, I think it's worth doing this and I'm ready for PR, so if you are willing to add it, I will raise a PR.
CodeQL is a tool for finding vulnerabilities in the entire codebase. Since it was acquired by GitHub last year, it has been integrated natively, and now it can be easily integrated to workflows with GitHub Actions. It's free for open-source repositories, so there's no harm in using it. Of course, it doesn't find all the vulnerabilities, but it's much better than nothing. https://securitylab.github.com/tools/codeql
CodeQL may be a good fit for lots of repositories, but I suspect this is not one of them. I could be wrong. Maybe there's a way to open a pull request adding it such that we can see what kind of results we get without having to land it in the repository first.
CodeQL was the tool created by/for LGTM.com. I therefore assume that enabling it here will result in similar results to LGTM.com. At the current time LGTM.com flags 160 things in the Node.js repository. Of those 160 alerts, 153 are in three dependencies that are vendored into the repository: V8, gyp, and inspector_protocol. The issues flagged appear to contain many false positives and/or test/tooling code that is not relevant to Node.js core.
Basically, my expectation is that a CodeQL analysis at the current time would produce too much noise to be useful. I could be wrong. Or I could be right but it will change as the tool evolves. As I said above, maybe there's a way to open a pull request adding it such that we can see what kind of results we get without having to land it in the repository first. If so, I'd welcome that.
If CodeQL is able to reliably detect use of unsafe array iteration or report where primordials should be used, I'd be up for that, personally. I guess it depends how configurable it is, I agree it's not helpful if it produces more false positives than useful warnings.
Activity
aduh95 commentedon Jan 16, 2021
What is CodeQL, and why is it worth doing?
smorimoto commentedon Jan 16, 2021
CodeQL is a tool for finding vulnerabilities in the entire codebase. Since it was acquired by GitHub last year, it has been integrated natively, and now it can be easily integrated to workflows with GitHub Actions. It's free for open-source repositories, so there's no harm in using it. Of course, it doesn't find all the vulnerabilities, but it's much better than nothing.
https://securitylab.github.com/tools/codeql
Trott commentedon Jan 16, 2021
CodeQL may be a good fit for lots of repositories, but I suspect this is not one of them. I could be wrong. Maybe there's a way to open a pull request adding it such that we can see what kind of results we get without having to land it in the repository first.
CodeQL was the tool created by/for LGTM.com. I therefore assume that enabling it here will result in similar results to LGTM.com. At the current time LGTM.com flags 160 things in the Node.js repository. Of those 160 alerts, 153 are in three dependencies that are vendored into the repository: V8, gyp, and inspector_protocol. The issues flagged appear to contain many false positives and/or test/tooling code that is not relevant to Node.js core.
Of the remaining 7, there are multiple false positives such as this incorrect flagging of a line in configure.py. I reported this as a bug in the CodeQL issue tracker in September. It's entirely possible a fix is imminent or already happened on the GitHub CodeQL side, but LGTM.com is still reporting this issue so maybe not.
Basically, my expectation is that a CodeQL analysis at the current time would produce too much noise to be useful. I could be wrong. Or I could be right but it will change as the tool evolves. As I said above, maybe there's a way to open a pull request adding it such that we can see what kind of results we get without having to land it in the repository first. If so, I'd welcome that.
aduh95 commentedon Jan 16, 2021
If CodeQL is able to reliably detect use of unsafe array iteration or report where primordials should be used, I'd be up for that, personally. I guess it depends how configurable it is, I agree it's not helpful if it produces more false positives than useful warnings.
jasnell commentedon Jan 16, 2021
I won't block but I'm generally not a fan due to the complexity and false positives. eslint seems to cover the majority of what we'd need.
Trott commentedon Feb 23, 2021
I'm going to close this, but feel free to comment or re-open if you think there's a reason to keep this open. Thanks!