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

* Use is_sequence, and Mapping throughout, add support for tuples. Fixes #65722
* Address tests
* Remove unused import
* Add changelog
* Add docstring for clarity

(cherry picked from commit f8654de851a841d33d2d2e76942c68a758535e56)
Co-authored-by: Matt Martz <matt@sivel.net>

5 files changed, 46 insertions, 20 deletions

diff --git a/changelogs/fragments/65722-unsafe-tuples.yml b/changelogs/fragments/65722-unsafe-tuples.yml
new file mode 100644
index 0000000000..7659b64c66
--- /dev/null
+++ b/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)
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index fe27754c84..506ff21a6b 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -682,7 +682,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'):
@@ -740,7 +740,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
@@ -771,7 +771,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 C._ACTION_WITH_CLEAN_FACTS:
diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py
index 8f0bab0f80..52b9f71977 100644
--- a/lib/ansible/template/__init__.py
+++ b/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.module_utils.compat.importlib import import_module
 from ansible.plugins.loader import filter_loader, lookup_loader, test_loader
 from ansible.template.safe_eval import safe_eval
@@ -640,7 +641,7 @@ class Templar:
 
                 return result
 
-            elif isinstance(variable, (list, tuple)):
+            elif is_sequence(variable):
                 return [self.template(
                     v,
                     preserve_trailing_newlines=preserve_trailing_newlines,
@@ -648,7 +649,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
diff --git a/lib/ansible/utils/unsafe_proxy.py b/lib/ansible/utils/unsafe_proxy.py
index b9122d28a2..ffb5f37503 100644
--- a/lib/ansible/utils/unsafe_proxy.py
+++ b/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
 
 
@@ -93,21 +94,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):
@@ -116,10 +115,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):
diff --git a/test/units/utils/test_unsafe_proxy.py b/test/units/utils/test_unsafe_proxy.py
index c2a9f861c5..205c0c652d 100644
--- a/test/units/utils/test_unsafe_proxy.py
+++ b/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)