From 66e72c79a0e197beea4a1b797aff842ecd938a55 Mon Sep 17 00:00:00 2001
From: Krzesimir Nowak <knowak@microsoft.com>
Date: Thu, 27 Apr 2023 11:44:26 +0200
Subject: [PATCH] ci-automation: Properly escape parameters passed to bash

Forwarding parameters to another bash invocation through a string
interpreted as a bash script is a bit troublesome. It is not enough to
wrap a parameter like 'foo bar' in escaped double quotes (\") to avoid
it being split into two parameters by bash executing the script
string. It mostly works, but there's always a risk of having a path
where this breaks. It's rare

Wrapping into escaped quotes, be them double or single, also won't
work for passing an array of parameters, so it's even easier here to
trigger globbing or bracket expansion or another unwanted splitting of
supposedly one parameter into multiple. Globbing can be temporarily
disabled with 'set -f' or 'set -o noglob', but this still leaves all
the other special bash characters unescaped. So each parameter in the
array should be escaped before they are put into the script string.

The escaping can be done with `printf` and its '%q` formatter, so
let's do so. For single parameters it is as simple as
`foo_escaped=$(printf '%q' "${foo}")`, for arrays a loop needs to be
used.
---
 ci-automation/test.sh | 44 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/ci-automation/test.sh b/ci-automation/test.sh
index 9010183cee..c3b14fe2f3 100644
--- a/ci-automation/test.sh
+++ b/ci-automation/test.sh
@@ -114,6 +114,21 @@ function __prepare_torcx() {
 }
 # --
 
+function __escape_multiple() {
+    local out_array_arg_name="${1}"; shift
+    # rest are args to be escape and appended into the array named
+    # after the first arg
+    local -n out_array_arg_ref="${out_array_arg_name}"
+    local arg arg_escaped
+
+    out_array_arg_ref=()
+    for arg; do
+        printf -v arg_escaped '%q' "${arg}"
+        out_array_arg_ref+=( "${arg_escaped}" )
+    done
+}
+# --
+
 function test_run() {
     # Run a subshell, so the traps, environment changes and global
     # variables are not spilled into the caller.
@@ -173,11 +188,27 @@ function _test_run_impl() {
     # A job on each worker prunes old mantle images (docker image prune)
     echo "docker rm -f '${container_name}'" >> ./ci-cleanup.sh
 
+    local image_escaped
+    printf -v image_escaped '%q' "${image}"
+    local common_test_args=(
+        "${work_dir}"
+        "${tests_dir}"
+        "${arch}"
+        "${vernum}"
+    )
+    local common_test_args_escaped=()
+    __escape_multiple common_test_args_escaped "${common_test_args[@]}"
+
+    local tests_escaped=()
+    __escape_multiple tests_escaped "${@}"
+
     # Vendor tests may need to know if it is a first run or a rerun
     touch "${work_dir}/first_run"
     for retry in $(seq "${retries}"); do
         local tapfile="results-run-${retry}.tap"
         local failfile="failed-run-${retry}.txt"
+        local tapfile_escaped
+        printf -v tapfile_escaped '%q' "${tapfile}"
 
         # Ignore retcode since tests are flaky. We'll re-run failed tests and
         #  determine success based on test results (tapfile).
@@ -185,13 +216,9 @@ function _test_run_impl() {
         touch sdk_container/.env
         docker run --pull always --rm --name="${container_name}" --privileged --net host -v /dev:/dev \
           -w /work -v "$PWD":/work "${mantle_ref}" \
-         bash -c "set -o noglob && git config --global --add safe.directory /work && source sdk_container/.env && ci-automation/vendor-testing/${image}.sh \
-                \"${work_dir}\" \
-                \"${tests_dir}\" \
-                \"${arch}\" \
-                \"${vernum}\" \
-                \"${tapfile}\" \
-                $*"
+         bash -c "git config --global --add safe.directory /work && \
+                  source sdk_container/.env && \
+                  ci-automation/vendor-testing/${image_escaped}.sh ${common_test_args_escaped[*]} ${tapfile_escaped} ${tests_escaped[*]}"
         set -e
         rm -f "${work_dir}/first_run"
 
@@ -226,10 +253,9 @@ function _test_run_impl() {
         echo "Failed tests:"
         printf '%s\n' "${failed_tests[@]}"
         echo "-----------"
-        set -- "${failed_tests[@]}"
+        __escape_multiple tests_escaped "${failed_tests[@]}"
     done
 
-
     if ${print_give_up}; then
         echo "########### All re-runs exhausted ($retries). Giving up. ###########"
     fi