Errors are displayed/counted as warnings

[jacob-ronstadt]

Using the python library, loading a sarif file that contains errors, then using "get_result_count_by_severity()" will display zero erros and give the number of warnings as number of warnings + number of erros. "get_records()" will show the errors, but they are classified as warnings.

The Visual Studio Code plugin displays these correctly as errors.

[balgillo]

Please can you tell us which tool this is, and share the SARIF file?

The SARIF standard defines three levels of severity, error, warning and note. The Sarif tools rely on those levels being set properly by the tools. Some tools use custom properties to store their own severity scale, e.g.

"properties":{"DevSkimSeverity":4}

But these are not comparable across tools. and at the moment sarif-tools can only use them in filters.

[balgillo]

Not much we can do about this, but I've added some words to the README to explain how some SAST tools don't use the SARIF standard severities correctly and therefore generic SARIF tools that are not specific to that SAST tool produce suboptimal output when reading their outputs.

[craslaw]

I don't think this issue should be closed yet. I have a similar problem, using SARIF output from Semgrep and converting it with the csv command of sarif-tools. This same SARIF output does render correctly in the VSCode Plugins for "SARIF Viewer" and "SARIF Explorer".

I compared the Semgrep SARIF output structure against the SARIF v2.1.0 spec and validated the SARIF output is aligned to the spec, but it is not being handled correctly by sarif-tools.

SARIF output structure

• "results" object:
  • contains "ruleId"
  • does not contain "rule", "level", or "kind"
• tool.driver.rules object:
  • contains defaultConfiguration.level
  • contains "id" with a value that matches the "ruleId" field of any corresponding "results" objects

SARIF spec

Here I list out each section that defines the evaluation logic that should apply based on the fields included in my SARIF output.

"results" object

3.27.9 kind property

If kind is absent, it SHALL default to "fail".

3.27.7 result.rule property

If rule is absent, it SHALL default to a reportingDescriptorReference object whose id property is set to thisObject.ruleId and whose index property is set to thisObject.ruleIndex. NOTE: If the relevant rule is defined by the driver (see §3.18.1), which is likely to be the most common case, then ruleId and/or ruleIndex suffice to identify the rule, and take up less space in the log file than rule.

3.27.10 result.level property

If kind has the value "fail" and level is absent, then level SHALL be determined by the following
procedure:
IF rule (§3.27.7) is present THEN
    LET theDescriptor be the reportingDescriptor object (§3.49) that it specifies.
    // Is there a configuration override for the level property?
...
// omitting section as there is no configuration override
...
    ELSE
        // There is no configuration override for level. Is there a default configuration for it?
        IF theDescriptor.defaultConfiguration.level (§3.49.14, §, §3.50.3) is present THEN
            SET level to theDescriptor.defaultConfiguration.level.
IF level has not yet been set THEN
    SET level to "warning".

tool.driver.rules object

3.49.14 defaultConfiguration property

A reportingDescriptor object MAY contain a property named defaultConfiguration whose value is a reportingConfiguration object (§3.50).

3.50.3 reportingConfiguration level property

A reportingConfiguration object MAY contain a property named level whose value is one of the strings "warning", "error", "note", or "none", with the same meanings as when those strings appear as the value of result.level (§3.27.10) or notification.level (§3.58.6). If level is absent, it SHALL default to "warning". If theDescriptor describes a rule, then if level is present, it SHALL provide the value for the level property of any result object (§3.27) whose ruleIndex (§3.27.6) or rule property (§3.27.7), either explicitly supplied or inferred from its default, identifies theDescriptor and which does not itself specify a level property. For details of the configuration property resolution procedure, see §3.27.10 (which illustrates the procedure for the specific case of the result.level property)

Summary

There should be a clear link between a result and its driver-defined rule, based on the results.rule inferring logic above.

So either one of the following is likely happening:

1. The link between a result and its driver-defined rule is not being inferred correctly, and the result level defaults to "warning".
2. The link between a result and its driver-defined rule is being inferred correctly, but reportingConfiguration.level is not being observed and it is defaulting to the "warning" level.

[debonte]

@craslaw, thanks for the details on what you're seeing, I've reopened the issue. We have a TODO comment about improving the logic that calculates a record's level. I'll use this issue to track that work.

As an example, I generated the following SARIF using semgrep:

{
  "$schema": "https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/schemas/sarif-schema-2.1.0.json",
  "runs": [
    {
      "results": [
        {
          "fingerprints": {
            "matchBasedId/v1": "0137c1fee54c9a785c51186d332d0ff67bc13858d64da5ff4e4c8dff1fc7e05fda867f153f84a843c438125dfe3ab0bb6b3c03235ae3c64c70b2364c4b16474b_0"
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "test.py",
                  "uriBaseId": "%SRCROOT%"
                },
                "region": {
                  "endColumn": 10,
                  "endLine": 2,
                  "snippet": {
                    "text": "y = x * 2"
                  },
                  "startColumn": 5,
                  "startLine": 2
                }
              }
            }
          ],
          "message": {
            "text": "2"
          },
          "properties": {},
          "ruleId": "-"
        }
      ],
      "tool": {
        "driver": {
          "name": "Semgrep OSS",
          "rules": [
            {
              "defaultConfiguration": {
                "level": "error"
              },
              "fullDescription": {
                "text": "2"
              },
              "help": {
                "markdown": "2",
                "text": "2"
              },
              "id": "-",
              "name": "-",
              "properties": {
                "precision": "very-high",
                "tags": []
              },
              "shortDescription": {
                "text": "Semgrep Finding: -"
              }
            }
          ],
          "semanticVersion": "1.90.0"
        }
      }
    }
  ],
  "version": "2.1.0"
}

Running sarif csv on this file, gave me the following CSV file, where the result is listed as a warning but should be an error given the defaultConfiguration.

Tool,Severity,Code,Description,Location,Line
Semgrep OSS,warning,-,2,test.py,2

[debonte]

@craslaw, I have a pending PR to fix this. If you have time, you could try out the wheel from the PR validation build and verify that it fixes your issue.

[debonte]

The fix has been released in v3.0.3 which is now availble on pypi.

[craslaw]

@debonte, just tested out v3.0.3 and it now handles warnings and errors as expected, thank you for the fix!