Prevent losing unsafe from lookups (#77609)

* Prevent losing unsafe from lookups

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

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)

lib/ansible/template/__init__.py

@@ -1080,8 +1080,12 @@ 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 not self.jinja2_native and not convert_data:
@@ -1089,7 +1093,7 @@ class Templar:
                 else:
                     res = self.environment.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:
@@ -1100,6 +1104,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 isinstance(res, string_types) and preserve_trailing_newlines:
                 # The low level calls above do not preserve the newline

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

test/units/template/test_templar.py

@@ -443,3 +443,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)