Repository navigation
upload-sarif@v2 Doesn't support suppressions property in Sarif files. #1230
Description
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?
Reacted by Patrik Ragnarsson, Danny Colmenares, Jim Witschey, Rocky Warren and Arseniy Terekhin
Activity
tausbn commentedon 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 commentedon 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.
ykim-r7 commentedon May 2, 2023
I am getting
instance.runs[0].results[5].suppressions[0] additionalProperty "status" exists in instance when not allowedwhen uploading an otherwise valid sarif file.
The "status" property is supported in the sarif spec: https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317736:~:text=to%20be%20ignored.-,3.35.3%20status%20property,-A%20suppression%20object
All other properties of the suppression object are supported except for this one. Would really like to see a fix for this.
EDIT: Looking at the source schema, the "status" property has erroneously been named "state":
codeql-action/src/sarif_v2.1.0_schema.json
Line 2698 in 8ca5570
I've opened a PR to fix this: #1667
aeisenberg commentedon May 2, 2023
Thanks for raising the PR. I think it's generally the right thing to do. However, I'm seeing that the version of the schema we have is quite old and there are a few other changes to it that we should pick up.
Current version: https://github.com/oasis-tcs/sarif-spec/blob/main/Schemata/sarif-schema-2.1.0.json
Version currently copied to this repo: https://github.com/oasis-tcs/sarif-spec/blob/3bfcda27093e0dd751ef13764bd215a446314282/Schemata/sarif-schema-2.1.0.json
I'll create a new PR with the latest version of the spec.
aeisenberg commentedon May 2, 2023
Please see the updated PR here. #1668
dentarg commentedon May 2, 2023
@aeisenberg can you comment on this?
Now there is?
aeisenberg commentedon May 2, 2023
Our plans haven't changed. Code scanning does not support suppressions. The linked PR is only to ensure that the upload action can gracefully accept the
suppressionsproperty. It will be ignored by code scanning.aibaars commentedon May 3, 2023
@dentarg As @aeisenberg mentions, at the moment, CodeScanning does not natively support suppressions. However, you can use https://github.com/advanced-security/dismiss-alerts to automatically mark any suppressed alerts as dismissed.
ykim-r7 commentedon May 8, 2023
@aibaars , I would like to see native support for suppressions without having to add another action to the pipeline. Suppressions are already respected in the VSCode sarif viewer and the official Microsoft sarif validator.
aeisenberg commentedon May 8, 2023
Thank you for the feedback. We will communicate this to our product team and determine if we can get this feature on the roadmap.
ilmax commentedon 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
roslyn correctly generates the required suppressions in the sarif file, but those are still reported in the GitHub UI.
aeisenberg commentedon Sep 5, 2024
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.