Skip to content

Add missing security tags #10180

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

Merged
merged 8 commits into from
Sep 2, 2022
Merged

Add missing security tags #10180

merged 8 commits into from
Sep 2, 2022

Conversation

erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Aug 25, 2022

I stumbled upon a query that had a missing @tag security.
So I took a look at the results of ql/missing-security-metadata, and added the missing tags were I could.

There still some queries with missing @security-severity, but they are also not tagged with any CWEs, so I'm not sure what to set the value to.

@erik-krogh erik-krogh marked this pull request as ready for review August 25, 2022 20:40
@erik-krogh erik-krogh requested review from a team as code owners August 25, 2022 20:40
@atorralba
Copy link
Contributor

According to #7613, I think the Java queries should have the security-severity tag removed, rather than the tag security added (or just be ignored).

Also, I think this needs a queryMetadata change note, akin to https://github.com/github/codeql/pull/8426/files#diff-5b76977c943c81b88d65fad2c4cad38a86773b31ddd1a3a744040eb4b81b5aaa.

RasmusWL
RasmusWL previously approved these changes Aug 26, 2022
Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python 👍 (thanks)

@erik-krogh erik-krogh removed the request for review from a team August 29, 2022 11:10
@github-actions github-actions bot removed the JS label Aug 29, 2022
* @problem.severity error
* @security-severity 5.8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention here was to remove the line below instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it was 👍

Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java 👍

@erik-krogh erik-krogh requested a review from RasmusWL August 29, 2022 11:48
@erik-krogh erik-krogh merged commit 6cee635 into github:main Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants