From efa40aa3eec57c0f6fda3656e3115725d6a9da0b Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Fri, 17 Aug 2018 14:05:20 +1000
Subject: [PATCH 1/2] Replace eval with xargs for parsing BUILDKITE_COMMAND

---
 commands/run.sh | 29 ++++++++++++++++-------------
 tests/run.bats  |  3 ++-
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/commands/run.sh b/commands/run.sh
index b9fac1c3..86333913 100755
--- a/commands/run.sh
+++ b/commands/run.sh
@@ -95,12 +95,12 @@ while IFS=$'\n' read -r vol ; do
   [[ -n "${vol:-}" ]] && run_params+=("-v" "$(expand_relative_volume_path "$vol")")
 done <<< "$(plugin_read_list VOLUMES)"
 
+# Parse BUILDKITE_DOCKER_DEFAULT_VOLUMES delimited by semi-colons, normalized to
+# ignore spaces and leading or trailing semi-colons
 IFS=';' read -r -a default_volumes <<< "${BUILDKITE_DOCKER_DEFAULT_VOLUMES:-}"
-for vol in "${default_volumes[@]:-}"
-do
-  # Trim all whitespace when checking for variable definition, handling cases
-  # with repeated delimiters.
-  [[ ! -z "${vol// }" ]] && run_params+=("-v" "$(expand_relative_volume_path "$vol")")
+for vol in "${default_volumes[@]:-}" ; do
+  trimmed_vol="$(echo -n "$vol" | sed -e 's/^[[:space:]]*//' | sed -e 's/[[:space:]]*$//')"
+  [[ -n "$trimmed_vol" ]] && run_params+=("-v" "$(expand_relative_volume_path "$trimmed_vol")")
 done
 
 # Optionally disable allocating a TTY
@@ -135,18 +135,21 @@ fi
 set +e
 
 (
-  # Reset bash to the default IFS with no glob expanding and no failing on error
+  # Reset bash to the default IFS
   unset IFS
-  set -f
 
-  # The eval statements below are used to allow $BUILDKITE_COMMAND to be interpolated correctly
-  # When paired with -f we ensure that it word splits correctly, e.g bash -c "pwd" should split
-  # into [bash, -c, "pwd"]. Eval ends up the simplest way to do this, and when paired with the
-  # set -f above we ensure globs aren't expanded (imagine a command like `cat *`, which bash would
-  # helpfully expand prior to passing it to docker-compose)
+  # Because we receive $BUILDKITE_COMMAND as a single string, it needs to be tokenized as a shell
+  # would to respect things like quoted strings. Previously we used eval to do this with some
+  # careful control around preventing glob expansion, but this ends up being difficult to get just
+  # right and also very risky. The new implementation uses xargs and printf to tokenize the string,
+  # which is portable and much harder to shoot ourselves in the foot 🎉
+
+  while IFS= read -rd '' token; do
+    run_params+=("$token")
+  done < <(xargs printf '%s\0' <<< "$BUILDKITE_COMMAND")
 
   echo "+++ :docker: Running command in Docker Compose service: $run_service" >&2
-  eval "run_docker_compose \${run_params[@]} $BUILDKITE_COMMAND"
+  run_docker_compose "${run_params[@]}"
 )
 
 exitcode=$?
diff --git a/tests/run.bats b/tests/run.bats
index f1a1f011..c61b5706 100644
--- a/tests/run.bats
+++ b/tests/run.bats
@@ -121,10 +121,11 @@ load '../lib/run'
   export BUILDKITE_PLUGIN_DOCKER_COMPOSE_ENV_1=MYENV
   export BUILDKITE_PLUGIN_DOCKER_COMPOSE_ENVIRONMENT_0=MYENV=2
   export BUILDKITE_PLUGIN_DOCKER_COMPOSE_ENVIRONMENT_1=MYENV
+  export BUILDKITE_PLUGIN_DOCKER_COMPOSE_ENVIRONMENT_2=ANOTHER="this is a long string with spaces; and semi-colons"
 
   stub docker-compose \
     "-f docker-compose.yml -p buildkite1111 build --pull myservice : echo built myservice" \
-    "-f docker-compose.yml -p buildkite1111 run --name buildkite1111_myservice_build_1 -e MYENV=0 -e MYENV -e MYENV=2 -e MYENV myservice pwd : echo ran myservice"
+    "-f docker-compose.yml -p buildkite1111 run --name buildkite1111_myservice_build_1 -e MYENV=0 -e MYENV -e MYENV=2 -e MYENV -e ANOTHER=this\ is\ a\ long\ string\ with\ spaces\;\ and\ semi-colons myservice pwd : echo ran myservice"
 
   stub buildkite-agent \
     "meta-data get docker-compose-plugin-built-image-tag-myservice : exit 1"

From 5cd983a85220a4202f9f63c616c6667c927ccda2 Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Fri, 17 Aug 2018 14:19:01 +1000
Subject: [PATCH 2/2] Handle empty BUILDKITE_COMMAND

---
 commands/run.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/run.sh b/commands/run.sh
index 86333913..0c7313af 100755
--- a/commands/run.sh
+++ b/commands/run.sh
@@ -145,7 +145,7 @@ set +e
   # which is portable and much harder to shoot ourselves in the foot 🎉
 
   while IFS= read -rd '' token; do
-    run_params+=("$token")
+    [[ -n "$token" ]] && run_params+=("$token")
   done < <(xargs printf '%s\0' <<< "$BUILDKITE_COMMAND")
 
   echo "+++ :docker: Running command in Docker Compose service: $run_service" >&2