Skip to content

refactor: decouple Jira integration into dojo/jira package#14743

Open
Maffooch wants to merge 2 commits intoDefectDojo:devfrom
Maffooch:jira-cleanup
Open

refactor: decouple Jira integration into dojo/jira package#14743
Maffooch wants to merge 2 commits intoDefectDojo:devfrom
Maffooch:jira-cleanup

Conversation

@Maffooch
Copy link
Copy Markdown
Contributor

Description

Extracts all Jira integration code into a dedicated dojo/jira package and introduces dojo/jira/services.py as the single entry point for the rest of the application. The goal is to remove direct coupling between core models/forms/views/importers and Jira internals so the Jira module can evolve without touching consumers across the codebase.

What changed:

  • Renamed dojo/jira_link/dojo/jira/ and added models.py, forms.py, and api/{serializers,views}.py to hold the Jira-specific pieces previously embedded in dojo/models.py, dojo/forms.py, and dojo/api_v2/.
  • New dojo/jira/services.py (service-layer module matching the services.py convention used elsewhere, e.g. dojo/engagement/services.py) exposing the public Jira surface consumed by non-Jira modules. Each wrapper is a one-line delegation to dojo.jira.helper, lazy-imported inside the module to break the import cycle between dojo.forms / dojo.models / dojo.utils and dojo.jira.helper. Real Jira errors propagate to callers instead of being swallowed.
  • Direct Jira imports removed from: dojo/models.py, dojo/forms.py, dojo/api_v2/{serializers,views}.py, dojo/finding/{helper,views}.py, dojo/finding_group/views.py, dojo/engagement/{services,views}.py, dojo/product/views.py, dojo/test/views.py, dojo/risk_acceptance/helper.py, dojo/templatetags/display_tags.py, dojo/importers/*, dojo/tasks.py, and dojo/utils.py (SLA notifications). Management commands (jira_async_updates, jira_refactor_data_migration, jira_status_reconciliation, push_to_jira_update) updated to import from the new package.
  • .dryrunsecurity.yaml sensitive-codepaths glob updated to track dojo/jira/**.

No behavior change is intended — this is a code-move / import-boundary cleanup. No DB schema changes and no new settings.

Introduce a new dojo/jira package (models, forms, api, helper, queries,
urls, views) and a top-level dojo/jira_facade.py. Route all Jira
references from the rest of the codebase (models.py properties, forms,
api_v2 serializers and views, view files, tasks, display_tags,
risk_acceptance/helper, finding/helper, importers, utils SLA
notifications, engagement/services, management commands) through the
facade so no module outside the Jira package imports Jira internals
directly.

Also resolves a circular import in dojo/jira/models.py, updates test
mock paths for the new module layout, and fixes ruff lint across the
moved files.
@Maffooch Maffooch requested a review from mtesauro as a code owner April 22, 2026 22:45
@dryrunsecurity
Copy link
Copy Markdown

DryRun Security

This pull request may introduce mass-assignment risks: it unpacks user-controlled request.data into dojo_dispatch_task(**request.data) without shown sanitization (potentially allowing arbitrary kwargs to affect model updates), and it adds JIRA serializers using ModelSerializer with fields='all' (which can let clients set any writable model fields unless callers or models explicitly restrict writes).

🔴 Potential Mass Assignment Vulnerability in dojo/api_v2/views.py (drs_841db40f)
Vulnerability Potential Mass Assignment Vulnerability
Description The code unpacks request.data (user-controlled) into keyword arguments when calling dojo_dispatch_task (dojo_dispatch_task(task, engagement.id, **request.data)). If the dispatched Celery task (retrieved from jira_services.get_epic_task) or the downstream helper accepts arbitrary kwargs and applies them to model updates without explicit allowlisting, this could allow mass-assignment of model fields. The patch shows no explicit filtering of request.data before unpacking here; the new jira services wrapper only returns helper task functions and does not sanitize request data. I could not find the implementation of dojo_dispatch_task or the actual epic task in the provided patch to prove they sanitize or validate kwargs, so the use of **request.data is potentially unsafe.

dojo_dispatch_task(task, engagement.id, **request.data)
response = Response(
{"info": "Jira Epic update query sent"},
status=status.HTTP_200_OK,

🟠 Potential Mass Assignment Vulnerability in dojo/jira/api/serializers.py (drs_84bb6ed7)
Vulnerability Potential Mass Assignment Vulnerability
Description The JIRA API serializers use ModelSerializer with fields='all', which would normally allow clients to set any writable model fields. The patch introduces JIRAIssueSerializer, JIRAInstanceSerializer, and JIRAProjectSerializer with fields='all' and only limited custom validation for a few fields. There is no explicit allowlist (e.g. .validated() or .only()) in the serializers themselves and many callers in the PR pass serializer data through create/update flows that may persist model fields. However, model-level protections (like Django model field write protections) and view-level usage determine actual exposure.

class Meta:
model = JIRA_Issue
fields = "__all__"

We've notified @mtesauro.


Comment to provide feedback on these findings.

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

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

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

@Maffooch Maffooch added this to the 2.58.0 milestone Apr 23, 2026
Copy link
Copy Markdown
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants