Skip to content

Commit

Permalink
squash! squash! squash! stdlib.run: Fix problems when an executable i…
Browse files Browse the repository at this point in the history
…s missing

* Move the check for the executable into the _call function to apply
  all other expected checks before we check the executable exists.

* 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.

* Added 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.
  • Loading branch information
pirat89 committed Aug 2, 2023
1 parent df86512 commit c840c61
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 16 deletions.
22 changes: 7 additions & 15 deletions leapp/libraries/stdlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
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.
"""
from distutils.spawn import find_executable
import base64
import errno
import logging
import os
import sys
Expand Down Expand Up @@ -178,21 +176,9 @@ 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
_path = (env or {}).get('PATH', None)
result = {}
try:
create_audit_entry('process-start', {'id': _id, 'parameters': args, 'env': env})
# 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(args[0], _path):
# NOTE: this does not handle issues with SELinux
result = {'exit_code': '127', 'stdout': '', 'signal': 0, 'pid': 0}
result['stderr'] = 'File not found or permission denied: {}'.format(args[0])
raise OSError(errno.ENOENT, os.strerror(errno.ENOENT), args[0])
result = _call(args, callback_raw=callback_raw, callback_linebuffered=callback_linebuffered,
stdin=stdin, env=env, encoding=encoding)
if checked and result['exit_code'] != 0:
Expand All @@ -207,6 +193,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:
Expand Down
24 changes: 23 additions & 1 deletion leapp/libraries/stdlib/call.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -121,11 +124,23 @@ def _call(command, callback_raw=lambda fd, value: None, callback_linebuffered=la
if not isinstance(read_buffer_size, int) or isinstance(read_buffer_size, bool) or read_buffer_size <= 0:
raise ValueError('read_buffer_size parameter has to be integer greater than zero')


environ = os.environ.copy()
if env:
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(args[0], _path):
raise OSError(errno.ENOENT, os.strerror(errno.ENOENT), args[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
Expand Down Expand Up @@ -214,4 +229,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)

0 comments on commit c840c61

Please sign in to comment.