Added MissingSecurityMetadata query

[atorralba]

Adds a QL for QL query that raises a warning whenever a .ql file is tagged as @tag security but it doesn't have a @security-severity tag, or vice versa.

[MathiasVP]

Should we maybe exclude queries in the experimental directory?

[atorralba]

Good point, thanks!

Also, can we prevent the .ql files used as "sources" from being considered test queries? I could always add a blank .expected file for them, but it seems a bit counter-intuitive.

[MathiasVP]

Also, can we prevent the .ql files used as "sources" from being considered test queries? I could always add a blank .expected file for them, but it seems a bit counter-intuitive.

Hm, yes. That is an interesting issue 😂. I don't know if that's possible.

[erik-krogh]

Maybe also exclude the examples directory. Most of the JS warnings are from that folder.

Also, can we prevent the .ql files used as "sources" from being considered test queries? I could always add a blank .expected file for them, but it seems a bit counter-intuitive.

Not currently possible.
Most of the time the PR checks work as the best test-case, as it's effectively a dist-upgrade on every PR.

[atorralba]

Maybe also exclude the examples directory. Most of the JS warnings are from that folder.

Done, thanks!

Not currently possible.

👍 Just added the blank .expected files for these tests, but will keep in mind what you mentioned about just looking at the PR checks for future queries.

[erik-krogh]

Just added the blank .expected files for these tests, but will keep in mind what you mentioned about just looking at the PR checks for future queries.

Yes, it's specifically the Code scanning results / CodeQL line that is interesting to look at.
You can see 38 results with the current version of the query, most of which looks like TPs 🙈

---

Currently you check that all queries that has a @tag security also has a @security-severity.
What if a query has a @security-severity but no @tag security? Should we also flag those?

[MathiasVP]

Yes, it's specifically the Code scanning results / CodeQL line that is interesting to look at.
You can see 38 results with the current version of the query, most of which looks like TPs 🙈

There are many very old C++ queries that have no security severity because they're not part of any modern suite. To remove those results, it would be helpful to exclude queries that have no @precision tag.

It would also remove the FPs from results like ExternalAPIsUsedWithUntrustedData.

[atorralba]

What if a query has a @security-severity but no @tag security? Should we also flag those?

I think that's less of a problem for us since AFAIK the requirement is "all security queries should have a security-severity score", not the other way around (which means a customer noticing this is less likely). But yes, it's indeed an inconsistency that could point out a mistake being made, so we could flag it too (although with a different QL for QL query? If not, the alert message may become confusing).

To remove those results, it would be helpful to exclude queries that have no @precision tag.

Makes sense! Done in 82b2fd2.

[erik-krogh]

If not, the alert message may become confusing).

You could change the alert message depending on what is flagged.

from TopLevel t, string msg
where 
  missingSeverity(t) and msg = "This query file is missing a `@security-severity` tag."
  or
  missingSecurityTag(t) and msg = "This query file is missing a `@tag secrity`". 
select t, msg

[atorralba]

You could change the alert message depending on what is flagged.

Yep, that's less complicated than what I initially imagined :-P

Done in fd4c9fd, I also renamed the query so that it adjusts better to what it's doing now.

[MathiasVP]

LGTM!