Skip to content

Commit 30e6864

Browse files
tdrueztdruez
authored
[cravex2-reachability] Consume extended reachability from symbols analysis (#568)
Signed-off-by: tdruez <tdruez@aboutcode.org>
1 parent d913770 commit 30e6864

9 files changed

Lines changed: 204 additions & 46 deletions

File tree

‎product_portfolio/importers.py‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -713,8 +713,11 @@ def update_scancode_project(self):
713713

714714
class ImportPackageFromScanCodeIO:
715715
"""
716-
Creates, and assign to a product, packages in Dejacode from a ScanCode.io project
717-
discovered packages.
716+
Import packages discovered by a ScanCode.io project and assign them to a product.
717+
718+
For each package, associated vulnerabilities are imported and linked, including
719+
reachability data when available.
720+
Dependencies can optionally be imported as well.
718721
"""
719722

720723
unique_together_fields = [
@@ -810,21 +813,43 @@ def import_vulnerability(vulnerability_data, product_package):
810813
if not vulnerabilities:
811814
return
812815

816+
vulnerability = vulnerabilities[0]
817+
813818
if cdx_vulnerability := vulnerability_data.get("cdx_vulnerability_data"):
814819
if analysis_data := cdx_vulnerability.get("analysis"):
815-
# CycloneDX model uses "response" while the local model uses "response"
820+
# CycloneDX model uses "response" while the local model uses "responses"
816821
if response_value := analysis_data.pop("response", None):
817822
analysis_data["responses"] = response_value
818823

819824
VulnerabilityAnalysis.create_from_data(
820825
user=product_package.dataspace,
821826
data={
822827
"product_package": product_package,
823-
"vulnerability": vulnerabilities[0],
828+
"vulnerability": vulnerability,
824829
**analysis_data,
825830
},
826831
)
827832

833+
# Import reachability from the "symbol reachability analysis" scan when available.
834+
is_reachable_raw = vulnerability_data.get("is_reachable")
835+
is_reachable = None
836+
if is_reachable_raw == "yes":
837+
is_reachable = True
838+
elif is_reachable_raw == "no":
839+
is_reachable = False
840+
841+
if is_reachable is not None:
842+
analysis, created = VulnerabilityAnalysis.objects.get_or_create(
843+
product_package=product_package,
844+
vulnerability=vulnerability,
845+
dataspace=product_package.dataspace,
846+
defaults={"is_reachable": is_reachable},
847+
)
848+
if not created and analysis.is_reachable is None:
849+
VulnerabilityAnalysis.objects.filter(pk=analysis.pk).update(
850+
is_reachable=is_reachable
851+
)
852+
828853
def import_package(self, package_data):
829854
# Vulnerabilities are assigned after the package creation.
830855
affected_by_vulnerabilities = package_data.pop("affected_by_vulnerabilities", [])

‎product_portfolio/tests/test_importers.py‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from product_portfolio.models import ProductPackage
4141
from product_portfolio.models import ProductRelationStatus
4242
from product_portfolio.models import ScanCodeProject
43+
from vulnerabilities.models import VulnerabilityAnalysis
4344

4445

4546
class ProductRelationImporterTestCase(TestCase):
@@ -1414,3 +1415,159 @@ def test_product_portfolio_import_packages_from_scio_importer_vex(
14141415
self.assertEqual("code_not_present", analysis.justification)
14151416
self.assertEqual("AAAA", analysis.detail)
14161417
self.assertEqual(["can_not_fix", "update"], analysis.responses)
1418+
1419+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_dependencies")
1420+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_packages")
1421+
def test_product_portfolio_import_packages_from_scio_importer_is_reachable(
1422+
self, mock_fetch_packages, mock_fetch_dependencies
1423+
):
1424+
def make_vulnerability_entry(advisory_id, is_reachable):
1425+
return {
1426+
"advisory_uid": f"github_osv/{advisory_id}",
1427+
"summary": "A vulnerability",
1428+
"is_reachable": is_reachable,
1429+
"cdx_vulnerability_data": {
1430+
"analysis": {"state": "in_triage", "detail": "Under review"},
1431+
},
1432+
}
1433+
1434+
mock_fetch_packages.return_value = [
1435+
{
1436+
"purl": "pkg:maven/abc/abc@1.0",
1437+
"type": "maven",
1438+
"namespace": "abc",
1439+
"name": "abc",
1440+
"version": "1.0",
1441+
"affected_by_vulnerabilities": [
1442+
make_vulnerability_entry("GHSA-yes", "yes"),
1443+
make_vulnerability_entry("GHSA-no", "no"),
1444+
make_vulnerability_entry("GHSA-unknown", "unknown"),
1445+
],
1446+
}
1447+
]
1448+
mock_fetch_dependencies.return_value = []
1449+
1450+
importer = ImportPackageFromScanCodeIO(
1451+
user=self.super_user,
1452+
project_uuid=uuid.uuid4(),
1453+
product=self.product1,
1454+
)
1455+
importer.save()
1456+
1457+
yes_analysis = VulnerabilityAnalysis.objects.get(
1458+
vulnerability__advisory_uid="github_osv/GHSA-yes"
1459+
)
1460+
no_analysis = VulnerabilityAnalysis.objects.get(
1461+
vulnerability__advisory_uid="github_osv/GHSA-no"
1462+
)
1463+
unknown_analysis = VulnerabilityAnalysis.objects.get(
1464+
vulnerability__advisory_uid="github_osv/GHSA-unknown"
1465+
)
1466+
1467+
self.assertTrue(yes_analysis.is_reachable)
1468+
self.assertFalse(no_analysis.is_reachable)
1469+
self.assertIsNone(unknown_analysis.is_reachable)
1470+
1471+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_dependencies")
1472+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_packages")
1473+
def test_product_portfolio_import_packages_from_scio_importer_is_reachable_not_overwritten(
1474+
self, mock_fetch_packages, mock_fetch_dependencies
1475+
):
1476+
mock_fetch_packages.return_value = [
1477+
{
1478+
"purl": "pkg:maven/abc/abc@1.0",
1479+
"type": "maven",
1480+
"namespace": "abc",
1481+
"name": "abc",
1482+
"version": "1.0",
1483+
"affected_by_vulnerabilities": [
1484+
{
1485+
"advisory_uid": "github_osv/GHSA-existing",
1486+
"summary": "A vulnerability",
1487+
"is_reachable": "no",
1488+
"cdx_vulnerability_data": {
1489+
"analysis": {"state": "in_triage", "detail": "Under review"},
1490+
},
1491+
}
1492+
],
1493+
}
1494+
]
1495+
mock_fetch_dependencies.return_value = []
1496+
1497+
importer = ImportPackageFromScanCodeIO(
1498+
user=self.super_user,
1499+
project_uuid=uuid.uuid4(),
1500+
product=self.product1,
1501+
)
1502+
importer.save()
1503+
1504+
analysis = VulnerabilityAnalysis.objects.get(
1505+
vulnerability__advisory_uid="github_osv/GHSA-existing"
1506+
)
1507+
self.assertFalse(analysis.is_reachable)
1508+
1509+
# A second import with a conflicting value must not overwrite the existing one.
1510+
# Reassign return_value because import_package pops "affected_by_vulnerabilities".
1511+
mock_fetch_packages.return_value = [
1512+
{
1513+
"purl": "pkg:maven/abc/abc@1.0",
1514+
"type": "maven",
1515+
"namespace": "abc",
1516+
"name": "abc",
1517+
"version": "1.0",
1518+
"affected_by_vulnerabilities": [
1519+
{
1520+
"advisory_uid": "github_osv/GHSA-existing",
1521+
"summary": "A vulnerability",
1522+
"is_reachable": "yes",
1523+
}
1524+
],
1525+
}
1526+
]
1527+
importer2 = ImportPackageFromScanCodeIO(
1528+
user=self.super_user,
1529+
project_uuid=uuid.uuid4(),
1530+
product=self.product1,
1531+
)
1532+
importer2.save()
1533+
1534+
analysis.refresh_from_db()
1535+
self.assertFalse(analysis.is_reachable)
1536+
1537+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_dependencies")
1538+
@mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.fetch_project_packages")
1539+
def test_product_portfolio_import_packages_from_scio_importer_is_reachable_without_cdx(
1540+
self, mock_fetch_packages, mock_fetch_dependencies
1541+
):
1542+
# When cdx_vulnerability_data is absent, a minimal VulnerabilityAnalysis is still
1543+
# created to record the is_reachable value from the scan.
1544+
mock_fetch_packages.return_value = [
1545+
{
1546+
"purl": "pkg:maven/abc/abc@1.0",
1547+
"type": "maven",
1548+
"namespace": "abc",
1549+
"name": "abc",
1550+
"version": "1.0",
1551+
"affected_by_vulnerabilities": [
1552+
{
1553+
"advisory_uid": "github_osv/GHSA-no-cdx",
1554+
"summary": "A vulnerability",
1555+
"is_reachable": "yes",
1556+
}
1557+
],
1558+
}
1559+
]
1560+
mock_fetch_dependencies.return_value = []
1561+
1562+
importer = ImportPackageFromScanCodeIO(
1563+
user=self.super_user,
1564+
project_uuid=uuid.uuid4(),
1565+
product=self.product1,
1566+
)
1567+
importer.save()
1568+
1569+
analysis = VulnerabilityAnalysis.objects.get(
1570+
vulnerability__advisory_uid="github_osv/GHSA-no-cdx"
1571+
)
1572+
self.assertTrue(analysis.is_reachable)
1573+
self.assertFalse(analysis.state)

‎product_portfolio/views.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2989,10 +2989,6 @@ def apply_analysis_preset_view(request, productpackage_uuid, advisory_uid, prese
29892989
dataspace=dataspace,
29902990
)
29912991
preset.apply_to_analysis(analysis)
2992-
2993-
if not analysis.has_content_fields():
2994-
return JsonResponse({"error": "This preset has no content fields to apply."}, status=400)
2995-
29962992
analysis.applied_by_preset = preset
29972993
analysis.save()
29982994

‎vulnerabilities/models.py‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -353,18 +353,6 @@ class Response(models.TextChoices):
353353
),
354354
)
355355

356-
def has_content_fields(self):
357-
return any([self.state, self.justification, self.responses, self.detail])
358-
359-
def save(self, *args, **kwargs):
360-
# At least one of those fields must be provided.
361-
if not self.has_content_fields():
362-
raise ValueError(
363-
"At least one of state, justification, responses or detail must be provided."
364-
)
365-
366-
super().save(*args, **kwargs)
367-
368356
class Meta:
369357
abstract = True
370358

@@ -385,13 +373,15 @@ class Meta:
385373
abstract = True
386374

387375
def as_cyclonedx(self):
388-
state = None
389-
if self.state:
390-
state = cdx_vulnerability.ImpactAnalysisState(self.state)
391-
392-
justification = None
393-
if self.justification:
394-
justification = cdx_vulnerability.ImpactAnalysisJustification(self.justification)
376+
if not any([self.state, self.justification, self.responses, self.detail]):
377+
return None
378+
379+
state = cdx_vulnerability.ImpactAnalysisState(self.state) if self.state else None
380+
justification = (
381+
cdx_vulnerability.ImpactAnalysisJustification(self.justification)
382+
if self.justification
383+
else None
384+
)
395385

396386
return cdx_vulnerability.VulnerabilityAnalysis(
397387
state=state,

‎vulnerabilities/tests/test_models.py‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,13 +327,8 @@ def test_vulnerability_model_vulnerability_analysis_save(self):
327327
product_package=product_package1,
328328
vulnerability=vulnerability1,
329329
dataspace=self.dataspace,
330+
state=VulnerabilityAnalysis.State.RESOLVED,
330331
)
331-
332-
msg = "At least one of state, justification, responses or detail must be provided."
333-
with self.assertRaisesMessage(ValueError, msg):
334-
analysis.save()
335-
336-
analysis.state = VulnerabilityAnalysis.State.RESOLVED
337332
analysis.save()
338333

339334
# Refresh from db

‎vulnerabilities/triage/engine.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,6 @@ def apply_preset_for_vulnerabilities(preset, product, vulnerability_ids):
106106
dataspace_id=product.dataspace_id,
107107
)
108108
preset.apply_to_analysis(analysis)
109-
if not analysis.has_content_fields():
110-
continue # Preset has no content fields - cannot save a new analysis
111109
else:
112110
analysis = existing
113111
preset.apply_to_analysis(analysis)

‎vulnerabilities/triage/models.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ class Meta:
5454
def __str__(self):
5555
return self.name
5656

57+
def save(self, *args, **kwargs):
58+
if not any([self.state, self.justification, self.responses, self.detail]):
59+
raise ValueError(
60+
"At least one of state, justification, responses or detail must be provided."
61+
)
62+
super().save(*args, **kwargs)
63+
5764
def apply_to_analysis(self, analysis):
5865
"""Copy non-blank preset fields onto the analysis instance (does not save)."""
5966
for field_name in ("state", "justification", "responses", "detail"):

‎vulnerabilities/triage/tests/test_engine.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,6 @@ def test_updates_an_existing_preset_owned_analysis(self):
145145
self.assertEqual(second_preset, analysis.applied_by_preset)
146146
self.assertEqual(1, VulnerabilityAnalysis.objects.count())
147147

148-
def test_skips_creation_when_the_preset_has_no_content_field_set(self):
149-
# An AnalysisPreset always requires at least one content field to be saved (see
150-
# VulnerabilityAnalysisContentMixin.save), so this can only happen with an in-memory
151-
# preset. This exercises the defensive guard against saving a content-less analysis.
152-
content_less_preset = AnalysisPreset(dataspace=self.dataspace, is_reachable=True)
153-
apply_preset_for_vulnerabilities(content_less_preset, self.product, [self.vulnerability.pk])
154-
self.assertFalse(VulnerabilityAnalysis.objects.exists())
155-
156148
def test_does_nothing_when_no_product_package_carries_the_vulnerability(self):
157149
other_package = make_package(self.dataspace)
158150
other_vulnerability = make_vulnerability(self.dataspace, affecting=other_package)

‎vulnerabilities/triage/tests/test_models.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ def setUp(self):
2929
self.dataspace = Dataspace.objects.create(name="nexB")
3030

3131
def test_save_requires_at_least_one_content_field(self):
32-
# AnalysisPreset shares its `save` validation with VulnerabilityAnalysis through
33-
# VulnerabilityAnalysisContentMixin: a preset that only sets `is_reachable` has no
34-
# content to apply and must be rejected the same way a bare analysis would be.
32+
# A preset that carries no content fields is useless: it has nothing to apply.
3533
preset = AnalysisPreset(dataspace=self.dataspace, name="No content", is_reachable=True)
3634
with self.assertRaises(ValueError):
3735
preset.save()

0 commit comments

Comments
 (0)