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.

Sign up for GitHub

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

Jump to bottom

Chore: Suppress unqualified CodeQL admonitions #676

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

Chore: Suppress unqualified CodeQL admonitions #676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d2b6f8
Chore: Suppress unqualified CodeQL admonitions
amotl Nov 13, 2024
8c4de4d
Add missing `id:` field
aibaars Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/codeql.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,21 @@ jobs:
uv pip install --system '.[test]'

- 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 }}
Comment on lines 64 to +82
Copy link
Member Author

@amotl amotl Nov 14, 2024 •
edited
Loading

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 •
edited
Loading

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.

6 changes: 3 additions & 3 deletions src/crate/client/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@
# regex!
__version__ = "1.0.0"

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 # codeql[py/unused-global-variable]

Check notice

Code scanning / CodeQL

Unused global variable Note

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

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'paramstyle' is not used.
Loading