(maint) Quote variables in shell scripts

This allows at least minimal support for whitespace and other special
characters such as hyphen in paths, and avoids many bugs where string
contents are treated as code:

- `test -z`, `test -n`, etc. get more than one argument:

        $ a='1 2'
        $ [ -z $a ]
        bash: [: foo: binary operator expected
- The error is different if more words are involved:

        $ a='1 2 3'
        $ [ -z $a ]
        bash: [: too many arguments
- Wrong return codes can be more insidious:

        $ a='-o 1'
        $ [ -z $a ]; echo $?
        0
        $ a='"" -a -z x'
        $ [ -n $a ]; echo $?
        1
        $ a=' '
        $ [ -z $a ] && [ -n $a ] && echo both
        both

acceptance/bin/ci-bootstrap-from-artifacts.sh

@@ -13,7 +13,7 @@
 set -x
 
 # If $GEM_SOURCE is not set, fall back to rubygems.org
-if [ -z $GEM_SOURCE ]; then
+if [ -z "$GEM_SOURCE" ]; then
   export GEM_SOURCE='https://rubygems.org'
 fi
 
@@ -37,7 +37,7 @@ if [[ "${platform}" =~ 'solaris' ]]; then
 fi
 
 # If the platform is Windows and $ruby_arch is set, append it
-if [[ "${platform}" =~ 'win' && ! -z $ruby_arch ]]; then
+if [[ "${platform}" =~ 'win' && ! -z "$ruby_arch" ]]; then
     platform="${platform}-${ruby_arch}"
 fi
 

acceptance/bin/ci-pe-puppet.sh

@@ -33,7 +33,7 @@ bundle install --path=./.bundle/gems
 
 #export pe_version=${pe_version_override:-$pe_version}
 #export pe_family=3.4
-if ! bundle exec genconfig ${platform}-${layout} > hosts.cfg; then
+if ! bundle exec genconfig "${platform}-${layout}" > hosts.cfg; then
   echo "Usage: ensure Gemfile.local exists requiring sqa-utils"
 fi
 
@@ -47,14 +47,14 @@ bundle exec beaker           \
   --debug                    \
   --repo-proxy               \
   --config hosts.cfg         \
-  --pre-suite ${PRE_SUITE}  \
-  --tests=${tests}           \
-  --keyfile ${HOME}/.ssh/id_rsa-acceptance \
+  --pre-suite "${PRE_SUITE}" \
+  --tests="${tests}"         \
+  --keyfile "${HOME}/.ssh/id_rsa-acceptance" \
   --root-keys \
   --helper lib/helper.rb \
   --preserve-hosts always \
   --no-color
 
 RESULT=$?
 
-exit $RESULT
+exit "$RESULT"

acceptance/bin/ci-rerun-pe-puppet.sh

@@ -23,7 +23,7 @@ echo "Must provide the hostname: got '$1'"
 exit 1
 fi
 
-domain=${domain:-delivery.puppetlabs.net}
+domain="${domain:-delivery.puppetlabs.net}"
 
 if [ -z "$2" ]; then
 
@@ -113,8 +113,8 @@ bundle exec beaker           \
   --repo-proxy               \
   --config hosts-immediate.cfg         \
   --pre-suite setup/common/pre-suite/110_SetPEPuppetService.rb \
-  --tests=${tests}           \
-  --keyfile ${HOME}/.ssh/id_rsa-acceptance \
+  --tests="${tests}"         \
+  --keyfile "${HOME}/.ssh/id_rsa-acceptance" \
   --root-keys \
   --helper lib/helper.rb \
   --preserve-hosts onfail \
@@ -123,4 +123,4 @@ bundle exec beaker           \
 
 RESULT=$?
 
-exit $RESULT
+exit "$RESULT"

acceptance/tests/external_ca_support/fixtures/certchain.sh

@@ -22,9 +22,9 @@ dedent() {
 # invoke openssl
 openssl() {
     echo "----"
-    echo "running" ${OPENSSL} ${@}
+    echo "running" "${OPENSSL}" "$@"
     echo "  in $PWD"
-    ${OPENSSL} "${@}"
+    "${OPENSSL}" "$@"
 }
 
 show_cert() {
@@ -36,7 +36,7 @@ show_cert() {
 hash_cert() {
     local cert="$1"
     local certdir="${B}/certdir"
-    local h=$(${OPENSSL} x509 -hash -noout -in ${cert})
+    local h=$("${OPENSSL}" x509 -hash -noout -in "${cert}")
     mkdir -p "${certdir}"
     ln -s "$cert" "${certdir}/${h}.0"
 }
@@ -49,7 +49,7 @@ show_crl() {
 hash_crl() {
     local crl="$1"
     local certdir="${B}/certdir"
-    local h=$(${OPENSSL} crl -hash -noout -in ${crl})
+    local h=$("${OPENSSL}" crl -hash -noout -in "${crl}")
     mkdir -p "${certdir}"
     ln -s "$crl" "${certdir}/${h}.r0"
 }
@@ -261,7 +261,7 @@ revoke_ca_cert() {
     openssl ca -config "${dir}/openssl.conf" -revoke "${B}/${master}/ca-${master}.crt"
     openssl ca -config "${dir}/openssl.conf" -gencrl -out "${dir}/ca-root.crl"
     show_crl "${dir}/ca-root.crl"
-    kill -HUP $(< "${B}/apache/httpd.pid")
+    kill -HUP "$(< "${B}/apache/httpd.pid")"
 }
 
 # create a "leaf" certificate for the given fqdn, signed by the given ca name.
@@ -281,7 +281,7 @@ create_leaf_cert() {
         openssl req -subj "/CN=${fqdn}" -new -key "${fname}.key" -out "${fname}.csr"
         CN="${fqdn}" SAN="DNS:${fqdn}, DNS:${fqdn%%.*}, DNS:puppet, DNS:puppetmaster" \
           openssl ca -config "${B}/${ca}/openssl.conf" -in "${fname}.csr" -notext \
-          -out "${fname}.crt" -batch $exts
+          -out "${fname}.crt" -batch "$exts"
     )
     show_cert "${dir}/${fname}.crt"
 }
@@ -318,7 +318,7 @@ create_leaf_email_cert() {
         openssl req -subj "/CN=${fqdn}/emailAddress=test@example.com" -new -key "${fname}.key" -out "${fname}.csr"
 
         openssl ca -config "${B}/${ca}/openssl.conf" -name master_ca_email_config \
-          -in "${fname}.csr" -notext -out "${fname}.crt" -batch $exts_arg
+          -in "${fname}.csr" -notext -out "${fname}.crt" -batch "$exts_arg"
     )
     show_cert "${dir}/${fname}.crt"
 }
@@ -496,14 +496,14 @@ run_agent() {
     if puppet agent --test --debug \
             --confdir=/tmp/certchain/agent/conf/ --vardir=/tmp/certchain/agent/var/ \
             --fqdn "${fqdn}"; then
-        if ${expfail}; then
+        if "${expfail}"; then
             false
         fi
         # This appears not to work in 3.1.x
         #test -f "${B}/i_was_here"
     else
         echo "expected failure"
-        if ! ${expfail}; then
+        if ! "${expfail}"; then
             false
         fi
         # This appears not to work in 3.1.x
@@ -512,8 +512,8 @@ run_agent() {
 }
 
 call() {
-    echo "==== $1 ===="
-    "${@}"
+    echo "==== ${1} ===="
+    "$@"
 }
 
 main() {

acceptance/tests/resource/service/ticket_4123_should_list_all_running_redhat.sh

@@ -7,20 +7,20 @@
 RALSH_FILE=/tmp/ralsh-running-list-$$
 SERVICE_FILE=/tmp/service-running-list-$$
 
-puppet resource service | egrep -B1 "ensure.*=>.*'running" | grep 'service {' | gawk -F"'" '{print $2}' | sort  > $RALSH_FILE
+puppet resource service | egrep -B1 "ensure.*=>.*'running" | grep 'service {' | gawk -F"'" '{print $2}' | sort  > "$RALSH_FILE"
 
-if [ -e $SERVICE_FILE ]; then
-  rm $SERVICE_FILE
+if [ -e "$SERVICE_FILE" ]; then
+  rm "$SERVICE_FILE"
 fi
 
 SERVICEDIR='/etc/init.d'
-for SERVICE in $( ls $SERVICEDIR | sort | egrep -v "(functions|halt|killall|single|linuxconf)" ) ; do
+for SERVICE in $( ls "$SERVICEDIR" | sort | egrep -v "(functions|halt|killall|single|linuxconf)" ) ; do
   if env -i LANG="$LANG" PATH="$PATH" TERM="$TERM" "${SERVICEDIR}/${SERVICE}" status; then
-    echo $SERVICE >> $SERVICE_FILE
+    echo "$SERVICE" >> "$SERVICE_FILE"
   fi
 done
 
-if diff $RALSH_FILE $SERVICE_FILE ; then
+if diff "$RALSH_FILE" "$SERVICE_FILE" ; then
   echo "Ralsh and system service count agree"
   exit 0
 else

acceptance/tests/resource/service/ticket_4124_should_list_all_disabled.sh

@@ -7,20 +7,20 @@
 RALSH_FILE=/tmp/ralsh-disabled-list-$$
 SERVICE_FILE=/tmp/service-disabled-list-$$
 
-puppet resource service | egrep -B2 "enable.*=>.*'false" | grep "service {" | awk -F"'" '{print $2}' | sort  > $RALSH_FILE
+puppet resource service | egrep -B2 "enable.*=>.*'false" | grep "service {" | awk -F"'" '{print $2}' | sort  > "$RALSH_FILE"
 
-if [ -e $SERVICE_FILE ]; then
-  rm $SERVICE_FILE
+if [ -e "$SERVICE_FILE" ]; then
+  rm "$SERVICE_FILE"
 fi
 
 SERVICEDIR='/etc/init.d'
-for SERVICE in $( ls $SERVICEDIR | sort | egrep -v "(functions|halt|killall|single|linuxconf)" ) ; do
-  if ! chkconfig $SERVICE; then
-    echo $SERVICE >> $SERVICE_FILE
+for SERVICE in $( ls "$SERVICEDIR" | sort | egrep -v "(functions|halt|killall|single|linuxconf)" ) ; do
+  if ! chkconfig "$SERVICE"; then
+    echo "$SERVICE" >> "$SERVICE_FILE"
   fi
 done
 
-if diff $RALSH_FILE $SERVICE_FILE ; then
+if diff "$RALSH_FILE" "$SERVICE_FILE" ; then
   echo "Ralsh and system service count agree"
   exit 0
 else

ext/envpuppet

@@ -48,7 +48,7 @@ will taint the development version
 
 The following enviornment variables configure the behavior of envpuppet
 
-    ENVPUPPET_BASEDIR=${HOME}/src 
+    ENVPUPPET_BASEDIR="${HOME}/src"
       the base directory where puppet, facter, etc...  live.
 
     ENVPUPPET_BLEEDING=true   Enables bleeding edge prototypes like

ext/gentoo/init.d/puppetmaster

@@ -36,16 +36,16 @@ start() {
     ebegin "Starting puppetmaster"
     start-stop-daemon --start --quiet --exec /usr/bin/ruby /usr/bin/puppetmasterd \
         -- ${options}
-    eend $? "Failed to start puppetmaster"
+    eend "$?" "Failed to start puppetmaster"
 }
 
 stop() {
     ebegin "Stopping puppetmaster"
     start-stop-daemon --stop --quiet \
         --pidfile ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
     local ret=$?
-    eend ${ret} "Failed to stop puppetmaster"
-    rm -f ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
-    return ${ret}
+    eend "${ret}" "Failed to stop puppetmaster"
+    rm -f "${PUPPETMASTER_PID_DIR}/puppetmasterd.pid"
+    return "${ret}"
 }
 

ext/ips/puppet-agent

@@ -2,16 +2,16 @@
 
 . /lib/svc/share/smf_include.sh
 
-[[ -z "${SMF_FMRI}" ]] && exit $SMF_EXIT_ERR
+[[ -z "${SMF_FMRI}" ]] && exit "$SMF_EXIT_ERR"
 
 typeset -r CONF_FILE=/etc/puppet/puppet.conf
-[[ ! -f "${CONF_FILE}" ]] && exit $SMF_EXIT_ERR_CONFIG
+[[ ! -f "${CONF_FILE}" ]] && exit "$SMF_EXIT_ERR_CONFIG"
 
 typeset -r PUPPET=/usr/bin/puppet
 
 case "$1" in
 start)
-  exec $PUPPET agent
+  exec "$PUPPET" agent
   ;;
 
 stop)
@@ -23,22 +23,22 @@ stop)
   # continues until all processes are killed.
   # svcs -p <fmri> lists all processes in the contract.
   # http://bnsmb.de/solaris/My_Little_SMF_FAQ.html
-  ctid=`svcprop -p restarter/contract $SMF_FMRI`
+  ctid=`svcprop -p restarter/contract "$SMF_FMRI"`
   if [ -n "$ctid" ]; then
-    smf_kill_contract $ctid TERM 1 5
+    smf_kill_contract "$ctid" TERM 1 5
     ret=$?
-    [ $ret -eq 1 ] && exit $SMF_EXIT_ERR_FATAL
+    [ "$ret" -eq 1 ] && exit "$SMF_EXIT_ERR_FATAL"
 
-    if [ $ret -eq 2 ] ; then
-      smf_kill_contract $ctid KILL 1
-      [ $? -ne 0 ] && exit $SMF_EXIT_ERR_FATAL
+    if [ "$ret" -eq 2 ] ; then
+      smf_kill_contract "$ctid" KILL 1
+      [ $? -ne 0 ] && exit "$SMF_EXIT_ERR_FATAL"
     fi
   fi
   ;;
 *)
   echo "Usage: $0 {start|stop}";
-  exit $SMF_EXIT_ERR_FATAL
+  exit "$SMF_EXIT_ERR_FATAL"
   ;;
 esac
-exit $SMF_EXIT_OK
+exit "$SMF_EXIT_OK"
 

ext/ips/puppet-master

@@ -2,15 +2,15 @@
 
 . /lib/svc/share/smf_include.sh
 
-[[ -z "${SMF_FMRI}" ]] && exit $SMF_EXIT_ERR
+[[ -z "${SMF_FMRI}" ]] && exit "$SMF_EXIT_ERR"
 
 typeset -r CONF_FILE=/etc/puppet/puppet.conf
-[[ ! -f ${CONF_FILE} ]] && exit $SMF_EXIT_ERR_CONFIG
+[[ ! -f "${CONF_FILE}" ]] && exit "$SMF_EXIT_ERR_CONFIG"
 typeset -r PUPPET=/usr/bin/puppet
 
 case "$1" in
 start)
-  exec $PUPPET master --daemonize
+  exec "$PUPPET" master --daemonize
   ;;
 
 stop)
@@ -23,22 +23,22 @@ stop)
   # svcs -p <fmri> lists all processes in the contract.
   # http://bnsmb.de/solaris/My_Little_SMF_FAQ.html
 
-  ctid=`svcprop -p restarter/contract $SMF_FMRI`
+  ctid=`svcprop -p restarter/contract "$SMF_FMRI"`
   if [ -n "$ctid" ]; then
-    smf_kill_contract $ctid TERM 1 5
+    smf_kill_contract "$ctid" TERM 1 5
     ret=$?
-    [ $ret -eq 1 ] && exit $SMF_EXIT_ERR_FATAL
+    [ "$ret" -eq 1 ] && exit "$SMF_EXIT_ERR_FATAL"
 
-    if [ $ret -eq 2 ] ; then
-      smf_kill_contract $ctid KILL 1
-      [ $? -ne 0 ] && exit $SMF_EXIT_ERR_FATAL
+    if [ "$ret" -eq 2 ] ; then
+      smf_kill_contract "$ctid" KILL 1
+      [ $? -ne 0 ] && exit "$SMF_EXIT_ERR_FATAL"
     fi
   fi
   ;;
 *)
   echo "Usage: $0 {start|stop}";
-  exit $SMF_EXIT_ERR_FATAL
+  exit "$SMF_EXIT_ERR_FATAL"
   ;;
 esac
-exit $SMF_EXIT_OK
+exit "$SMF_EXIT_OK"
 

ext/redhat/client.init

@@ -16,9 +16,9 @@
 [ -f /etc/sysconfig/puppet ] && . /etc/sysconfig/puppet
 lockfile=/var/lock/subsys/puppet
 piddir=/var/run/puppetlabs
-pidfile=${piddir}/agent.pid
+pidfile="${piddir}/agent.pid"
 puppetd=/opt/puppetlabs/puppet/bin/puppet
-pid=$(cat $pidfile 2> /dev/null)
+pid=$(cat "$pidfile" 2> /dev/null)
 RETVAL=0
 
 PUPPET_OPTS="agent "