From 518a811b9fd1fc776b0f37ce67fdc1135e59b3ee Mon Sep 17 00:00:00 2001 From: "ChangBo Guo(gcb)" Date: Thu, 30 Nov 2017 13:28:06 +0800 Subject: [PATCH] add bandit to pep8 job Add the bandit security scanner to the pep8 job. * convert assert statement to raise AssertionError * Don't hard code temporary file path * skip B404,B603 Change-Id: If4bdb9569236927449648a8b750ae0fa2da76f53 --- oslo_privsep/comm.py | 7 +++++-- oslo_privsep/priv_context.py | 22 ++++++++++++---------- oslo_privsep/tests/test_priv_context.py | 16 ++++++++++------ test-requirements.txt | 3 +++ tox.ini | 7 ++++++- 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/oslo_privsep/comm.py b/oslo_privsep/comm.py index e3e7d9e..5c01ff4 100644 --- a/oslo_privsep/comm.py +++ b/oslo_privsep/comm.py @@ -131,7 +131,9 @@ class ClientChannel(object): self.out_of_band(data) else: with self.lock: - assert msgid in self.outstanding_msgs + if msgid not in self.outstanding_msgs: + raise AssertionError("msgid should in " + "outstanding_msgs.") self.outstanding_msgs[msgid].set_result(data) # EOF. Perhaps the privileged process exited? @@ -154,7 +156,8 @@ class ClientChannel(object): future = Future(self.lock) with self.lock: - assert myid not in self.outstanding_msgs + if myid in self.outstanding_msgs: + raise AssertionError("myid shoudn't be in outstanding_msgs.") self.outstanding_msgs[myid] = future try: self.writer.send((myid, msg)) diff --git a/oslo_privsep/priv_context.py b/oslo_privsep/priv_context.py index 98b5df3..1036701 100644 --- a/oslo_privsep/priv_context.py +++ b/oslo_privsep/priv_context.py @@ -134,12 +134,12 @@ class PrivContext(object): # alternative above. # These asserts here are just attempts to catch errors earlier. # TODO(gus): Consider replacing with setuptools entry_points. - assert self.pypath is not None, ( - 'helper_command requires priv_context ' - 'pypath to be specified') - assert importutils.import_class(self.pypath) is self, ( - 'helper_command requires priv_context pypath ' - 'for context object') + if self.pypath is None: + raise AssertionError('helper_command requires priv_context ' + 'pypath to be specified') + if importutils.import_class(self.pypath) is not self: + raise AssertionError('helper_command requires priv_context ' + 'pypath for context object') # Note order is important here. Deployments will (hopefully) # have the exact arguments in sudoers/rootwrap configs and @@ -179,16 +179,18 @@ class PrivContext(object): def entrypoint(self, func): """This is intended to be used as a decorator.""" - assert func.__module__.startswith(self.prefix), ( - '%r entrypoints must be below "%s"' % (self, self.prefix)) + if not func.__module__.startswith(self.prefix): + raise AssertionError('%r entrypoints must be below "%s"' % + (self, self.prefix)) # Right now, we only track a single context in # _ENTRYPOINT_ATTR. This could easily be expanded into a set, # but that will increase the memory overhead. Revisit if/when # someone has a need to associate the same entrypoint with # multiple contexts. - assert getattr(func, _ENTRYPOINT_ATTR, None) is None, ( - '%r is already associated with another PrivContext' % func) + if getattr(func, _ENTRYPOINT_ATTR, None) is not None: + raise AssertionError('%r is already associated with another ' + 'PrivContext' % func) f = functools.partial(self._wrap, func) setattr(f, _ENTRYPOINT_ATTR, self) diff --git a/oslo_privsep/tests/test_priv_context.py b/oslo_privsep/tests/test_priv_context.py index 535e48f..87e1176 100644 --- a/oslo_privsep/tests/test_priv_context.py +++ b/oslo_privsep/tests/test_priv_context.py @@ -18,6 +18,7 @@ import os import pipes import platform import sys +import tempfile import mock import testtools @@ -82,37 +83,40 @@ class PrivContextTest(testctx.TestContextTestCase): def test_helper_command(self): self.privsep_conf.privsep.helper_command = 'foo --bar' - cmd = testctx.context.helper_command('/tmp/sockpath') + _, temp_path = tempfile.mkstemp() + cmd = testctx.context.helper_command(temp_path) expected = [ 'foo', '--bar', '--privsep_context', testctx.context.pypath, - '--privsep_sock_path', '/tmp/sockpath', + '--privsep_sock_path', temp_path, ] self.assertEqual(expected, cmd) def test_helper_command_default(self): self.privsep_conf.config_file = ['/bar.conf'] - cmd = testctx.context.helper_command('/tmp/sockpath') + _, temp_path = tempfile.mkstemp() + cmd = testctx.context.helper_command(temp_path) expected = [ 'sudo', 'privsep-helper', '--config-file', '/bar.conf', # --config-dir arg should be skipped '--privsep_context', testctx.context.pypath, - '--privsep_sock_path', '/tmp/sockpath', + '--privsep_sock_path', temp_path, ] self.assertEqual(expected, cmd) def test_helper_command_default_dirtoo(self): self.privsep_conf.config_file = ['/bar.conf', '/baz.conf'] self.privsep_conf.config_dir = ['/foo.d'] - cmd = testctx.context.helper_command('/tmp/sockpath') + _, temp_path = tempfile.mkstemp() + cmd = testctx.context.helper_command(temp_path) expected = [ 'sudo', 'privsep-helper', '--config-file', '/bar.conf', '--config-file', '/baz.conf', '--config-dir', '/foo.d', '--privsep_context', testctx.context.pypath, - '--privsep_sock_path', '/tmp/sockpath', + '--privsep_sock_path', temp_path, ] self.assertEqual(expected, cmd) diff --git a/test-requirements.txt b/test-requirements.txt index c3d3c4e..f0d969a 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -11,3 +11,6 @@ fixtures>=3.0.0 # Apache-2.0/BSD openstackdocstheme>=1.17.0 # Apache-2.0 sphinx>=1.6.2 # BSD reno>=2.5.0 # Apache-2.0 + +# Bandit security code scanner +bandit>=1.1.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index 105cd93..0391ae3 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,12 @@ deps = commands = python setup.py testr --slowest --testr-args='{posargs}' [testenv:pep8] -commands = flake8 +deps = + -r{toxinidir}/test-requirements.txt +commands = + flake8 + # Run security linter + bandit -r oslo_privsep tests -n5 --skip B404,B603 [testenv:venv] commands = {posargs}