From 255ccc8d61a92e5c5eb87a5380739cc7dc106969 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 00:17:30 +0000 Subject: [PATCH] fix(api): return a clear 400 when a scan import exceeds upload limits The import, reimport, and metadata-import permission checks parse request.data to resolve the target product/engagement/test before the serializer runs. When a scan import submits more form fields than DATA_UPLOAD_MAX_NUMBER_FIELDS (or a body larger than DATA_UPLOAD_MAX_MEMORY_SIZE), Django's multipart parser raises a SuspiciousOperation (TooManyFieldsSent / RequestDataTooBig) while request.data is evaluated. That exception escaped the permission check as an opaque error and generated error-reporting noise. Catch those exceptions in the three import/reimport permission classes and raise a DRF ValidationError with an actionable message instead. Also make DATA_UPLOAD_MAX_NUMBER_FIELDS configurable via DD_DATA_UPLOAD_MAX_NUMBER_FIELDS (default 10240), mirroring DD_DATA_UPLOAD_MAX_MEMORY_SIZE, so operators can raise the limit for instances that legitimately submit very large imports. Adds unit tests covering all three permission classes and the setting default. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWbcF7wUybs9Z2bFCpEi49 --- dojo/authorization/api_permissions.py | 40 +++++++++++ dojo/settings/settings.dist.py | 5 +- .../test_import_permission_upload_limits.py | 70 +++++++++++++++++++ 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 unittests/test_import_permission_upload_limits.py diff --git a/dojo/authorization/api_permissions.py b/dojo/authorization/api_permissions.py index 24e5fdd37f2..3a801e6dc35 100644 --- a/dojo/authorization/api_permissions.py +++ b/dojo/authorization/api_permissions.py @@ -1,5 +1,6 @@ from django.conf import settings +from django.core.exceptions import RequestDataTooBig, TooManyFieldsSent from django.db.models import Model from django.shortcuts import get_object_or_404 from rest_framework import permissions, serializers @@ -544,6 +545,19 @@ def has_permission(self, request, view): converted_dict["product_type"] = auto_create.get_target_product_type_if_exists(**converted_dict) converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict) converted_dict["engagement"] = auto_create.get_target_engagement_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) @@ -604,6 +618,19 @@ def has_permission(self, request, view): product = auto_create.get_target_product_if_exists(**converted_dict) if not product: product = auto_create.get_target_product_by_id_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) @@ -725,6 +752,19 @@ def has_permission(self, request, view): converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict) converted_dict["engagement"] = auto_create.get_target_engagement_if_exists(**converted_dict) converted_dict["test"] = auto_create.get_target_test_if_exists(**converted_dict) + except (TooManyFieldsSent, RequestDataTooBig) as e: + # A very large scan import (too many form fields, or a body over the size limit) + # trips Django's DATA_UPLOAD_MAX_NUMBER_FIELDS / DATA_UPLOAD_MAX_MEMORY_SIZE guard + # while this permission check parses request.data. Surface it as a clear client + # error instead of letting the SuspiciousOperation escape as an opaque 400 that + # also pages on-call via error reporting. + msg = ( + "The scan import request exceeded the server's upload limits " + "(too many form fields, or the request body is too large). Reduce the " + "number of fields in the request, or ask your administrator to increase " + "DD_DATA_UPLOAD_MAX_NUMBER_FIELDS / DD_DATA_UPLOAD_MAX_MEMORY_SIZE." + ) + raise ValidationError(msg) from e except (ValueError, TypeError) as e: # Raise an explicit drf exception here raise ValidationError(e) diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index e9293884764..3a3f9ff217b 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -157,6 +157,7 @@ DD_SECRET_KEY=(str, ""), DD_CREDENTIAL_AES_256_KEY=(str, "."), DD_DATA_UPLOAD_MAX_MEMORY_SIZE=(int, 8388608), # Max post size set to 8mb + DD_DATA_UPLOAD_MAX_NUMBER_FIELDS=(int, 10240), # Max number of GET/POST parameters in a request DD_MAX_ZIP_MEMBERS=(int, 1000), DD_MAX_ZIP_MEMBER_SIZE=(int, 512 * 1024 * 1024), # 512 MB per member (uncompressed) DD_MAX_ZIP_TOTAL_SIZE=(int, 1 * 1024 * 1024 * 1024), # 1 GB total (uncompressed) @@ -2096,7 +2097,9 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param DEFAULT_EXCEPTION_REPORTER_FILTER = "dojo.settings.exception_filter.CustomExceptionReporterFilter" # Issue on benchmark : "The number of GET/POST parameters exceeded settings.DATA_UPLOAD_MAX_NUMBER_FIELD S" -DATA_UPLOAD_MAX_NUMBER_FIELDS = 10240 +# Configurable so operators can raise it for instances that legitimately submit very large +# scan imports (many form fields), mirroring DD_DATA_UPLOAD_MAX_MEMORY_SIZE above. +DATA_UPLOAD_MAX_NUMBER_FIELDS = env("DD_DATA_UPLOAD_MAX_NUMBER_FIELDS") # Maximum size of a scan file in MB SCAN_FILE_MAX_SIZE = env("DD_SCAN_FILE_MAX_SIZE") diff --git a/unittests/test_import_permission_upload_limits.py b/unittests/test_import_permission_upload_limits.py new file mode 100644 index 00000000000..4d734e3bf57 --- /dev/null +++ b/unittests/test_import_permission_upload_limits.py @@ -0,0 +1,70 @@ +""" +Regression tests for scan-import permission checks under upload limits. + +The import/reimport permission classes parse ``request.data`` inside their +``has_permission`` to resolve the target product/engagement/test before the +serializer runs. A very large scan import (many form fields) makes Django's +multipart parser raise ``TooManyFieldsSent`` (a ``SuspiciousOperation``) while +``request.data`` is evaluated. That exception used to escape the permission +check as an opaque error and generate on-call noise; it must instead surface as +a clean DRF ``ValidationError`` (HTTP 400) with an actionable message. + +The limit itself (``DATA_UPLOAD_MAX_NUMBER_FIELDS``) is now configurable via the +``DD_DATA_UPLOAD_MAX_NUMBER_FIELDS`` environment variable so operators can raise +it for instances that legitimately submit very large imports. +""" +from django.conf import settings +from django.core.exceptions import TooManyFieldsSent +from django.test import SimpleTestCase, override_settings +from rest_framework.exceptions import ValidationError +from rest_framework.parsers import FormParser, MultiPartParser +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory + +from dojo.authorization.api_permissions import ( + UserHasImportPermission, + UserHasMetaImportPermission, + UserHasReimportPermission, +) + +IMPORT_PERMISSION_CLASSES = ( + UserHasImportPermission, + UserHasMetaImportPermission, + UserHasReimportPermission, +) + + +class ImportPermissionUploadLimitsTest(SimpleTestCase): + def _multipart_request(self, field_count: int) -> Request: + payload = {f"field_{i}": "x" for i in range(field_count)} + django_request = APIRequestFactory().post( + "/api/v2/import-scan/", payload, format="multipart", + ) + return Request(django_request, parsers=[MultiPartParser(), FormParser()]) + + @override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=5) + def test_too_many_fields_raises_validation_error(self): + # Without the fix Django's TooManyFieldsSent escapes the permission check; + # with the fix each import/reimport permission raises a DRF ValidationError. + for permission_class in IMPORT_PERMISSION_CLASSES: + with self.subTest(permission=permission_class.__name__): + request = self._multipart_request(field_count=20) + with self.assertRaises(ValidationError) as ctx: + permission_class().has_permission(request, view=None) + self.assertIn("upload limits", str(ctx.exception).lower()) + + @override_settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=5) + def test_request_within_limit_parses_without_size_error(self): + # A request under the limit parses normally (no SuspiciousOperation). + request = self._multipart_request(field_count=3) + try: + parsed = request.data + except TooManyFieldsSent: # pragma: no cover - would mean the guard misfired + self.fail("request under the field limit must not raise TooManyFieldsSent") + self.assertEqual(sorted(parsed.keys()), ["field_0", "field_1", "field_2"]) + + +class DataUploadMaxNumberFieldsSettingTest(SimpleTestCase): + def test_default_value(self): + # Default preserved while the value is now sourced from the environment. + self.assertEqual(settings.DATA_UPLOAD_MAX_NUMBER_FIELDS, 10240)