Code cleanup

ghostwriter/api/views.py

@@ -10,7 +10,6 @@
 from base64 import b64encode
 from datetime import date, datetime
 from http import HTTPStatus
-from json import JSONDecodeError
 from socket import gaierror
 
 # Django Imports
@@ -425,7 +424,7 @@ def setup(self, request, *args, **kwargs):
                 self.data = data
                 if "input" in data:
                     self.input = data["input"]
-        except JSONDecodeError:
+        except json.JSONDecodeError:
             logger.exception(
                 "Failed to decode JSON data from supposed Hasura Action request"
             )
@@ -638,7 +637,7 @@ def setup(self, request, *args, **kwargs):
                 self.event = self.data["event"]
                 self.old_data = self.data["event"]["data"]["old"]
                 self.new_data = self.data["event"]["data"]["new"]
-        except JSONDecodeError:
+        except json.JSONDecodeError:
             logger.exception(
                 "Failed to decode JSON data from supposed Hasura Event trigger: %s",
                 request.body,
@@ -1476,7 +1475,7 @@ def post(self, request, *args, **kwargs):
         try:
             entry.recording.delete()
         except OplogEntryRecording.DoesNotExist:
-            pass
+            logger.debug("Oplog entry %s has no existing recording to replace.", entry.id, exc_info=True)
 
         # Extract searchable text from the cast file before saving
         file_bytes = form.cleaned_data["file_base64"]
@@ -2954,7 +2953,7 @@ def _validate_passive_voice_request(request):
 
     try:
         data = json.loads(request.body)
-    except JSONDecodeError:
+    except json.JSONDecodeError:
         return None, JsonResponse(
             {"error": "Invalid JSON in request body"}, status=HTTPStatus.BAD_REQUEST
         )

ghostwriter/home/templatetags/custom_tags.py

@@ -1,6 +1,7 @@
 """This contains the custom template tags used by the Home application."""
 
 # Standard Libraries
+import logging
 from datetime import datetime, timedelta
 
 # Django Imports
@@ -22,6 +23,7 @@
 from ghostwriter.rolodex.models import ProjectAssignment
 
 register = template.Library()
+logger = logging.getLogger(__name__)
 
 
 @register.filter(name="has_group")
@@ -194,7 +196,7 @@ def add_days(date, days):
                 days += 1
         new_date = date_obj
     except ParserError:
-        pass
+        logger.debug("Unable to parse date value for business-day calculation.", exc_info=True)
     return new_date
 
 

ghostwriter/modules/cloud_monitors.py

@@ -313,8 +313,6 @@ def fetch_aws_s3(aws_key, aws_secret, ignore_tags=None):
     """
     message = ""
     buckets = []
-    if ignore_tags is None:
-        ignore_tags = []
     try:
         logger.info("Collecting bucket resources from AWS S3")
         # Create an S3 client

ghostwriter/modules/custom_serializers.py

@@ -100,7 +100,7 @@ def to_representation(self, instance):
                 elif isinstance(value, str) and value.strip() in ("<p></p>", "<p> </p>"):
                     data[key] = ""
             except KeyError:
-                pass
+                logger.debug("Serializer field disappeared while normalizing its value.", exc_info=True)
         return data
 
 
@@ -283,6 +283,9 @@ def get_cvss_score(self, obj):
 class ObservationLinkSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`reporting:ObservationLinkSerializer` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     tags = TagListSerializerField()
 
     extra_fields = ExtraFieldsSerField(Observation._meta.label)
@@ -303,6 +306,9 @@ class Meta:
 class ReportSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`reporting:Report` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     created_by = StringRelatedField()
 
     last_update = SerializerMethodField("get_creation")
@@ -341,6 +347,9 @@ class Meta:
 class ClientSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`rolodex:Client` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     short_name = SerializerMethodField("get_short_name")
     address = SerializerMethodField("get_address")
 
@@ -565,6 +574,9 @@ def get_end_date(self, obj):
 class StaticServerSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`shepherd.StaticServer` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     provider = serializers.CharField(source="server_provider")
     status = serializers.CharField(source="server_status")
     last_used_by = StringRelatedField()
@@ -655,6 +667,9 @@ class Meta:
 class ProjectSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`rolodex:Project` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     name = SerializerMethodField("get_name")
     type = serializers.CharField(source="project_type")
     start_date = SerializerMethodField("get_start_date")
@@ -761,6 +776,9 @@ class Meta:
 class OplogEntrySerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`oplog.OplogEntry` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     tags = TagListSerializerField()
     extra_fields = ExtraFieldsSerField(OplogEntry._meta.label)
     recording_url = serializers.SerializerMethodField()
@@ -772,7 +790,7 @@ def get_recording_url(self, obj):
                 from django.urls import reverse
                 return reverse("oplog:oplog_entry_recording_download", kwargs={"pk": rec.pk})
         except ObjectDoesNotExist:
-            pass
+            logger.debug("Oplog entry %s has no recording to serialize.", obj.pk, exc_info=True)
         return None
 
     class Meta:
@@ -783,6 +801,9 @@ class Meta:
 class OplogSerializer(TaggitSerializer, CustomModelSerializer):
     """Serialize :model:`oplog.Oplog` entries."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
     entries = OplogEntrySerializer(
         many=True,
         exclude=["id", "oplog_id"],

ghostwriter/modules/health_utils.py

@@ -126,7 +126,7 @@ def get_cache_status(self, *args, **kwargs) -> dict:
                 cache = django_caches[alias]
                 cache.set("django_status_test_cache", value)
                 status[alias] = True
-            except:  # pragma: no cover
+            except Exception:  # pragma: no cover
                 status[alias] = False
 
         return status

ghostwriter/modules/reportwriter/base/base.py

@@ -1,7 +1,7 @@
 
 from datetime import datetime
 import io
-from typing import Any, Iterable
+from typing import Any, Callable, Iterable
 import re
 from venv import logger
 
@@ -22,7 +22,8 @@ class ExportBase:
     # Fields
 
     * `input_object`: The object passed into `__init__`, unchanged
-    * `data`: The object passed into `__init__` ran through `serialize_object`, usually a dict, for passing into a Jinja env
+    * `data`: The object passed into `__init__` run through the supplied serializer,
+      usually a dict, for passing into a Jinja environment
     * `jinja_env`: Jinja2 environment for templating
     """
     input_object: Any
@@ -34,7 +35,14 @@ class ExportBase:
     preview_extra_field_model_label: str | None
     preview_extra_field_name: str | None
 
-    def __init__(self, input_object: Any, *, is_raw=False, jinja_debug=False):
+    def __init__(
+        self,
+        input_object: Any,
+        *,
+        is_raw: bool = False,
+        jinja_debug: bool = False,
+        object_serializer: Callable[[Any], Any] | None = None,
+    ):
         self.evidences_by_id = {}
         self.extra_fields_spec_cache = {}
         self.preview_extra_field_model_label = None
@@ -50,15 +58,11 @@ def __init__(self, input_object: Any, *, is_raw=False, jinja_debug=False):
             self.data = input_object
         else:
             self.input_object = input_object
-            self.data = self.serialize_object(input_object)
-
-    def serialize_object(self, object: Any) -> Any:
-        """
-        Called by __init__ to serialize the input object to a format appropriate for use in a jinja environment.
-
-        By default does nothing and returns `object` unchanged.
-        """
-        return object
+            self.data = (
+                object_serializer(input_object)
+                if object_serializer is not None
+                else input_object
+            )
 
     def extra_field_specs_for(self, model: Model) -> Iterable[ExtraFieldSpec]:
         """

ghostwriter/modules/reportwriter/project/base.py

@@ -13,16 +13,22 @@
 from ghostwriter.shepherd.models import Domain, StaticServer
 
 
+def serialize_project(project):
+    """Serialize a project for export without depending on exporter state."""
+    return FullProjectSerializer(project).data
+
+
 class ExportProjectBase(ExportBase):
     """
     Mixin class for exporting projects.
 
-    Provides a `serialize_object` implementation for serializing the `Project` database object,
-    and helper functions for creating Jinja contexts.
+    Configures project serialization and provides helpers for creating Jinja
+    contexts.
     """
 
-    def serialize_object(self, object):
-        return FullProjectSerializer(object).data
+    def __init__(self, *args, **kwargs):
+        kwargs["object_serializer"] = serialize_project
+        super().__init__(*args, **kwargs)
 
     def map_rich_texts(self):
         base_context = copy.deepcopy(self.data)

ghostwriter/modules/reportwriter/report/base.py

@@ -1,5 +1,6 @@
 from collections import ChainMap
 import copy
+from functools import partial
 import html
 from markupsafe import Markup
 from docxtpl import RichText as DocxRichText
@@ -17,28 +18,34 @@
 from ghostwriter.shepherd.models import Domain, StaticServer
 
 
+def serialize_report(report, *, include_bloodhound=True):
+    """Serialize a report for export without depending on exporter state."""
+    excludes = ["id"]
+    if not include_bloodhound:
+        excludes.append("bloodhound")
+    return ReportDataSerializer(
+        report,
+        exclude=excludes,
+    ).data
+
+
 class ExportReportBase(ExportBase):
     """
     Mixin class for exporting reports.
 
-    Provides a `serialize_object` implementation for serializing the `Report` database object,
-    and helper functions for creating Jinja contexts.
+    Configures report serialization and provides helpers for creating Jinja
+    contexts.
     """
     include_bloodhound: bool
 
     def __init__(self, *args, include_bloodhound=True, **kwargs):
         self.include_bloodhound = include_bloodhound
+        kwargs["object_serializer"] = partial(
+            serialize_report,
+            include_bloodhound=include_bloodhound,
+        )
         super().__init__(*args, **kwargs)
 
-    def serialize_object(self, report):
-        excludes = ["id"]
-        if not self.include_bloodhound:
-            excludes.append("bloodhound")
-        return ReportDataSerializer(
-            report,
-            exclude=excludes,
-        ).data
-
     def severity_rich_text(self, text: str, severity_color: str) -> str | DocxRichText:
         """
         Creates an exporter specific rich text object for some text related to finding severity.

ghostwriter/modules/reportwriter/richtext/docx.py

@@ -130,7 +130,7 @@ def style_run(self, run, style):
             try:
                 run.style = "CodeInline"
             except KeyError:
-                pass
+                logger.debug("The DOCX template does not define the CodeInline style.", exc_info=True)
             run.font.no_proof = True
         if style.get("highlight"):
             run.font.highlight_color = WD_COLOR_INDEX.YELLOW
@@ -204,7 +204,7 @@ def tag_p(self, el, *, par=None, **kwargs):
             try:
                 par.style = self.p_style
             except KeyError:
-                pass
+                logger.debug("The DOCX template does not define the requested paragraph style.", exc_info=True)
 
         par_classes = set(el.attrs.get("class", []))
         if "left" in par_classes:
@@ -289,7 +289,7 @@ def tag_blockquote(self, el, par=None, **kwargs):
         try:
             par.style = "Blockquote"
         except KeyError:
-            pass
+            logger.debug("The DOCX template does not define the Blockquote style.", exc_info=True)
         self.process_children(el.children, par=par, **kwargs)
 
     def tag_div(self, el, **kwargs):
@@ -740,9 +740,9 @@ def make_cross_ref(self, par, ref: str):
         r.append(fldChar)
 
         # Add runs for the figure label and number
-        run = par.add_run(self.global_report_config.label_figure)
+        par.add_run(self.global_report_config.label_figure)
         # This ``#`` is a placeholder Word will replace with the figure's number
-        run = par.add_run("#")
+        par.add_run("#")
 
         # Close the  field character run
         run = par.add_run()
@@ -852,6 +852,6 @@ def create(self, doc):
                     # python-docx's deprecated style_id lookup path.
                     par.style = "List Paragraph"
                 except KeyError:
-                    pass
+                    logger.debug("The DOCX template does not define a list paragraph style.", exc_info=True)
             par._p.get_or_add_pPr().get_or_add_numPr().get_or_add_numId().val = numbering_id
             par._p.get_or_add_pPr().get_or_add_numPr().get_or_add_ilvl().val = level

ghostwriter/modules/reportwriter/richtext/ooxml.py

@@ -163,7 +163,7 @@ def style_run(self, run, style):
             try:
                 run.font.size = int(style["font_size"])
             except ValueError:
-                pass
+                logger.debug("Unable to apply rich-text font size.", exc_info=True)
 
     tag_code = set_style_method("code", "inline_code")
     tag_b = set_style_method("b", "bold")

ghostwriter/oplog/tests/test_forms.py

@@ -191,7 +191,7 @@ def test_report_auto_selected_first_when_no_active(self):
         self.assertEqual(form.fields["report"].initial, self.report)
 
     def test_report_auto_selected_first_when_multiple_no_active(self):
-        second_report = ReportFactory(project=self.project)
+        _ = ReportFactory(project=self.project)
         form = OplogEvidenceForm(project=self.project)
         first_in_list = form.fields["report"].queryset.first()
         self.assertEqual(form.fields["report"].initial, first_in_list)

ghostwriter/oplog/views.py

@@ -797,7 +797,7 @@ def _write_file_to_zip(self, zf, field_file, arcname):
                 zf.write(path, arcname)
                 return True
         except self.path_lookup_errors:
-            pass
+            logger.debug("Unable to access attachment path; falling back to streamed ZIP output.", exc_info=True)
 
         try:
             field_file.open("rb")
@@ -920,7 +920,7 @@ def get(self, *args, **kwargs):
                                     arcname
                                 )
                         except OplogEntryRecording.DoesNotExist:
-                            pass
+                            logger.debug("Oplog entry %s has no recording to export.", entry.id, exc_info=True)
 
                     # Evidence files
                     if "evidence" in include_set or "all" in include_set: