Skip to content

Commit

Permalink
Improve logic for calculating result level (#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
debonte authored Sep 30, 2024
1 parent d41747d commit 6732313
Show file tree
Hide file tree
Showing 3 changed files with 255 additions and 7 deletions.
8 changes: 2 additions & 6 deletions sarif/sarif_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from sarif.sarif_file_utils import (
SARIF_SEVERITIES_WITHOUT_NONE,
SARIF_SEVERITIES_WITH_NONE,
read_result_severity,
)
from sarif.filter.general_filter import GeneralFilter
from sarif.filter.filter_stats import FilterStats
Expand Down Expand Up @@ -250,12 +251,7 @@ def result_to_record(self, result, include_blame_info=False):
file_path = file_path[prefixlen:]
break

# Get the error severity, if included, and code
severity = result.get(
"level", "warning"
) # If an error has no specified level then by default it is a warning
# TODO: improve this logic to match the rules in
# https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html#_Toc141790898
severity = read_result_severity(result, self.run_data)

if "message" in result:
# per RFC3629 At least one of the text (§3.11.8) or id (§3.11.10) properties SHALL be present https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#RFC3629
Expand Down
97 changes: 96 additions & 1 deletion sarif/sarif_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""

import textwrap
from typing import Tuple
from typing import Literal, Tuple, Union

# SARIF severity levels as per
# https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html#_Toc141790898
Expand Down Expand Up @@ -100,6 +100,101 @@ def read_result_location(result) -> Tuple[str, str]:
return (file_path, line_number)


def read_result_rule(result, run) -> Tuple[Union[dict, None], int]:
"""
Returns the corresponding rule object for the specified result, plus its index
in the rules array. Follows the rules at
https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html#_Toc141790895
"""
ruleIndex = result.get("ruleIndex")
ruleId = result.get("ruleId")
rule = result.get("rule")

if rule:
if ruleIndex is None:
ruleIndex = rule.get("index")

if ruleId is None:
ruleId = rule.get("id")

rules = run.get("tool", {}).get("driver", {}).get("rules", [])

if ruleIndex is not None and ruleIndex >= 0 and ruleIndex < len(rules):
return (rules[ruleIndex], ruleIndex)

if ruleId:
for i, rule in enumerate(rules):
if rule.get("id") == ruleId:
return (rule, i)

return (None, -1)


def read_result_invocation(result, run):
"""
Extract the invocation metadata for the result, following the rules at
https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html#_Toc141790917
"""
invocationIndex = result.get("provenance", {}).get("invocationIndex")
if invocationIndex is None:
return None

invocations = run.get("invocations")

if invocations and invocationIndex >= 0 and invocationIndex < len(invocations):
return invocations[invocationIndex]

return None


def read_result_severity(result, run) -> Literal["none", "note", "warning", "error"]:
"""
Extract the severity level from the result following the rules at
https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html#_Toc141790898
"""
severity = result.get("level")
if severity:
return severity

# If kind has any value other than "fail", then if level is absent,
# it SHALL default to "none"
kind = result.get("kind", "fail")
if kind and kind != "fail":
return "none"

# If kind has the value "fail" and level is absent, then...
rule, ruleIndex = read_result_rule(result, run)
if rule:
# Honor the invocation's configuration override if present...
invocation = read_result_invocation(result, run)
if invocation:
ruleConfigurationOverrides = invocation.get("ruleConfigurationOverrides", [])
override = next(
(
override
for override in ruleConfigurationOverrides
if override.get("descriptor", {}).get("id") == rule.get("id") or
override.get("descriptor", {}).get("index") == ruleIndex
),
None,
)

if override:
overrideLevel = override.get("configuration", {}).get("level")
if overrideLevel:
return overrideLevel

# Otherwise, use the rule's default configuraiton if present...
defaultConfiguration = rule.get("defaultConfiguration")
if defaultConfiguration:
severity = defaultConfiguration.get("level")
if severity:
return severity

# Otherwise, fall back to warning
return "warning"


def record_sort_key(record: dict) -> str:
"""Get a sort key for the record."""
return (
Expand Down
157 changes: 157 additions & 0 deletions tests/test_sarif_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,160 @@ def test_combine_code_and_description_long_code():
long_code, "wow that's a long code"
)
assert cd == long_code


def test_read_result_rule():
run = {"tool":
{"driver":
{"rules": [
{"id": "id0", "defaultConfiguration": {"level": "none"}},
{"id": "id1", "defaultConfiguration": {"level": "error"}}
]}}}
rule_id0 = run["tool"]["driver"]["rules"][0]
rule_id1 = run["tool"]["driver"]["rules"][1]

result = {}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule is None
assert ruleIndex == -1

result = {"ruleIndex": 1}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule == rule_id1
assert ruleIndex == 1

result = {"rule": { "index": 1}}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule == rule_id1
assert ruleIndex == 1

result = {"ruleId": "id1"}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule == rule_id1
assert ruleIndex == 1

result = {"rule": { "id": "id1"}}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule == rule_id1
assert ruleIndex == 1

result = {"ruleIndex": 0}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, run)
assert rule == rule_id0
assert ruleIndex == 0

result = {"ruleIndex": 0}
(rule, ruleIndex) = sarif_file_utils.read_result_rule(result, {})
assert rule is None
assert ruleIndex == -1


def test_read_result_invocation():
run = {"invocations": [
{"foo": 1},
{"bar": "baz"}
]}

result = {}
invocation = sarif_file_utils.read_result_invocation(result, run)
assert invocation is None

result = {"provenance": {}}
invocation = sarif_file_utils.read_result_invocation(result, run)
assert invocation is None

result = {"provenance": {"invocationIndex": 0}}
invocation = sarif_file_utils.read_result_invocation(result, {})
assert invocation is None

result = {"provenance": {"invocationIndex": -1}}
invocation = sarif_file_utils.read_result_invocation(result, run)
assert invocation is None

result = {"provenance": {"invocationIndex": 2}}
invocation = sarif_file_utils.read_result_invocation(result, run)
assert invocation is None

result = {"provenance": {"invocationIndex": 1}}
invocation = sarif_file_utils.read_result_invocation(result, run)
assert invocation == run["invocations"][1]


def test_read_result_severity():
result = {"level": "error"}
severity = sarif_file_utils.read_result_severity(result, {})
assert severity == "error"

# If kind has any value other than "fail", then if level is absent, it SHALL default to "none"...
result = {"kind": "other"}
severity = sarif_file_utils.read_result_severity(result, {})
assert severity == "none"

run = {"invocations": [
{"ruleConfigurationOverrides": [ {"descriptor": {"id": "id1"}, "configuration": {"level": "note"}} ]},
{"ruleConfigurationOverrides": [ {"descriptor": {"index": 1}, "configuration": {"level": "note"}} ]},
{ }
],
"tool":
{"driver":
{"rules": [
{"id": "id0", "defaultConfiguration": {"level": "none"}},
{"id": "id1", "defaultConfiguration": {"level": "error"}}
]}
}
}

# If kind has the value "fail" and level is absent, then level SHALL be determined by the following procedure:
# IF rule is present THEN
# LET theDescriptor be the reportingDescriptor object that it specifies.
# # Is there a configuration override for the level property?
# IF result.provenance.invocationIndex is >= 0 THEN
# LET theInvocation be the invocation object that it specifies.
# IF theInvocation.ruleConfigurationOverrides is present
# AND it contains a configurationOverride object whose
# descriptor property specifies theDescriptor THEN
# LET theOverride be that configurationOverride object.
# IF theOverride.configuration.level is present THEN
# Set level to theConfiguration.level.
result = {"ruleIndex": 1, "provenance": {"invocationIndex": 0}}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "note"

result = {"ruleIndex": 1, "provenance": {"invocationIndex": 1}}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "note"

# ELSE
# # There is no configuration override for level. Is there a default configuration for it?
# IF theDescriptor.defaultConfiguration.level is present THEN
# SET level to theDescriptor.defaultConfiguration.level.

result = {"ruleIndex": 1}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "error"

result = {"rule": { "index": 1}}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "error"

result = {"ruleId": "id1"}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "error"

result = {"rule": { "id": "id1"}}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "error"

result = {"ruleIndex": 1, "provenance": {"invocationIndex": 2}}
severity = sarif_file_utils.read_result_severity(result, run)
assert severity == "error"

# IF level has not yet been set THEN
# SET level to "warning".
result = {}
severity = sarif_file_utils.read_result_severity(result, {})
assert severity == "warning"

result = {"ruleIndex": -1}
severity = sarif_file_utils.read_result_severity(result, {})
assert severity == "warning"

0 comments on commit 6732313

Please sign in to comment.