Skip to content

Commit a58f164

Browse files
vvpoglazovmtesauro
andauthored
fix(metrics): use timezone.now() as upper bound for closed findings filter (#14464)
The closed findings counter showed 0 even when closed findings existed. Root cause: the upper bound for mitigated__range was derived from the latest finding discovery date (end_date), built as midnight (00:00:00). This made no semantic sense - a finding can be closed at any time after discovery, so using the discovery date as the cutoff for mitigated is incorrect. Fix: replace end_date with timezone.now() as the upper bound so any finding closed up to the current moment is counted. Applied in both dojo/product/views.py and dojo/metrics/utils.py. The bug "self-healed" the next day when a new finding with a later discovery date was imported - making it particularly hard to notice. Adds regression tests in unittests/test_product_metrics_closed_count.py. Updates test_closed_findings_filtered_by_mitigated_date to reflect correct behavior: findings closed after discovery date range but before now() should appear in closed metrics. Co-authored-by: Matt Tesauro <mtesauro@gmail.com>
1 parent a2aad99 commit a58f164

4 files changed

Lines changed: 143 additions & 8 deletions

File tree

dojo/metrics/utils.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@ def finding_queries(
7676

7777
# Filter by the date ranges supplied
7878
all_findings_within_date_range = all_authorized_findings.filter(date__range=[start_date, end_date])
79-
# Get the list of closed findings filtered by mitigated date (not discovery date)
80-
# This ensures findings closed within the date range are included even if discovered outside it
79+
# Filter closed findings by mitigated date (not discovery date).
80+
# Use timezone.now() as the upper bound so findings closed after the latest
81+
# discovery date are not excluded from the counter.
8182
closed_filtered_findings = all_authorized_findings.filter(
8283
CLOSED_FINDINGS_QUERY,
83-
mitigated__range=[start_date, end_date],
84+
mitigated__range=[start_date, timezone.now()],
8485
mitigated__isnull=False,
8586
)
8687
accepted_filtered_findings = all_findings_within_date_range.filter(ACCEPTED_FINDINGS_QUERY)

dojo/product/views.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,10 @@ def finding_queries(request, prod):
445445
filters["new_verified"] = findings_qs.filter(finding_helper.VERIFIED_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
446446
filters["open"] = findings_qs.filter(finding_helper.OPEN_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
447447
filters["inactive"] = findings_qs.filter(finding_helper.INACTIVE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
448-
# Filter closed findings by mitigated date (not discovery date) to show findings closed within the date range
449-
filters["closed"] = findings_qs.filter(finding_helper.CLOSED_FINDINGS_QUERY).filter(mitigated__range=[start_date, end_date], mitigated__isnull=False).order_by("mitigated")
448+
# Filter closed findings by mitigated date (not discovery date).
449+
# Use timezone.now() as the upper bound so findings closed after the latest
450+
# discovery date are not excluded from the counter.
451+
filters["closed"] = findings_qs.filter(finding_helper.CLOSED_FINDINGS_QUERY).filter(mitigated__range=[start_date, timezone.now()], mitigated__isnull=False).order_by("mitigated")
450452
filters["false_positive"] = findings_qs.filter(finding_helper.FALSE_POSITIVE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
451453
filters["out_of_scope"] = findings_qs.filter(finding_helper.OUT_OF_SCOPE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
452454
filters["all"] = findings_qs.order_by("date")

unittests/test_metrics_queries.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,10 @@ def test_closed_findings_filtered_by_mitigated_date(self, mock_timezone):
240240
self.assertIn(finding_discovered_before.id, closed_ids,
241241
"Finding discovered before range but closed within range should appear in closed metrics")
242242

243-
# The finding discovered within but closed after should NOT appear
244-
self.assertNotIn(finding_closed_after.id, closed_ids,
245-
"Finding discovered within range but closed after range should NOT appear in closed metrics")
243+
# The finding closed after the discovery date range but before now() should appear -
244+
# the upper bound for mitigated is timezone.now(), not end_date derived from discovery dates.
245+
self.assertIn(finding_closed_after.id, closed_ids,
246+
"Finding closed after discovery date range but before now() should appear in closed metrics")
246247

247248
# The finding discovered and closed within should appear
248249
self.assertIn(finding_both_within.id, closed_ids,
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
"""
2+
Regression test for: Product Metrics shows 0 Closed Findings while the findings
3+
list displays them correctly.
4+
5+
Root cause: finding_queries() in dojo/product/views.py used end_date (derived from
6+
the latest finding discovery date, built as midnight 00:00:00) as the upper bound
7+
for mitigated__range. This made no semantic sense - a finding can be closed at any
8+
time after discovery, so using the discovery date as the cutoff for mitigated is
9+
incorrect.
10+
11+
Fix: replace end_date with timezone.now() as the upper bound so any finding closed
12+
up to the current moment is counted.
13+
"""
14+
15+
import zoneinfo
16+
from datetime import date, datetime
17+
18+
from django.test import RequestFactory
19+
20+
from dojo.models import Engagement, Finding, Product, Product_Type, Test, Test_Type, User
21+
from dojo.product.views import finding_queries
22+
23+
from .dojo_test_case import DojoTestCase
24+
25+
UTC = zoneinfo.ZoneInfo("UTC")
26+
27+
28+
class ProductMetricsClosedCountTest(DojoTestCase):
29+
30+
"""Regression tests for closed finding counter in Product Metrics."""
31+
32+
def setUp(self):
33+
self.user = User.objects.create_superuser(username="admin_regression", password="test") # noqa: S106
34+
self.product_type = Product_Type.objects.create(name="Regression PT")
35+
self.product = Product.objects.create(
36+
name="Regression Product",
37+
prod_type=self.product_type,
38+
description="Regression test product",
39+
)
40+
self.engagement = Engagement.objects.create(
41+
name="Regression Eng",
42+
product=self.product,
43+
target_start=date(2024, 1, 1),
44+
target_end=date(2024, 12, 31),
45+
)
46+
self.test_type, _ = Test_Type.objects.get_or_create(name="Manual")
47+
self.test = Test.objects.create(
48+
engagement=self.engagement,
49+
test_type=self.test_type,
50+
target_start=datetime(2024, 1, 1, tzinfo=UTC),
51+
target_end=datetime(2024, 12, 31, tzinfo=UTC),
52+
)
53+
# date=7 corresponds to "Any date" in MetricsDateRangeFilter
54+
self.request = RequestFactory().get(f"/product/{self.product.id}/metrics", {"date": "7"})
55+
self.request.user = self.user
56+
57+
def _make_closed(self, title, discovery_date, mitigated_dt, severity="High"):
58+
return Finding.objects.create(
59+
title=title,
60+
test=self.test,
61+
severity=severity,
62+
active=False,
63+
is_mitigated=True,
64+
date=discovery_date,
65+
mitigated=mitigated_dt,
66+
reporter=self.user,
67+
verified=True,
68+
)
69+
70+
def test_closed_finding_same_day_after_midnight_is_counted(self):
71+
"""
72+
A finding discovered on day X and closed on day X at 10:00 must appear
73+
in the closed counter. Before the fix end_date was derived from the latest
74+
discovery date as midnight, so this finding was silently excluded.
75+
"""
76+
discovery = date(2024, 6, 15)
77+
# closed on the same day but well after midnight
78+
mitigated = datetime(2024, 6, 15, 10, 30, tzinfo=UTC)
79+
self._make_closed("Closed same-day after midnight", discovery, mitigated)
80+
81+
filters = finding_queries(self.request, self.product)
82+
closed_ids = list(filters["closed"].values_list("id", flat=True))
83+
self.assertEqual(len(closed_ids), 1, "Expected exactly 1 closed finding in metrics")
84+
85+
def test_closed_finding_on_same_day_as_end_date_is_counted(self):
86+
"""
87+
A finding with discovery date in the past but closed recently must appear
88+
in the closed counter. Before the fix end_date was the latest discovery date
89+
(midnight), so findings closed after that date were excluded.
90+
"""
91+
today = date(2025, 6, 15)
92+
# open finding - sets end_date to today
93+
Finding.objects.create(
94+
title="Open Finding - sets end_date",
95+
test=self.test, severity="Low",
96+
active=True, is_mitigated=False,
97+
date=today, reporter=self.user,
98+
)
99+
# closed finding - mitigated today at 10:00 (after midnight)
100+
closed = self._make_closed(
101+
"Closed same day as end_date",
102+
date(2024, 11, 27),
103+
datetime(2025, 6, 15, 10, 0, tzinfo=UTC),
104+
)
105+
106+
filters = finding_queries(self.request, self.product)
107+
closed_ids = list(filters["closed"].values_list("id", flat=True))
108+
self.assertIn(
109+
closed.id, closed_ids,
110+
"Finding mitigated on the same day as end_date (but after midnight) must appear in closed metrics",
111+
)
112+
113+
def test_closed_count_matches_findings_list_count(self):
114+
"""
115+
All closed findings whose mitigated date falls within [start_date, end_date_eod]
116+
must be counted. Specifically, findings closed on the same day as end_date
117+
at any time (including 23:59) must appear.
118+
"""
119+
# All three findings have the same discovery date, so end_date = 2024-03-01 23:59:59
120+
self._make_closed("F1 - closed at 00:01", date(2024, 3, 1), datetime(2024, 3, 1, 0, 1, tzinfo=UTC))
121+
self._make_closed("F2 - closed at 14:00", date(2024, 3, 1), datetime(2024, 3, 1, 14, 0, tzinfo=UTC))
122+
self._make_closed("F3 - closed at 23:58", date(2024, 3, 1), datetime(2024, 3, 1, 23, 58, tzinfo=UTC))
123+
124+
filters = finding_queries(self.request, self.product)
125+
metrics_count = filters["closed"].count()
126+
127+
self.assertEqual(
128+
metrics_count,
129+
3,
130+
f"All 3 findings closed on end_date must be counted, got {metrics_count}",
131+
)

0 commit comments

Comments
 (0)