From 68f4cbe3bbcb3231090597db856cd801f556d734 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:01:13 -0600 Subject: [PATCH] fix(importers): stop doubling the (scan_type) suffix in dynamic Test Type names Generic Findings Import (and other dynamic-test-type parsers like SARIF) built the Test Type name by unconditionally appending " Scan ({scan_type})" to the report's `type` field. When `type` already carried the " ({scan_type})" suffix, the suffix was doubled, e.g. "Prisma Cloud (Generic Findings Import) Scan (Generic Findings Import)". Extract the name resolution into resolve_dynamic_test_type_name(), which is now idempotent: a `type` already ending in " ({scan_type})" is used verbatim. The intentional "{type} Scan ({scan_type})" format is unchanged for all other cases (Tool1 -> "Tool1 Scan (Generic Findings Import)"), preserving SARIF and existing behavior. To avoid breaking existing data, the reimport validation accepts either the new idempotent name or the legacy (pre-patch) name via legacy_dynamic_test_type_name(), so reimports into tests whose test_type was created with the old doubled name keep working without a mismatch error. Also corrects the Generic Findings Import docs, which described a "{Test Name} (Generic Findings Import)" format the code never produced. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../OS_hierarchy/product_hierarchy.md | 1 + .../engagements_tests/OS__tests.md | 2 +- .../engagements_tests/PRO__tests.md | 2 +- .../supported_tools/parsers/file/generic.md | 2 + .../parsers/generic_findings_import.md | 30 +++- dojo/importers/base_importer.py | 63 +++++--- .../generic/generic_test_type_suffix.json | 13 ++ unittests/test_importers_importer.py | 149 ++++++++++++++++++ 8 files changed, 228 insertions(+), 34 deletions(-) create mode 100644 unittests/scans/generic/generic_test_type_suffix.json diff --git a/docs/content/asset_modelling/OS_hierarchy/product_hierarchy.md b/docs/content/asset_modelling/OS_hierarchy/product_hierarchy.md index 36e0149b34a..bbd1885ab93 100644 --- a/docs/content/asset_modelling/OS_hierarchy/product_hierarchy.md +++ b/docs/content/asset_modelling/OS_hierarchy/product_hierarchy.md @@ -155,6 +155,7 @@ Examples include: **Report-Defined Test Type Naming Rules:** - If the report's `type` field equals the scan type → uses scan type directly (e.g., "Generic Findings Import") - If the report's `type` field differs → creates "{type} Scan ({scan_type})" format (e.g., "Tool1 Scan (Generic Findings Import)") +- If the report's `type` field already ends with the " ({scan_type})" suffix → uses it verbatim, so the suffix is never doubled (e.g., "Tool1 (Generic Findings Import)" stays "Tool1 (Generic Findings Import)") - If no `type` field is provided → uses scan type directly **Important Considerations:** diff --git a/docs/content/asset_modelling/engagements_tests/OS__tests.md b/docs/content/asset_modelling/engagements_tests/OS__tests.md index 799a29d7303..aee04a4a161 100644 --- a/docs/content/asset_modelling/engagements_tests/OS__tests.md +++ b/docs/content/asset_modelling/engagements_tests/OS__tests.md @@ -37,7 +37,7 @@ While each method differs primarily in how scan data is parsed and ingested, the When no native parser exists for a given tool, **Generic Findings Import** allows you to import findings using a standardized JSON or CSV schema, regardless of the original source. -DefectDojo parses the provided data, creates a new Test (or imports into an existing one), and attaches the Findings. A corresponding Test Type is also created in the format “{Test Name} (Generic Findings Import).” +DefectDojo parses the provided data, creates a new Test (or imports into an existing one), and attaches the Findings. A corresponding Test Type is also created based on the report's optional `type` field: when `type` is omitted (or equals the scan type) the Test Type is “Generic Findings Import”; when `type` is provided it becomes “{type} Scan (Generic Findings Import)” (a `type` that already ends with the “(Generic Findings Import)” suffix is used verbatim). | | **Native Parsers** | **Generic Findings Import** | |----------|---------------|------------------------| diff --git a/docs/content/asset_modelling/engagements_tests/PRO__tests.md b/docs/content/asset_modelling/engagements_tests/PRO__tests.md index baecc9fedbd..59784a8cfd3 100644 --- a/docs/content/asset_modelling/engagements_tests/PRO__tests.md +++ b/docs/content/asset_modelling/engagements_tests/PRO__tests.md @@ -35,7 +35,7 @@ While each method differs primarily in how scan data is parsed and ingested, the When no native parser exists for a given tool, [**Generic Findings Import**](/supported_tools/parsers/generic_findings_import) allows you to import findings using a standardized JSON or CSV schema, regardless of the original source. -DefectDojo parses the provided data, creates a new Test (or imports into an existing one), and attaches the Findings. A corresponding Test Type is also created in the format “`{Test Name}` (Generic Findings Import).” +DefectDojo parses the provided data, creates a new Test (or imports into an existing one), and attaches the Findings. A corresponding Test Type is also created based on the report's optional `type` field: when `type` is omitted (or equals the scan type) the Test Type is “Generic Findings Import”; when `type` is provided it becomes “`{type}` Scan (Generic Findings Import)” (a `type` that already ends with the “(Generic Findings Import)” suffix is used verbatim). #### Universal Parser diff --git a/docs/content/supported_tools/parsers/file/generic.md b/docs/content/supported_tools/parsers/file/generic.md index 569309c05c1..edef7d948dc 100644 --- a/docs/content/supported_tools/parsers/file/generic.md +++ b/docs/content/supported_tools/parsers/file/generic.md @@ -226,6 +226,8 @@ Example: } ``` +The resulting Test Type name is derived from the `type` field: when `type` is omitted (or equals the scan type) the Test Type is `Generic Findings Import`; when `type` is provided it becomes `{type} Scan (Generic Findings Import)` (for the example above, `My custom Test type Scan (Generic Findings Import)`). A `type` that already ends with the `(Generic Findings Import)` suffix is used verbatim, so the suffix is never doubled. + ### Sample Scan Data Sample Generic Findings Import scans can be found [here](https://github.com/DefectDojo/django-DefectDojo/tree/master/unittests/scans/generic). diff --git a/docs/content/supported_tools/parsers/generic_findings_import.md b/docs/content/supported_tools/parsers/generic_findings_import.md index a7b16002917..899ec666c18 100644 --- a/docs/content/supported_tools/parsers/generic_findings_import.md +++ b/docs/content/supported_tools/parsers/generic_findings_import.md @@ -6,14 +6,28 @@ weight: 2 Open-source and Pro users can use Generic Findings Import as a method to ingest JSON or CSV files into DefectDojo which are not already in the supported Tools list. -Using Generic Findings Import will create a new Test Type in your DefectDojo instance called "`{The Name Of Your Test}` (Generic Findings Import)". For example, this JSON content will result in a Test Type called "Example Report (Generic Findings Import)": - -``` -{ - "name": "Example Report", - "findings": [] -} -``` +Using Generic Findings Import creates a Test Type in your DefectDojo instance based on the optional `type` field in the report. The naming rules are: + +- If no `type` field is provided (or it equals the scan type), the Test Type is simply **"Generic Findings Import"**. For example, this JSON content results in the Test Type "Generic Findings Import": + + ``` + { + "name": "Example Report", + "findings": [] + } + ``` + +- If a `type` field is provided and differs from the scan type, the Test Type is **"`{type}` Scan (Generic Findings Import)"**. For example, `"type": "Tool1"` results in the Test Type "Tool1 Scan (Generic Findings Import)": + + ``` + { + "name": "Example Report", + "type": "Tool1", + "findings": [] + } + ``` + +- If the `type` field already ends with the "(Generic Findings Import)" suffix, it is used verbatim (the suffix is never doubled). For example, `"type": "Tool1 (Generic Findings Import)"` results in the Test Type "Tool1 (Generic Findings Import)". DefectDojo Pro users can also consider using the [Universal Parser](../universal_parser), a tool which allows for highly customizable JSON, XML and CSV imports. diff --git a/dojo/importers/base_importer.py b/dojo/importers/base_importer.py index d87524185fe..fcfcf701354 100644 --- a/dojo/importers/base_importer.py +++ b/dojo/importers/base_importer.py @@ -197,33 +197,18 @@ def consolidate_dynamic_tests(self, tests: list[Test]) -> list[Finding]: # only if they are different. This is to support meta format like SARIF # so a report that have the label 'CodeScanner' will be changed to 'CodeScanner Scan (SARIF)' test_raw = tests[0] - test_type_name = self.scan_type # Create a new test if it has not already been created if not self.test: - # Determine if we should use a custom test type name - if test_raw.type: - # If test_raw.type equals scan_type, use scan_type directly - if test_raw.type == self.scan_type: - test_type_name = self.scan_type - else: - test_type_name = f"{tests[0].type} Scan" - if test_type_name != self.scan_type: - test_type_name = f"{test_type_name} ({self.scan_type})" - self.test = self.create_test(test_type_name) + # Resolve the Test_Type name from the report's type (idempotent: a type that already + # carries the " (scan_type)" suffix is used verbatim rather than doubled) + self.test = self.create_test(self.resolve_dynamic_test_type_name(test_raw.type)) else: - # During reimport, validate that the test_type matches - # Calculate the expected test_type_name from the incoming report - expected_test_type_name = self.scan_type - if test_raw.type: - # If test_raw.type equals scan_type, use scan_type directly - if test_raw.type == self.scan_type: - expected_test_type_name = self.scan_type - else: - expected_test_type_name = f"{test_raw.type} Scan" - if expected_test_type_name != self.scan_type: - expected_test_type_name = f"{expected_test_type_name} ({self.scan_type})" - # Compare with existing test's test_type name - if self.test.test_type.name != expected_test_type_name: + # During reimport, validate that the test_type matches the incoming report. + # Accept either the current (idempotent) name or the legacy name the pre-patch code + # produced, so reimports into tests created before the doubling fix keep working. + expected_test_type_name = self.resolve_dynamic_test_type_name(test_raw.type) + legacy_test_type_name = self.legacy_dynamic_test_type_name(test_raw.type) + if self.test.test_type.name not in {expected_test_type_name, legacy_test_type_name}: msg = ( f"Test type mismatch: Test {self.test.id} has test_type '{self.test.test_type.name}', " f"but the report contains test_type '{expected_test_type_name}'. " @@ -568,6 +553,36 @@ def update_test_progress( self.test.percent_complete = percentage_value self.test.save() + def resolve_dynamic_test_type_name(self, raw_type: str | None) -> str: + """ + Compute the Test_Type name for a dynamic-test-type report (e.g. Generic, SARIF). + + - No type, or type already equals the scan type -> use the scan type as-is. + - Type already carries the scan-type suffix (e.g. "Prisma Cloud (Generic Findings + Import)") -> use it verbatim. This keeps the composition idempotent and prevents + doubled names like "X (scan_type) Scan (scan_type)". + - Otherwise -> the intentional "{type} Scan ({scan_type})" format (also used by SARIF, + so a report labeled 'CodeScanner' becomes 'CodeScanner Scan (SARIF)'). + """ + if not raw_type or raw_type == self.scan_type: + return self.scan_type + if raw_type.endswith(f" ({self.scan_type})"): + return raw_type + return f"{raw_type} Scan ({self.scan_type})" + + def legacy_dynamic_test_type_name(self, raw_type: str | None) -> str: + """ + Reproduce the name the pre-idempotency code produced. Used only for the reimport + compatibility check so reimports into existing (pre-patch) tests whose test_type + already has a doubled suffix keep working instead of raising a mismatch error. + """ + if not raw_type or raw_type == self.scan_type: + return self.scan_type + name = f"{raw_type} Scan" + if name != self.scan_type: + name = f"{name} ({self.scan_type})" + return name + def get_or_create_test_type( self, test_type_name: str, diff --git a/unittests/scans/generic/generic_test_type_suffix.json b/unittests/scans/generic/generic_test_type_suffix.json new file mode 100644 index 00000000000..690e8afdefd --- /dev/null +++ b/unittests/scans/generic/generic_test_type_suffix.json @@ -0,0 +1,13 @@ +{ + "name": "Prisma Cloud", + "type": "Prisma Cloud (Generic Findings Import)", + "findings": [ + { + "title": "Test Finding Prisma", + "description": "This is a test finding whose type already carries the scan-type suffix", + "severity": "High", + "active": true, + "verified": true + } + ] +} diff --git a/unittests/test_importers_importer.py b/unittests/test_importers_importer.py index ad9dd8f66c3..4172f3459c1 100644 --- a/unittests/test_importers_importer.py +++ b/unittests/test_importers_importer.py @@ -16,6 +16,7 @@ Product, Product_Type, Test, + Test_Type, User, Vulnerability_Id, ) @@ -397,6 +398,154 @@ def test_reimport_generic_type_equals_scan_type(self): # Verify findings were processed self.assertGreater(len_new_findings, 0) + # Regression: Generic import doubled the (Generic Findings Import) suffix in the Test_Type name + def test_import_generic_type_with_suffix_is_not_doubled(self): + """When the report's type already carries the scan-type suffix, it must be used verbatim (no ' Scan (...)' re-append).""" + generic_test_type_suffix = get_unit_tests_scans_path("generic") / "generic_test_type_suffix.json" + with generic_test_type_suffix.open(encoding="utf-8") as scan: + scan_type = "Generic Findings Import" + user, _ = User.objects.get_or_create(username="admin") + product_type, _ = Product_Type.objects.get_or_create(name="test_generic_suffix") + product, _ = Product.objects.get_or_create( + name="TestGenericSuffix", + description="test product", + prod_type=product_type, + ) + engagement, _ = Engagement.objects.get_or_create( + name="Test Generic Suffix Engagement", + product=product, + target_start=timezone.now(), + target_end=timezone.now(), + ) + environment, _ = Development_Environment.objects.get_or_create(name="Development") + import_options = { + "user": user, + "lead": user, + "scan_date": None, + "environment": environment, + "minimum_severity": "Info", + "active": True, + "verified": True, + "scan_type": scan_type, + "engagement": engagement, + "close_old_findings": False, + } + importer = DefaultImporter(**import_options) + test, _, _, _, _, _, _ = importer.process_scan(scan) + self.assertIsNotNone(test) + # The suffix must NOT be doubled into "... (Generic Findings Import) Scan (Generic Findings Import)" + self.assertEqual( + "Prisma Cloud (Generic Findings Import)", test.test_type.name, + msg=f"expected 'Prisma Cloud (Generic Findings Import)', persisted='{test.test_type.name}'", + ) + + # Regression: Generic import doubled the (Generic Findings Import) suffix in the Test_Type name + def test_reimport_generic_type_with_suffix_is_idempotent(self): + """Reimporting a report whose type carries the scan-type suffix must not raise and must keep the same test/type.""" + generic_test_type_suffix = get_unit_tests_scans_path("generic") / "generic_test_type_suffix.json" + with generic_test_type_suffix.open(encoding="utf-8") as scan: + scan_type = "Generic Findings Import" + user, _ = User.objects.get_or_create(username="admin") + product_type, _ = Product_Type.objects.get_or_create(name="test_generic_suffix_reimport") + product, _ = Product.objects.get_or_create( + name="TestGenericSuffixReimport", + description="test product", + prod_type=product_type, + ) + engagement, _ = Engagement.objects.get_or_create( + name="Test Generic Suffix Reimport Engagement", + product=product, + target_start=timezone.now(), + target_end=timezone.now(), + ) + environment, _ = Development_Environment.objects.get_or_create(name="Development") + import_options = { + "user": user, + "lead": user, + "scan_date": None, + "environment": environment, + "minimum_severity": "Info", + "active": True, + "verified": True, + "scan_type": scan_type, + "engagement": engagement, + "close_old_findings": False, + } + importer = DefaultImporter(**import_options) + test, _, _, _, _, _, _ = importer.process_scan(scan) + original_test_type_name = test.test_type.name + self.assertEqual("Prisma Cloud (Generic Findings Import)", original_test_type_name) + + reimport_options = { + "test": test, + "user": user, + "lead": user, + "scan_date": None, + "environment": environment, + "minimum_severity": "Info", + "active": True, + "verified": True, + "scan_type": scan_type, + "close_old_findings": False, + } + reimporter = DefaultReImporter(**reimport_options) + scan.seek(0) + test_after_reimport, _, _, _, _, _, _ = reimporter.process_scan(scan) + self.assertEqual(test.id, test_after_reimport.id) + test.refresh_from_db() + self.assertEqual(original_test_type_name, test.test_type.name) + + # Regression: Generic import doubled the (Generic Findings Import) suffix in the Test_Type name + def test_reimport_into_legacy_doubled_test_type_still_works(self): + """Pre-patch data: a Test whose test_type has the old doubled name must still accept reimports.""" + generic_test_type_suffix = get_unit_tests_scans_path("generic") / "generic_test_type_suffix.json" + scan_type = "Generic Findings Import" + user, _ = User.objects.get_or_create(username="admin") + product_type, _ = Product_Type.objects.get_or_create(name="test_generic_legacy_doubled") + product, _ = Product.objects.get_or_create( + name="TestGenericLegacyDoubled", + description="test product", + prod_type=product_type, + ) + engagement, _ = Engagement.objects.get_or_create( + name="Test Generic Legacy Doubled Engagement", + product=product, + target_start=timezone.now(), + target_end=timezone.now(), + ) + environment, _ = Development_Environment.objects.get_or_create(name="Development") + # Simulate a test created by the pre-patch code: test_type name has the doubled suffix + legacy_doubled_name = "Prisma Cloud (Generic Findings Import) Scan (Generic Findings Import)" + legacy_test_type, _ = Test_Type.objects.get_or_create(name=legacy_doubled_name) + test = Test.objects.create( + engagement=engagement, + test_type=legacy_test_type, + environment=environment, + target_start=timezone.now(), + target_end=timezone.now(), + ) + reimport_options = { + "test": test, + "user": user, + "lead": user, + "scan_date": None, + "environment": environment, + "minimum_severity": "Info", + "active": True, + "verified": True, + "scan_type": scan_type, + "close_old_findings": False, + } + reimporter = DefaultReImporter(**reimport_options) + with generic_test_type_suffix.open(encoding="utf-8") as scan: + # Must NOT raise "Test type mismatch" for pre-existing doubled-name test types + test_after_reimport, _, len_new_findings, _, _, _, _ = reimporter.process_scan(scan) + self.assertEqual(test.id, test_after_reimport.id) + test.refresh_from_db() + # Historical name is preserved (we do not rename existing data) + self.assertEqual(legacy_doubled_name, test.test_type.name) + self.assertGreater(len_new_findings, 0) + class FlexibleImportTestAPI(DojoAPITestCase): def __init__(self, *args, **kwargs):