Skip to content

Commit 70533a5

Browse files
stevewalloneclaude
andauthored
fix: guard legacy endpoint access in mktemplate and merge under V3_FEATURE_LOCATIONS (#15139)
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 <noreply@anthropic.com>
1 parent 442695b commit 70533a5

2 files changed

Lines changed: 149 additions & 4 deletions

File tree

dojo/finding/views.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
IMPORT_UNTOUCHED_FINDING,
7777
BurpRawRequestResponse,
7878
Dojo_User,
79+
Endpoint,
7980
Endpoint_Status,
8081
Engagement,
8182
FileAccessToken,
@@ -1747,7 +1748,8 @@ def mktemplate(request, fid):
17471748

17481749
# Copy endpoints if they exist
17491750
if finding.endpoints.exists():
1750-
endpoint_urls = [str(ep) for ep in finding.endpoints.all()]
1751+
with Endpoint.allow_endpoint_init(): # TODO: Delete this after the move to Locations
1752+
endpoint_urls = [str(ep) for ep in finding.endpoints.all()]
17511753
finding_helper.save_endpoints_template(template, endpoint_urls)
17521754

17531755
messages.add_message(
@@ -2333,9 +2335,10 @@ def merge_finding_product(request, pid):
23332335

23342336
# if checked merge the endpoints
23352337
if form.cleaned_data["add_endpoints"]:
2336-
finding_to_merge_into.endpoints.add(
2337-
*finding.endpoints.all(),
2338-
)
2338+
with Endpoint.allow_endpoint_init(): # TODO: Delete this after the move to Locations
2339+
finding_to_merge_into.endpoints.add(
2340+
*finding.endpoints.all(),
2341+
)
23392342

23402343
# if checked merge the tags
23412344
if form.cleaned_data["tag_finding"]:
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
"""
2+
Regression tests for issue #15123: the V3_FEATURE_LOCATIONS crash when creating a
3+
Finding Template from a finding or merging findings.
4+
5+
When ``V3_FEATURE_LOCATIONS`` is enabled the legacy ``Endpoint`` model is deprecated and
6+
``Endpoint.__init__`` raises ``NotImplementedError``. Findings created before the V3
7+
migration still carry legacy endpoint rows (the migration keeps them as backup), so any
8+
un-guarded iteration of ``finding.endpoints.all()`` hydrates ``Endpoint`` instances via
9+
``Model.from_db()`` and produces a 500. Two such sites live in ``dojo/finding/views.py``:
10+
``mktemplate`` (copying endpoint URLs onto the template) and ``merge_finding_product``
11+
(adding the merged findings' endpoints onto the target finding).
12+
13+
The fix wraps both sites in ``Endpoint.allow_endpoint_init()``.
14+
"""
15+
import logging
16+
from types import SimpleNamespace
17+
18+
from django.contrib.auth.models import User
19+
from django.test import Client, override_settings
20+
from django.urls import reverse
21+
from django.utils import timezone
22+
23+
from dojo.models import (
24+
Endpoint,
25+
Endpoint_Status,
26+
Engagement,
27+
Finding,
28+
Finding_Template,
29+
Product,
30+
Product_Type,
31+
Test,
32+
Test_Type,
33+
UserContactInfo,
34+
)
35+
36+
from .dojo_test_case import DojoTestCase
37+
38+
logger = logging.getLogger(__name__)
39+
40+
41+
@override_settings(V3_FEATURE_LOCATIONS=True)
42+
class TestFindingTemplateAndMergeWithEndpointsV3(DojoTestCase):
43+
44+
"""
45+
Creating a finding template and merging findings must not 500 when
46+
``V3_FEATURE_LOCATIONS`` is enabled and the findings still carry legacy
47+
``Endpoint`` rows.
48+
"""
49+
50+
def setUp(self):
51+
super().setUp()
52+
53+
self.admin = User.objects.create(
54+
username="test_template_merge_endpoints_v3_admin",
55+
is_staff=True,
56+
is_superuser=True,
57+
)
58+
UserContactInfo.objects.create(user=self.admin, block_execution=True)
59+
60+
self.ui_client = Client()
61+
self.ui_client.force_login(self.admin)
62+
63+
self.system_settings(enable_jira=False)
64+
self.system_settings(enable_github=False)
65+
66+
self.test_type = Test_Type.objects.get_or_create(name="Manual Test")[0]
67+
68+
product_type = Product_Type.objects.create(name="Org for template/merge endpoints")
69+
self.product = Product.objects.create(
70+
name="Product for template/merge endpoints",
71+
description="regression fixture",
72+
prod_type=product_type,
73+
)
74+
engagement = Engagement.objects.create(
75+
name="Engagement for template/merge endpoints",
76+
product=self.product,
77+
target_start=timezone.now(),
78+
target_end=timezone.now(),
79+
)
80+
self.test = Test.objects.create(
81+
engagement=engagement,
82+
test_type=self.test_type,
83+
target_start=timezone.now(),
84+
target_end=timezone.now(),
85+
)
86+
87+
def _create_finding_with_legacy_endpoint(self, suffix):
88+
"""Create a finding carrying a legacy Endpoint row, as pre-V3 findings do."""
89+
finding = Finding.objects.create(
90+
test=self.test,
91+
title=f"Finding with legacy endpoint {suffix}",
92+
severity="High",
93+
description="regression fixture",
94+
mitigation="n/a",
95+
impact="n/a",
96+
reporter=self.admin,
97+
)
98+
# The Endpoint model is deprecated under V3; constructing/saving it in the
99+
# fixture requires the escape hatch. Linking it through Endpoint_Status
100+
# populates the legacy finding.endpoints m2m.
101+
with Endpoint.allow_endpoint_init():
102+
endpoint = Endpoint(
103+
product=self.product,
104+
protocol="https",
105+
host=f"host-{suffix}.example.com",
106+
)
107+
endpoint.save()
108+
endpoint_status = Endpoint_Status(endpoint=endpoint, finding=finding)
109+
endpoint_status.save()
110+
111+
return SimpleNamespace(finding=finding, endpoint=endpoint)
112+
113+
def test_mktemplate_with_legacy_endpoints(self):
114+
"""Creating a finding template from a finding with legacy endpoints must not crash."""
115+
fixture = self._create_finding_with_legacy_endpoint("mktemplate")
116+
response = self.ui_client.get(
117+
reverse("mktemplate", kwargs={"fid": fixture.finding.id}),
118+
)
119+
self.assertEqual(response.status_code, 302)
120+
template = Finding_Template.objects.get(title=fixture.finding.title)
121+
self.assertIn("host-mktemplate.example.com", template.endpoints_text)
122+
123+
def test_merge_findings_with_legacy_endpoints(self):
124+
"""Merging a finding that carries legacy endpoints must not crash."""
125+
target = self._create_finding_with_legacy_endpoint("merge-target")
126+
source = self._create_finding_with_legacy_endpoint("merge-source")
127+
url = reverse("merge_finding_product", kwargs={"pid": self.product.id})
128+
response = self.ui_client.post(
129+
f"{url}?finding_to_update={target.finding.id}&finding_to_update={source.finding.id}",
130+
{
131+
"finding_to_merge_into": target.finding.id,
132+
"findings_to_merge": [source.finding.id],
133+
"append_description": "on",
134+
"add_endpoints": "on",
135+
"finding_action": "inactive",
136+
},
137+
)
138+
self.assertEqual(response.status_code, 302)
139+
self.assertEqual(target.finding.endpoints.count(), 2)
140+
self.assertTrue(
141+
target.finding.endpoints.filter(pk=source.endpoint.pk).exists(),
142+
)

0 commit comments

Comments
 (0)