Bandit security recommendations for Cylc 8 target

.travis/install.sh

@@ -33,7 +33,7 @@ if grep -E '(unit-tests|functional-tests)' <<< "${args[@]}"; then
 fi
 
 if grep 'unit-tests' <<< "${args[@]}"; then
-    pip install pycodestyle pytest
+    pip install pycodestyle pytest mock testfixtures
     # TODO: EmPy removed from testing, see:  #2958
 fi
 

bin/cylc-check-software

@@ -32,10 +32,12 @@ Arguments:
                 valid module arguments (lower-case equivalents accepted).
 """
 
-import sys
-import re
 import os
-from subprocess import check_call, PIPE, Popen, CalledProcessError
+import re
+import sys
+from subprocess import PIPE, CalledProcessError, check_call
+
+from cylc.cylc_subproc import procopen
 
 # Standardised output messages
 FOUND_NOVER_MSG = 'FOUND'
@@ -72,7 +74,7 @@ opt_result = {}
 
 def output_width(min_width=65, max_width=90):
     """Return a suitable output alignment width given user terminal width."""
-    proc = Popen(['stty', 'size'], stdout=PIPE)
+    proc = procopen(['stty', 'size'], stdoutpipe=True)
     if proc.wait():
         return int((min_width + max_width) / 2)
     else:
@@ -176,7 +178,7 @@ def tex_module_search(tex_module, write=True):
     cmd = ['kpsewhich', '%s.sty' % tex_module]
     # This is less intensive & quicker than searching via 'find' or 'locate'.
     try:
-        process = Popen(cmd, stdin=open(os.devnull), stdout=PIPE)
+        process = procopen(cmd, stdin=open(os.devnull), stdoutpipe=True)
         check_call(['test', '-n', process.communicate()[0].strip()],
                    stdin=open(os.devnull), stdout=PIPE, stderr=PIPE)
     except (CalledProcessError, OSError):
@@ -189,8 +191,8 @@ def tex_module_search(tex_module, write=True):
         return True
 
 
-def cmd_find_ver(
-    module, min_ver, cmd_base, ver_opt, ver_extr, outfile=1, write=True):
+def cmd_find_ver(module, min_ver, cmd_base, ver_opt, ver_extr, outfile=1,
+                 write=True):
     """Print outcome & return Boolean (True for pass) of local module version
     requirement test using relevant custom command base keyword(s),
     version-checking option(s) & version-extraction regex.
@@ -199,14 +201,15 @@ def cmd_find_ver(
                        min_ver is not None else 'any')
     for cmd in cmd_base:
         try_next_cmd = True
-        if Popen(['which', cmd], stdin=open(os.devnull), stdout=PIPE,
-                 stderr=PIPE).wait():
+        if procopen(['which', cmd], stdin=open(os.devnull),
+                    stdoutpipe=True, stderrpipe=True).wait():
             res = [NOTFOUND_MSG, False]
         else:
             try:
-                output = Popen([cmd, ver_opt], stdin=open(os.devnull),
-                               stdout=PIPE, stderr=PIPE
-                ).communicate()[outfile - 1].decode().strip()
+                output = procopen([cmd, ver_opt], stdoutpipe=True,
+                                  stdin=open(os.devnull),
+                                  stderrpipe=True).communicate()
+                [outfile - 1].decode().strip()
                 version = re.search(ver_extr, output).groups()[0]
                 try_next_cmd = False
                 if min_ver is None:

bin/cylc-check-versions

@@ -35,18 +35,19 @@ Use -v/--verbose to see the command invoked to determine the remote version
 site dependent -- see cylc global config documentation."""
 
 import os
+import shlex
 import sys
-from subprocess import Popen, PIPE
 from time import sleep
 
 import cylc.flags
-from cylc.option_parsers import CylcOptionParser as COP
-from cylc.version import CYLC_VERSION
 from cylc.config import SuiteConfig
+from cylc.option_parsers import CylcOptionParser as COP
+from cylc.cylc_subproc import procopen
 from cylc.subprocpool import SuiteProcPool
 from cylc.suite_srv_files_mgr import SuiteSrvFilesManager
 from cylc.task_remote_mgr import TaskRemoteMgr
 from cylc.templatevars import load_template_vars
+from cylc.version import CYLC_VERSION
 
 
 def main():
@@ -112,7 +113,8 @@ def main():
             user_at_host = host
         if verbose:
             print("%s: %s" % (user_at_host, ' '.join(argv)))
-        proc = Popen(argv, stdin=open(os.devnull), stdout=PIPE, stderr=PIPE)
+        proc = procopen(argv, stdin=open(os.devnull), stdoutpipe=True,
+                        stderrpipe=True)
         out, err = proc.communicate()
         out = out.decode()
         err = err.decode()

etc/dev-bin/defn-order-test.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python3
 
+import secrets
+import string
 import time
-from string import ascii_letters
-import random
 from copy import deepcopy
 
 # This is a standalone performance test of the algorithm used in gcylc to
@@ -16,17 +16,16 @@
 # order").
 names = []
 for i in range(0, N):
-    names.append(''.join(
-        random.choice(ascii_letters)
-        for n in range(5 + random.randrange(10))))
+    names.append(''.join(secrets.choice(string.ascii_letters)
+                         for n in range(5 + secrets.randrange(10))))
 
 # N lists with 2-7 names each (c.f. tree view paths of the inheritance
 # hierarchy).
 paths1 = []
 for i in range(0, N):
     p = []
-    for j in range(0, 2 + random.randrange(6)):
-        z = random.randrange(0, N)
+    for j in range(0, 2 + secrets.randrange(6)):
+        z = secrets.randrange(0, N)
         p.append(names[z])
     paths1.append(p)
 

etc/dev-suites/xtrigger/kafka/consumer/lib/python/cylc_kafka_consumer.py

@@ -91,8 +91,7 @@ def cylc_kafka_consumer(kafka_server, kafka_topic, group_id, message, debug):
 
     A match occurs Kafka if all message dict items match, and the result
     returned is the sub-dict of the actual values of items containing
-    angle-bracket-delineated regex patterns.
-    E.g. above {'data': 'nwp-2025.nc'}.
+    angle-bracket-delineated regex patterns. E.g. above {'data': 'nwp-2025.nc'}
 
     """
 

lib/cylc/batch_sys_manager.py

@@ -107,27 +107,24 @@
 
 """
 
-import os
 import json
+import os
 import shlex
-from shutil import rmtree
-from signal import SIGKILL
 import stat
-from subprocess import Popen, PIPE
 import sys
 import traceback
-
-
-from parsec.OrderedDict import OrderedDict
-
+from shutil import rmtree
+from signal import SIGKILL
 
 from cylc.task_message import (
     CYLC_JOB_PID, CYLC_JOB_INIT_TIME, CYLC_JOB_EXIT_TIME, CYLC_JOB_EXIT,
     CYLC_MESSAGE)
+from cylc.cylc_subproc import procopen
+from cylc.task_job_logs import (JOB_LOG_ERR, JOB_LOG_JOB, JOB_LOG_OUT,
+                                JOB_LOG_STATUS)
 from cylc.task_outputs import TASK_OUTPUT_SUCCEEDED
-from cylc.task_job_logs import (
-    JOB_LOG_JOB, JOB_LOG_OUT, JOB_LOG_ERR, JOB_LOG_STATUS)
 from cylc.wallclock import get_current_time_string
+from parsec import OrderedDict
 
 
 class JobPollContext(object):
@@ -419,8 +416,8 @@ def job_kill(self, st_file_path):
                     command = shlex.split(
                         batch_sys.KILL_CMD_TMPL % {"job_id": job_id})
                     try:
-                        proc = Popen(
-                            command, stdin=open(os.devnull), stderr=PIPE)
+                        proc = procopen(command, stdin=open(os.devnull),
+                                        stderrpipe=True)
                     except OSError as exc:
                         # subprocess.Popen has a bad habit of not setting the
                         # filename of the executable when it raises an OSError.
@@ -552,8 +549,8 @@ def _jobs_poll_batch_sys(self, job_log_root, batch_sys_name, my_ctx_list):
                 # Simple poll command that takes a list of job IDs
                 cmd = [batch_sys.POLL_CMD] + exp_ids
             try:
-                proc = Popen(
-                    cmd, stdin=open(os.devnull), stderr=PIPE, stdout=PIPE)
+                proc = procopen(cmd, stdin=open(os.devnull),
+                                stderrpipe=True, stdoutpipe=True)
             except OSError as exc:
                 # subprocess.Popen has a bad habit of not setting the
                 # filename of the executable when it raises an OSError.
@@ -662,18 +659,17 @@ def _job_submit_impl(
                 # that we do not have a shell, and still manage to get as far
                 # as here.
                 batch_sys_cmd = batch_submit_cmd_tmpl % {"job": job_file_path}
-                proc = Popen(
-                    batch_sys_cmd,
-                    stdin=proc_stdin_arg, stdout=PIPE, stderr=PIPE,
-                    shell=True, env=env)
+                proc = procopen(batch_sys_cmd, stdin=proc_stdin_arg,
+                                stdoutpipe=True, stderrpipe=True, usesh=True,
+                                env=env)
+                # calls to open a shell are aggregated in
+                # cylc_subproc.procopen()
             else:
                 command = shlex.split(
                     batch_sys.SUBMIT_CMD_TMPL % {"job": job_file_path})
                 try:
-                    proc = Popen(
-                        command,
-                        stdin=proc_stdin_arg, stdout=PIPE, stderr=PIPE,
-                        env=env)
+                    proc = procopen(command, stdin=proc_stdin_arg,
+                                    stdoutpipe=True, stderrpipe=True, env=env)
                 except OSError as exc:
                     # subprocess.Popen has a bad habit of not setting the
                     # filename of the executable when it raises an OSError.

lib/cylc/cylc_subproc.py

@@ -0,0 +1,58 @@
+#!/usr/bin/env python3
+
+# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
+# Copyright (C) 2008-2018 NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+""" A wrapper function to aggregate these calls in one file.
+    Bandit B602: subprocess_popen_with_shell_equals_true
+    https://docs.openstack.org/developer/bandit/plugins/subprocess_popen_with_shell_equals_true.html
+    B605: start_process_with_a_shell
+    https://docs.openstack.org/developer/bandit/plugins/start_process_with_a_shell.html
+"""
+from shlex import split
+from subprocess import PIPE, STDOUT, Popen  # nosec
+
+# pylint: disable=too-many-arguments
+# pylint: disable=too-many-locals
+
+
+def procopen(cmd, bufsize=0, executable=None, stdin=None, stdout=None,
+             stderr=None, preexec_fn=None, close_fds=False, usesh=False,
+             cwd=None, env=None, universal_newlines=False, startupinfo=None,
+             creationflags=0, splitcmd=False, stdoutpipe=False,
+             stdoutout=False, stderrpipe=False, stderrout=False):
+
+    shell = usesh
+
+    if stdoutpipe is True:
+        stdout = PIPE
+    elif stdoutout is True:
+        stdout = STDOUT
+    if stderrpipe is True:
+        stderr = PIPE
+    elif stderrout is True:
+        stderr = STDOUT
+
+    if splitcmd is True:
+        command = split(cmd)
+    else:
+        command = cmd
+
+    process = Popen(command, bufsize, executable, stdin, stdout,  # nosec
+                    stderr, preexec_fn, close_fds, shell, cwd, env,
+                    universal_newlines, startupinfo, creationflags)
+
+    return process

lib/cylc/profiling/profile.py

@@ -21,16 +21,18 @@
 
 import os
 import shutil
-from subprocess import Popen, PIPE, call
 import sys
 import tempfile
 import time
 import traceback
+from subprocess import PIPE, Popen, call  # nosec
 
-from . import (PROFILE_MODE_TIME, PROFILE_MODE_CYLC, PROFILE_MODES,
-               PROFILE_FILES, SUITE_STARTUP_STRING)
+from cylc.cylc_subproc import procopen
+
+from . import (PROFILE_FILES, PROFILE_MODE_CYLC, PROFILE_MODE_TIME,
+               PROFILE_MODES, SUITE_STARTUP_STRING)
 from .analysis import extract_results
-from .git import (checkout, describe, GitCheckoutError,)
+from .git import GitCheckoutError, checkout, describe
 
 
 def cylc_env(cylc_conf_path=''):
@@ -187,8 +189,11 @@ def run_suite(reg, options, out_file, profile_modes, mode='live',
     # Execute.
     print('$ ' + ' '.join(cmds))
     try:
-        proc = Popen(' '.join(cmds), shell=True, stderr=open(time_err, 'w+'),
-                     stdout=open(startup_file, 'w+'), env=env)
+        proc = procopen([' '.join(cmds)], usesh=True,
+                        stderr=open(time_err, 'w+'),
+                        # calls to open a shell are aggregated in
+                        # cylc_subproc.procopen()
+                        stdout=open(startup_file, 'w+'), env=env)
         if proc.wait():
             raise SuiteFailedException(run_cmds, cmd_out, cmd_err)
     except KeyboardInterrupt:

lib/cylc/run_get_stdout.py

@@ -20,9 +20,10 @@
 
 from os import devnull, killpg, setpgrp
 from signal import SIGTERM
-from subprocess import Popen, PIPE
 from time import sleep, time
 
+from cylc.cylc_subproc import procopen
+
 
 ERR_TIMEOUT = "ERROR: command timed out (>%ds), terminated by signal %d\n%s"
 ERR_SIGNAL = "ERROR: command terminated by signal %d\n%s"
@@ -44,9 +45,9 @@ def run_get_stdout(command, timeout=None, poll_delay=None):
 
     """
     try:
-        proc = Popen(
-            command, shell=True, preexec_fn=setpgrp, stdin=open(devnull),
-            stderr=PIPE, stdout=PIPE)
+        proc = procopen(command, usesh=True, preexec_fn=setpgrp,
+                        stdin=open(devnull), stderrpipe=True, stdoutpipe=True)
+        # calls to open a shell are aggregated in cylc_subproc.procopen()
         is_killed_after_timeout = False
         if timeout:
             if poll_delay is None: