Repository: JonathonReinhart/scuba (python)

Issue:
Scuba can mangle certain script constructs
`shell_quote_cmd()` seems to be making bad decisions:

`.scuba.yml`:
```yaml
image: foo
aliases:
  foo:
    script:
      - cd somewhere && ./run.sh
```

Output `/.scuba/command.sh`:
```sh
cd somewhere '&&' ./run.sh
```

Bash doesn't like the `'&&'` and then ignores the rest of the line.

Pull request description:
Fix script mangling
This prevents scuba from mangling constructs like && in scripts.

Doing so required re-doing all of the script parsing logic, treating scripts as lists of full command-line strings, instead of lists of parsed command-line lists.

Fixes #100

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b9dc84..2a79b39 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 ### Fixed
 - Display nicer error message if no command is given and image doesn't specify a `Cmd` (#104)
+- Don't mangle && in scripts (#100)
 
 ## [2.1.0] - 2017-04-03
 ### Added
diff --git a/example/alias_multiline/.scuba.yml b/example/alias_multiline/.scuba.yml
index 90d7d1b..def161d 100644
--- a/example/alias_multiline/.scuba.yml
+++ b/example/alias_multiline/.scuba.yml
@@ -5,5 +5,4 @@ aliases:
   complex:
     script:
       - echo 'this is a complex script'
-      - date
-      - uname -a
+      - date && uname -a
diff --git a/scuba/__main__.py b/scuba/__main__.py
index 26c7b36..5c057a5 100644
--- a/scuba/__main__.py
+++ b/scuba/__main__.py
@@ -266,7 +266,7 @@ class ScubaDive(object):
             if not default_cmd:
                 raise ScubaError('No command given and no image-specified command')
             verbose_msg('{0} Cmd: "{1}"'.format(context.image, default_cmd))
-            context.script = [default_cmd]
+            context.script = [shell_quote_cmd(default_cmd)]
 
         # Make scubainit the entrypoint, and manually insert an existing
         # entrypoint before each user command
@@ -280,8 +280,9 @@ class ScubaDive(object):
             writeln(f, '# Auto-generated from scuba')
             writeln(f, 'set -e')
             for cmd in context.script:
-                cmd = entrypoint + cmd
-                writeln(f, shell_quote_cmd(cmd))
+                if entrypoint:
+                    cmd = shell_quote_cmd(entrypoint) + ' ' + cmd
+                writeln(f, cmd)
 
         self.context = context
 
diff --git a/scuba/config.py b/scuba/config.py
index f7e02de..d1d33eb 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -1,3 +1,4 @@
+from __future__ import print_function
 import os
 import yaml
 import shlex
@@ -7,6 +8,7 @@ except NameError:
     basestring = str    # Python 3
 
 from .constants import *
+from .utils import *
 
 class ConfigError(Exception):
     pass
@@ -14,13 +16,6 @@ class ConfigError(Exception):
 class ConfigNotFoundError(ConfigError):
     pass
 
-def shlex_split(s):
-    # shlex.split doesn't properly handle unicode input in Python 2.6.
-    # First try to encode it as an ASCII string. which
-    # may raise a UnicodeEncodeError.
-    s = str(s)
-    return shlex.split(s)
-
 # http://stackoverflow.com/a/9577670
 class Loader(yaml.Loader):
     def __init__(self, stream):
@@ -139,7 +134,7 @@ class ScubaAlias(object):
 
     @classmethod
     def from_dict(cls, name, node):
-        script = [shlex_split(cmd) for cmd in _process_script_node(node, name)]
+        script = _process_script_node(node, name)
         image = node.get('image') if isinstance(node, dict) else None
         return cls(name, script, image)
 
@@ -210,7 +205,7 @@ class ScubaConfig(object):
             command     A user command list (e.g. argv)
 
         Returns: A ScubaContext object with the following attributes:
-            script: a list of command lists
+            script: a list of command line strings
             image: the docker image name to use
         '''
         result = ScubaContext()
@@ -221,7 +216,7 @@ class ScubaConfig(object):
             alias = self.aliases.get(command[0])
             if not alias:
                 # Command is not an alias; use it as-is.
-                result.script = [command]
+                result.script = [shell_quote_cmd(command)]
             else:
                 # Using an alias
                 # Does this alias override the image?
@@ -239,7 +234,7 @@ class ScubaConfig(object):
                     # Alias is a single-line script; perform substituion
                     # and add user arguments.
                     command.pop(0)
-                    result.script = [alias.script[0] + command]
+                    result.script = [alias.script[0] + ' ' + shell_quote_cmd(command)]
 
         return result
 
diff --git a/scuba/utils.py b/scuba/utils.py
index 4dac376..299c2a7 100644
--- a/scuba/utils.py
+++ b/scuba/utils.py
@@ -1,5 +1,6 @@
 import errno
 import os
+import shlex
 try:
     from shlex import quote as shell_quote
 except ImportError:
@@ -9,6 +10,13 @@ except ImportError:
 def shell_quote_cmd(cmdlist):
     return ' '.join(map(shell_quote, cmdlist))
 
+def shlex_split(s):
+    # shlex.split doesn't properly handle unicode input in Python 2.6.
+    # First try to encode it as an ASCII string. which
+    # may raise a UnicodeEncodeError.
+    s = str(s)
+    return shlex.split(s)
+
 
 def format_cmdline(args, maxwidth=80):
     '''Format args into a shell-quoted command line.


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 81f4102..8e4d7c9 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -11,6 +11,7 @@ from os.path import join
 from tempfile import mkdtemp
 from shutil import rmtree
 
+from scuba.utils import shlex_split
 import scuba.config
 
 class TestConfig(TestCase):
@@ -115,8 +116,8 @@ class TestConfig(TestCase):
         config = scuba.config.load_config('.scuba.yml')
         assert_equals(config.image, 'busybox')
         assert_equals(len(config.aliases), 2)
-        assert_seq_equal(config.aliases['foo'].script, [['bar']])
-        assert_seq_equal(config.aliases['snap'].script, [['crackle', 'pop']])
+        assert_seq_equal(config.aliases['foo'].script, ['bar'])
+        assert_seq_equal(config.aliases['snap'].script, ['crackle pop'])
 
     def test_load_config__no_spaces_in_aliases(self):
         '''load_config refuses spaces in aliases'''
@@ -204,7 +205,8 @@ class TestConfig(TestCase):
                 image = 'na',
                 )
         result = cfg.process_command(['cmd', 'arg1', 'arg2'])
-        assert_equal(result.script, [['cmd', 'arg1', 'arg2']])
+        assert_equal(len(result.script), 1)
+        assert_equal(shlex_split(result.script[0]), ['cmd', 'arg1', 'arg2'])
 
     def test_process_command_aliases_unused(self):
         '''process_command handles unused aliases'''
@@ -216,7 +218,8 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['cmd', 'arg1', 'arg2'])
-        assert_equal(result.script, [['cmd', 'arg1', 'arg2']])
+        assert_equal(len(result.script), 1)
+        assert_equal(shlex_split(result.script[0]), ['cmd', 'arg1', 'arg2'])
 
     def test_process_command_aliases_used_noargs(self):
         '''process_command handles aliases with no args'''
@@ -228,7 +231,8 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['apple', 'arg1', 'arg2'])
-        assert_equal(result.script, [['banana', 'arg1', 'arg2']])
+        assert_equal(len(result.script), 1)
+        assert_equal(shlex_split(result.script[0]), ['banana', 'arg1', 'arg2'])
 
     def test_process_command_aliases_used_withargs(self):
         '''process_command handles aliases with args'''
@@ -239,8 +243,9 @@ class TestConfig(TestCase):
                     cat = 'dog',
                     ),
                 )
-        result = cfg.process_command(['apple', 'arg1', 'arg2'])
-        assert_equal(result.script, [['banana', 'cherry', 'pie is good', 'arg1', 'arg2']])
+        result = cfg.process_command(['apple', 'arg1', 'arg2 with spaces'])
+        assert_equal(len(result.script), 1)
+        assert_equal(shlex_split(result.script[0]), ['banana', 'cherry', 'pie is good', 'arg1', 'arg2 with spaces'])
 
     def test_process_command_multiline_aliases_used(self):
         '''process_command handles multiline aliases'''
@@ -255,10 +260,9 @@ class TestConfig(TestCase):
                     ),
                 )
         result = cfg.process_command(['apple'])
-        assert_equal(result.script, [
-            ['banana', 'cherry', 'pie is good'],
-            ['so', 'is', 'peach'],
-        ])
+        assert_equal(len(result.script), 2)
+        assert_equal(shlex_split(result.script[0]), ['banana', 'cherry', 'pie is good'])
+        assert_equal(shlex_split(result.script[1]), ['so', 'is', 'peach'])
 
     def test_process_command_multiline_aliases_forbid_user_args(self):
         '''process_command raises ConfigError when args are specified with multiline aliases'''
diff --git a/tests/test_main.py b/tests/test_main.py
index 9a50f2e..f047bf4 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -419,6 +419,22 @@ class TestMain(TestCase):
         out, _ = self.run_scuba(args)
         assert_str_equalish(test_string, out)
 
+    def test_complex_commands_in_alias(self):
+        '''Verify complex commands can be used in alias scripts'''
+        test_string = 'Hello world'
+        os.mkdir('foo')
+        with open('foo/bar.txt', 'w') as f:
+            f.write(test_string)
+        with open('.scuba.yml', 'w') as f:
+            f.write('image: {0}\n'.format(DOCKER_IMAGE))
+            f.write('aliases:\n')
+            f.write('  alias1:\n')
+            f.write('    script:\n')
+            f.write('      - cd foo && cat bar.txt\n')
+
+        out, _ = self.run_scuba(['alias1'])
+        assert_str_equalish(test_string, out)
+
 
     ############################################################################
     # Hooks