Enable more ruff rules

Signed-off-by: Stephen Finucane <stephen@that.guru>

Author: stephenfin

patchwork/api/project.py

@@ -70,7 +70,8 @@ class ProjectMixin(object):
     def get_object(self):
         queryset = self.filter_queryset(self.get_queryset())
 
-        assert 'pk' in self.kwargs
+        # Enforces a routing invariant, not guarding against user input.
+        assert 'pk' in self.kwargs  # noqa: S101
 
         try:
             obj = queryset.get(id=int(self.kwargs['pk']))

patchwork/fields.py

@@ -11,7 +11,7 @@
 
 class HashField(models.CharField):
     def __init__(self, *args, **kwargs):
-        self.n_bytes = len(hashlib.sha1().hexdigest())
+        self.n_bytes = len(hashlib.sha1(usedforsecurity=False).hexdigest())
         kwargs['max_length'] = self.n_bytes
 
         super(HashField, self).__init__(*args, **kwargs)
@@ -20,7 +20,7 @@ def construct(self, value):
         # TODO: should this be unconditional?
         if isinstance(value, str):
             value = value.encode('utf-8')
-        return hashlib.sha1(value)
+        return hashlib.sha1(value, usedforsecurity=False)
 
     def from_db_value(self, value, *args, **kwargs):
         return self.to_python(value)

patchwork/filters.py

@@ -278,7 +278,8 @@ def form(self):
                 escape(state.name),
             )
         out += '</select>'
-        return mark_safe(out)
+        # All user-supplied values are run through escape() above.
+        return mark_safe(out)  # noqa: S308
 
 
 class SearchFilter(Filter):
@@ -315,7 +316,8 @@ def form(self):
         value = ''
         if self.search:
             value = escape(self.search)
-        return mark_safe(
+        # self.param is a hardcoded class attribute; value is escaped above.
+        return mark_safe(  # noqa: S308
             '<input name="%s" class="form-control" value="%s">'
             % (self.param, value)
         )
@@ -389,7 +391,9 @@ def form(self):
                 'selected': selected,
                 'value': self.param_map[b],
             }
-        return mark_safe(s)
+        # all values come from hardcoded class attributes (description_map,
+        # param_map)
+        return mark_safe(s)  # noqa: S308
 
 
 class DelegateFilter(Filter):
@@ -463,7 +467,9 @@ def kwargs(self):
     @property
     def form(self):
         if self.forced:
-            return mark_safe(
+            # when forced, condition is a delegate username set
+            # programmatically, not from user input
+            return mark_safe(  # noqa: S308
                 '<input type="hidden" value="%s">%s'
                 % (self.param, self.condition)
             )
@@ -499,7 +505,9 @@ def form(self):
                 delegate.username,
             )
         out += '</select>'
-        return mark_safe(out)
+        # delegate.id is an integer; delegate.username is restricted to safe
+        # characters by Django
+        return mark_safe(out)  # noqa: S308
 
     def set_status(self, *args, **kwargs):
         if 'delegate' in kwargs:

patchwork/hasher.py

@@ -23,7 +23,7 @@ def hash_diff(diff):
     diff = diff.strip() + '\n'
 
     prefixes = ['-', '+', ' ']
-    hashed = hashlib.sha1()
+    hashed = hashlib.sha1(usedforsecurity=False)
 
     for line in diff.split('\n'):
         if len(line) <= 0:

patchwork/models.py

@@ -6,7 +6,7 @@
 
 from collections import Counter
 import datetime
-import random
+import secrets
 import re
 
 from django.conf import settings
@@ -381,7 +381,8 @@ def patch_responses(self):
     def url_msgid(self):
         """A trimmed Message ID, suitable for inclusion in URLs"""
         if settings.DEBUG:
-            assert self.msgid[0] == '<' and self.msgid[-1] == '>'
+            # Only reached in development; never runs in production.
+            assert self.msgid[0] == '<' and self.msgid[-1] == '>'  # noqa: S101
 
         return self.msgid.strip('<>')
 
@@ -1361,7 +1362,7 @@ def is_valid(self):
     def save(self, *args, **kwargs):
         limit = 1 << 32
         if not self.key:
-            key = '%s%s%d' % (self.user, self.email, random.randint(0, limit))
+            key = '%s%s%d' % (self.user, self.email, secrets.randbelow(limit))
             self.key = self._meta.get_field('key').construct(key).hexdigest()
         super(EmailConfirmation, self).save()
 

patchwork/notifications.py

@@ -73,11 +73,7 @@ def delete_notifications():
             'patchwork/mails/patch-change-notification.txt', context
         )
 
-        # Got one case where recipient.email == "<>". That causes Django to fail with:
-        #   File "/usr/lib/python3/dist-packages/django/core/mail/message.py", line 99, in sanitize_address
-        #    address_parts = nm + localpart + domain
-        #                    ~~~^~~~~~~~~~~
-        # TypeError: can only concatenate str (not "NoneType") to str
+        # we can't concatenate empty emails
         if (
             not recipient.email
             or recipient.email == '<>'

patchwork/parser.py

@@ -1069,7 +1069,7 @@ def find_series_from_url(url):
     try:
         result = resolve(parse_result.path)
     except Resolver404:
-        logging.warning('Failed to resolve series or patch URL: %s', url)
+        logger.warning('Failed to resolve series or patch URL: %s', url)
         return None
 
     # TODO: Use the series detail view here.
@@ -1083,15 +1083,16 @@ def find_series_from_url(url):
             try:
                 series_id_num = int(series_id[0])
             except ValueError:
-                logging.warning(
-                    'Series URL with an invalid series query parameter was given: %s',
+                logger.warning(
+                    'Series URL with an invalid series query parameter was '
+                    'given: %s',
                     url,
                 )
                 return None
             # This will return None if there are no matches.
             return Series.objects.filter(id=series_id_num).first()
 
-        logging.warning(
+        logger.warning(
             'Series URL did not have a series query parameter: %s', url
         )
         return None

patchwork/settings/production.example.py

@@ -61,6 +61,6 @@
         'BACKEND': 'django.core.files.storage.FileSystemStorage',
     },
     'staticfiles': {
-        'BACKEND': 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage',
+        'BACKEND': 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage',  # noqa: E501
     },
 }

patchwork/templatetags/admins.py

@@ -22,6 +22,8 @@ def site_admins():
         return ''
 
     if len(admins) == 1:
-        return mark_safe(admins[0])
+        # admins come from settings.ADMINS, a developer-controlled config value
+        return mark_safe(admins[0])  # noqa: S308
 
-    return mark_safe(', '.join(admins[:-2]) + admins[-2] + ' or ' + admins[-1])
+    # admins come from settings.ADMINS, a developer-controlled config value
+    return mark_safe(', '.join(admins[:-2]) + admins[-2] + ' or ' + admins[-1])  # noqa: S308

patchwork/templatetags/patch.py

@@ -27,7 +27,8 @@ def patch_tags(patch):
         else:
             counts.append(str(count))
 
-    return mark_safe(
+    # counts are integers; titles contain tag names set by project maintainers
+    return mark_safe(  # noqa: S308
         '<span title="%s">%s</span>' % (' / '.join(titles), ' '.join(counts))
     )
 
@@ -53,7 +54,9 @@ def patch_checks(patch):
 
     check_elements.reverse()
 
-    return mark_safe(
+    # titles are hardcoded strings; check_elements contain hardcoded CSS class
+    # names and integer counts
+    return mark_safe(  # noqa: S308
         '<span title="%s">%s</span>'
         % (' / '.join(titles), ''.join(check_elements))
     )
@@ -67,6 +70,7 @@ def patch_commit_display(patch):
     if not fmt:
         return escape(commit)
 
-    return mark_safe(
+    # both the URL and display text are run through escape() above
+    return mark_safe(  # noqa: S308
         '<a href="%s">%s</a>' % (escape(fmt.format(commit)), escape(commit))
     )