diff --git a/leapp/libraries/stdlib/__init__.py b/leapp/libraries/stdlib/__init__.py index 95974964a..77a2f7a29 100644 --- a/leapp/libraries/stdlib/__init__.py +++ b/leapp/libraries/stdlib/__init__.py @@ -3,11 +3,11 @@ represents a location for functions that otherwise would be defined multiple times across leapp actors and at the same time, they are really useful for other actors. """ +import base64 import logging import os import sys import uuid -import base64 from leapp.exceptions import LeappError from leapp.utils.audit import create_audit_entry @@ -167,6 +167,10 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb :type stdin: int, str :return: {'stdout' : stdout, 'stderr': stderr, 'signal': signal, 'exit_code': exit_code, 'pid': pid} :rtype: dict + :raises: OSError if an executable is missing or has wrong permissions + :raises: CalledProcessError if the cmd has non-zero exit code and `checked` is False + :raises: TypeError if any input parameters have an invalid type + :raises: valueError if any of input parameters have an invalid value """ if not args: message = 'Command to call is missing.' @@ -174,7 +178,7 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb raise ValueError(message) api.current_logger().debug('External command has started: {0}'.format(str(args))) _id = str(uuid.uuid4()) - result = None + result = {} try: create_audit_entry('process-start', {'id': _id, 'parameters': args, 'env': env}) result = _call(args, callback_raw=callback_raw, callback_linebuffered=callback_linebuffered, @@ -191,6 +195,12 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb result.update({ 'stdout': result['stdout'].splitlines() }) + except OSError: + # NOTE: currently we expect the result to be always set + # let's copy bash a little bit and set ecode 127 + result = {'exit_code': '127', 'stdout': '', 'signal': 0, 'pid': 0} + result['stderr'] = 'File not found or permission denied: {}'.format(args[0]) + raise finally: audit_result = result if not encoding: diff --git a/leapp/libraries/stdlib/call.py b/leapp/libraries/stdlib/call.py index 719bbd615..363fd279e 100644 --- a/leapp/libraries/stdlib/call.py +++ b/leapp/libraries/stdlib/call.py @@ -1,7 +1,10 @@ from __future__ import print_function +from distutils.spawn import find_executable import codecs +import errno import os +import sys from leapp.compat import string_types from leapp.libraries.stdlib.eventloop import POLL_HUP, POLL_IN, POLL_OUT, POLL_PRI, EventLoop @@ -107,6 +110,10 @@ def _call(command, callback_raw=lambda fd, value: None, callback_linebuffered=la :type env: dict :return: {'stdout' : stdout, 'stderr': stderr, 'signal': signal, 'exit_code': exit_code, 'pid': pid} :rtype: dict + :raises: OSError if an executable is missing or has wrong permissions + :raises: CalledProcessError if the cmd has non-zero exit code and `checked` is False + :raises: TypeError if any input parameters have an invalid type + :raises: valueError if any of input parameters have an invalid value """ if not isinstance(command, (list, tuple)): raise TypeError('command parameter has to be a list or tuple') @@ -126,6 +133,17 @@ def _call(command, callback_raw=lambda fd, value: None, callback_linebuffered=la if not isinstance(env, dict): raise TypeError('env parameter has to be a dictionary') environ.update(env) + + _path = (env or {}).get('PATH', None) + # NOTE(pstodulk): the find_executable function is from the distutils + # module which is deprecated and it is going to be removed in Python 3.12. + # In future, we should use the shutil.which function, however that one is + # not available for Python2. We are going to address the problem in future + # (e.g. when we drop support for Python 2). + # https://peps.python.org/pep-0632/ + if not find_executable(command[0], _path): + raise OSError(errno.ENOENT, os.strerror(errno.ENOENT), command[0]) + # Create a separate pipe for stdout/stderr # # The parent process is going to use the read-end of the pipes for reading child's @@ -214,4 +232,11 @@ def _call(command, callback_raw=lambda fd, value: None, callback_linebuffered=la os.close(stderr) os.dup2(wstdout, STDOUT) os.dup2(wstderr, STDERR) - os.execvpe(command[0], command, env=environ) + try: + os.execvpe(command[0], command, env=environ) + except OSError as e: + # This is a seatbelt in case the execvpe cannot be performed + # (e.g. permission denied) and we didn't catch this prior the fork. + # See the PR for more details: https://github.com/oamg/leapp/pull/836 + sys.stderr.write('Error: Cannot execute {}: {}\n'.format(command[0], str(e))) + os._exit(1) diff --git a/tests/scripts/test_stdlib_call_audit.py b/tests/scripts/test_stdlib_call_audit.py index 7f61e8307..521358e95 100644 --- a/tests/scripts/test_stdlib_call_audit.py +++ b/tests/scripts/test_stdlib_call_audit.py @@ -1,8 +1,19 @@ +import os import pytest import six from leapp.libraries.stdlib import run +CUR_DIR = os.path.dirname(os.path.abspath(__file__)) + + +@pytest.fixture +def adjust_cwd(): + previous_cwd = os.getcwd() + os.chdir(os.path.join(CUR_DIR, "../")) + yield + os.chdir(previous_cwd) + def test_invalid_command(): """ @@ -35,3 +46,24 @@ def test_no_encoding(): cmd = ['echo', '-n', '-e', '\\xeb'] result = run(cmd, encoding=None) assert isinstance(result['stdout'], six.binary_type) + + +def test_missing_executable(monkeypatch, adjust_cwd): # no-qa: W0613; pylint: disable=unused-argument + """ + In case the executable is missing. + + :return: Pass/Fail + """ + def mocked_fork(): + # raise the generic exception as we want to prevent fork in this case + # and want to bypass possible catch + raise Exception() # no-qa: W0719; pylint: disable=broad-exception-raised + + monkeypatch.setattr(os, 'fork', mocked_fork) + with pytest.raises(OSError): + run(['my-non-existing-exec-2023-asdf']) + + with pytest.raises(OSError): + # this file is not executable, so even if it exists the OSError should + # be still raised + run(['panagrams'], env={'PATH': '../data/call_data/'}) diff --git a/tests/scripts/test_utils_project.py b/tests/scripts/test_utils_project.py index c58a8a6b4..33b09c595 100644 --- a/tests/scripts/test_utils_project.py +++ b/tests/scripts/test_utils_project.py @@ -5,13 +5,13 @@ from helpers import TESTING_REPOSITORY_NAME from leapp.exceptions import CommandError from leapp.utils.repository import ( - requires_repository, - to_snake_case, - make_class_name, - make_name, find_repository_basedir, - get_repository_name, get_repository_metadata, + get_repository_name, + make_class_name, + make_name, + requires_repository, + to_snake_case, )