Skip to content

Commit 6dd70f8

Browse files
Maffoochclaude
andcommitted
Strip undeclared engagement field in reimport permission check
The engagement field is not declared on ReImportScanSerializer and gets stripped during validation. The permission check must also strip it so it resolves targets the same way execution does — by name, not by a stale engagement ID from request.data. Update test to verify the engagement param is ignored and permission is checked against the name-resolved target. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 92cd890 commit 6dd70f8

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

dojo/api_v2/permissions.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,11 @@ def has_permission(self, request, view):
770770
try:
771771
converted_dict = auto_create.convert_querydict_to_dict(request.data)
772772
auto_create.process_import_meta_data_from_dict(converted_dict)
773+
# engagement is not a declared field on ReImportScanSerializer and will be
774+
# stripped during validation — don't use it in the permission check either,
775+
# so the permission check resolves targets the same way execution does
776+
converted_dict.pop("engagement", None)
777+
converted_dict.pop("engagement_id", None)
773778
# Get an existing product
774779
converted_dict["product_type"] = auto_create.get_target_product_type_if_exists(**converted_dict)
775780
converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict)

unittests/test_rest_framework.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3433,8 +3433,13 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self
34333433

34343434
@patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan")
34353435
@patch("dojo.importers.default_importer.DefaultImporter.process_scan")
3436-
def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock):
3437-
"""Sending engagement ID from one product with product_name from another must be rejected."""
3436+
@patch("dojo.api_v2.permissions.user_has_permission")
3437+
def test_reimport_engagement_param_ignored_permission_checked_on_name_resolved_target(self, mock, importer_mock, reimporter_mock):
3438+
"""
3439+
Engagement is not a declared field on ReImportScanSerializer — verify
3440+
the permission check uses the name-resolved target, not the engagement param.
3441+
"""
3442+
mock.return_value = False
34383443
importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE
34393444
reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE
34403445

@@ -3445,15 +3450,20 @@ def test_reimport_with_engagement_id_mismatched_product_name_is_rejected(self, i
34453450
"verified": True,
34463451
"scan_type": "ZAP Scan",
34473452
"file": testfile,
3448-
# Engagement 1 belongs to Product 2 ("Security How-to")
3453+
# engagement=1 belongs to Product 2 Engagement 1, but it should be ignored
34493454
"engagement": 1,
3450-
# But product_name points to Product 1 ("Python How-to")
3451-
"product_name": "Python How-to",
3455+
# These names resolve to Product 2's Engagement 4 -> Test 4
3456+
"product_name": "Security How-to",
34523457
"engagement_name": "April monthly engagement",
34533458
"version": "1.0.0",
34543459
}
34553460
response = self.client.post(self.url, payload)
3456-
self.assertEqual(400, response.status_code, response.content[:1000])
3461+
self.assertEqual(403, response.status_code, response.content[:1000])
3462+
# Permission must be checked on name-resolved Test 4 (in Engagement 4),
3463+
# NOT on Test 3 (which belongs to the engagement=1 param)
3464+
mock.assert_called_with(User.objects.get(username="admin"),
3465+
Test.objects.get(id=4),
3466+
Permissions.Import_Scan_Result)
34573467
importer_mock.assert_not_called()
34583468
reimporter_mock.assert_not_called()
34593469

0 commit comments

Comments
 (0)