From fe98408c5cac28a5c4c616cfb717bd04a09de73f Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Tue, 22 Mar 2022 05:30:40 -0400 Subject: [PATCH 1/3] Better parsing of command-line options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes qubes-dom0-update aware of various command-line options that take arguments, so it can process them accordingly. It is not a perfect solution; that would require using a C wrapper program to reject abbreviated or unknown options. It also uses bash regex matching instead of the `echo | grep` anti-pattern, replaces an `==` with `=`, adds a missing `--`, and unsets a variable that will be checked for being set later. The same approach used here could be used to refactor qubes-gpg-client-wrapper if it were worth the time, which it probably isn’t unless qubes-gpg-client-wrapper will see future use. --- dom0-updates/qubes-dom0-update | 86 +++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 16 deletions(-) diff --git a/dom0-updates/qubes-dom0-update b/dom0-updates/qubes-dom0-update index 3a3b60a8..70a9fe1c 100755 --- a/dom0-updates/qubes-dom0-update +++ b/dom0-updates/qubes-dom0-update @@ -1,13 +1,12 @@ -#!/bin/bash - -find_regex_in_args() { - local regex="${1}" - shift 1 - - for arg in "${@}"; do - if echo "${arg}" | grep -q -e "${regex}"; then - return 0 - fi +#!/bin/bash -- +set -euo pipefail +shopt -s assoc_expand_once +unset YUM_ACTION UPDATEVM shift_amount + +check_template_in_args () { + local pkg + for pkg; do + if [[ "$pkg" =~ ^qubes-template- ]]; then return 0; fi done return 1 @@ -57,9 +56,60 @@ TEMPLATE= TEMPLATE_BACKUP= FORCE_XEN_UPGRADE= REBOOT_REQUIRED= +declare -A options_with_args +options_with_args=( + [--action]= + [--advisory]= + [--advisories]= + [--bz]= + [--bzs]= + [--color]= + [--comment]= + [--cve]= + [--cves]= + [--debuglevel]= + [--disableexcludes]= + [--disableexcludepkgs]= + [--disableplugin]= + [--disablerepo]= + [--downloaddir]= + [--destdir]= + [--errorlevel]= + [--enableplugin]= + [--enablerepo]= + [--exclude]= + [--excludepkgs]= + [--forcearch]= + [--installroot]= + [--releasever]= + [--repofrompath]= + [--repo]= + [--repoid]= + [--rpmverbosity]= + [--sec-severity]= + [--secseverity]= + [--setopt]= +) + +option_takes_arg () { + [[ "$1" =~ ^-[^dexR]*[dexR]$ ]] || [[ -v options_with_args["$1"] ]] +} + # Filter out some dnf options and collect packages list while [ $# -gt 0 ]; do + shift_amount=1 + if option_takes_arg "$1"; then + if [[ "$#" -lt 2 ]]; then + printf 'Missing argument to %s\n' "$1" >&2 + exit 1 + fi + shift_amount=2 + fi case "$1" in + --enablerepo|--disablerepo) + UPDATEVM_OPTS+=("$1=$2") + QVMTEMPLATE_OPTS+=("$1=$2") + ;; --enablerepo=*|\ --disablerepo=*) UPDATEVM_OPTS+=( "$1" ) @@ -92,6 +142,10 @@ while [ $# -gt 0 ]; do --force-xen-upgrade) FORCE_XEN_UPGRADE=1 ;; + --action=) + YUM_ACTION=$1 + UPDATEVM_OPTS+=( "$1=$2" ) + ;; --action=*) YUM_ACTION=${1#--action=} UPDATEVM_OPTS+=( "$1" ) @@ -105,9 +159,9 @@ while [ $# -gt 0 ]; do fi ;; -*) - YUM_OPTS+=( "${1}" ) - UPDATEVM_OPTS+=( "$1" ) - QVMTEMPLATE_OPTS+=( "$1" ) + YUM_OPTS+=( "${@:1:shift_amount}" ) + UPDATEVM_OPTS+=( "${@:1:shift_amount}" ) + QVMTEMPLATE_OPTS+=( "${@:1:shift_amount}" ) ;; *) PKGS+=( "${1}" ) @@ -115,7 +169,7 @@ while [ $# -gt 0 ]; do : "${YUM_ACTION=install}" ;; esac - shift + shift "$shift_amount" done if [[ "$GUI" = 1 ]]; then @@ -137,7 +191,7 @@ case ${YUM_ACTION=upgrade} in esac # Redirect operations on templates to qvm-template command -if find_regex_in_args '^qubes-template-' "${PKGS[@]}"; then +if check_template_in_args "${PKGS[@]}"; then if [[ ${#PKGS[@]} -ne 1 ]]; then echo "ERROR: Specify only one package to reinstall template" exit 2 @@ -201,7 +255,7 @@ if [ "$GUI" == "1" ]; then fi # Do not start VM automatically when running from cron (only checking for updates) -if [ "$CHECK_ONLY" == "1" ] && ! qvm-check -q --running "$UPDATEVM" > /dev/null 2>&1; then +if [ "$CHECK_ONLY" = "1" ] && ! qvm-check -q --running -- "$UPDATEVM" > /dev/null 2>&1; then echo "ERROR: UpdateVM not running, not starting it in non-interactive mode" >&2 exit 1 fi From 66ee22a8bca4631191944f439d42b301a81598b8 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sat, 26 Mar 2022 10:28:02 -0400 Subject: [PATCH 2/3] Fail if options are passed incorrectly The previous commit tried to fix up incorrectly passed options, but that didn't always work. Instead, try to detect known bad options (on a best-effort basis) and print an error message. --- dom0-updates/qubes-dom0-update | 41 +++++++++++++++------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/dom0-updates/qubes-dom0-update b/dom0-updates/qubes-dom0-update index 70a9fe1c..deea2517 100755 --- a/dom0-updates/qubes-dom0-update +++ b/dom0-updates/qubes-dom0-update @@ -1,7 +1,7 @@ #!/bin/bash -- set -euo pipefail shopt -s assoc_expand_once -unset YUM_ACTION UPDATEVM shift_amount +unset YUM_ACTION UPDATEVM check_template_in_args () { local pkg @@ -91,25 +91,24 @@ options_with_args=( [--setopt]= ) -option_takes_arg () { - [[ "$1" =~ ^-[^dexR]*[dexR]$ ]] || [[ -v options_with_args["$1"] ]] -} - # Filter out some dnf options and collect packages list while [ $# -gt 0 ]; do - shift_amount=1 - if option_takes_arg "$1"; then + if [[ -v options_with_args["$1"] ]]; then if [[ "$#" -lt 2 ]]; then - printf 'Missing argument to %s\n' "$1" >&2 - exit 1 - fi - shift_amount=2 + printf 'Missing argument to %s\n' "$1" + else + printf '%s %q must be passed as %s=%q\n' "$1" "$2" "$1" "$2" + fi >&2 + exit 1 + elif [[ "$1" =~ ^-[^dexR]*[dexR]$ ]]; then + if [[ "$#" -lt 2 ]]; then + printf 'Missing argument to %q\n' "$1" + else + printf '%q %q must be written as %q%q\n' "$1" "$2" "$1" "$2" + fi >&2 + exit 1 fi case "$1" in - --enablerepo|--disablerepo) - UPDATEVM_OPTS+=("$1=$2") - QVMTEMPLATE_OPTS+=("$1=$2") - ;; --enablerepo=*|\ --disablerepo=*) UPDATEVM_OPTS+=( "$1" ) @@ -142,10 +141,6 @@ while [ $# -gt 0 ]; do --force-xen-upgrade) FORCE_XEN_UPGRADE=1 ;; - --action=) - YUM_ACTION=$1 - UPDATEVM_OPTS+=( "$1=$2" ) - ;; --action=*) YUM_ACTION=${1#--action=} UPDATEVM_OPTS+=( "$1" ) @@ -159,9 +154,9 @@ while [ $# -gt 0 ]; do fi ;; -*) - YUM_OPTS+=( "${@:1:shift_amount}" ) - UPDATEVM_OPTS+=( "${@:1:shift_amount}" ) - QVMTEMPLATE_OPTS+=( "${@:1:shift_amount}" ) + YUM_OPTS+=( "$1" ) + UPDATEVM_OPTS+=( "$1" ) + QVMTEMPLATE_OPTS+=( "$1" ) ;; *) PKGS+=( "${1}" ) @@ -169,7 +164,7 @@ while [ $# -gt 0 ]; do : "${YUM_ACTION=install}" ;; esac - shift "$shift_amount" + shift done if [[ "$GUI" = 1 ]]; then From 0be9c7ea6c829c96510ea410a3250bd594b73294 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Sun, 3 Apr 2022 18:37:23 -0400 Subject: [PATCH 3/3] Change error to a warning and fixup This changes the error message to a warning and fixes up the user's mistake. --- dom0-updates/qubes-dom0-update | 104 ++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/dom0-updates/qubes-dom0-update b/dom0-updates/qubes-dom0-update index deea2517..1017c26a 100755 --- a/dom0-updates/qubes-dom0-update +++ b/dom0-updates/qubes-dom0-update @@ -58,56 +58,68 @@ FORCE_XEN_UPGRADE= REBOOT_REQUIRED= declare -A options_with_args options_with_args=( - [--action]= - [--advisory]= - [--advisories]= - [--bz]= - [--bzs]= - [--color]= - [--comment]= - [--cve]= - [--cves]= - [--debuglevel]= - [--disableexcludes]= - [--disableexcludepkgs]= - [--disableplugin]= - [--disablerepo]= - [--downloaddir]= - [--destdir]= - [--errorlevel]= - [--enableplugin]= - [--enablerepo]= - [--exclude]= - [--excludepkgs]= - [--forcearch]= - [--installroot]= - [--releasever]= - [--repofrompath]= - [--repo]= - [--repoid]= - [--rpmverbosity]= - [--sec-severity]= - [--secseverity]= - [--setopt]= + [action]= + [advisory]= + [advisories]= + [bz]= + [bzs]= + [color]= + [comment]= + [cve]= + [cves]= + [debuglevel]= + [disableexcludes]= + [disableexcludepkgs]= + [disableplugin]= + [disablerepo]= + [downloaddir]= + [destdir]= + [errorlevel]= + [enableplugin]= + [enablerepo]= + [exclude]= + [excludepkgs]= + [forcearch]= + [installroot]= + [releasever]= + [repofrompath]= + [repo]= + [repoid]= + [rpmverbosity]= + [sec-severity]= + [secseverity]= + [setopt]= ) # Filter out some dnf options and collect packages list while [ $# -gt 0 ]; do - if [[ -v options_with_args["$1"] ]]; then - if [[ "$#" -lt 2 ]]; then - printf 'Missing argument to %s\n' "$1" - else - printf '%s %q must be passed as %s=%q\n' "$1" "$2" "$1" "$2" - fi >&2 - exit 1 - elif [[ "$1" =~ ^-[^dexR]*[dexR]$ ]]; then - if [[ "$#" -lt 2 ]]; then - printf 'Missing argument to %q\n' "$1" - else - printf '%q %q must be written as %q%q\n' "$1" "$2" "$1" "$2" - fi >&2 - exit 1 - fi + case $1 in + (--*) + if [[ -v options_with_args["${1:2}"] ]]; then + if [[ "$#" -lt 2 ]]; then + printf 'Missing argument to %s\n' "$1" + exit 1 + else + printf 'WARNING: %s %q must be passed as %s=%q\n' "$1" "$2" "$1" "$2" + printf 'This will be an error in a future version!\n' + fi >&2 + set -- "$1=$2" "${@:3:$# - 2}" + fi + ;; + (-[!-]*) + if [[ "$1" =~ ^-[^dexR]*[dexR]$ ]]; then + if [[ "$#" -lt 2 ]]; then + printf 'Missing argument to %q\n' "$1" + exit 1 + else + printf 'WARNING: %q %q must be written as %q%q\n' "$1" "$2" "$1" "$2" + printf 'This will be an error in a future version!\n' + fi >&2 + set -- "$1$2" "${@:3:$# - 2}" + fi + ;; + esac + printf %q\\n "$1" case "$1" in --enablerepo=*|\ --disablerepo=*)