Skip to content

Commit 2976c43

Browse files
Maffoochclaude
andcommitted
feat(vuln-id): flag-gated entity read seam, wire-compatible
Reads switch to the VulnerabilityId entity/reference store when V3_FEATURE_VULNERABILITY_IDS is on (default True); off keeps today's legacy-table reads. The flag is consulted ONLY in dojo/vulnerability_id/queries.py and the two Finding methods. Writes stay unconditionally dual (WP3), so both stores hold identical data and the flip is reversible with no drift. queries.py seam: use_entity_reads, finding_ids_with_vulnerability_ids (in/exact), vulnerability_id_prefetch (with nested prefix), first_vulnerability_id_subquery (order==0), finding_vulnerability_id_strings. Re-pointed (identical output flag-off; entity read flag-on): - Finding.vulnerability_ids property + get_vulnerability_ids() (cve prepend / sorted-join kept byte-for-byte). - dojo/filters.py both filter fns; risk_acceptance exact-match mixin. - The 7 prefetch sites (deduplication, commands/dedupe, finding/ui + test/ui views, finding/queries x2) -> vulnerability_id_prefetch(). - v2 serializer: VulnerabilityIdsField reads [{"vulnerability_id": s}] from the flag-appropriate store (schema pinned via extend_schema_field); create/update pop `parsed_vulnerability_ids` and still funnel writes to save_vulnerability_ids. - Finding.copy() now dual-writes copied ids through persist_for_finding (was a legacy-only .objects.create — would have been invisible flag-on). Deliberately NOT re-pointed this PR (kept on the dual-written legacy table, which stays correct in both flag states; entity migration deferred to the legacy-drop phase): watson registration + the classic OSS search view. Rationale: watson is disabled in the Pro runtime (untestable locally), the classic search view/template are coupled to watson indexing Vulnerability_Id, and the classic UI is being retired. This also removes the buildwatson-on-flip requirement. The Pro Vue global search IS re-pointed to the entity in the Pro PR. Tests (fixture-free, run locally + CI): test_vulnerability_id.TestReadSeamFlagParity asserts the v2 wire shape, hash input (get_vulnerability_ids), the property, and the filter seam are byte-identical flag-off vs flag-on; test_importers_importer reconcile seeds via the dual-write seam so entity reads resolve. Verified green in BOTH flag states locally; the fixture-based suites (rest_framework schema, import_reimport, deduplication_logic, finding_helper API, watson trio) run in OSS CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7317260 commit 2976c43

13 files changed

Lines changed: 260 additions & 63 deletions

File tree

dojo/apps.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ def register_watson_models(app_config):
113113
watson.register(app_config.get_model("Location"))
114114
watson.register(app_config.get_model("Engagement"), fields=get_model_fields_with_extra(app_config.get_model("Engagement"), ("id", "product__name")), store=("product__name", ))
115115
watson.register(app_config.get_model("App_Analysis"))
116+
# Watson stays on the legacy Vulnerability_Id table (unconditionally dual-written), so global
117+
# search keeps working identically in both flag states with no buildwatson reindex. Moving the
118+
# watson index (and the classic search view) to FindingVulnerabilityReference is a legacy-drop
119+
# phase concern, not this reads-only flag flip.
116120
watson.register(app_config.get_model("Vulnerability_Id"), store=("finding__test__engagement__product__name", ))
117121

118122
# YourModel = app_config.get_model("YourModel")

dojo/filters.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@
5252
Product,
5353
Product_Type,
5454
Test,
55-
Vulnerability_Id,
5655
)
5756
from dojo.product_type.queries import get_authorized_product_types
5857
from dojo.utils import get_system_setting, is_finding_groups_enabled, truncate_timezone_aware
58+
from dojo.vulnerability_id.queries import finding_ids_with_vulnerability_ids
5959

6060
logger = logging.getLogger(__name__)
6161

@@ -73,17 +73,11 @@ def custom_filter(queryset, name, value):
7373

7474
def custom_vulnerability_id_filter(queryset, name, value):
7575
values = value.split(",")
76-
ids = Vulnerability_Id.objects \
77-
.filter(vulnerability_id__in=values) \
78-
.values_list("finding_id", flat=True)
79-
return queryset.filter(id__in=ids)
76+
return queryset.filter(id__in=finding_ids_with_vulnerability_ids(values, lookup="in"))
8077

8178

8279
def vulnerability_id_filter(queryset, name, value):
83-
ids = Vulnerability_Id.objects \
84-
.filter(vulnerability_id=value) \
85-
.values_list("finding_id", flat=True)
86-
return queryset.filter(id__in=ids)
80+
return queryset.filter(id__in=finding_ids_with_vulnerability_ids(value, lookup="exact"))
8781

8882

8983
class NumberInFilter(filters.BaseInFilter, filters.NumberFilter):

dojo/finding/api/serializer.py

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,51 @@ class Meta:
301301
fields = ["vulnerability_id"]
302302

303303

304+
@extend_schema_field(VulnerabilityIdSerializer(many=True))
305+
class VulnerabilityIdsField(serializers.Field):
306+
307+
"""
308+
Wire-frozen v2 vulnerability_ids field.
309+
310+
Reads ``[{"vulnerability_id": str}]`` from the flag-appropriate store (legacy rows or the
311+
entity references) — byte-identical either way because writes are dual. Accepts the same
312+
object list (tolerating bare strings) on write and hands the parsed strings to create/update
313+
under ``parsed_vulnerability_ids``, which funnel to save_vulnerability_ids (unchanged path).
314+
"""
315+
316+
def __init__(self, **kwargs):
317+
kwargs["source"] = "*"
318+
kwargs.setdefault("required", False)
319+
super().__init__(**kwargs)
320+
321+
def get_attribute(self, instance):
322+
# source="*" -> to_representation receives the whole Finding.
323+
return instance
324+
325+
def to_representation(self, finding):
326+
from dojo.vulnerability_id.queries import finding_vulnerability_id_strings # noqa: PLC0415 -- flag seam
327+
return [{"vulnerability_id": value} for value in finding_vulnerability_id_strings(finding)]
328+
329+
def to_internal_value(self, data):
330+
if not isinstance(data, list):
331+
msg = "Expected a list of vulnerability ids."
332+
raise serializers.ValidationError(msg)
333+
parsed = []
334+
for item in data:
335+
if isinstance(item, dict):
336+
if "vulnerability_id" not in item:
337+
msg = 'Each vulnerability id object requires a "vulnerability_id" key.'
338+
raise serializers.ValidationError(msg)
339+
parsed.append(item["vulnerability_id"])
340+
elif isinstance(item, str):
341+
parsed.append(item)
342+
else:
343+
msg = 'Each vulnerability id must be a string or {"vulnerability_id": "..."}.'
344+
raise serializers.ValidationError(msg)
345+
# source="*" merges this dict into validated_data; create/update pop the key.
346+
return {"parsed_vulnerability_ids": parsed}
347+
348+
304349
@extend_schema_field(serializers.CharField())
305350
class CweField(serializers.Field):
306351

@@ -345,9 +390,7 @@ class FindingSerializer(serializers.ModelSerializer):
345390
finding_groups = FindingGroupSerializer(
346391
source="finding_group_set", many=True, read_only=True,
347392
)
348-
vulnerability_ids = VulnerabilityIdSerializer(
349-
source="vulnerability_id_set", many=True, required=False,
350-
)
393+
vulnerability_ids = VulnerabilityIdsField(required=False)
351394
cwes = FindingCweSerializer(
352395
source="finding_cwe_set", many=True, required=False,
353396
)
@@ -439,12 +482,10 @@ def update(self, instance, validated_data):
439482
# push_all_issues already checked in api views.py
440483
push_to_jira = validated_data.pop("push_to_jira")
441484

442-
# Save vulnerability ids and pop them
443-
parsed_vulnerability_ids = []
444-
if (vulnerability_ids := validated_data.pop("vulnerability_id_set", None)):
445-
logger.debug("VULNERABILITY_ID_SET: %s", vulnerability_ids)
446-
parsed_vulnerability_ids.extend(vulnerability_id["vulnerability_id"] for vulnerability_id in vulnerability_ids)
447-
logger.debug("SETTING CVE FROM VULNERABILITY_ID_SET: %s", parsed_vulnerability_ids[0])
485+
# Save vulnerability ids and pop them (VulnerabilityIdsField parsed them to strings)
486+
parsed_vulnerability_ids = validated_data.pop("parsed_vulnerability_ids", None) or []
487+
if parsed_vulnerability_ids:
488+
logger.debug("SETTING CVE FROM VULNERABILITY_IDS: %s", parsed_vulnerability_ids[0])
448489
validated_data["cve"] = parsed_vulnerability_ids[0]
449490

450491
# CWEs (mirror vulnerability_ids): the first entry is the primary Finding.cwe; the rest
@@ -603,9 +644,7 @@ class FindingCreateSerializer(serializers.ModelSerializer):
603644
)
604645
url = serializers.CharField(allow_null=True, default=None)
605646
push_to_jira = serializers.BooleanField(default=False)
606-
vulnerability_ids = VulnerabilityIdSerializer(
607-
source="vulnerability_id_set", many=True, required=False,
608-
)
647+
vulnerability_ids = VulnerabilityIdsField(required=False)
609648
cwes = FindingCweSerializer(
610649
source="finding_cwe_set", many=True, required=False,
611650
)
@@ -639,15 +678,12 @@ def create(self, validated_data):
639678
notes = validated_data.pop("notes", None)
640679
found_by = validated_data.pop("found_by", None)
641680
reviewers = validated_data.pop("reviewers", None)
642-
# Process the vulnerability IDs specially
643-
parsed_vulnerability_ids = []
644-
if (vulnerability_ids := validated_data.pop("vulnerability_id_set", None)):
645-
logger.debug("VULNERABILITY_ID_SET: %s", vulnerability_ids)
646-
parsed_vulnerability_ids.extend(vulnerability_id["vulnerability_id"] for vulnerability_id in vulnerability_ids)
647-
logger.debug("PARSED_VULNERABILITY_IDST: %s", parsed_vulnerability_ids)
648-
logger.debug("SETTING CVE FROM VULNERABILITY_ID_SET: %s", parsed_vulnerability_ids[0])
681+
# Process the vulnerability IDs specially (VulnerabilityIdsField parsed them to strings)
682+
parsed_vulnerability_ids = validated_data.pop("parsed_vulnerability_ids", None) or []
683+
if parsed_vulnerability_ids:
684+
logger.debug("PARSED_VULNERABILITY_IDS: %s", parsed_vulnerability_ids)
685+
logger.debug("SETTING CVE FROM VULNERABILITY_IDS: %s", parsed_vulnerability_ids[0])
649686
validated_data["cve"] = parsed_vulnerability_ids[0]
650-
# validated_data["unsaved_vulnerability_ids"] = parsed_vulnerability_ids
651687

652688
# CWEs (mirror vulnerability_ids): first entry is the primary cwe, the rest are extras.
653689
parsed_cwes = None

dojo/finding/deduplication.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from dojo.celery import app
1111
from dojo.models import Endpoint_Status, Finding, System_Settings
12+
from dojo.vulnerability_id.queries import vulnerability_id_prefetch
1213

1314
logger = logging.getLogger(__name__)
1415
deduplicationLogger = logging.getLogger("dojo.specific-loggers.deduplication")
@@ -340,11 +341,11 @@ def build_candidate_scope_queryset(test, mode="deduplication", service=None):
340341
queryset = Finding.objects.filter(scope_q)
341342

342343
if settings.V3_FEATURE_LOCATIONS:
343-
prefetch_list = ["locations__location__url", "vulnerability_id_set", "finding_cwe_set", "found_by"]
344+
prefetch_list = ["locations__location__url", vulnerability_id_prefetch(), "finding_cwe_set", "found_by"]
344345
else:
345346
# TODO: Delete this after the move to Locations
346347
# Base prefetches for both modes
347-
prefetch_list = ["endpoints", "vulnerability_id_set", "finding_cwe_set", "found_by"]
348+
prefetch_list = ["endpoints", vulnerability_id_prefetch(), "finding_cwe_set", "found_by"]
348349

349350
# Prefetch all endpoint statuses with their endpoint for reimport mode.
350351
# The non-special filtering (excluding false_positive, out_of_scope, risk_accepted)

dojo/finding/models.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -705,9 +705,13 @@ def copy(self, test=None):
705705
copy.found_by.set(old_found_by)
706706
# Assign any tags
707707
copy.tags.set(old_tags)
708-
# Copy the vulnerability ids and CWEs (relation rows aren't copied by copy_model_util)
709-
for vulnerability_id in self.vulnerability_id_set.all():
710-
Vulnerability_Id.objects.create(finding=copy, vulnerability_id=vulnerability_id.vulnerability_id)
708+
# Copy the vulnerability ids and CWEs (relation rows aren't copied by copy_model_util).
709+
# Route vulnerability ids through the dual-write seam so the copy gets legacy rows AND
710+
# entity references (copy.cve was already carried over by copy_model_util).
711+
from dojo.vulnerability_id.manager import persist_for_finding # noqa: PLC0415 -- avoid import cycle
712+
vulnerability_id_strings = [row.vulnerability_id for row in self.vulnerability_id_set.all()]
713+
if vulnerability_id_strings:
714+
persist_for_finding(copy, vulnerability_id_strings, delete_existing=False)
711715
for finding_cwe in self.finding_cwe_set.all():
712716
Finding_CWE.objects.create(finding=copy, cwe=finding_cwe.cwe)
713717

@@ -838,12 +842,18 @@ def _get_unsaved_vulnerability_ids(finding) -> str:
838842

839843
def _get_saved_vulnerability_ids(finding) -> str:
840844
if finding.id is not None:
841-
# Use the reverse relation (vulnerability_id_set) rather than a fresh
842-
# Vulnerability_Id.objects.filter(...) so prefetch_related("vulnerability_id_set")
843-
# is honored — avoids an N+1 (COUNT + SELECT per finding) during dedupe/hashcode.
844-
vulnerability_id_str_list = [str(vulnerability_id) for vulnerability_id in finding.vulnerability_id_set.all()]
845+
from dojo.vulnerability_id.queries import use_entity_reads # noqa: PLC0415
846+
if use_entity_reads():
847+
# Entity store: the prefetch-honoring reverse relation (vulnerability_references
848+
# + select_related("vulnerability")) so Prefetch is honored — no N+1 in dedupe.
849+
vulnerability_id_str_list = [ref.vulnerability.vulnerability_id for ref in finding.vulnerability_references.all()]
850+
else:
851+
# Use the reverse relation (vulnerability_id_set) rather than a fresh
852+
# Vulnerability_Id.objects.filter(...) so prefetch_related("vulnerability_id_set")
853+
# is honored — avoids an N+1 (COUNT + SELECT per finding) during dedupe/hashcode.
854+
vulnerability_id_str_list = [str(vulnerability_id) for vulnerability_id in finding.vulnerability_id_set.all()]
845855
deduplicationLogger.debug("get_vulnerability_ids after the finding was saved. Vulnerability references count: " + str(len(vulnerability_id_str_list)))
846-
# sort vulnerability_ids strings
856+
# sort vulnerability_ids strings (no dedupe — byte parity with the legacy store)
847857
return "".join(sorted(vulnerability_id_str_list))
848858
return ""
849859

@@ -1383,9 +1393,14 @@ def get_references_with_links(self):
13831393

13841394
@cached_property
13851395
def vulnerability_ids(self):
1396+
from dojo.vulnerability_id.queries import use_entity_reads # noqa: PLC0415
13861397
# Get vulnerability ids from database and convert to list of strings
1387-
vulnerability_ids_model = self.vulnerability_id_set.all()
1388-
vulnerability_ids = [vulnerability_id.vulnerability_id for vulnerability_id in vulnerability_ids_model]
1398+
if use_entity_reads():
1399+
# Entity store: reverse relation is ordered by FindingVulnerabilityReference.Meta.ordering.
1400+
vulnerability_ids = [ref.vulnerability.vulnerability_id for ref in self.vulnerability_references.all()]
1401+
else:
1402+
vulnerability_ids_model = self.vulnerability_id_set.all()
1403+
vulnerability_ids = [vulnerability_id.vulnerability_id for vulnerability_id in vulnerability_ids_model]
13891404

13901405
# Synchronize the cve field with the unsaved_vulnerability_ids
13911406
# We do this to be as flexible as possible to handle the fields until

dojo/finding/queries.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ def get_auth_filter(key): return None
2020
Vulnerability_Id,
2121
)
2222
from dojo.request_cache import cache_for_request_or_task
23+
from dojo.vulnerability_id.queries import vulnerability_id_prefetch
2324

2425
logger = logging.getLogger(__name__)
2526

@@ -111,7 +112,7 @@ def prefetch_for_findings(findings, prefetch_type="all", *, exclude_untouched=Tr
111112
"status_finding",
112113
"finding_group_set",
113114
"finding_group_set__jira_issue", # Include both variants
114-
"vulnerability_id_set",
115+
vulnerability_id_prefetch(),
115116
)
116117
base_status = LocationFindingReference.objects.prefetch_related("location__url").all()
117118
prefetched_findings = prefetched_findings.annotate(
@@ -147,7 +148,7 @@ def prefetch_for_findings(findings, prefetch_type="all", *, exclude_untouched=Tr
147148
"status_finding",
148149
"finding_group_set",
149150
"finding_group_set__jira_issue", # Include both variants
150-
"vulnerability_id_set",
151+
vulnerability_id_prefetch(),
151152
)
152153
base_status = Endpoint_Status.objects.prefetch_related("endpoint")
153154
status = Case(

dojo/finding/ui/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
reopen_external_issue,
122122
update_external_issue,
123123
)
124+
from dojo.vulnerability_id.queries import vulnerability_id_prefetch
124125

125126
JFORM_PUSH_TO_JIRA_MESSAGE = "jform.push_to_jira: %s"
126127

@@ -168,7 +169,7 @@ def prefetch_for_similar_findings(findings):
168169
prefetched_findings = prefetched_findings.prefetch_related("notes")
169170
prefetched_findings = prefetched_findings.prefetch_related("tags")
170171
prefetched_findings = prefetched_findings.prefetch_related(
171-
"vulnerability_id_set",
172+
vulnerability_id_prefetch(),
172173
)
173174
else:
174175
logger.debug("unable to prefetch because query was already executed")

dojo/management/commands/dedupe.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
get_system_setting,
2121
mass_model_updater,
2222
)
23+
from dojo.vulnerability_id.queries import vulnerability_id_prefetch
2324

2425
logger = logging.getLogger(__name__)
2526
deduplicationLogger = logging.getLogger("dojo.specific-loggers.deduplication")
@@ -97,10 +98,10 @@ def _run_dedupe(self, *, restrict_to_parsers, hash_code_only, dedupe_only, dedup
9798
"test", "test__engagement", "test__engagement__product", "test__test_type",
9899
).prefetch_related(
99100
"locations",
100-
# vulnerability_id_set feeds hash_code computation for parsers whose
101+
# vulnerability id store feeds hash_code computation for parsers whose
101102
# HASHCODE_FIELDS_PER_SCANNER includes vulnerability_ids; prefetch to avoid
102103
# a per-finding query in get_vulnerability_ids().
103-
"vulnerability_id_set",
104+
vulnerability_id_prefetch(),
104105
Prefetch(
105106
"original_finding",
106107
queryset=Finding.objects.only("id", "duplicate_finding_id").order_by("-id"),
@@ -113,10 +114,10 @@ def _run_dedupe(self, *, restrict_to_parsers, hash_code_only, dedupe_only, dedup
113114
"test", "test__engagement", "test__engagement__product", "test__test_type",
114115
).prefetch_related(
115116
"endpoints",
116-
# vulnerability_id_set feeds hash_code computation for parsers whose
117+
# vulnerability id store feeds hash_code computation for parsers whose
117118
# HASHCODE_FIELDS_PER_SCANNER includes vulnerability_ids; prefetch to avoid
118119
# a per-finding query in get_vulnerability_ids().
119-
"vulnerability_id_set",
120+
vulnerability_id_prefetch(),
120121
Prefetch(
121122
"original_finding",
122123
queryset=Finding.objects.only("id", "duplicate_finding_id").order_by("-id"),

dojo/risk_acceptance/api/mixins.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212

1313
from dojo.authorization.api_permissions import UserHasRiskAcceptanceRelatedObjectPermission
1414
from dojo.engagement.queries import get_authorized_engagements
15-
from dojo.models import Engagement, Risk_Acceptance, User, Vulnerability_Id
15+
from dojo.models import Engagement, Risk_Acceptance, User
1616
from dojo.risk_acceptance.api.serializer import RiskAcceptanceSerializer
17+
from dojo.vulnerability_id.queries import finding_ids_with_vulnerability_ids
1718

1819
AcceptedRisk = NamedTuple("AcceptedRisk", (("vulnerability_id", str), ("justification", str), ("accepted_by", str)))
1920

@@ -90,10 +91,8 @@ def accept_risks(self, request):
9091
def _accept_risks(accepted_risks: list[AcceptedRisk], base_findings: QuerySet, owner: User):
9192
accepted = []
9293
for risk in accepted_risks:
93-
vulnerability_ids = Vulnerability_Id.objects \
94-
.filter(vulnerability_id=risk.vulnerability_id) \
95-
.values("finding")
96-
findings = base_findings.filter(id__in=vulnerability_ids)
94+
finding_ids = finding_ids_with_vulnerability_ids(risk.vulnerability_id, lookup="exact")
95+
findings = base_findings.filter(id__in=finding_ids)
9796
if findings.exists():
9897
# TODO: we could use risk.vulnerability_id to name the risk_acceptance, but would need to check for existing risk_acceptances in that case
9998
# so for now we add some timestamp based suffix

dojo/test/ui/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
process_tag_notifications,
8383
redirect_to_return_url_or_else,
8484
)
85+
from dojo.vulnerability_id.queries import vulnerability_id_prefetch
8586

8687
logger = logging.getLogger(__name__)
8788
parse_logger = logging.getLogger("dojo")
@@ -172,7 +173,7 @@ def get_initial_context(self, request: HttpRequest, test: Test):
172173
"jira_project": jira_services.get_project(test),
173174
"bulk_edit_form": FindingBulkUpdateForm(request.GET),
174175
"enable_table_filtering": get_system_setting("enable_ui_table_based_searching"),
175-
"finding_groups": test.finding_group_set.all().prefetch_related("findings", "jira_issue", "creator", "findings__vulnerability_id_set"),
176+
"finding_groups": test.finding_group_set.all().prefetch_related("findings", "jira_issue", "creator", vulnerability_id_prefetch(prefix="findings__")),
176177
"finding_group_by_options": Finding_Group.GROUP_BY_OPTIONS,
177178
}
178179
# Set the form using the context, and then update the context

0 commit comments

Comments
 (0)