Improve shell quoting hygiene

Most of these problems were found by ShellCheck (http://www.shellcheck.net). Signed-off-by: Anders Kaseorg <[email protected]>
himao · Sep 26, 2015 · 0d12dfd · 0d12dfd
1 parent 6888826
commit 0d12dfd
Show file tree

Hide file tree

Showing 23 changed files with 81 additions and 81 deletions.
diff --git a/bin/write-rabbitmq-consumers-state-file b/bin/write-rabbitmq-consumers-state-file
@@ -11,5 +11,5 @@ ZULIP_DIR=/home/zulip/deployments/current
 STATE_DIR=/var/lib/nagios_state
 STATE_FILE=$STATE_DIR/check-rabbitmq-consumers-$queue
 
-$ZULIP_DIR/bots/check-rabbitmq-consumers --queue=$queue &> ${STATE_FILE}-tmp;
-mv ${STATE_FILE}-tmp $STATE_FILE
+"$ZULIP_DIR/bots/check-rabbitmq-consumers" "--queue=$queue" &> "${STATE_FILE}-tmp";
+mv "${STATE_FILE}-tmp" "$STATE_FILE"
diff --git a/puppet/zulip_internal/files/munin-plugins/humbug_send_receive b/puppet/zulip_internal/files/munin-plugins/humbug_send_receive
@@ -4,4 +4,4 @@ if [ "$(hostname)" = "staging.zulip.net" ]; then
 else
     site="https://api.zulip.com"
 fi
-/home/zulip/deployments/current/bots/check_send_receive.py --munin $1 --site="$site"
+/home/zulip/deployments/current/bots/check_send_receive.py --munin "$1" --site="$site"
diff --git a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_backlog
@@ -9,11 +9,11 @@ cd /home/zulip/deployments/current
 
 BACKLOG=$(./manage.py print_email_delivery_backlog)
 
-if [ $BACKLOG -gt 0 -a $BACKLOG -lt 10 ]
+if [ "$BACKLOG" -gt 0 ] && [ "$BACKLOG" -lt 10 ]
 then
     echo "backlog of $BACKLOG"
     exit 1
-elif [ $BACKLOG -ge 10 ]
+elif [ "$BACKLOG" -ge 10 ]
 then
     echo "backlog of $BACKLOG"
     exit 2

diff --git a/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process b/puppet/zulip_internal/files/nagios_plugins/check_email_deliverer_process
@@ -13,14 +13,14 @@ if [ "$STATUS" == "RUNNING" ]
 then
     echo "Running"
     exit 0
-elif [ $(echo "$STATUS" | egrep '(STOPPED)|(STARTING)|(BACKOFF)|(STOPPING)|(EXITED)|(FATAL)|(UNKNOWN)$') ]
+elif [ "$(echo "$STATUS" | egrep '(STOPPED)|(STARTING)|(BACKOFF)|(STOPPING)|(EXITED)|(FATAL)|(UNKNOWN)$')" ]
 then
     # not "RUNNING", but a recognized supervisor status
-    echo $STATUS
+    echo "$STATUS"
     exit 1
 else
     # we don't recognize the second column in this SUPERVISOR_STATUS.
     # This may be indicative of a supervisor configuration problem
-    echo $SUPERVISOR_STATUS
+    echo "$SUPERVISOR_STATUS"
     exit 3
 fi
diff --git a/puppet/zulip_internal/files/nagios_plugins/check_worker_memory b/puppet/zulip_internal/files/nagios_plugins/check_worker_memory
@@ -1,7 +1,7 @@
 #!/bin/bash
 # Checks for any Zulip queue workers that are leaking memory and thus have a high vsize
 datafile=$(mktemp)
-ps -o vsize,size,pid,user,command --sort -vsize $(pgrep -f '^python /home/zulip/deployments/current/manage.py process_queue') > "$datafile"
+ps -o vsize,size,pid,user,command --sort -vsize "$(pgrep -f '^python /home/zulip/deployments/current/manage.py process_queue')" > "$datafile"
 cat "$datafile"
 top_worker=$(cat "$datafile" | head -n2 | tail -n1)
 top_worker_memory_usage=$(echo "$top_worker" | cut -f1 -d" ")

diff --git a/puppet/zulip_internal/files/postgresql/env-wal-e b/puppet/zulip_internal/files/postgresql/env-wal-e
@@ -5,9 +5,9 @@ fi
 
 export AWS_ACCESS_KEY_ID=xxxxxxxxxxxxxxxxxxxx
 export AWS_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
-s3_backup_bucket=$(crudini --get $ZULIP_CONF database s3_backup_bucket 2>&1)
+s3_backup_bucket=$(crudini --get "$ZULIP_CONF" database s3_backup_bucket 2>&1)
 if [ $? -ne 0 ]; then
-    echo "Could not determine which s3 bucket to use:" $s3_backup_bucket
+    echo "Could not determine which s3 bucket to use:" "$s3_backup_bucket"
     exit 1
 fi
 export WALE_S3_PREFIX=s3://$s3_backup_bucket

diff --git a/scripts/setup/configure-rabbitmq b/scripts/setup/configure-rabbitmq
@@ -3,9 +3,9 @@
 # Delete the "guest" default user and replace it with a Zulip user
 # with a real password
 
-RMQPW=$($(dirname $0)/../../bin/get-django-setting RABBITMQ_PASSWORD)
+RMQPW=$("$(dirname "$0")/../../bin/get-django-setting" RABBITMQ_PASSWORD)
 sudo rabbitmqctl delete_user zulip || true
 sudo rabbitmqctl delete_user guest || true
-sudo rabbitmqctl add_user zulip $RMQPW
+sudo rabbitmqctl add_user zulip "$RMQPW"
 sudo rabbitmqctl set_user_tags zulip administrator
 sudo rabbitmqctl set_permissions -p / zulip '.*' '.*' '.*'
diff --git a/scripts/setup/initialize-database b/scripts/setup/initialize-database
@@ -1,7 +1,7 @@
 #!/bin/bash -xe
 
 # Change to root directory of the checkout that we're running from
-cd $(dirname $0)/../..
+cd "$(dirname "$0")/../.."
 
 python manage.py checkconfig
 

diff --git a/scripts/setup/install b/scripts/setup/install
@@ -1,3 +1,3 @@
 #!/bin/bash
 mkdir -p /var/log/zulip
-$(dirname $(dirname $0))/lib/install "$@" 2>&1 | tee -a /var/log/zulip/install.log
+"$(dirname "$(dirname "$0")")/lib/install" "$@" 2>&1 | tee -a /var/log/zulip/install.log
diff --git a/scripts/upgrade-zulip b/scripts/upgrade-zulip
@@ -1,2 +1,2 @@
 #!/bin/bash
-$(dirname $0)/lib/upgrade-zulip "$@" 2>&1 | tee -a /var/log/zulip/upgrade.log
+"$(dirname "$0")/lib/upgrade-zulip" "$@" 2>&1 | tee -a /var/log/zulip/upgrade.log
diff --git a/tools/build-deb b/tools/build-deb
@@ -12,7 +12,7 @@ dist=$1
 file=$2
 
 if [ -z "$dist" ] || [ -z "$file" ]; then
-    echo "$(echo $0 | rev | cut -d "/" -f  1-1 | rev) -- build Debian packages on a Zulip buildslave"
+    echo "$(echo "$0" | rev | cut -d "/" -f  1-1 | rev) -- build Debian packages on a Zulip buildslave"
     echo
     echo "USAGE: $0 dist path/to/package.dsc"
     exit 1
@@ -21,10 +21,10 @@ fi
 set -xe
 
 ret=0
-path=$(ssh -q -l $BUILDD_USERNAME $BUILDD_HOST -- mktemp -d $BUILDD_BASE_PATH/$USER.`date -u +%F.%R`.XXXXXXX)/
+path=$(ssh -q -l "$BUILDD_USERNAME" "$BUILDD_HOST" -- "mktemp -d '$BUILDD_BASE_PATH/$USER.`date -u +%F.%R`.XXXXXXX'")/
 
-dcmd rsync -vz --copy-links $file $BUILDD_USERNAME@$BUILDD_HOST:$path/
-file=$(basename $file)
+dcmd rsync -vz --copy-links "$file" "$BUILDD_USERNAME@$BUILDD_HOST:$path/"
+file=$(basename "$file")
 
 # -A specifies to build arch-all packages (non-arch dependent) in addition to
 # binary packages
@@ -38,9 +38,9 @@ file=$(basename $file)
 #
 # We always build for amd64. There is no 32-bit.
 
-ssh -t -l $BUILDD_USERNAME $BUILDD_HOST -- "cd $path && sbuild -A -s --force-orig-source --dist=$dist --arch=amd64 $file"
+ssh -t -l "$BUILDD_USERNAME" "$BUILDD_HOST" -- "cd '$path' && sbuild -A -s --force-orig-source '--dist=$dist' --arch=amd64 '$file'"
 
-rsync -Lvz --copy-links $BUILDD_USERNAME@$BUILDD_HOST:$path/* .
-ssh -l  $BUILDD_USERNAME $BUILDD_HOST rm -r $path
+rsync -Lvz --copy-links "$BUILDD_USERNAME@$BUILDD_HOST:$path/*" .
+ssh -l  "$BUILDD_USERNAME" "$BUILDD_HOST" rm -r "$path"
 
-exit $ret
+exit "$ret"
diff --git a/tools/build-release-tarball b/tools/build-release-tarball
@@ -11,8 +11,8 @@ prefix="zulip-server-$version"
 
 if [ "$(uname)" = "Darwin" ]; then
     TMPDIR=/tmp/voyager-build
-    rm -Rf $TMPDIR
-    mkdir -p $TMPDIR
+    rm -Rf "$TMPDIR"
+    mkdir -p "$TMPDIR"
 else
     TMPDIR=$(mktemp -d)
 fi
@@ -27,10 +27,10 @@ TMP_CHECKOUT=$TMPDIR/$prefix/
 TARBALL=$TMPDIR/$prefix.tar
 
 # .gitattributes lists the files that are not exported
-git archive -o $TARBALL --prefix=$prefix/ HEAD
+git archive -o "$TARBALL" "--prefix=$prefix/" HEAD
 
 
-if tar -tf $TARBALL | grep -q -e zilencer -e zproject/local_settings.py -e puppet/zulip_internal; then
+if tar -tf "$TARBALL" | grep -q -e zilencer -e zproject/local_settings.py -e puppet/zulip_internal; then
 	echo "Excluded files remain in tarball!";
 	echo "Versions of git 1.8.1.1 - 1.8.1.6 have broken .gitattributes syntax";
 	exit 1;
@@ -39,8 +39,8 @@ else
 fi
 
 # Check out a temporary full copy of the index to generate static files
-git checkout-index -f -a --prefix $TMP_CHECKOUT
-cd $TMP_CHECKOUT
+git checkout-index -f -a --prefix "$TMP_CHECKOUT"
+cd "$TMP_CHECKOUT"
 
 # Use default settings so there is no chance of leaking secrets
 cp zproject/local_settings_template.py zproject/local_settings.py
@@ -70,20 +70,20 @@ echo; echo "\033[33mRunning update-prod-static; check ${TMP_CHECKOUT}update-prod
 set -x
 
 ./tools/update-prod-static
-echo $GITID > build_id
-echo $version > version
+echo "$GITID" > build_id
+echo "$version" > version
 mv update-prod-static.log ..
 
 rm -f zproject/dev-secrets.conf
 
 # We don't need duplicate copies of emoji with hashed paths, and they would break bugdown
 find prod-static/serve/third/gemoji/images/emoji/ -regex '.*\.[0-9a-f]+\.png' -delete
 
-cd $TMPDIR
+cd "$TMPDIR"
 
-tar --append -f $TARBALL $prefix/prod-static $prefix/build_id $prefix/version
+tar --append -f "$TARBALL" "$prefix/prod-static" "$prefix/build_id" "$prefix/version"
 
-rm -rf $prefix
+rm -rf "$prefix"
 
-gzip $TARBALL
+gzip "$TARBALL"
 echo "Generated $TARBALL.gz"
diff --git a/tools/clean-branches b/tools/clean-branches
@@ -4,7 +4,7 @@
 # and also any branches in origin which are ancestors of
 # origin/master and are named like $USER-*.
 
-push_args=''
+push_args=()
 
 function is_merged {
     ! git rev-list -n 1 origin/master.."$1" | grep -q .
@@ -31,7 +31,7 @@ function clean_ref {
                 echo -n "Deleting remote branch $remote_name"
                 echo " (was $(git rev-parse --short "$ref"))"
                 # NB: this won't handle spaces in ref names
-                push_args="$push_args :$remote_name"
+                push_args=("${push_args[@]}" ":$remote_name")
             fi
             ;;
     esac
@@ -44,8 +44,8 @@ fi
 
 git fetch --prune origin
 
-eval $(git for-each-ref --shell --format='clean_ref %(refname);')
+eval "$(git for-each-ref --shell --format='clean_ref %(refname);')"
 
-if [ -n "$push_args" ]; then
-    git push origin $push_args
+if [ "${#push_args}" -ne 0 ]; then
+    git push origin "${push_args[@]}"
 fi
diff --git a/tools/deploy-branch b/tools/deploy-branch
@@ -2,7 +2,7 @@
 
 function error_out {
     echo -en '\e[0;31m'
-    echo $1
+    echo "$1"
     echo -en '\e[0m'
     exit 1
 }
@@ -12,16 +12,16 @@ status=$(git status --porcelain | grep -v '^??')
 
 old_ref=$(git rev-list --max-count=1 HEAD)
 branch=$1
-branch_ref=$(git rev-list --max-count=1 $branch)
+branch_ref=$(git rev-list --max-count=1 "$branch")
 
 [ $? -ne 0 ] && error_out "Unknown branch: $branch"
 
 if [ "$old_ref" == "$branch_ref" ]; then
     new_ref=master
 else
-    ref_name=$(git describe --all --exact $old_ref)
+    ref_name=$(git describe --all --exact "$old_ref")
     if [ $? -eq 0 ]; then
-        new_ref=$(echo $ref_name | perl -pe 's{^(heads|remotes)/}{}')
+        new_ref=$(echo "$ref_name" | perl -pe 's{^(heads|remotes)/}{}')
     else
         new_ref=$old_ref
     fi
@@ -31,13 +31,13 @@ fi
 
 git fetch -p
 
-git rebase origin/master $branch
+git rebase origin/master "$branch"
 [ $? -ne 0 ] && error_out "Rebase onto origin/master failed"
 
 git push . HEAD:master
 git push origin master
 [ $? -ne 0 ] && error_out "Push of master to origin/master failed"
 
-git checkout $new_ref
-git branch -D $branch
-git push origin :$branch
+git checkout "$new_ref"
+git branch -D "$branch"
+git push origin ":$branch"
diff --git a/tools/deployment-lock-ctl b/tools/deployment-lock-ctl
@@ -15,23 +15,23 @@ exit 1
 }
 
 
-if [[ $# < 1 || $# >2 ]]; then
+if [[ $# -lt 1 || $# -gt 2 ]]; then
     usage
 fi
 
 function get_status {
-    if (ssh zulip@$1.zulip.net '[ -d "'"$lockdir"'" ]'); then
+    if (ssh "zulip@$1.zulip.net" '[ -d "'"$lockdir"'" ]'); then
         printf "%-10s %b" "$1" "is currently \e[31mlocked\e[0m\n"
     else
         printf "%-10s %b" "$1" "is currently \e[32munlocked\e[0m\n"
     fi
 }
 
-if [[ $1 == "lock" ]]; then
+if [[ "$1" == "lock" ]]; then
     verb="mkdir"
-elif [[ $1 == "unlock" ]]; then
+elif [[ "$1" == "unlock" ]]; then
     verb="rmdir"
-elif [[ $# == 1 && $1 == "status" ]]; then
+elif [[ $# == 1 && "$1" == "status" ]]; then
     get_status "staging"
     get_status "prod0"
     exit
@@ -40,11 +40,11 @@ else
 fi
 
 
-if [[ $2 == "staging" ]]; then
+if [[ "$2" == "staging" ]]; then
     ssh [email protected] "$verb $lockdir"
     get_status "staging"
     exit
-elif [[ $2 == "prod" ]]; then
+elif [[ "$2" == "prod" ]]; then
     ssh [email protected] "$verb $lockdir"
     get_status "prod0"
     exit

diff --git a/tools/deprecated/iframe-bot/last-messages-wrapper.sh b/tools/deprecated/iframe-bot/last-messages-wrapper.sh
@@ -11,7 +11,7 @@ COUNT=50
 mkdir -p output
 while true; do
 
-if python show-last-messages --api-key=$API_KEY --user=$BOT_EMAIL --streams="$STREAMS" --count=$COUNT; then
+if python show-last-messages --api-key="$API_KEY" --user="$BOT_EMAIL" --streams="$STREAMS" --count="$COUNT"; then
     echo "[`date`] Success";
     mv output-candidate.html output/zulip.html
     touch output/zulip.html

diff --git a/tools/deprecated/include-supported.example b/tools/deprecated/include-supported.example
@@ -21,5 +21,5 @@ if [ $# = 2 ]; then
 fi
 
 for dist in $DISTS; do
-    reprepro --ignore=wrongdistribution include$type $dist "$1"
+    reprepro --ignore=wrongdistribution "include$TYPE" "$dist" "$1"
 done
diff --git a/tools/deprecated/install-server b/tools/deprecated/install-server
@@ -8,7 +8,7 @@ if [ -z "$hostname" ]; then
     echo "USAGE: $0 server type hostname [branch]"
     exit 1
 fi
-if ! $(echo "$hostname" | grep -q zulip); then
+if ! echo "$hostname" | grep -q zulip; then
     echo "USAGE: $0 server type hostname [branch]"
     echo "Hostname must have zulip in it."
     exit 1
@@ -35,11 +35,11 @@ fi
 # Force RSA keys. We do this because the ECDSA key is not printed on syslog,
 # and our puppet configuration does not use ECDSA. If we don't do this,
 # we'll get key errors after puppet apply.
-SSH_OPTS="-o HostKeyAlgorithms=ssh-rsa"
+SSH_OPTS=(-o HostKeyAlgorithms=ssh-rsa)
 
 set +e
 
-ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -ladmin -o "ControlMaster=no" <<EOF
+ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -ladmin -o "ControlMaster=no" <<EOF
 sudo sed -i 's/PermitRootLogin no/PermitRootLogin yes/g' /etc/ssh/sshd_config
 sudo mkdir -p ~root/.ssh && sudo cp .ssh/authorized_keys ~root/.ssh/authorized_keys
 sudo service ssh restart
@@ -48,7 +48,7 @@ EOF
 
 set -e
 
-ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -lroot <<EOF
+ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lroot <<EOF
 resize2fs /dev/xvda1
 echo "$hostname" > /etc/hostname
 sed -i 's/localhost$/localhost $hostname/' /etc/hosts
@@ -69,9 +69,9 @@ EOF
 
 # Give new server git access
 # TODO: Don't give servers push access to our git!
-scp $SSH_OPTS -i "$amazon_key_file" "$server_private_key_file" root@"$server":/root/.ssh/id_rsa
+scp "${SSH_OPTS[@]}" -i "$amazon_key_file" "$server_private_key_file" root@"$server":/root/.ssh/id_rsa
 
-ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -lroot <<EOF
+ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lroot <<EOF
 chmod 600 /root/.ssh/id_rsa
 # Setup initial known_hosts including git server
 cat > /root/.ssh/known_hosts <<EOF2
@@ -85,7 +85,7 @@ cd /root/zulip
 git checkout $branch
 EOF
 
-ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -lroot <<EOF
+ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lroot <<EOF
 cp -a /root/zulip/puppet/zulip/files/puppet.conf /etc/puppet/
 
 userdel admin
@@ -107,8 +107,8 @@ fi
 EOF
 
 # TODO: Don't give servers push access to our git!
-scp $SSH_OPTS -i "$amazon_key_file" "$server_private_key_file" zulip@"$server":/home/zulip/.ssh/id_rsa
-ssh $SSH_OPTS "$server" -t -i "$amazon_key_file" -lzulip <<EOF
+scp "${SSH_OPTS[@]}" -i "$amazon_key_file" "$server_private_key_file" zulip@"$server":/home/zulip/.ssh/id_rsa
+ssh "${SSH_OPTS[@]}" "$server" -t -i "$amazon_key_file" -lzulip <<EOF
 chmod 600 /home/zulip/.ssh/id_rsa
 EOF