From 568f0cebefb47b212f656228f1c2c69af2721c52 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Tue, 21 Apr 2026 16:06:30 -0400 Subject: [PATCH 1/3] fix jira bug --- dojo/jira_link/helper.py | 63 ++++++++++++++--- dojo/jira_link/views.py | 22 +++++- unittests/test_jira_webhook.py | 121 +++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 10 deletions(-) diff --git a/dojo/jira_link/helper.py b/dojo/jira_link/helper.py index feff72003ef..76c2208185e 100644 --- a/dojo/jira_link/helper.py +++ b/dojo/jira_link/helper.py @@ -1133,7 +1133,23 @@ def failure_to_update_message(message: str, exception: Exception, obj: Any) -> t message = f"Failed to fetch fields for {jira_instance.default_issue_type} under project {jira_project.project_key} - {e}" return failure_to_update_message(message, e, obj) - # Update the issue in jira + # Update the status in jira FIRST, before applying the other field + # updates (priority, description, labels, etc.). This ordering matters + # because each call to the Jira REST API fires its own + # jira:issue_updated webhook. If we updated fields first, the + # pre-transition webhook would see the old (stale) resolution / + # statusCategory - for a reopen, the webhook would still report the + # issue as resolved and our handler would ricochet the linked finding + # back to mitigated before the transition webhook even arrived. By + # transitioning first, every webhook that fires during this sync sees + # the intended post-transition state. + try: + push_status_to_jira(obj, jira_instance, jira, issue) + except Exception as e: + message = f"Failed to update the jira issue status - {e}" + return failure_to_update_message(message, e, obj) + # Update the rest of the issue fields in jira (summary, description, + # priority, labels, due date, etc.) AFTER the transition above. try: logger.debug("Updating JIRA issue with fields: %s", json.dumps(fields, indent=4)) issue.update( @@ -1145,12 +1161,6 @@ def failure_to_update_message(message: str, exception: Exception, obj: Any) -> t except Exception as e: message = f"Failed to update the jira issue with the following payload: {fields} - {e}" return failure_to_update_message(message, e, obj) - # Update the status in jira - try: - push_status_to_jira(obj, jira_instance, jira, issue) - except Exception as e: - message = f"Failed to update the jira issue status - {e}" - return failure_to_update_message(message, e, obj) # Upload dojo finding screenshots to Jira try: findings = [obj] @@ -1887,11 +1897,46 @@ def escape_for_jira(text): return text.replace("|", "%7D") -def process_resolution_from_jira(finding, resolution_id, resolution_name, assignee_name, jira_now, jira_issue, finding_group: Finding_Group = None) -> bool: +def process_resolution_from_jira( + finding, + resolution_id, + resolution_name, + assignee_name, + jira_now, + jira_issue, + finding_group: Finding_Group = None, + *, + status_category_key: str | None = None, +) -> bool: """Processes the resolution field in the JIRA issue and updated the finding in Defect Dojo accordingly""" import dojo.risk_acceptance.helper as ra_helper # noqa: PLC0415 import error status_changed = False - resolved = resolution_id is not None + # A Jira issue is treated as "resolved" (i.e. the linked finding should be + # mitigated / risk-accepted / false-positive'd) only when BOTH a non-null + # resolution is present AND the issue's statusCategory is "done". Jira's + # statusCategory.key is the canonical closure signal (always one of + # "new", "indeterminate", "done"), and checking it alongside the + # resolution field prevents several real-world bugs: + # 1. Workflows where reopen transitions do not clear the resolution - + # the issue ends up in an "open but resolved" state and every + # webhook event for the issue would otherwise mis-mitigate the + # finding. + # 2. Workflows that set a default resolution on brand-new issues + # (for example "Unresolved") - a webhook would otherwise mis- + # mitigate the finding as soon as the issue was created. + # 3. Ricochets during DefectDojo's own push to Jira: the issue.update() + # call and the subsequent status transition each fire a webhook, + # and the first webhook sees the pre-transition (still-resolved) + # state even though the intended final state is reopened. + # + # status_category_key is keyword-only and optional to preserve backward + # compatibility with any caller that has not yet been updated to extract + # it from the webhook payload; when it is None we fall back to the + # historical resolution-only behavior. + if status_category_key is None: + resolved = resolution_id is not None + else: + resolved = resolution_id is not None and status_category_key == "done" jira_instance = get_jira_instance(finding) if resolved: diff --git a/dojo/jira_link/views.py b/dojo/jira_link/views.py index 061ad83d83c..58b738d4068 100644 --- a/dojo/jira_link/views.py +++ b/dojo/jira_link/views.py @@ -149,11 +149,31 @@ def webhook(request, secret=None): resolution = resolution if resolution and resolution != "None" else None resolution_id = resolution["id"] if resolution else None resolution_name = resolution["name"] if resolution else None + + # Extract the statusCategory.key from the issue status. Jira + # returns one of "new" (To Do), "indeterminate" (In Progress) + # or "done" (Done). We pass this alongside the resolution into + # process_resolution_from_jira so mitigation is only triggered + # when the issue is genuinely closed, not just when a + # resolution value happens to be present. + status = parsed["issue"]["fields"].get("status") or {} + status_category = status.get("statusCategory") or {} + status_category_key = status_category.get("key") + jira_now = parse_datetime(parsed["issue"]["fields"]["updated"]) if findings: for finding in findings: - jira_helper.process_resolution_from_jira(finding, resolution_id, resolution_name, assignee_name, jira_now, jissue, finding_group=jissue.finding_group) + jira_helper.process_resolution_from_jira( + finding, + resolution_id, + resolution_name, + assignee_name, + jira_now, + jissue, + finding_group=jissue.finding_group, + status_category_key=status_category_key, + ) # Check for any comment that could have come along with the resolution if (error_response := check_for_and_create_comment(parsed)) is not None: return error_response diff --git a/unittests/test_jira_webhook.py b/unittests/test_jira_webhook.py index 8d99a60dac9..1ede1202602 100644 --- a/unittests/test_jira_webhook.py +++ b/unittests/test_jira_webhook.py @@ -667,3 +667,124 @@ def test_webhook_issue_updated_extracts_comment(self): last_note = finding.notes.order_by("-id").first() self.assertIsNotNone(last_note) self.assertEqual(last_note.entry, "(Valentijn Scholten (valentijn)): test2") + + # ---- statusCategory guard on the mitigation path ---- + # The webhook payload's issue.fields.resolution used to be the sole + # signal for whether the Jira issue was closed, which caused several + # real-world false-mitigations: + # (a) Jira workflows where the Reopen transition does not clear + # resolution - the issue ends up in status=To-Do with a stale + # resolution value, and every webhook mis-mitigated the finding. + # (b) Ricochets during DefectDojo's own push to Jira: when a finding + # is reopened, the issue.update() and the subsequent transition + # fire separate webhooks, and the first one sees the + # pre-transition (still-resolved) state. + # The fix: a webhook only mitigates when BOTH resolution is non-null + # AND statusCategory.key == "done". These tests lock that in. + + def _update_body_with_status(self, *, status_name, status_category_key): + body = json.loads(self.jira_issue_update_template_string) + # Target the JIRA_Issue linked to finding 5 (jira_id=2 in the fixture) + body["issue"]["id"] = 2 + body["issue"]["fields"]["status"]["name"] = status_name + body["issue"]["fields"]["status"]["statusCategory"]["key"] = status_category_key + # The template already carries a non-null resolution ("Cancelled"). + # That is the interesting state: resolution is set but status may or + # may not actually be "done". + return body + + def _reset_finding_to_active(self, finding): + finding.active = True + finding.is_mitigated = False + finding.mitigated = None + finding.mitigated_by = None + finding.false_p = False + finding.risk_accepted = False + finding.save() + + def test_webhook_update_does_not_mitigate_when_status_category_is_new(self): + """ + Regression: resolution set + statusCategory 'new' must NOT mitigate. + + This is the real-world symptom seen when a Jira Reopen transition + forgets to clear the resolution field, or when DefectDojo's own + multi-step push to Jira fires a webhook against the pre-transition + state during a finding reopen. + """ + self.system_settings( + enable_jira=True, enable_jira_web_hook=True, + disable_jira_webhook_secret=False, + jira_webhook_secret=self.correct_secret, + ) + jira_issue = JIRA_Issue.objects.get(jira_id=2) + finding = jira_issue.finding + self._reset_finding_to_active(finding) + + body = self._update_body_with_status( + status_name="To Do", status_category_key="new", + ) + response = self.client.post( + reverse("jira_web_hook_secret", args=(self.correct_secret, )), + body, + content_type="application/json", + ) + self.assertEqual(200, response.status_code, response.content[:1000]) + + finding.refresh_from_db() + self.assertTrue(finding.active) + self.assertFalse(finding.is_mitigated) + self.assertIsNone(finding.mitigated) + self.assertIsNone(finding.mitigated_by) + + def test_webhook_update_does_not_mitigate_when_status_category_is_indeterminate(self): + """Same guard for issues in the 'In Progress' category.""" + self.system_settings( + enable_jira=True, enable_jira_web_hook=True, + disable_jira_webhook_secret=False, + jira_webhook_secret=self.correct_secret, + ) + jira_issue = JIRA_Issue.objects.get(jira_id=2) + finding = jira_issue.finding + self._reset_finding_to_active(finding) + + body = self._update_body_with_status( + status_name="In Progress", status_category_key="indeterminate", + ) + response = self.client.post( + reverse("jira_web_hook_secret", args=(self.correct_secret, )), + body, + content_type="application/json", + ) + self.assertEqual(200, response.status_code, response.content[:1000]) + + finding.refresh_from_db() + self.assertTrue(finding.active) + self.assertFalse(finding.is_mitigated) + + def test_webhook_update_mitigates_when_status_category_is_done(self): + """Happy path: a genuinely-closed Jira issue still mitigates its finding.""" + self.system_settings( + enable_jira=True, enable_jira_web_hook=True, + disable_jira_webhook_secret=False, + jira_webhook_secret=self.correct_secret, + ) + jira_issue = JIRA_Issue.objects.get(jira_id=2) + finding = jira_issue.finding + self._reset_finding_to_active(finding) + + body = self._update_body_with_status( + status_name="Done", status_category_key="done", + ) + response = self.client.post( + reverse("jira_web_hook_secret", args=(self.correct_secret, )), + body, + content_type="application/json", + ) + self.assertEqual(200, response.status_code, response.content[:1000]) + + finding.refresh_from_db() + self.assertFalse(finding.active) + self.assertTrue(finding.is_mitigated) + self.assertIsNotNone(finding.mitigated) + self.assertIsNotNone(finding.mitigated_by) + self.assertEqual(finding.mitigated_by.username, "JIRA") From c05ce7003f49e89bcd1e557da4b9bc9c34a785eb Mon Sep 17 00:00:00 2001 From: Paul Osinski <42211303+paulOsinski@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:42:00 -0400 Subject: [PATCH 2/3] Update dojo/jira_link/helper.py Co-authored-by: valentijnscholten --- dojo/jira_link/helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dojo/jira_link/helper.py b/dojo/jira_link/helper.py index 76c2208185e..6827bc3fc3f 100644 --- a/dojo/jira_link/helper.py +++ b/dojo/jira_link/helper.py @@ -1936,7 +1936,7 @@ def process_resolution_from_jira( if status_category_key is None: resolved = resolution_id is not None else: - resolved = resolution_id is not None and status_category_key == "done" + resolved = status_category_key == "done" jira_instance = get_jira_instance(finding) if resolved: From 9af22c9a9e1734791afb03b84186491157c63f38 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Wed, 22 Apr 2026 18:38:54 -0400 Subject: [PATCH 3/3] fix ruff --- dojo/jira_link/helper.py | 27 +++++++++---------- .../commands/jira_status_reconciliation.py | 21 ++++++++++++++- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/dojo/jira_link/helper.py b/dojo/jira_link/helper.py index 6827bc3fc3f..e1e874c3205 100644 --- a/dojo/jira_link/helper.py +++ b/dojo/jira_link/helper.py @@ -1911,12 +1911,14 @@ def process_resolution_from_jira( """Processes the resolution field in the JIRA issue and updated the finding in Defect Dojo accordingly""" import dojo.risk_acceptance.helper as ra_helper # noqa: PLC0415 import error status_changed = False - # A Jira issue is treated as "resolved" (i.e. the linked finding should be - # mitigated / risk-accepted / false-positive'd) only when BOTH a non-null - # resolution is present AND the issue's statusCategory is "done". Jira's - # statusCategory.key is the canonical closure signal (always one of - # "new", "indeterminate", "done"), and checking it alongside the - # resolution field prevents several real-world bugs: + # A Jira issue is treated as "resolved" (the linked finding should be + # mitigated / risk-accepted / false-positive'd) when the issue's + # statusCategory.key is "done". Jira's statusCategory is the canonical + # closure signal (one of "new", "indeterminate", "done", "undefined"), + # always returned by the API, and does not depend on per-workflow + # resolution-field hygiene. + # + # Relying on statusCategory prevents real-world bugs: # 1. Workflows where reopen transitions do not clear the resolution - # the issue ends up in an "open but resolved" state and every # webhook event for the issue would otherwise mis-mitigate the @@ -1929,14 +1931,11 @@ def process_resolution_from_jira( # and the first webhook sees the pre-transition (still-resolved) # state even though the intended final state is reopened. # - # status_category_key is keyword-only and optional to preserve backward - # compatibility with any caller that has not yet been updated to extract - # it from the webhook payload; when it is None we fall back to the - # historical resolution-only behavior. - if status_category_key is None: - resolved = resolution_id is not None - else: - resolved = status_category_key == "done" + # The resolution value (when present) is still used downstream to + # classify "done" issues as risk-accepted, false-positive, or the + # default mitigated category (see jira_instance.accepted_resolutions + # and .false_positive_resolutions below). + resolved = status_category_key == "done" jira_instance = get_jira_instance(finding) if resolved: diff --git a/dojo/management/commands/jira_status_reconciliation.py b/dojo/management/commands/jira_status_reconciliation.py index 2b8a649ed9f..215c0fb7a33 100644 --- a/dojo/management/commands/jira_status_reconciliation.py +++ b/dojo/management/commands/jira_status_reconciliation.py @@ -102,6 +102,13 @@ def _reconcile_findings(mode, product_obj, engagement_obj, timestamp, dryrun, me resolution = issue_from_jira.fields.resolution if issue_from_jira.fields.resolution and issue_from_jira.fields.resolution != "None" else None resolution_id = resolution.id if resolution else None resolution_name = resolution.name if resolution else None + # statusCategory.key is the canonical "is this issue done" signal + # that process_resolution_from_jira uses to decide whether to mitigate. + status_category_key = getattr( + getattr(getattr(issue_from_jira.fields, "status", None), "statusCategory", None), + "key", + None, + ) # convert from str to datetime issue_from_jira.fields.updated = parse_datetime(issue_from_jira.fields.updated) @@ -175,7 +182,11 @@ def _reconcile_findings(mode, product_obj, engagement_obj, timestamp, dryrun, me if action == "import_status_from_jira": message_action = "deactivating" if find.active else "reactivating" - status_changed = jira_helper.process_resolution_from_jira(find, resolution_id, resolution_name, assignee_name, issue_from_jira.fields.updated, find.jira_issue) if not dryrun else "dryrun" + status_changed = jira_helper.process_resolution_from_jira( + find, resolution_id, resolution_name, assignee_name, + issue_from_jira.fields.updated, find.jira_issue, + status_category_key=status_category_key, + ) if not dryrun else "dryrun" if status_changed: message = f"{find.jira_issue.jira_key}; {settings.SITE_URL}/finding/{find.id};{find.status()};{resolution_name};{flag1};{flag2};{flag3};{find.jira_issue.jira_change};{issue_from_jira.fields.updated};{find.last_status_update};{issue_from_jira.fields.updated};{find.last_reviewed};{issue_from_jira.fields.updated};{message_action} finding in defectdojo;{status_changed}" messages.append(message) @@ -264,6 +275,13 @@ def _reconcile_finding_groups(mode, product_obj, engagement_obj, timestamp, dryr resolution = issue_from_jira.fields.resolution if issue_from_jira.fields.resolution and issue_from_jira.fields.resolution != "None" else None resolution_id = resolution.id if resolution else None resolution_name = resolution.name if resolution else None + # statusCategory.key is the canonical "is this issue done" signal + # that process_resolution_from_jira uses to decide whether to mitigate. + status_category_key = getattr( + getattr(getattr(issue_from_jira.fields, "status", None), "statusCategory", None), + "key", + None, + ) # convert from str to datetime issue_from_jira.fields.updated = parse_datetime(issue_from_jira.fields.updated) @@ -335,6 +353,7 @@ def _reconcile_finding_groups(mode, product_obj, engagement_obj, timestamp, dryr find, resolution_id, resolution_name, assignee_name, issue_from_jira.fields.updated, finding_group.jira_issue, finding_group=finding_group, + status_category_key=status_category_key, ) else: status_changed = "dryrun"