fix: fail on none severities (#534)

safety/scan/ecosystems/python/main.py

@@ -1,6 +1,6 @@
-
 from datetime import datetime
 import itertools
+import logging
 from typing import List
 from safety_schemas.models import FileType, PythonDependency, ClosestSecureVersion, \
     ConfigModel, PythonSpecification, RemediationModel, DependencyResultModel, \
@@ -26,6 +26,9 @@
 from packaging.specifiers import SpecifierSet
 
 
+LOG = logging.getLogger(__name__)
+
+
 def ignore_vuln_if_needed(dependency: PythonDependency, file_type: FileType, 
                           vuln_id: str, cve, ignore_vulns,
                           ignore_unpinned: bool, ignore_environment: bool,
@@ -75,15 +78,46 @@ def ignore_vuln_if_needed(dependency: PythonDependency, file_type: FileType,
             code=IgnoreCodes.unpinned_specification, reason=reason, 
             specifications=specifications)
 
+
 def should_fail(config: ConfigModel, vulnerability: Vulnerability) -> bool:
-    if config.depedendency_vulnerability.fail_on.enabled and vulnerability.severity:
-        if vulnerability.severity.cvssv3 and vulnerability.severity.cvssv3.get("base_severity", None):
-            severity_label = VulnerabilitySeverityLabels(
-                vulnerability.severity.cvssv3["base_severity"].lower())
-            if severity_label in config.depedendency_vulnerability.fail_on.cvss_severity:
-                return True
+    if not config.depedendency_vulnerability.fail_on.enabled:
+        return False
+
+    # If Severity is None type, it will be considered as UNKNOWN and NONE
+    # They are not the same, but we are handling like the same when a 
+    # vulnerability does not have a severity value.
+    severities = [VulnerabilitySeverityLabels.NONE,
+                  VulnerabilitySeverityLabels.UNKNOWN]
+
+    if vulnerability.severity and vulnerability.severity.cvssv3:
+        base_severity = vulnerability.severity.cvssv3.get(
+            "base_severity")
+
+        if base_severity:
+            base_severity = base_severity.lower()
+
+        # A vulnerability only has a single severity value, this is just
+        # to handle cases where the severity value is not in the expected
+        # format and fallback to the default severity values [None, unknown].
+        matched_severities = [
+            label
+            for label in VulnerabilitySeverityLabels
+            if label.value == base_severity
+        ]
+
+        if matched_severities:
+            severities = matched_severities
+        else:
+            LOG.warning(
+                f"Unexpected base severity value {base_severity} for "
+                f"{vulnerability.vulnerability_id}"
+            )
+
+    return any(
+        severity in config.depedendency_vulnerability.fail_on.cvss_severity
+        for severity in severities
+    )
 
-    return False
 
 def get_vulnerability(vuln_id: str, cve,
                       data, specifier,
@@ -336,4 +370,3 @@ def remediate(self):
                                                    closest_secure=closest_secure if recommended else None, 
                                                    recommended=recommended, 
                                                    other_recommended=other_recommended)
-

tests/scan/ecosystems/python/test_main.py

@@ -0,0 +1,65 @@
+import unittest
+from unittest.mock import MagicMock
+from safety.scan.ecosystems.python.main import should_fail, VulnerabilitySeverityLabels
+
+class TestMain(unittest.TestCase):
+    def setUp(self):
+        self.config = MagicMock()
+        self.vulnerability = MagicMock()
+
+    def test_fail_on_disabled(self):
+        self.config.depedendency_vulnerability.fail_on.enabled = False
+        result = should_fail(self.config, self.vulnerability)
+        self.assertFalse(result)
+
+    def test_severity_none(self):
+        self.config.depedendency_vulnerability.fail_on.enabled = True
+        self.vulnerability.severity = None
+        result = should_fail(self.config, self.vulnerability)
+        self.assertFalse(result)
+
+    def test_severity_none_with_fail_on_unknow_none(self):
+        self.config.depedendency_vulnerability.fail_on.enabled = True        
+        self.vulnerability.severity = None
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.UNKNOWN]
+        self.assertTrue(should_fail(self.config, self.vulnerability))
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.NONE]
+        self.assertTrue(should_fail(self.config, self.vulnerability))
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.UNKNOWN, 
+                                                                        VulnerabilitySeverityLabels.NONE]
+        self.assertTrue(should_fail(self.config, self.vulnerability))
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.LOW, 
+                                                                        VulnerabilitySeverityLabels.MEDIUM]
+        self.assertFalse(should_fail(self.config, self.vulnerability))
+
+        self.vulnerability.severity = MagicMock()
+        self.vulnerability.severity.cvssv3 = {"base_severity": "NONE"}
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.NONE]
+        self.assertTrue(should_fail(self.config, self.vulnerability))
+
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.UNKNOWN]
+        self.assertFalse(should_fail(self.config, self.vulnerability))
+
+        self.vulnerability.severity.cvssv3 = {"base_severity": "UNKNOWN"}
+        self.assertTrue(should_fail(self.config, self.vulnerability))
+
+    def test_known_severity_failure(self):
+        self.config.depedendency_vulnerability.fail_on.enabled = True
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.HIGH]
+        self.vulnerability.severity.cvssv3 = {"base_severity": "HIGH"}
+        result = should_fail(self.config, self.vulnerability)
+        self.assertTrue(result)
+
+    def test_unexpected_severity_with_warning(self):
+        self.config.depedendency_vulnerability.fail_on.enabled = True
+        self.config.depedendency_vulnerability.fail_on.cvss_severity = [VulnerabilitySeverityLabels.HIGH]
+        self.vulnerability.severity.cvssv3 = {"base_severity": "UNKNOWN_SEVERITY"}
+        with self.assertLogs(level='WARNING') as log:
+            result = should_fail(self.config, self.vulnerability)
+            self.assertIn("Unexpected base severity value", log.output[0])
+        self.assertFalse(result)