From 7f242e732bdfcfa90e0b089b6a3af59200461484 Mon Sep 17 00:00:00 2001 From: Steve Wall Date: Wed, 1 Jul 2026 14:56:46 -0600 Subject: [PATCH] fix: guard legacy endpoint access in mktemplate and merge under V3_FEATURE_LOCATIONS Creating a Finding Template from a finding or merging findings crashed with NotImplementedError when V3_FEATURE_LOCATIONS is enabled and the finding still carries legacy Endpoint rows (kept as backup by the locations migration). Both sites in dojo/finding/views.py iterated finding.endpoints.all() without the Endpoint.allow_endpoint_init() escape hatch used by the other legacy endpoint call sites. Fixes #15123. Co-Authored-By: Claude Fable 5 --- dojo/finding/views.py | 11 +- ...est_finding_template_merge_endpoints_v3.py | 142 ++++++++++++++++++ 2 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 unittests/test_finding_template_merge_endpoints_v3.py diff --git a/dojo/finding/views.py b/dojo/finding/views.py index e39a0a8fea8..fae3b0a6c68 100644 --- a/dojo/finding/views.py +++ b/dojo/finding/views.py @@ -76,6 +76,7 @@ IMPORT_UNTOUCHED_FINDING, BurpRawRequestResponse, Dojo_User, + Endpoint, Endpoint_Status, Engagement, FileAccessToken, @@ -1747,7 +1748,8 @@ def mktemplate(request, fid): # Copy endpoints if they exist if finding.endpoints.exists(): - endpoint_urls = [str(ep) for ep in finding.endpoints.all()] + with Endpoint.allow_endpoint_init(): # TODO: Delete this after the move to Locations + endpoint_urls = [str(ep) for ep in finding.endpoints.all()] finding_helper.save_endpoints_template(template, endpoint_urls) messages.add_message( @@ -2333,9 +2335,10 @@ def merge_finding_product(request, pid): # if checked merge the endpoints if form.cleaned_data["add_endpoints"]: - finding_to_merge_into.endpoints.add( - *finding.endpoints.all(), - ) + with Endpoint.allow_endpoint_init(): # TODO: Delete this after the move to Locations + finding_to_merge_into.endpoints.add( + *finding.endpoints.all(), + ) # if checked merge the tags if form.cleaned_data["tag_finding"]: diff --git a/unittests/test_finding_template_merge_endpoints_v3.py b/unittests/test_finding_template_merge_endpoints_v3.py new file mode 100644 index 00000000000..fd5c853d997 --- /dev/null +++ b/unittests/test_finding_template_merge_endpoints_v3.py @@ -0,0 +1,142 @@ +""" +Regression tests for issue #15123: the V3_FEATURE_LOCATIONS crash when creating a +Finding Template from a finding or merging findings. + +When ``V3_FEATURE_LOCATIONS`` is enabled the legacy ``Endpoint`` model is deprecated and +``Endpoint.__init__`` raises ``NotImplementedError``. Findings created before the V3 +migration still carry legacy endpoint rows (the migration keeps them as backup), so any +un-guarded iteration of ``finding.endpoints.all()`` hydrates ``Endpoint`` instances via +``Model.from_db()`` and produces a 500. Two such sites live in ``dojo/finding/views.py``: +``mktemplate`` (copying endpoint URLs onto the template) and ``merge_finding_product`` +(adding the merged findings' endpoints onto the target finding). + +The fix wraps both sites in ``Endpoint.allow_endpoint_init()``. +""" +import logging +from types import SimpleNamespace + +from django.contrib.auth.models import User +from django.test import Client, override_settings +from django.urls import reverse +from django.utils import timezone + +from dojo.models import ( + Endpoint, + Endpoint_Status, + Engagement, + Finding, + Finding_Template, + Product, + Product_Type, + Test, + Test_Type, + UserContactInfo, +) + +from .dojo_test_case import DojoTestCase + +logger = logging.getLogger(__name__) + + +@override_settings(V3_FEATURE_LOCATIONS=True) +class TestFindingTemplateAndMergeWithEndpointsV3(DojoTestCase): + + """ + Creating a finding template and merging findings must not 500 when + ``V3_FEATURE_LOCATIONS`` is enabled and the findings still carry legacy + ``Endpoint`` rows. + """ + + def setUp(self): + super().setUp() + + self.admin = User.objects.create( + username="test_template_merge_endpoints_v3_admin", + is_staff=True, + is_superuser=True, + ) + UserContactInfo.objects.create(user=self.admin, block_execution=True) + + self.ui_client = Client() + self.ui_client.force_login(self.admin) + + self.system_settings(enable_jira=False) + self.system_settings(enable_github=False) + + self.test_type = Test_Type.objects.get_or_create(name="Manual Test")[0] + + product_type = Product_Type.objects.create(name="Org for template/merge endpoints") + self.product = Product.objects.create( + name="Product for template/merge endpoints", + description="regression fixture", + prod_type=product_type, + ) + engagement = Engagement.objects.create( + name="Engagement for template/merge endpoints", + product=self.product, + target_start=timezone.now(), + target_end=timezone.now(), + ) + self.test = Test.objects.create( + engagement=engagement, + test_type=self.test_type, + target_start=timezone.now(), + target_end=timezone.now(), + ) + + def _create_finding_with_legacy_endpoint(self, suffix): + """Create a finding carrying a legacy Endpoint row, as pre-V3 findings do.""" + finding = Finding.objects.create( + test=self.test, + title=f"Finding with legacy endpoint {suffix}", + severity="High", + description="regression fixture", + mitigation="n/a", + impact="n/a", + reporter=self.admin, + ) + # The Endpoint model is deprecated under V3; constructing/saving it in the + # fixture requires the escape hatch. Linking it through Endpoint_Status + # populates the legacy finding.endpoints m2m. + with Endpoint.allow_endpoint_init(): + endpoint = Endpoint( + product=self.product, + protocol="https", + host=f"host-{suffix}.example.com", + ) + endpoint.save() + endpoint_status = Endpoint_Status(endpoint=endpoint, finding=finding) + endpoint_status.save() + + return SimpleNamespace(finding=finding, endpoint=endpoint) + + def test_mktemplate_with_legacy_endpoints(self): + """Creating a finding template from a finding with legacy endpoints must not crash.""" + fixture = self._create_finding_with_legacy_endpoint("mktemplate") + response = self.ui_client.get( + reverse("mktemplate", kwargs={"fid": fixture.finding.id}), + ) + self.assertEqual(response.status_code, 302) + template = Finding_Template.objects.get(title=fixture.finding.title) + self.assertIn("host-mktemplate.example.com", template.endpoints_text) + + def test_merge_findings_with_legacy_endpoints(self): + """Merging a finding that carries legacy endpoints must not crash.""" + target = self._create_finding_with_legacy_endpoint("merge-target") + source = self._create_finding_with_legacy_endpoint("merge-source") + url = reverse("merge_finding_product", kwargs={"pid": self.product.id}) + response = self.ui_client.post( + f"{url}?finding_to_update={target.finding.id}&finding_to_update={source.finding.id}", + { + "finding_to_merge_into": target.finding.id, + "findings_to_merge": [source.finding.id], + "append_description": "on", + "add_endpoints": "on", + "finding_action": "inactive", + }, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(target.finding.endpoints.count(), 2) + self.assertTrue( + target.finding.endpoints.filter(pk=source.endpoint.pk).exists(), + )