From ffa548503d311a37194c7528e5d6026816c2ae58 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 4 Jun 2021 09:40:53 -0500 Subject: [PATCH] Wrap all results, regardless of register, except for actions with clean facts (#73161) * Wrap all results, regardless of register, except for actions with clean facts. Fixes #21088 * ci_complete * Add tests * Add clog frag --- ...unsafe-set-fact-include-vars-with-register.yml | 4 ++++ lib/ansible/executor/task_executor.py | 15 ++++++++++++--- .../targets/include_vars/tasks/main.yml | 11 +++++++++++ .../targets/include_vars/vars/no_auto_unsafe.yml | 1 + test/integration/targets/set_fact/runme.sh | 3 +++ .../targets/set_fact/set_fact_auto_unsafe.yml | 10 ++++++++++ 6 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/21088-no-auto-unsafe-set-fact-include-vars-with-register.yml create mode 100644 test/integration/targets/include_vars/vars/no_auto_unsafe.yml create mode 100644 test/integration/targets/set_fact/set_fact_auto_unsafe.yml diff --git a/changelogs/fragments/21088-no-auto-unsafe-set-fact-include-vars-with-register.yml b/changelogs/fragments/21088-no-auto-unsafe-set-fact-include-vars-with-register.yml new file mode 100644 index 00000000000..c4620e65ed5 --- /dev/null +++ b/changelogs/fragments/21088-no-auto-unsafe-set-fact-include-vars-with-register.yml @@ -0,0 +1,4 @@ +bugfixes: +- register - Ensure that ``register`` used on ``set_fact`` or ``include_vars`` + does not automatically wrap the facts as unsafe. + (https://github.com/ansible/ansible/issues/21088) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 84248ec133c..4891c0772f8 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -603,13 +603,16 @@ class TaskExecutor: # preserve no log result["_ansible_no_log"] = self._play_context.no_log + if self._task.action not in C._ACTION_WITH_CLEAN_FACTS: + result = wrap_var(result) + # update the local copy of vars with the registered value, if specified, # or any facts which may have been generated by the module execution if self._task.register: if not isidentifier(self._task.register): raise AnsibleError("Invalid variable name in 'register' specified: '%s'" % self._task.register) - vars_copy[self._task.register] = result = wrap_var(result) + vars_copy[self._task.register] = result if self._task.async_val > 0: if self._task.poll > 0 and not result.get('skipped') and not result.get('failed'): @@ -661,12 +664,15 @@ class TaskExecutor: if 'changed' not in result: result['changed'] = False + if self._task.action not in C._ACTION_WITH_CLEAN_FACTS: + result = wrap_var(result) + # re-update the local copy of vars with the registered value, if specified, # or any facts which may have been generated by the module execution # This gives changed/failed_when access to additional recently modified # attributes of result if self._task.register: - vars_copy[self._task.register] = result = wrap_var(result) + vars_copy[self._task.register] = result # if we didn't skip this task, use the helpers to evaluate the changed/ # failed_when properties @@ -708,10 +714,13 @@ class TaskExecutor: result['attempts'] = retries - 1 result['failed'] = True + if self._task.action not in C._ACTION_WITH_CLEAN_FACTS: + result = wrap_var(result) + # do the final update of the local variables here, for both registered # values and any facts which may have been created if self._task.register: - variables[self._task.register] = result = wrap_var(result) + variables[self._task.register] = result if 'ansible_facts' in result and self._task.action not in C._ACTION_DEBUG: if self._task.action in C._ACTION_WITH_CLEAN_FACTS: diff --git a/test/integration/targets/include_vars/tasks/main.yml b/test/integration/targets/include_vars/tasks/main.yml index 799d7b26a66..79f03d6e76b 100644 --- a/test/integration/targets/include_vars/tasks/main.yml +++ b/test/integration/targets/include_vars/tasks/main.yml @@ -57,6 +57,8 @@ include_vars: dir: vars extensions: ['', 'yaml', 'yml', 'json'] + ignore_files: + - no_auto_unsafe.yml register: include_every_dir - name: verify that the correct files have been loaded and overwrite based on alphabetical order @@ -78,6 +80,7 @@ ignore_files: - webapp.yml - file_without_extension + - no_auto_unsafe.yml register: include_without_webapp - name: verify that the webapp.yml file was not included @@ -162,3 +165,11 @@ that: - "'my_custom_service' == service_name_fqcn" - "'my_custom_service' == service_name_tmpl_fqcn" + +- include_vars: + file: no_auto_unsafe.yml + register: baz + +- assert: + that: + - baz.ansible_facts.foo|type_debug != "AnsibleUnsafeText" diff --git a/test/integration/targets/include_vars/vars/no_auto_unsafe.yml b/test/integration/targets/include_vars/vars/no_auto_unsafe.yml new file mode 100644 index 00000000000..20e9ff3feaa --- /dev/null +++ b/test/integration/targets/include_vars/vars/no_auto_unsafe.yml @@ -0,0 +1 @@ +foo: bar diff --git a/test/integration/targets/set_fact/runme.sh b/test/integration/targets/set_fact/runme.sh index 781894a0f03..93093599ec9 100755 --- a/test/integration/targets/set_fact/runme.sh +++ b/test/integration/targets/set_fact/runme.sh @@ -31,3 +31,6 @@ ANSIBLE_JINJA2_NATIVE=1 ansible-playbook -v set_fact_bool_conv_jinja2_native.yml # Test parsing of values when using an empty string as a key ansible-playbook -i inventory set_fact_empty_str_key.yml + +# https://github.com/ansible/ansible/issues/21088 +ansible-playbook -i inventory "$@" set_fact_auto_unsafe.yml diff --git a/test/integration/targets/set_fact/set_fact_auto_unsafe.yml b/test/integration/targets/set_fact/set_fact_auto_unsafe.yml new file mode 100644 index 00000000000..b0fb4dcf653 --- /dev/null +++ b/test/integration/targets/set_fact/set_fact_auto_unsafe.yml @@ -0,0 +1,10 @@ +- hosts: localhost + gather_facts: false + tasks: + - set_fact: + foo: bar + register: baz + + - assert: + that: + - baz.ansible_facts.foo|type_debug != "AnsibleUnsafeText"