Re-allow templating of complex_args, but count params to prevent injection

Fixes #8810

lib/ansible/playbook/play.py

@@ -757,11 +757,6 @@ class Play(object):
 
     # *************************************************
 
-    def _has_vars_in(self, msg):
-        return "$" in msg or "{{" in msg
-
-    # *************************************************
-
     def _update_vars_files_for_host(self, host, vault_password=None):
 
         def generate_filenames(host, inject, filename):
@@ -782,7 +777,7 @@ class Play(object):
 
             # filename4 is the dwim'd path, but may also be mixed-scope, so we use
             # both play scoped vars and host scoped vars to template the filepath
-            if self._has_vars_in(filename3) and host is not None:
+            if utils.contains_vars(filename3) and host is not None:
                 inject.update(self.vars)
                 filename4 = template(self.basedir, filename3, inject)
                 filename4 = utils.path_dwim(self.basedir, filename4)
@@ -810,8 +805,8 @@ class Play(object):
                     raise errors.AnsibleError("%s must be stored as a dictionary/hash" % filename4)
                 if host is not None:
                     target_filename = None
-                    if self._has_vars_in(filename2):
-                        if not self._has_vars_in(filename3):
+                    if utils.contains_vars(filename2):
+                        if not utils.contains_vars(filename3):
                             target_filename = filename3
                         else:
                             target_filename = filename4
@@ -860,7 +855,7 @@ class Play(object):
             else:
                 # just one filename supplied, load it!
                 filename2, filename3, filename4 = generate_filenames(host, inject, filename)
-                if self._has_vars_in(filename4):
+                if utils.contains_vars(filename4):
                     continue
                 if process_files(filename, filename2, filename3, filename4, host=host):
                     processed.append(filename)

lib/ansible/runner/__init__.py

@@ -226,10 +226,6 @@ class Runner(object):
         # changed later via options like accelerate
         self.original_transport = self.transport
 
-        # enforce complex_args as a dict
-        if type(self.complex_args) != dict:
-            raise errors.AnsibleError("args must be a dictionary, received %s (%s)" % (self.complex_args, type(self.complex_args)))
-
         # misc housekeeping
         if subset and self.inventory._subset is None:
             # don't override subset when passed from playbook
@@ -677,11 +673,37 @@ class Runner(object):
                 inject['item'] = ",".join(use_these_items)
                 items = None
 
-        # logic to replace complex args if possible
-        complex_args = self.complex_args
+        def _safe_template_complex_args(args, inject):
+            # Ensure the complex args here are a dictionary, but
+            # first template them if they contain a variable
+            returned_args = args
+            if isinstance(args, basestring):
+                # If the complex_args were evaluated to a dictionary and there are
+                # more keys in the templated version than the evaled version, some
+                # param inserted additional keys (the template() call also runs
+                # safe_eval on the var if it looks like it's a datastructure). If the
+                # evaled_args are not a dict, it's most likely a whole variable (ie.
+                # args: {{var}}), in which case there's no way to detect the proper
+                # count of params in the dictionary.
+
+                templated_args = template.template(self.basedir, args, inject, convert_bare=True)
+                evaled_args = utils.safe_eval(args)
+
+                if isinstance(evaled_args, dict) and len(evaled_args) > 0 and len(evaled_args) != len(templated_args):
+                    raise errors.AnsibleError("a variable tried to insert extra parameters into the args for this task")
+
+                # set the returned_args to the templated_args
+                returned_args = templated_args
+
+            # and a final check to make sure the complex args are a dict
+            if not isinstance(returned_args, dict):
+                raise errors.AnsibleError("args must be a dictionary, received %s" % returned_args)
+
+            return returned_args
 
         # logic to decide how to run things depends on whether with_items is used
         if items is None:
+            complex_args = _safe_template_complex_args(self.complex_args, inject)
             return self._executor_internal_inner(host, self.module_name, self.module_args, inject, port, complex_args=complex_args)
         elif len(items) > 0:
 
@@ -700,12 +722,8 @@ class Runner(object):
                 this_inject = inject.copy()
                 this_inject['item'] = x
 
-                # TODO: this idiom should be replaced with an up-conversion to a Jinja2 template evaluation
-                if isinstance(self.complex_args, basestring):
-                    complex_args = template.template(self.basedir, self.complex_args, this_inject, convert_bare=True)
-                    complex_args = utils.safe_eval(complex_args)
-                    if type(complex_args) != dict:
-                        raise errors.AnsibleError("args must be a dictionary, received %s" % complex_args)
+                complex_args = _safe_template_complex_args(self.complex_args, this_inject)
+
                 result = self._executor_internal_inner(
                      host,
                      self.module_name,

lib/ansible/utils/__init__.py

@@ -1268,6 +1268,12 @@ def list_difference(a, b):
             result.append(x)
     return result
 
+def contains_vars(data):
+    '''
+    returns True if the data contains a variable pattern
+    '''
+    return "$" in data or "{{" in data
+
 def safe_eval(expr, locals={}, include_exceptions=False):
     '''
     This is intended for allowing things like:

test/units/TestUtils.py

@@ -518,6 +518,11 @@ class TestUtils(unittest.TestCase):
         self.assertEqual(ansible.utils.is_list_of_strings(['foo', 'bar', True]), False)
         self.assertEqual(ansible.utils.is_list_of_strings(['one', 2, 'three']), False)
 
+    def test_contains_vars(self):
+        self.assertTrue(ansible.utils.contains_vars('{{foo}}'))
+        self.assertTrue(ansible.utils.contains_vars('$foo'))
+        self.assertFalse(ansible.utils.contains_vars('foo'))
+
     def test_safe_eval(self):
         # Not basestring
         self.assertEqual(ansible.utils.safe_eval(len), len)