From ece85abbc46e087187caf6e05b1515b97c578531 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 19 Aug 2022 19:45:51 -0700 Subject: [PATCH] ansible-test - Verify executables are executable. (#78606) --- .../fragments/ansible-test-verify-executables.yml | 6 ++++++ .../_internal/commands/integration/coverage.py | 5 +++-- test/lib/ansible_test/_internal/util.py | 10 ++++++++++ test/lib/ansible_test/_internal/util_common.py | 9 +++++---- .../_util/controller/sanity/pylint/plugins/unwanted.py | 9 +++++++++ 5 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/ansible-test-verify-executables.yml diff --git a/changelogs/fragments/ansible-test-verify-executables.yml b/changelogs/fragments/ansible-test-verify-executables.yml new file mode 100644 index 00000000000..a1eff95d095 --- /dev/null +++ b/changelogs/fragments/ansible-test-verify-executables.yml @@ -0,0 +1,6 @@ +bugfixes: + - ansible-test - Temporary executables are now verified as executable after creation. + Without this check, path injected scripts may not be found, + typically on systems with ``/tmp`` mounted using the "noexec" option. + This can manifest as a missing Python interpreter, or use of the wrong Python interpreter, as well + as other error conditions. diff --git a/test/lib/ansible_test/_internal/commands/integration/coverage.py b/test/lib/ansible_test/_internal/commands/integration/coverage.py index c27c9fc379a..e9917692b11 100644 --- a/test/lib/ansible_test/_internal/commands/integration/coverage.py +++ b/test/lib/ansible_test/_internal/commands/integration/coverage.py @@ -33,6 +33,7 @@ from ...util import ( get_type_map, remove_tree, sanitize_host_name, + verified_chmod, ) from ...util_common import ( @@ -166,9 +167,9 @@ class PosixCoverageHandler(CoverageHandler[PosixConfig]): write_text_file(coverage_config_path, coverage_config, create_directories=True) - os.chmod(coverage_config_path, MODE_FILE) + verified_chmod(coverage_config_path, MODE_FILE) os.mkdir(coverage_output_path) - os.chmod(coverage_output_path, MODE_DIRECTORY_WRITE) + verified_chmod(coverage_output_path, MODE_DIRECTORY_WRITE) def setup_target(self): """Perform setup for code coverage on the target.""" diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py index 85a366aa3cb..11bfc1070f3 100644 --- a/test/lib/ansible_test/_internal/util.py +++ b/test/lib/ansible_test/_internal/util.py @@ -683,6 +683,16 @@ def pass_vars(required: c.Collection[str], optional: c.Collection[str]) -> dict[ return env +def verified_chmod(path: str, mode: int) -> None: + """Perform chmod on the specified path and then verify the permissions were applied.""" + os.chmod(path, mode) # pylint: disable=ansible-bad-function + + executable = any(mode & perm for perm in (stat.S_IXUSR, stat.S_IXGRP, stat.S_IXOTH)) + + if executable and not os.access(path, os.X_OK): + raise ApplicationError(f'Path "{path}" should executable, but is not. Is the filesystem mounted with the "noexec" option?') + + def remove_tree(path: str) -> None: """Remove the specified directory, siliently continuing if the directory does not exist.""" try: diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 2c9da4b5108..96beae0c11a 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -37,6 +37,7 @@ from .util import ( ApplicationError, SubprocessError, generate_name, + verified_chmod, ) from .io import ( @@ -257,9 +258,9 @@ def get_injector_path() -> str: script = set_shebang(script, shebang) write_text_file(dst, script) - os.chmod(dst, mode) + verified_chmod(dst, mode) - os.chmod(injector_path, MODE_DIRECTORY) + verified_chmod(injector_path, MODE_DIRECTORY) def cleanup_injector(): """Remove the temporary injector directory.""" @@ -320,7 +321,7 @@ def get_python_path(interpreter: str) -> str: create_interpreter_wrapper(interpreter, injected_interpreter) - os.chmod(python_path, MODE_DIRECTORY) + verified_chmod(python_path, MODE_DIRECTORY) if not PYTHON_PATHS: atexit.register(cleanup_python_paths) @@ -358,7 +359,7 @@ def create_interpreter_wrapper(interpreter: str, injected_interpreter: str) -> N write_text_file(injected_interpreter, code) - os.chmod(injected_interpreter, MODE_FILE_EXECUTE) + verified_chmod(injected_interpreter, MODE_FILE_EXECUTE) def cleanup_python_paths(): diff --git a/test/lib/ansible_test/_util/controller/sanity/pylint/plugins/unwanted.py b/test/lib/ansible_test/_util/controller/sanity/pylint/plugins/unwanted.py index 6ef5dc2ac97..1be42f51f23 100644 --- a/test/lib/ansible_test/_util/controller/sanity/pylint/plugins/unwanted.py +++ b/test/lib/ansible_test/_util/controller/sanity/pylint/plugins/unwanted.py @@ -21,11 +21,13 @@ class UnwantedEntry: modules_only=False, # type: bool names=None, # type: t.Optional[t.Tuple[str, ...]] ignore_paths=None, # type: t.Optional[t.Tuple[str, ...]] + ansible_test_only=False, # type: bool ): # type: (...) -> None self.alternative = alternative self.modules_only = modules_only self.names = set(names) if names else set() self.ignore_paths = ignore_paths + self.ansible_test_only = ansible_test_only def applies_to(self, path, name=None): # type: (str, t.Optional[str]) -> bool """Return True if this entry applies to the given path, otherwise return False.""" @@ -39,6 +41,9 @@ class UnwantedEntry: if self.ignore_paths and any(path.endswith(ignore_path) for ignore_path in self.ignore_paths): return False + if self.ansible_test_only and '/test/lib/ansible_test/_internal/' not in path: + return False + if self.modules_only: return is_module_path(path) @@ -114,6 +119,10 @@ class AnsibleUnwantedChecker(BaseChecker): # see https://docs.python.org/3/library/tempfile.html#tempfile.mktemp 'tempfile.mktemp': UnwantedEntry('tempfile.mkstemp'), + # os.chmod resolves as posix.chmod + 'posix.chmod': UnwantedEntry('verified_chmod', + ansible_test_only=True), + 'sys.exit': UnwantedEntry('exit_json or fail_json', ignore_paths=( '/lib/ansible/module_utils/basic.py',