From 79b2b00ed30af0fd55f8df7b56f5adea7e05dbfa Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 20 Jul 2023 15:11:48 -0400 Subject: [PATCH 1/2] Pass environment variables privately to modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add POSIX note as windows works very diff Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) --- changelogs/fragments/hidden_envvars.yml | 2 ++ lib/ansible/executor/module_common.py | 12 +++++++++--- lib/ansible/keyword_desc.yml | 3 ++- lib/ansible/module_utils/basic.py | 6 ++++++ lib/ansible/module_utils/common/parameters.py | 1 + lib/ansible/module_utils/csharp/Ansible.Basic.cs | 1 + lib/ansible/playbook/base.py | 1 + lib/ansible/playbook/task.py | 2 ++ lib/ansible/plugins/action/__init__.py | 7 +++++-- test/units/plugins/action/test_action.py | 1 + 10 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/hidden_envvars.yml diff --git a/changelogs/fragments/hidden_envvars.yml b/changelogs/fragments/hidden_envvars.yml new file mode 100644 index 00000000000..f7922c93531 --- /dev/null +++ b/changelogs/fragments/hidden_envvars.yml @@ -0,0 +1,2 @@ +minor_changes: + - module_environment, new keyword that adds the ability to pass environment variables for POSIX targets directly into the module instead of wrapping all task execution. diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 150ed9acc79..13ee5bfc522 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -987,7 +987,8 @@ def _find_module_utils( async_timeout: int, become_plugin: BecomeBase | None, environment: dict[str, str], - remote_is_local: bool = False + remote_is_local: bool = False, + module_env: dict[str, str] | None = None, ) -> _BuiltModule: """ Given the source of the module, convert it to a Jinja2 template to insert @@ -996,6 +997,8 @@ def _find_module_utils( module_substyle: t.Literal['binary', 'jsonargs', 'non_native_want_json', 'old', 'powershell', 'python'] module_style: t.Literal['binary', 'new', 'non_native_want_json', 'old'] module_substyle = module_style = 'old' + if module_env is None: + module_env = {} # module_style is something important to calling code (ActionBase). It # determines how arguments are formatted (json vs k=v) and whether @@ -1010,7 +1013,8 @@ def _find_module_utils( # we substitute "from ansible.module_utils basic" for REPLACER module_style = 'new' module_substyle = 'python' - b_module_data = b_module_data.replace(REPLACER, b'from ansible.module_utils.basic import *') + replacer_header = ['from ansible.module_utils.basic import *', '', 'import os, json', '', f'os.environ.update({json.dumps(module_env)}))'] + b_module_data = b_module_data.replace(REPLACER, to_bytes('\n'.join(replacer_header))) elif NEW_STYLE_PYTHON_MODULE_RE.search(b_module_data): module_style = 'new' module_substyle = 'python' @@ -1197,7 +1201,7 @@ if __name__ == "__main__": module_data=b_module_data, module_path=module_path, module_args=module_args, - environment=environment, + environment=environment.update(module_env), async_timeout=async_timeout, become_plugin=become_plugin, substyle=module_substyle, @@ -1280,6 +1284,7 @@ def modify_module( become_plugin=None, environment=None, remote_is_local=False, + moduile_env=None, ) -> _BuiltModule: """ Used to insert chunks of code into modules before transfer rather than @@ -1321,6 +1326,7 @@ def modify_module( become_plugin=become_plugin, environment=environment, remote_is_local=remote_is_local, + module_env=module_env, ) b_module_data = module_bits.b_module_data diff --git a/lib/ansible/keyword_desc.yml b/lib/ansible/keyword_desc.yml index 2d4ec9aaa2a..c8d91a0a7ed 100644 --- a/lib/ansible/keyword_desc.yml +++ b/lib/ansible/keyword_desc.yml @@ -39,7 +39,8 @@ ignore_unreachable: Boolean that allows you to ignore task failures due to an un loop: "Takes a list for the task to iterate over, saving each list element into the ``item`` variable (configurable via loop_control)" loop_control: Several keys here allow you to modify/set loop behavior in a task. See :ref:`loop_control`. max_fail_percentage: can be used to abort the run after a given percentage of hosts in the current batch has failed. This only works on linear or linear-derived strategies. -module_defaults: Specifies default parameter values for modules. +module_environment: A dictionary of environment variables to be provided for the module(s) that are executed as a result of a task. Unlike `environment` this will not affect shell or privilege escalation and is only available to subprocesses of the module itself. +module_defaults: Specifies default parameter values for modules on POSIX targets. name: "Identifier. Can be used for documentation, or in tasks/handlers." no_log: Boolean that controls information disclosure. notify: "List of handlers to notify when the task returns a 'changed=True' status." diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index a630e4bf026..2fa0dd600cf 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -408,6 +408,7 @@ class AnsibleModule(object): self._legal_inputs = [] self._options_context = list() self._tmpdir = None + self._module_env = {} if add_file_common_args: for k, v in FILE_COMMON_ARGUMENTS.items(): @@ -421,9 +422,14 @@ class AnsibleModule(object): # a known valid (LANG=C) if it's an invalid/unavailable locale self._check_locale() + # get input from controller self._load_params() self._set_internal_properties() + # setup env based on module env vars if we have any + if self._module_env: + os.environ.update(self._module_env) + self.validator = ModuleArgumentSpecValidator(self.argument_spec, self.mutually_exclusive, self.required_together, diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index 129bb05178a..726e2c0abc3 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/lib/ansible/module_utils/common/parameters.py @@ -84,6 +84,7 @@ PASS_VARS: dict[str, t.Any] = { 'ignore_unknown_opts': ('_ignore_unknown_opts', False), 'module_name': ('_name', None), 'no_log': ('no_log', False), + 'module_env': ('_module_env', {}), 'remote_tmp': ('_remote_tmp', None), 'target_log_info': ('_target_log_info', None), 'selinux_special_fs': ('_selinux_special_fs', ['fuse', 'nfs', 'vboxsf', 'ramfs', '9p', 'vfat']), diff --git a/lib/ansible/module_utils/csharp/Ansible.Basic.cs b/lib/ansible/module_utils/csharp/Ansible.Basic.cs index 2752d8c3367..7d4afdfbfc0 100644 --- a/lib/ansible/module_utils/csharp/Ansible.Basic.cs +++ b/lib/ansible/module_utils/csharp/Ansible.Basic.cs @@ -83,6 +83,7 @@ namespace Ansible.Basic { "tmpdir", "tmpdir" }, { "verbosity", "Verbosity" }, { "version", "AnsibleVersion" }, + { "module_env", null }, }; private List passBools = new List() { "check_mode", "debug", "diff", "keep_remote_files", "ignore_unknown_opts", "no_log" }; private List passInts = new List() { "verbosity" }; diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index e1b2d4af12a..d546b9b4805 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -699,6 +699,7 @@ class Base(FieldAttributeBase): # flags and misc. settings environment = FieldAttribute(isa='list', extend=True, prepend=True) + module_environment = FieldAttribute(isa='dict') no_log = FieldAttribute(isa='bool', default=C.DEFAULT_NO_LOG) run_once = FieldAttribute(isa='bool') ignore_errors = FieldAttribute(isa='bool') diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 0876c0fa0d6..1afed69e1bf 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -443,6 +443,8 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl return env + _post_validate_module_env = _post_validate_environment + def _post_validate_changed_when(self, attr, value, templar): """ changed_when is evaluated after the execution of the task is complete, diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 9bcba7fbe99..47d6f08b432 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -320,8 +320,8 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): environment=final_environment, remote_is_local=bool(getattr(self._connection, '_remote_is_local', False)), become_plugin=self._connection.become, + module_env=self._task.module_environment, ) - break except InterpreterDiscoveryRequiredError as idre: self._discovered_interpreter = discover_interpreter(action=self, interpreter_name=idre.interpreter_name, @@ -724,7 +724,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): # of the other logic below will get run. This is fairly hacky and a # corner case, but probably one that shows up pretty often in # Solaris-based environments (and possibly others). - pass + display.debug(f"Ignoring auth failure on chmod: {e!r}") else: if res['rc'] == 0: return remote_paths @@ -1005,6 +1005,9 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): module_args['_ansible_tracebacks_for'] = _traceback.traceback_for() + # pass through confidential environment variables + module_args['_ansible_module_env'] = getattr(self._task, 'module_environment', {}) + def _execute_module(self, module_name=None, module_args=None, tmp=None, task_vars=None, persist_files=False, delete_remote_tmp=None, wrap_async=False, ignore_unknown_opts: bool = False): """ diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 5891be5c647..a34fd24ea46 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -684,6 +684,7 @@ class TestActionBase(unittest.TestCase): mock_task.diff = False mock_task.check_mode = False mock_task.no_log = False + mock_task.module_environment = {} # create a mock connection, so we don't actually try and connect to things def get_option(option): From 2bca90f07685bb262451fe4b2b185ae3c9c4d9ce Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 20 Oct 2025 21:42:35 -0400 Subject: [PATCH 2/2] new approach --- lib/ansible/plugins/action/__init__.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 47d6f08b432..8af8ea81d67 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -20,7 +20,10 @@ from abc import ABC, abstractmethod from collections.abc import Sequence from ansible import constants as C +from ansible import _internal +from ansible._internal._datatag._tags import SourceWasEncrypted from ansible._internal._errors import _captured, _error_utils +from ansible._internal._templating import _engine from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleActionSkip, AnsibleActionFail, AnsibleAuthenticationFailure from ansible.executor.module_common import modify_module, _BuiltModule from ansible.executor.interpreter_discovery import discover_interpreter, InterpreterDiscoveryRequiredError @@ -35,8 +38,7 @@ from ansible.utils.collection_loader import resource_from_fqcr from ansible.utils.display import Display from ansible.vars.clean import remove_internal_keys from ansible.utils.plugin_docs import get_versioned_doclink -from ansible import _internal -from ansible._internal._templating import _engine + from .. import _AnsiblePluginInfoMixin @@ -101,6 +103,8 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): # Backwards compat: self._display isn't really needed, just import the global display and use that. self._display = display + self.__internal_env = {} + @abstractmethod def run(self, tmp: str | None = None, task_vars: dict[str, t.Any] | None = None) -> dict[str, t.Any]: """ Action Plugins should implement this method to perform their @@ -320,7 +324,6 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): environment=final_environment, remote_is_local=bool(getattr(self._connection, '_remote_is_local', False)), become_plugin=self._connection.become, - module_env=self._task.module_environment, ) break except InterpreterDiscoveryRequiredError as idre: @@ -367,13 +370,20 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): temp_environment = self._templar.template(environment) if not isinstance(temp_environment, dict): raise AnsibleError("environment must be a dictionary, received %s (%s)" % (temp_environment, type(temp_environment))) + # very deliberately using update here instead of combine_vars, as # these environment settings should not need to merge sub-dicts - final_environment.update(temp_environment) + if SourceWasEncrypted.is_tagged_on(environment): + final_environment.update(temp_environment) + else: + self.__internal_env.update(temp_environment) if len(final_environment) > 0: final_environment = self._templar.template(final_environment) + if len(self.__internal_env) > 0: + self.__internal_env = self._templar.template(self.__internal_env) + if isinstance(raw_environment_out, dict): raw_environment_out.clear() raw_environment_out.update(final_environment) @@ -1006,7 +1016,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): module_args['_ansible_tracebacks_for'] = _traceback.traceback_for() # pass through confidential environment variables - module_args['_ansible_module_env'] = getattr(self._task, 'module_environment', {}) + module_args['_ansible_internal_env'] = self.__internal_env def _execute_module(self, module_name=None, module_args=None, tmp=None, task_vars=None, persist_files=False, delete_remote_tmp=None, wrap_async=False, ignore_unknown_opts: bool = False):