fix(tags): order single-finding delete tag-count decrements so concurrent deletes cannot deadlock

[Maffooch]

Description

A single-object finding delete — DELETE /api/v2/findings/{id}/ → Finding.delete() → super().delete() — surfaced a Postgres deadlock to the client:

django.db.utils.OperationalError: deadlock detected
DETAIL:  Process A waits for ShareLock on transaction N; blocked by process B.
         Process B waits for ShareLock on transaction M; blocked by process A.
CONTEXT:  while updating tuple (x,y) in relation "dojo_tagulous_finding_tags"

Root cause. dojo_tagulous_finding_tags is the tagulous tag table (it holds count), so the deadlocked statement is the per-tag count = count - 1 UPDATE, which takes a row lock on each tag. The synchronous single-object delete does not clear tags itself; it relies on super().delete(), whose pre_delete fires tagulous's per-object clear(). That clear() issues one count UPDATE per tag in the manager's own iteration order, not in ascending tag-id order. Because the UPDATE order is the lock order, two concurrent single-finding deletes whose tag sets overlap can take the same tag-row locks in opposite orders, forming a cycle Postgres breaks by aborting one side (SQLSTATE 40P01).

This is the single-object twin of two deadlocks already fixed for the other tag-count paths:

• fix(tags): order tag-count decrements so concurrent removals cannot deadlock #15486 made the bulk/cascade remove path (bulk_remove_all_tags) decrement counts in a deterministic ascending tag-id order, and added an async_delete_task retry backstop.
• fix(tags): order add-path tag-count updates so concurrent imports cannot deadlock #15652 gave the import add path the same ascending tag-id ordering.

The synchronous Finding.delete() path — used by the REST destroy() in both open source and the Pro override, which calls super().destroy() → instance.delete() — was never routed through that shared ordering, so it can still deadlock against a concurrent single delete, bulk delete, or import.

Fix. Finding.delete() now removes the finding's tags via bulk_remove_all_tags(Finding, Finding.objects.filter(pk=self.pk)) before super().delete(). This issues the count decrements in the same ascending tag-id order every other tag-count path already uses, so all callers acquire the tag-row locks in one shared order and the cycle cannot form. Clearing the through rows first also leaves the tagulous pre_delete handler nothing to decrement, so tag counts are not double-processed. Behavior is now consistent with the bulk cascade delete, which is the dominant deletion path.

No migration. This is a pure ordering change to an existing code path — no model or schema change. A migration was neither generated nor needed; the fix reuses the already-shipped bulk_remove_all_tags helper.

Downstream (Pro) impact. DefectDojo Pro's FindingViewSet.destroy() overrides the OSS one but delegates to super().destroy(), which calls the model-level Finding.delete() — so the fix covers the Pro delete path with no Pro-side change. Pro's Finding pre_delete receiver only dispatches integrator notifications and does not touch tag counts. Pro does not subclass Finding or override Finding.delete().

Test results

Added FindingDeleteTagLockOrderTest in unittests/test_tag_utils_bulk.py, mirroring the existing BulkRemoveAllTagsLockOrderTest (#15486) and BulkAddTagsToInstancesLockOrderTest (#15652). Three tags are attached to a finding in an order unrelated to their ids; the test captures the tag rows locked by the count UPDATEs during finding.delete() and asserts they are issued in ascending tag-id order. It fails on the old per-object-clear() order (name order, [2,3,1]) and passes once the delete is routed through bulk_remove_all_tags ([1,2,3]).

The test is deterministic — it asserts the update/lock ordering, not a live race. As with #15652, a full Postgres-backed suite run could not be stood up in the ephemeral environment used to author this change; validation is being carried through CI.

ruff check clean on both changed files (0.16.0, repo ruff.toml).

Documentation

No documentation change: no user-visible behavior change and no new setting.

Checklist

• Bugfix submitted against the bugfix branch.
• Meaningful PR name.
• No model changes / no migration.
• Ruff compliant.
• Unit test added covering the ordering guarantee.

---

Generated by Claude Code

[dryrunsecurity]

This pull request contains a critical finding where the sensitive file 'dojo/finding/models.py' was modified by an unauthorized author, 'claude'.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/models.py (drs_5b02d002)

Vulnerability	Configured Sensitive Codepath Modified by Non-Allowed Author
Description	File 'dojo/finding/models.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'claude' (commit 3a48e0c) who is not in the allowed authors list.

We've notified @mtesauro.

---

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.