From 5c47b148b5c2322e72d66dd57999f59c731631e1 Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Wed, 8 Apr 2026 14:14:45 -0400 Subject: [PATCH 01/30] add unit test --- unittests/test_pdf_report_rendering.py | 223 +++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 unittests/test_pdf_report_rendering.py diff --git a/unittests/test_pdf_report_rendering.py b/unittests/test_pdf_report_rendering.py new file mode 100644 index 00000000000..283fd4f6272 --- /dev/null +++ b/unittests/test_pdf_report_rendering.py @@ -0,0 +1,223 @@ +from django.template import engines +from django.utils.timezone import now + +from dojo.models import ( + Engagement, + Finding, + Product, + Product_Type, + Test, + Test_Type, + User, +) +from unittests.dojo_test_case import DojoTestCase + + +class TestPdfReportTextWrapping(DojoTestCase): + + """ + Tests that PDF report templates render long and pre-wrapped content + within margins instead of overflowing. + """ + + fixtures = ["dojo_testdata.json"] + + LONG_URL = "https://app.example.com/assets/vendor-" + "a1b2c3d4" * 8 + ".js.map" + + # Content with an embedded
 tag, simulating imports (e.g. BugCrowd CSV)
+    # that store HTML-wrapped text in finding fields.
+    DESCRIPTION_WITH_PRE = (
+        "
\n"
+        "An internal debug configuration file (debug-config-e7f3a901.json) is publicly "
+        "accessible at the URL: " + LONG_URL + ". "
+        "Debug configuration files can reveal internal service addresses, feature flags, "
+        "and environment variables. Exposing such files can leak sensitive information "
+        "about the application infrastructure, aiding attackers in lateral movement and "
+        "facilitating exploitation of internal services.\n"
+        "
" + ) + + MITIGATION_WITH_PRE = ( + '
\n'
+        "Remove Debug Files From Public Directories: Ensure .json debug configuration "
+        "files are not deployed to publicly accessible paths on the web server.\n"
+        "Restrict Access: If debug configurations are required in staging environments, "
+        "restrict access to authenticated admin users only via IP whitelisting.\n"
+        "Environment-Specific Builds: Use separate build profiles for development and "
+        "production to ensure debug artifacts are excluded from release bundles.\n"
+        "Audit Build Artifacts: Regularly scan deployment artifacts for unintended "
+        "inclusion of debug or configuration files.\n"
+        "
" + ) + + IMPACT_WITH_PRE = ( + '
\n'
+        "Information Disclosure: Attackers can discover internal microservice endpoints, "
+        "feature flag states, and environment-specific configuration values.\n"
+        "Lateral Movement: Revealed internal addresses may allow attackers to probe "
+        "backend services that are not intended to be publicly reachable.\n"
+        "Credential Exposure: If the debug configuration includes API keys or tokens "
+        "left by developers, this could lead to unauthorized access.\n"
+        "
" + ) + + STEPS_WITH_PRE = ( + '
\n'
+        "Open a web browser.\n"
+        "Navigate to the URL: " + LONG_URL + ".\n"
+        "Observe that the configuration file is accessible and can be downloaded.\n"
+        "Review the file contents for internal service addresses and environment variables.\n"
+        "
" + ) + + # Plain markdown content (no embedded
 tags) with a very long unbroken token
+    DESCRIPTION_LONG_TOKEN = (
+        "A session token was observed in the query string: "
+        "token=" + "x" * 300 + " "
+        "which exceeds normal length and may cause rendering issues in reports."
+    )
+
+    def setUp(self):
+        super().setUp()
+        self.user = User.objects.get(username="admin")
+        self.product_type = Product_Type.objects.create(name="Report Test PT")
+        self.product = Product.objects.create(
+            name="Report Test Product",
+            description="Product for report tests",
+            prod_type=self.product_type,
+        )
+        self.engagement = Engagement.objects.create(
+            name="Report Test Engagement",
+            product=self.product,
+            target_start=now(),
+            target_end=now(),
+        )
+        self.test_type = Test_Type.objects.create(name="Report Test Scan")
+        self.test_obj = Test.objects.create(
+            engagement=self.engagement,
+            test_type=self.test_type,
+            title="Report Rendering Test",
+            target_start=now(),
+            target_end=now(),
+        )
+        self.django_engine = engines["django"]
+
+    def _create_finding(self, **kwargs):
+        defaults = {
+            "title": "Debug Configuration File Exposed",
+            "test": self.test_obj,
+            "severity": "Medium",
+            "description": self.DESCRIPTION_WITH_PRE,
+            "mitigation": self.MITIGATION_WITH_PRE,
+            "impact": self.IMPACT_WITH_PRE,
+            "steps_to_reproduce": self.STEPS_WITH_PRE,
+            "active": True,
+            "verified": True,
+            "reporter": self.user,
+            "numerical_severity": "S2",
+            "date": now().date(),
+        }
+        defaults.update(kwargs)
+        return Finding.objects.create(**defaults)
+
+    def _render_finding_report(self, findings):
+        """Render finding_pdf_report.html with the given findings and return HTML."""
+        template = self.django_engine.get_template("dojo/finding_pdf_report.html")
+        context = {
+            "report_name": "Finding Report",
+            "findings": findings,
+            "include_finding_notes": 0,
+            "include_finding_images": 0,
+            "include_executive_summary": 0,
+            "include_table_of_contents": 0,
+            "include_disclaimer": 0,
+            "disclaimer": "",
+            "user": self.user,
+            "team_name": "Test Team",
+            "title": "Finding Report",
+            "host": "http://localhost:8080",
+            "user_id": self.user.id,
+        }
+        return template.render(context)
+
+    def test_no_nested_pre_tags_in_report(self):
+        """
+        Markdown-rendered fields should not produce nested 
 elements.
+
+        When imported data already contains 
 tags (common with BugCrowd CSV
+        imports), the template wrapper must not add an additional 
 layer.
+        The outer wrapper should be a 
. + """ + finding = self._create_finding() + html = self._render_finding_report(Finding.objects.filter(pk=finding.pk)) + + # The template should wrap markdown-rendered fields in div.report-field, + # not in
 tags. We should not see 
 nesting.
+        self.assertNotIn("
", html)
+        self.assertNotIn("
+        # Find the section containing our long token
+        idx = html.index("x" * 300)
+        # Walk backwards to find the nearest opening tag
+        preceding = html[max(0, idx - 500):idx]
+        self.assertIn("report-field", preceding)
+
+    def test_report_base_css_has_overflow_wrap(self):
+        """The report base template must include overflow-wrap for text wrapping."""
+        template = self.django_engine.get_template("report_base.html")
+        # Render with minimal context to get the CSS
+        html = template.render({"report_name": "Test"})
+
+        self.assertIn("overflow-wrap: break-word", html)
+
+    def test_report_base_css_styles_nested_pre(self):
+        """
+        The report base CSS must style .report-field pre to prevent
+        nested 
 elements from breaking out of margins.
+        """
+        template = self.django_engine.get_template("report_base.html")
+        html = template.render({"report_name": "Test"})
+
+        self.assertIn(".report-field pre", html)
+        self.assertIn("overflow-wrap: break-word", html)
+
+    def test_raw_request_pre_tags_preserved(self):
+        """
+        Raw request/response 
 tags should remain unchanged.
+
+        Only markdown-rendered fields should use div.report-field wrappers.
+        The raw_request class pre tags are for literal request/response data
+        and should stay as 
.
+        """
+        template = self.django_engine.get_template("dojo/finding_pdf_report.html")
+        source = template.template.source
+        self.assertIn('class="raw_request"', source)
+        # raw_request should still be inside 
 tags
+        self.assertIn('
', source)

From 21aab026fa10e422d370f208b3c5db7130b71760 Mon Sep 17 00:00:00 2001
From: Paul Osinski 
Date: Wed, 8 Apr 2026 14:12:54 -0400
Subject: [PATCH 02/30] replace 
 wrappers and update CSS on reports

---
 dojo/templates/dojo/endpoint_pdf_report.html   | 14 +++++++-------
 dojo/templates/dojo/engagement_pdf_report.html | 14 +++++++-------
 dojo/templates/dojo/finding_pdf_report.html    | 14 +++++++-------
 .../dojo/product_endpoint_pdf_report.html      | 14 +++++++-------
 dojo/templates/dojo/product_pdf_report.html    | 14 +++++++-------
 .../dojo/product_type_pdf_report.html          | 14 +++++++-------
 dojo/templates/dojo/test_pdf_report.html       | 14 +++++++-------
 dojo/templates/report_base.html                | 18 ++++++++++++++++++
 8 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/dojo/templates/dojo/endpoint_pdf_report.html b/dojo/templates/dojo/endpoint_pdf_report.html
index ef187bb1b76..b6e6ded2948 100644
--- a/dojo/templates/dojo/endpoint_pdf_report.html
+++ b/dojo/templates/dojo/endpoint_pdf_report.html
@@ -178,15 +178,15 @@ 
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -203,22 +203,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/engagement_pdf_report.html b/dojo/templates/dojo/engagement_pdf_report.html index bb10597d7e9..f574a195cac 100644 --- a/dojo/templates/dojo/engagement_pdf_report.html +++ b/dojo/templates/dojo/engagement_pdf_report.html @@ -313,15 +313,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -338,22 +338,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/finding_pdf_report.html b/dojo/templates/dojo/finding_pdf_report.html index a40739d041b..e82dcb82a3d 100644 --- a/dojo/templates/dojo/finding_pdf_report.html +++ b/dojo/templates/dojo/finding_pdf_report.html @@ -157,15 +157,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -182,22 +182,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/product_endpoint_pdf_report.html b/dojo/templates/dojo/product_endpoint_pdf_report.html index f7de27fe7a3..6da95cd5bbd 100644 --- a/dojo/templates/dojo/product_endpoint_pdf_report.html +++ b/dojo/templates/dojo/product_endpoint_pdf_report.html @@ -227,15 +227,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -252,22 +252,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/product_pdf_report.html b/dojo/templates/dojo/product_pdf_report.html index 9768c006888..df1cf7b1b26 100644 --- a/dojo/templates/dojo/product_pdf_report.html +++ b/dojo/templates/dojo/product_pdf_report.html @@ -283,15 +283,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -308,22 +308,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/product_type_pdf_report.html b/dojo/templates/dojo/product_type_pdf_report.html index 483d024edc7..0a623ee3637 100644 --- a/dojo/templates/dojo/product_type_pdf_report.html +++ b/dojo/templates/dojo/product_type_pdf_report.html @@ -212,15 +212,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -237,22 +237,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/dojo/test_pdf_report.html b/dojo/templates/dojo/test_pdf_report.html index 17af82137bc..986ec8fccaa 100644 --- a/dojo/templates/dojo/test_pdf_report.html +++ b/dojo/templates/dojo/test_pdf_report.html @@ -325,15 +325,15 @@
{% if finding.cvssv3 %}
CVSS v3
-
{{ finding.cvssv3|markdown_render }}
+
{{ finding.cvssv3|markdown_render }}
{% endif %}
Description
-
{{ finding.description|markdown_render }}
+
{{ finding.description|markdown_render }}
{% if finding.mitigation %}
Mitigation
-
{{ finding.mitigation|markdown_render }}
+
{{ finding.mitigation|markdown_render }}
{% endif %} {% if finding.get_report_requests %} @@ -350,22 +350,22 @@
Response {{forloop.counter}}
{% if finding.impact %}
Impact
-
{{ finding.impact|markdown_render }}
+
{{ finding.impact|markdown_render }}
{% endif %} {% if finding.steps_to_reproduce %}
Steps to Reproduce
-
{{ finding.steps_to_reproduce|markdown_render }}
+
{{ finding.steps_to_reproduce|markdown_render }}
{% endif %} {% if finding.severity_justification %}
Severity Justification
-
{{ finding.severity_justification|markdown_render }}
+
{{ finding.severity_justification|markdown_render }}
{% endif %} {% if finding.references %}
References
-
{{ finding.references|markdown_render }}
+
{{ finding.references|markdown_render }}
{% endif %} {% if include_finding_images %} diff --git a/dojo/templates/report_base.html b/dojo/templates/report_base.html index a27264a0038..a3328eaadba 100644 --- a/dojo/templates/report_base.html +++ b/dojo/templates/report_base.html @@ -134,6 +134,24 @@ padding: 0 !important; overflow: visible !important; word-break: normal !important; + overflow-wrap: break-word !important; + } + + .report-field { + margin-bottom: 15px !important; + overflow-wrap: break-word !important; + word-break: normal !important; + } + + .report-field pre { + white-space: pre-wrap !important; + padding: 0 !important; + border: 0 !important; + background-color: transparent !important; + overflow: visible !important; + overflow-wrap: break-word !important; + word-break: normal !important; + margin: 0 !important; } ul{ From 5f13e4d4e1193a1c343151cf76e1598ff4478080 Mon Sep 17 00:00:00 2001 From: DefectDojo release bot Date: Mon, 13 Apr 2026 16:07:56 +0000 Subject: [PATCH 03/30] 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 f224ee344bd..9abaf4184c6 100644 --- a/components/package.json +++ b/components/package.json @@ -1,6 +1,6 @@ { "name": "defectdojo", - "version": "2.57.1", + "version": "2.58.0-dev", "license" : "BSD-3-Clause", "private": true, "dependencies": { diff --git a/helm/defectdojo/Chart.yaml b/helm/defectdojo/Chart.yaml index 35c4265e7a2..f0fe4814a66 100644 --- a/helm/defectdojo/Chart.yaml +++ b/helm/defectdojo/Chart.yaml @@ -1,8 +1,8 @@ apiVersion: v2 -appVersion: "2.57.1" +appVersion: "2.58.0-dev" description: A Helm chart for Kubernetes to install DefectDojo name: defectdojo -version: 1.9.22 +version: 1.9.23-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: Bump DefectDojo to 2.57.1\n" + artifacthub.io/prerelease: "true" + artifacthub.io/changes: "" diff --git a/helm/defectdojo/README.md b/helm/defectdojo/README.md index 50b6f1de8bc..6006a2203b0 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.22](https://img.shields.io/badge/Version-1.9.22-informational?style=flat-square) ![AppVersion: 2.57.1](https://img.shields.io/badge/AppVersion-2.57.1-informational?style=flat-square) +![Version: 1.9.23-dev](https://img.shields.io/badge/Version-1.9.23--dev-informational?style=flat-square) ![AppVersion: 2.58.0-dev](https://img.shields.io/badge/AppVersion-2.58.0--dev-informational?style=flat-square) A Helm chart for Kubernetes to install DefectDojo From 8a2100effcde2cf2987f94dcc5297a27b7cfd008 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 08:44:32 -0500 Subject: [PATCH 04/30] chore(deps): bump pillow from 12.1.1 to 12.2.0 (#14680) Bumps [pillow](https://github.com/python-pillow/Pillow) from 12.1.1 to 12.2.0. - [Release notes](https://github.com/python-pillow/Pillow/releases) - [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst) - [Commits](https://github.com/python-pillow/Pillow/compare/12.1.1...12.2.0) --- updated-dependencies: - dependency-name: pillow dependency-version: 12.2.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 0b6931e7177..d782fdcd5c8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,7 @@ PyGithub==2.9.0 lxml==6.0.2 Markdown==3.10.2 openpyxl==3.1.5 -Pillow==12.1.1 # required by django-imagekit +Pillow==12.2.0 # required by django-imagekit psycopg[c]==3.3.3 cryptography==46.0.7 python-dateutil==2.9.0.post0 From 76d8ed8f206f724ddc44b2cb61fe2204a24684ec Mon Sep 17 00:00:00 2001 From: Manuel Sommer Date: Thu, 16 Apr 2026 08:52:47 +0200 Subject: [PATCH 05/30] :tada: add mozilla foundation sec advice to vulnid --- dojo/settings/settings.dist.py | 1 + dojo/templatetags/display_tags.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index 429b5646b90..2da65a5ebb6 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -2036,6 +2036,7 @@ def saml2_attrib_map_format(din): "KHV": "https://avd.aquasec.com/misconfig/kubernetes/", # e.g. https://avd.aquasec.com/misconfig/kubernetes/khv045 "LEN-": "https://support.lenovo.com/cl/de/product_security/", # e.g. https://support.lenovo.com/cl/de/product_security/LEN-94953 "MAL-": "https://cvepremium.circl.lu/vuln/", # e.g. https://cvepremium.circl.lu/vuln/mal-2025-49305 + "MFSA": "https://www.mozilla.org/en-US/security/advisories/", # e.g. https://www.mozilla.org/en-US/security/advisories/mfsa2025-01/ "MGAA-": "https://advisories.mageia.org/&&.html", # e.g. https://advisories.mageia.org/MGAA-2013-0054.html "MGASA-": "https://advisories.mageia.org/&&.html", # e.g. https://advisories.mageia.org/MGASA-2025-0023.html "MSRC_": "https://cvepremium.circl.lu/vuln/", # e.g. https://cvepremium.circl.lu/vuln/msrc_cve-2025-59200 diff --git a/dojo/templatetags/display_tags.py b/dojo/templatetags/display_tags.py index 25dee916bd1..be102783d03 100644 --- a/dojo/templatetags/display_tags.py +++ b/dojo/templatetags/display_tags.py @@ -854,7 +854,7 @@ def vulnerability_url(vulnerability_id): return settings.VULNERABILITY_URLS[key] + str(vulnerability_id.replace("SSA:", "SSA-")) if key == "SSA-" and not re.findall(r"SSA-\d{4}-", vulnerability_id): return "https://cert-portal.siemens.com/productcert/html/" + str(vulnerability_id.lower()) + ".html" - if key in {"AVD", "KHV", "C-", "ELA-"}: + if key in {"AVD", "KHV", "C-", "ELA-", "MFSA"}: return settings.VULNERABILITY_URLS[key] + str(vulnerability_id.lower()) if key == "SUSE-SU-": return settings.VULNERABILITY_URLS[key] + str(vulnerability_id.lower().removeprefix("suse-su-")[:4]) + "/" + vulnerability_id.replace(":", "") From aec3fef0a49196977e7183989b989550f595d532 Mon Sep 17 00:00:00 2001 From: Jino Tesauro <53376807+Jino-T@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:52:27 -0500 Subject: [PATCH 06/30] Added ssrf utils file to check urls and applied it to risk recon parser (#14631) * added ssrf utils to check urls and applied it to risk recon parser * update risk recon unit tests * add unit tests for SSRF protection in risk recon API init Co-Authored-By: Claude Sonnet 4.6 * add unit tests for utils_ssrf module Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- dojo/tools/risk_recon/api.py | 16 +- dojo/utils_ssrf.py | 172 ++++++++++++++++++++++ unittests/test_utils_ssrf.py | 75 ++++++++++ unittests/tools/test_risk_recon_parser.py | 24 ++- 4 files changed, 280 insertions(+), 7 deletions(-) create mode 100644 dojo/utils_ssrf.py create mode 100644 unittests/test_utils_ssrf.py diff --git a/dojo/tools/risk_recon/api.py b/dojo/tools/risk_recon/api.py index d9691d41aa9..5bf26d83115 100644 --- a/dojo/tools/risk_recon/api.py +++ b/dojo/tools/risk_recon/api.py @@ -1,6 +1,7 @@ -import requests from django.conf import settings +from dojo.utils_ssrf import SSRFError, make_ssrf_safe_session, validate_url_for_ssrf + class RiskReconAPI: def __init__(self, api_key, endpoint, data): @@ -26,7 +27,14 @@ def __init__(self, api_key, endpoint, data): raise Exception(msg) if self.url.endswith("/"): self.url = endpoint[:-1] - self.session = requests.Session() + + try: + validate_url_for_ssrf(self.url) + except SSRFError as exc: + msg = f"Invalid Risk Recon API url: {exc}" + raise Exception(msg) from exc + + self.session = make_ssrf_safe_session() self.map_toes() self.get_findings() @@ -54,7 +62,7 @@ def map_toes(self): filters = comps.get(name) self.toe_map[toe_id] = filters or self.data else: - msg = f"Unable to query Target of Evaluations due to {response.status_code} - {response.content}" + msg = f"Unable to query Target of Evaluations due to {response.status_code}" raise Exception(msg) # TODO: when implementing ruff BLE001, please fix also TODO in unittests/test_risk_recon.py def filter_finding(self, finding): @@ -86,5 +94,5 @@ def get_findings(self): if not self.filter_finding(finding): self.findings.append(finding) else: - msg = f"Unable to collect findings from toe: {toe} due to {response.status_code} - {response.content}" + msg = f"Unable to collect findings from toe: {toe} due to {response.status_code}" raise Exception(msg) diff --git a/dojo/utils_ssrf.py b/dojo/utils_ssrf.py new file mode 100644 index 00000000000..42334e0173b --- /dev/null +++ b/dojo/utils_ssrf.py @@ -0,0 +1,172 @@ +""" +SSRF (Server-Side Request Forgery) protection utilities. + +Provides a requests.Session that validates outbound URLs against private/reserved +IP ranges at socket-creation time, closing the DNS rebinding (TOCTOU) window that +exists when validation is performed only as a pre-flight step. + +Usage: + from dojo.utils_ssrf import make_ssrf_safe_session, validate_url_for_ssrf, SSRFError + + # Pre-flight validation (raises SSRFError with a human-readable message): + validate_url_for_ssrf(url) + + # Safe session (validates at socket-creation time on every request): + session = make_ssrf_safe_session() + response = session.get(url) +""" + +import ipaddress +import socket +from urllib.parse import urlparse + +import requests +import urllib3.connection +import urllib3.connectionpool +from requests.adapters import DEFAULT_POOLBLOCK, DEFAULT_POOLSIZE, HTTPAdapter + + +class SSRFError(ValueError): + + """Raised when a URL is determined to be unsafe for server-side requests.""" + + +_ALLOWED_SCHEMES = frozenset({"http", "https"}) + + +def _check_ip(ip_str: str) -> None: + """Raise SSRFError if the IP address is not globally routable.""" + try: + ip = ipaddress.ip_address(ip_str) + except ValueError as exc: + msg = f"Cannot parse IP address: {ip_str!r}" + raise SSRFError(msg) from exc + + # ip.is_global is False for loopback, link-local (169.254.x.x), RFC 1918, + # reserved, multicast, and unspecified addresses. + if not ip.is_global: + msg = ( + f"Blocked: URL resolved to non-public address {ip}. " + "Requests to private, loopback, link-local, or reserved " + "addresses are not permitted." + ) + raise SSRFError(msg) + + +def _resolve_and_check(hostname: str, port: int) -> None: + """Resolve hostname and verify every returned address is publicly routable.""" + try: + addr_infos = socket.getaddrinfo( + hostname, port, socket.AF_UNSPEC, socket.SOCK_STREAM, + ) + except socket.gaierror as exc: + msg = f"Unable to resolve hostname {hostname!r}: {exc}" + raise SSRFError(msg) from exc + + if not addr_infos: + msg = f"No addresses returned for hostname {hostname!r}" + raise SSRFError(msg) + + for _family, _type, _proto, _canon, sockaddr in addr_infos: + _check_ip(sockaddr[0]) + + +def validate_url_for_ssrf(url: str) -> None: + """ + Pre-flight SSRF validation for a URL. + + Checks: + - Scheme is http or https (blocks file://, gopher://, etc.) + - Every resolved IP address is globally routable (blocks RFC 1918, + loopback 127.x, link-local 169.254.x.x, and other reserved ranges) + + Raises SSRFError with a descriptive message if the URL is unsafe. + This is a best-effort pre-flight check; use make_ssrf_safe_session() for + socket-level enforcement that also mitigates DNS rebinding. + """ + try: + parsed = urlparse(url) + except Exception as exc: + msg = f"Malformed URL: {url!r}" + raise SSRFError(msg) from exc + + if parsed.scheme not in _ALLOWED_SCHEMES: + msg = ( + f"URL scheme {parsed.scheme!r} is not permitted. " + "Only 'http' and 'https' are allowed." + ) + raise SSRFError(msg) + + hostname = parsed.hostname + if not hostname: + msg = f"URL has no hostname: {url!r}" + raise SSRFError(msg) + + port = parsed.port or (443 if parsed.scheme == "https" else 80) + _resolve_and_check(hostname, port) + + +# --------------------------------------------------------------------------- +# urllib3 connection subclasses — validation runs at socket-creation time. +# Overriding _new_conn() (called immediately before the OS connect() syscall) +# minimises the TOCTOU window to microseconds, making DNS rebinding attacks +# impractical in practice. +# --------------------------------------------------------------------------- + +class _SSRFSafeHTTPConnection(urllib3.connection.HTTPConnection): + def _new_conn(self) -> socket.socket: + _resolve_and_check(self._dns_host, self.port) + return super()._new_conn() + + +class _SSRFSafeHTTPSConnection(urllib3.connection.HTTPSConnection): + def _new_conn(self) -> socket.socket: + _resolve_and_check(self._dns_host, self.port) + return super()._new_conn() + + +class _SSRFSafeHTTPConnectionPool(urllib3.connectionpool.HTTPConnectionPool): + ConnectionCls = _SSRFSafeHTTPConnection + + +class _SSRFSafeHTTPSConnectionPool(urllib3.connectionpool.HTTPSConnectionPool): + ConnectionCls = _SSRFSafeHTTPSConnection + + +_SAFE_POOL_CLASSES = { + "http": _SSRFSafeHTTPConnectionPool, + "https": _SSRFSafeHTTPSConnectionPool, +} + + +class _SSRFSafeAdapter(HTTPAdapter): + + """ + A requests HTTPAdapter that injects SSRF-safe connection classes into the + urllib3 pool manager so that IP validation happens at socket-creation time + on every request, including after redirects. + """ + + def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs): + super().init_poolmanager(connections, maxsize, block, **pool_kwargs) + # Replace the pool classes after the manager is created. + # pool_classes_by_scheme is a plain dict on the instance, so this + # only affects this adapter's pool manager. + self.poolmanager.pool_classes_by_scheme = _SAFE_POOL_CLASSES + + +def make_ssrf_safe_session() -> requests.Session: + """ + Return a requests.Session with SSRF protection applied at the socket level. + + Every outbound request made through this session will have its resolved IP + validated against the private/reserved range blocklist immediately before + the OS socket is opened, preventing both: + - Direct requests to internal IP ranges + - DNS rebinding attacks + """ + session = requests.Session() + adapter = _SSRFSafeAdapter(pool_maxsize=DEFAULT_POOLSIZE) + session.mount("http://", adapter) + session.mount("https://", adapter) + return session diff --git a/unittests/test_utils_ssrf.py b/unittests/test_utils_ssrf.py new file mode 100644 index 00000000000..904abf8101c --- /dev/null +++ b/unittests/test_utils_ssrf.py @@ -0,0 +1,75 @@ +import socket +from unittest.mock import patch + +import requests + +from dojo.utils_ssrf import SSRFError, _SSRFSafeAdapter, make_ssrf_safe_session, validate_url_for_ssrf # noqa: PLC2701 +from unittests.dojo_test_case import DojoTestCase + + +def _addr_info(ip, port=80): + """Build a minimal getaddrinfo-style return value for a single IP.""" + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (ip, port))] + + +_MIXED_ADDR_INFO = [ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("8.8.8.8", 80)), + (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("192.168.1.1", 80)), +] + + +class TestValidateUrlForSsrf(DojoTestCase): + + @patch("dojo.utils_ssrf.socket.getaddrinfo", return_value=_addr_info("8.8.8.8")) + def test_valid_public_url_does_not_raise(self, mock_getaddrinfo): + validate_url_for_ssrf("http://example.com/api") # should not raise + + def test_file_scheme_raises(self): + with self.assertRaisesRegex(SSRFError, "not permitted"): + validate_url_for_ssrf("file:///etc/passwd") + + def test_gopher_scheme_raises(self): + with self.assertRaisesRegex(SSRFError, "not permitted"): + validate_url_for_ssrf("gopher://example.com") + + def test_no_hostname_raises(self): + with self.assertRaisesRegex(SSRFError, "no hostname"): + validate_url_for_ssrf("http://") + + def test_loopback_ip_raises(self): + with self.assertRaisesRegex(SSRFError, "non-public address"): + validate_url_for_ssrf("http://127.0.0.1/") + + def test_private_class_c_raises(self): + with self.assertRaisesRegex(SSRFError, "non-public address"): + validate_url_for_ssrf("http://192.168.1.1/") + + def test_private_class_a_raises(self): + with self.assertRaisesRegex(SSRFError, "non-public address"): + validate_url_for_ssrf("http://10.0.0.1/") + + def test_link_local_raises(self): + with self.assertRaisesRegex(SSRFError, "non-public address"): + validate_url_for_ssrf("http://169.254.1.1/") + + @patch("dojo.utils_ssrf.socket.getaddrinfo", side_effect=socket.gaierror("Name or service not known")) + def test_unresolvable_hostname_raises(self, mock_getaddrinfo): + with self.assertRaisesRegex(SSRFError, "Unable to resolve"): + validate_url_for_ssrf("http://nonexistent.invalid/") + + @patch("dojo.utils_ssrf.socket.getaddrinfo", return_value=_MIXED_ADDR_INFO) + def test_multi_address_with_private_ip_raises(self, mock_getaddrinfo): + with self.assertRaisesRegex(SSRFError, "non-public address"): + validate_url_for_ssrf("http://example.com/") + + +class TestMakeSsrfSafeSession(DojoTestCase): + + def test_returns_requests_session(self): + session = make_ssrf_safe_session() + self.assertIsInstance(session, requests.Session) + + def test_http_and_https_mounted_with_safe_adapter(self): + session = make_ssrf_safe_session() + self.assertIsInstance(session.get_adapter("http://example.com"), _SSRFSafeAdapter) + self.assertIsInstance(session.get_adapter("https://example.com"), _SSRFSafeAdapter) diff --git a/unittests/tools/test_risk_recon_parser.py b/unittests/tools/test_risk_recon_parser.py index c59b39bd7d9..a2fec88180d 100644 --- a/unittests/tools/test_risk_recon_parser.py +++ b/unittests/tools/test_risk_recon_parser.py @@ -1,9 +1,10 @@ import datetime - -import requests +from unittest.mock import MagicMock, patch from dojo.models import Test +from dojo.tools.risk_recon.api import RiskReconAPI from dojo.tools.risk_recon.parser import RiskReconParser +from dojo.utils_ssrf import SSRFError from unittests.dojo_test_case import DojoTestCase, get_unit_tests_scans_path @@ -11,7 +12,7 @@ class TestRiskReconAPIParser(DojoTestCase): def test_api_with_bad_url(self): with (get_unit_tests_scans_path("risk_recon") / "bad_url.json").open(encoding="utf-8") as testfile, \ - self.assertRaises(requests.exceptions.ConnectionError): + self.assertRaises(Exception): # noqa: B017 # SSRFError is caught and re-raised as Exception in api.py parser = RiskReconParser() parser.get_findings(testfile, Test()) @@ -34,3 +35,20 @@ def test_parser_without_api(self): finding = findings[1] self.assertEqual(datetime.date(2017, 3, 17), finding.date.date()) self.assertEqual("ff2bbdbfc2b6gsrgwergwe6b1fasfwefb", finding.unique_id_from_tool) + + @patch("dojo.tools.risk_recon.api.validate_url_for_ssrf", side_effect=SSRFError("blocked: private address")) + def test_ssrf_error_is_raised_as_exception(self, mock_validate): + with self.assertRaisesRegex(Exception, "Invalid Risk Recon API url"): + RiskReconAPI(api_key="somekey", endpoint="http://192.168.1.1/api", data=[]) + mock_validate.assert_called_once_with("http://192.168.1.1/api") + + @patch.object(RiskReconAPI, "get_findings") + @patch.object(RiskReconAPI, "map_toes") + @patch("dojo.tools.risk_recon.api.make_ssrf_safe_session") + @patch("dojo.tools.risk_recon.api.validate_url_for_ssrf") + def test_make_ssrf_safe_session_called_on_init(self, mock_validate, mock_make_session, mock_map_toes, mock_get_findings): + mock_session = MagicMock() + mock_make_session.return_value = mock_session + api = RiskReconAPI(api_key="somekey", endpoint="https://api.riskrecon.com/v1", data=[]) + mock_make_session.assert_called_once() + self.assertIs(api.session, mock_session) From 7f546711bc9221630237fcf586a2946a2f71c293 Mon Sep 17 00:00:00 2001 From: Jino Tesauro <53376807+Jino-T@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:53:02 -0500 Subject: [PATCH 07/30] Use RBAC for accept_risks API endpoints (#14632) * Fix accept_risks API endpoints to use RBAC instead of IsAdminUser Replace DRF's IsAdminUser permission with DefectDojo's RBAC system on all accept_risks endpoints. IsAdminUser only checked is_staff, bypassing role-based access control entirely. - Use UserHasRiskAcceptanceRelatedObjectPermission for detail endpoints (engagement/test accept_risks) to enforce Permissions.Risk_Acceptance - Change mass endpoint to query engagements with Risk_Acceptance permission instead of Engagement_View - Enforce product-level enable_full_risk_acceptance setting on all accept_risks endpoints - Add 9 RBAC unit tests covering writer/reader roles and the enable_full_risk_acceptance product setting Co-Authored-By: Claude Opus 4.6 (1M context) * Fix ruff lint: add blank line before class docstring Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- dojo/risk_acceptance/api.py | 19 ++- unittests/test_bulk_risk_acceptance_api.py | 188 +++++++++++++++++++++ 2 files changed, 201 insertions(+), 6 deletions(-) diff --git a/dojo/risk_acceptance/api.py b/dojo/risk_acceptance/api.py index 2fdaadf0afb..4db4e09c76d 100644 --- a/dojo/risk_acceptance/api.py +++ b/dojo/risk_acceptance/api.py @@ -1,18 +1,20 @@ from abc import ABC, abstractmethod from typing import NamedTuple +from django.core.exceptions import PermissionDenied from django.db.models import QuerySet from django.utils import timezone from drf_spectacular.utils import extend_schema from rest_framework import serializers, status from rest_framework.decorators import action -from rest_framework.permissions import IsAdminUser +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from dojo.api_v2.permissions import UserHasRiskAcceptanceRelatedObjectPermission from dojo.api_v2.serializers import RiskAcceptanceSerializer from dojo.authorization.roles_permissions import Permissions from dojo.engagement.queries import get_authorized_engagements -from dojo.models import Risk_Acceptance, User, Vulnerability_Id +from dojo.models import Engagement, Risk_Acceptance, User, Vulnerability_Id AcceptedRisk = NamedTuple("AcceptedRisk", (("vulnerability_id", str), ("justification", str), ("accepted_by", str))) @@ -40,10 +42,13 @@ def risk_application_model_class(self): request=AcceptedRiskSerializer(many=True), responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)}, ) - @action(methods=["post"], detail=True, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer, - filter_backends=[], pagination_class=None) + @action(methods=["post"], detail=True, permission_classes=[IsAuthenticated, UserHasRiskAcceptanceRelatedObjectPermission], + serializer_class=AcceptedRiskSerializer, filter_backends=[], pagination_class=None) def accept_risks(self, request, pk=None): model = self.get_object() + product = model.product if isinstance(model, Engagement) else model.engagement.product + if not product.enable_full_risk_acceptance: + raise PermissionDenied serializer = AcceptedRiskSerializer(data=request.data, many=True) if serializer.is_valid(): accepted_risks = serializer.save() @@ -63,7 +68,7 @@ class AcceptedFindingsMixin(ABC): request=AcceptedRiskSerializer(many=True), responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)}, ) - @action(methods=["post"], detail=False, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer) + @action(methods=["post"], detail=False, permission_classes=[IsAuthenticated], serializer_class=AcceptedRiskSerializer) def accept_risks(self, request): serializer = AcceptedRiskSerializer(data=request.data, many=True) if serializer.is_valid(): @@ -72,7 +77,9 @@ def accept_risks(self, request): return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST) owner = request.user accepted_result = [] - for engagement in get_authorized_engagements(Permissions.Engagement_View): + for engagement in get_authorized_engagements(Permissions.Risk_Acceptance): + if not engagement.product.enable_full_risk_acceptance: + continue base_findings = engagement.unaccepted_open_findings accepted = _accept_risks(accepted_risks, base_findings, owner) engagement.accept_risks(accepted) diff --git a/unittests/test_bulk_risk_acceptance_api.py b/unittests/test_bulk_risk_acceptance_api.py index 05bbe10e7a8..cc5ace94517 100644 --- a/unittests/test_bulk_risk_acceptance_api.py +++ b/unittests/test_bulk_risk_acceptance_api.py @@ -9,6 +9,7 @@ Engagement, Finding, Product, + Product_Member, Product_Type, Product_Type_Member, Role, @@ -117,3 +118,190 @@ def test_finding_accept_risks(self): for ra in self.engagement_2b.risk_acceptance.all(): for finding in ra.accepted_findings.all(): self.assertEqual(self.engagement_2a.product, finding.test.engagement.product) + + +class TestBulkRiskAcceptanceRbac(APITestCase): + + """Tests that accept_risks endpoints use RBAC (Permissions.Risk_Acceptance) instead of is_staff.""" + + @classmethod + def setUpTestData(cls): + cls.product_type = Product_Type.objects.create(name="RBAC Test Type") + cls.test_type = Test_Type.objects.create(name="RBAC Mock Scan", static_tool=True) + + # Product with full risk acceptance enabled (default) + cls.product_enabled = Product.objects.create( + prod_type=cls.product_type, name="RBAC Enabled", + description="Full risk acceptance enabled", + enable_full_risk_acceptance=True, + ) + # Product with full risk acceptance disabled + cls.product_disabled = Product.objects.create( + prod_type=cls.product_type, name="RBAC Disabled", + description="Full risk acceptance disabled", + enable_full_risk_acceptance=False, + ) + + cls.engagement_enabled = Engagement.objects.create( + product=cls.product_enabled, + target_start=datetime.datetime(2000, 1, 1, tzinfo=datetime.UTC), + target_end=datetime.datetime(2000, 2, 1, tzinfo=datetime.UTC), + ) + cls.engagement_disabled = Engagement.objects.create( + product=cls.product_disabled, + target_start=datetime.datetime(2000, 1, 1, tzinfo=datetime.UTC), + target_end=datetime.datetime(2000, 2, 1, tzinfo=datetime.UTC), + ) + + cls.test_enabled = Test.objects.create( + engagement=cls.engagement_enabled, test_type=cls.test_type, + target_start=datetime.datetime(2000, 1, 1, tzinfo=datetime.UTC), + target_end=datetime.datetime(2000, 2, 1, tzinfo=datetime.UTC), + ) + cls.test_disabled = Test.objects.create( + engagement=cls.engagement_disabled, test_type=cls.test_type, + target_start=datetime.datetime(2000, 1, 1, tzinfo=datetime.UTC), + target_end=datetime.datetime(2000, 2, 1, tzinfo=datetime.UTC), + ) + + # Writer user: has Risk_Acceptance permission, NOT is_staff + cls.writer = User.objects.create(username="rbac_writer", is_staff=False) + cls.writer_token = Token.objects.create(user=cls.writer) + Product_Member.objects.create( + product=cls.product_enabled, user=cls.writer, + role=Role.objects.get(id=Roles.Writer), + ) + Product_Member.objects.create( + product=cls.product_disabled, user=cls.writer, + role=Role.objects.get(id=Roles.Writer), + ) + + # Reader user: does NOT have Risk_Acceptance permission, NOT is_staff + cls.reader = User.objects.create(username="rbac_reader", is_staff=False) + cls.reader_token = Token.objects.create(user=cls.reader) + Product_Member.objects.create( + product=cls.product_enabled, user=cls.reader, + role=Role.objects.get(id=Roles.Reader), + ) + + def create_finding(test, reporter, cve): + return Finding( + test=test, title=f"Finding {cve}", cve=cve, severity="High", + verified=True, description="Test", mitigation="Test", + impact="Test", reporter=reporter, numerical_severity="S1", + static_finding=True, dynamic_finding=False, + ) + + # Findings on the enabled product + Finding.objects.bulk_create( + create_finding(cls.test_enabled, cls.writer, f"CVE-2024-{i}") for i in range(10)) + for f in Finding.objects.filter(test=cls.test_enabled): + Vulnerability_Id.objects.get_or_create(finding=f, vulnerability_id=f.cve) + + # Findings on the disabled product + Finding.objects.bulk_create( + create_finding(cls.test_disabled, cls.writer, f"CVE-2024-{i + 100}") for i in range(5)) + for f in Finding.objects.filter(test=cls.test_disabled): + Vulnerability_Id.objects.get_or_create(finding=f, vulnerability_id=f.cve) + + def _client_for(self, token): + client = APIClient() + client.credentials(HTTP_AUTHORIZATION="Token " + token.key) + return client + + def _accepted_risks(self, cve_ids): + return [{"vulnerability_id": cve, "justification": "Test", "accepted_by": "Tester"} for cve in cve_ids] + + # --- Writer (has Risk_Acceptance) succeeds on enabled product --- + + def test_writer_can_accept_risks_on_engagement(self): + client = self._client_for(self.writer_token) + result = client.post( + reverse("engagement-accept-risks", kwargs={"pk": self.engagement_enabled.id}), + data=self._accepted_risks(["CVE-2024-0"]), + format="json", + ) + self.assertEqual(result.status_code, 201) + + def test_writer_can_accept_risks_on_test(self): + client = self._client_for(self.writer_token) + result = client.post( + reverse("test-accept-risks", kwargs={"pk": self.test_enabled.id}), + data=self._accepted_risks(["CVE-2024-1"]), + format="json", + ) + self.assertEqual(result.status_code, 201) + + def test_writer_can_accept_risks_on_findings(self): + client = self._client_for(self.writer_token) + result = client.post( + reverse("finding-accept-risks"), + data=self._accepted_risks(["CVE-2024-2"]), + format="json", + ) + self.assertEqual(result.status_code, 201) + + # --- Reader (no Risk_Acceptance) is forbidden --- + + def test_reader_forbidden_on_engagement(self): + client = self._client_for(self.reader_token) + result = client.post( + reverse("engagement-accept-risks", kwargs={"pk": self.engagement_enabled.id}), + data=self._accepted_risks(["CVE-2024-3"]), + format="json", + ) + self.assertEqual(result.status_code, 403) + + def test_reader_forbidden_on_test(self): + client = self._client_for(self.reader_token) + result = client.post( + reverse("test-accept-risks", kwargs={"pk": self.test_enabled.id}), + data=self._accepted_risks(["CVE-2024-4"]), + format="json", + ) + self.assertEqual(result.status_code, 403) + + def test_reader_gets_empty_result_on_findings(self): + client = self._client_for(self.reader_token) + result = client.post( + reverse("finding-accept-risks"), + data=self._accepted_risks(["CVE-2024-5"]), + format="json", + ) + # Mass endpoint returns 201 with empty results (no authorized engagements) + self.assertEqual(result.status_code, 201) + self.assertEqual(len(result.json()), 0) + + # --- enable_full_risk_acceptance=False blocks risk acceptance --- + + def test_engagement_blocked_when_full_risk_acceptance_disabled(self): + client = self._client_for(self.writer_token) + result = client.post( + reverse("engagement-accept-risks", kwargs={"pk": self.engagement_disabled.id}), + data=self._accepted_risks(["CVE-2024-100"]), + format="json", + ) + self.assertEqual(result.status_code, 403) + + def test_test_blocked_when_full_risk_acceptance_disabled(self): + client = self._client_for(self.writer_token) + result = client.post( + reverse("test-accept-risks", kwargs={"pk": self.test_disabled.id}), + data=self._accepted_risks(["CVE-2024-101"]), + format="json", + ) + self.assertEqual(result.status_code, 403) + + def test_mass_endpoint_skips_disabled_products(self): + client = self._client_for(self.writer_token) + # Use a CVE that exists only on the disabled product + result = client.post( + reverse("finding-accept-risks"), + data=self._accepted_risks(["CVE-2024-102"]), + format="json", + ) + self.assertEqual(result.status_code, 201) + # No risk acceptances created because the matching engagement's product has it disabled + self.assertEqual(len(result.json()), 0) + # Findings on disabled product remain unaccepted + self.assertEqual(self.engagement_disabled.unaccepted_open_findings.count(), 5) From eea3e47d0976a2ba1726bf0b3a988170b24210bc Mon Sep 17 00:00:00 2001 From: Jino Tesauro <53376807+Jino-T@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:53:36 -0500 Subject: [PATCH 08/30] Change to reactivating risk accepted findings (#14633) * change to reactivating risk accepted findinggs * Fix remove_finding BFLA and add test coverage (PR #14633) Gate the remove_finding POST branch on edit_mode so only the edit URL (requiring Risk_Acceptance permission) can process finding removals. Scope the finding lookup to risk_acceptance.accepted_findings to prevent cross-product blind enumeration via sequential IDs. Add 6 security tests covering: edit_mode guard, scoped lookup, cross-product IDOR, decorator enforcement, and positive regression. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- dojo/engagement/views.py | 5 +- unittests/test_permissions_audit.py | 261 ++++++++++++++++++++++++++++ unittests/test_risk_acceptance.py | 2 +- 3 files changed, 265 insertions(+), 3 deletions(-) diff --git a/dojo/engagement/views.py b/dojo/engagement/views.py index 1b94ace21f4..c955439814f 100644 --- a/dojo/engagement/views.py +++ b/dojo/engagement/views.py @@ -1442,9 +1442,10 @@ def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False): "Since you are not the note's author, it was not deleted.", extra_tags="alert-danger") - if "remove_finding" in request.POST: + if edit_mode and "remove_finding" in request.POST: finding = get_object_or_404( - Finding, pk=request.POST["remove_finding_id"]) + risk_acceptance.accepted_findings, + pk=request.POST["remove_finding_id"]) ra_helper.remove_finding_from_risk_acceptance(request.user, risk_acceptance, finding) diff --git a/unittests/test_permissions_audit.py b/unittests/test_permissions_audit.py index 8904c6270be..26ef3fb78c8 100644 --- a/unittests/test_permissions_audit.py +++ b/unittests/test_permissions_audit.py @@ -12,6 +12,7 @@ 8. Questionnaire cross-engagement IDOR (H1 #3571957) 9. Finding Templates exposure via find_template_to_apply (H1 #3577363) 10. Jira Epic BFLA - Reader cannot trigger update_jira_epic (H1 #3577193) +11. Risk Acceptance remove_finding: edit_mode guard + scoped finding lookup (PR #14633) """ import datetime @@ -701,6 +702,266 @@ def test_view_risk_acceptance_same_engagement(self): self.assertEqual(response.status_code, 200) +class TestRiskAcceptanceRemoveFindingGuard(DojoTestCase): + + """ + PR #14633: view_edit_risk_acceptance must: + 1. Only process 'remove_finding' when edit_mode is True (Writer+ via edit URL). + 2. Scope the finding lookup to risk_acceptance.accepted_findings (not global Finding). + + Prevents a Reader from removing findings via the view URL, and prevents + cross-product blind enumeration of finding IDs. + """ + + @classmethod + def setUpTestData(cls): + cls.reader_role = Role.objects.get(name="Reader") + cls.owner_role = Role.objects.get(name="Owner") + + # ── Product A ──────────────────────────────────────────────── + cls.product_type_a = Product_Type.objects.create( + name="RA Remove Guard Test PT A", + ) + cls.product_a = Product.objects.create( + name="RA Remove Guard Product A", + description="Test", + prod_type=cls.product_type_a, + enable_full_risk_acceptance=True, + ) + + # ── Product B (for cross-product IDOR test) ───────────────── + cls.product_type_b = Product_Type.objects.create( + name="RA Remove Guard Test PT B", + ) + cls.product_b = Product.objects.create( + name="RA Remove Guard Product B", + description="Test", + prod_type=cls.product_type_b, + enable_full_risk_acceptance=True, + ) + + # ── Users ──────────────────────────────────────────────────── + cls.reader_user_a = Dojo_User.objects.create_user( + username="ra_remove_reader_a", + password="testTEST1234!@#$", # noqa: S106 + is_active=True, + ) + cls.owner_user_a = Dojo_User.objects.create_user( + username="ra_remove_owner_a", + password="testTEST1234!@#$", # noqa: S106 + is_active=True, + ) + cls.owner_user_b = Dojo_User.objects.create_user( + username="ra_remove_owner_b", + password="testTEST1234!@#$", # noqa: S106 + is_active=True, + ) + + # ── Role assignments ───────────────────────────────────────── + Product_Member.objects.create( + product=cls.product_a, + user=cls.reader_user_a, + role=cls.reader_role, + ) + Product_Member.objects.create( + product=cls.product_a, + user=cls.owner_user_a, + role=cls.owner_role, + ) + Product_Member.objects.create( + product=cls.product_b, + user=cls.owner_user_b, + role=cls.owner_role, + ) + + # ── Product A: engagement, test, findings, risk acceptance ─── + cls.engagement_a = Engagement.objects.create( + name="RA Remove Guard Engagement A", + product=cls.product_a, + target_start=datetime.date(2024, 1, 1), + target_end=datetime.date(2024, 12, 31), + ) + test_type, _ = Test_Type.objects.get_or_create( + name="Manual Code Review", + ) + cls.test_a = Test.objects.create( + engagement=cls.engagement_a, + test_type=test_type, + target_start=timezone.now(), + target_end=timezone.now(), + ) + + # Finding that IS in the risk acceptance + cls.finding_a = Finding.objects.create( + title="RA Remove Guard Finding A", + test=cls.test_a, + severity="High", + numerical_severity="S1", + active=False, + risk_accepted=True, + reporter=cls.owner_user_a, + ) + + # Finding in same engagement but NOT in the risk acceptance + cls.finding_a_extra = Finding.objects.create( + title="RA Remove Guard Finding A Extra", + test=cls.test_a, + severity="Medium", + numerical_severity="S2", + active=True, + risk_accepted=False, + reporter=cls.owner_user_a, + ) + + cls.risk_acceptance_a = Risk_Acceptance.objects.create( + name="RA Remove Guard RA A", + owner=cls.owner_user_a, + ) + cls.risk_acceptance_a.accepted_findings.add(cls.finding_a) + cls.engagement_a.risk_acceptance.add(cls.risk_acceptance_a) + + # ── Product B: engagement, test, finding, risk acceptance ──── + cls.engagement_b = Engagement.objects.create( + name="RA Remove Guard Engagement B", + product=cls.product_b, + target_start=datetime.date(2024, 1, 1), + target_end=datetime.date(2024, 12, 31), + ) + cls.test_b = Test.objects.create( + engagement=cls.engagement_b, + test_type=test_type, + target_start=timezone.now(), + target_end=timezone.now(), + ) + cls.finding_b = Finding.objects.create( + title="RA Remove Guard Finding B", + test=cls.test_b, + severity="High", + numerical_severity="S1", + active=False, + risk_accepted=True, + reporter=cls.owner_user_b, + ) + + cls.risk_acceptance_b = Risk_Acceptance.objects.create( + name="RA Remove Guard RA B", + owner=cls.owner_user_b, + ) + cls.risk_acceptance_b.accepted_findings.add(cls.finding_b) + cls.engagement_b.risk_acceptance.add(cls.risk_acceptance_b) + + def _login(self, username): + client = Client() + client.login( + username=username, + password="testTEST1234!@#$", # noqa: S106 + ) + return client + + def _remove_finding_data(self, finding_id): + return { + "remove_finding": "Remove", + "remove_finding_id": finding_id, + } + + # ── Test 1: edit_mode guard (BFLA) ─────────────────────────────── + + def test_reader_cannot_remove_finding_via_view_url(self): + """Reader POSTing remove_finding to view URL must be silently ignored.""" + client = self._login("ra_remove_reader_a") + url = reverse("view_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + response = client.post(url, self._remove_finding_data(self.finding_a.id)) + # View still redirects (302) because errors=False, but finding is untouched + self.assertEqual(response.status_code, 302) + self.finding_a.refresh_from_db() + self.assertFalse(self.finding_a.active) + self.assertTrue(self.finding_a.risk_accepted) + self.assertTrue( + self.risk_acceptance_a.accepted_findings + .filter(pk=self.finding_a.pk) + .exists(), + ) + + # ── Test 2: positive regression (edit URL works) ───────────────── + + def test_owner_can_remove_finding_via_edit_url(self): + """Owner POSTing remove_finding to edit URL must succeed.""" + client = self._login("ra_remove_owner_a") + url = reverse("edit_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + response = client.post(url, self._remove_finding_data(self.finding_a.id)) + self.assertEqual(response.status_code, 302) + self.finding_a.refresh_from_db() + self.assertTrue(self.finding_a.active) + self.assertFalse(self.finding_a.risk_accepted) + self.assertFalse( + self.risk_acceptance_a.accepted_findings + .filter(pk=self.finding_a.pk) + .exists(), + ) + + # ── Test 3: scoped lookup (finding not in this RA) ─────────────── + + def test_finding_not_in_risk_acceptance_returns_404(self): + """Supplying a finding ID not in the RA's accepted_findings must 404.""" + client = self._login("ra_remove_owner_a") + url = reverse("edit_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + # finding_a_extra exists in the same engagement but is NOT accepted + response = client.post( + url, self._remove_finding_data(self.finding_a_extra.id), + ) + self.assertEqual(response.status_code, 404) + + # ── Test 4: cross-product IDOR ─────────────────────────────────── + + def test_cross_product_finding_id_rejected(self): + """Finding from Product B cannot be removed via Product A's RA.""" + client = self._login("ra_remove_owner_a") + url = reverse("edit_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + response = client.post( + url, self._remove_finding_data(self.finding_b.id), + ) + self.assertEqual(response.status_code, 404) + # Product B's finding must remain untouched + self.finding_b.refresh_from_db() + self.assertFalse(self.finding_b.active) + self.assertTrue(self.finding_b.risk_accepted) + + # ── Test 5: Reader blocked by decorator on edit URL ────────────── + + def test_reader_blocked_on_edit_url_by_decorator(self): + """Reader lacks Risk_Acceptance permission — edit URL itself is denied.""" + client = self._login("ra_remove_reader_a") + url = reverse("edit_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + response = client.post(url, self._remove_finding_data(self.finding_a.id)) + # PermissionDenied raised; custom handler403 returns 400 (DD bug) + self.assertIn(response.status_code, [400, 403]) + # Finding must remain untouched + self.finding_a.refresh_from_db() + self.assertFalse(self.finding_a.active) + self.assertTrue(self.finding_a.risk_accepted) + + # ── Test 6: nonexistent finding ID ─────────────────────────────── + + def test_nonexistent_finding_id_returns_404(self): + """A bogus finding ID must produce 404 from the scoped lookup.""" + client = self._login("ra_remove_owner_a") + url = reverse("edit_risk_acceptance", args=( + self.engagement_a.id, self.risk_acceptance_a.id, + )) + response = client.post(url, self._remove_finding_data(999999)) + self.assertEqual(response.status_code, 404) + + class TestEngagementPresetsCrossProductIDOR(DojoTestCase): """ diff --git a/unittests/test_risk_acceptance.py b/unittests/test_risk_acceptance.py index c0613bd49ee..628624b6db1 100644 --- a/unittests/test_risk_acceptance.py +++ b/unittests/test_risk_acceptance.py @@ -111,7 +111,7 @@ def test_remove_findings_from_risk_acceptance_findings_active(self): data = copy.copy(self.data_remove_finding_from_ra) data["remove_finding_id"] = 2 ra = Risk_Acceptance.objects.last() - response = self.client.post(reverse("view_risk_acceptance", args=(1, ra.id)), data) + response = self.client.post(reverse("edit_risk_acceptance", args=(1, ra.id)), data) self.assertEqual(302, response.status_code, response.content[:1000]) self.assert_all_active_not_risk_accepted(Finding.objects.filter(id=2)) self.assert_all_inactive_risk_accepted(Finding.objects.filter(id=3)) From 9d661d72e9f92b8e6e3eef334afb7257dd968fb1 Mon Sep 17 00:00:00 2001 From: Jino Tesauro <53376807+Jino-T@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:54:22 -0500 Subject: [PATCH 09/30] Add permission checks for moving engagements between products (#14634) * change-to-moving-engagements * fix-migration-issue: * Revert PR #14634 changes (editable=False approach) Reverting the approach of making Engagement.product editable=False and splitting serializers. Will replace with proper permission checks on the destination product when moving engagements. Co-Authored-By: Claude Opus 4.6 (1M context) * Add permission check on destination product when moving engagements When a user changes an engagement's product (via API PUT/PATCH or the UI edit form), verify they have Engagement_Edit permission on the destination product. Previously only the source product was checked, allowing users to move engagements to products they lack write access to. - API: EngagementSerializer.validate() checks destination product permission on update, following the ProductMemberSerializer pattern - UI: edit_engagement() view checks destination product permission before saving - Tests: 8 new tests covering PATCH, PUT, and UI paths for both authorized and unauthorized product moves Co-Authored-By: Claude Opus 4.6 (1M context) * Fix UI test: form queryset already rejects unauthorized products The EngForm product queryset is filtered to authorized products, so submitting an unauthorized product fails form validation (200) before the view-level permission check runs. Update the test to accept both 200 and 403 -- the key assertion is that the engagement does not move. Co-Authored-By: Claude Opus 4.6 (1M context) * Fix ruff lint: docstring formatting Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- dojo/api_v2/serializers.py | 12 +++ dojo/engagement/views.py | 6 ++ unittests/test_permissions_audit.py | 159 ++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+) diff --git a/dojo/api_v2/serializers.py b/dojo/api_v2/serializers.py index 342287aba94..7c9bbd6ae79 100644 --- a/dojo/api_v2/serializers.py +++ b/dojo/api_v2/serializers.py @@ -1123,6 +1123,18 @@ def validate(self, data): if data.get("target_start") > data.get("target_end"): msg = "Your target start date exceeds your target end date" raise serializers.ValidationError(msg) + if ( + self.instance is not None + and "product" in data + and data.get("product") != self.instance.product + and not user_has_permission( + self.context["request"].user, + data.get("product"), + Permissions.Engagement_Edit, + ) + ): + msg = "You are not permitted to edit engagements in the destination product" + raise PermissionDenied(msg) return data def build_relational_field(self, field_name, relation_info): diff --git a/dojo/engagement/views.py b/dojo/engagement/views.py index c955439814f..ca36d81c0dd 100644 --- a/dojo/engagement/views.py +++ b/dojo/engagement/views.py @@ -286,6 +286,12 @@ def edit_engagement(request, eid): if form.is_valid(): # first save engagement details new_status = form.cleaned_data.get("status") + if form.cleaned_data.get("product") != engagement.product: + user_has_permission_or_403( + request.user, + form.cleaned_data.get("product"), + Permissions.Engagement_Edit, + ) engagement.product = form.cleaned_data.get("product") engagement = form.save(commit=False) if (new_status in {"Cancelled", "Completed"}): diff --git a/unittests/test_permissions_audit.py b/unittests/test_permissions_audit.py index 26ef3fb78c8..9d4ca0a746e 100644 --- a/unittests/test_permissions_audit.py +++ b/unittests/test_permissions_audit.py @@ -1686,3 +1686,162 @@ def test_risk_acceptance_download_proof_writer_allowed(self): # Clean up uploaded file self.risk_acceptance.path.delete(save=True) + + +class TestEngagementMovePermission(DojoTestCase): + + """Moving an engagement to another product requires Engagement_Edit on the destination.""" + + @classmethod + def setUpTestData(cls): + cls.owner_role = Role.objects.get(name="Owner") + cls.product_type = Product_Type.objects.create(name="Eng Move Test PT") + + cls.product_a = Product.objects.create( + name="Eng Move Product A", description="Test", prod_type=cls.product_type, + ) + cls.product_b = Product.objects.create( + name="Eng Move Product B", description="Test", prod_type=cls.product_type, + ) + cls.product_c = Product.objects.create( + name="Eng Move Product C", description="Test", prod_type=cls.product_type, + ) + + cls.user = Dojo_User.objects.create_user( + username="eng_move_owner", + password="testTEST1234!@#$", # noqa: S106 + is_active=True, + ) + Product_Member.objects.create(product=cls.product_a, user=cls.user, role=cls.owner_role) + # No membership on product_b -- user cannot move engagements there + Product_Member.objects.create(product=cls.product_c, user=cls.user, role=cls.owner_role) + + def setUp(self): + self.engagement = Engagement.objects.create( + name="Move Test Engagement", + product=self.product_a, + target_start=datetime.date.today(), + target_end=datetime.date.today(), + ) + + def _api_client(self): + token, _ = Token.objects.get_or_create(user=self.user) + client = APIClient() + client.credentials(HTTP_AUTHORIZATION="Token " + token.key) + return client + + def _ui_client(self): + client = Client() + client.login(username="eng_move_owner", password="testTEST1234!@#$") # noqa: S106 + return client + + # ── API: PATCH ──────────────────────────────────────────────────── + + def test_api_patch_move_to_authorized_product(self): + """PATCH with product the user has access to should succeed.""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + response = client.patch(url, {"product": self.product_c.id}, format="json") + self.assertEqual(response.status_code, 200, response.content) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_c) + + def test_api_patch_move_to_unauthorized_product(self): + """PATCH with product the user lacks access to should be denied.""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + response = client.patch(url, {"product": self.product_b.id}, format="json") + self.assertEqual(response.status_code, 403, response.content) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_a) + + def test_api_patch_same_product(self): + """PATCH with the same product should succeed without extra permission check.""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + response = client.patch(url, {"product": self.product_a.id}, format="json") + self.assertEqual(response.status_code, 200, response.content) + + def test_api_patch_without_product_field(self): + """PATCH without product field should succeed (no spurious check).""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + response = client.patch(url, {"version": "1.0"}, format="json") + self.assertEqual(response.status_code, 200, response.content) + + # ── API: PUT ────────────────────────────────────────────────────── + + def test_api_put_move_to_authorized_product(self): + """PUT with product the user has access to should succeed.""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + payload = { + "name": "Move Test Engagement", + "product": self.product_c.id, + "target_start": str(datetime.date.today()), + "target_end": str(datetime.date.today()), + "engagement_type": "Interactive", + "status": "Not Started", + } + response = client.put(url, payload, format="json") + self.assertEqual(response.status_code, 200, response.content) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_c) + + def test_api_put_move_to_unauthorized_product(self): + """PUT with product the user lacks access to should be denied.""" + client = self._api_client() + url = reverse("engagement-detail", args=(self.engagement.id,)) + payload = { + "name": "Move Test Engagement", + "product": self.product_b.id, + "target_start": str(datetime.date.today()), + "target_end": str(datetime.date.today()), + "engagement_type": "Interactive", + "status": "Not Started", + } + response = client.put(url, payload, format="json") + self.assertEqual(response.status_code, 403, response.content) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_a) + + # ── UI ──────────────────────────────────────────────────────────── + + def test_ui_move_to_authorized_product(self): + """Edit engagement form moving to authorized product should succeed.""" + client = self._ui_client() + url = reverse("edit_engagement", args=(self.engagement.id,)) + form_data = { + "product": self.product_c.id, + "target_start": datetime.date.today().strftime("%Y-%m-%d"), + "target_end": datetime.date.today().strftime("%Y-%m-%d"), + "lead": self.user.id, + "status": "Not Started", + } + response = client.post(url, form_data) + self.assertIn(response.status_code, [200, 302], response.content[:500]) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_c) + + def test_ui_move_to_unauthorized_product(self): + """ + Edit engagement form moving to unauthorized product should be denied. + + The form's product queryset is filtered to authorized products, so + submitting an unauthorized product fails form validation (200 with + errors) before the view-level permission check runs. Either way the + engagement must NOT move. + """ + client = self._ui_client() + url = reverse("edit_engagement", args=(self.engagement.id,)) + form_data = { + "product": self.product_b.id, + "target_start": datetime.date.today().strftime("%Y-%m-%d"), + "target_end": datetime.date.today().strftime("%Y-%m-%d"), + "lead": self.user.id, + "status": "Not Started", + } + response = client.post(url, form_data) + self.assertIn(response.status_code, [200, 403]) + self.engagement.refresh_from_db() + self.assertEqual(self.engagement.product, self.product_a) From 4df60d02b88e92612d0be15e72685c6a1216b913 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:30:27 -0600 Subject: [PATCH 10/30] Add CLAUDE.md with module reorganization playbook Documents a repeatable 10-phase process for reorganizing domain modules (finding, test, engagement, product, product_type) to match the dojo/url/ reference pattern. Includes service-layer extraction guidance to support the long-term goal of removing the classic UI and going fully API-based. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 250 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000000..14e1db91640 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,250 @@ +# DefectDojo Development Guide + +## Project Overview + +DefectDojo is a Django application (`dojo` app) for vulnerability management. The codebase is undergoing a modular reorganization to move from monolithic files toward self-contained domain modules. + +## Module Reorganization + +### Reference Pattern: `dojo/url/` + +All domain modules should match the structure of `dojo/url/`. This is the canonical example of a fully reorganized module. + +``` +dojo/{module}/ +├── __init__.py # import dojo.{module}.admin # noqa: F401 +├── models.py # Domain models, constants, factory methods +├── admin.py # @admin.register() for the module's models +├── services.py # Business logic (no HTTP concerns) +├── queries.py # Complex DB aggregations/annotations +├── signals.py # Django signal handlers +├── [manager.py] # Custom QuerySet/Manager if needed +├── [validators.py] # Field-level validators if needed +├── [helpers.py] # Async task wrappers, tag propagation, etc. +├── ui/ +│ ├── __init__.py # Empty +│ ├── forms.py # Django ModelForms +│ ├── filters.py # UI-layer django-filter classes +│ ├── views.py # Thin view functions — delegates to services.py +│ └── urls.py # URL routing +└── api/ + ├── __init__.py # path = "{module}" + ├── serializer.py # DRF serializers + ├── views.py # API ViewSets — delegates to services.py + ├── filters.py # API-layer filters + └── urls.py # add_{module}_urls(router) registration +``` + +### Architecture Principles + +**Long-term goal**: Remove the classic Django template UI and make the app fully API-based. The `ui/` subdirectory should be deletable with zero impact on `api/` or shared business logic. + +**services.py is the critical layer**: Both `ui/views.py` and `api/views.py` call `services.py` for business logic. Services accept domain objects and primitives — never request/response objects, forms, or serializers. + +**Backward-compatible re-exports**: When moving code out of monolithic files (`dojo/models.py`, `dojo/forms.py`, `dojo/filters.py`, `dojo/api_v2/serializers.py`, `dojo/api_v2/views.py`), always leave a re-export at the original location: +```python +from dojo.{module}.models import {Model} # noqa: F401 -- backward compat +``` +Never remove re-exports until all consumers are updated in a dedicated cleanup pass. + +### Current State + +Modules in various stages of reorganization: + +| Module | models.py | services.py | ui/ | api/ | Status | +|--------|-----------|-------------|-----|------|--------| +| **url** | In module | N/A | Done | Done | **Complete** | +| **location** | In module | N/A | N/A | Done | **Complete** | +| **product_type** | In dojo/models.py | Missing | Partial (views at root) | In dojo/api_v2/ | Needs work | +| **test** | In dojo/models.py | Missing | Partial (views at root) | In dojo/api_v2/ | Needs work | +| **engagement** | In dojo/models.py | Partial (32 lines) | Partial (views at root) | In dojo/api_v2/ | Needs work | +| **product** | In dojo/models.py | Missing | Partial (views at root) | In dojo/api_v2/ | Needs work | +| **finding** | In dojo/models.py | Missing | Partial (views at root) | In dojo/api_v2/ | Needs work | + +### Monolithic Files Being Decomposed + +These files still contain code for multiple modules. Extract code to the target module's subdirectory and leave a re-export stub. + +- `dojo/models.py` (4,973 lines) — All model definitions +- `dojo/forms.py` (4,127 lines) — All Django forms +- `dojo/filters.py` (4,016 lines) — All UI and API filter classes +- `dojo/api_v2/serializers.py` (3,387 lines) — All DRF serializers +- `dojo/api_v2/views.py` (3,519 lines) — All API viewsets + +--- + +## Reorganization Playbook + +When asked to reorganize a module, follow these phases in order. Each phase should be independently verifiable. + +### Phase 0: Pre-Flight (Read-Only) + +Before any changes, identify all code to extract: + +```bash +# 1. Model classes and line ranges in dojo/models.py +grep -n "class {Model}" dojo/models.py + +# 2. Form classes in dojo/forms.py +grep -n "class.*{Module}" dojo/forms.py +grep -n "model = {Model}" dojo/forms.py + +# 3. Filter classes in dojo/filters.py +grep -n "class.*{Module}\|class.*{Model}" dojo/filters.py + +# 4. Serializer classes +grep -n "class.*{Model}" dojo/api_v2/serializers.py + +# 5. ViewSet classes +grep -n "class.*{Model}\|class.*{Module}" dojo/api_v2/views.py + +# 6. Admin registrations +grep -n "admin.site.register({Model}" dojo/models.py + +# 7. All import sites (to verify backward compat) +grep -rn "from dojo.models import.*{Model}" dojo/ unittests/ + +# 8. Business logic in current views +# Scan dojo/{module}/views.py for: .save(), .delete(), create_notification(), +# jira_helper.*, dojo_dispatch_task(), multi-model workflows +``` + +### Phase 1: Extract Models + +1. Create `dojo/{module}/models.py` with the model class(es) and associated constants +2. Create `dojo/{module}/admin.py` with `admin.site.register()` calls (remove from `dojo/models.py`) +3. Update `dojo/{module}/__init__.py` to `import dojo.{module}.admin # noqa: F401` +4. Add re-exports in `dojo/models.py` +5. Remove original model code (keep re-export line) + +**Import rules for models.py:** +- Upward FKs (e.g., Test -> Engagement): import from `dojo.models` if not yet extracted, or `dojo.{module}.models` if already extracted +- Downward references (e.g., Product_Type querying Finding): use lazy imports inside method bodies +- Shared utilities (`copy_model_util`, `_manage_inherited_tags`, `get_current_date`, etc.): import from `dojo.models` +- Do NOT set `app_label` in Meta — all models inherit `dojo` app_label automatically + +**Verify:** +```bash +python manage.py check +python manage.py makemigrations --check +python -c "from dojo.{module}.models import {Model}" +python -c "from dojo.models import {Model}" +``` + +### Phase 2: Extract Services + +Create `dojo/{module}/services.py` with business logic extracted from UI views. + +**What belongs in services.py:** +- State transitions (close, reopen, status changes) +- Multi-step creation/update workflows +- External integration calls (JIRA, GitHub) +- Notification dispatching +- Copy/clone operations +- Bulk operations +- Merge operations + +**What stays in views:** +- HTTP request/response handling +- Form instantiation and validation +- Serialization/deserialization +- Authorization checks (`@user_is_authorized`, `user_has_permission_or_403`) +- Template rendering, redirects +- Pagination, breadcrumbs + +**Service function pattern:** +```python +def close_engagement(engagement: Engagement, user: User) -> Engagement: + engagement.active = False + engagement.status = "Completed" + engagement.save() + if jira_helper.get_jira_project(engagement): + dojo_dispatch_task(jira_helper.close_epic, engagement.id, push_to_jira=True) + return engagement +``` + +Update UI views and API viewsets to call the service instead of containing logic inline. + +### Phase 3: Extract Forms to `ui/forms.py` + +1. Create `dojo/{module}/ui/__init__.py` (empty) +2. Create `dojo/{module}/ui/forms.py` — move form classes from `dojo/forms.py` +3. Add re-exports in `dojo/forms.py` + +### Phase 4: Extract UI Filters to `ui/filters.py` + +1. Create `dojo/{module}/ui/filters.py` — move module-specific filters from `dojo/filters.py` +2. Shared base classes (`DojoFilter`, `DateRangeFilter`, `ReportBooleanFilter`) stay in `dojo/filters.py` +3. Add re-exports in `dojo/filters.py` + +### Phase 5: Move UI Views/URLs into `ui/` + +1. Move `dojo/{module}/views.py` -> `dojo/{module}/ui/views.py` +2. Move `dojo/{module}/urls.py` -> `dojo/{module}/ui/urls.py` +3. Update URL imports: + - product: update `dojo/asset/urls.py` + - product_type: update `dojo/organization/urls.py` + - others: update the include in `dojo/urls.py` + +### Phase 6: Extract API Serializers to `api/serializer.py` + +1. Create `dojo/{module}/api/__init__.py` with `path = "{module}"` +2. Create `dojo/{module}/api/serializer.py` — move from `dojo/api_v2/serializers.py` +3. Add re-exports in `dojo/api_v2/serializers.py` + +### Phase 7: Extract API Filters to `api/filters.py` + +1. Create `dojo/{module}/api/filters.py` — move `Api{Model}Filter` from `dojo/filters.py` +2. Add re-exports + +### Phase 8: Extract API ViewSets to `api/views.py` + +1. Create `dojo/{module}/api/views.py` — move from `dojo/api_v2/views.py` +2. Add re-exports in `dojo/api_v2/views.py` + +### Phase 9: Extract API URL Registration + +1. Create `dojo/{module}/api/urls.py`: + ```python + from dojo.{module}.api import path + from dojo.{module}.api.views import {ViewSet} + + def add_{module}_urls(router): + router.register(path, {ViewSet}, path) + return router + ``` +2. Update `dojo/urls.py` — replace `v2_api.register(...)` with `add_{module}_urls(v2_api)` + +### After Each Phase: Verify + +```bash +python manage.py check +python manage.py makemigrations --check +python -m pytest unittests/ -x --timeout=120 +``` + +--- + +## Cross-Module Dependencies + +The model hierarchy is: Product_Type -> Product -> Engagement -> Test -> Finding + +Extract in this order (top to bottom) so that upward FKs can import from already-extracted modules. The recommended order is: product_type, test, engagement, product, finding. + +For downward references (e.g., Product_Type's cached properties querying Finding), always use lazy imports: +```python +@cached_property +def critical_present(self): + from dojo.models import Finding # lazy import + return Finding.objects.filter(test__engagement__product__prod_type=self, severity="Critical").exists() +``` + +--- + +## Key Technical Details + +- **Single Django app**: Everything is under `app_label = "dojo"`. Moving models to subdirectories does NOT require migration changes. +- **Model discovery**: Triggered by `__init__.py` importing `admin.py`, which imports `models.py`. This is the same chain `dojo/url/` uses. +- **Signal registration**: Handled in `dojo/apps.py` via `import dojo.{module}.signals`. Already set up for test, engagement, product, product_type. +- **Watson search**: Uses `self.get_model("Product")` in `apps.py` — works via Django's model registry regardless of file location. +- **Admin registration**: Currently at the bottom of `dojo/models.py` (lines 4888-4973). Must be moved to `{module}/admin.py` and removed from `dojo/models.py` to avoid `AlreadyRegistered` errors. From 56c84a72221f6f307084cde31f0da780356b2200 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:33:01 -0600 Subject: [PATCH 11/30] Update CLAUDE.md --- CLAUDE.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 14e1db91640..64279977619 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,6 @@ dojo/{module}/ ### Architecture Principles -**Long-term goal**: Remove the classic Django template UI and make the app fully API-based. The `ui/` subdirectory should be deletable with zero impact on `api/` or shared business logic. **services.py is the critical layer**: Both `ui/views.py` and `api/views.py` call `services.py` for business logic. Services accept domain objects and primitives — never request/response objects, forms, or serializers. From c1b2526ee495fed3a14bb81c065be51a7a986e39 Mon Sep 17 00:00:00 2001 From: manuelsommer <47991713+manuel-sommer@users.noreply.github.com> Date: Fri, 17 Apr 2026 06:40:54 +0200 Subject: [PATCH 12/30] :tada: add fix_available and fix_version to govulncheck (#14681) --- dojo/tools/govulncheck/parser.py | 24 +++++++++++++++++++++- unittests/tools/test_govulncheck_parser.py | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/dojo/tools/govulncheck/parser.py b/dojo/tools/govulncheck/parser.py index 4bdea0908ff..9214218d68b 100644 --- a/dojo/tools/govulncheck/parser.py +++ b/dojo/tools/govulncheck/parser.py @@ -40,6 +40,22 @@ def get_location(data, node): def get_version(data, node): return data["Requires"]["Modules"][str(node)]["Version"] + @staticmethod + def get_fix_info(affected_ranges): + for r in affected_ranges: + for event in r.get("events", []): + if "fixed" in event: + return True, event["fixed"] + return False, "" + + @staticmethod + def get_introduced_version(affected_ranges): + for r in affected_ranges: + for event in r.get("events", []): + if "introduced" in event: + return event["introduced"] + return "" + def get_finding_trace_info(self, data, osv_id): # Browse the findings to look for matching OSV-id. If the OSV-id is matching, extract traces. trace_info_strs = [] @@ -202,8 +218,12 @@ def get_findings(self, scan_file, test): else: title = f"{osv_data['id']} - {affected_package['name']}" - affected_version = self.get_affected_version(data, osv_data["id"]) + fix_available, fix_version = self.get_fix_info(affected_ranges) + affected_version = ( + self.get_affected_version(data, osv_data["id"]) + or self.get_introduced_version(affected_ranges) + ) severity = elem["osv"].get("severity", SEVERITY) d = { @@ -215,6 +235,8 @@ def get_findings(self, scan_file, test): "description": description, "impact": impact, "references": references, + "fix_available": fix_available, + "fix_version": fix_version, "file_path": path, "url": db_specific_url, "unique_id_from_tool": osv_id, diff --git a/unittests/tools/test_govulncheck_parser.py b/unittests/tools/test_govulncheck_parser.py index 90499dc48fd..3b0a8efc474 100644 --- a/unittests/tools/test_govulncheck_parser.py +++ b/unittests/tools/test_govulncheck_parser.py @@ -127,6 +127,8 @@ def test_parse_new_version_many_findings_custom_severity(self): self.assertIsNotNone(finding.impact) self.assertIsNotNone(finding.description) self.assertIsNotNone(finding.references) + self.assertTrue(finding.fix_available) + self.assertEqual("0.3.8", finding.fix_version) def test_parse_issue_14642(self): with (get_unit_tests_scans_path("govulncheck") / "issue_14642.json").open(encoding="utf-8") as testfile: From 4a0abbd7e91ac68da2abc22d9bed2c905e843660 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:41:04 -0600 Subject: [PATCH 13/30] fix: clean up template rendering for endpoint user fields (#14682) Remove unnecessary template filter from auditor and mitigated_by fields in the endpoints snippet to align with standard Django template rendering conventions. Co-authored-by: Claude Opus 4.6 (1M context) --- dojo/templates/dojo/snippets/endpoints.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dojo/templates/dojo/snippets/endpoints.html b/dojo/templates/dojo/snippets/endpoints.html index e7d4d740ea8..84f10304d00 100644 --- a/dojo/templates/dojo/snippets/endpoints.html +++ b/dojo/templates/dojo/snippets/endpoints.html @@ -72,13 +72,13 @@
Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }})
{% if V3_FEATURE_LOCATIONS %} {{ endpoint.location }}{% if endpoint.is_broken %} 🚩{% endif %} {{ endpoint.status }} - {{ endpoint.auditor|safe }} + {{ endpoint.auditor }} {{ endpoint.audit_time|date }} {% else %} - {% comment %} TODO: Delete this after the move to Locations {% endcomment %} + {% comment %} TODO: Delete this after the move to Locations {% endcomment %} {{ endpoint }}{% if endpoint.endpoint.is_broken %} 🚩{% endif %} {{ endpoint.status }} - {{ endpoint.mitigated_by|safe }} + {{ endpoint.mitigated_by }} {{ endpoint.mitigated_time|date }} {% endif %} @@ -256,7 +256,7 @@

Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }}) {% include "dojo/snippets/tags.html" with tags=endpoint.location.tags.all %} {{ endpoint.get_status_display }} - {{ endpoint.auditor|safe }} + {{ endpoint.auditor }} {{ endpoint.audit_time|date }} {% else %} {% comment %} TODO: Delete this after the move to Locations {% endcomment %} @@ -265,7 +265,7 @@

Mitigated Endpoints / Systems ({{ finding.mitigated_endpoint_count }}) {% include "dojo/snippets/tags.html" with tags=endpoint.endpoint.tags.all %} {{ endpoint.status }} - {{ endpoint.mitigated_by|safe }} + {{ endpoint.mitigated_by }} {{ endpoint.mitigated_time|date }} {% endif %} From 4decd88fe513bdf325b2ce536d8d197ef834c36f Mon Sep 17 00:00:00 2001 From: Jino Tesauro <53376807+Jino-T@users.noreply.github.com> Date: Thu, 16 Apr 2026 23:48:21 -0500 Subject: [PATCH 14/30] Validate consistency between ID-based and name-based identifiers in import/reimport (#14636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix reimport-scan API authorization bypass via conflicting identifiers Validate that ID-resolved objects (test, engagement) are consistent with name-based identifiers (product_name, engagement_name) in both the permission check layer and the AutoCreateContextManager resolution layer. This prevents an attacker from passing their own engagement/test ID to satisfy the permission check while using name-based fields to target a victim's product. Co-Authored-By: Claude Opus 4.6 (1M context) * Use ID-based comparisons and add engagement_name check to import - Switch permission checks to use ID comparisons (product_id, engagement_id) where resolved objects are available, with name fallback for unresolved cases - Add engagement_name validation to UserHasImportPermission (was missing) - Fix ruff string quoting in auto_create_context.py Co-Authored-By: Claude Opus 4.6 (1M context) * Strip undeclared engagement field in reimport permission check The engagement field is not declared on ReImportScanSerializer and gets stripped during validation. The permission check must also strip it so it resolves targets the same way execution does — by name, not by a stale engagement ID from request.data. Update test to verify the engagement param is ignored and permission is checked against the name-resolved target. Co-Authored-By: Claude Opus 4.6 (1M context) * Fix information disclosure in conflict validation error messages Replace error messages that leaked resolved object names (product names, engagement names) with generic messages. An attacker could enumerate object names by sending conflicting ID-based and name-based identifiers and reading the detailed error responses. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Paul Osinski <42211303+paulOsinski@users.noreply.github.com> --- dojo/api_v2/permissions.py | 33 +++++++++- dojo/importers/auto_create_context.py | 6 ++ unittests/test_rest_framework.py | 89 +++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 807bbcf7b2a..10e012ede5b 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -473,7 +473,13 @@ def has_permission(self, request, view): # Raise an explicit drf exception here raise ValidationError(e) if engagement := converted_dict.get("engagement"): - # existing engagement, nothing special to check + # Validate the resolved engagement's parent chain matches any provided identifiers + if (product := converted_dict.get("product")) and engagement.product_id != product.id: + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." + raise ValidationError(msg) + if (engagement_name := converted_dict.get("engagement_name")) and engagement.name != engagement_name: + msg = "The provided identifiers are inconsistent — the engagement name does not match the specified engagement." + raise ValidationError(msg) return user_has_permission( request.user, engagement, Permissions.Import_Scan_Result, ) @@ -764,6 +770,11 @@ def has_permission(self, request, view): try: converted_dict = auto_create.convert_querydict_to_dict(request.data) auto_create.process_import_meta_data_from_dict(converted_dict) + # engagement is not a declared field on ReImportScanSerializer and will be + # stripped during validation — don't use it in the permission check either, + # so the permission check resolves targets the same way execution does + converted_dict.pop("engagement", None) + converted_dict.pop("engagement_id", None) # Get an existing product converted_dict["product_type"] = auto_create.get_target_product_type_if_exists(**converted_dict) converted_dict["product"] = auto_create.get_target_product_if_exists(**converted_dict) @@ -774,7 +785,20 @@ def has_permission(self, request, view): raise ValidationError(e) if test := converted_dict.get("test"): - # existing test, nothing special to check + # Validate the resolved test's parent chain matches any provided identifiers + if (product := converted_dict.get("product")) and test.engagement.product_id != product.id: + msg = "The provided identifiers are inconsistent — the test does not belong to the specified product." + raise ValidationError(msg) + if (engagement := converted_dict.get("engagement")) and test.engagement_id != engagement.id: + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." + raise ValidationError(msg) + # Also validate by name when the objects were not resolved (e.g. names that match no existing record) + if not converted_dict.get("product") and (product_name := converted_dict.get("product_name")) and test.engagement.product.name != product_name: + msg = "The provided identifiers are inconsistent — the test does not belong to the specified product." + raise ValidationError(msg) + if not converted_dict.get("engagement") and (engagement_name := converted_dict.get("engagement_name")) and test.engagement.name != engagement_name: + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." + raise ValidationError(msg) return user_has_permission( request.user, test, Permissions.Import_Scan_Result, ) @@ -1181,7 +1205,10 @@ def check_auto_create_permission( raise ValidationError(msg) if engagement: - # existing engagement, nothing special to check + # Validate the resolved engagement's parent chain matches any provided names + if product is not None and engagement.product_id != product.id: + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." + raise ValidationError(msg) return user_has_permission( user, engagement, Permissions.Import_Scan_Result, ) diff --git a/dojo/importers/auto_create_context.py b/dojo/importers/auto_create_context.py index 26d37ae65b0..916bbea056e 100644 --- a/dojo/importers/auto_create_context.py +++ b/dojo/importers/auto_create_context.py @@ -181,6 +181,9 @@ def get_target_engagement_if_exists( """ if engagement := get_object_or_none(Engagement, pk=engagement_id): logger.debug("Using existing engagement by id: %s", engagement_id) + if product is not None and engagement.product_id != product.id: + msg = "The provided identifiers are inconsistent — the engagement does not belong to the specified product." + raise ValueError(msg) return engagement # if there's no product, then for sure there's no engagement either if product is None: @@ -203,6 +206,9 @@ def get_target_test_if_exists( """ if test := get_object_or_none(Test, pk=test_id): logger.debug("Using existing Test by id: %s", test_id) + if engagement is not None and test.engagement_id != engagement.id: + msg = "The provided identifiers are inconsistent — the test does not belong to the specified engagement." + raise ValueError(msg) return test # If the engagement is not supplied, we cannot do anything if not engagement: diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 95807d4c536..7199e08c126 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -3429,6 +3429,95 @@ def test_create_not_authorized_product_name_engagement_name_scan_type_title(self importer_mock.assert_not_called() reimporter_mock.assert_not_called() + # Security tests: verify that conflicting ID-based and name-based identifiers are rejected + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + @patch("dojo.api_v2.permissions.user_has_permission") + def test_reimport_engagement_param_ignored_permission_checked_on_name_resolved_target(self, mock, importer_mock, reimporter_mock): + """ + Engagement is not a declared field on ReImportScanSerializer — verify + the permission check uses the name-resolved target, not the engagement param. + """ + mock.return_value = False + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # engagement=1 belongs to Product 2 Engagement 1, but it should be ignored + "engagement": 1, + # These names resolve to Product 2's Engagement 4 -> Test 4 + "product_name": "Security How-to", + "engagement_name": "April monthly engagement", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(403, response.status_code, response.content[:1000]) + # Permission must be checked on name-resolved Test 4 (in Engagement 4), + # NOT on Test 3 (which belongs to the engagement=1 param) + mock.assert_called_with(User.objects.get(username="admin"), + Test.objects.get(id=4), + Permissions.Import_Scan_Result) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + def test_reimport_with_test_id_mismatched_product_name_is_rejected(self, importer_mock, reimporter_mock): + """Sending test ID from one product with product_name from another must be rejected.""" + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # Test 3 belongs to Engagement 1 -> Product 2 ("Security How-to") + "test": 3, + # But product_name points to Product 1 ("Python How-to") + "product_name": "Python How-to", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(400, response.status_code, response.content[:1000]) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + + @patch("dojo.importers.default_reimporter.DefaultReImporter.process_scan") + @patch("dojo.importers.default_importer.DefaultImporter.process_scan") + def test_reimport_with_test_id_mismatched_engagement_name_is_rejected(self, importer_mock, reimporter_mock): + """Sending test ID from one engagement with engagement_name from another must be rejected.""" + importer_mock.return_value = IMPORTER_MOCK_RETURN_VALUE + reimporter_mock.return_value = REIMPORTER_MOCK_RETURN_VALUE + + with Path("tests/zap_sample.xml").open(encoding="utf-8") as testfile: + payload = { + "minimum_severity": "Low", + "active": True, + "verified": True, + "scan_type": "ZAP Scan", + "file": testfile, + # Test 3 belongs to Engagement 1 ("1st Quarter Engagement") + "test": 3, + # But engagement_name points to a different engagement + "product_name": "Security How-to", + "engagement_name": "April monthly engagement", + "version": "1.0.0", + } + response = self.client.post(self.url, payload) + self.assertEqual(400, response.status_code, response.content[:1000]) + importer_mock.assert_not_called() + reimporter_mock.assert_not_called() + @versioned_fixtures class ProductTypeTest(BaseClass.BaseClassTest): From c51d018324dfebaa49ee5b84a188a99b0194828c Mon Sep 17 00:00:00 2001 From: Paul Osinski Date: Fri, 17 Apr 2026 13:21:23 -0400 Subject: [PATCH 15/30] tests: read raw template string --- unittests/test_pdf_report_rendering.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/unittests/test_pdf_report_rendering.py b/unittests/test_pdf_report_rendering.py index 283fd4f6272..f2d794baa81 100644 --- a/unittests/test_pdf_report_rendering.py +++ b/unittests/test_pdf_report_rendering.py @@ -192,10 +192,9 @@ def test_long_unbroken_string_in_report_field(self): def test_report_base_css_has_overflow_wrap(self): """The report base template must include overflow-wrap for text wrapping.""" template = self.django_engine.get_template("report_base.html") - # Render with minimal context to get the CSS - html = template.render({"report_name": "Test"}) + source = template.template.source - self.assertIn("overflow-wrap: break-word", html) + self.assertIn("overflow-wrap: break-word", source) def test_report_base_css_styles_nested_pre(self): """ @@ -203,10 +202,10 @@ def test_report_base_css_styles_nested_pre(self): nested
 elements from breaking out of margins.
         """
         template = self.django_engine.get_template("report_base.html")
-        html = template.render({"report_name": "Test"})
+        source = template.template.source
 
-        self.assertIn(".report-field pre", html)
-        self.assertIn("overflow-wrap: break-word", html)
+        self.assertIn(".report-field pre", source)
+        self.assertIn("overflow-wrap: break-word", source)
 
     def test_raw_request_pre_tags_preserved(self):
         """

From 6b26aae4de4a39040f130da2c9894776a1fce5c7 Mon Sep 17 00:00:00 2001
From: Paul Osinski 
Date: Fri, 17 Apr 2026 13:59:41 -0400
Subject: [PATCH 16/30] implement versioned_fixtures in test

---
 unittests/test_pdf_report_rendering.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/unittests/test_pdf_report_rendering.py b/unittests/test_pdf_report_rendering.py
index f2d794baa81..1cdfe1e34e1 100644
--- a/unittests/test_pdf_report_rendering.py
+++ b/unittests/test_pdf_report_rendering.py
@@ -10,9 +10,10 @@
     Test_Type,
     User,
 )
-from unittests.dojo_test_case import DojoTestCase
+from unittests.dojo_test_case import DojoTestCase, versioned_fixtures
 
 
+@versioned_fixtures
 class TestPdfReportTextWrapping(DojoTestCase):
 
     """

From 43b2238132d3ec0e6e958dfb3845eaac2be4346b Mon Sep 17 00:00:00 2001
From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>
Date: Mon, 20 Apr 2026 08:35:36 -0600
Subject: [PATCH 17/30] Add centralized banner system with OS messaging support
 (#14708)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Add OSS subscriber for Open Source Messaging banner

Fetches a markdown message from the DaaS-published GCS bucket, renders
the bleached headline and optional expanded section through the existing
additional_banners template loop. Cached for 1h; any fetch/parse failure
silently yields no banner. No Django settings introduced — disabling the
banner requires forking.

Co-Authored-By: Claude Opus 4.7 (1M context) 

* Enable nl2br in expanded markdown and fold module into dojo.announcement

Single newlines in the expanded body now render as 
, so authored markdown lays out multi-line. Module folded into the existing dojo/announcement/ app and test patch paths updated. Co-Authored-By: Claude Opus 4.7 (1M context) * Use

{% endif %} {% for banner in additional_banners %} -