From 7ccd6036dcedb6197ea1a2bdfe950fb8c63db4a1 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Tue, 21 Apr 2026 16:06:30 -0400 Subject: [PATCH 1/5] fix jira bug --- dojo/jira/helper.py | 63 ++++++++++++++--- dojo/jira/views.py | 22 +++++- unittests/test_jira_webhook.py | 121 +++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 10 deletions(-) diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index feff72003ef..76c2208185e 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/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/views.py b/dojo/jira/views.py index 908393d10ca..eb79c5aa670 100644 --- a/dojo/jira/views.py +++ b/dojo/jira/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 098eada180b..a63e35f07aa 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 9cd9f2d3dafc280ec4bb3efa088a9e5db08d24cf 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/5] Update dojo/jira_link/helper.py Co-authored-by: valentijnscholten --- dojo/jira/helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index 76c2208185e..6827bc3fc3f 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/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 08b9a60863c750bf73c7580e11cc7374632a77a1 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Wed, 22 Apr 2026 18:38:54 -0400 Subject: [PATCH 3/5] fix ruff --- dojo/jira/helper.py | 27 +++++++++---------- .../commands/jira_status_reconciliation.py | 21 ++++++++++++++- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index 6827bc3fc3f..e1e874c3205 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/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 57a7e79d43d..3bcdc20665a 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" From 62cc0fdb8f4aa53779c3264ca6ddac301fb36053 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 29 Apr 2026 10:29:32 -0600 Subject: [PATCH 4/5] chore: forward status_category_key through jira services wrapper --- dojo/jira/services.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dojo/jira/services.py b/dojo/jira/services.py index 0a6a2aa9149..54dec073be1 100644 --- a/dojo/jira/services.py +++ b/dojo/jira/services.py @@ -167,7 +167,8 @@ def process_epic_form(request, engagement=None): def process_resolution_from_jira(finding, resolution_id, resolution_name, assignee_name, jira_now, jira_issue, - finding_group=None): + finding_group=None, *, + status_category_key: str | None = None): """ Process a resolution change from Jira webhook. @@ -177,6 +178,7 @@ def process_resolution_from_jira(finding, resolution_id, resolution_name, finding, resolution_id, resolution_name, assignee_name, jira_now, jira_issue, finding_group=finding_group, + status_category_key=status_category_key, ) From 4a6cbb8c9b4f35ca298c160c9e65c26deeee4ec4 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Wed, 29 Apr 2026 13:13:29 -0400 Subject: [PATCH 5/5] chore(helm): add artifacthub.io/changes entry for Jira webhook fix Lint chart (version) job requires the annotation to differ from the target branch when chart files change. Branch drift bumped Chart.yaml versions, triggering the check. Co-Authored-By: Claude Opus 4.7 (1M context) --- helm/defectdojo/Chart.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/helm/defectdojo/Chart.yaml b/helm/defectdojo/Chart.yaml index 8511d608d13..2a904c53a90 100644 --- a/helm/defectdojo/Chart.yaml +++ b/helm/defectdojo/Chart.yaml @@ -34,4 +34,6 @@ dependencies: # description: Critical bug annotations: artifacthub.io/prerelease: "true" - artifacthub.io/changes: "" + artifacthub.io/changes: | + - kind: fixed + description: 'Jira webhook: stop mis-mitigating findings on non-"done" issues'