Proper script argument escaping

I chose to make Script.parse to always operate in POSIX mode so it is much
easier to write commands compatible on all platforms.

Script.cmdify is the important part on Windows. It ensures the argument
line is always escaped and joined properly on Windows, not just for the
subset that works both under POSIX and Windows (as is the case of
shlex.escape).

pipenv/cmdparse.py

@@ -0,0 +1,60 @@
+import re
+import shlex
+
+import six
+
+
+class Script(object):
+    """Parse a script line (in Pipfile's [scripts] section).
+
+    This always works in POSIX mode, even on Windows.
+    """
+    def __init__(self, parts):
+        if not parts:
+            raise ValueError('invalid script')
+        self._parts = parts
+
+
+    @classmethod
+    def parse(cls, value):
+        if isinstance(value, six.text_type):
+            value = shlex.split(value)
+        return cls(value)
+
+    def __repr__(self):
+        return 'Script({0!r})'.format(self._parts)
+
+    @property
+    def command(self):
+        return self._parts[0]
+
+    @property
+    def args(self):
+        return self._parts[1:]
+
+    def extend(self, extra_args):
+        self._parts.extend(extra_args)
+
+    def cmdify(self):
+        """Encode into a cmd-executable string.
+
+        This re-implements CreateProcess's quoting logic to turn a list of
+        arguments into one single string for the shell to interpret.
+
+        * All double quotes are escaped with a backslash.
+        * Existing backslashes before a quote are doubled, so they are all
+          escaped properly.
+        * Backslashes elsewhere are left as-is; cmd will interpret them
+          literally.
+
+        The result is then quoted into a pair of double quotes to be grouped.
+
+        The intended use of this function is to pre-process an argument list
+        before passing it into ``subprocess.Popen(..., shell=True)``.
+
+        See also: https://docs.python.org/3/library/subprocess.html#converting-argument-sequence
+        """
+        return ' '.join(
+            '"{0}"'.format(re.sub(r'(\\*)"', r'\1\1\\"', arg))
+            for arg in self._parts
+        )

pipenv/core.py

@@ -2199,46 +2199,28 @@ def inline_activate_virtualenv():
 def do_run_nt(command, args):
     """Run command by appending space-joined args to it!"""
     import subprocess
-    try:
-        from shlex import quote as shellquote
-    except ImportError:
-        from pipenv.vendor.backports.shlex import quote as shellquote
-    command = project.scripts.get(command, command)
-    command = [command] + [shellquote(a) for a in args]
-
-    # if you've passed something with crazy quoting...
-    # ...just don't. (or put it in a script!)
-    p = subprocess.Popen(command, shell=True, universal_newlines=True)
+    p = subprocess.Popen(
+        project.build_script(command, args).cmdify(),
+        shell=True, universal_newlines=True,
+    )
     p.communicate()
     sys.exit(p.returncode)
 
 
-def _get_command_posix(project, command, args):
-    """Fully bake command into executable and args, based upon project"""
-    # Script was found…
-    if command in project.scripts:
-        command = project.scripts[command]
-    parsed_command = shlex.split(command)
-    executable = parsed_command[0]
-    # prepend arguments
-    args = list(parsed_command[1:]) + list(args)
-    return executable, args
-
-
 def do_run_posix(command, args):
     """Attempt to run command either pulling from project or interpreting as executable.
 
     Args are appended to the command in [scripts] section of project if found.
     """
-    executable, args = _get_command_posix(project, command, args)
-    command_path = system_which(executable)
+    script = project.build_script(command, args)
+    command_path = system_which(script.command)
     if not command_path:
-        if command in project.scripts:
+        if project.has_script(command):
             click.echo(
                 '{0}: the command {1} (from {2}) could not be found within {3}.'
                 ''.format(
                     crayons.red('Error', bold=True),
-                    crayons.red(executable),
+                    crayons.red(script.command),
                     crayons.normal(command, bold=True),
                     crayons.normal('PATH', bold=True),
                 ),
@@ -2256,7 +2238,7 @@ def do_run_posix(command, args):
                 err=True,
             )
         sys.exit(1)
-    os.execl(command_path, command_path, *args)
+    os.execl(command_path, command_path, *script.args)
 
 
 def do_run(command, args, three=None, python=False):

pipenv/project.py

@@ -16,6 +16,7 @@ import pipfile.api
 import toml
 
 from pip9 import ConfigOptionParser
+from .cmdparse import Script
 from .utils import (
     mkdir_p,
     convert_deps_from_pip,
@@ -388,9 +389,22 @@ class Project(object):
         """A dictionary of the settings added to the Pipfile."""
         return self.parsed_pipfile.get('pipenv', {})
 
-    @property
-    def scripts(self):
-        return dict(self.parsed_pipfile.get('scripts', {}))
+    def has_script(self, name):
+        try:
+            return name in self.parsed_pipfile['scripts']
+        except KeyError:
+            return False
+
+    def build_script(self, name, extra_args=None):
+        try:
+            script = Script.parse(self.parsed_pipfile['scripts'][name])
+        except KeyError:
+            script = Script([name])
+        except ValueError:
+            raise ValueError('invalid script entry {0!r}'.format(name))
+        if extra_args:
+            script.extend(extra_args)
+        return script
 
     def update_settings(self, d):
         settings = self.settings

tests/test_cmdparse.py

@@ -0,0 +1,27 @@
+import textwrap
+
+from pipenv.cmdparse import Script
+
+
+def test_parse():
+    script = Script.parse(['python', '-c', "print('hello')"])
+    assert script.command == 'python'
+    assert script.args == ['-c', "print('hello')"], script
+
+
+def test_cmdify():
+    script = Script.parse(['python', '-c', "print('hello')"])
+    cmd = script.cmdify(['--verbose'])
+    assert cmd == '"python" "-c" "print(\'hello\')" "--verbose"', script
+
+
+def test_cmdify_complex():
+    script = Script.parse(' '.join([
+        '"C:\\Program Files\\Python36\\python.exe" -c',
+        """ "print(\'Double quote: \\\"\')" """.strip(),
+    ]))
+    assert script.cmdify([]) == ' '.join([
+        '"C:\\Program Files\\Python36\\python.exe"',
+        '"-c"',
+        """ "print(\'Double quote: \\\"\')" """.strip(),
+    ]), script

tests/test_pipenv.py

@@ -5,7 +5,7 @@ import shutil
 import json
 import pytest
 import warnings
-from pipenv.core import activate_virtualenv, _get_command_posix
+from pipenv.core import activate_virtualenv
 from pipenv.utils import (
     temp_environ, get_windows_path, mkdir_p, normalize_drive, TemporaryDirectory
 )
@@ -794,9 +794,8 @@ requests = {version = "*"}
                 # If we can do this we can theoretically make a subshell
                 # This test doesn't work on *nix
                 if os.name == 'nt':
-                    args = ['pewtwo', 'in', '.venv', 'pip', 'freeze']
                     process = subprocess.Popen(
-                        args,
+                        'pewtwo in .venv pip freeze',
                         shell=True,
                         universal_newlines=True,
                         stdin=subprocess.PIPE,
@@ -1261,12 +1260,14 @@ multicommand = "bash -c \"cd docs && make html\""
                 assert c.out == ''
                 assert 'Error' in c.err
                 assert 'randomthingtotally (from notfoundscript)' in c.err
-            executable, argv = _get_command_posix(Project(), 'multicommand', [])
-            assert executable == 'bash'
-            assert argv == ['-c', 'cd docs && make html']
-            executable, argv = _get_command_posix(Project(), 'appendscript', ['a', 'b'])
-            assert executable == 'cmd'
-            assert argv == ['arg1', 'a', 'b']
+
+            project = Project()
+            script = project.build_script('multicommand')
+            assert script.command == 'bash'
+            assert script.args == ['-c', 'cd docs && make html']
+            script = project.build_script('appendscript', ['a', 'b'])
+            assert script.command == 'cmd'
+            assert script.args == ['arg1', 'a', 'b']
 
     @pytest.mark.lock
     @pytest.mark.complex