-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
expose severities inside the affected by packages #1799
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
from vulnerabilities.models import ApiUser | ||
from vulnerabilities.models import Package | ||
from vulnerabilities.models import Vulnerability | ||
from vulnerabilities.models import VulnerabilitySeverity | ||
from vulnerabilities.models import VulnerabilityReference | ||
from vulnerabilities.models import Weakness | ||
|
||
|
@@ -210,13 +211,23 @@ def setUp(self): | |
self.client = APIClient(enforce_csrf_checks=True) | ||
self.client.credentials(HTTP_AUTHORIZATION=self.auth) | ||
|
||
#create vulnerability severities | ||
self.severity = VulnerabilitySeverity.objects.create( | ||
scoring_system="CVSSv3", | ||
scoring_elements="", | ||
url="https://example.com", | ||
value="7.5" | ||
) | ||
|
||
self.vuln1.severities.add(self.severity) | ||
|
||
def test_list_packages(self): | ||
""" | ||
Test listing packages without filters. | ||
Should return a list of packages with their details and associated vulnerabilities. | ||
""" | ||
url = reverse("package-v2-list") | ||
with self.assertNumQueries(32): | ||
with self.assertNumQueries(33): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way to avoid the extra query? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes , i had tested select_related(), but it didn't worked . |
||
response = self.client.get(url, format="json") | ||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertIn("results", response.data) | ||
|
@@ -238,7 +249,7 @@ def test_filter_packages_by_purl(self): | |
Test filtering packages by one or more PURLs. | ||
""" | ||
url = reverse("package-v2-list") | ||
with self.assertNumQueries(20): | ||
with self.assertNumQueries(21): | ||
response = self.client.get(url, {"purl": "pkg:pypi/[email protected]"}, format="json") | ||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertEqual(len(response.data["results"]["packages"]), 1) | ||
|
@@ -249,7 +260,7 @@ def test_filter_packages_by_affected_vulnerability(self): | |
Test filtering packages by affected_by_vulnerability. | ||
""" | ||
url = reverse("package-v2-list") | ||
with self.assertNumQueries(20): | ||
with self.assertNumQueries(21): | ||
response = self.client.get( | ||
url, {"affected_by_vulnerability": "VCID-1234"}, format="json" | ||
) | ||
|
@@ -275,13 +286,22 @@ def test_package_serializer_fields(self): | |
# Fetch the package | ||
package = Package.objects.get(package_url="pkg:pypi/[email protected]") | ||
|
||
#retrieving the vulnerability | ||
Vulnerability_with_severities = Vulnerability.objects.get(vulnerability_id="VCID-1234") | ||
Vulnerability_without_severities = Vulnerability.objects.get(vulnerability_id="VCID-5678") | ||
|
||
severity_created = self.severity | ||
Vulnerability_with_severities.severities.add(severity_created) | ||
|
||
package.affected_by_vulnerabilities.add(Vulnerability_with_severities, Vulnerability_without_severities) | ||
|
||
# Ensure prefetched data is available for the serializer | ||
package = ( | ||
Package.objects.filter(package_url="pkg:pypi/[email protected]") | ||
.prefetch_related( | ||
Prefetch( | ||
"affected_by_vulnerabilities", | ||
queryset=Vulnerability.objects.prefetch_related("fixed_by_packages"), | ||
queryset=Vulnerability.objects.prefetch_related("fixed_by_packages","severities"), | ||
to_attr="prefetched_affected_vulnerabilities", | ||
) | ||
) | ||
|
@@ -311,8 +331,22 @@ def test_package_serializer_fields(self): | |
"VCID-1234": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-1234", | ||
"severities": [ | ||
{ | ||
"url":"https://example.com", | ||
"value": "7.5", | ||
"scoring_system": "CVSSv3", | ||
"scoring_elements": "", | ||
} | ||
], | ||
"fixed_by_packages": None, | ||
} | ||
}, | ||
"VCID-5678": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-5678", | ||
"severities": [], | ||
"fixed_by_packages": "pkg:npm/[email protected]", | ||
}, | ||
} | ||
self.assertEqual(data["affected_by_vulnerabilities"], expected_affected_by_vulnerabilities) | ||
|
||
|
@@ -375,30 +409,61 @@ def test_get_affected_by_vulnerabilities(self): | |
""" | ||
Test the get_affected_by_vulnerabilities method in the serializer. | ||
""" | ||
serializer = PackageV2Serializer() | ||
package = Package.objects.get(package_url="pkg:pypi/[email protected]") | ||
|
||
vulnerability_with_severities, vulnerability_without_severities = list(Vulnerability.objects.filter(vulnerability_id__in=["VCID-1234", "VCID-5678"] | ||
).prefetch_related("fixed_by_packages", "severities")) | ||
package.affected_by_vulnerabilities.add(vulnerability_with_severities, vulnerability_without_severities) | ||
|
||
package = ( | ||
Package.objects.filter(package_url="pkg:pypi/[email protected]") | ||
.prefetch_related( | ||
Prefetch( | ||
"affected_by_vulnerabilities", | ||
queryset=Vulnerability.objects.prefetch_related("fixed_by_packages"), | ||
queryset=Vulnerability.objects.prefetch_related("fixed_by_packages","severities"), | ||
to_attr="prefetched_affected_vulnerabilities", | ||
) | ||
) | ||
.first() | ||
) | ||
|
||
serializer = PackageV2Serializer() | ||
severity_created = self.severity | ||
vulnerability_with_severities.severities.add(severity_created) | ||
|
||
vulnerabilities = serializer.get_affected_by_vulnerabilities(package) | ||
|
||
|
||
for vuln_data in vulnerabilities.values(): | ||
for severity in vuln_data["severities"]: | ||
self.assertIn("url", severity) | ||
self.assertIn("value", severity) | ||
self.assertIn("scoring_system", severity) | ||
self.assertIn("scoring_elements", severity) | ||
|
||
self.assertEqual( | ||
vulnerabilities, | ||
{ | ||
"VCID-1234": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-1234", | ||
"severities": [ | ||
{ | ||
"url":"https://example.com", | ||
"value": "7.5", | ||
"scoring_system": "CVSSv3", | ||
"scoring_elements": "", | ||
} | ||
], | ||
"fixed_by_packages": None, | ||
} | ||
}, | ||
"VCID-5678": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-5678", | ||
"severities": [], #No severity for this vulnerability | ||
"fixed_by_packages": "pkg:npm/[email protected]", | ||
}, | ||
) | ||
},) | ||
|
||
def test_get_fixing_vulnerabilities(self): | ||
""" | ||
|
@@ -601,7 +666,25 @@ def test_lookup_with_valid_purl(self): | |
""" | ||
url = reverse("package-v2-lookup") | ||
data = {"purl": "pkg:pypi/[email protected]"} | ||
with self.assertNumQueries(13): | ||
package = Package.objects.filter(package_url="pkg:pypi/[email protected]").prefetch_related( | ||
Prefetch( | ||
"affected_by_vulnerabilities", | ||
queryset=Vulnerability.objects.prefetch_related(Prefetch("fixed_by_packages",queryset=Package.objects.all()), | ||
Prefetch("severities",queryset=VulnerabilitySeverity.objects.all())), | ||
to_attr="prefetched_affected_vulnerabilities", | ||
) | ||
).first() | ||
|
||
vulnerability_with_severities, vulnerability_without_severities = list(Vulnerability.objects.filter(vulnerability_id__in=["VCID-1234", "VCID-5678"] | ||
).prefetch_related("fixed_by_packages", "severities")) | ||
|
||
severity_created = self.severity | ||
vulnerability_with_severities.severities.add(severity_created) | ||
|
||
package.affected_by_vulnerabilities.add(vulnerability_with_severities, vulnerability_without_severities) | ||
|
||
|
||
with self.assertNumQueries(15): | ||
response = self.client.post(url, data, format="json") | ||
self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
self.assertEqual(1, len(response.data)) | ||
|
@@ -617,10 +700,24 @@ def test_lookup_with_valid_purl(self): | |
"VCID-1234": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-1234", | ||
"severities": [ | ||
{ | ||
"url":"https://example.com", | ||
"value": "7.5", | ||
"scoring_system": "CVSSv3", | ||
"scoring_elements": '', | ||
} | ||
], | ||
"fixed_by_packages": None, | ||
} | ||
}, | ||
) | ||
}, | ||
"VCID-5678": { | ||
"code_fixes": [], | ||
"vulnerability_id": "VCID-5678", | ||
"severities": [], | ||
"fixed_by_packages": "pkg:npm/[email protected]", | ||
}, | ||
}) | ||
|
||
self.assertEqual(response.data[0]["fixing_vulnerabilities"], []) | ||
|
||
def test_lookup_with_invalid_purl(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really the way to send both to the same attribute below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified that severities and fixed_by_packages are sent separately in the API response. Also, I tested removing severities from prefetch_related(), and the response remained the same, confirming that explicit prefetching was unnecessary. So in further changes i will remove severities .
Let me know if you have any further suggestions.
Thank you for spending your time on reviewing the changes i made and giving feedback..!!