Skip to content

Commit 6ba8cc7

Browse files
committed
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. 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.
1 parent 154e1c5 commit 6ba8cc7

1 file changed

Lines changed: 17 additions & 1 deletion

File tree

leapp/libraries/stdlib/__init__.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
represents a location for functions that otherwise would be defined multiple times across leapp actors
44
and at the same time, they are really useful for other actors.
55
"""
6+
from distutils.spawn import find_executable
7+
import base64
8+
import errno
69
import logging
710
import os
811
import sys
912
import uuid
10-
import base64
1113

1214
from leapp.exceptions import LeappError
1315
from leapp.utils.audit import create_audit_entry
@@ -167,6 +169,8 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb
167169
:type stdin: int, str
168170
:return: {'stdout' : stdout, 'stderr': stderr, 'signal': signal, 'exit_code': exit_code, 'pid': pid}
169171
:rtype: dict
172+
:raises: OSError if an executable is missing or has wrong permissions
173+
:raises: CalledProcessError if the cmd has non-zero exit code and `checked` is False
170174
"""
171175
if not args:
172176
message = 'Command to call is missing.'
@@ -175,8 +179,20 @@ def run(args, split=False, callback_raw=_console_logging_handler, callback_lineb
175179
api.current_logger().debug('External command has started: {0}'.format(str(args)))
176180
_id = str(uuid.uuid4())
177181
result = None
182+
_path = (env or {}).get('PATH', None)
178183
try:
179184
create_audit_entry('process-start', {'id': _id, 'parameters': args, 'env': env})
185+
# NOTE(pstodulk): the find_executable function is from the distutils
186+
# module which is deprecated and it is going to be removed in Python 3.12.
187+
# In future, we should use the shutil.which function, however that one is
188+
# not available for Python2. We are going to address the problem in future
189+
# (e.g. when we drop support for Python 2).
190+
# https://peps.python.org/pep-0632/
191+
if not find_executable(args[0], _path):
192+
# NOTE: this does not handle issues with SELinux
193+
result = {'exit_code': '127', 'stdout': '', 'signal': 0, 'pid': 0}
194+
result['stderr'] = 'File not found or permission denied: {}'.format(args[0])
195+
raise OSError(errno.ENOENT, os.strerror(errno.ENOENT), args[0])
180196
result = _call(args, callback_raw=callback_raw, callback_linebuffered=callback_linebuffered,
181197
stdin=stdin, env=env, encoding=encoding)
182198
if checked and result['exit_code'] != 0:

0 commit comments

Comments
 (0)