summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMartin Krizek <martin.krizek@gmail.com>2022-05-09 22:46:42 +0200
committerGitHub <noreply@github.com>2022-05-09 15:46:42 -0500
commitd5e5bd34d6249b3b0c66c3423fa09a9f3b032467 (patch)
tree555ad5e02c374d1ec81b22174b44b9bcee7fa414
parent19415e12384cad75115e0dda2c4df7b71c0bc6f6 (diff)
downloadansible-d5e5bd34d6249b3b0c66c3423fa09a9f3b032467.tar.gz
Prevent losing unsafe from lookups (#77609) (#77652)
This patch fixes a bug which under certain conditions results in data returned from lookups not being marked as unsafe. Each time Templar.do_template is invoked a new AnsibleContext is created and stored effectively at two places: 1) as an instance variable in templar_obj.cur_context 2) as a local variable called new_context in do_template method of Templar Due to custom functionality in Ansible's Context that allows for nested templating it is possible that during resolving variable's value template/do_template method is called recursively again, again creating a new context. At that point the problem manifests itself because as mentioned in 1) above the context is overwriten on the templar object which means that any subsequent calls to _lookup will use the new context to mark it as unsafe which is now different to the local new_context which is used for testing for unsafe property. The solution to the problem appears to be to restore the original context inside do_template and also to eliminate the local variable new_context to prevent problems in the future. It appears that we don't have a better way of storing the context other than as some form of global variable and so this appears to be the "best" solution possible at this point. Hopefully data tagging will be the solution here. For more examples see unit and integration tests included in this patch. Fixes #77535 (cherry picked from commit 3980eb8c09d170a861351f8aff4a1aa1a8cbb626)
-rw-r--r--changelogs/fragments/77535-prevent-losing-unsafe-lookups.yml2
-rw-r--r--lib/ansible/template/__init__.py12
-rw-r--r--test/integration/targets/template/unsafe.yml45
-rw-r--r--test/units/template/test_templar.py25
4 files changed, 81 insertions, 3 deletions
diff --git a/changelogs/fragments/77535-prevent-losing-unsafe-lookups.yml b/changelogs/fragments/77535-prevent-losing-unsafe-lookups.yml
new file mode 100644
index 0000000000..14ae6f2fed
--- /dev/null
+++ b/changelogs/fragments/77535-prevent-losing-unsafe-lookups.yml
@@ -0,0 +1,2 @@
+bugfixes:
+ - Prevent losing unsafe on results returned from lookups (https://github.com/ansible/ansible/issues/77535)
diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py
index 5006c035d4..00cfa00671 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -1107,15 +1107,19 @@ class Templar:
jvars = AnsibleJ2Vars(self, t.globals)
- self.cur_context = new_context = t.new_context(jvars, shared=True)
- rf = t.root_render_func(new_context)
+ # In case this is a recursive call to do_template we need to
+ # save/restore cur_context to prevent overriding __UNSAFE__.
+ cached_context = self.cur_context
+
+ self.cur_context = t.new_context(jvars, shared=True)
+ rf = t.root_render_func(self.cur_context)
try:
if self.jinja2_native:
res = ansible_native_concat(rf)
else:
res = j2_concat(rf)
- unsafe = getattr(new_context, 'unsafe', False)
+ unsafe = getattr(self.cur_context, 'unsafe', False)
if unsafe:
res = wrap_var(res)
except TypeError as te:
@@ -1126,6 +1130,8 @@ class Templar:
else:
display.debug("failing because of a type error, template data is: %s" % to_text(data))
raise AnsibleError("Unexpected templating type error occurred on (%s): %s" % (to_native(data), to_native(te)))
+ finally:
+ self.cur_context = cached_context
if self.jinja2_native and not isinstance(res, string_types):
return res
diff --git a/test/integration/targets/template/unsafe.yml b/test/integration/targets/template/unsafe.yml
index 6746e1ea0c..bef9a4b450 100644
--- a/test/integration/targets/template/unsafe.yml
+++ b/test/integration/targets/template/unsafe.yml
@@ -17,3 +17,48 @@
that:
- this_always_safe == imunsafe
- imunsafe == this_was_unsafe.strip()
+
+
+- hosts: localhost
+ gather_facts: false
+ vars:
+ output_dir: "{{ lookup('env', 'OUTPUT_DIR') }}"
+ tasks:
+ - set_fact:
+ unsafe_foo: "{{ lookup('list', var0) }}"
+ vars:
+ var0: "{{ var1 }}"
+ var1:
+ - unsafe
+
+ - assert:
+ that:
+ - "{{ unsafe_foo[0] | type_debug == 'AnsibleUnsafeText' }}"
+
+ - block:
+ - copy:
+ dest: "{{ file_name }}"
+ content: !unsafe "{{ i_should_not_be_templated }}"
+
+ - set_fact:
+ file_content: "{{ lookup('file', file_name) }}"
+
+ - assert:
+ that:
+ - not file_content is contains('unsafe')
+
+ - set_fact:
+ file_content: "{{ lookup('file', file_name_tmpl) }}"
+ vars:
+ file_name_tmpl: "{{ file_name }}"
+
+ - assert:
+ that:
+ - not file_content is contains('unsafe')
+ vars:
+ file_name: "{{ output_dir }}/unsafe_file"
+ i_should_not_be_templated: unsafe
+ always:
+ - file:
+ dest: "{{ file_name }}"
+ state: absent
diff --git a/test/units/template/test_templar.py b/test/units/template/test_templar.py
index dd6985ce3b..181bb31e9f 100644
--- a/test/units/template/test_templar.py
+++ b/test/units/template/test_templar.py
@@ -444,3 +444,28 @@ class TestAnsibleContext(BaseTemplar, unittest.TestCase):
def test_is_unsafe(self):
context = self._context()
self.assertFalse(context._is_unsafe(AnsibleUndefined()))
+
+
+def test_unsafe_lookup():
+ res = Templar(
+ None,
+ variables={
+ 'var0': '{{ var1 }}',
+ 'var1': ['unsafe'],
+ }
+ ).template('{{ lookup("list", var0) }}')
+ assert getattr(res[0], '__UNSAFE__', False)
+
+
+def test_unsafe_lookup_no_conversion():
+ res = Templar(
+ None,
+ variables={
+ 'var0': '{{ var1 }}',
+ 'var1': ['unsafe'],
+ }
+ ).template(
+ '{{ lookup("list", var0) }}',
+ convert_data=False,
+ )
+ assert getattr(res, '__UNSAFE__', False)