-
Notifications
You must be signed in to change notification settings - Fork 31
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
Chore: Suppress unqualified CodeQL admonitions #676
Conversation
c469eae
to
fa2b5f4
Compare
fa2b5f4
to
4d2b6f8
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
- name: Perform CodeQL Analysis | ||
id: analyze | ||
uses: github/codeql-action/analyze@v3 | ||
with: | ||
category: "/language:${{matrix.language}}" | ||
# define the output folder for SARIF files | ||
output: sarif-results | ||
|
||
# Unlock inline mechanism to suppress CodeQL warnings. | ||
# https://github.com/github/codeql/issues/11427#issuecomment-1721059096 | ||
- name: Dismiss alerts | ||
# if: github.ref == 'refs/heads/main' | ||
uses: advanced-security/dismiss-alerts@v1 | ||
with: | ||
# specify a 'sarif-id' and 'sarif-file' | ||
sarif-id: ${{ steps.analyze.outputs.sarif-id }} | ||
sarif-file: sarif-results/${{ matrix.language }}.sarif | ||
env: | ||
GITHUB_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, it looks like the admonitions that have been opened are not automatically being dismissed again. On the CodeQL workflow run, there is a warning:
1 configuration not found
Warning: Code scanning may not have found all the alerts introduced by this pull request, because 1 configuration present on refs/heads/main was not found:
Actions workflow (codeql.yml)
⌛ .github/workflows/codeql.yml:analyze/language:python
-- https://github.com/crate/crate-python/pull/676/checks?check_run_id=32972101671
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to run an alert-suppression
query. That is a QL analysis that searches for the #codeql
comments. According to the example in the readme this can be done as follows:
- name: Initialize CodeQL
uses: github/codeql-action/init@v2
with:
languages: ${{ matrix.language }}
# run an 'alert-suppression' query
packs: "codeql/${{ matrix.language }}-queries:AlertSuppression.ql"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another problem, you'll likely end up with a bunch of pairs of new and fixed alerts because you are adding the #codeql
comments on the same line as the alert. CodeScanning uses a hash-based "fingerprint" to track an alert across commits. This hash is calculated from the 100 non-whitespace character counted from the start of the line on which the alert occurs. When you put a suppression comment on the same line, then CodeScanning considers the alert without the comment to be a different one. As a result the old one is closed as "fixed" and a new one is created in the "dismissed" state.
You can put the comment on the line before the alert to avoid this problem in most cases. This is why the documentation recommends the following style:
# codeql[py/unused-global-variable]
apilevel = "2.0"
However, if you have alerts very close to each other (fewer than 100 characters apart) you still see this behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aibaars,
thank you so much for your elaborate and valuable comments. 💯
I've adjusted the patch to use your suggestions, which resolved the admonitions by @Github-advanced-security at #676 (review). Excellent! 🌻
We guess that will also resolve the items at security/code-scanning after merging?
With kind regards,
Andreas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this warning / stalling workflow task may resolve itself also after merging?
1 configuration not found
Warning: Code scanning cannot determine the alerts introduced by this pull request, because 1 configuration present on
refs/heads/main
was not found:-- https://github.com/crate/crate-python/pull/676/checks?check_run_id=33421035019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: After merging, the admonitions in GitHub » Security » Code scanning have not been resolved automatically.
- https://github.com/crate/crate-python/security/code-scanning/107
- https://github.com/crate/crate-python/security/code-scanning/108
- https://github.com/crate/crate-python/security/code-scanning/109
I guess this can be expected, because there is a corresponding note:
Rule
py/unused-global-variable
is not supported by Copilot Autofix for CodeQL.
I will just go ahead and dismiss those manually?
8c4de4d
to
3085922
Compare
GitHub's CodeQL flags [1] those spots with "Unused global variable" [2]. Based on a suggestion [3], this patch attempts to use the `advanced-security/dismiss-alerts` [4] GitHub Action recipe to provide measures to suppress CodeQL flagging by using inline code annotations. [1] https://github.com/crate/crate-python/security/code-scanning [2] https://codeql.github.com/codeql-query-help/python/py-unused-global-variable/ [3] Issue 11427 at https://github.com/github/codeql/issues [4] https://github.com/advanced-security/dismiss-alerts
3085922
to
be97a8d
Compare
About
GitHub's CodeQL flags [1] a few spots unqualified with "Unused global variable" [2].
Solution?
Based on a suggestion [3], this patch attempts to use the
advanced-security/dismiss-alerts
[4] GitHub Action recipe to provide measures to suppress CodeQL flagging by using inline code annotations.[1] https://github.com/crate/crate-python/security/code-scanning
[2] https://codeql.github.com/codeql-query-help/python/py-unused-global-variable/
[3] github/codeql#11427 (comment)
[4] https://github.com/advanced-security/dismiss-alerts
References