search

github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.6k stars 1.52k forks source link

Ignore Jekyll wrapping in JS files #6642

Open SeanKilleen opened 3 years ago

SeanKilleen commented 3 years ago

Description of the false positive

In some cases in Jekyll projects, a JS file may still receive a front-matter heading, which would look incorrect (and to just a JS parser, would indeed be incorrect.) But in this case, I believe the front-matter is necessary.

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/mmistakes/minimal-mistakes/snapshot/2d5eb9fb06827645b4bf5d55d96584353159dff0/files/assets/js/lunr/lunr-gr.js?sort=name&dir=ASC&mode=heatmap#x5ea38509661d9a53:1

hmakholm commented 3 years ago

Thanks for your comment. CodeQL does indeed expect that .js files it finds will contain ready-to-parse Javascript.

We recognize that it would be valuable to be able to find and analyze Javascript wrapped/embedded in common ways, but Jekyll is not something we're working on right now. I'll keep your issue open to document the request.

Right now a workaround to get analysis to happen might be to run Jekyll and analyze just its output instead of the source directory. (I don't know enough about Jekyll to know for sure whether that idea is viable, but I think it's the best option we have to offer right now).

SeanKilleen commented 3 years ago

Thanks for responding! I'm not worried about a workaround currently as I only came across this when browsing and the issue was immediately evident; just wanted to report it in case the CodeQL team was able to adjust for it. My guess is it's probably pretty much an edge case in the grand scheme of things.

Feel free to close if you don't intend to put it on the roadmap. 👍