From 63a63906916a1845288cc6e5aca3b642d086c039 Mon Sep 17 00:00:00 2001 From: DefectDojo release bot Date: Mon, 1 Jun 2026 17:39:18 +0000 Subject: [PATCH 1/9] Update versions in application files --- components/package.json | 2 +- helm/defectdojo/Chart.yaml | 8 ++++---- helm/defectdojo/README.md | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/package.json b/components/package.json index 2651883b8b7..04a55d739d4 100644 --- a/components/package.json +++ b/components/package.json @@ -1,6 +1,6 @@ { "name": "defectdojo", - "version": "2.59.0", + "version": "2.60.0-dev", "license": "BSD-3-Clause", "private": true, "dependencies": { diff --git a/helm/defectdojo/Chart.yaml b/helm/defectdojo/Chart.yaml index eb061f77bb6..49095de5ffa 100644 --- a/helm/defectdojo/Chart.yaml +++ b/helm/defectdojo/Chart.yaml @@ -1,8 +1,8 @@ apiVersion: v2 -appVersion: "2.59.0" +appVersion: "2.60.0-dev" description: A Helm chart for Kubernetes to install DefectDojo name: defectdojo -version: 1.9.30 +version: 1.9.31-dev icon: https://defectdojo.com/hubfs/DefectDojo_favicon.png maintainers: - name: madchap @@ -33,5 +33,5 @@ dependencies: # - kind: security # description: Critical bug annotations: - artifacthub.io/prerelease: "false" - artifacthub.io/changes: "- kind: changed\n description: Update valkey Docker tag from 0.20.0 to v0.20.1 (_/defect_/Chart.yaml)\n- kind: changed\n description: Update valkey Docker tag from 0.20.1 to v0.20.2 (_/defect_/Chart.yaml)\n- kind: changed\n description: Bump DefectDojo to 2.59.0\n" + artifacthub.io/prerelease: "true" + artifacthub.io/changes: "" diff --git a/helm/defectdojo/README.md b/helm/defectdojo/README.md index f8b2e30f27c..30f81ff839e 100644 --- a/helm/defectdojo/README.md +++ b/helm/defectdojo/README.md @@ -511,7 +511,7 @@ The HELM schema will be generated for you. # General information about chart values -![Version: 1.9.30](https://img.shields.io/badge/Version-1.9.30-informational?style=flat-square) ![AppVersion: 2.59.0](https://img.shields.io/badge/AppVersion-2.59.0-informational?style=flat-square) +![Version: 1.9.31-dev](https://img.shields.io/badge/Version-1.9.31--dev-informational?style=flat-square) ![AppVersion: 2.60.0-dev](https://img.shields.io/badge/AppVersion-2.60.0--dev-informational?style=flat-square) A Helm chart for Kubernetes to install DefectDojo From 74a525ec45f7eeda9c4c521e39687269f1e4eec3 Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Fri, 5 Jun 2026 04:47:43 +0200 Subject: [PATCH 2/9] feat: allow users to request peer review from themselves (#14946) Remove the exclusion of the current user from the reviewer dropdown in ReviewFindingForm so users can self-assign as reviewer. --- dojo/forms.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dojo/forms.py b/dojo/forms.py index e33d7ef51c4..cb90d0f57de 100644 --- a/dojo/forms.py +++ b/dojo/forms.py @@ -2176,16 +2176,13 @@ class ReviewFindingForm(forms.Form): def __init__(self, *args, **kwargs): finding = kwargs.pop("finding", None) - user = kwargs.pop("user", None) + kwargs.pop("user", None) super().__init__(*args, **kwargs) # Get the list of users if finding is not None: users = get_authorized_users_for_product_and_product_type(None, finding.test.engagement.product, "edit") else: users = get_authorized_users("edit").filter(is_active=True) - # Remove the current user - if user is not None: - users = users.exclude(id=user.id) # Save a copy of the original query to be used in the validator self.reviewer_queryset = users # Set the users in the form From bae53d1038b364adc1d7ca567998ea0864b8eb82 Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Fri, 5 Jun 2026 04:48:32 +0200 Subject: [PATCH 3/9] Preserve verified flag when promoting duplicate to new original (#14934) When the original of a duplicate cluster is deleted (e.g. via engagement deletion), reconfigure_duplicate_cluster promotes the first remaining duplicate to the new primary. It already copies active and is_mitigated from the original, but not verified. The promoted finding kept its own verified=False, which blocked Jira's "Push All Issues" (requires active+verified). Add verified to the fields copied to the new original. Fixes #14911 --- dojo/finding/helper.py | 1 + .../test_prepare_duplicates_for_delete.py | 46 ++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/dojo/finding/helper.py b/dojo/finding/helper.py index 7808fdd7cf5..140a6bd426d 100644 --- a/dojo/finding/helper.py +++ b/dojo/finding/helper.py @@ -603,6 +603,7 @@ def reconfigure_duplicate_cluster(original, cluster_outside): duplicate=False, duplicate_finding=None, active=original.active, + verified=original.verified, is_mitigated=original.is_mitigated, ) new_original.found_by.set(original.found_by.all()) diff --git a/unittests/test_prepare_duplicates_for_delete.py b/unittests/test_prepare_duplicates_for_delete.py index 78d612dfdec..89ed84069db 100644 --- a/unittests/test_prepare_duplicates_for_delete.py +++ b/unittests/test_prepare_duplicates_for_delete.py @@ -294,15 +294,56 @@ def test_multiple_originals(self): self.assertFalse(dupe_of_b.duplicate) self.assertIsNone(dupe_of_b.duplicate_finding) - def test_original_status_copied_to_new_original(self): - """New original inherits active/is_mitigated status from deleted original.""" + def test_original_status_copied_to_new_original_active_verified(self): + """ + New original inherits active/verified/is_mitigated from deleted original. + + Positive case: original is an open, verified, not-mitigated finding. + Duplicate starts with the opposite of each field so every copy is observable. + + Regression test for issue #14911: promoted duplicates kept verified=False + even when the original was verified, blocking Jira "Push All Issues". + """ + original = self._create_finding(self.test1, "Original") + original.active = True + original.verified = True + original.is_mitigated = False + super(Finding, original).save(skip_validation=True) + + outside_dupe = self._create_finding(self.test2, "Outside Dupe") + outside_dupe.is_mitigated = True + super(Finding, outside_dupe).save(skip_validation=True) + self._make_duplicate(outside_dupe, original) # forces active=False, verified default False + + with impersonate(self.testuser): + prepare_duplicates_for_delete(self.test1) + + outside_dupe.refresh_from_db() + self.assertFalse(outside_dupe.duplicate) + self.assertTrue(outside_dupe.active) + self.assertTrue(outside_dupe.verified) + self.assertFalse(outside_dupe.is_mitigated) + + def test_original_status_copied_to_new_original_inactive_mitigated(self): + """ + New original inherits active/verified/is_mitigated from deleted original. + + Negative case: original is closed, unverified, mitigated. + Duplicate starts with the opposite of each field so every copy is observable. + """ original = self._create_finding(self.test1, "Original") original.active = False + original.verified = False original.is_mitigated = True super(Finding, original).save(skip_validation=True) outside_dupe = self._create_finding(self.test2, "Outside Dupe") + outside_dupe.verified = True + super(Finding, outside_dupe).save(skip_validation=True) self._make_duplicate(outside_dupe, original) + # _make_duplicate forces active=False; flip to True so the copy is observable + outside_dupe.active = True + super(Finding, outside_dupe).save(skip_validation=True) with impersonate(self.testuser): prepare_duplicates_for_delete(self.test1) @@ -310,6 +351,7 @@ def test_original_status_copied_to_new_original(self): outside_dupe.refresh_from_db() self.assertFalse(outside_dupe.duplicate) self.assertFalse(outside_dupe.active) + self.assertFalse(outside_dupe.verified) self.assertTrue(outside_dupe.is_mitigated) def test_found_by_copied_to_new_original(self): From 136f54fdb436e1e550edeba91e91e244255dc4fc Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Fri, 5 Jun 2026 04:48:53 +0200 Subject: [PATCH 4/9] Prevent reimport from reactivating duplicate findings as active/verified (#14935) * Prevent reimport from reactivating duplicate findings as active/verified Fixes #14910. process_matched_mitigated_finding reactivated a matched mitigated finding without checking whether it is a duplicate, producing an invalid active/verified duplicate state that the finding edit form rejects. Keep duplicates inactive/unverified on reactivation (un-mitigate only), matching the set_duplicate invariant. * Initialise reimporter accumulators in duplicate reactivation tests process_matched_mitigated_finding appends to self.reactivated_items, which is normally created in process_findings(). The tests drive the method directly, so set the accumulator lists explicitly. --- dojo/importers/default_reimporter.py | 19 ++++- unittests/test_importers_importer.py | 113 ++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/dojo/importers/default_reimporter.py b/dojo/importers/default_reimporter.py index aa33c6153b0..e9c6567107a 100644 --- a/dojo/importers/default_reimporter.py +++ b/dojo/importers/default_reimporter.py @@ -801,9 +801,16 @@ def process_matched_mitigated_finding( existing_finding.mitigated = None existing_finding.is_mitigated = False existing_finding.mitigated_by = None - existing_finding.active = True - if self.verified is not None: - existing_finding.verified = self.verified + # A duplicate finding must stay inactive/unverified (see set_duplicate and the + # "Duplicate findings cannot be verified or active" form validation). Un-mitigate it + # but do not reactivate it, otherwise we create an invalid active/verified duplicate state. + if existing_finding.duplicate: + existing_finding.active = False + existing_finding.verified = False + else: + existing_finding.active = True + if self.verified is not None: + existing_finding.verified = self.verified component_name = getattr(unsaved_finding, "component_name", None) component_version = getattr(unsaved_finding, "component_version", None) @@ -819,7 +826,11 @@ def process_matched_mitigated_finding( # don't dedupe before endpoints/locations are added, postprocessing will be done on next save (in calling method) existing_finding.save_no_options() - note = Notes(entry=f"Re-activated by {self.scan_type} re-upload.", author=self.user) + if existing_finding.duplicate: + note_entry = f"Un-mitigated by {self.scan_type} re-upload but kept inactive because the finding is a duplicate." + else: + note_entry = f"Re-activated by {self.scan_type} re-upload." + note = Notes(entry=note_entry, author=self.user) note.save() self.location_handler.record_reactivations_for_finding(existing_finding) existing_finding.notes.add(note) diff --git a/unittests/test_importers_importer.py b/unittests/test_importers_importer.py index aa14ace8beb..ad9dd8f66c3 100644 --- a/unittests/test_importers_importer.py +++ b/unittests/test_importers_importer.py @@ -932,4 +932,115 @@ def test_change_vulnerability_ids_on_reimport(self): # Verify only new Vulnerability_Id objects exist vuln_ids = list(Vulnerability_Id.objects.filter(finding=finding).values_list("vulnerability_id", flat=True)) self.assertEqual(set(new_vulnerability_ids), set(vuln_ids)) - finding.delete() + + +class ReimportDuplicateReactivationTest(DojoTestCase): + + """ + Regression test for https://github.com/DefectDojo/django-DefectDojo/issues/14910 + + Reimport reactivation of a mitigated finding must not produce an invalid + active/verified duplicate finding state. + """ + + def setUp(self): + self.user, _ = User.objects.get_or_create(username="admin", is_superuser=True) + Development_Environment.objects.get_or_create(name="Development") + self.product_type, _ = Product_Type.objects.get_or_create(name="dup_reactivation_pt") + self.product, _ = Product.objects.get_or_create( + name="DupReactivationProduct", + description="test product", + prod_type=self.product_type, + ) + self.engagement = Engagement.objects.create( + name="Dup Reactivation Engagement", + product=self.product, + target_start=timezone.now(), + target_end=timezone.now(), + ) + self.test = self.create_test(engagement=self.engagement, scan_type=NPM_AUDIT_SCAN_TYPE, title="dup reactivation test") + + def _make_finding(self, title, **kwargs): + return Finding.objects.create( + title=title, + test=self.test, + severity="High", + reporter=self.user, + **kwargs, + ) + + def test_reactivation_keeps_duplicate_inactive_and_unverified(self): + # Original active finding + original = self._make_finding("original finding", active=True, verified=True) + # Mitigated finding that is marked as a duplicate of the original + existing_duplicate = self._make_finding( + "duplicate finding", + active=False, + verified=False, + duplicate=True, + duplicate_finding=original, + is_mitigated=True, + mitigated=timezone.now(), + mitigated_by=self.user, + ) + # The reimported (unsaved) finding that re-matches the duplicate, and is active/not mitigated + unsaved_finding = self._make_finding("duplicate finding incoming", active=True, verified=True) + + reimporter = DefaultReImporter( + test=self.test, + user=self.user, + scan_type=NPM_AUDIT_SCAN_TYPE, + active=True, + verified=True, + do_not_reactivate=False, + ) + # These accumulators are normally initialised inside process_findings(); set them + # here because the test drives process_matched_mitigated_finding() directly. + reimporter.new_items = [] + reimporter.reactivated_items = [] + reimporter.unchanged_items = [] + + result_finding, _ = reimporter.process_matched_mitigated_finding(unsaved_finding, existing_duplicate) + + result_finding.refresh_from_db() + # The mitigation is cleared (the finding reappeared in the scan)... + self.assertFalse(result_finding.is_mitigated) + self.assertIsNone(result_finding.mitigated) + # ...but a duplicate must never become active or verified (issue #14910) + self.assertTrue(result_finding.duplicate) + self.assertFalse(result_finding.active) + self.assertFalse(result_finding.verified) + + def test_reactivation_of_non_duplicate_still_activates(self): + # A regular mitigated finding (not a duplicate) must still reactivate as before + existing = self._make_finding( + "regular finding", + active=False, + verified=False, + is_mitigated=True, + mitigated=timezone.now(), + mitigated_by=self.user, + ) + unsaved_finding = self._make_finding("regular finding incoming", active=True, verified=True) + + reimporter = DefaultReImporter( + test=self.test, + user=self.user, + scan_type=NPM_AUDIT_SCAN_TYPE, + active=True, + verified=True, + do_not_reactivate=False, + ) + # These accumulators are normally initialised inside process_findings(); set them + # here because the test drives process_matched_mitigated_finding() directly. + reimporter.new_items = [] + reimporter.reactivated_items = [] + reimporter.unchanged_items = [] + + result_finding, _ = reimporter.process_matched_mitigated_finding(unsaved_finding, existing) + + result_finding.refresh_from_db() + self.assertFalse(result_finding.is_mitigated) + self.assertIsNone(result_finding.mitigated) + self.assertTrue(result_finding.active) + self.assertTrue(result_finding.verified) From 75e7834824f9f1cabf5be0f031936aeadc7c9e1c Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Fri, 5 Jun 2026 04:49:24 +0200 Subject: [PATCH 5/9] fix(dependency_check): fold related dependency paths into description (#14941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dependency-Check's DependencyBundlingAnalyzer merges co-grouped artifacts into one main dependency and lists the others under . The vulnerability is attached only to the main dependency in the XML; related entries are metadata for other files in the same logical component. Previously the parser emitted one finding per related entry in addition to the main finding. This multiplied a single CVE into N findings sharing the same title, CVE, component name, and version — only the file path differed. Projects with Spring Boot, ActiveMQ, or other libraries whose CPE matches many sibling artifacts (DC bundling scenario 4) were hit hardest. Instead, emit one finding per vulnerability per main dependency and surface related file paths in the description under a "**Related Filepaths:**" block. The five DependencyBundlingAnalyzer bundling scenarios are documented in the new build_related_dependencies_block() helper, the parser docs page, and the 2.59.1 upgrade notes. Closes-style note: findings previously tagged `related` will be closed on the next reimport as they are no longer emitted. The `related` tag is not applied. --- docs/content/releases/os_upgrading/2.59.1.md | 32 +++ .../parsers/file/dependency_check.md | 16 +- dojo/tools/dependency_check/parser.py | 75 ++++-- .../tools/test_dependency_check_parser.py | 220 +++++++----------- 4 files changed, 193 insertions(+), 150 deletions(-) create mode 100644 docs/content/releases/os_upgrading/2.59.1.md diff --git a/docs/content/releases/os_upgrading/2.59.1.md b/docs/content/releases/os_upgrading/2.59.1.md new file mode 100644 index 00000000000..701fe466513 --- /dev/null +++ b/docs/content/releases/os_upgrading/2.59.1.md @@ -0,0 +1,32 @@ +--- +title: 'Upgrading to DefectDojo Version 2.59.1' +toc_hide: true +weight: -20260603 +description: Dependency Check parser no longer emits separate findings for related dependencies; related file paths are now listed in the main finding's description. +--- + +## Dependency Check parser: related dependencies folded into the main finding + +The Dependency Check parser previously created one finding per `` in the report, in addition to the finding for the main vulnerable dependency. Because OWASP Dependency-Check attaches the vulnerability only to the main dependency in the XML and the related entries are metadata pointing to other files in the same logical component, this produced N findings sharing the same title, CVE, component name and component version — only the file path differed. For projects with Spring Boot, ActiveMQ, or other libraries whose CPE matches many sibling artifacts this produced significant noise. + +Starting in 2.59.1, the parser emits exactly one finding per vulnerability per main dependency. The file paths of any related dependencies are surfaced in the finding description under a `**Related Filepaths:**` block. + +### Background: what `` actually contains + +OWASP Dependency-Check's `DependencyBundlingAnalyzer` merges co-grouped artifacts into a single main dependency and lists the others under ``. It does this under five scenarios: + +1. **Identical content (`hashesMatch`)** — the same jar (matching sha1) found at multiple paths, e.g. the same library packaged into multiple ear/war archives. +2. **Shaded jar (`isShadedJar`)** — a `.jar` and a `pom.xml` extracted from inside it share the same CPE; the pom.xml is recorded as related. +3. **WebJar (`isWebJar`)** — a `.js` file extracted from a WebJar matches the jar's CPE (mapped via `pkg:maven/org.webjars/@`); the js file is recorded as related to the jar. +4. **Same CPE + base path + vulnerabilities + filename match** — sibling artifacts in the same project that share a CPE. Example: `spring-boot`, `spring-boot-actuator`, `spring-boot-starter-jdbc`, etc. all map to the `spring_boot` CPE and are grouped under the main `spring-boot` jar. +5. **NPM same name + version** — the same npm package discovered via different resolution paths (for example `package-lock.json` plus `node_modules`). + +Only scenario 1 represents the same vulnerable artifact at multiple deploy locations. Scenarios 2-5 are different files representing one logical component. Both cases were previously inflated into separate findings; both now collapse to one finding with the related paths listed in the description. + +### Required actions + +- **Users filtering or grouping by the `related` tag**: that tag is no longer applied because related findings are no longer created. Update any saved filters, dashboards, or rules that depend on it. Equivalent information is now available in the finding description (look for `**Related Filepaths:**`). +- **Reimport behavior**: on the next reimport of an existing Dependency Check report, the previously-created `related` findings will be closed as no longer present in the report. This is expected and matches the new parsing behavior. + + +For more information, check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/2.59.1). diff --git a/docs/content/supported_tools/parsers/file/dependency_check.md b/docs/content/supported_tools/parsers/file/dependency_check.md index f4f56ce8325..76d51277801 100644 --- a/docs/content/supported_tools/parsers/file/dependency_check.md +++ b/docs/content/supported_tools/parsers/file/dependency_check.md @@ -7,7 +7,21 @@ OWASP Dependency Check output can be imported in Xml format. This parser ingests * Suppressed vulnerabilities are tagged with the tag: `suppressed`. * Suppressed vulnerabilities are marked as mitigated. * If the suppression is missing any `` tag, it tags them as `no_suppression_document`. -* Related vulnerable dependencies are tagged with `related` tag. + +### Related dependencies + +OWASP Dependency-Check's `DependencyBundlingAnalyzer` merges co-grouped artifacts into a single main dependency and lists the others under `` in the report. The vulnerability is attached only to the main dependency; the related entries are metadata pointing to other files in the same logical component. + +The parser emits one finding per vulnerability per main dependency and surfaces the related file paths in the finding's description under a `**Related Filepaths:**` block (rather than creating a separate finding per related entry, which produced N findings sharing the same title and CVE differing only in file path). + +DC bundles dependencies under five scenarios: + +1. **Identical content (`hashesMatch`)** — the same jar (matching sha1) found at multiple paths (e.g. packaged into multiple ear/war archives). +2. **Shaded jar (`isShadedJar`)** — a `.jar` and a `pom.xml` extracted from inside it share the same CPE. +3. **WebJar (`isWebJar`)** — a `.js` file extracted from a WebJar maps to the jar's CPE via `pkg:maven/org.webjars/@`. +4. **Same CPE + base path + vulnerabilities + filename match** — sibling artifacts sharing a CPE (e.g. `spring-boot`, `spring-boot-actuator`, `spring-boot-starter-jdbc` all map to the `spring_boot` CPE). +5. **NPM same name + version** — the same npm package discovered via different resolution paths (e.g. `package-lock.json` + `node_modules`). + ### Sample Scan Data Sample Dependency Check scans can be found [here](https://github.com/DefectDojo/django-DefectDojo/tree/master/unittests/scans/dependency_check). diff --git a/dojo/tools/dependency_check/parser.py b/dojo/tools/dependency_check/parser.py index 4c472e2f4c4..03590bdd02d 100644 --- a/dojo/tools/dependency_check/parser.py +++ b/dojo/tools/dependency_check/parser.py @@ -86,6 +86,57 @@ def add_finding(self, finding, dupes): if key not in dupes: dupes[key] = finding + def build_related_dependencies_block(self, dependency, namespace): + """ + Return a markdown block listing related dependencies, or ''. + + Dependency-Check's DependencyBundlingAnalyzer merges co-grouped artifacts + into one main dependency and lists the others under . + The vulnerability is attached only to the main dependency in the XML; the + related entries are metadata pointing to other files in the same logical + component. Previously we emitted one finding per related entry, which + multiplied a single CVE into N findings sharing the same title and CVE + with only the file_path differing — pure noise for the user. Instead we + keep one finding for the main dependency and surface the related file + paths in its description. + + DC bundles dependencies under five scenarios (see + DependencyBundlingAnalyzer.evaluateDependencies): + + 1. hashesMatch — identical content (same sha1) found at multiple paths, + e.g. the same jar packaged into multiple ear/war archives. + 2. isShadedJar — a .jar and a pom.xml extracted from inside it share the + same CPE; the pom.xml is recorded as related to the jar. + 3. isWebJar — a .js file extracted from a WebJar matches the jar's CPE + (mapped via pkg:maven/org.webjars/@); the js is + related to the jar. + 4. CPE + base path + vulnerabilities + filename match — sibling artifacts + in the same project that share a CPE (e.g. spring-boot, + spring-boot-actuator, spring-boot-starter all map to the + spring_boot CPE). + 5. NPM same name + version — the same npm package discovered via + different resolution paths (e.g. package-lock.json + node_modules). + + Scenario 1 is the only case where the related entries are genuinely + separate vulnerable locations; scenarios 2-5 are different files + representing one logical component. Listing all related paths in the + description preserves the per-location information for scenario 1 while + removing the noise from scenarios 2-5. + """ + related = dependency.find(namespace + "relatedDependencies") + if related is None: + return "" + entries = [] + for rd in related.findall(namespace + "relatedDependency"): + file_name = rd.findtext(f"{namespace}fileName") + if not file_name: + continue + file_path = (rd.findtext(f"{namespace}filePath") or "").strip() + entries.append(f"- {file_name} ({file_path})" if file_path else f"- {file_name}") + if not entries: + return "" + return "\n**Related Filepaths:**\n" + "\n".join(entries) + def get_filename_and_path_from_dependency( self, dependency, @@ -448,6 +499,7 @@ def get_findings(self, filename, test): namespace + "vulnerabilities", ) if vulnerabilities is not None: + related_block = self.build_related_dependencies_block(dependency, namespace) for vulnerability in vulnerabilities.findall( namespace + "vulnerability", ): @@ -459,29 +511,12 @@ def get_findings(self, filename, test): test, namespace, ) + if related_block: + finding.description += related_block if scan_date: finding.date = scan_date self.add_finding(finding, dupes) - relatedDependencies = dependency.find( - namespace + "relatedDependencies", - ) - if relatedDependencies is not None: - for relatedDependency in relatedDependencies.findall( - namespace + "relatedDependency", - ): - finding = self.get_finding_from_vulnerability( - dependency, - relatedDependency, - vulnerability, - test, - namespace, - ) - if finding: # could be None - if scan_date: - finding.date = scan_date - self.add_finding(finding, dupes) - for suppressedVulnerability in vulnerabilities.findall( namespace + "suppressedVulnerability", ): @@ -493,6 +528,8 @@ def get_findings(self, filename, test): test, namespace, ) + if related_block: + finding.description += related_block if scan_date: finding.date = scan_date self.add_finding(finding, dupes) diff --git a/unittests/tools/test_dependency_check_parser.py b/unittests/tools/test_dependency_check_parser.py index 31e1394ec51..67349f25871 100644 --- a/unittests/tools/test_dependency_check_parser.py +++ b/unittests/tools/test_dependency_check_parser.py @@ -58,11 +58,11 @@ def test_parse_file_with_multiple_vulnerabilities_has_multiple_findings(self): parser = DependencyCheckParser() findings = parser.get_findings(testfile, Test()) items = findings - self.assertEqual(11, len(items)) + self.assertEqual(9, len(items)) # test also different component_name formats with self.subTest(i=0): - # identifier -> package url java + 2 relateddependencies + # identifier -> package url java + 2 relateddependencies (now folded into description) self.assertEqual(items[0].title, "org.dom4j:dom4j:2.1.1.redhat-00001 | CVE-0000-0001") self.assertEqual(items[0].component_name, "org.dom4j:dom4j") self.assertEqual(items[0].component_version, "2.1.1.redhat-00001") @@ -74,6 +74,15 @@ def test_parse_file_with_multiple_vulnerabilities_has_multiple_findings(self): "/var/lib/adapter-ear1.ear/dom4j-2.1.1.jar", items[0].description, ) + self.assertIn("**Related Filepaths:**", items[0].description) + self.assertIn( + "adapter-ear8.ear: dom4j-2.1.1.jar (/var/lib/adapter-ear8.ear/dom4j-2.1.1.jar)", + items[0].description, + ) + self.assertIn( + "adapter-ear1.ear: dom4j-extensions-2.1.1.jar (/var/lib/adapter-ear1.ear/dom4j-extensions-2.1.1.jar)", + items[0].description, + ) self.assertEqual(items[0].severity, "High") self.assertEqual(items[0].cvssv3, None) self.assertEqual(items[0].cvssv3_score, None) @@ -89,192 +98,143 @@ def test_parse_file_with_multiple_vulnerabilities_has_multiple_findings(self): self.assertEqual("CVE-0000-0001", items[0].unsaved_vulnerability_ids[0]) with self.subTest(i=1): - self.assertEqual(items[1].title, "org.dom4j:dom4j:2.1.1.redhat-00001 | CVE-0000-0001") - self.assertEqual(items[1].component_name, "org.dom4j:dom4j") - self.assertEqual(items[1].component_version, "2.1.1.redhat-00001") - self.assertIn( - "Description of a bad vulnerability.", - items[1].description, - ) - self.assertIn( - "/var/lib/adapter-ear8.ear/dom4j-2.1.1.jar", - items[1].description, + # identifier -> package url javascript, no vulnerabilitids, 3 vulnerabilities, relateddependencies without filename (pre v6.0.0) + self.assertEqual( + items[1].title, "yargs-parser:5.0.0 | 1500", ) - self.assertEqual(items[1].severity, "High") + self.assertEqual(items[1].component_name, "yargs-parser") + self.assertEqual(items[1].component_version, "5.0.0") + self.assertEqual(items[1].severity, "Low") self.assertEqual(items[1].cvssv3, None) self.assertEqual(items[1].cvssv3_score, None) - self.assertEqual(items[1].file_path, "adapter-ear8.ear: dom4j-2.1.1.jar") + self.assertEqual(items[1].file_path, "yargs-parser:5.0.0") self.assertEqual( - items[1].mitigation, - "Update org.dom4j:dom4j:2.1.1.redhat-00001 to at least the version recommended in the description", - ) - self.assertEqual(items[1].unsaved_tags, ["related"]) - self.assertEqual(1, len(items[1].unsaved_vulnerability_ids)) - self.assertEqual("CVE-0000-0001", items[1].unsaved_vulnerability_ids[0]) - - with self.subTest(i=2): - self.assertEqual(items[2].title, "org.dom4j:dom4j:2.1.1.redhat-00001 | CVE-0000-0001") - self.assertEqual(items[2].component_name, "org.dom4j:dom4j") - self.assertEqual(items[2].component_version, "2.1.1.redhat-00001") - self.assertIn( - "Description of a bad vulnerability.", - items[2].description, - ) - self.assertIn( - "/var/lib/adapter-ear1.ear/dom4j-extensions-2.1.1.jar", - items[2].description, - ) - self.assertEqual(items[2].severity, "High") - self.assertEqual(items[2].cvssv3, None) - self.assertEqual(items[2].cvssv3_score, None) - self.assertEqual(items[2].file_path, "adapter-ear1.ear: dom4j-extensions-2.1.1.jar") - self.assertEqual( - items[2].mitigation, - "Update org.dom4j:dom4j:2.1.1.redhat-00001 to at least the version recommended in the description", - ) - self.assertEqual(1, len(items[2].unsaved_vulnerability_ids)) - self.assertEqual("CVE-0000-0001", items[2].unsaved_vulnerability_ids[0]) - - with self.subTest(i=3): - # identifier -> package url javascript, no vulnerabilitids, 3 vulnerabilities, relateddependencies without filename (pre v6.0.0) - self.assertEqual( - items[3].title, "yargs-parser:5.0.0 | 1500", - ) - self.assertEqual(items[3].component_name, "yargs-parser") - self.assertEqual(items[3].component_version, "5.0.0") - # assert fails due to special characters, not too important - # self.assertEqual(items[1].description, "Affected versions of `yargs-parser` are vulnerable to prototype pollution. Arguments are not properly sanitized, allowing an attacker to modify the prototype of `Object`, causing the addition or modification of an existing property that will exist on all objects.Parsing the argument `--foo.__proto__.bar baz'` adds a `bar` property with value `baz` to all objects. This is only exploitable if attackers have control over the arguments being passed to `yargs-parser`.") - self.assertEqual(items[3].severity, "Low") - self.assertEqual(items[3].cvssv3, None) - self.assertEqual(items[3].cvssv3_score, None) - self.assertEqual(items[3].file_path, "yargs-parser:5.0.0") - self.assertEqual( - items[3].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", + items[1].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", ) self.assertIn( "**Source:** NPM", - items[3].description, + items[1].description, ) - self.assertIsNone(items[3].unsaved_vulnerability_ids) + self.assertIsNone(items[1].unsaved_vulnerability_ids) - with self.subTest(i=4): + with self.subTest(i=2): self.assertEqual( - items[4].title, + items[2].title, "yargs-parser:5.0.0 | CVE-2020-7608", ) - self.assertEqual(items[4].component_name, "yargs-parser") - self.assertEqual(items[4].component_version, "5.0.0") + self.assertEqual(items[2].component_name, "yargs-parser") + self.assertEqual(items[2].component_version, "5.0.0") self.assertIn( 'yargs-parser could be tricked into adding or modifying properties\n of Object.prototype using a "__proto__" payload.\n**Source:** OSSINDEX\n**Filepath:** \n /var/lib/jenkins/workspace/nl-selfservice_-_metrics_develop/package-lock.json?yargs-parser', - items[4].description, + items[2].description, ) self.assertIn( "/var/lib/jenkins/workspace/nl-selfservice_-_metrics_develop/package-lock.json?yargs-parser", - items[4].description, + items[2].description, ) - self.assertEqual(items[4].severity, "High") - self.assertEqual(items[4].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N") - self.assertEqual(items[4].cvssv3_score, 7.5) - self.assertEqual(items[4].file_path, "yargs-parser:5.0.0") + self.assertEqual(items[2].severity, "High") + self.assertEqual(items[2].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N") + self.assertEqual(items[2].cvssv3_score, 7.5) + self.assertEqual(items[2].file_path, "yargs-parser:5.0.0") self.assertEqual( - items[4].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", + items[2].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", ) - self.assertEqual(1, len(items[4].unsaved_vulnerability_ids)) - self.assertEqual("CVE-2020-7608", items[4].unsaved_vulnerability_ids[0]) + self.assertEqual(1, len(items[2].unsaved_vulnerability_ids)) + self.assertEqual("CVE-2020-7608", items[2].unsaved_vulnerability_ids[0]) - with self.subTest(i=5): + with self.subTest(i=3): self.assertEqual( - items[5].title, + items[3].title, "yargs-parser:5.0.0 | CWE-400: Uncontrolled Resource Consumption ('Resource Exhaustion')", ) - self.assertEqual(items[5].component_name, "yargs-parser") - self.assertEqual(items[5].component_version, "5.0.0") + self.assertEqual(items[3].component_name, "yargs-parser") + self.assertEqual(items[3].component_version, "5.0.0") self.assertIn( "The software does not properly restrict the size or amount of resources that are requested or influenced by an actor, which can be used to consume more resources than intended.", - items[5].description, + items[3].description, ) # check that the filepath is in the description self.assertIn( "/var/lib/jenkins/workspace/nl-selfservice_-_metrics_develop/package-lock.json?yargs-parser", - items[5].description, + items[3].description, ) - self.assertEqual(items[5].severity, "High") - self.assertEqual(items[5].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H") - self.assertEqual(items[5].cvssv3_score, 7.5) - self.assertEqual(items[5].file_path, "yargs-parser:5.0.0") + self.assertEqual(items[3].severity, "High") + self.assertEqual(items[3].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H") + self.assertEqual(items[3].cvssv3_score, 7.5) + self.assertEqual(items[3].file_path, "yargs-parser:5.0.0") self.assertEqual( - items[5].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", + items[3].mitigation, "Update yargs-parser:5.0.0 to at least the version recommended in the description", ) - self.assertIsNone(items[5].unsaved_vulnerability_ids) + self.assertIsNone(items[3].unsaved_vulnerability_ids) - with self.subTest(i=6): + with self.subTest(i=4): # identifier -> cpe java - self.assertEqual(items[6].title, "org.dom4j:dom4j:2.1.1.redhat-00001 | CVE-0000-0001") - self.assertEqual(items[6].component_name, "org.dom4j:dom4j") - self.assertEqual(items[6].component_version, "2.1.1.redhat-00001") - self.assertEqual(items[6].severity, "High") - self.assertEqual(items[6].cvssv3, None) - self.assertEqual(items[6].cvssv3_score, None) - self.assertEqual(items[6].file_path, "adapter-ear2.ear: dom4j-2.1.1.jar") + self.assertEqual(items[4].title, "org.dom4j:dom4j:2.1.1.redhat-00001 | CVE-0000-0001") + self.assertEqual(items[4].component_name, "org.dom4j:dom4j") + self.assertEqual(items[4].component_version, "2.1.1.redhat-00001") + self.assertEqual(items[4].severity, "High") + self.assertEqual(items[4].cvssv3, None) + self.assertEqual(items[4].cvssv3_score, None) + self.assertEqual(items[4].file_path, "adapter-ear2.ear: dom4j-2.1.1.jar") self.assertEqual( - items[6].mitigation, + items[4].mitigation, "Update org.dom4j:dom4j:2.1.1.redhat-00001 to at least the version recommended in the description", ) - self.assertEqual(1, len(items[6].unsaved_vulnerability_ids)) - self.assertEqual("CVE-0000-0001", items[6].unsaved_vulnerability_ids[0]) + self.assertEqual(1, len(items[4].unsaved_vulnerability_ids)) + self.assertEqual("CVE-0000-0001", items[4].unsaved_vulnerability_ids[0]) - with self.subTest(i=7): + with self.subTest(i=5): # identifier -> maven java - self.assertEqual(items[7].title, "dom4j:2.1.1 | CVE-0000-0001") - self.assertEqual(items[7].component_name, "dom4j") - self.assertEqual(items[7].component_version, "2.1.1") - self.assertEqual(items[7].severity, "High") - self.assertEqual(items[7].cvssv3, None) - self.assertEqual(items[7].cvssv3_score, None) + self.assertEqual(items[5].title, "dom4j:2.1.1 | CVE-0000-0001") + self.assertEqual(items[5].component_name, "dom4j") + self.assertEqual(items[5].component_version, "2.1.1") + self.assertEqual(items[5].severity, "High") + self.assertEqual(items[5].cvssv3, None) + self.assertEqual(items[5].cvssv3_score, None) self.assertEqual( - items[7].mitigation, "Update dom4j:2.1.1 to at least the version recommended in the description", + items[5].mitigation, "Update dom4j:2.1.1 to at least the version recommended in the description", ) - with self.subTest(i=8): + with self.subTest(i=6): # evidencecollected -> single product + single verison javascript self.assertEqual( - items[8].title, + items[6].title, "jquery:3.1.1 | CVE-0000-0001", ) - self.assertEqual(items[8].component_name, "jquery") - self.assertEqual(items[8].component_version, "3.1.1") - self.assertEqual(items[8].severity, "High") - self.assertEqual(items[8].cvssv3, None) - self.assertEqual(items[8].cvssv3_score, None) + self.assertEqual(items[6].component_name, "jquery") + self.assertEqual(items[6].component_version, "3.1.1") + self.assertEqual(items[6].severity, "High") + self.assertEqual(items[6].cvssv3, None) + self.assertEqual(items[6].cvssv3_score, None) self.assertEqual( - items[8].mitigation, "Update jquery:3.1.1 to at least the version recommended in the description", + items[6].mitigation, "Update jquery:3.1.1 to at least the version recommended in the description", ) - with self.subTest(i=9): + with self.subTest(i=7): # Tests for two suppressed vulnerabilities, # One for Suppressed with notes, the other is without. - self.assertEqual(items[9].active, False) + self.assertEqual(items[7].active, False) self.assertEqual( - items[9].mitigation, + items[7].mitigation, "**This vulnerability is mitigated and/or suppressed:** Document on why we are suppressing this vulnerability is missing!\nUpdate jquery:3.1.1 to at least the version recommended in the description", ) - self.assertEqual(items[9].unsaved_tags, ["no_suppression_document", "suppressed"]) - self.assertEqual(items[9].severity, "Critical") - self.assertEqual(items[9].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H") - self.assertEqual(items[9].cvssv3_score, 9.8) - self.assertEqual(items[9].is_mitigated, True) + self.assertEqual(items[7].unsaved_tags, ["no_suppression_document", "suppressed"]) + self.assertEqual(items[7].severity, "Critical") + self.assertEqual(items[7].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H") + self.assertEqual(items[7].cvssv3_score, 9.8) + self.assertEqual(items[7].is_mitigated, True) - with self.subTest(i=10): - self.assertEqual(items[10].active, False) + with self.subTest(i=8): + self.assertEqual(items[8].active, False) self.assertEqual( - items[10].mitigation, + items[8].mitigation, "**This vulnerability is mitigated and/or suppressed:** This is our reason for not to upgrade it.\nUpdate jquery:3.1.1 to at least the version recommended in the description", ) - self.assertEqual(items[10].unsaved_tags, ["suppressed"]) - self.assertEqual(items[10].severity, "Critical") - self.assertEqual(items[10].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H") - self.assertEqual(items[10].cvssv3_score, 9.8) - self.assertEqual(items[10].is_mitigated, True) + self.assertEqual(items[8].unsaved_tags, ["suppressed"]) + self.assertEqual(items[8].severity, "Critical") + self.assertEqual(items[8].cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H") + self.assertEqual(items[8].cvssv3_score, 9.8) + self.assertEqual(items[8].is_mitigated, True) def test_parse_java_6_5_3(self): """Test with version 6.5.3""" @@ -303,7 +263,7 @@ def test_parse_file_pr6439(self): parser = DependencyCheckParser() findings = parser.get_findings(testfile, Test()) items = findings - self.assertEqual(37, len(items)) + self.assertEqual(34, len(items)) # test also different component_name formats with self.subTest(i=0): From 51f926156f330c9a6319cae636a0b6e7e428e3c2 Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Fri, 5 Jun 2026 04:49:45 +0200 Subject: [PATCH 6/9] fix: guard filter snippet include when no form passed to metrics template (#14945) critical_product_metrics view renders metrics.html without a form context variable. Django resolves undefined template vars as empty string, causing get_filter_groups to crash with AttributeError on str.visible_fields(). Wrapping the filter_snippet include in {% if form %} prevents the crash. Fixes #14944. --- dojo/templates/dojo/metrics.html | 2 ++ dojo/templates_classic/dojo/metrics.html | 2 ++ 2 files changed, 4 insertions(+) diff --git a/dojo/templates/dojo/metrics.html b/dojo/templates/dojo/metrics.html index 6e56099463f..2c7ab40fbf6 100644 --- a/dojo/templates/dojo/metrics.html +++ b/dojo/templates/dojo/metrics.html @@ -147,9 +147,11 @@

+ {% if form %}
{% include "dojo/filter_snippet.html" with form=form clear_link="/metrics/product/type" %}
+ {% endif %} diff --git a/dojo/templates_classic/dojo/metrics.html b/dojo/templates_classic/dojo/metrics.html index e011fc21055..777467e0e59 100644 --- a/dojo/templates_classic/dojo/metrics.html +++ b/dojo/templates_classic/dojo/metrics.html @@ -155,9 +155,11 @@

+ {% if form %}
{% include "dojo/filter_snippet.html" with form=form clear_link="/metrics/product/type" %}
+ {% endif %} From 68a272f299d096249fd3ba9c2676bf69012857bf Mon Sep 17 00:00:00 2001 From: dogboat Date: Thu, 4 Jun 2026 22:51:41 -0400 Subject: [PATCH 7/9] Fix for GHSA-w2j3-x3j3-mm43 (#14952) * prevent non-superusers from setting is_staff on a user * tighten /admin/ access to superusers only * linter fixes * disable django admin panel by default --- dojo/admin.py | 14 ++++ dojo/api_v2/serializers.py | 6 ++ dojo/settings/settings.dist.py | 2 +- dojo/settings/template-env | 5 +- unittests/test_adminsite.py | 56 ++++++++++++++++ unittests/test_apiv2_user.py | 113 +++++++++++++++++++++++++++++++++ 6 files changed, 193 insertions(+), 3 deletions(-) diff --git a/dojo/admin.py b/dojo/admin.py index c7a21b91019..94d149af035 100644 --- a/dojo/admin.py +++ b/dojo/admin.py @@ -14,6 +14,20 @@ TextQuestion, ) + +# Django's default admin gate is `is_active and is_staff`. Under the legacy +# OS auth model is_staff is already a near-superuser bypass for queryset +# filters and most permission checks, so the standard UserAdmin change form +# would let any is_staff user with auth.change_user tick is_superuser on +# themselves. Rather than narrowing each ModelAdmin individually, restrict +# the entire admin site to superusers — there is no DefectDojo OS role that +# legitimately needs /admin/ access without being a superuser. +def _admin_site_has_permission(request): + return request.user.is_active and request.user.is_superuser + + +admin.site.has_permission = _admin_site_has_permission + # Conditionally unregister LogEntry from auditlog if it's registered try: from auditlog.models import LogEntry diff --git a/dojo/api_v2/serializers.py b/dojo/api_v2/serializers.py index dc15ac6c2dc..b3b83a239b5 100644 --- a/dojo/api_v2/serializers.py +++ b/dojo/api_v2/serializers.py @@ -613,6 +613,12 @@ def validate(self, data): msg = "Only superusers are allowed to add or edit superusers." raise ValidationError(msg) + instance_is_staff = self.instance.is_staff if self.instance is not None else False + data_is_staff = data.get("is_staff", instance_is_staff) + if not self.context["request"].user.is_superuser and data_is_staff != instance_is_staff: + msg = "Only superusers are allowed to add or edit staff users." + raise ValidationError(msg) + if self.context["request"].method in {"PATCH", "PUT"} and "password" in data: msg = "Update of password though API is not allowed" raise ValidationError(msg) diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index f99e62eca2e..4409fcf44ee 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -51,7 +51,7 @@ DD_DJANGO_METRICS_ENABLED=(bool, False), DD_LOGIN_REDIRECT_URL=(str, "/"), DD_LOGIN_URL=(str, "/login"), - DD_DJANGO_ADMIN_ENABLED=(bool, True), + DD_DJANGO_ADMIN_ENABLED=(bool, False), DD_SESSION_COOKIE_HTTPONLY=(bool, True), DD_CSRF_COOKIE_HTTPONLY=(bool, True), DD_SECURE_SSL_REDIRECT=(bool, False), diff --git a/dojo/settings/template-env b/dojo/settings/template-env index bbfc9e90ad9..0263dc941a7 100644 --- a/dojo/settings/template-env +++ b/dojo/settings/template-env @@ -1,8 +1,9 @@ # Django Debug, don't enable on production! DD_DEBUG=False -# Enables Django Admin -DD_DJANGO_ADMIN_ENABLED=True +# Enables the Django admin interface at /admin/. Off by default; uncomment +# this line to expose it. Access is restricted to superusers regardless. +# DD_DJANGO_ADMIN_ENABLED=True # A secret key for a particular Django installation. DD_SECRET_KEY=#DD_SECRET_KEY# diff --git a/unittests/test_adminsite.py b/unittests/test_adminsite.py index 015c65ee467..70f08da8df2 100644 --- a/unittests/test_adminsite.py +++ b/unittests/test_adminsite.py @@ -1,5 +1,8 @@ import django.apps from django.contrib import admin +from django.contrib.auth import get_user_model +from django.contrib.auth.models import AnonymousUser +from django.http import HttpRequest from .dojo_test_case import DojoTestCase @@ -20,3 +23,56 @@ def test_is_model_defined(self): else: with self.subTest(type="tag", subclass=subclass): self.assertIn(subclass, admin.site._registry.keys(), f"{subclass} is not registered in 'tagulous.admin' in models.py") + + +class AdminAccessGate(DojoTestCase): + + """ + is_staff is a near-superuser bypass under the legacy OS auth model, so + /admin/ must require is_superuser. Django's default UserAdmin change + form would otherwise let any is_staff user with auth.change_user tick + is_superuser on themselves. Tested at the gate-function level so the + assertions hold regardless of whether DD_DJANGO_ADMIN_ENABLED mounts + the admin URLConf in the current environment. + """ + + @staticmethod + def _request_for(user): + req = HttpRequest() + req.user = user + return req + + def test_staff_non_superuser_denied(self): + User = get_user_model() + password = "testTEST1234!@#$" + staff = User.objects.create_user( + username="staff-no-root", password=password, is_staff=True, + ) + self.assertFalse(admin.site.has_permission(self._request_for(staff))) + + def test_non_staff_non_superuser_denied(self): + User = get_user_model() + password = "testTEST1234!@#$" + plain = User.objects.create_user(username="plain-user", password=password) + self.assertFalse(admin.site.has_permission(self._request_for(plain))) + + def test_anonymous_denied(self): + self.assertFalse(admin.site.has_permission(self._request_for(AnonymousUser()))) + + def test_inactive_superuser_denied(self): + User = get_user_model() + password = "testTEST1234!@#$" + root = User.objects.create_superuser( + username="inactive-root", email="i@example.com", password=password, + ) + root.is_active = False + root.save(update_fields=["is_active"]) + self.assertFalse(admin.site.has_permission(self._request_for(root))) + + def test_active_superuser_allowed(self): + User = get_user_model() + password = "testTEST1234!@#$" + root = User.objects.create_superuser( + username="root-test", email="r@example.com", password=password, + ) + self.assertTrue(admin.site.has_permission(self._request_for(root))) diff --git a/unittests/test_apiv2_user.py b/unittests/test_apiv2_user.py index afce21948f2..f6273fe9b47 100644 --- a/unittests/test_apiv2_user.py +++ b/unittests/test_apiv2_user.py @@ -1,3 +1,4 @@ +from django.contrib.auth.models import Permission from django.urls import reverse from django.utils import timezone from rest_framework.authtoken.models import Token @@ -201,6 +202,118 @@ def test_user_reset_api_token_denies_non_privileged(self): r = nonpriv_client.post(url) self.assertEqual(r.status_code, 403, r.content[:1000]) + def test_non_superuser_cannot_set_is_staff_via_api(self): + """ + A delegated user-manager (auth.change_user) must not be able to + flip is_staff on themselves or anyone else — is_staff is a + superuser-only flag under the legacy OS auth model, and granting + it via API would let a non-superuser pivot into Django admin / + full RBAC bypass. + """ + password = "testTEST1234!@#$" + r = self.client.post(reverse("user-list"), { + "username": "api-user-mgr", + "email": "admin@dojo.com", + "password": password, + }, format="json") + self.assertEqual(r.status_code, 201, r.content[:1000]) + mgr = User.objects.get(username="api-user-mgr") + mgr.user_permissions.add( + Permission.objects.get(codename="change_user"), + Permission.objects.get(codename="add_user"), + ) + + token_resp = self.client.post(reverse("api-token-auth"), { + "username": "api-user-mgr", + "password": password, + }, format="json") + self.assertEqual(token_resp.status_code, 200, token_resp.content[:1000]) + mgr_client = APIClient() + mgr_client.credentials(HTTP_AUTHORIZATION="Token " + token_resp.json()["token"]) + + # Self-escalation: setting is_staff on own account must be rejected. + r = mgr_client.patch("{}{}/".format(reverse("user-list"), mgr.id), { + "is_staff": True, + }, format="json") + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.assertIn( + "Only superusers are allowed to add or edit staff users.", + r.content.decode("utf-8"), + ) + mgr.refresh_from_db() + self.assertFalse(mgr.is_staff) + + # Target-escalation: setting is_staff on another user must be rejected. + r = self.client.post(reverse("user-list"), { + "username": "api-user-target", + "email": "admin@dojo.com", + "password": password, + }, format="json") + self.assertEqual(r.status_code, 201, r.content[:1000]) + target_id = r.json()["id"] + + r = mgr_client.patch("{}{}/".format(reverse("user-list"), target_id), { + "is_staff": True, + }, format="json") + self.assertEqual(r.status_code, 400, r.content[:1000]) + target = User.objects.get(id=target_id) + self.assertFalse(target.is_staff) + + # Create-time escalation must also be rejected. + r = mgr_client.post(reverse("user-list"), { + "username": "api-user-staff-on-create", + "email": "admin@dojo.com", + "password": password, + "is_staff": True, + }, format="json") + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.assertFalse(User.objects.filter(username="api-user-staff-on-create").exists()) + + def test_non_superuser_can_patch_self_without_touching_is_staff(self): + """ + Negative control for the is_staff guard: a delegated user-manager + can still PATCH non-privileged fields on their own account; the + new check only fires when is_staff actually changes. + """ + password = "testTEST1234!@#$" + r = self.client.post(reverse("user-list"), { + "username": "api-user-mgr2", + "email": "admin@dojo.com", + "password": password, + }, format="json") + self.assertEqual(r.status_code, 201, r.content[:1000]) + mgr = User.objects.get(username="api-user-mgr2") + mgr.user_permissions.add(Permission.objects.get(codename="change_user")) + + token_resp = self.client.post(reverse("api-token-auth"), { + "username": "api-user-mgr2", + "password": password, + }, format="json") + self.assertEqual(token_resp.status_code, 200, token_resp.content[:1000]) + mgr_client = APIClient() + mgr_client.credentials(HTTP_AUTHORIZATION="Token " + token_resp.json()["token"]) + + r = mgr_client.patch("{}{}/".format(reverse("user-list"), mgr.id), { + "first_name": "Renamed", + }, format="json") + self.assertEqual(r.status_code, 200, r.content[:1000]) + + def test_superuser_can_set_is_staff_via_api(self): + """Positive control: a superuser is still allowed to toggle is_staff.""" + r = self.client.post(reverse("user-list"), { + "username": "api-user-promotable", + "email": "admin@dojo.com", + "password": "testTEST1234!@#$", + }, format="json") + self.assertEqual(r.status_code, 201, r.content[:1000]) + user_id = r.json()["id"] + + r = self.client.patch("{}{}/".format(reverse("user-list"), user_id), { + "is_staff": True, + }, format="json") + self.assertEqual(r.status_code, 200, r.content[:1000]) + self.assertTrue(User.objects.get(id=user_id).is_staff) + def test_user_reset_api_token_denies_global_owner_legacy(self): """ Legacy: Global_Role(role=Owner) is inert. Resetting another From 81782818bdd1877b64a3ee89b2bab117bcdf4653 Mon Sep 17 00:00:00 2001 From: valentijnscholten Date: Mon, 8 Jun 2026 23:41:50 +0200 Subject: [PATCH 8/9] test(perf): re-enable import performance tests with recalibrated query counts (#14968) Re-baseline expected query counts after upstream merge that switched from RBAC to legacy authorization. Legacy auth has lower per-action overhead (no role-permission lookups, simpler dispatch), so all counts decreased by 1-7 queries. Also removes the unused `unittest.skip` import. --- unittests/test_importers_performance.py | 73 +++++++++++-------------- 1 file changed, 32 insertions(+), 41 deletions(-) diff --git a/unittests/test_importers_performance.py b/unittests/test_importers_performance.py index dc82f28114d..ce9133e4a6b 100644 --- a/unittests/test_importers_performance.py +++ b/unittests/test_importers_performance.py @@ -20,7 +20,6 @@ import logging from contextlib import contextmanager -from unittest import skip from unittest.mock import patch from crum import impersonate @@ -275,11 +274,6 @@ def _import_reimport_performance( self.assertGreater(len_closed_findings4, 0, "Step 4 (empty reimport with close_old_findings=True) should close findings") -@skip("Re-baseline pending: Track B legacy authorization reduces auth-layer query " - "overhead (no per-action role-permission lookups, simpler permission_to_action " - "dispatch). Expected query counts here were calibrated under RBAC and are " - "consistently 1-7 queries higher than legacy actual. Re-baseline with a fresh " - "calibration run after the upstream merge.") @tag("performance") @skip_unless_v2 class TestDojoImporterPerformanceSmall(TestDojoImporterPerformanceBase): @@ -349,13 +343,13 @@ def test_import_reimport_reimport_performance_pghistory_async(self): configure_pghistory_triggers() self._import_reimport_performance( - expected_num_queries1=171, + expected_num_queries1=170, expected_num_async_tasks1=2, - expected_num_queries2=124, + expected_num_queries2=123, expected_num_async_tasks2=1, - expected_num_queries3=29, + expected_num_queries3=28, expected_num_async_tasks3=1, - expected_num_queries4=100, + expected_num_queries4=99, expected_num_async_tasks4=0, ) @@ -373,13 +367,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=187, + expected_num_queries1=184, expected_num_async_tasks1=2, - expected_num_queries2=132, + expected_num_queries2=131, expected_num_async_tasks2=1, - expected_num_queries3=37, + expected_num_queries3=36, expected_num_async_tasks3=1, - expected_num_queries4=100, + expected_num_queries4=99, expected_num_async_tasks4=0, ) @@ -398,13 +392,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=197, + expected_num_queries1=194, expected_num_async_tasks1=4, - expected_num_queries2=142, + expected_num_queries2=141, expected_num_async_tasks2=3, - expected_num_queries3=44, + expected_num_queries3=43, expected_num_async_tasks3=3, - expected_num_queries4=109, + expected_num_queries4=108, expected_num_async_tasks4=2, ) @@ -530,9 +524,9 @@ def test_deduplication_performance_pghistory_async(self): self.system_settings(enable_deduplication=True) self._deduplication_performance( - expected_num_queries1=110, + expected_num_queries1=109, expected_num_async_tasks1=2, - expected_num_queries2=90, + expected_num_queries2=89, expected_num_async_tasks2=2, check_duplicates=False, # Async mode - deduplication happens later ) @@ -551,18 +545,15 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=126, + expected_num_queries1=123, expected_num_async_tasks1=2, - expected_num_queries2=107, + expected_num_queries2=104, expected_num_async_tasks2=2, ) @tag("performance") @override_settings(V3_FEATURE_LOCATIONS=True) -@skip("Re-baseline pending: same RBAC→legacy query-count drift as " - "TestDojoImporterPerformanceSmall. See that class's skip note for the " - "rationale.") class TestDojoImporterPerformanceSmallLocations(TestDojoImporterPerformanceBase): r""" @@ -642,13 +633,13 @@ def test_import_reimport_reimport_performance_pghistory_async(self): configure_pghistory_triggers() self._import_reimport_performance( - expected_num_queries1=178, + expected_num_queries1=177, expected_num_async_tasks1=2, - expected_num_queries2=133, + expected_num_queries2=132, expected_num_async_tasks2=1, - expected_num_queries3=37, + expected_num_queries3=36, expected_num_async_tasks3=1, - expected_num_queries4=101, + expected_num_queries4=100, expected_num_async_tasks4=0, ) @@ -666,13 +657,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=196, + expected_num_queries1=193, expected_num_async_tasks1=2, - expected_num_queries2=143, + expected_num_queries2=142, expected_num_async_tasks2=1, - expected_num_queries3=47, + expected_num_queries3=46, expected_num_async_tasks3=1, - expected_num_queries4=101, + expected_num_queries4=100, expected_num_async_tasks4=0, ) @@ -691,13 +682,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=209, + expected_num_queries1=206, expected_num_async_tasks1=4, - expected_num_queries2=156, + expected_num_queries2=155, expected_num_async_tasks2=3, - expected_num_queries3=54, + expected_num_queries3=53, expected_num_async_tasks3=3, - expected_num_queries4=113, + expected_num_queries4=112, expected_num_async_tasks4=2, ) @@ -798,9 +789,9 @@ def test_deduplication_performance_pghistory_async(self): self.system_settings(enable_deduplication=True) self._deduplication_performance( - expected_num_queries1=117, + expected_num_queries1=116, expected_num_async_tasks1=2, - expected_num_queries2=93, + expected_num_queries2=92, expected_num_async_tasks2=2, check_duplicates=False, # Async mode - deduplication happens later ) @@ -818,8 +809,8 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=135, + expected_num_queries1=132, expected_num_async_tasks1=2, - expected_num_queries2=218, + expected_num_queries2=215, expected_num_async_tasks2=2, ) From e392bd8179c896857770b3f72537300597e42a48 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:35:21 -0600 Subject: [PATCH 9/9] fix: update app version to 2.59.0 and adjust artifacthub annotations; add upgrade notes for version 3.0.0 --- docs/content/releases/os_upgrading/2.59.md | 96 ------------------- .../os_upgrading/{2.59.1.md => 3.0.md} | 3 +- helm/defectdojo/Chart.yaml | 6 +- 3 files changed, 4 insertions(+), 101 deletions(-) delete mode 100644 docs/content/releases/os_upgrading/2.59.md rename docs/content/releases/os_upgrading/{2.59.1.md => 3.0.md} (98%) diff --git a/docs/content/releases/os_upgrading/2.59.md b/docs/content/releases/os_upgrading/2.59.md deleted file mode 100644 index a36e9e88b65..00000000000 --- a/docs/content/releases/os_upgrading/2.59.md +++ /dev/null @@ -1,96 +0,0 @@ ---- -title: 'Upgrading to DefectDojo Version 2.59.x' -toc_hide: true -weight: -20260602 -description: Authorized Users panel replaces Members/Groups under legacy authorization; SSO providers move to DefectDojo Pro; removal of Questionnaire API Endpoints, Credential Manager, and Stub Findings ---- - -## Authorized Users panel replaces Members/Groups under legacy authorization - -Open Source DefectDojo uses the legacy authorization model: access to a Product is granted by `Product.authorized_users` (with cascade via `Product_Type.authorized_users`), and `is_staff` / `is_superuser` bypass everything. - -In 2.59 the classic UI restores the **"Authorized Users"** panel on the Product and Product Type detail pages. The panel reads from and writes to `Product.authorized_users` / `Product_Type.authorized_users` directly, so adding a user actually grants them the access the UI suggests it does. - -### New endpoints - -- `GET/POST /product//authorized_users/add` — list / add users to `Product.authorized_users` -- `POST /product//authorized_users//delete` — remove a user -- `GET/POST /product/type//authorized_users/add` — same for `Product_Type.authorized_users` -- `POST /product/type//authorized_users//delete` - -Both endpoints are gated so only `is_staff` / `is_superuser` users can add or remove. Non-staff users see the panel but no management actions. - -### How RBAC rows are converted - -The data migration `0267_backfill_authorized_users` translates RBAC tables into the legacy model with the following rules: - -| RBAC row | Legacy effect | -|---|---| -| `Product_Member` (any role, direct or via `Product_Group` + `Dojo_Group_Member`) | Adds the user to `Product.authorized_users` | -| `Product_Type_Member` (any role, direct or via `Product_Type_Group` + `Dojo_Group_Member`) | Adds the user to `Product_Type.authorized_users` | -| `Global_Role(Owner)` (direct or via group) | Sets `User.is_superuser = True` | -| `Global_Role(Writer | Maintainer | API_Importer)` (direct or via group) | Sets `User.is_staff = True` | -| `Global_Role(Reader)` | No global elevation — relies on per-product membership | - -Per-product role granularity (Reader vs Writer vs Maintainer vs Owner) collapses to membership-only because the legacy model has no per-product role concept. `Dojo_Group` structure as a permission-bearing entity is also lost; only the flattened individual user memberships remain. - -### Required actions - -- **Database migrations run automatically on upgrade.** Existing access is carried forward into the legacy `authorized_users` model. Existing data is preserved. -- **Audit the upgrade in staging first.** A new `python manage.py preview_legacy_authorization_migration` management command is shipped in 2.59 to summarize what an upgrade would change against a given database. It is read-only. Recommended workflow: install 2.59 in a staging environment with a snapshot of your production database, run the command, review the summary, then upgrade production. -- **Migrating from OS to Pro?** A new `python manage.py reconcile_authorized_users_to_rbac` management command is available on Pro to bring any access changes you made under OS forward into Pro RBAC. It supports `--dry-run` and is idempotent. - -### Pro customers are not impacted - -DefectDojo Pro deployments retain full RBAC. The Pro UX is unchanged — same Members/Groups management surface as before. - -## SSO providers are available in DefectDojo Pro only - -Single sign-on (SAML, OIDC, Google, Okta, Azure AD, GitLab, Auth0, Keycloak, GitHub Enterprise, and remote-user header authentication) has been consolidated into DefectDojo Pro. Open source DefectDojo now exposes only local username/password login and the password-reset flow. - -### Required actions - -- **No customizations or local-only login:** No action required. -- **Currently logging in via SSO on open source:** Existing user accounts and group memberships are preserved on upgrade, but SSO sign-in will no longer work after 2.59. To keep an SSO-driven login experience, switch to [DefectDojo Pro](https://defectdojo.com), which carries forward and extends the SSO surface (provider configuration moves to a UI-managed tuner). - -## Removal: Questionnaire API Endpoints - -As announced in DefectDojo 2.56.0, the following Questionnaire API endpoints have been removed: - -- `/api/v2/questionnaire_answered_questionnaires/` -- `/api/v2/questionnaire_answers/` -- `/api/v2/questionnaire_engagement_questionnaires/` -- `/api/v2/questionnaire_general_questionnaires/` -- `/api/v2/questionnaire_questions` - -### Required Actions - -Any requests to these endpoints will now return a 404 Not Found error. - -## Removal: Credential Manager - -As announced in DefectDojo 2.57.0, the Credential Manager feature has been removed. The following API endpoints are no longer available: - -- `/api/v2/credentials/` -- `/api/v2/credential_mappings/` - -### Required Actions - -Any requests to these endpoints will now return a 404 Not Found error. The Credential Manager UI is no longer available. - -## Removal: Stub Findings - -As announced in DefectDojo 2.57.0, the Stub Findings feature has been removed. The following API endpoint is no longer available: - -- `/api/v2/stub_findings/` - -### Required Actions - -Any requests to this endpoint will now return a 404 Not Found error. The Stub Findings UI is no longer available. - -## Configuration change in Watson Search Indexing - -In [PR 14881](https://github.com/DefectDojo/django-DefectDojo/pull/14881)We optimized the way the Django Watson search index is updated during imports and reimports. There is not a single configuration setting to manage the threshold: `DD_WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE`. The default value should work fine for most instances. - - -For more information, check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/2.59.0). diff --git a/docs/content/releases/os_upgrading/2.59.1.md b/docs/content/releases/os_upgrading/3.0.md similarity index 98% rename from docs/content/releases/os_upgrading/2.59.1.md rename to docs/content/releases/os_upgrading/3.0.md index 701fe466513..4f4f2a6934c 100644 --- a/docs/content/releases/os_upgrading/2.59.1.md +++ b/docs/content/releases/os_upgrading/3.0.md @@ -1,5 +1,5 @@ --- -title: 'Upgrading to DefectDojo Version 2.59.1' +title: "Upgrading to DefectDojo Version 3.0.0" toc_hide: true weight: -20260603 description: Dependency Check parser no longer emits separate findings for related dependencies; related file paths are now listed in the main finding's description. @@ -28,5 +28,4 @@ Only scenario 1 represents the same vulnerable artifact at multiple deploy locat - **Users filtering or grouping by the `related` tag**: that tag is no longer applied because related findings are no longer created. Update any saved filters, dashboards, or rules that depend on it. Equivalent information is now available in the finding description (look for `**Related Filepaths:**`). - **Reimport behavior**: on the next reimport of an existing Dependency Check report, the previously-created `related` findings will be closed as no longer present in the report. This is expected and matches the new parsing behavior. - For more information, check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/2.59.1). diff --git a/helm/defectdojo/Chart.yaml b/helm/defectdojo/Chart.yaml index 49095de5ffa..af9c542b028 100644 --- a/helm/defectdojo/Chart.yaml +++ b/helm/defectdojo/Chart.yaml @@ -1,5 +1,5 @@ apiVersion: v2 -appVersion: "2.60.0-dev" +appVersion: "2.59.0" description: A Helm chart for Kubernetes to install DefectDojo name: defectdojo version: 1.9.31-dev @@ -33,5 +33,5 @@ dependencies: # - kind: security # description: Critical bug annotations: - artifacthub.io/prerelease: "true" - artifacthub.io/changes: "" + artifacthub.io/prerelease: "false" + artifacthub.io/changes: "- kind: changed\n description: Update valkey Docker tag from 0.20.0 to v0.20.1 (_/defect_/Chart.yaml)\n- kind: changed\n description: Update valkey Docker tag from 0.20.1 to v0.20.2 (_/defect_/Chart.yaml)\n- kind: changed\n description: Bump DefectDojo to 2.59.0\n"