Skip to content
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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

amotl
Copy link
Member

@amotl amotl commented Nov 13, 2024

About

GitHub's CodeQL flags [1] a few spots unqualified with "Unused global variable" [2].

image

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

@cla-bot cla-bot bot added the cla-signed label Nov 13, 2024
apilevel = "2.0"
threadsafety = 1
paramstyle = "qmark"
apilevel = "2.0" # codeql[py/unused-global-variable]

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'apilevel' is not used.
threadsafety = 1
paramstyle = "qmark"
apilevel = "2.0" # codeql[py/unused-global-variable]
threadsafety = 1 # codeql[py/unused-global-variable]

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'threadsafety' is not used.
paramstyle = "qmark"
apilevel = "2.0" # codeql[py/unused-global-variable]
threadsafety = 1 # codeql[py/unused-global-variable]
paramstyle = "qmark" # codeql[py/unused-global-variable]

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'paramstyle' is not used.
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

This comment was marked as off-topic.

@cla-bot cla-bot bot removed the cla-signed label Nov 14, 2024
Comment on lines 64 to +82
- 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 }}
Copy link
Member Author

@amotl amotl Nov 14, 2024

Choose a reason for hiding this comment

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

Hi @aibaars. Thank you very much for submitting a fix to add the missing id: analyze per GH-677. The workflow recipe works well now, running to completion.

Copy link
Member Author

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

Copy link

@aibaars aibaars Nov 14, 2024

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"

Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants