From 50eda2e6ded28992a3ebd698c2539ce7bf762857 Mon Sep 17 00:00:00 2001
From: Andrew Matthews <andrew.matthews@wpengine.com>
Date: Tue, 20 Aug 2024 17:50:14 -0400
Subject: [PATCH] Split FLAGS variable into an array

Rsync flags sometimes include whitespace in the flag's value. For
example, --filter=':- .gitignore' includes whitespace between the filter
rule and the file name. The single quotes surrounding the flag's value
are meant to prevent splitting. Unfortunately, when bash expands the
$FLAGS variable as part of the main rsync command it escapes the single
quotes and then splits on the whitespace between :- and .gitignore. This
was causing rsync to see the filter flag as two separate arguments,
--filter=:- and .gitignore.

Simply double-quoting $FLAGS string doesn't help because it prevents
splitting between flags multiple are provided.

To fix this, we needed to split the FLAGS variable into an array while
respecting single quotes. This pre-split array can be passed to rsync
with double-quotes to prevent further splitting.

Using the --filter example from before, our new strategy will insert
--filter=':- .gitignore' as a single item in the FLAGS array which is then
passed to rsync as '--filter=:- .gitignore'.
---
 .changeset/nervous-bobcats-fry.md |  5 +++++
 README.md                         | 13 +++++++++--
 entrypoint.sh                     | 37 ++++++++++++++++++++++++++-----
 3 files changed, 47 insertions(+), 8 deletions(-)
 create mode 100644 .changeset/nervous-bobcats-fry.md

diff --git a/.changeset/nervous-bobcats-fry.md b/.changeset/nervous-bobcats-fry.md
new file mode 100644
index 0000000..5aac22c
--- /dev/null
+++ b/.changeset/nervous-bobcats-fry.md
@@ -0,0 +1,5 @@
+---
+"@wpengine/site-deploy": patch
+---
+
+Fixes a bug that caused certain flags in the FLAGS option to be incorrectly parsed by rsync
diff --git a/README.md b/README.md
index 96e581f..3b600e1 100644
--- a/README.md
+++ b/README.md
@@ -20,13 +20,22 @@ You can use this image to deploy a site from your local machine.
 2. Change directories into the root of the local site you'd like to deploy.
 3. Create a `.env` file with the following variables, changing their values as needed.
 
+> [!WARNING]  
+> Since `docker run` does not strip double-quotes from variables in the .env file, we don't use them
+> to wrap entire variable values. Instead, we must use single or double-quotes around flag values that
+> contain whitespace to prevent splitting (`--filter=':= .gitignore'`). The `=` sign between the flag
+> and its value is also required (`--filter=':= .gitignore'` rather than `--filter ':= .gitignore'`).
+
 ```sh
-WPE_ENV=yourinstall # The target WP Engine install name.
+# Required. The target WP Engine install name.
+WPE_ENV=yourinstall
+# Optional. Default values shown.
 REMOTE_PATH=
 SRC_PATH=.
-PHP_LINT=TRUE
+PHP_LINT=FALSE
 CACHE_CLEAR=TRUE
 SCRIPT=
+FLAGS=-azvr --inplace --exclude='.*'
 ```
 
 3. Set an environment variable with your private SSH key, replacing the key file name with your own.
diff --git a/entrypoint.sh b/entrypoint.sh
index e1579db..4263f42 100755
--- a/entrypoint.sh
+++ b/entrypoint.sh
@@ -8,12 +8,36 @@ validate() {
   # optional params
   : REMOTE_PATH="${REMOTE_PATH:=""}"
   : SRC_PATH="${SRC_PATH:="."}"
-  : FLAGS="${FLAGS:="-azvr --inplace --exclude=".*""}"
+  : FLAGS="${FLAGS:="-azvr --inplace --exclude='.*'"}"
   : PHP_LINT="${PHP_LINT:="FALSE"}"
   : CACHE_CLEAR="${CACHE_CLEAR:="TRUE"}"
   : SCRIPT="${SCRIPT:=""}"
 }
 
+# Function to parse FLAGS into an array
+# 
+# Bash doesn't respect quotes when splitting the FLAGS string on whitespace
+# which can lead to incorrect splitting on arguments like --filter=':- .gitignore'.
+#
+# xargs does respect quotes, so we use that here to convert the string into a null-delimited
+# sequence of arguments. We then read that sequence into an array by splitting on the null character.
+#
+# https://superuser.com/questions/1529226/get-bash-to-respect-quotes-when-word-splitting-subshell-output
+parse_flags() {
+  local flags="$1"
+  FLAGS_ARRAY=()
+  while IFS= read -r -d '' flag; do FLAGS_ARRAY+=("$flag"); done < <(echo "$flags" | xargs printf '%s\0')
+}
+
+print_deployment_info() {
+  echo "Deploying your code to:"
+  echo -e "\t${WPE_ENV_NAME}"
+  echo -e "with the following ${#FLAGS_ARRAY[@]} rsync argument(s):"
+  for flag in "${FLAGS_ARRAY[@]}"; do
+    echo -e "\t$flag"
+  done
+}
+
 setup_env() {
   if [[ -n ${WPE_ENV} ]]; then
       WPE_ENV_NAME="${WPE_ENV}";
@@ -33,8 +57,8 @@ setup_env() {
     else CICD_VENDOR="wpe_cicd"
   fi
 
-  echo "Deploying your code to:"
-  echo "${WPE_ENV_NAME}"
+  parse_flags "$FLAGS"
+  print_deployment_info
 
   WPE_SSH_HOST="${WPE_ENV_NAME}.ssh.wpengine.net"
   DIR_PATH="${REMOTE_PATH}"
@@ -105,8 +129,9 @@ sync_files() {
   ssh -nNf -v -i "${WPE_SSHG_KEY_PRIVATE_PATH}" -o StrictHostKeyChecking=no -o ControlMaster=yes -o ControlPath="$SSH_PATH/ctl/%C" "$WPE_FULL_HOST"
   echo "!!! MULTIPLEX SSH CONNECTION ESTABLISHED !!!"
 
-  # shellcheck disable=SC2086
-  rsync --rsh="ssh -v -p 22 -i ${WPE_SSHG_KEY_PRIVATE_PATH} -o StrictHostKeyChecking=no -o 'ControlPath=$SSH_PATH/ctl/%C'" ${FLAGS} --exclude-from='/exclude.txt' --chmod=D775,F664 "${SRC_PATH}" "${WPE_DESTINATION}"
+  set -x
+  rsync --rsh="ssh -v -p 22 -i ${WPE_SSHG_KEY_PRIVATE_PATH} -o StrictHostKeyChecking=no -o 'ControlPath=$SSH_PATH/ctl/%C'" "${FLAGS_ARRAY[@]}" --exclude-from='/exclude.txt' --chmod=D775,F664 "${SRC_PATH}" "${WPE_DESTINATION}"
+  set +x
   
   if [[ -n ${SCRIPT} || -n ${CACHE_CLEAR} ]]; then
 
@@ -139,4 +164,4 @@ setup_env
 setup_ssh_dir
 check_lint
 check_cache
-sync_files
\ No newline at end of file
+sync_files