Introduce the security-experimental CodeQL suite and experimental tag #11702
Conversation
…ries with an experimental tag
…ew tagging system
I'd be surprised if there aren't one or two CPP experimental queries with performance issues on larger projects, but these issues aren't exactly 'known', we don't have a list of them. Is it worth me putting aside an hour or two to identify them now? Or just deal with things as and when problems are reported? |
That sounds reasonable, but if you find yourself spending more time on it, it's OK to just exclude them 🙂 |
|
After a bit of experimentation: CPP and I guess Swift LGTM. I will be away for Christmas so please don't wait for my final 👍. |
|
You haven't added a |
…o turbo/experimental/combined
|
@erik-krogh fixed @geoffw0 excluded |
There was a problem hiding this comment.
JS 👍
All the new queries perform OK.
And we will get way more experimental queries once I finish creating some heuristic variants of our ordinary queries.
|
Will Java be an included language for these query pack? |
|
@cgaudette Please see the diff - all languages are included. Thanks for all your stamps. Now waiting for a final review from Python, C#, core (@adityasharad) and a stamp from another PM. |
|
C#: The changes look good, but I would like to run the new suite against our existing DCA projects. I will circle back on this. Also a question: Are there any plans on changing the existing selectors such that they are not explicitly excluding the experimental queries by folder but instead using tags? |
|
The DCA experiment to run the new query suite did terminate in time. |
|
@michaelnebel Thanks for looking into this, I think since the evaluation succeeded, we can move ahead with this and address any lower-level performance issues as they arise later. On:
We can definitely do that in a separate batch of work. If I understand your suggestion correctly and we were to implement this right now, that would actually pull in the non-security experimental queries into the existing suite selectors. To prevent that a thorough cleanup of the directory structure and tags would be in order, which is definitely worth doing, but probably not required now. Or did I misunderstand and you actually see this current PR impacting the existing selectors (something I wanted to avoid)? |
I completely agree that it shouldn't be a part of this PR. |
This PR implements the experimental CodeQL suite, by introducing a new suite selector
security-experimentaland suite implementations for all supported (and beta) languages. For more details and caveats, please see our internal roadmap item.Goal of the experimental suite
The
security-experimentalsuite captures all queries captured bysecurity-extended, but adds all existing queries that:experimentalfoldersecurityThe experimental suite is not going to be advertised for use by code scanning users; not as part of the workflow, docs, neither the setup experience. It is meant as an internal or field testing shortcut. It does not change our current support level for experimental queries (which is "none"), and it does not impose any restrictions on contributions or experimental query writing, with the exception of adding a tag introduced by this PR.
Structure of this change
I've structured this PR in three distinct commits:
4ec401aintroduces the tagging mechanism. Previously, experimental queries were just put into each language's experimental directory, each with a different structure. The standard suite selectors have excluded this directory by path, instead of metadata/tags. This commit adds a newexperimentaltag to all applicable queries (see Goal section above for criterion used) to allow for a change to select by tags insteadb35a1d4is a docs change and updates the contribution guidelines to require theexperimentaltag, as well as the experimental doc stub to indicate the existence of the experimental suite5fd5ebcfinally creates thesecurity-experimentalsuite selector, and implementations for all supported languages. Plus Swift, which doesn't have an experimental folder, but gets this change anyway in anticipation.It's worth expanding on how the logic in 3. works: We still exclude everything in all experimental directories, but then explicitly re-include all queries tagged both security and experimental. This ignores quality, debugging, and other queries that shouldn't be considered for this and allows some amount of freedom of use of the experimental folder without every single query immediately ending up in this suite.
Things to review in detail
While each single change is simple, the combination may cause complex consequences to arise, so please pay close attention to the following, in addition to the general review and comments 🙂
code-scanning,security-extended, orsecurity-and-qualitysuites, how users use them, and what results they produce. I'm somewhat confident that's the case, but I want to make sure.Timeline
I'll be offline until Dec 17 to Jan 9, but please do add your review and questions in the meantime if you are around. I'm aiming to address most of them within January so this can ship in the same timeframe.
cc @NickLiffen