issue #291: more Ansible-compatible script invocation

When running any kind of script, rewrite the hashbang like Ansible does,
but subsequently ignore it and explicitly use a fragment of shell from
the ansible_*_interpreter variable to call the interpreter, just like
Ansible does.

This fixes hashbangs containing '/usr/bin/env A=1 bash' on Linux, where
putting that into a hashbang line results in an infinite loop.

ansible_mitogen/connection.py

@@ -251,16 +251,13 @@ CONNECTION_METHOD = {
 
 def parse_python_path(s):
     """
-    Given the string set for ansible_python_interpeter, parse it as hashbang
+    Given the string set for ansible_python_interpeter, parse it using shell
     syntax and return an appropriate argument vector.
     """
     if not s:
         return None
 
-    interpreter, arg = ansible_mitogen.parsing.parse_script_interpreter(s)
-    if arg:
-        return [interpreter, arg]
-    return [interpreter]
+    return shlex.split(s)
 
 
 def config_from_play_context(transport, inventory_name, connection):

ansible_mitogen/planner.py

@@ -187,27 +187,51 @@ class ScriptPlanner(BinaryPlanner):
     Common functionality for script module planners -- handle interpreter
     detection and rewrite.
     """
+    def _rewrite_interpreter(self, path):
+        """
+        Given the original interpreter binary extracted from the script's
+        interpreter line, look up the associated `ansible_*_interpreter`
+        variable, render it and return it.
+
+        :param str path:
+            Absolute UNIX path to original interpreter.
+
+        :returns:
+            Shell fragment prefix used to execute the script via "/bin/sh -c".
+            While `ansible_*_interpreter` documentation suggests shell isn't
+            involved here, the vanilla implementation uses it and that use is
+            exploited in common playbooks.
+        """
+        try:
+            key = u'ansible_%s_interpreter' % os.path.basename(path).strip()
+            template = self._inv.task_vars[key]
+        except KeyError:
+            return path
+
+        return mitogen.utils.cast(
+            self._inv.templar.template(self._inv.task_vars[key])
+        )
+
     def _get_interpreter(self):
-        interpreter, arg = ansible_mitogen.parsing.parse_hashbang(
+        path, arg = ansible_mitogen.parsing.parse_hashbang(
             self._inv.module_source
         )
-        if interpreter is None:
+        if path is None:
             raise ansible.errors.AnsibleError(NO_INTERPRETER_MSG % (
                 self._inv.module_name,
             ))
 
-        key = u'ansible_%s_interpreter' % os.path.basename(interpreter).strip()
-        try:
-            template = self._inv.task_vars[key].strip()
-            return self._inv.templar.template(template), arg
-        except KeyError:
-            return interpreter, arg
+        fragment = self._rewrite_interpreter(path)
+        if arg:
+            fragment += ' ' + arg
+
+        return fragment, path.startswith('python')
 
     def get_kwargs(self, **kwargs):
-        interpreter, arg = self._get_interpreter()
+        interpreter_fragment, is_python = self._get_interpreter()
         return super(ScriptPlanner, self).get_kwargs(
-            interpreter_arg=arg,
-            interpreter=interpreter,
+            interpreter_fragment=interpreter_fragment,
+            is_python=is_python,
             **kwargs
         )
 

ansible_mitogen/runner.py

@@ -381,11 +381,10 @@ class ProgramRunner(Runner):
         )
 
     def _get_program_args(self):
-        return [
-            self.args['_ansible_shell_executable'],
-            '-c',
-            self.program_fp.name
-        ]
+        """
+        Return any arguments to pass to the program.
+        """
+        return []
 
     def revert(self):
         """
@@ -395,14 +394,20 @@ class ProgramRunner(Runner):
             self.program_fp.close()
         super(ProgramRunner, self).revert()
 
+    def _get_argv(self):
+        """
+        Return the final argument vector used to execute the program.
+        """
+        return [self.program_fp.name] + self._get_program_args()
+
     def _run(self):
         try:
             rc, stdout, stderr = ansible_mitogen.target.exec_args(
-                args=self._get_program_args(),
+                args=self._get_argv(),
                 emulate_tty=self.emulate_tty,
             )
         except Exception as e:
-            LOG.exception('While running %s', self._get_program_args())
+            LOG.exception('While running %s', self._get_argv())
             return {
                 'rc': 1,
                 'stdout': '',
@@ -442,11 +447,7 @@ class ArgsFileRunner(Runner):
         return json.dumps(self.args)
 
     def _get_program_args(self):
-        return [
-            self.args['_ansible_shell_executable'],
-            '-c',
-            "%s %s" % (self.program_fp.name, self.args_fp.name),
-        ]
+        return [self.args_fp.name]
 
     def revert(self):
         """
@@ -461,10 +462,10 @@ class BinaryRunner(ArgsFileRunner, ProgramRunner):
 
 
 class ScriptRunner(ProgramRunner):
-    def __init__(self, interpreter, interpreter_arg, **kwargs):
+    def __init__(self, interpreter_fragment, is_python, **kwargs):
         super(ScriptRunner, self).__init__(**kwargs)
-        self.interpreter = interpreter
-        self.interpreter_arg = interpreter_arg
+        self.interpreter_fragment = interpreter_fragment
+        self.is_python = is_python
 
     b_ENCODING_STRING = b'# -*- coding: utf-8 -*-'
 
@@ -473,21 +474,34 @@ class ScriptRunner(ProgramRunner):
             super(ScriptRunner, self)._get_program()
         )
 
+    def _get_argv(self):
+        return [
+            self.args['_ansible_shell_executable'],
+            '-c',
+            self._get_shell_fragment(),
+        ]
+
+    def _get_shell_fragment(self):
+        """
+        Scripts are eligible for having their hashbang line rewritten, and to
+        be executed via /bin/sh using the ansible_*_interpreter value used as a
+        shell fragment prefixing to the invocation.
+        """
+        return "%s %s %s" % (
+            self.interpreter_fragment,
+            shlex_quote(self.program_fp.name),
+            ' '.join(map(shlex_quote, self._get_program_args())),
+        )
+
     def _rewrite_source(self, s):
         """
         Mutate the source according to the per-task parameters.
         """
-        # Couldn't find shebang, so let shell run it, because shell assumes
-        # executables like this are just shell scripts.
-        if not self.interpreter:
-            return s
-
-        shebang = b'#!' + utf8(self.interpreter)
-        if self.interpreter_arg:
-            shebang += b' ' + utf8(self.interpreter_arg)
-
-        new = [shebang]
-        if os.path.basename(self.interpreter).startswith('python'):
+        # While Ansible rewrites the #! using ansible_*_interpreter, it is
+        # never actually used to execute the script, instead it is a shell
+        # fragment consumed by shell/__init__.py::build_module_command().
+        new = [b'#!' + utf8(self.interpreter_fragment)]
+        if self.is_python:
             new.append(self.b_ENCODING_STRING)
 
         _, _, rest = s.partition(b'\n')

docs/changelog.rst

@@ -22,10 +22,9 @@ Mitogen for Ansible
 ~~~~~~~~~~~~~~~~~~~
 
 * `#291 <https://github.com/dw/mitogen/issues/291>`_: compatibility:
-  ``ansible_*_interpreter`` variables are parsed using UNIX hashbang syntax,
-  i.e. with support for a single space-separated argument. This supports a
-  common idiom where ``ansible_python_interpreter`` is set to ``/usr/bin/env
-  python``.
+  ``ansible_*_interpreter`` variables are parsed using a restrictive shell-like
+  syntax, supporting a common idiom where ``ansible_python_interpreter`` is set
+  to ``/usr/bin/env python``.
 
 * `#299 <https://github.com/dw/mitogen/issues/299>`_: fix the ``network_cli``
   connection type when the Mitogen strategy is active. Mitogen does not help