From 9c839d789f2bdb0e86b252a6fdd885ba47469a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 30 Sep 2017 04:45:31 +0200 Subject: [PATCH] qubes-rpc: fix issues found by shellcheck Most of them are missing quotes, `` -> $(), and -o/-a usage in conditions. Also add few directives disabling checks where were too verbose. --- qrexec/qubes-rpc-multiplexer | 2 +- qubes-rpc/prepare-suspend | 25 +++++++++++++++---------- qubes-rpc/qubes.Backup | 14 +++++++------- qubes-rpc/qubes.DetachPciDevice | 4 ++-- qubes-rpc/qubes.GetAppmenus | 1 + qubes-rpc/qubes.GetImageRGBA | 14 +++++++++----- qubes-rpc/qubes.InstallUpdatesGUI | 6 +++--- qubes-rpc/qubes.OpenURL | 2 +- qubes-rpc/qubes.ResizeDisk | 4 ++-- qubes-rpc/qubes.Restore | 28 +++++++++++++++------------- qubes-rpc/qubes.SetDateTime | 6 +++--- qubes-rpc/qubes.StartApp | 2 +- qubes-rpc/qubes.WaitForSession | 2 +- qubes-rpc/qvm-copy-to-vm | 5 +++-- qubes-rpc/qvm-copy-to-vm.gnome | 7 ++++--- qubes-rpc/qvm-copy-to-vm.kde | 13 +++++++------ qubes-rpc/qvm-move-to-vm.gnome | 7 ++++--- qubes-rpc/qvm-move-to-vm.kde | 15 ++++++++------- qubes-rpc/qvm-open-in-dvm | 1 + qubes-rpc/qvm-run-vm | 1 + qubes-rpc/qvm-sync-clock | 1 + 21 files changed, 90 insertions(+), 70 deletions(-) diff --git a/qrexec/qubes-rpc-multiplexer b/qrexec/qubes-rpc-multiplexer index bdc39c8..76133a7 100755 --- a/qrexec/qubes-rpc-multiplexer +++ b/qrexec/qubes-rpc-multiplexer @@ -18,7 +18,7 @@ QUBES_RPC=/etc/qubes-rpc LOCAL_QUBES_RPC=/usr/local/etc/qubes-rpc if ! [ $# = 2 ] ; then - echo $0: bad argument count, usage: $0 SERVICE-NAME REMOTE-DOMAIN-NAME >&2 + echo "$0: bad argument count, usage: $0 SERVICE-NAME REMOTE-DOMAIN-NAME" >&2 exit 1 fi export QREXEC_REMOTE_DOMAIN="$2" diff --git a/qubes-rpc/prepare-suspend b/qubes-rpc/prepare-suspend index f5f3721..55331a1 100755 --- a/qubes-rpc/prepare-suspend +++ b/qubes-rpc/prepare-suspend @@ -8,10 +8,10 @@ action=$1 MODULES_BLACKLIST="" if [ -r /etc/qubes-suspend-module-blacklist ]; then - MODULES_BLACKLIST="$MODULES_BLACKLIST `grep -v '^#' /etc/qubes-suspend-module-blacklist`" + MODULES_BLACKLIST="$MODULES_BLACKLIST $(grep -v '^#' /etc/qubes-suspend-module-blacklist)" fi if [ -r /rw/config/suspend-module-blacklist ]; then - MODULES_BLACKLIST="$MODULES_BLACKLIST `grep -v '^#' /rw/config/suspend-module-blacklist`" + MODULES_BLACKLIST="$MODULES_BLACKLIST $(grep -v '^#' /rw/config/suspend-module-blacklist)" fi if [ x"$action" = x"suspend" ]; then @@ -23,23 +23,28 @@ if [ x"$action" = x"suspend" ]; then service NetworkManager stop fi # Force interfaces down, just in case when NM didn't done it - for if in `ls /sys/class/net|grep -v "lo\|vif"`; do - if [ "`cat /sys/class/net/$if/device/devtype 2>/dev/null`" = "vif" ]; then + for intf in /sys/class/net/*; do + intf=$(basename "$intf") + if [ "$intf" = "lo" ] || [[ "$intf" = "vif"* ]]; then continue fi - ip l s $if down + if [ "$(cat "/sys/class/net/$intf/device/devtype" 2>/dev/null$)" = "vif" ]; then + continue + fi + ip l s "$intf" down done LOADED_MODULES="" for mod in $MODULES_BLACKLIST; do - if lsmod |grep -q $mod; then + if lsmod |grep -q "$mod"; then LOADED_MODULES="$LOADED_MODULES $mod" - modprobe -r $mod + modprobe -r "$mod" fi done - echo $LOADED_MODULES > /var/run/qubes/suspend-modules-loaded + echo "$LOADED_MODULES" > /var/run/qubes/suspend-modules-loaded else - for mod in `cat /var/run/qubes/suspend-modules-loaded`; do - modprobe $mod + # shellcheck disable=SC2013 + for mod in $(cat /var/run/qubes/suspend-modules-loaded); do + modprobe "$mod" done if qsvc network-manager ; then dbus-send --system --print-reply \ diff --git a/qubes-rpc/qubes.Backup b/qubes-rpc/qubes.Backup index fed97e4..e46b766 100755 --- a/qubes-rpc/qubes.Backup +++ b/qubes-rpc/qubes.Backup @@ -1,22 +1,22 @@ #!/bin/sh echo Starting Backupcopy -read args -echo Arguments: $args +read -r args +echo Arguments: "$args" if [ -d "$args" ] ; then echo "Performing backup to directory $args" - TARGET="$args/qubes-backup-`date +'%Y-%m-%dT%H%M%S'`" + TARGET="$args/qubes-backup-$(date +'%Y-%m-%dT%H%M%S')" echo "Copying STDIN data to $TARGET" - cat > $TARGET + cat > "$TARGET" else echo "Checking if arguments is matching a command" - COMMAND=`echo $args | cut -d ' ' -f 1` - if type "$COMMAND"; then + COMMAND=$(echo "$args" | cut -d ' ' -f 1) + if command -v "$COMMAND" >/dev/null; then echo "Redirecting STDIN to $args" # Parsing args to handle quotes correctly # Dangerous method if args are uncontrolled eval "set -- $args" - $@ + "$@" else echo "Invalid command $COMMAND" exit 1 diff --git a/qubes-rpc/qubes.DetachPciDevice b/qubes-rpc/qubes.DetachPciDevice index 0edfadf..31aaff8 100755 --- a/qubes-rpc/qubes.DetachPciDevice +++ b/qubes-rpc/qubes.DetachPciDevice @@ -1,4 +1,4 @@ #!/bin/sh -read dev +read -r dev BDF=0000:$dev -echo $BDF > /sys/bus/pci/devices/$BDF/driver/unbind +echo "$BDF" > "/sys/bus/pci/devices/$BDF/driver/unbind" diff --git a/qubes-rpc/qubes.GetAppmenus b/qubes-rpc/qubes.GetAppmenus index d75570c..d8a057a 100755 --- a/qubes-rpc/qubes.GetAppmenus +++ b/qubes-rpc/qubes.GetAppmenus @@ -1,4 +1,5 @@ #!/bin/sh +# shellcheck disable=SC2016 find /usr/share/applications/ /usr/local/share/applications/ -name '*.desktop' -print0 2>/dev/null | \ xargs -0 awk ' BEGINFILE { entry="" } diff --git a/qubes-rpc/qubes.GetImageRGBA b/qubes-rpc/qubes.GetImageRGBA index db68e61..ac13087 100755 --- a/qubes-rpc/qubes.GetImageRGBA +++ b/qubes-rpc/qubes.GetImageRGBA @@ -1,6 +1,6 @@ #!/bin/sh set -e -read filename +read -r filename ICON_MAXSIZE=512 @@ -29,8 +29,8 @@ if [ "$m" = SVG ]; then tmpfile2="$(mktemp /tmp/qimg-XXXXXXXX.png)" rsvg-convert -o "$tmpfile2" "$filename" # downscale the image if necessary - if [ -n "$forcemaxsize" -a \ - \( "$w" -gt "$forcemaxsize" -o "$h" -gt "$forcemaxsize" \) ]; then + if [ -n "$forcemaxsize" ] && \ + ( [ "$w" -gt "$forcemaxsize" ] || [ "$h" -gt "$forcemaxsize" ] ); then convert "$tmpfile2" -scale "${forcemaxsize}x${forcemaxsize}" "$tmpfile2" # read the size again, because icon may not be a square s="$(identify -format '%w %h' "$tmpfile2")" @@ -42,7 +42,11 @@ fi echo "$w $h" convert -depth 8 -size "${w}x${h}" "$filename" rgba:- -[ -n "${tmpfile}" ] && rm -f "${tmpfile}" || true -[ -n "${tmpfile2}" ] && rm -f "${tmpfile2}" || true +if [ -n "${tmpfile}" ]; then + rm -f "${tmpfile}" +fi +if [ -n "${tmpfile2}" ]; then + rm -f "${tmpfile2}" +fi # vim: ft=sh ts=4 sw=4 et diff --git a/qubes-rpc/qubes.InstallUpdatesGUI b/qubes-rpc/qubes.InstallUpdatesGUI index c9818c7..9c14e6c 100755 --- a/qubes-rpc/qubes.InstallUpdatesGUI +++ b/qubes-rpc/qubes.InstallUpdatesGUI @@ -4,13 +4,13 @@ # If you are creating package for other distribution, feel free to replace it # with distribution-specific script. -if [ -e /etc/redhat-release -a -x /usr/bin/dnf ]; then +if [ -e /etc/redhat-release ] && [ -x /usr/bin/dnf ]; then update_cmd='dnf update --best' -elif [ -e /etc/redhat-release -a -x /usr/bin/yum ]; then +elif [ -e /etc/redhat-release ] && [ -x /usr/bin/yum ]; then update_cmd='yum update' elif [ -e /etc/debian_version ]; then update_cmd='apt-get update && apt-get -V dist-upgrade' -elif [ -e /etc/arch-release -a -x /usr/bin/powerpill ]; then +elif [ -e /etc/arch-release ] && [ -x /usr/bin/powerpill ]; then update_cmd='powerpill -Suy' elif [ -e /etc/arch-release ]; then update_cmd='pacman -Suy' diff --git a/qubes-rpc/qubes.OpenURL b/qubes-rpc/qubes.OpenURL index 17daa2b..dae1782 100755 --- a/qubes-rpc/qubes.OpenURL +++ b/qubes-rpc/qubes.OpenURL @@ -1,6 +1,6 @@ #!/bin/sh -read url +read -r url case "$url" in http://*|\ diff --git a/qubes-rpc/qubes.ResizeDisk b/qubes-rpc/qubes.ResizeDisk index a1ad8b1..0ac0656 100755 --- a/qubes-rpc/qubes.ResizeDisk +++ b/qubes-rpc/qubes.ResizeDisk @@ -1,6 +1,6 @@ #!/bin/sh -read disk_name +read -r disk_name set -e @@ -15,7 +15,7 @@ case $disk_name in head /dev/xvda > /dev/null new_size=$(cat /sys/block/xvda/size) ro=$(/sys/block/xvda/ro) - if [ $ro -eq 1 ]; then + if [ "$ro" -eq 1 ]; then new_table="0 $new_size snapshot /dev/xvda /dev/xvdc2 N 16" else new_table="0 $new_size linear /dev/xvda 0" diff --git a/qubes-rpc/qubes.Restore b/qubes-rpc/qubes.Restore index b97e4b6..3185e7e 100755 --- a/qubes-rpc/qubes.Restore +++ b/qubes-rpc/qubes.Restore @@ -1,27 +1,29 @@ #!/bin/sh echo Starting Restorecopy >&2 -read args -read paths -echo Arguments: $args >&2 -echo Paths: $paths >&2 +read -r args +read -r paths +echo "Arguments: $args" >&2 +echo "Paths: $paths" >&2 if [ -f "$args" ] ; then echo "Performing restore from backup file $args" >&2 TARGET="$args" echo "Copying $TARGET to STDOUT" >&2 + # shellcheck disable=SC2086 /usr/lib/qubes/tar2qfile "$TARGET" $paths else echo "Checking if arguments is matching a command" >&2 - COMMAND=`echo $args | cut -d ' ' -f 1` - if type "$COMMAND" >/dev/null; then - tmpdir=`mktemp -d` - mkfifo $tmpdir/backup-data + COMMAND=$(echo "$args" | cut -d ' ' -f 1) + if command -v "$COMMAND" >/dev/null; then + tmpdir=$(mktemp -d) + mkfifo "$tmpdir/backup-data" echo "Redirecting $args to STDOUT" >&2 # Parsing args to handle quotes correctly # Dangerous method if args are uncontrolled eval "set -- $args" # Use named pipe to pass original stdin to tar2file - $@ > $tmpdir/backup-data < /dev/null & - /usr/lib/qubes/tar2qfile $tmpdir/backup-data $paths + "$@" > "$tmpdir/backup-data" < /dev/null & + # shellcheck disable=SC2086 + /usr/lib/qubes/tar2qfile "$tmpdir/backup-data" $paths # Restoration may be terminated earlier because of selected files. This # will be seen as EPIPE to the retrieving process, which may cause retcode # other than 0 in some cases - which would be incorrectly treated as backup @@ -29,9 +31,9 @@ else # detect if anything wrong with actual data) retcode=$? wait -n - rm $tmpdir/backup-data - rmdir $tmpdir - exit $retcode + rm "$tmpdir/backup-data" + rmdir "$tmpdir" + exit "$retcode" else echo "Invalid command $COMMAND" >&2 exit 2 diff --git a/qubes-rpc/qubes.SetDateTime b/qubes-rpc/qubes.SetDateTime index a55e850..abbefd8 100755 --- a/qubes-rpc/qubes.SetDateTime +++ b/qubes-rpc/qubes.SetDateTime @@ -2,9 +2,9 @@ # it is in format of `date -u -Iseconds`, example: 2014-09-29T22:59:21+0000 # it comes from dom0, so is trusted -read timestamp -timediff=$(( `date -u +'+%Y%m%d%H%M%S'` - `date -u -d "$timestamp" +'+%Y%m%d%H%M%S'` )) -if [ $timediff -le 2 -a $timediff -ge -2 ]; then +read -r timestamp +timediff=$(( $(date -u +'+%Y%m%d%H%M%S') - $(date -u -d "$timestamp" +'+%Y%m%d%H%M%S') )) +if [ "$timediff" -le 2 ] && [ "$timediff" -ge -2 ]; then # don't bother exit 0 fi diff --git a/qubes-rpc/qubes.StartApp b/qubes-rpc/qubes.StartApp index 6062672..92896fd 100755 --- a/qubes-rpc/qubes.StartApp +++ b/qubes-rpc/qubes.StartApp @@ -16,7 +16,7 @@ for dir in $(echo "$XDG_DATA_HOME:$XDG_DATA_DIRS" | tr : ' '); do if ! [ -d "$dir/applications" ]; then continue fi - for subdir in $(find $dir/applications -type d | sort); do + for subdir in $(find "$dir/applications" -type d | sort); do if [ -f "$subdir/$app_basename" ]; then exec qubes-desktop-run "$subdir/$app_basename" fi diff --git a/qubes-rpc/qubes.WaitForSession b/qubes-rpc/qubes.WaitForSession index 2d82d34..34bc2b9 100755 --- a/qubes-rpc/qubes.WaitForSession +++ b/qubes-rpc/qubes.WaitForSession @@ -1,5 +1,5 @@ #!/bin/sh -read USERNAME +read -r USERNAME cmd='echo $$ >> /tmp/qubes-session-waiter; [ ! -f /tmp/qubes-session-env ] && exec sleep inf' if [ "$(id -un)" = "$USERNAME" ]; then sh -c "$cmd" 2>/dev/null diff --git a/qubes-rpc/qvm-copy-to-vm b/qubes-rpc/qvm-copy-to-vm index e1c3338..e33331b 100755 --- a/qubes-rpc/qvm-copy-to-vm +++ b/qubes-rpc/qvm-copy-to-vm @@ -22,7 +22,7 @@ set -e # if [ $# -lt 2 ] ; then - echo usage: $0 '[--without-progress] dest_vmname file [file]+' + echo "usage: $0 [--without-progress] dest_vmname file [file]+" exit 1 fi @@ -38,7 +38,8 @@ VM="$1" shift if [ $PROGRESS_TYPE = console ] ; then - export FILECOPY_TOTAL_SIZE=$(du --apparent-size -c -- "$@" 2> /dev/null | tail -1 | cut -f 1) + FILECOPY_TOTAL_SIZE=$(du --apparent-size -c -- "$@" 2> /dev/null | tail -1 | cut -f 1) + export FILECOPY_TOTAL_SIZE fi /usr/lib/qubes/qrexec-client-vm "$VM" qubes.Filecopy /usr/lib/qubes/qfile-agent "$@" diff --git a/qubes-rpc/qvm-copy-to-vm.gnome b/qubes-rpc/qvm-copy-to-vm.gnome index f8fcd8f..1d01581 100755 --- a/qubes-rpc/qvm-copy-to-vm.gnome +++ b/qubes-rpc/qvm-copy-to-vm.gnome @@ -24,8 +24,9 @@ SIZE=$(du --apparent-size -c -- "$@" 2>/dev/null | tail -1 | cut -f 1) export PROGRESS_TYPE=gui +# shellcheck disable=SC2016 /usr/lib/qubes/qrexec-client-vm '$default' qubes.Filecopy /usr/lib/qubes/qfile-agent "$@" | -(while read sentsize ; do - CURRSIZE=$(($sentsize/1024)) - echo $((100*$CURRSIZE/$SIZE)) +(while read -r sentsize ; do + CURRSIZE=$((sentsize / 1024)) + echo $((100 * CURRSIZE / SIZE)) done) | zenity --progress --text="Copying files..." --auto-close diff --git a/qubes-rpc/qvm-copy-to-vm.kde b/qubes-rpc/qvm-copy-to-vm.kde index a87433b..3e51ae1 100755 --- a/qubes-rpc/qvm-copy-to-vm.kde +++ b/qubes-rpc/qvm-copy-to-vm.kde @@ -19,21 +19,22 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # # -if type kdialog 2> /dev/null; then +if command -v kdialog 2> /dev/null; then SIZE=$(du --apparent-size -c -- "$@" 2> /dev/null | tail -1 | cut -f 1) REF=$(kdialog --progressbar "Copy progress") - qdbus $REF org.freedesktop.DBus.Properties.Set "" maximum $SIZE + qdbus "$REF" org.freedesktop.DBus.Properties.Set "" maximum "$SIZE" export PROGRESS_TYPE=gui + # shellcheck disable=SC2016 /usr/lib/qubes/qrexec-client-vm '$default' qubes.Filecopy \ /usr/lib/qubes/qfile-agent "$@" | - (while read sentsize ; do - CURRSIZE=$(($sentsize/1024)) - qdbus $REF org.freedesktop.DBus.Properties.Set "" value $CURRSIZE + (while read -r sentsize ; do + CURRSIZE=$((sentsize / 1024)) + qdbus "$REF" org.freedesktop.DBus.Properties.Set "" value "$CURRSIZE" done) - qdbus $REF close + qdbus "$REF" close # we do not want a dozen error messages, do we # if ! [ "x"$agentstatus = xDONE ] ; then # kdialog --sorry 'Abnormal file copy termination; see /var/log/qubes/qrexec.xid.log in dom0 for more details' diff --git a/qubes-rpc/qvm-move-to-vm.gnome b/qubes-rpc/qvm-move-to-vm.gnome index 0750994..1385c9f 100755 --- a/qubes-rpc/qvm-move-to-vm.gnome +++ b/qubes-rpc/qvm-move-to-vm.gnome @@ -26,9 +26,10 @@ export PROGRESS_TYPE=gui set -o pipefail set -e +# shellcheck disable=SC2016 /usr/lib/qubes/qrexec-client-vm '$default' qubes.Filecopy /usr/lib/qubes/qfile-agent "$@" | -(while read sentsize ; do - CURRSIZE=$(($sentsize/1024)) - echo $((100*$CURRSIZE/$SIZE)) +(while read -r sentsize ; do + CURRSIZE=$((sentsize / 1024)) + echo $((100 * CURRSIZE / SIZE)) done) | zenity --progress --text="Moving files..." --auto-close rm -rf "$@" diff --git a/qubes-rpc/qvm-move-to-vm.kde b/qubes-rpc/qvm-move-to-vm.kde index 4c42cbb..da934cb 100755 --- a/qubes-rpc/qvm-move-to-vm.kde +++ b/qubes-rpc/qvm-move-to-vm.kde @@ -21,26 +21,27 @@ # if type kdialog 2> /dev/null; then VM=$(kdialog -inputbox "Enter the VM name to send files to:") - if [ X$VM = X ] ; then exit 0 ; fi + if [ "X$VM" = X ] ; then exit 0 ; fi SIZE=$(du --apparent-size -c -- "$@" 2> /dev/null | tail -1 | cut -f 1) REF=$(kdialog --progressbar "Move progress") - qdbus $REF org.freedesktop.DBus.Properties.Set "" maximum $SIZE + qdbus "$REF" org.freedesktop.DBus.Properties.Set "" maximum "$SIZE" export PROGRESS_TYPE=gui set -o pipefail - /usr/lib/qubes/qrexec-client-vm $VM qubes.Filecopy \ + /usr/lib/qubes/qrexec-client-vm "$VM" qubes.Filecopy \ /usr/lib/qubes/qfile-agent "$@" | - (while read sentsize ; do - CURRSIZE=$(($sentsize/1024)) - qdbus $REF org.freedesktop.DBus.Properties.Set "" value $CURRSIZE + (while read -r sentsize ; do + CURRSIZE=$((sentsize / 1024)) + qdbus "$REF" org.freedesktop.DBus.Properties.Set "" value "$CURRSIZE" done) + # shellcheck disable=SC2181 if [ $? -eq 0 ]; then rm -rf "$@" fi - qdbus $REF close + qdbus "$REF" close # we do not want a dozen error messages, do we # if ! [ "x"$agentstatus = xDONE ] ; then # kdialog --sorry 'Abnormal file copy termination; see /var/log/qubes/qrexec.xid.log in dom0 for more details' diff --git a/qubes-rpc/qvm-open-in-dvm b/qubes-rpc/qvm-open-in-dvm index 3a01cdf..84af921 100755 --- a/qubes-rpc/qvm-open-in-dvm +++ b/qubes-rpc/qvm-open-in-dvm @@ -25,4 +25,5 @@ if ! [ $# = 1 ] ; then exit 1 fi +# shellcheck disable=SC2016 exec qvm-open-in-vm '$dispvm' "$1" diff --git a/qubes-rpc/qvm-run-vm b/qubes-rpc/qvm-run-vm index 7ae4883..b695a36 100755 --- a/qubes-rpc/qvm-run-vm +++ b/qubes-rpc/qvm-run-vm @@ -44,6 +44,7 @@ VMNAME="$1" shift if [ "$VMNAME" = "--dispvm" ] ; then + # shellcheck disable=SC2016 VMNAME='$dispvm' elif [ "$VMNAME" = "" ] ; then print_usage diff --git a/qubes-rpc/qvm-sync-clock b/qubes-rpc/qvm-sync-clock index 6b67fa9..eaaf6f6 100644 --- a/qubes-rpc/qvm-sync-clock +++ b/qubes-rpc/qvm-sync-clock @@ -18,4 +18,5 @@ # # +# shellcheck disable=SC2016 qrexec-client-vm '$default' qubes.GetDate /usr/lib/qubes/qubes-sync-clock