Ensure that data within a tuple is marked as unsafe (#65918)

* Use is_sequence, and Mapping throughout, add support for tuples. Fixes #65722

* Address tests

* Remove unused import

* Add changelog

* Add docstring for clarity

* Argh, linting fix

* Not chasing this rabbit

* wrap_var doesn't return a ref to the original item

* no ref tests

* Remove unused import

changelogs/fragments/65722-unsafe-tuples.yml

@@ -0,0 +1,3 @@
+bugfixes:
+- unsafe_proxy - Ensure that data within a tuple is marked as unsafe
+  (https://github.com/ansible/ansible/issues/65722)

lib/ansible/executor/task_executor.py

@@ -662,7 +662,7 @@ class TaskExecutor:
                 if not isidentifier(self._task.register):
                     raise AnsibleError("Invalid variable name in 'register' specified: '%s'" % self._task.register)
 
-                vars_copy[self._task.register] = wrap_var(result)
+                vars_copy[self._task.register] = result = wrap_var(result)
 
             if self._task.async_val > 0:
                 if self._task.poll > 0 and not result.get('skipped') and not result.get('failed'):
@@ -720,7 +720,7 @@ class TaskExecutor:
             # This gives changed/failed_when access to additional recently modified
             # attributes of result
             if self._task.register:
-                vars_copy[self._task.register] = wrap_var(result)
+                vars_copy[self._task.register] = result = wrap_var(result)
 
             # if we didn't skip this task, use the helpers to evaluate the changed/
             # failed_when properties
@@ -751,7 +751,7 @@ class TaskExecutor:
         # 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] = wrap_var(result)
+            variables[self._task.register] = result = wrap_var(result)
 
         if 'ansible_facts' in result:
             if self._task.action in ('set_fact', 'include_vars'):

lib/ansible/template/__init__.py

@@ -44,6 +44,7 @@ from ansible.errors import AnsibleError, AnsibleFilterError, AnsibleUndefinedVar
 from ansible.module_utils.six import iteritems, string_types, text_type
 from ansible.module_utils._text import to_native, to_text, to_bytes
 from ansible.module_utils.common._collections_compat import Sequence, Mapping, MutableMapping
+from ansible.module_utils.common.collections import is_sequence
 from ansible.plugins.loader import filter_loader, lookup_loader, test_loader
 from ansible.template.safe_eval import safe_eval
 from ansible.template.template import AnsibleJ2Template
@@ -632,7 +633,7 @@ class Templar:
 
                 return result
 
-            elif isinstance(variable, (list, tuple)):
+            elif is_sequence(variable):
                 return [self.template(
                     v,
                     preserve_trailing_newlines=preserve_trailing_newlines,
@@ -640,7 +641,7 @@ class Templar:
                     overrides=overrides,
                     disable_lookups=disable_lookups,
                 ) for v in variable]
-            elif isinstance(variable, (dict, Mapping)):
+            elif isinstance(variable, Mapping):
                 d = {}
                 # we don't use iteritems() here to avoid problems if the underlying dict
                 # changes sizes due to the templating, which can happen with hostvars

lib/ansible/utils/unsafe_proxy.py

@@ -54,7 +54,8 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 from ansible.module_utils._text import to_bytes, to_text
-from ansible.module_utils.common._collections_compat import Mapping, MutableSequence, Set
+from ansible.module_utils.common._collections_compat import Mapping, Set
+from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.six import string_types, binary_type, text_type
 
 
@@ -97,21 +98,19 @@ class UnsafeProxy(object):
 
 
 def _wrap_dict(v):
-    for k in v.keys():
-        if v[k] is not None:
-            v[wrap_var(k)] = wrap_var(v[k])
-    return v
+    return dict((wrap_var(k), wrap_var(item)) for k, item in v.items())
 
 
-def _wrap_list(v):
-    for idx, item in enumerate(v):
-        if item is not None:
-            v[idx] = wrap_var(item)
-    return v
+def _wrap_sequence(v):
+    """Wraps a sequence with unsafe, not meant for strings, primarily
+    ``tuple`` and ``list``
+    """
+    v_type = type(v)
+    return v_type(wrap_var(item) for item in v)
 
 
 def _wrap_set(v):
-    return set(item if item is None else wrap_var(item) for item in v)
+    return set(wrap_var(item) for item in v)
 
 
 def wrap_var(v):
@@ -120,10 +119,10 @@ def wrap_var(v):
 
     if isinstance(v, Mapping):
         v = _wrap_dict(v)
-    elif isinstance(v, MutableSequence):
-        v = _wrap_list(v)
     elif isinstance(v, Set):
         v = _wrap_set(v)
+    elif is_sequence(v):
+        v = _wrap_sequence(v)
     elif isinstance(v, binary_type):
         v = AnsibleUnsafeBytes(v)
     elif isinstance(v, text_type):

test/units/utils/test_unsafe_proxy.py

@@ -62,8 +62,12 @@ def test_wrap_var_set_None():
 def test_wrap_var_tuple():
     assert isinstance(wrap_var(('foo',)), tuple)
     assert not isinstance(wrap_var(('foo',)), AnsibleUnsafe)
-    assert isinstance(wrap_var(('foo',))[0], type(''))
-    assert not isinstance(wrap_var(('foo',))[0], AnsibleUnsafe)
+    assert isinstance(wrap_var(('foo',))[0], AnsibleUnsafe)
+
+
+def test_wrap_var_tuple_None():
+    assert wrap_var((None,))[0] is None
+    assert not isinstance(wrap_var((None,))[0], AnsibleUnsafe)
 
 
 def test_wrap_var_None():
@@ -79,6 +83,25 @@ def test_wrap_var_unsafe_bytes():
     assert isinstance(wrap_var(AnsibleUnsafeBytes(b'foo')), AnsibleUnsafeBytes)
 
 
+def test_wrap_var_no_ref():
+    thing = {
+        'foo': {
+            'bar': 'baz'
+        },
+        'bar': ['baz', 'qux'],
+        'baz': ('qux',),
+        'none': None,
+        'text': 'text',
+    }
+    wrapped_thing = wrap_var(thing)
+    thing is not wrapped_thing
+    thing['foo'] is not wrapped_thing['foo']
+    thing['bar'][0] is not wrapped_thing['bar'][0]
+    thing['baz'][0] is not wrapped_thing['baz'][0]
+    thing['none'] is not wrapped_thing['none']
+    thing['text'] is not wrapped_thing['text']
+
+
 def test_AnsibleUnsafeText():
     assert isinstance(AnsibleUnsafeText(u'foo'), AnsibleUnsafe)