From 5a4c5908aa4ff206a55899e03678eb5cd054b4a3 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Fri, 5 Mar 2021 19:28:37 +0530 Subject: [PATCH 01/12] Send severity data along with vulnerability Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 69 ++++++++++++++------------------------- vulnerabilities/models.py | 4 +++ 2 files changed, 28 insertions(+), 45 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index aff7965e5..c36cea169 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -38,20 +38,26 @@ from vulnerabilities.models import Package from vulnerabilities.models import Vulnerability from vulnerabilities.models import VulnerabilityReference +from vulnerabilities.models import VulnerabilitySeverity + # This serializer is used for the bulk apis, to prevent wrong auto documentation # TODO: Fix the swagger documentation for bulk apis placeholder_serializer = inline_serializer(name="Placeholder", fields={}) +class VulnerabilitySeveritySerializer(serializers.ModelSerializer): + class Meta: + model = VulnerabilitySeverity + fields = ["value", "scoring_system"] + + class VulnerabilityReferenceSerializer(serializers.ModelSerializer): + scores = VulnerabilitySeveritySerializer(many=True) + class Meta: model = VulnerabilityReference - fields = [ - "source", - "reference_id", - "url", - ] + fields = ["source", "reference_id", "url", "scores"] class HyperLinkedPackageSerializer(serializers.HyperlinkedModelSerializer): @@ -63,9 +69,11 @@ class Meta: class HyperLinkedVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): + references = VulnerabilityReferenceSerializer(many=True, source="vulnerabilityreference_set") + class Meta: model = Vulnerability - fields = ["url", "vulnerability_id"] + fields = ["url", "vulnerability_id", "references"] class MinimalVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): @@ -149,7 +157,7 @@ def bulk_search(self, request): See https://github.com/nexB/vulnerablecode/pull/303#issuecomment-761801639 for docs """ filter_list = Q() - response = {} + response = [] if not isinstance(request.data.get("packages"), list): return Response( status=400, @@ -157,19 +165,18 @@ def bulk_search(self, request): "Error": "Request needs to contain a key 'packages' which has the value of a list of package urls" # nopep8 }, ) - for purl in request.data["packages"]: + for index, purl in enumerate(request.data["packages"]): try: - filter_list |= Q( - **{k: v for k, v in PackageURL.from_string(purl).to_dict().items() if v} - ) + purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: return Response(status=400, data={"Error": str(ve)}) - - # This handles the case when the said purl doesnt exist in db - response[purl] = {} - res = Package.objects.filter(filter_list) - for p in res: - response[p.package_url] = MinimalPackageSerializer(p, context={"request": request}).data + purl_data = Package.objects.filter( + **{key: value for key, value in purl.items() if value} + ) + purl_response = {} + if purl_data: + purl_response = PackageSerializer(purl_data[0], context={"request": request}).data + response.append(purl_response) return Response(response) @@ -186,31 +193,3 @@ class VulnerabilityViewSet(viewsets.ReadOnlyModelViewSet): paginate_by = 50 filter_backends = (filters.DjangoFilterBackend,) filterset_class = VulnerabilityFilterSet - - # TODO: Fix the swagger documentation for this endpoint - @extend_schema(request=placeholder_serializer, responses=placeholder_serializer) - @action(detail=False, methods=["post"]) - def bulk_search(self, request): - """ - See https://github.com/nexB/vulnerablecode/pull/303#issuecomment-761801619 for docs - """ - filter_list = [] - response = {} - if not isinstance(request.data.get("vulnerabilities"), list): - return Response( - status=400, - data={ - "Error": "Request needs to contain a key 'vulnerabilities' which has the value of a list of vulnerability ids" # nopep8 - }, - ) - - for vulnerability_id in request.data["vulnerabilities"]: - filter_list.append(vulnerability_id) - # This handles the case when the said cve doesnt exist in db - response[vulnerability_id] = {} - res = Vulnerability.objects.filter(vulnerability_id__in=filter_list) - for vuln in res: - response[vuln.vulnerability_id] = MinimalVulnerabilitySerializer( - vuln, context={"request": request} - ).data - return Response(response) diff --git a/vulnerabilities/models.py b/vulnerabilities/models.py index fcd09a831..f101c98d4 100644 --- a/vulnerabilities/models.py +++ b/vulnerabilities/models.py @@ -106,6 +106,10 @@ class VulnerabilityReference(models.Model): ) url = models.URLField(max_length=1024, help_text="URL of Vulnerability data", blank=True) + @property + def scores(self): + return VulnerabilitySeverity.objects.filter(reference=self.id) + class Meta: unique_together = ("vulnerability", "source", "reference_id", "url") From 1f3f7371ebc18e84f8f95db5dfa83fddd1527374 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Sat, 6 Mar 2021 14:58:49 +0530 Subject: [PATCH 02/12] Update tests for api Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 2 +- vulnerabilities/fixtures/github.json | 130 ++++++++++++++++++++++ vulnerabilities/tests/test_api.py | 155 ++++++++++++--------------- 3 files changed, 198 insertions(+), 89 deletions(-) create mode 100644 vulnerabilities/fixtures/github.json diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index c36cea169..6fb9c15ce 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -165,7 +165,7 @@ def bulk_search(self, request): "Error": "Request needs to contain a key 'packages' which has the value of a list of package urls" # nopep8 }, ) - for index, purl in enumerate(request.data["packages"]): + for purl in request.data.get("packages"): try: purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: diff --git a/vulnerabilities/fixtures/github.json b/vulnerabilities/fixtures/github.json new file mode 100644 index 000000000..11c9423c2 --- /dev/null +++ b/vulnerabilities/fixtures/github.json @@ -0,0 +1,130 @@ +[ + { + "model": "vulnerabilities.vulnerability", + "pk": 60, + "fields": { + "vulnerability_id": "CVE-2021-21331", + "old_vulnerability_id": null, + "summary": "Local Information Disclosure Vulnerability" + } + }, + { + "model": "vulnerabilities.vulnerabilityreference", + "pk": 136, + "fields": { + "vulnerability": 60, + "source": "", + "reference_id": "GHSA-2cxf-6567-7pp6", + "url": "https://github.com/DataDog/datadog-api-client-java/security/advisories/GHSA-2cxf-6567-7pp6" + } + }, + { + "model": "vulnerabilities.vulnerabilityreference", + "pk": 137, + "fields": { + "vulnerability": 60, + "source": "", + "reference_id": "", + "url": "https://nvd.nist.gov/vuln/detail/CVE-2021-21331" + } + }, + { + "model": "vulnerabilities.vulnerabilityreference", + "pk": 138, + "fields": { + "vulnerability": 60, + "source": "", + "reference_id": "GHSA-2cxf-6567-7pp6", + "url": "https://github.com/advisories/GHSA-2cxf-6567-7pp6" + } + }, + { + "model": "vulnerabilities.package", + "pk": 3467, + "fields": { + "type": "maven", + "namespace": "com.datadoghq", + "name": "datadog-api-client", + "version": "1.0.0-beta.7", + "subpath": "", + "qualifiers": {} + } + }, + { + "model": "vulnerabilities.package", + "pk": 3468, + "fields": { + "type": "maven", + "namespace": "com.datadoghq", + "name": "datadog-api-client", + "version": "1.0.0-beta.6", + "subpath": "", + "qualifiers": {} + } + }, + { + "model": "vulnerabilities.package", + "pk": 3469, + "fields": { + "type": "maven", + "namespace": "com.datadoghq", + "name": "datadog-api-client", + "version": "1.0.0-beta.9", + "subpath": "", + "qualifiers": {} + } + }, + { + "model": "vulnerabilities.packagerelatedvulnerability", + "pk": 3844, + "fields": { + "package": 3469, + "vulnerability": 60, + "is_vulnerable": false + } + }, + { + "model": "vulnerabilities.packagerelatedvulnerability", + "pk": 3845, + "fields": { + "package": 3467, + "vulnerability": 60, + "is_vulnerable": true + } + }, + { + "model": "vulnerabilities.packagerelatedvulnerability", + "pk": 3846, + "fields": { + "package": 3468, + "vulnerability": 60, + "is_vulnerable": true + } + }, + { + "model": "vulnerabilities.importer", + "pk": 18, + "fields": { + "name": "github", + "license": "", + "last_run": "2021-03-06T09:09:01.523Z", + "data_source": "GitHubAPIDataSource", + "data_source_cfg": { + "endpoint": "https://api.github.com/graphql", + "ecosystems": [ + "MAVEN" + ] + } + } + }, + { + "model": "vulnerabilities.vulnerabilityseverity", + "pk": 57, + "fields": { + "vulnerability": 60, + "value": "LOW", + "scoring_system": "cvssv3.1_qr", + "reference": 136 + } + } +] \ No newline at end of file diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index e3904423e..1c1c9ceae 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -194,90 +194,89 @@ def test_package_serializer(self): class TestBulkAPIResponse(TestCase): - fixtures = ["debian.json"] - - def test_bulk_vulnerabilities_api(self): - request_body = {"vulnerabilities": ["CVE-2009-1382", "CVE-2014-8242", "RANDOM-CVE"]} - expected_response = { - "CVE-2009-1382": { - "resolved_packages": [ - OrderedDict( - [ - ("url", "http://testserver/api/packages/2"), - ("purl", "pkg:deb/debian/mimetex@1.74-1?distro=jessie"), - ] - ), - OrderedDict( - [ - ("url", "http://testserver/api/packages/3"), - ("purl", "pkg:deb/debian/mimetex@1.50-1.1?distro=jessie"), - ] - ), - ], - "unresolved_packages": [], - "url": "http://testserver/api/vulnerabilities/2", - }, - "CVE-2014-8242": { - "resolved_packages": [], - "unresolved_packages": [ - OrderedDict( - [ - ("url", "http://testserver/api/packages/1"), - ("purl", "pkg:deb/debian/librsync@0.9.7-10?distro=jessie"), - ] - ) - ], - "url": "http://testserver/api/vulnerabilities/1", - }, - "RANDOM-CVE": {}, - } - - response = self.client.post( - "/api/vulnerabilities/bulk_search/", data=request_body, content_type="application/json" - ).data - assert response == expected_response + fixtures = ["github.json"] def test_bulk_packages_api(self): request_body = { "packages": [ - "pkg:deb/debian/librsync@0.9.7-10?distro=jessie", - "pkg:deb/debian/mimetex@1.50-1.1?distro=jessie", + "pkg:deb/debian/doesnotexist@0.9.7-10?distro=jessie", + "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", ] } response = self.client.post( "/api/packages/bulk_search/", data=request_body, content_type="application/json" ).data - expected_response = { - "pkg:deb/debian/librsync@0.9.7-10?distro=jessie": { + + expected_response = [ + {}, + { + "name": "datadog-api-client", + "namespace": "com.datadoghq", + "purl": "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", + "qualifiers": {}, "resolved_vulnerabilities": [], + "subpath": "", + "type": "maven", "unresolved_vulnerabilities": [ OrderedDict( [ - ("url", "http://testserver/api/vulnerabilities/1"), - ("vulnerability_id", "CVE-2014-8242"), + ("url", "http://testserver/api/vulnerabilities/60"), + ("vulnerability_id", "CVE-2021-21331"), + ( + "references", + [ + OrderedDict( + [ + ("source", ""), + ("reference_id", "GHSA-2cxf-6567-7pp6"), + ( + "url", + "https://github.com/advisories/GHSA-2cxf-6567-7pp6", + ), + ("scores", []), + ] + ), + OrderedDict( + [ + ("source", ""), + ("reference_id", ""), + ( + "url", + "https://nvd.nist.gov/vuln/detail/CVE-2021-21331", + ), + ("scores", []), + ] + ), + OrderedDict( + [ + ("source", ""), + ("reference_id", "GHSA-2cxf-6567-7pp6"), + ( + "url", + "https://github.com/DataDog/datadog-api-client-java/security/advisories/GHSA-2cxf-6567-7pp6", + ), + ( + "scores", + [ + OrderedDict( + [ + ("value", "LOW"), + ("scoring_system", "cvssv3.1_qr"), + ] + ) + ], + ), + ] + ), + ], + ), ] ) ], + "url": "http://testserver/api/packages/3467", + "version": "1.0.0-beta.7", }, - "pkg:deb/debian/mimetex@1.50-1.1?distro=jessie": { - "resolved_vulnerabilities": [ - OrderedDict( - [ - ("url", "http://testserver/api/vulnerabilities/2"), - ("vulnerability_id", "CVE-2009-1382"), - ] - ), - OrderedDict( - [ - ("url", "http://testserver/api/vulnerabilities/3"), - ("vulnerability_id", "CVE-2009-2459"), - ] - ), - ], - "unresolved_vulnerabilities": [], - }, - } - + ] assert response == expected_response def test_invalid_request_bulk_packages(self): @@ -298,6 +297,7 @@ def test_invalid_request_bulk_packages(self): data=valid_key_invalid_datatype_request_data, content_type="application/json", ).data + assert response == error_response invalid_purl_request_data = { @@ -315,24 +315,3 @@ def test_invalid_request_bulk_packages(self): "Error": "purl is missing the required \"pkg\" scheme component: 'pg:deb/debian/mimetex@1.50-1.1?distro=jessie'." # nopep8 } assert response == purl_error_respones - - def test_invalid_request_bulk_vulnerabilities(self): - error_response = { - "Error": "Request needs to contain a key 'vulnerabilities' which has the value of a list of vulnerability ids" # nopep8 - } - - wrong_key_data = {"xyz": []} - response = self.client.post( - "/api/vulnerabilities/bulk_search/", - data=wrong_key_data, - content_type="application/json", - ).data - assert response == error_response - - wrong_type_data = {"vulnerabilities": {}} - response = self.client.post( - "/api/vulnerabilities/bulk_search/", - data=wrong_key_data, - content_type="application/json", - ).data - assert response == error_response From 2b6546e789424b8ece0533e42d2ecb0d9dba2d92 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Sat, 6 Mar 2021 15:16:51 +0530 Subject: [PATCH 03/12] Remove redundant serializers Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 6fb9c15ce..4b3bb1e6b 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -60,7 +60,8 @@ class Meta: fields = ["source", "reference_id", "url", "scores"] -class HyperLinkedPackageSerializer(serializers.HyperlinkedModelSerializer): +# Used for nesting inside vulnerability focused APIs. +class MinimalPackageSerializer(serializers.HyperlinkedModelSerializer): purl = serializers.CharField(source="package_url") class Meta: @@ -68,7 +69,8 @@ class Meta: fields = ["url", "purl"] -class HyperLinkedVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): +# Used for nesting inside package focused APIs. +class MinimalVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): references = VulnerabilityReferenceSerializer(many=True, source="vulnerabilityreference_set") class Meta: @@ -76,21 +78,13 @@ class Meta: fields = ["url", "vulnerability_id", "references"] -class MinimalVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): +class VulnerabilitySerializer(serializers.HyperlinkedModelSerializer): - resolved_packages = HyperLinkedPackageSerializer( - many=True, source="resolved_to", read_only=True - ) - unresolved_packages = HyperLinkedPackageSerializer( + resolved_packages = MinimalPackageSerializer(many=True, source="resolved_to", read_only=True) + unresolved_packages = MinimalPackageSerializer( many=True, source="vulnerable_to", read_only=True ) - class Meta: - model = Vulnerability - fields = ["url", "unresolved_packages", "resolved_packages"] - - -class VulnerabilitySerializer(MinimalVulnerabilitySerializer): references = VulnerabilityReferenceSerializer(many=True, source="vulnerabilityreference_set") class Meta: @@ -98,23 +92,14 @@ class Meta: fields = "__all__" -class MinimalPackageSerializer(serializers.HyperlinkedModelSerializer): - unresolved_vulnerabilities = HyperLinkedVulnerabilitySerializer( +class PackageSerializer(serializers.HyperlinkedModelSerializer): + + unresolved_vulnerabilities = MinimalVulnerabilitySerializer( many=True, source="vulnerable_to", read_only=True ) - resolved_vulnerabilities = HyperLinkedVulnerabilitySerializer( + resolved_vulnerabilities = MinimalVulnerabilitySerializer( many=True, source="resolved_to", read_only=True ) - - class Meta: - model = Package - fields = [ - "resolved_vulnerabilities", - "unresolved_vulnerabilities", - ] - - -class PackageSerializer(MinimalPackageSerializer): purl = serializers.CharField(source="package_url") class Meta: From 1a665b17f609f0fb8b8564effc1b5818f5207504 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Sat, 6 Mar 2021 15:34:02 +0530 Subject: [PATCH 04/12] Use 'purls' instead of 'packages' in bulk api request body Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 17 ++++++----------- vulnerabilities/tests/test_api.py | 6 +++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 4b3bb1e6b..c11cde447 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -22,25 +22,21 @@ # Visit https://github.com/nexB/vulnerablecode/ for support and download. from urllib.parse import unquote -from typing import List from django.db.models import Q from django.urls import reverse from django_filters import rest_framework as filters +from drf_spectacular.utils import extend_schema, inline_serializer from packageurl import PackageURL -from rest_framework import serializers -from rest_framework import viewsets + +from rest_framework import serializers, viewsets from rest_framework.decorators import action from rest_framework.response import Response -from drf_spectacular.utils import extend_schema, inline_serializer -from drf_spectacular.types import OpenApiTypes - from vulnerabilities.models import Package from vulnerabilities.models import Vulnerability from vulnerabilities.models import VulnerabilityReference from vulnerabilities.models import VulnerabilitySeverity - # This serializer is used for the bulk apis, to prevent wrong auto documentation # TODO: Fix the swagger documentation for bulk apis placeholder_serializer = inline_serializer(name="Placeholder", fields={}) @@ -141,16 +137,15 @@ def bulk_search(self, request): """ See https://github.com/nexB/vulnerablecode/pull/303#issuecomment-761801639 for docs """ - filter_list = Q() response = [] - if not isinstance(request.data.get("packages"), list): + if not isinstance(request.data.get("purls"), list): return Response( status=400, data={ - "Error": "Request needs to contain a key 'packages' which has the value of a list of package urls" # nopep8 + "Error": "Request needs to contain a key 'purls' which has the value of a list of package urls" # nopep8 }, ) - for purl in request.data.get("packages"): + for purl in request.data.get("purls"): try: purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index 1c1c9ceae..2afefdb86 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -198,7 +198,7 @@ class TestBulkAPIResponse(TestCase): def test_bulk_packages_api(self): request_body = { - "packages": [ + "purls": [ "pkg:deb/debian/doesnotexist@0.9.7-10?distro=jessie", "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", ] @@ -281,7 +281,7 @@ def test_bulk_packages_api(self): def test_invalid_request_bulk_packages(self): error_response = { - "Error": "Request needs to contain a key 'packages' which has the value of a list of package urls" # nopep8 + "Error": "Request needs to contain a key 'purls' which has the value of a list of package urls" # nopep8 } invalid_key_request_data = {"pkg": []} response = self.client.post( @@ -301,7 +301,7 @@ def test_invalid_request_bulk_packages(self): assert response == error_response invalid_purl_request_data = { - "packages": [ + "purls": [ "pkg:deb/debian/librsync@0.9.7-10?distro=jessie", "pg:deb/debian/mimetex@1.50-1.1?distro=jessie", ] From 7222ceaf156fe60a395357888aaa33afc1b90b2f Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Sat, 6 Mar 2021 15:49:44 +0530 Subject: [PATCH 05/12] Make api tests deterministic Signed-off-by: Shivam Sandbhor --- vulnerabilities/tests/test_api.py | 109 ++++++++++++------------------ 1 file changed, 44 insertions(+), 65 deletions(-) diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index 2afefdb86..de4991365 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -204,80 +204,59 @@ def test_bulk_packages_api(self): ] } response = self.client.post( - "/api/packages/bulk_search/", data=request_body, content_type="application/json" - ).data + "/api/packages/bulk_search/", + data=request_body, + content_type="application/json", + ).json() expected_response = [ {}, { - "name": "datadog-api-client", - "namespace": "com.datadoghq", - "purl": "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", - "qualifiers": {}, - "resolved_vulnerabilities": [], - "subpath": "", - "type": "maven", + "url": "http://testserver/api/packages/3467", "unresolved_vulnerabilities": [ - OrderedDict( - [ - ("url", "http://testserver/api/vulnerabilities/60"), - ("vulnerability_id", "CVE-2021-21331"), - ( - "references", - [ - OrderedDict( - [ - ("source", ""), - ("reference_id", "GHSA-2cxf-6567-7pp6"), - ( - "url", - "https://github.com/advisories/GHSA-2cxf-6567-7pp6", - ), - ("scores", []), - ] - ), - OrderedDict( - [ - ("source", ""), - ("reference_id", ""), - ( - "url", - "https://nvd.nist.gov/vuln/detail/CVE-2021-21331", - ), - ("scores", []), - ] - ), - OrderedDict( - [ - ("source", ""), - ("reference_id", "GHSA-2cxf-6567-7pp6"), - ( - "url", - "https://github.com/DataDog/datadog-api-client-java/security/advisories/GHSA-2cxf-6567-7pp6", - ), - ( - "scores", - [ - OrderedDict( - [ - ("value", "LOW"), - ("scoring_system", "cvssv3.1_qr"), - ] - ) - ], - ), - ] - ), - ], - ), - ] - ) + { + "url": "http://testserver/api/vulnerabilities/60", + "vulnerability_id": "CVE-2021-21331", + "references": [ + { + "source": "", + "reference_id": "GHSA-2cxf-6567-7pp6", + "url": "https://github.com/advisories/GHSA-2cxf-6567-7pp6", + "scores": [], + }, + { + "source": "", + "reference_id": "", + "url": "https://nvd.nist.gov/vuln/detail/CVE-2021-21331", + "scores": [], + }, + { + "source": "", + "reference_id": "GHSA-2cxf-6567-7pp6", + "url": "https://github.com/DataDog/datadog-api-client-java/security/advisories/GHSA-2cxf-6567-7pp6", + "scores": [{"value": "LOW", "scoring_system": "cvssv3.1_qr"}], + }, + ], + } ], - "url": "http://testserver/api/packages/3467", + "resolved_vulnerabilities": [], + "purl": "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", + "type": "maven", + "namespace": "com.datadoghq", + "name": "datadog-api-client", "version": "1.0.0-beta.7", + "subpath": "", + "qualifiers": {}, }, ] - assert response == expected_response + + # This normalization is brittle + expected_response[1]["unresolved_vulnerabilities"][0]["references"].sort( + key=lambda x: x["url"] + ) + response[1]["unresolved_vulnerabilities"][0]["references"].sort(key=lambda x: x["url"]) + + assert all([purl_response in expected_response for purl_response in response]) def test_invalid_request_bulk_packages(self): error_response = { From fc7352a98e476acf2970af4b3cf74d5bb991f2a9 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 16:27:34 +0530 Subject: [PATCH 06/12] Improve bulk api request validation Co-authored-by: Philippe Ombredanne Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index c11cde447..87f11f3c5 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -138,7 +138,8 @@ def bulk_search(self, request): See https://github.com/nexB/vulnerablecode/pull/303#issuecomment-761801639 for docs """ response = [] - if not isinstance(request.data.get("purls"), list): + purls = request.data.get("purls", []) or [] + if not purls or not isinstance(purls, list): return Response( status=400, data={ From c3ae53af28213753ef0cab2312f2ad81d251f0b4 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 16:28:04 +0530 Subject: [PATCH 07/12] Improve error message in bulk api response Co-authored-by: Philippe Ombredanne Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 87f11f3c5..83e978c44 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -143,7 +143,7 @@ def bulk_search(self, request): return Response( status=400, data={ - "Error": "Request needs to contain a key 'purls' which has the value of a list of package urls" # nopep8 + "Error": "A non-empty 'purls' list of package URLs is required." }, ) for purl in request.data.get("purls"): From 7b46414e775d82a91c9d3cfc6479bf3e616d6f10 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 16:28:40 +0530 Subject: [PATCH 08/12] Remove redundant "get" in bulk api code Co-authored-by: Philippe Ombredanne Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 83e978c44..f11b54109 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -146,7 +146,7 @@ def bulk_search(self, request): "Error": "A non-empty 'purls' list of package URLs is required." }, ) - for purl in request.data.get("purls"): + for purl in request.data["purls"]: try: purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: From c8729c857c81ea65afb304c7a685c00c81952af9 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 16:29:03 +0530 Subject: [PATCH 09/12] Don't leak stack trace on failure in bulk api Co-authored-by: Philippe Ombredanne Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index f11b54109..8d75b3487 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -150,7 +150,7 @@ def bulk_search(self, request): try: purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: - return Response(status=400, data={"Error": str(ve)}) + return Response(status=400, data={"Error": f"Invalid Package URL: {purl}") purl_data = Package.objects.filter( **{key: value for key, value in purl.items() if value} ) From d33f76cb8a4b8c44c7a27e779c68565d469d6a70 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 20:04:54 +0530 Subject: [PATCH 10/12] Normalize responses in api tests. Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 4 ++ vulnerabilities/tests/test_api.py | 105 ++++++++++++++++++++++-------- 2 files changed, 81 insertions(+), 28 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 8d75b3487..d8b078a04 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -157,6 +157,10 @@ def bulk_search(self, request): purl_response = {} if purl_data: purl_response = PackageSerializer(purl_data[0], context={"request": request}).data + else: + purl_response = purl + purl_response["unresolved_vulnerabilities"] = [] + purl_response["resolved_vulnerabilities"] = [] response.append(purl_response) return Response(response) diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index de4991365..fddcfa285 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -40,6 +40,53 @@ TEST_DATA = os.path.join(BASE_DIR, "test_data/") +def cleaned_response(response): + """ + Return a cleaned response suitable for comparison in tests in particular: + - sort lists with a stable order + """ + cleaned_response = [] + response_copy = sorted(response, key=lambda x: x.get("purl", "")) + for package_data in response_copy: + package_data["unresolved_vulnerabilities"] = sorted( + package_data["unresolved_vulnerabilities"], key=lambda x: x["vulnerability_id"] + ) + for index, vulnerability in enumerate(package_data["unresolved_vulnerabilities"]): + package_data["unresolved_vulnerabilities"][index]["references"] = sorted( + vulnerability["references"], key=lambda x: (x["reference_id"], x["url"]) + ) + for index2, reference in enumerate( + package_data["unresolved_vulnerabilities"][index]["references"] + ): + reference["scores"] = sorted( + reference["scores"], key=lambda x: (x["value"], x["scoring_system"]) + ) + package_data["unresolved_vulnerabilities"][index]["references"][index2][ + "scores" + ] = reference["scores"] + + package_data["resolved_vulnerabilities"] = sorted( + package_data["resolved_vulnerabilities"], key=lambda x: x["vulnerability_id"] + ) + for index, vulnerability in enumerate(package_data["resolved_vulnerabilities"]): + package_data["resolved_vulnerabilities"][index]["references"] = sorted( + vulnerability["references"], key=lambda x: (x["reference_id"], x["url"]) + ) + for index2, reference in enumerate( + package_data["resolved_vulnerabilities"][index]["references"] + ): + reference["scores"] = sorted( + reference["scores"], key=lambda x: (x["value"], x["scoring_system"]) + ) + package_data["resolved_vulnerabilities"][index]["references"][index2][ + "scores" + ] = reference["scores"] + + cleaned_response.append(package_data) + + return cleaned_response + + class TestDebianResponse(TestCase): fixtures = ["debian.json"] @@ -210,53 +257,55 @@ def test_bulk_packages_api(self): ).json() expected_response = [ - {}, { - "url": "http://testserver/api/packages/3467", + "name": "doesnotexist", + "namespace": "debian", + "qualifiers": {"distro": "jessie"}, + "resolved_vulnerabilities": [], + "subpath": None, + "type": "deb", + "unresolved_vulnerabilities": [], + "version": "0.9.7-10", + }, + { + "name": "datadog-api-client", + "namespace": "com.datadoghq", + "purl": "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", + "qualifiers": {}, + "resolved_vulnerabilities": [], + "subpath": "", + "type": "maven", "unresolved_vulnerabilities": [ { - "url": "http://testserver/api/vulnerabilities/60", - "vulnerability_id": "CVE-2021-21331", "references": [ { - "source": "", - "reference_id": "GHSA-2cxf-6567-7pp6", - "url": "https://github.com/advisories/GHSA-2cxf-6567-7pp6", + "reference_id": "", "scores": [], - }, - { "source": "", - "reference_id": "", "url": "https://nvd.nist.gov/vuln/detail/CVE-2021-21331", - "scores": [], }, { - "source": "", "reference_id": "GHSA-2cxf-6567-7pp6", + "scores": [{"scoring_system": "cvssv3.1_qr", "value": "LOW"}], + "source": "", "url": "https://github.com/DataDog/datadog-api-client-java/security/advisories/GHSA-2cxf-6567-7pp6", - "scores": [{"value": "LOW", "scoring_system": "cvssv3.1_qr"}], + }, + { + "reference_id": "GHSA-2cxf-6567-7pp6", + "scores": [], + "source": "", + "url": "https://github.com/advisories/GHSA-2cxf-6567-7pp6", }, ], + "url": "http://testserver/api/vulnerabilities/60", + "vulnerability_id": "CVE-2021-21331", } ], - "resolved_vulnerabilities": [], - "purl": "pkg:maven/com.datadoghq/datadog-api-client@1.0.0-beta.7", - "type": "maven", - "namespace": "com.datadoghq", - "name": "datadog-api-client", + "url": "http://testserver/api/packages/3467", "version": "1.0.0-beta.7", - "subpath": "", - "qualifiers": {}, }, ] - - # This normalization is brittle - expected_response[1]["unresolved_vulnerabilities"][0]["references"].sort( - key=lambda x: x["url"] - ) - response[1]["unresolved_vulnerabilities"][0]["references"].sort(key=lambda x: x["url"]) - - assert all([purl_response in expected_response for purl_response in response]) + assert cleaned_response(expected_response) == cleaned_response(response) def test_invalid_request_bulk_packages(self): error_response = { From ba81f50ebb2134ae30c7e6e71c1c96b4021fd272 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Wed, 10 Mar 2021 20:15:57 +0530 Subject: [PATCH 11/12] Refactor api tests to handle new concise error messages Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 6 ++---- vulnerabilities/tests/test_api.py | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index d8b078a04..1a32fdb2e 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -142,15 +142,13 @@ def bulk_search(self, request): if not purls or not isinstance(purls, list): return Response( status=400, - data={ - "Error": "A non-empty 'purls' list of package URLs is required." - }, + data={"Error": "A non-empty 'purls' list of package URLs is required."}, ) for purl in request.data["purls"]: try: purl = PackageURL.from_string(purl).to_dict() except ValueError as ve: - return Response(status=400, data={"Error": f"Invalid Package URL: {purl}") + return Response(status=400, data={"Error": f"Invalid Package URL: {purl}"}) purl_data = Package.objects.filter( **{key: value for key, value in purl.items() if value} ) diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index fddcfa285..f95024e31 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -309,7 +309,7 @@ def test_bulk_packages_api(self): def test_invalid_request_bulk_packages(self): error_response = { - "Error": "Request needs to contain a key 'purls' which has the value of a list of package urls" # nopep8 + "Error": "A non-empty 'purls' list of package URLs is required." # nopep8 } invalid_key_request_data = {"pkg": []} response = self.client.post( @@ -340,6 +340,6 @@ def test_invalid_request_bulk_packages(self): content_type="application/json", ).data purl_error_respones = { - "Error": "purl is missing the required \"pkg\" scheme component: 'pg:deb/debian/mimetex@1.50-1.1?distro=jessie'." # nopep8 + "Error": "Invalid Package URL: pg:deb/debian/mimetex@1.50-1.1?distro=jessie" } assert response == purl_error_respones From b399046e73c7959461903e7f32a1dc76ea25c270 Mon Sep 17 00:00:00 2001 From: Shivam Sandbhor Date: Thu, 11 Mar 2021 09:17:44 +0530 Subject: [PATCH 12/12] Add docstring for minimal serializers Signed-off-by: Shivam Sandbhor --- vulnerabilities/api.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 1a32fdb2e..fce2f2516 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -56,8 +56,11 @@ class Meta: fields = ["source", "reference_id", "url", "scores"] -# Used for nesting inside vulnerability focused APIs. class MinimalPackageSerializer(serializers.HyperlinkedModelSerializer): + """ + Used for nesting inside vulnerability focused APIs. + """ + purl = serializers.CharField(source="package_url") class Meta: @@ -65,8 +68,11 @@ class Meta: fields = ["url", "purl"] -# Used for nesting inside package focused APIs. class MinimalVulnerabilitySerializer(serializers.HyperlinkedModelSerializer): + """ + Used for nesting inside package focused APIs. + """ + references = VulnerabilityReferenceSerializer(many=True, source="vulnerabilityreference_set") class Meta: