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 14 commits into
base: bugfix
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.

@github-actions github-actions bot added the docs label May 20, 2025
@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 risks, including a confirmed mass assignment vulnerability in a test file, potential information disclosure through error logs and test data, and various configuration and categorization risks that could expose internal system details or compromise security thresholds.

⚠️ 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 System_Settings.objects.update(**{field: value}) which allows dynamic attribute setting without explicit validation

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):

💭 Unconfirmed Findings (6)
Vulnerability Potential Information Disclosure
Description Risks include detailed error logging in Jira helper that could expose internal system details, and test VCR YAML files revealing network configurations, IP addresses, and account information.
Vulnerability Potential Configuration and Authentication Risks
Description Involves hardcoded authentication tokens in test files and conditional status enforcement in Jira templates that might introduce configuration-dependent security behaviors.
Vulnerability Severity Misclassification Risks
Description Includes downgrading vulnerability severities in npm audit test fixtures and changing severity mapping from 'Trivial' to 'Lowest' in test data, potentially obscuring actual risk levels.
Vulnerability Potential Threshold and Categorization Vulnerabilities
Description Concerns include potential severity threshold bypass in finding group processing and detailed findings categorization that might expose internal classification criteria.
Vulnerability Potential Test Data Exposure
Description Test sample filenames and VCR recordings might inadvertently contain sensitive scan or network information, risking unintended data leakage.
Vulnerability Potential Typo-Related Risks
Description A typo in the method name get_jira_priortiy() could potentially lead to unexpected system behavior or method invocation errors.

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.

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.

1 participant