Adds ruff linter, fixes unused variables errors (#9123)

* Adds ruff linter, fixes unused variables errors

* Correct Flake8

* Correct some unit tests

* Attempt to correct unit tests

* Pull out dependencies

.github/workflows/ruff.yml

@@ -0,0 +1,36 @@
+name: Ruff Linter
+
+on:
+  workflow_dispatch:
+  pull_request_target:
+  push:
+
+jobs:
+  ruff-linting:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
+        uses: actions/checkout@v4
+        # by default the pull_requst_target event checks out the base branch, i.e. dev
+        # so we need to explicitly checkout the head of the PR
+        # we use fetch-depth 0 to make sure the full history is checked out and we can compare against
+        # the base commit (branch) of the PR
+        # more info https://github.community/t/github-actions-are-severely-limited-on-prs/18179/16
+        # we checkout merge_commit here as this contains all new code from dev also. we don't need to compare against base_commit
+        with:
+          persist-credentials: false
+          fetch-depth: 0
+          ref: refs/pull/${{ github.event.pull_request.number }}/merge
+          # repository: ${{github.event.pull_request.head.repo.full_name}}
+
+      - name: Checkout
+        # for non PR runs we just checkout the default, which is a sha on a branch probably
+        if: github.event_name != 'pull_request' && github.event_name != 'pull_request_target'
+        uses: actions/checkout@v4
+
+      - name: Install Ruff Linter
+        run: pip install -r requirements-lint.txt
+
+      - name: Run Ruff Linter
+        run: ruff dojo

.gitignore

@@ -38,6 +38,7 @@ pip-delete-this-directory.txt
 .tox/
 .coverage
 .cache
+.ruff_cache
 nosetests.xml
 coverage.xml
 

dojo/api_v2/views.py

@@ -111,7 +111,6 @@
 from django.conf import settings
 from datetime import datetime
 from dojo.utils import (
-    get_period_counts_legacy,
     get_system_setting,
     get_setting,
     async_delete,
@@ -582,9 +581,6 @@ def notes(self, request, pk=None):
             serialized_note = serializers.NoteSerializer(
                 {"author": author, "entry": entry, "private": private}
             )
-            result = serializers.EngagementToNotesSerializer(
-                {"engagement_id": engagement, "notes": [serialized_note.data]}
-            )
             return Response(
                 serialized_note.data, status=status.HTTP_201_CREATED
             )
@@ -1229,9 +1225,6 @@ def notes(self, request, pk=None):
             serialized_note = serializers.NoteSerializer(
                 {"author": author, "entry": entry, "private": private}
             )
-            result = serializers.FindingToNotesSerializer(
-                {"finding_id": finding, "notes": [serialized_note.data]}
-            )
             return Response(
                 serialized_note.data, status=status.HTTP_201_CREATED
             )
@@ -2592,9 +2585,6 @@ def notes(self, request, pk=None):
             serialized_note = serializers.NoteSerializer(
                 {"author": author, "entry": entry, "private": private}
             )
-            result = serializers.TestToNotesSerializer(
-                {"test_id": test, "notes": [serialized_note.data]}
-            )
             return Response(
                 serialized_note.data, status=status.HTTP_201_CREATED
             )
@@ -3286,7 +3276,6 @@ def report_generate(request, obj, options):
     test = None
     endpoint = None
     endpoints = None
-    endpoint_monthly_counts = None
 
     include_finding_notes = False
     include_finding_images = False
@@ -3320,16 +3309,7 @@ def report_generate(request, obj, options):
                 )
             ),
         )
-        products = Product.objects.filter(
-            prod_type=product_type, engagement__test__finding__in=findings.qs
-        ).distinct()
-        engagements = Engagement.objects.filter(
-            product__prod_type=product_type, test__finding__in=findings.qs
-        ).distinct()
-        tests = Test.objects.filter(
-            engagement__product__prod_type=product_type,
-            finding__in=findings.qs,
-        ).distinct()
+
         if len(findings.qs) > 0:
             start_date = timezone.make_aware(
                 datetime.combine(findings.qs.last().date, datetime.min.time())
@@ -3344,15 +3324,6 @@ def report_generate(request, obj, options):
         # include current month
         months_between += 1
 
-        endpoint_monthly_counts = get_period_counts_legacy(
-            findings.qs.order_by("numerical_severity"),
-            findings.qs.order_by("numerical_severity"),
-            None,
-            months_between,
-            start_date,
-            relative_delta="months",
-        )
-
     elif type(obj).__name__ == "Product":
         product = obj
 
@@ -3365,11 +3336,6 @@ def report_generate(request, obj, options):
                 Finding.objects.filter(test__engagement__product=product)
             ),
         )
-        ids = set(finding.id for finding in findings.qs)
-        engagements = Engagement.objects.filter(
-            test__finding__id__in=ids
-        ).distinct()
-        tests = Test.objects.filter(finding__id__in=ids).distinct()
         ids = get_endpoint_ids(
             Endpoint.objects.filter(product=product).distinct()
         )
@@ -3387,7 +3353,6 @@ def report_generate(request, obj, options):
         report_name = "Engagement Report: " + str(engagement)
 
         ids = set(finding.id for finding in findings.qs)
-        tests = Test.objects.filter(finding__id__in=ids).distinct()
         ids = get_endpoint_ids(
             Endpoint.objects.filter(product=engagement.product).distinct()
         )

dojo/cred/views.py

@@ -214,11 +214,6 @@ def view_cred_product_engagement(request, eid, ttid):
         title="Credential Manager", top_level=False, request=request)
     cred_type = "Engagement"
     edit_link = ""
-    view_link = reverse(
-        'view_cred_product_engagement', args=(
-            eid,
-            cred.id,
-        ))
     delete_link = reverse(
         'delete_cred_engagement', args=(
             eid,
@@ -270,11 +265,6 @@ def view_cred_engagement_test(request, tid, ttid):
         title="Credential Manager", top_level=False, request=request)
     cred_type = "Test"
     edit_link = None
-    view_link = reverse(
-        'view_cred_engagement_test', args=(
-            tid,
-            cred.id,
-        ))
     delete_link = reverse(
         'delete_cred_test', args=(
             tid,
@@ -326,11 +316,6 @@ def view_cred_finding(request, fid, ttid):
         title="Credential Manager", top_level=False, request=request)
     cred_type = "Finding"
     edit_link = None
-    view_link = reverse(
-        'view_cred_finding', args=(
-            fid,
-            cred.id,
-        ))
     delete_link = reverse(
         'delete_cred_finding', args=(
             fid,

dojo/endpoint/utils.py

@@ -11,6 +11,7 @@
 from django.core.validators import validate_ipv46_address
 from django.core.exceptions import ValidationError
 from django.db.models import Q, Count
+from django.http import HttpResponseRedirect
 
 from dojo.models import Endpoint, DojoMeta
 
@@ -308,21 +309,19 @@ def endpoint_meta_import(file, product, create_endpoints, create_tags, create_me
     content = file.read()
     sig = content.decode('utf-8-sig')
     content = sig.encode("utf-8")
-    if type(content) is bytes:
+    if isinstance(content, bytes):
         content = content.decode('utf-8')
     reader = csv.DictReader(io.StringIO(content))
 
     if 'hostname' not in reader.fieldnames:
         if origin == 'UI':
-            from django.http import HttpResponseRedirect
             messages.add_message(
                 request,
                 messages.ERROR,
                 'The column "hostname" must be present to map host to Endpoint.',
                 extra_tags='alert-danger')
             return HttpResponseRedirect(reverse('import_endpoint_meta', args=(product.id, )))
         elif origin == 'API':
-            from rest_framework.serializers import ValidationError
             raise ValidationError('The column "hostname" must be present to map host to Endpoint.')
 
     keys = [key for key in reader.fieldnames if key != 'hostname']
@@ -368,8 +367,6 @@ def endpoint_meta_import(file, product, create_endpoints, create_tags, create_me
 
 
 def remove_broken_endpoint_statuses(apps):
-    Finding = apps.get_model('dojo', 'Finding')
-    Endpoint = apps.get_model('dojo', 'Endpoint')
     Endpoint_Status = apps.get_model('dojo', 'endpoint_status')
     broken_eps = Endpoint_Status.objects.filter(Q(endpoint=None) | Q(finding=None))
     if broken_eps.count() == 0:

dojo/engagement/views.py

@@ -213,7 +213,6 @@ def edit_engagement(request, eid):
     jira_project_form = None
     jira_epic_form = None
     jira_project = None
-    jira_error = False
 
     if request.method == 'POST':
         form = EngForm(request.POST, instance=engagement, cicd=is_ci_cd, product=engagement.product, user=request.user)
@@ -430,7 +429,6 @@ def view_engagement(request, eid):
                 form = TypedNoteForm(available_note_types=available_note_types)
             else:
                 form = NoteForm()
-            url = request.build_absolute_uri(reverse("view_engagement", args=(eng.id,)))
             title = "Engagement: %s on %s" % (eng.name, eng.product.name)
             messages.add_message(request,
                                  messages.SUCCESS,
@@ -1107,7 +1105,8 @@ def view_edit_risk_acceptance(request, eid, raid, edit_mode=False):
 @user_is_authorized(Engagement, Permissions.Risk_Acceptance, 'eid')
 def expire_risk_acceptance(request, eid, raid):
     risk_acceptance = get_object_or_404(prefetch_for_expiration(Risk_Acceptance.objects.all()), pk=raid)
-    eng = get_object_or_404(Engagement, pk=eid)
+    # Validate the engagement ID exists before moving forward
+    get_object_or_404(Engagement, pk=eid)
 
     ra_helper.expire_now(risk_acceptance)
 
@@ -1231,7 +1230,7 @@ def engagement_ics(request, eid):
 def get_list_index(list, index):
     try:
         element = list[index]
-    except Exception as e:
+    except Exception:
         element = None
     return element
 

dojo/filters.py

@@ -228,7 +228,7 @@ def cwe_options(queryset):
     cwe = dict()
     cwe = dict([cwe, cwe]
                 for cwe in queryset.order_by().values_list('cwe', flat=True).distinct()
-                if type(cwe) is int and cwe is not None and cwe > 0)
+                if isinstance(cwe, int) and cwe is not None and cwe > 0)
     cwe = collections.OrderedDict(sorted(cwe.items()))
     return list(cwe.items())
 
@@ -267,7 +267,7 @@ def get_tags_model_from_field_name(field):
         parts = field.split('__')
         model_name = parts[-2]
         return apps.get_model('dojo.%s' % model_name, require_ready=True), exclude
-    except Exception as e:
+    except Exception:
         return None, exclude
 
 

dojo/forms.py

@@ -49,6 +49,7 @@
 from dojo.user.queries import get_authorized_users_for_product_and_product_type, get_authorized_users
 from dojo.user.utils import get_configuration_permissions_fields
 from dojo.group.queries import get_authorized_groups, get_group_member_roles
+import dojo.jira_link.helper as jira_helper
 
 logger = logging.getLogger(__name__)
 
@@ -111,7 +112,6 @@ def render(self, name, value, attrs=None, renderer=None):
                 if match:
                     year_val,
                     month_val,
-                    day_val = [int(v) for v in match.groups()]
 
         output = []
 
@@ -673,7 +673,7 @@ class MergeFindings(forms.ModelForm):
         help_text="The action to take on the merged finding. Set the findings to inactive or delete the findings.")
 
     def __init__(self, *args, **kwargs):
-        finding = kwargs.pop('finding')
+        _ = kwargs.pop('finding')
         findings = kwargs.pop('findings')
         super(MergeFindings, self).__init__(*args, **kwargs)
 
@@ -2279,7 +2279,8 @@ def clean(self):
         form_data = self.cleaned_data
 
         try:
-            jira = jira_helper.get_jira_connection_raw(form_data['url'], form_data['username'], form_data['password'])
+            # Attempt to validate the credentials before moving forward
+            _ = jira_helper.get_jira_connection_raw(form_data['url'], form_data['username'], form_data['password'])
             logger.debug('valid JIRA config!')
         except Exception as e:
             # form only used by admins, so we can show full error message using str(e) which can help debug any problems
@@ -2306,7 +2307,8 @@ def clean(self):
         form_data = self.cleaned_data
 
         try:
-            jira = jira_helper.get_jira_connection_raw(form_data['url'], form_data['username'], form_data['password'],)
+            # Attempt to validate the credentials before moving forward
+            _ = jira_helper.get_jira_connection_raw(form_data['url'], form_data['username'], form_data['password'],)
             logger.debug('valid JIRA config!')
         except Exception as e:
             # form only used by admins, so we can show full error message using str(e) which can help debug any problems
@@ -2817,8 +2819,7 @@ def __init__(self, *args, **kwargs):
 
     def clean(self):
         logger.debug('jform clean')
-        import dojo.jira_link.helper as jira_helper
-        cleaned_data = super(JIRAFindingForm, self).clean()
+        super(JIRAFindingForm, self).clean()
         jira_issue_key_new = self.cleaned_data.get('jira_issue')
         finding = self.instance
         jira_project = self.jira_project
@@ -3021,8 +3022,6 @@ def __init__(self, *args, **kwargs):
             initial=initial_answer,
         )
 
-        answer = self.fields['answer']
-
     def save(self):
         if not self.is_valid():
             raise forms.ValidationError('form is not valid')
@@ -3095,7 +3094,7 @@ def clean_answer(self):
         real_answer = self.cleaned_data.get('answer')
 
         # for single choice questions, the selected answer is a single string
-        if type(real_answer) is not list:
+        if not isinstance(real_answer, list):
             real_answer = [real_answer]
         return real_answer
 

dojo/group/utils.py

@@ -15,7 +15,8 @@ def get_auth_group_name(group, attempt=0):
         auth_group_name = group.name + '_' + str(attempt)
 
     try:
-        auth_group = Group.objects.get(name=auth_group_name)
+        # Attempt to fetch an existing group before moving forward with the real operation
+        _ = Group.objects.get(name=auth_group_name)
         return get_auth_group_name(group, attempt + 1)
     except Group.DoesNotExist:
         return auth_group_name

dojo/importers/importer/importer.py

@@ -72,7 +72,6 @@ def process_parsed_findings(self, test, parsed_findings, scan_type, user, active
         new_findings = []
         items = parsed_findings
         logger.debug('starting import of %i items.', len(items) if items else 0)
-        i = 0
         group_names_to_findings_dict = {}
 
         for item in items:

dojo/importers/reimporter/reimporter.py

@@ -49,7 +49,6 @@ def process_parsed_findings(
         items = parsed_findings
         original_items = list(test.finding_set.all())
         new_items = []
-        mitigated_count = 0
         finding_count = 0
         finding_added_count = 0
         reactivated_count = 0