Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdlib.run: Fix missing executable #836

Merged
merged 1 commit into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions leapp/libraries/stdlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
27 changes: 26 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 @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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)
32 changes: 32 additions & 0 deletions tests/scripts/test_stdlib_call_audit.py
Original file line number Diff line number Diff line change
@@ -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():
"""
Expand Down Expand Up @@ -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
fernflower marked this conversation as resolved.
Show resolved Hide resolved
"""
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/'})
10 changes: 5 additions & 5 deletions tests/scripts/test_utils_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down
Loading