Skip to content

Finding Groups: Respect minimum severity and active/verified rules when pushing to JIRA #12475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented May 18, 2025

Fixes #12454 and [sc-11185]

In Defect Dojo single JIRA issues are only pushed of they are "qualified" to be pushed to JIRA:

  • They must be Active
  • They must be Verified (if Verified status is enforced globally or for the JIRA integration)
  • They must be above the jira_minimum threshold

This PR now enforces these same rules for Finding Groups.

At first I tried to model this the same as for findings where the status() method inside the model calculates the status. But I didn't want to clutter the models.py with JIRA related logic, so I put everything in dojo/jira_link/helper.py as much as possible

Changes to the Finding Group JIRA issue:

  • Description is updated. At first I filtered out any findings not matching the active/verified/severity criteria. But this may get confusing if findings get resolved in Defect Dojo they just disappear from JIRA until the JIRA ticket is empty. Same for changes in Severity or Verified fields in findings. The JIRA Group Issue now shows two tables. The first contains findings matching the criteria ("qualified findings"), the second findings that do not match the criteria ("non-qualified findings"). This way JIRA users will always have an overview of the full group.

  • Status is based on qualified findings

  • Priority is based on qualified findings

  • SLA Deadline and due date is based on qualified findings.

Screenshot 2025-05-20 213332

Unit tests have been updated, but I can imagine this PR would be a good one to test manually in the UI.

@valentijnscholten valentijnscholten marked this pull request as ready for review May 20, 2025 20:31
Copy link

dryrunsecurity bot commented May 20, 2025

DryRun Security

🟡 Please give this pull request extra attention during review.

This pull request contains multiple security vulnerabilities, including potential mass assignment risks, system setting modification dangers, resource exhaustion concerns in Jira-related functions, debug logging information disclosure, and an environment variable injection risk in the unittest script.

⚠️ Potential Mass Assignment Vulnerability in unittests/dojo_test_case.py
Vulnerability Potential Mass Assignment Vulnerability
Description The code is potentially vulnerable to Mass Assignment because it uses the objects.update method with a dynamic field and value, which could allow unintended attributes to be set if not properly controlled

return decorator
def with_system_setting(field, value):
"""Decorator to temporarily set a value in System Settings."""
def decorator(test_func):
@wraps(test_func)
def wrapper(*args, **kwargs):
old_value = getattr(System_Settings.objects.get(), field)
# Set the flag to the specified value
System_Settings.objects.update(**{field: value})
try:
return test_func(*args, **kwargs)
finally:
# Reset the flag to its original state after the test
System_Settings.objects.update(**{field: old_value})
return wrapper
return decorator
class DojoTestUtilsMixin:
def get_test_admin(self, *args, **kwargs):

⚠️ Arbitrary System Setting Modification in unittests/dojo_test_case.py
Vulnerability Arbitrary System Setting Modification
Description The with_system_setting decorator in dojo_test_case.py allows dynamic modification of system settings using an arbitrary field name. While intended for testing, this pattern could be dangerous if similar code were used in a production context with untrusted input.

return decorator
def with_system_setting(field, value):
"""Decorator to temporarily set a value in System Settings."""
def decorator(test_func):
@wraps(test_func)
def wrapper(*args, **kwargs):
old_value = getattr(System_Settings.objects.get(), field)
# Set the flag to the specified value
System_Settings.objects.update(**{field: value})
try:
return test_func(*args, **kwargs)
finally:
# Reset the flag to its original state after the test
System_Settings.objects.update(**{field: old_value})
return wrapper
return decorator
class DojoTestUtilsMixin:
def get_test_admin(self, *args, **kwargs):

⚠️ Potential Denial of Service via Resource Exhaustion in dojo/jira_link/helper.py
Vulnerability Potential Denial of Service via Resource Exhaustion
Description Functions like jira_priority() and get_sla_deadline() for Finding_Group iterate over potentially large collections of findings without efficient processing limits. If an attacker can manipulate the number or complexity of findings, this could lead to performance degradation or resource consumption.

def jira_priority(obj):
if isinstance(obj, Finding):
return get_jira_instance(obj).get_priority(obj.severity)
if isinstance(obj, Finding_Group):
# priority based on qualified findings, so if alls criticals get closed, the priority will gets lowered etc
active_findings = get_qualified_findings(obj)
if not active_findings:
# using a string literal "Info" as we don't really have a "enum" for this anywhere
max_number_severity = Finding.get_number_severity("Info")
else:
max_number_severity = max(Finding.get_number_severity(find.severity) for find in active_findings)
return get_jira_instance(obj).get_priority(Finding.get_severity(max_number_severity))
msg = f"Unsupported object type for jira_priority: {obj.__class__.__name__}"
raise ValueError(msg)
def jira_environment(obj):

⚠️ Information Disclosure via Debug Logging in dojo/jira_link/helper.py
Vulnerability Information Disclosure via Debug Logging
Description Multiple debug logging statements added in jira_link/helper.py that could expose sensitive internal application metadata if debug logging is enabled in production. Specifically, logging entire Jira metadata objects via json.dumps(meta, indent=4) could reveal configuration details, project structures, or system information.

try:
project = meta["projects"][0]
except Exception:
logger.debug("JIRA meta: %s", json.dumps(meta, indent=4)) # this is None safe
msg = "Project misconfigured or no permissions in Jira ?"
raise JIRAError(msg)
try:
issuetype_fields = project["issuetypes"][0]["fields"].keys()
except Exception:
logger.debug("JIRA meta: %s", json.dumps(meta, indent=4)) # this is None safe
msg = "Misconfigured default issue type ?"
raise JIRAError(msg)

⚠️ Environment Variable Injection Risk in run-unittest.sh
Vulnerability Environment Variable Injection Risk
Description The run-unittest.sh script removes the unset TEST_CASE line, which could allow an attacker to pre-set the TEST_CASE environment variable and potentially inject arbitrary arguments into the test command execution within the Docker container.

#!/usr/bin/env bash
unset TEST_CASE
bash ./docker/docker-compose-check.sh


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

@valentijnscholten valentijnscholten added this to the 2.47.0 milestone May 21, 2025
@valentijnscholten valentijnscholten marked this pull request as draft May 24, 2025 08:35
@valentijnscholten valentijnscholten marked this pull request as ready for review May 24, 2025 11:29
@valentijnscholten
Copy link
Member Author

Per request I've also added syncing of the JIRA Priority field for finding groups based on the current list of qualified findings in the group.

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

For folks that do not update their jira templates, will anything break here?

@valentijnscholten
Copy link
Member Author

For folks that do not update their jira templates, will anything break here?

Depends on what they've put in their templates :-)

But in general this PR doesn't remove any methods on the finding_group model or jira helper, so I don't expect (m)any things breaking. There is a note in the upgrade notes to make people aware.

Copy link
Contributor

github-actions bot commented Jun 2, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Maffooch Maffooch modified the milestones: 2.47.0, 2.48.0 Jun 3, 2025
@valentijnscholten valentijnscholten force-pushed the finding-group-respect-jira-requirements branch from 43f6e62 to 9b2020e Compare June 8, 2025 09:37
Copy link
Contributor

github-actions bot commented Jun 8, 2025

Conflicts have been resolved. A maintainer will review the pull request shortly.

@valentijnscholten valentijnscholten force-pushed the finding-group-respect-jira-requirements branch from 983fc9a to 2aa4f91 Compare June 8, 2025 09:49
@valentijnscholten valentijnscholten changed the base branch from bugfix to dev June 8, 2025 09:49
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.

2 participants