Skip to content

Commit 5b8a8b1

Browse files
authored
Merge branch 'bugfix' into sc-13470_defender_cloud_docs
2 parents a1c2473 + fa72db5 commit 5b8a8b1

6 files changed

Lines changed: 234 additions & 42 deletions

File tree

dojo/importers/default_importer.py

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,12 @@ def _process_findings_internal(
182182
parsed_findings: list[Finding],
183183
**kwargs: dict,
184184
) -> list[Finding]:
185-
# Batched post-processing (no chord): dispatch a task per 1000 findings or on final finding
186-
batch_finding_ids: list[int] = []
185+
# Batched post-processing (no chord): dispatch a task per 1000 findings or on final finding.
186+
# Each entry carries the finding's own push_to_jira flag: grouped findings are pushed to
187+
# JIRA as a group after the loop, so their individual push is suppressed while ungrouped
188+
# findings in the same batch must still be pushed. The batch is partitioned by flag at
189+
# dispatch time instead of applying one finding's flag to the whole batch.
190+
batch_finding_ids: list[tuple[int, bool]] = []
187191
batch_findings: list[Finding] = []
188192
batch_max_size = getattr(settings, "IMPORT_REIMPORT_DEDUPE_BATCH_SIZE", 1000)
189193

@@ -274,7 +278,7 @@ def _process_findings_internal(
274278
self.push_to_jira, self.findings_groups_enabled, self.group_by)
275279
push_to_jira = self.push_to_jira and ((not self.findings_groups_enabled or not self.group_by) or not finding_will_be_grouped)
276280
logger.debug("process_findings: computed push_to_jira=%s", push_to_jira)
277-
batch_finding_ids.append(finding.id)
281+
batch_finding_ids.append((finding.id, push_to_jira))
278282
batch_findings.append(finding)
279283

280284
# If batch is full or we're at the end, persist locations/endpoints and dispatch
@@ -293,25 +297,31 @@ def _process_findings_internal(
293297
# dispatches, so rules/dedup see inherited tags on .tags.
294298
apply_inherited_tags_for_findings(batch_findings)
295299
batch_findings.clear()
296-
finding_ids_batch = list(batch_finding_ids)
300+
# Partition the batch by each finding's own push_to_jira flag so one
301+
# finding's grouping state is not applied to the whole batch. Uniform
302+
# batches (grouping disabled, or push_to_jira off) stay a single dispatch.
303+
finding_ids_by_push: dict[bool, list[int]] = {}
304+
for finding_id, finding_push_to_jira in batch_finding_ids:
305+
finding_ids_by_push.setdefault(finding_push_to_jira, []).append(finding_id)
297306
batch_finding_ids.clear()
298-
logger.debug("process_findings: dispatching batch with push_to_jira=%s (batch_size=%d, is_final=%s)",
299-
push_to_jira, len(finding_ids_batch), is_final_finding)
300-
result = dojo_dispatch_task(
301-
finding_helper.post_process_findings_batch,
302-
finding_ids_batch,
303-
dedupe_option=True,
304-
rules_option=True,
305-
product_grading_option=True,
306-
issue_updater_option=True,
307-
push_to_jira=push_to_jira,
308-
# 'async_wait' joins on this dispatch via AsyncResult.get(), so its
309-
# result must be stored despite the global CELERY_TASK_IGNORE_RESULT.
310-
**({"ignore_result": False} if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT else {}),
311-
**self.post_processing_dispatch_kwargs(**kwargs),
312-
)
313-
if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT:
314-
self.record_post_processing_result(result)
307+
for push_to_jira_batch, finding_ids_batch in finding_ids_by_push.items():
308+
logger.debug("process_findings: dispatching batch with push_to_jira=%s (batch_size=%d, is_final=%s)",
309+
push_to_jira_batch, len(finding_ids_batch), is_final_finding)
310+
result = dojo_dispatch_task(
311+
finding_helper.post_process_findings_batch,
312+
finding_ids_batch,
313+
dedupe_option=True,
314+
rules_option=True,
315+
product_grading_option=True,
316+
issue_updater_option=True,
317+
push_to_jira=push_to_jira_batch,
318+
# 'async_wait' joins on this dispatch via AsyncResult.get(), so its
319+
# result must be stored despite the global CELERY_TASK_IGNORE_RESULT.
320+
**({"ignore_result": False} if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT else {}),
321+
**self.post_processing_dispatch_kwargs(**kwargs),
322+
)
323+
if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT:
324+
self.record_post_processing_result(result)
315325

316326
# No chord: tasks are dispatched immediately above per batch
317327

@@ -402,7 +412,7 @@ def close_old_findings(
402412
# Remove all the findings that are coming from the report already mitigated
403413
new_hash_codes = []
404414
new_unique_ids_from_tool = []
405-
for finding in findings.values():
415+
for finding in findings.values("is_mitigated", "hash_code", "unique_id_from_tool"):
406416
# Do not process closed findings in the report
407417
if finding.get("is_mitigated", False):
408418
continue

dojo/importers/default_reimporter.py

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,11 @@ def _process_findings_internal(
320320
continue
321321
cleaned_findings.append(sanitized)
322322

323-
batch_finding_ids: list[int] = []
323+
# Each entry carries the finding's own push_to_jira flag: grouped findings are pushed to
324+
# JIRA as a group, so their individual push is suppressed while ungrouped findings in the
325+
# same batch must still be pushed. The batch is partitioned by flag at dispatch time
326+
# instead of applying one finding's flag to the whole batch.
327+
batch_finding_ids: list[tuple[int, bool]] = []
324328
batch_findings: list[Finding] = []
325329
# Findings that were newly created (else branch below) — pass these to
326330
# `apply_inherited_tags_for_findings` instead of `batch_findings` so
@@ -423,7 +427,7 @@ def _process_findings_internal(
423427
)
424428
# all data is already saved on the finding, we only need to trigger post processing in batches
425429
push_to_jira = self.push_to_jira and ((not self.findings_groups_enabled or not self.group_by) or not finding_will_be_grouped)
426-
batch_finding_ids.append(finding.id)
430+
batch_finding_ids.append((finding.id, push_to_jira))
427431
batch_findings.append(finding)
428432

429433
# Post-processing batches (deduplication, rules, etc.) are separate from matching batches.
@@ -460,24 +464,30 @@ def _process_findings_internal(
460464
apply_inherited_tags_for_findings(new_findings_in_batch)
461465
new_findings_in_batch.clear()
462466
batch_findings.clear()
463-
finding_ids_batch = list(batch_finding_ids)
467+
# Partition the batch by each finding's own push_to_jira flag so one
468+
# finding's grouping state is not applied to the whole batch. Uniform
469+
# batches (grouping disabled, or push_to_jira off) stay a single dispatch.
470+
finding_ids_by_push: dict[bool, list[int]] = {}
471+
for finding_id, finding_push_to_jira in batch_finding_ids:
472+
finding_ids_by_push.setdefault(finding_push_to_jira, []).append(finding_id)
464473
batch_finding_ids.clear()
465-
result = dojo_dispatch_task(
466-
finding_helper.post_process_findings_batch,
467-
finding_ids_batch,
468-
dedupe_option=True,
469-
rules_option=True,
470-
product_grading_option=True,
471-
issue_updater_option=True,
472-
push_to_jira=push_to_jira,
473-
jira_instance_id=getattr(self.jira_instance, "id", None),
474-
# 'async_wait' joins on this dispatch via AsyncResult.get(), so its
475-
# result must be stored despite the global CELERY_TASK_IGNORE_RESULT.
476-
**({"ignore_result": False} if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT else {}),
477-
**self.post_processing_dispatch_kwargs(**kwargs),
478-
)
479-
if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT:
480-
self.record_post_processing_result(result)
474+
for push_to_jira_batch, finding_ids_batch in finding_ids_by_push.items():
475+
result = dojo_dispatch_task(
476+
finding_helper.post_process_findings_batch,
477+
finding_ids_batch,
478+
dedupe_option=True,
479+
rules_option=True,
480+
product_grading_option=True,
481+
issue_updater_option=True,
482+
push_to_jira=push_to_jira_batch,
483+
jira_instance_id=getattr(self.jira_instance, "id", None),
484+
# 'async_wait' joins on this dispatch via AsyncResult.get(), so its
485+
# result must be stored despite the global CELERY_TASK_IGNORE_RESULT.
486+
**({"ignore_result": False} if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT else {}),
487+
**self.post_processing_dispatch_kwargs(**kwargs),
488+
)
489+
if self.deduplication_execution_mode == DEDUPLICATION_EXECUTION_MODE_ASYNC_WAIT:
490+
self.record_post_processing_result(result)
481491

482492
# No chord: tasks are dispatched immediately above per batch
483493

dojo/middleware.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,15 @@ def install_intermediate_flush_hook():
312312

313313
def add_to_context_with_flush(self, engine, obj):
314314
original_add(self, engine, obj)
315+
# The intermediate flush is a request-path optimization on the global singleton
316+
# context (AsyncSearchContextMiddleware). The async reindex task
317+
# update_watson_search_index_for_model() builds its OWN SearchContextManager and
318+
# IS the drain target -- if it re-drained its batch it would dispatch a clone of
319+
# itself, discard those pks unindexed, and loop forever (queue ~0, worker pegged,
320+
# nothing indexed). Only the singleton intermediate-flushes; any ad-hoc context
321+
# manager indexes its own batch on end().
322+
if self is not search_context_manager:
323+
return
315324
threshold = getattr(settings, "WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE", 1000)
316325
if threshold <= 0 or not self._stack:
317326
return

unittests/test_importers_closeold.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from unittest import mock
23

34
from django.utils import timezone
45

@@ -12,6 +13,61 @@
1213

1314

1415
class TestDojoCloseOld(DojoTestCase):
16+
# Regression: close_old_findings fetched every Finding column (incl. description) via
17+
# findings.values() just to collect is_mitigated/hash_code/unique_id_from_tool.
18+
def test_close_old_findings_only_fetches_needed_columns(self):
19+
scan_type = "Acunetix Scan"
20+
user, _ = User.objects.get_or_create(username="admin")
21+
product_type, _ = Product_Type.objects.get_or_create(name="closeold")
22+
environment, _ = Development_Environment.objects.get_or_create(name="Development")
23+
product, _ = Product.objects.get_or_create(
24+
name="TestDojoCloseOldColumns",
25+
description="Test",
26+
prod_type=product_type,
27+
)
28+
engagement, _ = Engagement.objects.get_or_create(
29+
name="Close Old Needed Columns",
30+
product=product,
31+
target_start=timezone.now(),
32+
target_end=timezone.now(),
33+
)
34+
import_options = {
35+
"user": user,
36+
"lead": user,
37+
"scan_date": None,
38+
"environment": environment,
39+
"active": True,
40+
"verified": False,
41+
"engagement": engagement,
42+
"scan_type": scan_type,
43+
}
44+
# Seed an older test with 4 findings that the close-old pass should mitigate
45+
with (get_unit_tests_scans_path("acunetix") / "many_findings.xml").open(encoding="utf-8") as many_findings_scan:
46+
importer = DefaultImporter(close_old_findings=False, **import_options)
47+
_, _, len_new_findings, _, _, _, _ = importer.process_scan(many_findings_scan)
48+
self.assertEqual(4, len_new_findings)
49+
# Import a report with a single finding, without closing, to get the new test
50+
with (get_unit_tests_scans_path("acunetix") / "one_finding.xml").open(encoding="utf-8") as single_finding_scan:
51+
importer = DefaultImporter(close_old_findings=False, **import_options)
52+
test, _, len_new_findings, _, _, _, _ = importer.process_scan(single_finding_scan)
53+
self.assertEqual(1, len_new_findings)
54+
# Call close_old_findings directly with the new test's queryset and spy on values()
55+
importer = DefaultImporter(close_old_findings=True, **import_options)
56+
importer.test = test
57+
findings_queryset = test.finding_set.all()
58+
with mock.patch.object(findings_queryset, "values", wraps=findings_queryset.values) as values_spy:
59+
closed_findings = importer.close_old_findings(findings_queryset)
60+
values_spy.assert_called_once_with(
61+
"is_mitigated", "hash_code", "unique_id_from_tool",
62+
)
63+
# Control: narrowing the columns must not change which findings get closed
64+
# (the 4 findings from the older test; dedupe is off, so the overlapping
65+
# finding closes too — same semantics as test_close_old_same_engagement)
66+
self.assertEqual(
67+
4, len(closed_findings),
68+
msg=f"expected 4 old findings closed, got {len(closed_findings)}",
69+
)
70+
1571
def test_close_old_same_engagement(self):
1672
scan_type = "Acunetix Scan"
1773
user, _ = User.objects.get_or_create(username="admin")

unittests/test_importers_importer.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,3 +1143,91 @@ def test_reactivation_of_non_duplicate_still_activates(self):
11431143
self.assertIsNone(result_finding.mitigated)
11441144
self.assertTrue(result_finding.active)
11451145
self.assertTrue(result_finding.verified)
1146+
1147+
1148+
# Regression: the per-batch post-processing dispatch read the push_to_jira flag computed
1149+
# for the LAST finding of the batch and applied it to the entire batch. With finding
1150+
# groups enabled and push_to_jira=True, a mixed batch ending in a grouped finding
1151+
# suppressed the JIRA push for every ungrouped finding in the batch (and vice versa).
1152+
class TestDojoImporterBatchPushToJira(DojoTestCase):
1153+
def _process_findings_with_groups(self, parsed_findings):
1154+
"""Run process_findings with push_to_jira + group_by and capture per-finding dispatch flags."""
1155+
scan_type = "Acunetix Scan"
1156+
user, _ = User.objects.get_or_create(username="admin")
1157+
product_type, _ = Product_Type.objects.get_or_create(name="batch_push")
1158+
product, _ = Product.objects.get_or_create(
1159+
name="TestDojoImporterBatchPushToJira",
1160+
description="test product",
1161+
prod_type=product_type,
1162+
)
1163+
engagement, _ = Engagement.objects.get_or_create(
1164+
name="Batch Push To Jira",
1165+
product=product,
1166+
target_start=timezone.now(),
1167+
target_end=timezone.now(),
1168+
)
1169+
environment, _ = Development_Environment.objects.get_or_create(name="Development")
1170+
self.system_settings(enable_finding_groups=True)
1171+
importer = DefaultImporter(
1172+
user=user,
1173+
lead=user,
1174+
scan_date=None,
1175+
environment=environment,
1176+
minimum_severity="Info",
1177+
active=True,
1178+
verified=True,
1179+
scan_type=scan_type,
1180+
engagement=engagement,
1181+
push_to_jira=True,
1182+
group_by="component_name",
1183+
)
1184+
test = importer.create_test(scan_type)
1185+
for finding in parsed_findings:
1186+
finding.test = test
1187+
with (
1188+
patch("dojo.importers.default_importer.dojo_dispatch_task") as dispatch_mock,
1189+
patch("dojo.importers.default_importer.jira_services.push"),
1190+
):
1191+
new_findings = importer.process_findings(parsed_findings)
1192+
# Map each dispatched finding id to the push_to_jira flag its batch was sent with
1193+
flag_by_finding_id = {}
1194+
for call in dispatch_mock.call_args_list:
1195+
finding_ids = call.args[1]
1196+
for finding_id in finding_ids:
1197+
flag_by_finding_id[finding_id] = call.kwargs["push_to_jira"]
1198+
findings_by_title = {finding.title: finding for finding in new_findings}
1199+
return flag_by_finding_id, findings_by_title
1200+
1201+
def test_batch_push_to_jira_last_finding_grouped(self):
1202+
# Last finding of the batch is grouped -> its False flag must NOT leak onto the
1203+
# ungrouped finding processed before it
1204+
ungrouped = Finding(title="Ungrouped Finding", severity="Medium", description="no component")
1205+
grouped = Finding(title="Grouped Finding", severity="Medium", description="has component", component_name="lib-a")
1206+
flags, findings_by_title = self._process_findings_with_groups([ungrouped, grouped])
1207+
grouped_db = findings_by_title["Grouped Finding"]
1208+
ungrouped_db = findings_by_title["Ungrouped Finding"]
1209+
self.assertFalse(
1210+
flags[grouped_db.id],
1211+
msg=f"grouped finding must not be pushed individually, dispatched with push_to_jira={flags[grouped_db.id]}",
1212+
)
1213+
self.assertTrue(
1214+
flags[ungrouped_db.id],
1215+
msg=f"ungrouped finding must be pushed individually, dispatched with push_to_jira={flags[ungrouped_db.id]}",
1216+
)
1217+
1218+
def test_batch_push_to_jira_last_finding_ungrouped(self):
1219+
# Last finding of the batch is ungrouped -> its True flag must NOT leak onto the
1220+
# grouped finding processed before it
1221+
grouped = Finding(title="Grouped Finding 2", severity="Medium", description="has component", component_name="lib-b")
1222+
ungrouped = Finding(title="Ungrouped Finding 2", severity="Medium", description="no component")
1223+
flags, findings_by_title = self._process_findings_with_groups([grouped, ungrouped])
1224+
grouped_db = findings_by_title["Grouped Finding 2"]
1225+
ungrouped_db = findings_by_title["Ungrouped Finding 2"]
1226+
self.assertFalse(
1227+
flags[grouped_db.id],
1228+
msg=f"grouped finding must not be pushed individually, dispatched with push_to_jira={flags[grouped_db.id]}",
1229+
)
1230+
self.assertTrue(
1231+
flags[ungrouped_db.id],
1232+
msg=f"ungrouped finding must be pushed individually, dispatched with push_to_jira={flags[ungrouped_db.id]}",
1233+
)

0 commit comments

Comments
 (0)