stdlib.run: Fix problems when an executable is missing

Currently, if the executable required in `run` does not exist (or
is not executable), the child process's code is not replaced by
`os.execvpe` function and it raises the OSError instead. However,
the parent process does not get this OSError. It consumes exit code,
stderr, ... of the child process. So in case the code does something
like this:
    ```
    try:
        result = run(['non-executable'])
    except OSError:
        pass
    except CalledProcessError:
        # do something..
    ```
the child process passes and do whatever.. In case it ends with
zero exit code, the obtained result is usually something totally
different than expected in actors. Also there could be problems
with non-idempotent code, when some actions could be done twice
(once executed by the child, send time executed by the parent
[current] process).

We have realized that number of existing leapp actors for in-place
upgrades already count with the raise of OSError when executable
cannot be used. So we choosed for now to check whether executables
are present and raise OSError if not, so we are sure that only
one process leave the function really.

Also applied another seatbelt into the child process - if the
OSError is raised anyway despite our checks (e.g. SELinux prevents
the execution) let's just kill the process instead giving
it a possibility to continue. In such a case, always print a msg to
stderr of the child process and exit with ecode 1.

Note: To check an executable we use `distutils.spawn.find_executable`
which is deprecated in Python 3 and will be dropped in Python 3.12.
However it exists now for Python 2 & 3, so we use this one for now
and will replace it in future by `shutils.which` when the time comes.

Additional changes:
* Make pylint happy (set noqa for E721 for the check the Fields is not
created directly). The "isinstance" function has a different behaviour
in this case than we want.

* Fix imports to make pylint happy

* Set `result` to empty dict instead of None: in case a ValueError
  or TypeError is raised inside the _call function, it's expected
  to copy the `result` content inside the final block, however in
  case the results is None, it fails and raise additional exception
  that covers the original one.

* Update unit tests to cover issues with missing executable.

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,14 +167,18 @@ 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.'
         api.current_logger().error(message)
         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:

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)

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/'})

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,
 )