Enable upgrade, security rules

Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

pwclient/api.py

@@ -159,7 +159,7 @@ def __init__(self, server, *, username=None, password=None, token=None):
                 transport=transport,
                 allow_none=True,
             )
-        except (IOError, OSError):
+        except OSError:
             raise exceptions.APIError(f'Unable to connect to {self._server}')
 
         self._client = rpc
@@ -269,8 +269,8 @@ def patch_list(
             state_id = self._state_id_by_name(state)
             if state_id == 0:
                 sys.stderr.write(
-                    "Note: No State found matching %s*, "
-                    "ignoring filter\n" % state
+                    f"Note: No State found matching {state}*, "
+                    f"ignoring filter\n"
                 )
             else:
                 filters['state_id'] = state_id
@@ -279,8 +279,8 @@ def patch_list(
             project_id = self._project_id_by_name(project)
             if project_id == 0:
                 sys.stderr.write(
-                    "Note: No Project found matching %s, "
-                    "ignoring filter\n" % project
+                    f"Note: No Project found matching {project}, "
+                    f"ignoring filter\n"
                 )
             else:
                 filters['project_id'] = project_id
@@ -296,7 +296,7 @@ def patch_list(
             person_ids = self._person_ids_by_name(submitter)
             if len(person_ids) == 0:
                 sys.stderr.write(
-                    "Note: Nobody found matching *%s*\n" % submitter
+                    f"Note: Nobody found matching *{submitter}*\n"
                 )
             else:
                 for person_id in person_ids:
@@ -308,9 +308,7 @@ def patch_list(
             patches = []
             delegate_ids = self._person_ids_by_name(delegate)
             if len(delegate_ids) == 0:
-                sys.stderr.write(
-                    "Note: Nobody found matching *%s*\n" % delegate
-                )
+                sys.stderr.write(f"Note: Nobody found matching *{delegate}*\n")
             else:
                 for delegate_id in delegate_ids:
                     filters['delegate_id'] = delegate_id
@@ -324,7 +322,7 @@ def patch_get(self, patch_id):
         patch = self._client.patch_get(patch_id)
         if patch == {}:
             raise exceptions.APIError(
-                'Unable to fetch patch %d; does it exist?' % patch_id
+                f'Unable to fetch patch {patch_id}; does it exist?'
             )
 
         return self._decode_patch(patch)
@@ -341,7 +339,7 @@ def patch_get_mbox(self, patch_id):
         mbox = self._client.patch_get_mbox(patch_id)
         if len(mbox) == 0:
             raise exceptions.APIError(
-                'Unable to fetch mbox for patch %d; does it exist?' % patch_id
+                f'Unable to fetch mbox for patch {patch_id}; does it exist?'
             )
 
         return mbox, patch['filename']
@@ -361,9 +359,7 @@ def patch_set(
         if state:
             state_id = self._state_id_by_name(state)
             if state_id == 0:
-                raise exceptions.APIError(
-                    'No State found matching %s*' % state
-                )
+                raise exceptions.APIError(f'No State found matching {state}*')
                 sys.exit(1)
 
             params['state'] = state_id
@@ -377,9 +373,7 @@ def patch_set(
         try:
             self._client.patch_set(patch_id, params)
         except xmlrpclib.Fault as f:
-            raise exceptions.APIError(
-                'Error updating patch: %s' % f.faultString
-            )
+            raise exceptions.APIError(f'Error updating patch: {f.faultString}')
 
     # states
 
@@ -423,9 +417,7 @@ def check_create(
                 description,
             )
         except xmlrpclib.Fault as f:
-            raise exceptions.APIError(
-                'Error creating check: %s' % f.faultString
-            )
+            raise exceptions.APIError(f'Error creating check: {f.faultString}')
 
 
 class REST(API):

pwclient/patches.py

@@ -4,7 +4,6 @@
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-import io
 import itertools
 import os
 import re
@@ -48,12 +47,10 @@ def patch_field(matchobj):
         for patch in patches:
             print(format_field_re.sub(patch_field, format_str))
     else:
-        print("%-7s %-12s %s" % ("ID", "State", "Name"))
-        print("%-7s %-12s %s" % ("--", "-----", "----"))
+        print("ID      State        Name")
+        print("--      -----        ----")
         for patch in patches:
-            print(
-                "%-7d %-12s %s" % (patch['id'], patch['state'], patch['name'])
-            )
+            print(f"{patch['id']:<7} {patch['state']:<12} {patch['name']}")
 
 
 def action_list(
@@ -124,14 +121,14 @@ def action_info(api, patch_id):
         print(str(exc), file=sys.stderr)
         sys.exit(1)
 
-    s = "Information for patch id %d" % (patch_id)
+    s = f"Information for patch id {patch_id}"
     print(s)
     print('-' * len(s))
     for key, value in sorted(patch.items()):
         if value != '':
-            print("- %- 14s: %s" % (key, value))
+            print(f"- {key:<14}: {value}")
         else:
-            print("- %- 14s:" % key)
+            print(f"- {key:<14}:")
 
 
 def action_get(api, patch_id):
@@ -145,12 +142,12 @@ def action_get(api, patch_id):
     fname += '.patch'
     i = 0
     while os.path.exists(fname):
-        fname = "%s.%d.patch" % (base_fname, i)
+        fname = f"{base_fname}.{i}.patch"
         i += 1
 
-    with io.open(fname, 'x', encoding='utf-8') as f:
+    with open(fname, 'x', encoding='utf-8') as f:
         f.write(mbox)
-        print('Saved patch to %s' % fname)
+        print(f'Saved patch to {fname}')
 
 
 def action_view(api, patch_ids):
@@ -159,7 +156,7 @@ def action_view(api, patch_ids):
     for patch_id in patch_ids:
         try:
             mbox, _ = api.patch_get_mbox(patch_id)
-        except Exception:
+        except Exception:  # noqa
             # TODO(stephenfin): We skip this for historical reasons, but should
             # we log/raise an error?
             continue
@@ -197,14 +194,13 @@ def action_apply(api, patch_id, apply_cmd=None):
         sys.exit(1)
 
     if apply_cmd is None:
-        print('Applying patch #%d to current directory' % patch_id)
+        print(f'Applying patch #{patch_id} to current directory')
         apply_cmd = ['patch', '-p1']
     else:
-        print(
-            'Applying patch #%d using "%s"' % (patch_id, ' '.join(apply_cmd))
-        )
+        _cmd = ' '.join(apply_cmd)
+        print(f'Applying patch #{patch_id} using "{_cmd}"')
 
-    print('Description: %s' % patch['name'])
+    print(f"Description: {patch['name']}")
 
     try:
         mbox, _ = api.patch_get_mbox(patch_id)

pwclient/shell.py

@@ -39,7 +39,7 @@ def main(argv=sys.argv[1:]):
     action = args.subcmd
 
     if not os.path.exists(CONFIG_FILE):
-        sys.stderr.write('Config file not found at %s.\n' % CONFIG_FILE)
+        sys.stderr.write(f'Config file not found at {CONFIG_FILE}.\n')
         sys.exit(1)
 
     # grab settings from config files
@@ -52,8 +52,7 @@ def main(argv=sys.argv[1:]):
 
     if not config.has_section('options'):
         sys.stderr.write(
-            'No options section in %s. Did you forget to uncomment it?\n'
-            % CONFIG_FILE
+            f'No options section in {CONFIG_FILE}. Did you forget to uncomment it?\n'
         )
         sys.exit(1)
 
@@ -67,19 +66,19 @@ def main(argv=sys.argv[1:]):
             configparser.NoOptionError,
         ):
             sys.stderr.write(
-                'No default project configured in %s\n' % CONFIG_FILE
+                f'No default project configured in {CONFIG_FILE}\n'
             )
             sys.exit(1)
 
     if not config.has_section(project_str):
         sys.stderr.write(
-            'No section for project %s in %s\n' % (project_str, CONFIG_FILE)
+            f'No section for project {project_str} in {CONFIG_FILE}\n'
         )
         sys.exit(1)
 
     if not config.has_option(project_str, 'url'):
         sys.stderr.write(
-            'No URL for project %s in %s\n' % (project_str, CONFIG_FILE)
+            f'No URL for project {project_str} in {CONFIG_FILE}\n'
         )
         sys.exit(1)
 
@@ -111,9 +110,9 @@ def main(argv=sys.argv[1:]):
                 or config.has_option(project_str, 'token')
             ):
                 sys.stderr.write(
-                    "The %s action requires authentication, but no "
-                    "username/password or\n"
-                    "token is configured\n" % action
+                    f"The {action} action requires authentication, but no "
+                    f"username/password or\n"
+                    f"token is configured\n"
                 )
                 sys.exit(1)
         else:
@@ -122,9 +121,9 @@ def main(argv=sys.argv[1:]):
                 and config.has_option(project_str, 'password')
             ):
                 sys.stderr.write(
-                    "The %s action requires authentication, but no "
-                    "username or password\n"
-                    "is configured\n" % action
+                    f"The {action} action requires authentication, but no "
+                    f"username or password\n"
+                    f"is configured\n"
                 )
                 sys.exit(1)
 
@@ -195,7 +194,7 @@ def main(argv=sys.argv[1:]):
         for patch_id in patch_ids:
             ret = patches.action_apply(api, patch_id)
             if ret:
-                sys.stderr.write("Apply failed with exit status %d\n" % ret)
+                sys.stderr.write(f"Apply failed with exit status {ret}\n")
                 sys.exit(1)
 
     elif action == 'git_am':
@@ -237,7 +236,7 @@ def main(argv=sys.argv[1:]):
         for patch_id in patch_ids:
             ret = patches.action_apply(api, patch_id, cmd)
             if ret:
-                sys.stderr.write("'git am' failed with exit status %d\n" % ret)
+                sys.stderr.write(f"'git am' failed with exit status {ret}\n")
                 sys.exit(1)
 
     elif action == 'update':

pwclient/utils.py

@@ -11,7 +11,7 @@
 
 def migrate_old_config_file(config_file, config):
     """Convert a config file to the Patchwork 1.0 format."""
-    sys.stderr.write('%s is in the old format. Migrating it...' % config_file)
+    sys.stderr.write(f'{config_file} is in the old format. Migrating it...')
 
     old_project = config.get('base', 'project')
 
@@ -35,7 +35,7 @@ def migrate_old_config_file(config_file, config):
 
     sys.stderr.write(' Done.\n')
     sys.stderr.write(
-        'Your old %s was saved to %s\n' % (config_file, old_config_file)
+        f'Your old {config_file} was saved to {old_config_file}\n'
     )
     sys.stderr.write('and was converted to the new format. You may want to\n')
     sys.stderr.write('inspect it before continuing.\n')

pwclient/xmlrpc.py

@@ -24,7 +24,7 @@ def __init__(self, url):
             self.https = self.proxy.startswith('https')
 
     def set_credentials(self, username=None, password=None):
-        self.credentials = '%s:%s' % (username, password)
+        self.credentials = f'{username}:{password}'
 
     def make_connection(self, host):
         self.host = host
@@ -38,7 +38,7 @@ def make_connection(self, host):
             return xmlrpclib.Transport.make_connection(self, host)
 
     def send_request(self, host, handler, request_body, debug):
-        handler = '%s://%s%s' % (self.scheme, host, handler)
+        handler = f'{self.scheme}://{host}{handler}'
         return xmlrpclib.Transport.send_request(
             self,
             host,

pyproject.toml

@@ -7,4 +7,13 @@ quote-style = "preserve"
 docstring-code-format = true
 
 [tool.ruff.lint]
-select = ["E4", "E7", "E9", "F"]
+select = ["E4", "E7", "E9", "F", "S", "U"]
+ignore = [
+    # we only use asserts for type narrowing
+    "S101",
+    # this is client tool running on a user's own system: users can open
+    # whatever urls they want to
+    "S310",
+    # ditto for running executables via subprocess
+    "S603",
+]

tests/test_patches.py

@@ -1,3 +1,4 @@
+import builtins
 from unittest import mock
 
 import pytest
@@ -229,7 +230,7 @@ def test_action_info__invalid_id(capsys):
     assert captured.err == 'foo\n'
 
 
-@mock.patch.object(patches.io, 'open')
+@mock.patch.object(builtins, 'open')
 @mock.patch.object(patches.os.path, 'basename')
 @mock.patch.object(patches.os.path, 'exists')
 def test_action_get(mock_exists, mock_basename, mock_open, capsys):

tests/test_shell.py

@@ -15,7 +15,7 @@
 _UNSET = object()
 
 
-class FakeConfig(object):
+class FakeConfig:
     def __init__(self, updates=None):
         self._data = {
             'options': {
@@ -119,7 +119,7 @@ def test_no_project_url(mock_config, capsys):
 
     captured = capsys.readouterr()
 
-    assert 'No URL for project %s' % DEFAULT_PROJECT in captured.err
+    assert f'No URL for project {DEFAULT_PROJECT}' in captured.err
     assert captured.out == ''