Validate flaw impact and RH CVSSv3 score properly (#890)

This PR consolidates the flaw's impact and RH CVSSv3 score validations
as they are currently inconsistent and incomplete.

If RH CVSSv3 is present, score and impact should comply with the
following:
- RH CVSSv3 score is not zero and flaw impact is set
- RH CVSSv3 score is zero and flaw impact is not set

Closes OSIDB-3738

.secrets.baseline

@@ -372,7 +372,7 @@
         "filename": "osidb/tests/endpoints/flaws/test_flaws.py",
         "hashed_secret": "3c3b274d119ff5a5ec6c1e215c1cb794d9973ac1",
         "is_verified": false,
-        "line_number": 1335,
+        "line_number": 1344,
         "is_secret": false
       }
     ],
@@ -475,5 +475,5 @@
       }
     ]
   },
-  "generated_at": "2025-01-23T09:12:19Z"
+  "generated_at": "2025-01-23T19:49:54Z"
 }

apps/trackers/tests/test_jira.py

@@ -1825,12 +1825,18 @@ def test_generate_cve_cvss_cwe(
         if flaw_cvss_present:
             assert flaw.cvss_scores.all().count() == 0
             flawcvss = []
-            flawcvss.append(FlawCVSSFactory(flaw=flaw))
+            flawcvss.append(FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.NIST))
             assert flaw.cvss_scores.all().count() == 1
             if multiscore:
-                flawcvss.append(FlawCVSSFactory(flaw=flaw))
-                flawcvss.append(FlawCVSSFactory(flaw=flaw))
-                flawcvss.append(FlawCVSSFactory(flaw=flaw))
+                flawcvss.append(
+                    FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.CVEORG)
+                )
+                flawcvss.append(
+                    FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.CVEORG)
+                )
+                flawcvss.append(
+                    FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.CVEORG)
+                )
 
         if aff_cvss_present:
             assert affect.cvss_scores.all().count() == 0
@@ -2367,7 +2373,11 @@ def test_generate_query_multiflaw(self):
             source="REDHAT",
             cwe_id="CWE-1",
         )
-        flwcvss = FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.REDHAT)
+        flwcvss = FlawCVSSFactory(
+            flaw=flaw,
+            issuer=FlawCVSS.CVSSIssuer.REDHAT,
+            version=FlawCVSS.CVSSVersion.VERSION2,
+        )
         affect = AffectFactory(
             flaw=flaw,
             ps_module="foo-module",
@@ -2386,7 +2396,11 @@ def test_generate_query_multiflaw(self):
             cwe_id="CWE-2",
             created_dt=datetime(2024, 10, 1, tzinfo=timezone.utc),
         )
-        flwcvss2 = FlawCVSSFactory(flaw=flaw2, issuer=FlawCVSS.CVSSIssuer.REDHAT)
+        flwcvss2 = FlawCVSSFactory(
+            flaw=flaw2,
+            issuer=FlawCVSS.CVSSIssuer.REDHAT,
+            version=FlawCVSS.CVSSVersion.VERSION2,
+        )
         affect2 = AffectFactory(
             flaw=flaw2,
             ps_module="foo-module",
@@ -2407,7 +2421,11 @@ def test_generate_query_multiflaw(self):
             cwe_id="CWE-3",
             created_dt=datetime(2024, 9, 1, tzinfo=timezone.utc),
         )
-        flwcvss3 = FlawCVSSFactory(flaw=flaw3, issuer=FlawCVSS.CVSSIssuer.REDHAT)
+        flwcvss3 = FlawCVSSFactory(
+            flaw=flaw3,
+            issuer=FlawCVSS.CVSSIssuer.REDHAT,
+            version=FlawCVSS.CVSSVersion.VERSION2,
+        )
         affect3 = AffectFactory(
             flaw=flaw3,
             ps_module="foo-module",
@@ -2426,7 +2444,11 @@ def test_generate_query_multiflaw(self):
             cwe_id="CWE-4",
             created_dt=datetime(2024, 10, 2, tzinfo=timezone.utc),
         )
-        flwcvss4 = FlawCVSSFactory(flaw=flaw4, issuer=FlawCVSS.CVSSIssuer.REDHAT)
+        flwcvss4 = FlawCVSSFactory(
+            flaw=flaw4,
+            issuer=FlawCVSS.CVSSIssuer.REDHAT,
+            version=FlawCVSS.CVSSVersion.VERSION2,
+        )
         affect4 = AffectFactory(
             flaw=flaw4,
             ps_module="foo-module",

apps/workflows/tests/test_models.py

@@ -3,7 +3,15 @@
 from apps.workflows.exceptions import LastStateException, MissingRequirementsException
 from apps.workflows.models import Check, Condition, State, Workflow
 from apps.workflows.workflow import WorkflowFramework, WorkflowModel
-from osidb.models import Affect, Flaw, FlawReference, FlawSource, Impact, Tracker
+from osidb.models import (
+    Affect,
+    Flaw,
+    FlawCVSS,
+    FlawReference,
+    FlawSource,
+    Impact,
+    Tracker,
+)
 from osidb.tests.factories import (
     AffectFactory,
     FlawCVSSFactory,
@@ -33,7 +41,12 @@ class TestCheck:
         "field,factory",
         [
             ("affects", lambda flaw: AffectFactory(flaw=flaw)),
-            ("cvss_scores", lambda flaw: FlawCVSSFactory(flaw=flaw)),
+            (
+                "cvss_scores",
+                lambda flaw: FlawCVSSFactory(
+                    flaw=flaw, issuer=FlawCVSS.CVSSIssuer.NIST
+                ),
+            ),
             ("package_versions", lambda flaw: PackageFactory(flaw=flaw)),
         ],
     )

collectors/nvd/tests/test_collectors.py

@@ -261,7 +261,7 @@ def test_reset_flag_on_removal(self, old_flag, new_flag):
         """
         test that NIST CVSS validation flag is correctly adjusted when NVD CVSSv3 is removed
         """
-        flaw = FlawFactory()
+        flaw = FlawFactory(impact=Impact.LOW)
         FlawCVSSFactory(
             flaw=flaw,
             issuer=FlawCVSS.CVSSIssuer.REDHAT,

docs/CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - Removed `last_validated_dt` from exposed JSON Flaw History data (OSIDB-3814), handled edge-case that would cause failure (OSIDB-3858)
 - Trim Jira task summary if flaw's `cve_id` and `title` are too long (OSIDB-3847)
+- Validate that a flaw has an impact set and RH CVSSv3 score is non-zero,
+  or it does not have an impact set and RH CVSSv3 score is zero (OSIDB-3738)
 
 ## [4.6.5] - 2025-01-10
 ### Changed

osidb/models/flaw/cvss.py

@@ -51,8 +51,21 @@ class Meta:
             GinIndex(fields=["acl_read"]),
         ]
 
-    def _validate_empty_impact_and_cvss_score(self, **kwargs):
-        if not self.flaw.impact and self.cvss_object.base_score != Decimal("0.0"):
-            raise ValidationError(
-                {"cvss_score": ["CVSS score must be 0.0 for flaws with no impact"]}
-            )
+    def _validate_rh_cvss3_and_impact(self, **kwargs):
+        """
+        Validate that flaw's RH CVSSv3 score and impact comply with the following:
+        * RH CVSSv3 score is not zero and flaw impact is set
+        * RH CVSSv3 score is zero and flaw impact is not set
+        """
+        if (
+            self.issuer == self.CVSSIssuer.REDHAT
+            and self.version == self.CVSSVersion.VERSION3
+        ):
+            if self.flaw.impact and self.cvss_object.base_score == Decimal("0.0"):
+                raise ValidationError(
+                    "RH CVSSv3 score must not be zero if flaw impact is set."
+                )
+            if not self.flaw.impact and self.cvss_object.base_score != Decimal("0.0"):
+                raise ValidationError(
+                    "RH CVSSv3 score must be zero if flaw impact is not set."
+                )

osidb/models/flaw/flaw.py

@@ -456,20 +456,27 @@ def _validate_impact_and_cve_description(self, **kwargs):
                 **kwargs,
             )
 
-    def _validate_empty_impact_and_cvss_score(self, **kwargs):
+    def _validate_rh_cvss3_and_impact(self, **kwargs):
         """
-        Checks that if impact is None, CVSSv3 score must be zero.
+        Validate that flaw's RH CVSSv3 score and impact comply with the following:
+        * RH CVSSv3 score is not zero and flaw impact is set
+        * RH CVSSv3 score is zero and flaw impact is not set
         """
         from .cvss import FlawCVSS
 
         rh_cvss3 = self.cvss_scores.filter(
             version=FlawCVSS.CVSSVersion.VERSION3, issuer=FlawCVSS.CVSSIssuer.REDHAT
         ).first()
 
-        if not self.impact and rh_cvss3 and rh_cvss3.score != Decimal("0.0"):
-            raise ValidationError(
-                {"impact": ["Impact is set to None but CVSSv3 score is not zero."]}
-            )
+        if rh_cvss3:
+            if rh_cvss3.cvss_object.base_score == Decimal("0.0") and self.impact:
+                raise ValidationError(
+                    "Flaw impact must not be set if RH CVSSv3 score is zero."
+                )
+            if rh_cvss3.cvss_object.base_score != Decimal("0.0") and not self.impact:
+                raise ValidationError(
+                    "Flaw impact must be set if RH CVSSv3 score is not zero."
+                )
 
     def _validate_cve_description_and_requires_cve_description(self, **kwargs):
         """

osidb/tests/endpoints/flaws/test_cvss_scores.py

@@ -1,7 +1,7 @@
 import pytest
 from rest_framework import status
 
-from osidb.models import FlawCVSS
+from osidb.models import FlawCVSS, Impact
 from osidb.tests.factories import AffectFactory, FlawCVSSFactory, FlawFactory
 
 pytestmark = pytest.mark.unit
@@ -17,7 +17,7 @@ def test_flawcvss_create(self, auth_client, test_api_uri):
         """
         Test the creation of FlawCVSS records via a REST API POST request.
         """
-        flaw = FlawFactory()
+        flaw = FlawFactory(impact=Impact.LOW)
         cvss_data = {
             "issuer": FlawCVSS.CVSSIssuer.REDHAT,
             "cvss_version": FlawCVSS.CVSSVersion.VERSION3,
@@ -55,7 +55,12 @@ def test_flawcvss_update(self, auth_client, test_api_uri):
         Test the update of FlawCVSS records via a REST API PUT request.
         """
         flaw = FlawFactory()
-        cvss = FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.REDHAT, comment="")
+        cvss = FlawCVSSFactory(
+            flaw=flaw,
+            issuer=FlawCVSS.CVSSIssuer.REDHAT,
+            version=FlawCVSS.CVSSVersion.VERSION2,
+            comment="",
+        )
 
         response = auth_client().get(
             f"{test_api_uri}/flaws/{str(flaw.uuid)}/cvss_scores/{cvss.uuid}"
@@ -83,7 +88,7 @@ def test_flawcvss_delete(self, auth_client, test_api_uri):
         """
         flaw = FlawFactory()
         AffectFactory(flaw=flaw)
-        cvss = FlawCVSSFactory(flaw=flaw)
+        cvss = FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.NIST)
 
         url = f"{test_api_uri}/flaws/{str(flaw.uuid)}/cvss_scores/{cvss.uuid}"
         response = auth_client().get(url)

osidb/tests/endpoints/flaws/test_flaws.py

@@ -111,7 +111,7 @@ def test_get_flaw_with_cvss(self, auth_client, test_api_uri):
         assert response.status_code == status.HTTP_200_OK
         assert len(response.data["cvss_scores"]) == 0
 
-        FlawCVSSFactory(flaw=flaw)
+        FlawCVSSFactory(flaw=flaw, issuer=FlawCVSS.CVSSIssuer.NIST)
 
         response = auth_client().get(f"{test_api_uri}/flaws/{flaw.uuid}")
         assert response.status_code == status.HTTP_200_OK
@@ -327,18 +327,27 @@ def test_list_flaws_empty_cvss(
         body = response.json()
         assert body["count"] == 0
 
-        flaw = FlawFactory()
+        flaw = FlawFactory(impact=Impact.LOW)
 
         response = auth_client().get(f"{test_api_uri}/flaws?{filter_name}__isempty=1")
         assert response.status_code == 200
         body = response.json()
         assert body["count"] == 1
 
-        FlawCVSSFactory(
-            flaw=flaw,
-            issuer=issuer,
-            version=version,
-        )
+        # RH CVSSv3 needs to match with flaw impact
+        if filter_name == "cvss3_rh":
+            FlawCVSSFactory(
+                flaw=flaw,
+                issuer=issuer,
+                version=version,
+                vector="CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H",
+            )
+        else:
+            FlawCVSSFactory(
+                flaw=flaw,
+                issuer=issuer,
+                version=version,
+            )
         response = auth_client().get(f"{test_api_uri}/flaws?{filter_name}__isempty=1")
         assert response.status_code == 200
         body = response.json()

osidb/tests/endpoints/flaws/test_unembargo.py

@@ -93,7 +93,7 @@ def test_complex(self, auth_client, test_api_uri):
         FlawAcknowledgmentFactory(flaw=flaw, affiliation="Corp2")
         FlawCommentFactory(flaw=flaw)
         FlawCommentFactory(flaw=flaw)
-        FlawCVSSFactory(flaw=flaw)
+        FlawCVSSFactory(flaw=flaw, version=FlawCVSS.CVSSVersion.VERSION4)
         FlawReferenceFactory(flaw=flaw)
         PackageFactory(flaw=flaw)
         ps_module = PsModuleFactory()

osidb/tests/factories.py

@@ -1,8 +1,10 @@
 import uuid
+from decimal import Decimal
 from random import choice
 
 import factory
 import factory.fuzzy
+from cvss import CVSS2, CVSS3, CVSS4
 from cvss.constants2 import METRICS_VALUE_NAMES as CVSS2_METRICS_VALUE_NAMES
 from cvss.constants3 import METRICS_VALUE_NAMES as CVSS3_METRICS_VALUE_NAMES
 from cvss.constants4 import METRICS_VALUE_NAMES as CVSS4_METRICS_VALUE_NAMES
@@ -784,7 +786,35 @@ class Meta:
         model = FlawCVSS
         django_get_or_create = ("flaw", "issuer", "version")
 
-    flaw = factory.SubFactory(FlawFactory)
+    class Params:
+        CVSS_TO_CVSSLIB = {
+            FlawCVSS.CVSSVersion.VERSION2: CVSS2,
+            FlawCVSS.CVSSVersion.VERSION3: CVSS3,
+            FlawCVSS.CVSSVersion.VERSION4: CVSS4,
+        }
+
+        # if RH CVSSv3 score is not zero, flaw impact must not be NOVALUE
+        is_rh_cvss3_non_zero = factory.LazyAttribute(
+            lambda f: f.issuer == FlawCVSS.CVSSIssuer.REDHAT
+            and f.version == FlawCVSS.CVSSVersion.VERSION3
+            and f.CVSS_TO_CVSSLIB[f.version](f.vector).base_score != Decimal("0.0")
+        )
+
+        # if RH CVSSv3 score is zero, flaw impact must be NOVALUE
+        is_rh_cvss3_zero = factory.LazyAttribute(
+            lambda f: f.issuer == FlawCVSS.CVSSIssuer.REDHAT
+            and f.version == FlawCVSS.CVSSVersion.VERSION3
+            and f.CVSS_TO_CVSSLIB[f.version](f.vector).base_score == Decimal("0.0")
+        )
+
+    flaw = factory.SubFactory(
+        FlawFactory,
+        impact=factory.LazyAttribute(
+            lambda f: Impact.MODERATE
+            if f.factory_parent.is_rh_cvss3_non_zero
+            else (Impact.NOVALUE if f.factory_parent.is_rh_cvss3_zero else Impact.LOW)
+        ),
+    )
 
     # let us inherit the parent flaw ACLs if not specified
     acl_read = factory.LazyAttribute(lambda o: o.flaw.acl_read)

osidb/tests/test_audit.py

@@ -6,7 +6,7 @@
 from django.db import transaction
 
 from osidb.core import set_user_acls
-from osidb.models import Affect, Flaw, FlawSource, Impact, Tracker
+from osidb.models import Affect, Flaw, FlawCVSS, FlawSource, Impact, Tracker
 from osidb.tests.factories import (
     AffectCVSSFactory,
     AffectFactory,
@@ -182,7 +182,7 @@ def test_access_flawevent(
         assert flaw_com.acl_read == embargoed_read_groups
         assert flaw_com.acl_write == embargoed_write_groups
 
-        flaw_cvss = FlawCVSSFactory(flaw=flaw)
+        flaw_cvss = FlawCVSSFactory(flaw=flaw, version=FlawCVSS.CVSSVersion.VERSION4)
         assert flaw_cvss.acl_read == embargoed_read_groups
         assert flaw_cvss.acl_write == embargoed_write_groups