From e6d7198e4b16a01f5fd2b9fe62247eb84576d60a Mon Sep 17 00:00:00 2001 From: Daniil Klimuk Date: Mon, 15 Apr 2024 10:38:02 +0200 Subject: [PATCH] fix issues reported after implementing pre-commit hooks For list of pre-commit warnings checkout: https://www.shellcheck.net/wiki/ Signed-off-by: Daniil Klimuk --- include/dts-functions.sh | 41 +++++++++++++++++++++----------------- reports/dasharo-hcl-report | 26 +++++++++++++++--------- scripts/dasharo-deploy | 21 +++++++++++-------- scripts/dts-boot | 1 - 4 files changed, 53 insertions(+), 36 deletions(-) diff --git a/include/dts-functions.sh b/include/dts-functions.sh index 439f865..f28a71b 100644 --- a/include/dts-functions.sh +++ b/include/dts-functions.sh @@ -5,6 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 # shellcheck disable=SC2034 +# shellcheck source=../include/dts-environment.sh source $DTS_ENV ### Color functions @@ -191,7 +192,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS regions - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.5.2 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.5.2 @@ -225,7 +226,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS regions - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.5.2 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.5.2 @@ -258,7 +259,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS regions - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.7.2 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.7.2 @@ -295,7 +296,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS regions - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.7.2 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.7.2 @@ -348,7 +349,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS region - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.1.3 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.1.3 @@ -387,7 +388,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS region - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 1.1.3 if [ $? -eq 1 ]; then # For Dasharo version lesser than 1.1.3 @@ -432,7 +433,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS region - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 0.9.1 if [ $? -eq 1 ]; then # For Dasharo version lesser than 0.9.1 @@ -470,7 +471,7 @@ board_config() { # TODO: Let DTS determine which parameters are suitable. # FIXME: Can we ever get rid of that? We change so much in each release, # that we almost always need to flash whole BIOS region - # because of non-backward compatbile or breaking changes. + # because of non-backward compatible or breaking changes. compare_versions $DASHARO_VERSION 0.9.1 if [ $? -eq 1 ]; then # For Dasharo version lesser than 0.9.1 @@ -682,6 +683,9 @@ check_se_creds() { local _check_dwn_req_resp_uefi="0" local _check_dwn_req_resp_heads="0" local _check_logs_req_resp="0" + # Ignore "SC2154 (warning): SE_credential_file is referenced but not assigned" + # for external variable: + # shellcheck disable=SC2154 CLOUDSEND_LOGS_URL=$(sed -n '1p' < ${SE_credential_file} | tr -d '\n') CLOUDSEND_DOWNLOAD_URL=$(sed -n '2p' < ${SE_credential_file} | tr -d '\n') CLOUDSEND_PASSWORD=$(sed -n '3p' < ${SE_credential_file} | tr -d '\n') @@ -724,10 +728,11 @@ compare_versions() { # that only the major, minor, and patch versions are compared. If the # resulting versions are the same (ver1 equals ver2), then the one without # any suffix is considered to be the newer version. - local ver1="$(echo $1 | awk -F '-' '{print $1}')" - local ver2="$(echo $2 | awk -F '-' '{print $1}')" - local suffix1="$(echo $1 | sed 's/^[0-9]*\.[0-9]*\.[0-9]*-*//')" - local suffix2="$(echo $2 | sed 's/^[0-9]*\.[0-9]*\.[0-9]*-*//')" + local ver1 ver2 suffix1 suffix2 + ver1="$(echo $1 | awk -F '-' '{print $1}')" + ver2="$(echo $2 | awk -F '-' '{print $1}')" + suffix1="$(echo $1 | sed 's/^[0-9]*\.[0-9]*\.[0-9]*-*//')" + suffix2="$(echo $2 | sed 's/^[0-9]*\.[0-9]*\.[0-9]*-*//')" if [[ $ver1 =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] && [[ $ver2 =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then IFS='.' read -r -a arr_ver1 <<< "$ver1" @@ -879,7 +884,7 @@ verify_artifacts() { ;; esac echo -n "Checking $_name firmware checksum... " - sha256sum --check <(echo $(cat $_hash_file | cut -d ' ' -f 1) $_update_file) >> $ERR_LOG_FILE 2>&1 + sha256sum --check <(echo "$(cat $_hash_file | cut -d ' ' -f 1)" $_update_file) >> $ERR_LOG_FILE 2>&1 error_check "Failed to verify $_name firmware checksum" print_green "Verified." if [ -v PLATFORM_SIGN_KEY ]; then @@ -923,19 +928,19 @@ check_blobs_in_binary() { if [ $BOARD_HAS_FD_REGION -ne 0 ]; then ME_OFFSET=$(ifdtool -d $1 2> /dev/null | grep "Flash Region 2 (Intel ME):" | sed 's/Flash Region 2 (Intel ME)\://' |awk '{print $1;}') # Check for IFD signature at offset 0 (old descriptors) - if [ $(tail -c +0 $1|head -c 4|xxd -ps) == "5aa5f00f" ]; then + if [ "$(tail -c +0 $1|head -c 4|xxd -ps)" == "5aa5f00f" ]; then BINARY_HAS_FD=1 fi # Check for IFD signature at offset 16 (new descriptors) - if [ $(tail -c +17 $1|head -c 4|xxd -ps) == "5aa5f00f" ]; then + if [ "$(tail -c +17 $1|head -c 4|xxd -ps)" == "5aa5f00f" ]; then BINARY_HAS_FD=1 fi # Check for ME FPT signature at ME offset + 16 (old ME) - if [ $(tail -c +$((0x$ME_OFFSET + 17)) $1|head -c 4|tr -d '\0') == "\$FPT" ]; then + if [ "$(tail -c +$((0x$ME_OFFSET + 17)) $1|head -c 4|tr -d '\0')" == "\$FPT" ]; then BINARY_HAS_ME=1 fi # Check for aa55 signature at ME offset + 4096 (new ME) - if [ $(tail -c +$((0x$ME_OFFSET + 4097)) $1|head -c 2|xxd -ps) == "aa55" ]; then + if [ "$(tail -c +$((0x$ME_OFFSET + 4097)) $1|head -c 2|xxd -ps)" == "aa55" ]; then BINARY_HAS_ME=1 fi fi @@ -1008,7 +1013,7 @@ force_me_update() { print_warning "You have been warned." while : ; do echo - read -r -p "Skip ME flashing and proceed with BIOS/firmware flashing/udpating? (Y|n) " OPTION + read -r -p "Skip ME flashing and proceed with BIOS/firmware flashing/updating? (Y|n) " OPTION echo case ${OPTION} in diff --git a/reports/dasharo-hcl-report b/reports/dasharo-hcl-report index 9ad90ae..8b6a2c7 100755 --- a/reports/dasharo-hcl-report +++ b/reports/dasharo-hcl-report @@ -4,7 +4,9 @@ # # SPDX-License-Identifier: Apache-2.0 +# shellcheck source=../include/dts-environment.sh source $DTS_ENV +# shellcheck source=../include/dts-functions.sh source $DTS_FUNCS update_result() { @@ -137,8 +139,8 @@ printf '######################## |\r' # Thread is continued here https://github.com/Dasharo/dasharo-issues/issues/247 for t in {1..12} do - SND_HW_FILES=/sys/class/sound/card0/hw*/init_pin_configs - SND_CODEC_FILES=/proc/asound/card0/codec#* + SND_HW_FILES="/sys/class/sound/card0/hw*/init_pin_configs" + SND_CODEC_FILES="/proc/asound/card0/codec#*" SND_HW_FILE=`echo $SND_HW_FILES | cut -d ' ' -f 1` SND_CODEC_FILE=`echo $SND_CODEC_FILES | cut -d ' ' -f 1` @@ -218,7 +220,7 @@ mkdir -p logs/acpi if pushd logs/acpi &> /dev/null; then acpixtract -a ../acpidump.log &>/dev/null iasl -d ./*.dat &>/dev/null - popd &> /dev/null + popd &> /dev/null || return 1 fi update_result "ACPI tables" 0 UNKNOWN printf '###################################### |\r' @@ -239,7 +241,7 @@ update_result "CBMEM table information" logs/cbmem.err.log printf '############################################ |\r' # echo "Getting TPM information..." -find `realpath /sys/class/tpm/tpm*` -type f -print -exec cat {} \; > logs/tpm_version.log 2> logs/tpm_version.err.log +find "$(realpath /sys/class/tpm/tpm*)" -type f -print -exec cat {} \; > logs/tpm_version.log 2> logs/tpm_version.err.log update_result "TPM information" logs/tpm_version.err.log printf '############################################# |\r' @@ -285,14 +287,17 @@ filename+=" $(dmidecode -s bios-version)" # MAC address of device that is used to connect the internet # it could return none only when there is no internet connection but -# in those cases report will be stored locally only -uuid_string=`cat /sys/class/net/$(ip route show default | head -1 | awk '/default/ {print $5}')/address` +# in those cases report will be stored locally only. +# Ignore "SC2046 (warning): Quote this to prevent word splitting" shellcheck +# warning: +# shellcheck disable=SC2046 +uuid_string="$(cat /sys/class/net/$(ip route show default | head -1 | awk '/default/ {print $5}')/address)" # next two values are hardware related so they will be always the same uuid_string+="_$(dmidecode -s system-product-name)" uuid_string+="_$(dmidecode -s system-manufacturer)" # using values from above should generate the same uuid all the time if only -# the MAC address will not change +# the MAC address will not change. uuid=`uuidgen -n @x500 -N $uuid_string -s` filename+="_$uuid" @@ -303,7 +308,10 @@ if [ $DEPLOY_REPORT = "false" ]; then echo "Creating archive with logs..." fi -# remove MAC address from logs as sensitive data +# Remove MAC address from logs as sensitive data. +# Ignore "SC2046 (warning): Quote this to prevent word splitting" shellcheck +# warning: +# shellcheck disable=SC2046 MAC_ADDR=`cat /sys/class/net/$(ip route show default | head -1 | awk '/default/ {print $5}')/address` grep -rl "${MAC_ADDR}" logs > /dev/null && grep -rl "${MAC_ADDR}" logs | xargs sed -i 's/'${MAC_ADDR}'/MAC ADDRESS REMOVED/g' tar -zcf "$filename.tar.gz" logs/* @@ -322,7 +330,7 @@ if [ "$SEND_LOGS" = "true" ]; then fi cloudsend.sh \ ${CLOUDSEND_OPTS} \ - $(readlink -f $filename.tar.gz) \ + "$(readlink -f $filename.tar.gz)" \ ${FULL_UPLOAD_URL} if [ "$?" -ne "0" ]; then echo "Failed to send logs to the cloud" diff --git a/scripts/dasharo-deploy b/scripts/dasharo-deploy index 8196171..a2d98eb 100755 --- a/scripts/dasharo-deploy +++ b/scripts/dasharo-deploy @@ -4,7 +4,9 @@ # # SPDX-License-Identifier: Apache-2.0 +# shellcheck source=../include/dts-environment.sh source $DTS_ENV +# shellcheck source=../include/dts-functions.sh source $DTS_FUNCS [ -z "$BOARD_VENDOR" ] && error_exit "BOARD_VENDOR not given" @@ -112,13 +114,15 @@ display_flashing_warning() { while : ; do echo "Following firmware will be used to install Dasharo" if [ -v BIOS_HASH_LINK ]; then - local _bios_hash="$(cat $BIOS_HASH_FILE | cut -d ' ' -f 1)" + local _bios_hash + _bios_hash="$(cat $BIOS_HASH_FILE | cut -d ' ' -f 1)" echo "Dasharo BIOS firmware:" echo " - link: $BIOS_LINK" echo " - hash: $_bios_hash" fi if [ -v EC_HASH_LINK ]; then - local _ec_hash="$(cat $EC_HASH_FILE | cut -d ' ' -f 1)" + local _ec_hash + _ec_hash="$(cat $EC_HASH_FILE | cut -d ' ' -f 1)" echo "Dasharo EC firmware:" echo " - link: $EC_LINK" echo " - hash: $_ec_hash" @@ -508,7 +512,7 @@ update() { fi if [ -v HAVE_HEADS_FW ] && [ "$DASHARO_FLAVOR" != "Dasharo (coreboot+heads)" ]; then # Check if given SE credentials give access to heads, if not, - # then it means DES is for regualar releases + # then it means DES is for regular releases curl -sfI -u "$USER_DETAILS" -H "$CLOUD_REQUEST" "$HEADS_LINK_DES" -o /dev/null if [ $? -ne 0 ]; then print_warning "Dasharo Heads firmware version is available, but your" @@ -534,10 +538,12 @@ update() { echo "Latest available Dasharo version: $DASHARO_REL_VER" fi BIOS_HASH_LINK=$BIOS_HASH_LINK_COMM + # shellcheck disable=SC2034 BIOS_SIGN_LINK=$BIOS_SIGN_LINK_COMM BIOS_LINK=$BIOS_LINK_COMM if [ "$HAVE_EC" == "true" ]; then EC_HASH_LINK=$EC_HASH_LINK_COMM + # shellcheck disable=SC2034 EC_SIGN_LINK=$EC_SIGN_LINK_COMM EC_LINK=$EC_LINK_COMM fi @@ -662,7 +668,7 @@ update() { fi fi if [ $UPDATE_ME -eq 0 ]; then - UPDATE_STRING+="Managment Engine" + UPDATE_STRING+="Management Engine" fi echo "Updating $UPDATE_STRING" $FLASHROM -p "$PROGRAMMER_BIOS" ${FLASH_CHIP_SELECT} ${FLASHROM_ADD_OPT_REGIONS} -w "$BIOS_UPDATE_FILE" >> $FLASHROM_LOG_FILE 2>> $ERR_LOG_FILE @@ -682,8 +688,7 @@ update() { print_warning "This is expected. Run OEM Factory Reset / Re-Ownership to finish deploying Heads." ;; esac - echo "Press any key to continue" # Make sure the user acknowledges. - read -n 1 -r input + read -p "Press enter to continue" # Make sure the user acknowledges. else # Regular update flow print_green "Successfully updated Dasharo firmware." @@ -736,7 +741,7 @@ restore() { # HCL report should be named as in dasharo-hcl-report so we can find # the package based on uuid saved in name, we need to check two options # with and without MAC address - uuid_string=`cat /sys/class/net/$(ip route show default | awk '/default/ {print $5}')/address` + uuid_string="$(cat /sys/class/net/$(ip route show default | awk '/default/ {print $5}')/address)" # if above gives error then there is no internet connection and first # part of uuid should be blank if [ ! $? -eq 0 ]; then @@ -796,7 +801,7 @@ restore() { ${CMD_CLOUD_LIST} $uuid error_check "Could not download HCL report from cloud." - HCL_REPORT_PACKAGE="$(find / -name *$uuid* | head -n1)" + HCL_REPORT_PACKAGE="$(find / -name "*$uuid*" | head -n1)" tar -zxf "$HCL_REPORT_PACKAGE" -C /tmp echo "Restoring BIOS firmware..." if [ -f "/tmp/logs/rom.bin" ]; then diff --git a/scripts/dts-boot b/scripts/dts-boot index 798d6bb..7e5ad1e 100644 --- a/scripts/dts-boot +++ b/scripts/dts-boot @@ -15,4 +15,3 @@ if [ -d $FUM_EFIVAR ]; then else $SBIN_DIR/dts fi -