Skip to content

Commit b84ff1f

Browse files
authored
Merge pull request #15097 from valentijnscholten/fix/finding-tag-inheritance-overwrite
fix(tags): keep user-set tags when creating a finding under product tag inheritance
2 parents 8bc05ce + 7958948 commit b84ff1f

2 files changed

Lines changed: 68 additions & 0 deletions

File tree

dojo/tags/signals.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from django.db.models import signals
55
from django.dispatch import receiver
6+
from tagulous.models.fields import SingleTagField, TagField
67

78
from dojo.celery_dispatch import dojo_dispatch_task
89
from dojo.location.models import Location, LocationFindingReference, LocationProductReference
@@ -17,6 +18,31 @@
1718
logger = logging.getLogger(__name__)
1819

1920

21+
def _flush_pending_tag_fields(instance):
22+
"""
23+
Persist any tags assigned to ``instance`` *before* it was first saved.
24+
25+
Tagulous holds tags set on an unsaved instance (``finding.tags = [...]``
26+
followed by ``finding.save()``) in an in-memory manager and only writes
27+
them to the through table from its own ``post_save`` handler. That handler
28+
is registered by ``tagulous``' AppConfig.ready(), which runs *after*
29+
``dojo``'s (``dojo`` precedes ``tagulous`` in ``INSTALLED_APPS``), so the
30+
inheritance ``post_save`` handler below fires first. When it reads
31+
``instance.tags`` on the freshly-saved row it loads an empty, DB-backed
32+
manager and the pending pre-save tags are lost — the user's tags silently
33+
disappear on finding creation when product tag inheritance is enabled
34+
(#15092).
35+
36+
Flushing here makes the inheritance path independent of signal-handler
37+
ordering: the instance's own tags are committed first, then inheritance
38+
merges the product tags on top.
39+
"""
40+
for field in instance._meta.get_fields():
41+
if isinstance(field, SingleTagField | TagField):
42+
descriptor = getattr(type(instance), field.name)
43+
descriptor.get_manager(instance).post_save_handler()
44+
45+
2046
def auto_inherit_product_tags(instance):
2147
"""
2248
Apply product-inherited tags to ``instance`` from the auto-inheritance
@@ -32,6 +58,9 @@ def auto_inherit_product_tags(instance):
3258
products = get_products_to_inherit_tags_from(instance)
3359
if not products:
3460
return
61+
# Commit the instance's own (possibly pre-save assigned) tags before we
62+
# read/merge them, so inheritance never clobbers them — see #15092.
63+
_flush_pending_tag_fields(instance)
3564
incoming_inherited_tags = [tag.name for product in products for tag in product.tags.all()]
3665
_sync_inherited_tags(instance, incoming_inherited_tags)
3766

unittests/test_tag_inheritance.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,45 @@ def test_mixed_products_only_flagged_product_propagates(self):
307307
self.assertEqual(sorted(t.name for t in eng_with.tags.all()), ["inherit-me"])
308308
self.assertEqual(list(eng_without.tags.all()), [])
309309

310+
def test_finding_created_with_own_tag_keeps_it_and_inherits(self):
311+
"""
312+
Regression test for #15092.
313+
314+
Assigning tags to an unsaved finding and then saving it (the UI
315+
"add finding" flow: ``finding.tags = [...]; finding.save()``) must keep
316+
the user's own tags *and* merge the inherited product tags. Previously
317+
the inheritance post_save handler ran before tagulous persisted the
318+
pre-save tags and silently dropped them.
319+
"""
320+
product = self.create_product("Inherit On Create", tags=["product-tag"])
321+
product.enable_product_tag_inheritance = True
322+
product.save()
323+
engagement = self.create_engagement("Eng", product)
324+
test = self.create_test(engagement=engagement, scan_type="ZAP Scan")
325+
326+
finding = Finding(test=test, title="With own tag", severity="Medium", reporter=self.get_test_admin())
327+
finding.tags = ["my-own-tag"]
328+
finding.save()
329+
finding.refresh_from_db()
330+
331+
self.assertEqual(sorted(t.name for t in finding.tags.all()), ["my-own-tag", "product-tag"])
332+
self.assertEqual([t.name for t in finding.inherited_tags.all()], ["product-tag"])
333+
334+
def test_finding_created_without_own_tag_still_inherits(self):
335+
"""A finding created with no tags must still receive the product's inherited tags (#15092 guard)."""
336+
product = self.create_product("Inherit No Own Tag", tags=["product-tag"])
337+
product.enable_product_tag_inheritance = True
338+
product.save()
339+
engagement = self.create_engagement("Eng", product)
340+
test = self.create_test(engagement=engagement, scan_type="ZAP Scan")
341+
342+
finding = Finding(test=test, title="No own tag", severity="Medium", reporter=self.get_test_admin())
343+
finding.save()
344+
finding.refresh_from_db()
345+
346+
self.assertEqual([t.name for t in finding.tags.all()], ["product-tag"])
347+
self.assertEqual([t.name for t in finding.inherited_tags.all()], ["product-tag"])
348+
310349

311350
# ---------------------------------------------------------------------------
312351
# Integration tests — endpoint inheritance (v2 only)

0 commit comments

Comments
 (0)