From df3a200df3847a50d27fbe5477759ad8248342bf Mon Sep 17 00:00:00 2001 From: Brendan Harley Date: Fri, 26 May 2017 04:20:56 +0200 Subject: [PATCH 1/2] Fix unfailing CheckExternalCommands On Unix, with shell=True, the shell default to /bin/sh. Using Popen(['type', command], shell=True) is equivalent to calling Popen(['/bin/sh', '-c', 'type', command]). In this case 'command' becomes a positional parameter to the shell, and not an argument to the command 'type'. The solution is to pass a single string as parameter. The problem is that with shell=True, we are never safe from a shell injection, so it is wiser to use a python only solution. The package distutils is part of the standard distribution, so it doesn't add extra dependencies. The method find_executable has the same behaviour as 'which' on bash. --- bootstrapvz/common/tasks/host.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bootstrapvz/common/tasks/host.py b/bootstrapvz/common/tasks/host.py index 075a912..412e226 100644 --- a/bootstrapvz/common/tasks/host.py +++ b/bootstrapvz/common/tasks/host.py @@ -9,14 +9,14 @@ class CheckExternalCommands(Task): @classmethod def run(cls, info): - from ..tools import log_check_call - from subprocess import CalledProcessError import re + import logging + from distutils.spawn import find_executable missing_packages = [] + log = logging.getLogger(__name__) for command, package in info.host_dependencies.items(): - try: - log_check_call(['type', command], shell=True) - except CalledProcessError: + log.debug('Checking availability of ' + command) + if find_executable(command) is None: if re.match('^https?:\/\/', package): msg = ('The command `{command}\' is not available, ' 'you can download the software at `{package}\'.' From acb17a98d03d82c82cee5c668ecd46cd5497a3e4 Mon Sep 17 00:00:00 2001 From: Brendan Harley Date: Sun, 2 Jul 2017 17:16:25 +0200 Subject: [PATCH 2/2] Add executable check to find_executable Find_executable returns a file in the path, so it must be checked for executability. --- bootstrapvz/common/tasks/host.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bootstrapvz/common/tasks/host.py b/bootstrapvz/common/tasks/host.py index 412e226..73d7d3d 100644 --- a/bootstrapvz/common/tasks/host.py +++ b/bootstrapvz/common/tasks/host.py @@ -10,13 +10,15 @@ class CheckExternalCommands(Task): @classmethod def run(cls, info): import re + import os import logging from distutils.spawn import find_executable missing_packages = [] log = logging.getLogger(__name__) for command, package in info.host_dependencies.items(): log.debug('Checking availability of ' + command) - if find_executable(command) is None: + path = find_executable(command) + if path is None or not os.access(path, os.X_OK): if re.match('^https?:\/\/', package): msg = ('The command `{command}\' is not available, ' 'you can download the software at `{package}\'.'