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

upload-sarif@v2 Doesn't support suppressions property in Sarif files. #1230

Closed
tdonaworth opened this issue Sep 2, 2022 · 12 comments
Closed

Comments

@tdonaworth
Copy link

I've recently noticed, when running semgrep, that findings that are suppressed in code with #nosemgrep flag the results with a suppressions property. This seems to be valid sarif formatting.

Example of a result:

{
    "$schema": "https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/schemas/sarif-schema-2.1.0.json",
    "runs": [
        {
            "invocations": [
                {
                    "executionSuccessful": true,
                    "toolExecutionNotifications": []
                }
            ],
            "results": [
                {
                    "locations": [
                        {
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "backend/opre_ops/django_config/settings/common.py",
                                    "uriBaseId": "%SRCROOT%"
                                },
                                "region": {
                                    "endColumn": 15,
                                    "endLine": 91,
                                    "snippet": {
                                        "text": "REST_FRAMEWORK = {  # nosemgrep: python.django.security.audit.django-rest-framework.missing-throttle-config.missing-throttle-config"
                                    },
                                    "startColumn": 1,
                                    "startLine": 91
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Django REST framework configuration is missing default rate- limiting options. This could inadvertently allow resource starvation or Denial of Service (DoS) attacks. Add 'DEFAULT_THROTTLE_CLASSES' and 'DEFAULT_THROTTLE_RATES' to add rate-limiting to your application."
                    },
                    "ruleId": "python.django.security.audit.django-rest-framework.missing-throttle-config.missing-throttle-config",
                    "suppressions": [
                        {
                            "kind": "inSource"
                        }
                    ]
                }
            ],
            "tool": {
                "driver": {
                    "name": "semgrep",
                    "rules": [
                        {
                            "defaultConfiguration": {
                                "level": "error"
                            },
                           ...
                    "semanticVersion": "0.111.1"
                }
            }
        }
    ],
    "version": "2.1.0"
}

When results like this are uploaded via the github/codeql-action/upload-sarif@v2 the results are still propagated as-if valid findings.

Shouldn't these be ignored, or flagged in some other way?

@tausbn
Copy link

tausbn commented Sep 2, 2022

Thank you for your question!

There are currently no concrete plans for adding support for alert suppression comments to CodeQL and Code Scanning.

However, we are continually reevaluating the need for this feature, especially seeing as it has some benefits over dismissing alerts through the Code Scanning UI.

Your comment has been added (as a point in favour of adding support for this) to our internal tracking of this matter.

@dentarg
Copy link

dentarg commented Nov 14, 2022

@tausbn That's very surprising to hear. Please also add my vote for supporting suppressions, it is very useful to be able to document and communicate suppressions in code. You can see an example of that at https://github.com/Starkast/wikimum/blob/49b0656dc593e387377390cd475c8423bff254eb/config/brakeman.ignore.

@ilmax
Copy link

ilmax commented Sep 5, 2024

@aeisenberg any news here? It's quite unfortunate that GitHub only partially supports the sarif specification. My use case is to ignore false positive in code (C# in my case) via the

#pragma warning disable CAXXX

#pragma warning restore CAXXX

roslyn correctly generates the required suppressions in the sarif file, but those are still reported in the GitHub UI.

@aeisenberg
Copy link
Contributor

Thanks for your comment. This feature has not been prioritized. As mentioned above, there are workarounds you can use. I'll mention this again to our product team.

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

No branches or pull requests

7 participants