This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8539 in repository https://gitbox.apache.org/repos/asf/allura.git
commit 99daa1fdfc121810da84be60ad75c7256f386949 Author: Dave Brondsema <dbronds...@slashdotmedia.com> AuthorDate: Wed Mar 20 16:37:16 2024 -0400 [#8539] add bandit checks --- Allura/allura/command/script.py | 2 +- Allura/allura/command/taskd.py | 2 +- Allura/allura/config/middleware.py | 2 +- Allura/allura/controllers/rest.py | 2 +- Allura/allura/lib/decorators.py | 2 +- Allura/allura/lib/helpers.py | 2 +- Allura/allura/lib/import_api.py | 2 +- Allura/allura/lib/phone/nexmo.py | 4 ++-- Allura/allura/lib/plugin.py | 5 +++-- Allura/allura/lib/project_create_helpers.py | 2 +- Allura/allura/model/index.py | 2 +- Allura/allura/model/notification.py | 4 ++-- Allura/allura/scripts/trac_export.py | 6 +++--- .../allura/tests/functional/test_neighborhood.py | 7 ++----- Allura/allura/tests/unit/phone/test_nexmo.py | 16 ++++++++++++---- Allura/allura/websetup/bootstrap.py | 2 +- Allura/ldap-setup.py | 2 +- Allura/setup.py | 2 +- ForgeImporters/forgeimporters/base.py | 2 +- .../forgetracker/tests/unit/test_ticket_model.py | 2 +- fuse/accessfs.py | 2 +- ruff.toml | 22 +++++++++++++++++++--- scripts/ApacheAccessHandler.py | 2 +- scripts/changelog.py | 2 +- scripts/new_ticket.py | 2 +- scripts/wiki-copy.py | 4 ++-- 26 files changed, 63 insertions(+), 41 deletions(-) diff --git a/Allura/allura/command/script.py b/Allura/allura/command/script.py index 52e58b2fb..78edd8746 100644 --- a/Allura/allura/command/script.py +++ b/Allura/allura/command/script.py @@ -73,7 +73,7 @@ class ScriptCommand(base.Command): pr = cProfile.Profile() pr.enable() - exec(code, ns) + exec(code, ns) # noqa: S102 if self.options.profile: pr.disable() diff --git a/Allura/allura/command/taskd.py b/Allura/allura/command/taskd.py index 9d6c42c67..782c216cd 100644 --- a/Allura/allura/command/taskd.py +++ b/Allura/allura/command/taskd.py @@ -160,7 +160,7 @@ class TaskdCommand(base.Command): if self.restart_when_done: base.log.info('taskd pid %s restarting itself' % os.getpid()) - os.execv(sys.argv[0], sys.argv) + os.execv(sys.argv[0], sys.argv) # noqa: S606 class TaskCommand(base.Command): diff --git a/Allura/allura/config/middleware.py b/Allura/allura/config/middleware.py index e481a1621..edf28a968 100644 --- a/Allura/allura/config/middleware.py +++ b/Allura/allura/config/middleware.py @@ -91,7 +91,7 @@ def make_app(global_conf: dict, **app_conf): class BeakerPickleSerializerWithLatin1(PickleSerializer): def loads(self, data_string): # need latin1 to decode py2 timestamps in py https://docs.python.org/3/library/pickle.html#pickle.Unpickler - return pickle.loads(data_string, encoding='latin1') + return pickle.loads(data_string, encoding='latin1') # noqa: S301 def _make_core_app(root, global_conf: dict, **app_conf): diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py index ef4ae4ca5..f077b3d40 100644 --- a/Allura/allura/controllers/rest.py +++ b/Allura/allura/controllers/rest.py @@ -251,7 +251,7 @@ class OAuthNegotiator: return AlluraOauth1Server(Oauth1Validator()) def _authenticate(self): - bearer_token_prefix = 'Bearer ' + bearer_token_prefix = 'Bearer ' # noqa: S105 auth = request.headers.get('Authorization') if auth and auth.startswith(bearer_token_prefix): access_token = auth[len(bearer_token_prefix):] diff --git a/Allura/allura/lib/decorators.py b/Allura/allura/lib/decorators.py index eaf8016f8..a0fecc1f4 100644 --- a/Allura/allura/lib/decorators.py +++ b/Allura/allura/lib/decorators.py @@ -142,7 +142,7 @@ def reconfirm_auth(func, *args, **kwargs): session.save() kwargs.pop('password', None) else: - request.validation.errors['password'] = 'Invalid password.' + request.validation.errors['password'] = 'Invalid password.' # noqa: S105 allowed_timedelta = timedelta(seconds=asint(config.get('auth.reconfirm.seconds', 60))) last_reconfirm = session.get('auth-reconfirmed', datetime.min) diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py index f0675e443..5f5c41bdf 100644 --- a/Allura/allura/lib/helpers.py +++ b/Allura/allura/lib/helpers.py @@ -1080,7 +1080,7 @@ def urlopen(url: str | urllib.request.Request, retries=3, codes=(408, 500, 502, attempts = 0 while True: try: - return urllib.request.urlopen(url, timeout=timeout) + return urllib.request.urlopen(url, timeout=timeout) # noqa: S310 except OSError as e: no_retry = isinstance(e, urllib.error.HTTPError) and e.code not in codes if attempts < retries and not no_retry: diff --git a/Allura/allura/lib/import_api.py b/Allura/allura/lib/import_api.py index 141c03c00..f6d6282ab 100644 --- a/Allura/allura/lib/import_api.py +++ b/Allura/allura/lib/import_api.py @@ -47,7 +47,7 @@ class AlluraImportApiClient: while True: try: - result = six.moves.urllib.request.urlopen(url, six.ensure_binary(urlencode(params))) + result = six.moves.urllib.request.urlopen(url, six.ensure_binary(urlencode(params))) # noqa: S310 resp = result.read() return json.loads(resp) except six.moves.urllib.error.HTTPError as e: diff --git a/Allura/allura/lib/phone/nexmo.py b/Allura/allura/lib/phone/nexmo.py index 653159da2..630d7b94f 100644 --- a/Allura/allura/lib/phone/nexmo.py +++ b/Allura/allura/lib/phone/nexmo.py @@ -75,13 +75,13 @@ class NexmoPhoneService(PhoneService): url = urljoin(url, 'json') headers = {'Content-Type': 'application/json'} params = self.add_common_params(params) - log_params = dict(params, api_key='...', api_secret='...') + log_params = dict(params, api_key='...', api_secret='...') # noqa: S106 if 'number' in log_params: log_params['number'] = phone_number_hash(log_params['number']) post_params = json.dumps(params, sort_keys=True) log.info('PhoneService (nexmo) request: %s %s', url, log_params) try: - resp = requests.post(url, data=post_params, headers=headers) + resp = requests.post(url, data=post_params, headers=headers, timeout=30) log.info('PhoneService (nexmo) response: %s', resp.content) resp = resp.json() except Exception: diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py index 03b6d61f2..d32227d13 100644 --- a/Allura/allura/lib/plugin.py +++ b/Allura/allura/lib/plugin.py @@ -27,7 +27,6 @@ import string import crypt import random from contextlib import contextmanager -from urllib.request import urlopen from urllib.parse import urlparse from io import BytesIO from random import randint @@ -36,6 +35,8 @@ from base64 import b64encode from datetime import datetime, timedelta import typing import calendar + +import requests import six try: @@ -1129,7 +1130,7 @@ class ProjectRegistrationProvider: troves.append( M.TroveCategory.query.get(trove_cat_id=trove_id)._id) if 'icon' in project_template: - icon_file = BytesIO(urlopen(project_template['icon']['url']).read()) + icon_file = BytesIO(requests.get(project_template['icon']['url'], timeout=30).content) p.save_icon(project_template['icon']['filename'], icon_file) if user_project: diff --git a/Allura/allura/lib/project_create_helpers.py b/Allura/allura/lib/project_create_helpers.py index 77c2c5adf..387eb0f68 100644 --- a/Allura/allura/lib/project_create_helpers.py +++ b/Allura/allura/lib/project_create_helpers.py @@ -309,7 +309,7 @@ def create_project_with_attrs(p, nbhd, update=False, ensure_tools=False): granted_by_neighborhood_id=nbhd._id) if p.icon_url: - req = requests.get(p.icon_url) + req = requests.get(p.icon_url, timeout=30) req.raise_for_status() project.save_icon(urlparse(p.icon_url).path, BytesIO(req.content), diff --git a/Allura/allura/model/index.py b/Allura/allura/model/index.py index 374fcb9cf..c778db3b3 100644 --- a/Allura/allura/model/index.py +++ b/Allura/allura/model/index.py @@ -93,7 +93,7 @@ class ArtifactReference(MappedClass): '''Look up the artifact referenced''' aref = self.artifact_reference try: - cls = loads(bytes(aref.cls)) + cls = loads(bytes(aref.cls)) # noqa: S301 with h.push_context(aref.project_id): return cls.query.get(_id=aref.artifact_id) except Exception: diff --git a/Allura/allura/model/notification.py b/Allura/allura/model/notification.py index d97f86ee3..164cee374 100644 --- a/Allura/allura/model/notification.py +++ b/Allura/allura/model/notification.py @@ -109,7 +109,7 @@ class Notification(MappedClass): ref = RelationProperty('ArtifactReference') - view = jinja2.Environment( + view = jinja2.Environment( # noqa: S701 loader=jinja2.PackageLoader('allura', 'templates'), auto_reload=asbool(config.get('auto_reload_templates', True)), ) @@ -688,7 +688,7 @@ class Mailbox(MappedClass): class MailFooter: - view = jinja2.Environment( + view = jinja2.Environment( # noqa: S701 loader=jinja2.PackageLoader('allura', 'templates'), auto_reload=asbool(config.get('auto_reload_templates', True)), ) diff --git a/Allura/allura/scripts/trac_export.py b/Allura/allura/scripts/trac_export.py index e40aa85f4..ce9d2c9e2 100644 --- a/Allura/allura/scripts/trac_export.py +++ b/Allura/allura/scripts/trac_export.py @@ -143,7 +143,7 @@ class TracExport: def csvopen(self, url): self.log_url(url) - f = urlopen(url) + f = urlopen(url) # noqa: S310 # Trac doesn't throw 403 error, just shows normal 200 HTML page # telling that access denied. So, we'll emulate 403 ourselves. # TODO: currently, any non-csv result treated as 403. @@ -166,7 +166,7 @@ class TracExport: html2text.BODY_WIDTH = 0 url = self.full_url(self.TICKET_URL % id) self.log_url(url) - d = BeautifulSoup(urlopen(url)) + d = BeautifulSoup(urlopen(url)) # noqa: S310 self.clean_missing_wiki_links(d) desc = d.find('div', 'description').find('div', 'searchable') ticket['description'] = html2text.html2text( @@ -195,7 +195,7 @@ class TracExport: # Scrape HTML to get ticket attachments url = self.full_url(self.ATTACHMENT_LIST_URL % id) self.log_url(url) - f = urlopen(url) + f = urlopen(url) # noqa: S310 soup = BeautifulSoup(f) attach = soup.find('div', id='attachments') list = [] diff --git a/Allura/allura/tests/functional/test_neighborhood.py b/Allura/allura/tests/functional/test_neighborhood.py index 13f46dfff..ec1633172 100644 --- a/Allura/allura/tests/functional/test_neighborhood.py +++ b/Allura/allura/tests/functional/test_neighborhood.py @@ -670,9 +670,6 @@ class TestNeighborhood(TestController): def test_project_template(self): setup_trove_categories() - icon_url = 'file://' + \ - os.path.join(allura.__path__[0], 'nf', 'allura', - 'images', 'neo-icon-set-454545-256x350.png') test_groups = [{ "name": "Viewer", # group will be created, all params are valid "permissions": ["read"], @@ -699,7 +696,7 @@ class TestNeighborhood(TestController): project_template="""{ "private":true, "icon":{ - "url":"%s", + "url":"https://httpbin.org/image/png", "filename":"icon.png" }, "tools":{ @@ -727,7 +724,7 @@ class TestNeighborhood(TestController): "developmentstatus":[11] }, "groups": %s - }""" % (icon_url, json.dumps(test_groups))), + }""" % (json.dumps(test_groups))), extra_environ=dict(username='root')) r = self.app.post( '/adobe/register', diff --git a/Allura/allura/tests/unit/phone/test_nexmo.py b/Allura/allura/tests/unit/phone/test_nexmo.py index f2e99dacd..8b27d4de1 100644 --- a/Allura/allura/tests/unit/phone/test_nexmo.py +++ b/Allura/allura/tests/unit/phone/test_nexmo.py @@ -95,7 +95,9 @@ class TestPhoneService: req.post.assert_called_once_with( 'https://api.nexmo.com/verify/json', data=data, - headers=headers) + headers=headers, + timeout=30, + ) req.post.reset_mock() req.post.return_value.json.return_value = { @@ -108,7 +110,9 @@ class TestPhoneService: req.post.assert_called_once_with( 'https://api.nexmo.com/verify/json', data=data, - headers=headers) + headers=headers, + timeout=30, + ) @patch('allura.lib.phone.nexmo.requests', autospec=True) def test_verify_exception(self, req): @@ -138,7 +142,9 @@ class TestPhoneService: req.post.assert_called_once_with( 'https://api.nexmo.com/verify/check/json', data=data, - headers=headers) + headers=headers, + timeout=30, + ) req.post.reset_mock() req.post.return_value.json.return_value = { @@ -151,7 +157,9 @@ class TestPhoneService: req.post.assert_called_once_with( 'https://api.nexmo.com/verify/check/json', data=data, - headers=headers) + headers=headers, + timeout=30, + ) @patch('allura.lib.phone.nexmo.requests', autospec=True) def test_check_exception(self, req): diff --git a/Allura/allura/websetup/bootstrap.py b/Allura/allura/websetup/bootstrap.py index 765c91228..eadd141e9 100644 --- a/Allura/allura/websetup/bootstrap.py +++ b/Allura/allura/websetup/bootstrap.py @@ -307,7 +307,7 @@ def clear_all_database_tables(): db.drop_collection(coll) -def create_user(display_name, username=None, password='foo', make_project=False): +def create_user(display_name, username=None, password='foo', make_project=False): # noqa: S107 if not username: username = display_name.lower().replace(' ', '-') user = M.User.register(dict(username=username, diff --git a/Allura/ldap-setup.py b/Allura/ldap-setup.py index baf5432ac..e91343048 100644 --- a/Allura/ldap-setup.py +++ b/Allura/ldap-setup.py @@ -95,7 +95,7 @@ def get_value(key, default): def run(command): - rc = os.system(command) + rc = os.system(command) # noqa: S605 if rc != 0: log.error('Error running %s', command) assert rc == 0 diff --git a/Allura/setup.py b/Allura/setup.py index e5c549376..63613d66a 100644 --- a/Allura/setup.py +++ b/Allura/setup.py @@ -17,7 +17,7 @@ from setuptools import setup, find_packages -exec(open('allura/version.py').read()) +exec(open('allura/version.py').read()) # noqa: S102 PROJECT_DESCRIPTION = ''' Allura is an open source implementation of a software "forge", a web site diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py index e66cce270..79984d011 100644 --- a/ForgeImporters/forgeimporters/base.py +++ b/ForgeImporters/forgeimporters/base.py @@ -167,7 +167,7 @@ class ProjectExtractor: @staticmethod def urlopen(url, retries=3, codes=(408, 500, 502, 503, 504), timeout=120, unredirected_hdrs=None, **kw): - req = urllib.request.Request(url, **kw) + req = urllib.request.Request(url, **kw) # noqa: S310 if unredirected_hdrs: for key, val in unredirected_hdrs.items(): req.add_unredirected_header(key, val) diff --git a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py index afd604678..9ea0c37a0 100644 --- a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py +++ b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py @@ -349,7 +349,7 @@ class TestTicketModel(TrackerTestWithModel): ticket.summary = 'test ticket' ticket.description = 'test description' assert len(ticket.attachments) == 0 - f = six.moves.urllib.request.urlopen('file://%s' % __file__) + f = six.moves.urllib.request.urlopen('file://%s' % __file__) # noqa: S310 TicketAttachment.save_attachment( 'test_ticket_model.py', ResettableStream(f), artifact_id=ticket._id) diff --git a/fuse/accessfs.py b/fuse/accessfs.py index 4d43e7e2e..d5e843622 100644 --- a/fuse/accessfs.py +++ b/fuse/accessfs.py @@ -341,7 +341,7 @@ class PermissionCache: repo_path=path, username=uname))) print(f'Checking access for {uname} at {url} ({path})') - fp = six.moves.urllib.request.urlopen(url) + fp = six.moves.urllib.request.urlopen(url) # noqa: S310 result = json.load(fp) print(result) entry = 0 diff --git a/ruff.toml b/ruff.toml index 2bfa1ec9e..7da944339 100644 --- a/ruff.toml +++ b/ruff.toml @@ -29,7 +29,7 @@ lint.select = [ "ISC001", # NIC001 in flake8 codes "B", "PGH", # https://github.com/pre-commit/pygrep-hooks - "S307", # eval + "S", # bandit "G010", # logging.warn "T10", # debugger breakpoints "T20", # print() @@ -47,13 +47,29 @@ lint.ignore = [ 'B007', # Loop control variable not used within the loop body 'B904', # use raise from 'B905', # zip(strict=True) would be good, but need to closely evaluate all existing cases first + 'S101', # assert + 'S103', # permissive filesystem mask + 'S104', # network binding, which doesn't apply to our code, and gives false-positive on test IPs + 'S108', # temp files + 'S110', # try-except-pass, too many exist already + 'S303', # md5 & sha1 + 'S311', # random + 'S324', # md5 & sha1 + 'S603', # subprocess + 'S607', # partial path ] [lint.per-file-ignores] '__init__.py' = ['F403'] # import * -'**/{alluratest,tests}/*' = [ +'{**/alluratest/*,**/tests/*,run_tests}' = [ 'B011', # assert False 'T20', # print + 'S105', # hard-coded secrets + 'S106', # hard-coded secrets + 'S107', # hard-coded password + 'S314', # XML parsing + 'S602', # subprocess shell=True + 'S604', # other shell=True ] '{scripts,Allura/allura/scripts,Allura/allura/command}/*' = ['T20'] # print -'{run_tests,fuse/accessfs.py,ForgeSVN/setup.py}' = ['T20'] # print +'{fuse/accessfs.py,ForgeSVN/setup.py}' = ['T20'] # print diff --git a/scripts/ApacheAccessHandler.py b/scripts/ApacheAccessHandler.py index 63ed8431f..217e4ecf6 100644 --- a/scripts/ApacheAccessHandler.py +++ b/scripts/ApacheAccessHandler.py @@ -49,7 +49,7 @@ def load_requests_lib(req): if virtualenv_path: activate_this = '%s/bin/activate_this.py' % virtualenv_path try: - exec(compile(open(activate_this, "rb").read(), activate_this, 'exec'), {'__file__': activate_this}) + exec(compile(open(activate_this, "rb").read(), activate_this, 'exec'), {'__file__': activate_this}) # noqa: S102 except Exception as e: log(req, "Couldn't activate venv via {}: {}".format(activate_this, repr(e))) global requests diff --git a/scripts/changelog.py b/scripts/changelog.py index bdada653f..6d4c2a7e9 100755 --- a/scripts/changelog.py +++ b/scripts/changelog.py @@ -56,7 +56,7 @@ def get_tickets(from_ref, to_ref): def get_ticket_summaries(tickets): summaries = {} - r = requests.get(API_URL.format(' '.join(tickets))) + r = requests.get(API_URL.format(' '.join(tickets)), timeout=30) if r.status_code != 200: raise ValueError(f'Unexpected response code: {r.status_code}') for ticket in r.json()['tickets']: diff --git a/scripts/new_ticket.py b/scripts/new_ticket.py index 7588dbfd8..6dbf5cc73 100755 --- a/scripts/new_ticket.py +++ b/scripts/new_ticket.py @@ -51,7 +51,7 @@ if __name__ == '__main__': 'access_token': access_token, 'ticket_form.summary': summary, 'ticket_form.description': description, - }) + }, timeout=30) if r.status_code == 200: print('Ticket created at: %s' % r.url) pprint(r.json()) diff --git a/scripts/wiki-copy.py b/scripts/wiki-copy.py index ddb3187d3..c023684b0 100644 --- a/scripts/wiki-copy.py +++ b/scripts/wiki-copy.py @@ -41,12 +41,12 @@ def main(): base_url = options.to_wiki.split('/rest/')[0] oauth_client = make_oauth_client(base_url) - wiki_json = requests.get(options.from_wiki).json()['pages'] + wiki_json = requests.get(options.from_wiki, timeout=30).json()['pages'] for p in wiki_json: from_url = options.from_wiki.rstrip('/') + '/' + p to_url = options.to_wiki.rstrip('/') + '/' + p try: - page_json = requests.get(from_url).json() + page_json = requests.get(from_url, timeout=30).json() if options.debug: print(page_json['text']) break