Improve bash quoting, and other similar best practices

This patch updates the shell scripts with some of the common best
practices, which should make them more resilient to unusual failures and
unexpected environments (in particular, directories with spaces).

Most of these were identified by shellcheck.

cmd/chasquid-util/test.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../../test/util/lib.sh
+. "$(dirname "$0")/../../test/util/lib.sh"
 
 init
 

cmd/dovecot-auth-cli/test.sh

@@ -1,13 +1,14 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../../test/util/lib.sh
+. "$(dirname "$0")/../../test/util/lib.sh"
 
 init
 
 # Build the binary once, so we can use it and launch it in chamuyero scripts.
 # Otherwise, we not only spend time rebuilding it over and over, but also "go
 # run" masks the exit code, which is something we care about.
+# shellcheck disable=SC2086
 if [ "${COVER_DIR}" != "" ]; then
 	go test -covermode=count -coverpkg=../../... -c \
 		$GOFLAGS -tags="coveragebin $GOTAGS"
@@ -22,9 +23,9 @@ if ! ./dovecot-auth-cli lalala 2>&1 | grep -q "invalid arguments"; then
 fi
 
 for i in *.cmy; do
-	if ! chamuyero $i > $i.log 2>&1 ; then
+	if ! chamuyero "$i" > "$i.log" 2>&1 ; then
 		echo "# Test $i failed, log follows"
-		cat $i.log
+		cat "$i.log"
 		exit 1
 	fi
 done

cmd/mda-lmtp/test.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../../test/util/lib.sh
+. "$(dirname "$0")/../../test/util/lib.sh"
 
 init
 
@@ -11,9 +11,9 @@ init
 go build
 
 for i in *.cmy; do
-	if ! chamuyero $i > $i.log 2>&1 ; then
+	if ! chamuyero "$i" > "$i.log" 2>&1 ; then
 		echo "# Test $i failed, log follows"
-		cat $i.log
+		cat "$i.log"
 		exit 1
 	fi
 done

docker/add-user.sh

@@ -7,15 +7,15 @@
 
 set -e
 
-read -p "Email (full user@domain format): " EMAIL
+read -r -p "Email (full user@domain format): " EMAIL
 
 if ! echo "${EMAIL}" | grep -q @; then
 	echo "Error: email should have '@'."
 	exit 1
 fi
 
 
-read -p "Password: " -s PASSWORD
+read -r -p "Password: " -s PASSWORD
 echo
 
 DOMAIN=$(echo echo "${EMAIL}" | cut -d '@' -f 2)
@@ -33,7 +33,7 @@ mkdir -p /data/dovecot
 touch /data/dovecot/users
 if grep -q "^${EMAIL}:" /data/dovecot/users; then
 	cp /data/dovecot/users /data/dovecot/users.old
-	cat /data/dovecot/users.old | grep -v "^${EMAIL}:" \
+	grep -v "^${EMAIL}:" /data/dovecot/users.old \
 		> /data/dovecot/users
 fi
 

docker/entrypoint.sh

@@ -28,15 +28,16 @@ if [ "$AUTO_CERTS" != "" ]; then
 		MAIL_OPTS="-m $CERTS_MAIL"
 	fi
 
-	for DOMAIN in $(echo $AUTO_CERTS); do
+	for DOMAIN in $AUTO_CERTS; do
 		# If it has never been set up, then do so.
-		if ! [ -e /etc/letsencrypt/live/$DOMAIN/fullchain.pem ]; then
+		if ! [ -e "/etc/letsencrypt/live/$DOMAIN/fullchain.pem" ]; then
+			# shellcheck disable=SC2086
 			certbot certonly \
 				--non-interactive \
 				--standalone \
 				--agree-tos \
 				$MAIL_OPTS \
-				-d $DOMAIN
+				-d "$DOMAIN"
 		else
 			echo "$DOMAIN certificate already set up."
 		fi
@@ -53,9 +54,10 @@ if [ "$AUTO_CERTS" != "" ]; then
 fi
 
 CERT_DOMAINS=""
-for i in $(ls /etc/letsencrypt/live/); do
-	if [ -e "/etc/letsencrypt/live/$i/fullchain.pem" ]; then
-		CERT_DOMAINS="$CERT_DOMAINS $i"
+for i in /etc/letsencrypt/live/*; do
+	D="${i##*/}"  # Extract the last component of the path.
+	if [ -e "/etc/letsencrypt/live/$D/fullchain.pem" ]; then
+		CERT_DOMAINS="$CERT_DOMAINS $D"
 	fi
 done
 
@@ -104,4 +106,5 @@ echo "hostname: '$ONE_DOMAIN'" >> /etc/chasquid/chasquid.conf
 start-stop-daemon --start --quiet --pidfile /run/dovecot.pid \
 	--exec /usr/sbin/dovecot -- -c /etc/dovecot/dovecot.conf
 
+# shellcheck disable=SC2086
 sudo -u chasquid -g chasquid  /usr/bin/chasquid  $CHASQUID_FLAGS

docs/man/generate.sh

@@ -8,20 +8,20 @@
 set -e
 
 for IN in *.pod; do
-	OUT=$( basename $IN .pod )
+	OUT=$(basename "$IN" .pod)
 	SECTION=${OUT##*.}
 	NAME=${OUT%.*}
 
 	# If it has not changed in git, set the mtime to the last commit that
 	# touched the file.
 	CHANGED=$( git status --porcelain -- "$IN" | wc -l )
-	if [ $CHANGED -eq 0 ]; then
+	if [ "$CHANGED" -eq 0 ]; then
 		GIT_MTIME=$( git log --pretty=%at -n1 -- "$IN" )
 		touch -d "@$GIT_MTIME" "$IN"
 	fi
 
-	podchecker $IN
-	pod2man --section=$SECTION --name=$NAME \
+	podchecker "$IN"
+	pod2man --section="$SECTION" --name="$NAME" \
 		--release "" --center "" \
-		$IN $OUT
+		"$IN" "$OUT"
 done

test/cover.sh

@@ -10,7 +10,7 @@
 # coverage_test.go), but works for now.
 
 set -e
-. $(dirname ${0})/util/lib.sh
+. "$(dirname "$0")/util/lib.sh"
 
 init
 
@@ -28,11 +28,11 @@ export COVER_DIR="$PWD/.coverage"
 # tests, which don't expect any expvars to exists besides the one registered
 # in the tests themselves.
 for pkg in $(go list ./... | grep -v -E 'chasquid/cmd/|chasquid/test'); do
-	OUT_FILE="$COVER_DIR/pkg-`echo $pkg | sed s+/+_+g`.out"
+	OUT_FILE="$COVER_DIR/pkg-${pkg//\//_}.out"
 	go test -tags coverage \
 		-covermode=count \
 		-coverprofile="$OUT_FILE" \
-		-coverpkg=./... $pkg
+		-coverpkg=./... "$pkg"
 done
 
 # Integration tests.
@@ -61,5 +61,5 @@ go run "${UTILDIR}/coverhtml/coverhtml.go" \
 echo
 echo
 echo "Coverage report can be found in:"
-echo file://$COVER_DIR/coverage.html
+echo "file://$COVER_DIR/coverage.html"
 

test/run.sh

@@ -1,16 +1,16 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/util/lib.sh
+. "$(dirname "$0")/util/lib.sh"
 
 init
 
 FAILED=0
 
 for i in t-*; do
-	echo $i ...
-	setsid -w $i/run.sh
-	FAILED=$(( $FAILED + $? ))
+	echo "$i" ...
+	setsid -w "$i/run.sh"
+	FAILED=$(( FAILED + $? ))
 	echo
 done
 

test/stress-01-load/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 
@@ -14,18 +14,18 @@ mkdir -p .logs
 chasquid -v=-1 --logfile=.logs/chasquid.log --config_dir=config &
 wait_until_ready 1025
 
-echo Peak RAM: `chasquid_ram_peak`
+echo "Peak RAM: $(chasquid_ram_peak)"
 
 if ! loadgen -logtime -addr=localhost:1025 -run_for=3s -noop; then
-	fail
+	fail "loadgen -noop error"
 fi
 
-echo Peak RAM: `chasquid_ram_peak`
+echo "Peak RAM: $(chasquid_ram_peak)"
 
 if ! loadgen -logtime -addr=localhost:1025 -run_for=3s; then
-	fail
+	fail "loadgen error"
 fi
 
-echo Peak RAM: `chasquid_ram_peak`
+echo "Peak RAM: $(chasquid_ram_peak)"
 
 success

test/stress-02-connections/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 
@@ -14,22 +14,22 @@ mkdir -p .logs
 chasquid -v=-1 --logfile=.logs/chasquid.log --config_dir=config &
 wait_until_ready 1025
 
-echo Peak RAM: `chasquid_ram_peak`
+echo "Peak RAM: $(chasquid_ram_peak)"
 
 # Set connection count to (max open files) - (leeway).
 # We set the leeway to account for file descriptors opened by the runtime and
 # listeners; 20 should be enough for now.
 # Cap it to 2000, as otherwise it can be problematic due to port availability.
-COUNT=$(( `ulimit -n` - 20 ))
+COUNT=$(( $(ulimit -n) - 20 ))
 if [ $COUNT -gt 2000 ]; then
 	COUNT=2000
 fi
 
 if ! conngen -logtime -addr=localhost:1025 -count=$COUNT; then
 	tail -n 1 .logs/chasquid.log
-	fail
+	fail "conngen error"
 fi
 
-echo Peak RAM: `chasquid_ram_peak`
+echo "Peak RAM: $(chasquid_ram_peak)"
 
 success

test/stress.sh

@@ -1,16 +1,16 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/util/lib.sh
+. "$(dirname "$0")/util/lib.sh"
 
 init
 
 FAILED=0
 
 for i in stress-*; do
-	echo $i ...
-	setsid -w $i/run.sh
-	FAILED=$(( $FAILED + $? ))
+	echo "$i ..."
+	setsid -w "$i/run.sh"
+	FAILED=$(( FAILED + $? ))
 	echo
 done
 

test/t-01-simple_local/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 check_hostaliases

test/t-02-exim/get-exim4-debian.sh

@@ -5,19 +5,19 @@
 # given the nature of these tests that's acceptable for now.
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 
 # Download and extract the package in .exim-bin
 apt download exim4-daemon-light
-dpkg -x exim4-daemon-light_*.deb $PWD/.exim-bin/
+dpkg -x exim4-daemon-light_*.deb "$PWD/.exim-bin/"
 
 # Create a symlink to .exim4, which is the directory we will use to store
 # configuration, spool, etc.
 # The configuration template will look for it here.
 mkdir -p .exim4
-ln -sf $PWD/.exim-bin/usr/sbin/exim4 .exim4/
+ln -sf "$PWD/.exim-bin/usr/sbin/exim4" .exim4/
 
 # Remove the setuid bit, if there is one - we don't need it and may cause
 # confusion and/or security troubles.

test/t-02-exim/run.sh

@@ -23,7 +23,7 @@
 #   chasquid will receive the email from exim, and deliver it locally.
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 check_hostaliases

test/t-03-queue_persistency/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 

test/t-04-aliases/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 check_hostaliases
@@ -14,12 +14,12 @@ chasquid -v=2 --logfile=.logs/chasquid.log --config_dir=config &
 wait_until_ready 1025
 
 function send_and_check() {
-	run_msmtp $1@testserver < content
+	run_msmtp "$1@testserver" < content
 	shift
-	for i in $@; do
-		wait_for_file .mail/$i@testserver
-		mail_diff content .mail/$i@testserver
-		rm -f .mail/$i@testserver
+	for i in "$@"; do
+		wait_for_file ".mail/$i@testserver"
+		mail_diff content ".mail/$i@testserver"
+		rm -f ".mail/$i@testserver"
 	done
 }
 

test/t-05-null_address/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 check_hostaliases

test/t-06-idna/run.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init
 check_hostaliases

test/t-07-smtputf8/run.sh

@@ -5,7 +5,7 @@
 # capitalizations.
 
 set -e
-. $(dirname ${0})/../util/lib.sh
+. "$(dirname "$0")/../util/lib.sh"
 
 init