Fix more shellcheck warnings (#6155)

.github/workflows/test.yml

@@ -19,6 +19,8 @@ jobs:
     steps:
       - name: Checkout repository
         uses: actions/checkout@v4.2.2
+        with:
+          fetch-depth: 0 # Differential ShellCheck requires full git history
 
       - name: Check scripts in repository are executable
         run: |
@@ -28,12 +30,12 @@ jobs:
           # If FAIL is 1 then we fail.
           [[ $FAIL == 1 ]] && exit 1 || echo "Scripts are executable!"
 
-      - name: Run shellcheck
+      - name: Differential ShellCheck
-        uses: ludeeus/action-shellcheck@master
+        uses: redhat-plumbers-in-action/differential-shellcheck@v5
         with:
-          check_together: 'yes'
-          format: tty
           severity: warning
+          display-engine: sarif-fmt
+
 
       - name: Spell-Checking
         uses: codespell-project/actions-codespell@master

.shellcheckrc

@@ -1 +1,2 @@
-disable=SC1090,SC1091 # Ignore warnings about being unable to follow sourced files
+external-sources=true # allow shellcheck to read external sources
+disable=SC3043 #disable SC3043: In POSIX sh, local is undefined.

advanced/Scripts/api.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env sh
-# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)

advanced/Scripts/list.sh

@@ -11,9 +11,11 @@
 
 readonly PI_HOLE_SCRIPT_DIR="/opt/pihole"
 readonly utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source="./advanced/Scripts/utils.sh"
 source "${utilsfile}"
 
 readonly apifile="${PI_HOLE_SCRIPT_DIR}/api.sh"
+# shellcheck source="./advanced/Scripts/api.sh"
 source "${apifile}"
 
 # Determine database location
@@ -38,6 +40,7 @@ typeId=""
 comment=""
 
 colfile="/opt/pihole/COL_TABLE"
+# shellcheck source="./advanced/Scripts/COL_TABLE"
 source ${colfile}
 
 helpFunc() {

advanced/Scripts/piholeARPTable.sh

@@ -11,11 +11,13 @@
 
 coltable="/opt/pihole/COL_TABLE"
 if [[ -f ${coltable} ]]; then
+# shellcheck source="./advanced/Scripts/COL_TABLE"
     source ${coltable}
 fi
 
 readonly PI_HOLE_SCRIPT_DIR="/opt/pihole"
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source=./advanced/Scripts/utils.sh
 source "${utilsfile}"
 
 # Determine database location

advanced/Scripts/piholeCheckout.sh

@@ -9,8 +9,8 @@
 # Please see LICENSE file for your rights under this license.
 
 readonly PI_HOLE_FILES_DIR="/etc/.pihole"
-# shellcheck disable=SC2034
 SKIP_INSTALL="true"
+# shellcheck source="./automated install/basic-install.sh"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 
 # webInterfaceGitUrl set in basic-install.sh
@@ -60,7 +60,6 @@ checkout() {
         exit 1;
     fi
 
-    # shellcheck disable=SC2154
     if ! is_repo "${webInterfaceDir}" ; then
         echo -e "  ${COL_LIGHT_RED}Error: Web Admin repo is missing from system!"
         echo -e "  Please re-run install script from https://github.com/pi-hole/pi-hole${COL_NC}"
@@ -105,7 +104,6 @@ checkout() {
         echo "master" > /etc/pihole/ftlbranch
         chmod 644 /etc/pihole/ftlbranch
     elif [[ "${1}" == "core" ]] ; then
-        # shellcheck disable=SC2154
         str="Fetching branches from ${piholeGitUrl}"
         echo -ne "  ${INFO} $str"
         if ! fully_fetch_repo "${PI_HOLE_FILES_DIR}" ; then
@@ -133,7 +131,6 @@ checkout() {
         fi
         checkout_pull_branch "${PI_HOLE_FILES_DIR}" "${2}"
     elif [[ "${1}" == "web" ]] ; then
-        # shellcheck disable=SC2154
         str="Fetching branches from ${webInterfaceGitUrl}"
         echo -ne "  ${INFO} $str"
         if ! fully_fetch_repo "${webInterfaceDir}" ; then
@@ -218,7 +215,7 @@ checkout() {
             if [ $status -eq 1 ]; then
                 # Binary for requested branch is not available, may still be
                 # int he process of being built or CI build job failed
-                printf "  %b Binary for requested branch is not available, please try again later.\\n" ${CROSS}
+                printf "  %b Binary for requested branch is not available, please try again later.\\n" "${CROSS}"
                 printf "      If the issue persists, please contact Pi-hole Support and ask them to re-generate the binary.\\n"
                 exit 1
             elif [ $status -eq 2 ]; then

advanced/Scripts/piholeDebug.sh

@@ -8,7 +8,6 @@
 # This file is copyright under the latest version of the EUPL.
 # Please see LICENSE file for your rights under this license.
 
-# shellcheck source=/dev/null
 
 # -e option instructs bash to immediately exit if any command [1] has a non-zero exit status
 # -u a reference to any variable you haven't previously defined
@@ -27,6 +26,7 @@ PIHOLE_COLTABLE_FILE="${PIHOLE_SCRIPTS_DIRECTORY}/COL_TABLE"
 
 # These provide the colors we need for making the log more readable
 if [[ -f ${PIHOLE_COLTABLE_FILE} ]]; then
+# shellcheck source=./advanced/Scripts/COL_TABLE
     source ${PIHOLE_COLTABLE_FILE}
 else
     COL_NC='\e[0m' # No Color
@@ -41,7 +41,7 @@ else
     #OVER="\r\033[K"
 fi
 
-# shellcheck disable=SC1091
+# shellcheck source=/dev/null
 . /etc/pihole/versions
 
 # Read the value of an FTL config key. The value is printed to stdout.
@@ -213,7 +213,7 @@ compare_local_version_to_git_version() {
             local local_status
             local_status=$(git status -s)
             # echo this information out to the user in a nice format
-            if [ ${local_version} ]; then
+            if [ "${local_version}" ]; then
               log_write "${TICK} Version: ${local_version}"
             elif [ -n "${DOCKER_VERSION}" ]; then
               log_write "${TICK} Version: Pi-hole Docker Container ${COL_BOLD}${DOCKER_VERSION}${COL_NC}"
@@ -488,7 +488,9 @@ run_and_print_command() {
     local output
     output=$(${cmd} 2>&1)
     # If the command was successful,
-    if [[ $? -eq 0 ]]; then
+    local return_code
+    return_code=$?
+    if [[ "${return_code}" -eq 0 ]]; then
         # show the output
         log_write "${output}"
     else
@@ -933,7 +935,6 @@ parse_file() {
     # Get the lines that are in the file(s) and store them in an array for parsing later
     local file_info
     if [[ -f "$filename" ]]; then
-        #shellcheck disable=SC2016
         IFS=$'\r\n' command eval 'file_info=( $(cat "${filename}") )'
     else
         read -r -a file_info <<< "$filename"

advanced/Scripts/piholeLogFlush.sh

@@ -9,10 +9,12 @@
 # Please see LICENSE file for your rights under this license.
 
 colfile="/opt/pihole/COL_TABLE"
+# shellcheck source="./advanced/Scripts/COL_TABLE"
 source ${colfile}
 
 readonly PI_HOLE_SCRIPT_DIR="/opt/pihole"
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source="./advanced/Scripts/utils.sh"
 source "${utilsfile}"
 
 # In case we're running at the same time as a system logrotate, use a

advanced/Scripts/query.sh

@@ -1,10 +1,5 @@
 #!/usr/bin/env sh
 
-
-# Ignore warning about `local` being undefinded in POSIX
-# shellcheck disable=SC3043
-# https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
-
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2023 Pi-hole, LLC (https://pi-hole.net)
 # Network-wide ad blocking via your own hardware.
@@ -22,9 +17,11 @@ domain=""
 
 # Source color table
 colfile="/opt/pihole/COL_TABLE"
+# shellcheck source="./advanced/Scripts/COL_TABLE"
 . "${colfile}"
 
 # Source api functions
+# shellcheck source="./advanced/Scripts/api.sh"
 . "${PI_HOLE_INSTALL_DIR}/api.sh"
 
 Help() {

advanced/Scripts/update.sh

@@ -15,16 +15,16 @@ readonly ADMIN_INTERFACE_GIT_URL="https://github.com/pi-hole/web.git"
 readonly PI_HOLE_GIT_URL="https://github.com/pi-hole/pi-hole.git"
 readonly PI_HOLE_FILES_DIR="/etc/.pihole"
 
-# shellcheck disable=SC2034
 SKIP_INSTALL=true
 
 # when --check-only is passed to this script, it will not perform the actual update
 CHECK_ONLY=false
 
+# shellcheck source="./automated install/basic-install.sh"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
-# shellcheck disable=SC1091
+# shellcheck source=./advanced/Scripts/COL_TABLE
 source "/opt/pihole/COL_TABLE"
-# shellcheck disable=SC1091
+# shellcheck source="./advanced/Scripts/utils.sh"
 source "${PI_HOLE_INSTALL_DIR}/utils.sh"
 
 # is_repo() sourced from basic-install.sh

advanced/Scripts/updatecheck.sh

@@ -39,7 +39,7 @@ function get_remote_hash() {
 }
 
 # Source the utils file for addOrEditKeyValPair()
-# shellcheck disable=SC1091
+# shellcheck source="./advanced/Scripts/utils.sh"
 . /opt/pihole/utils.sh
 
 ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")

advanced/Scripts/utils.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env sh
-# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)

advanced/Scripts/version.sh

@@ -8,18 +8,16 @@
 # This file is copyright under the latest version of the EUPL.
 # Please see LICENSE file for your rights under this license.
 
-# Ignore warning about `local` being undefinded in POSIX
-# shellcheck disable=SC3043
-# https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
-
 # Source the versions file populated by updatechecker.sh
 cachedVersions="/etc/pihole/versions"
 
 if [ -f ${cachedVersions} ]; then
+    # shellcheck source=/dev/null
     . "$cachedVersions"
 else
     echo "Could not find /etc/pihole/versions. Running update now."
     pihole updatechecker
+     # shellcheck source=/dev/null
     . "$cachedVersions"
 fi
 

advanced/Templates/pihole-FTL-poststop.sh

@@ -3,6 +3,7 @@
 # Source utils.sh for getFTLConfigValue()
 PI_HOLE_SCRIPT_DIR='/opt/pihole'
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source="./advanced/Scripts/utils.sh"
 . "${utilsfile}"
 
 # Get file paths

advanced/Templates/pihole-FTL-prestart.sh

@@ -3,6 +3,7 @@
 # Source utils.sh for getFTLConfigValue()
 PI_HOLE_SCRIPT_DIR='/opt/pihole'
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source="./advanced/Scripts/utils.sh"
 . "${utilsfile}"
 
 # Get file paths

automated install/basic-install.sh

@@ -153,6 +153,7 @@ done
 # If the color table file exists,
 if [[ -f "${coltable}" ]]; then
     # source it
+    # shellcheck source="./advanced/Scripts/COL_TABLE"
     source "${coltable}"
 # Otherwise,
 else
@@ -768,7 +769,6 @@ chooseInterface() {
             # All further interfaces are deselected
             status="OFF"
         done
-        # shellcheck disable=SC2086
         # Disable check for double quote here as we are passing a string with spaces
         PIHOLE_INTERFACE=$(dialog --no-shadow --keep-tite --output-fd 1 \
             --cancel-label "Exit" --ok-label "Select" \
@@ -1871,7 +1871,6 @@ clone_or_reset_repos() {
 
 # Download FTL binary to random temp directory and install FTL binary
 # Disable directive for SC2120 a value _can_ be passed to this function, but it is passed from an external script that sources this one
-# shellcheck disable=SC2120
 FTLinstall() {
     # Local, named variables
     local str="Downloading and Installing FTL"
@@ -2400,7 +2399,7 @@ main() {
 
     # /opt/pihole/utils.sh should be installed by installScripts now, so we can use it
     if [ -f "${PI_HOLE_INSTALL_DIR}/utils.sh" ]; then
-        # shellcheck disable=SC1091
+        # shellcheck source="./advanced/Scripts/utils.sh"
         source "${PI_HOLE_INSTALL_DIR}/utils.sh"
     else
         printf "  %b Failure: /opt/pihole/utils.sh does not exist .\\n" "${CROSS}"

automated install/uninstall.sh

@@ -8,7 +8,9 @@
 # This file is copyright under the latest version of the EUPL.
 # Please see LICENSE file for your rights under this license.
 
+# shellcheck source="./advanced/Scripts/COL_TABLE"
 source "/opt/pihole/COL_TABLE"
+# shellcheck source="./advanced/Scripts/utils.sh"
 source "/opt/pihole/utils.sh"
 
 ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
@@ -40,8 +42,8 @@ else
 fi
 
 readonly PI_HOLE_FILES_DIR="/etc/.pihole"
-# shellcheck disable=SC2034
 SKIP_INSTALL="true"
+# shellcheck source="./automated install/basic-install.sh"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 
 # package_manager_detect() sourced from basic-install.sh

gravity.sh

@@ -15,11 +15,13 @@ export LC_ALL=C
 PI_HOLE_SCRIPT_DIR="/opt/pihole"
 # Source utils.sh for GetFTLConfigValue
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source=./advanced/Scripts/utils.sh
 . "${utilsfile}"
 
 coltable="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
+# shellcheck source=./advanced/Scripts/COL_TABLE
 . "${coltable}"
-# shellcheck disable=SC1091
+# shellcheck source=./advanced/Scripts/database_migration/gravity-db.sh
 . "/etc/.pihole/advanced/Scripts/database_migration/gravity-db.sh"
 
 basename="pihole"
@@ -767,8 +769,7 @@ gravity_DownloadBlocklistFromUrl() {
   fi
 
   if [[ "${download}" == true ]]; then
-    # shellcheck disable=SC2086
+    httpCode=$(curl --connect-timeout ${curl_connect_timeout} -s -L "${compression}" "${cmd_ext}" "${modifiedOptions}" -w "%{http_code}" "${url}" -o "${listCurlBuffer}" 2>/dev/null)
-    httpCode=$(curl --connect-timeout ${curl_connect_timeout} -s -L ${compression} ${cmd_ext} ${modifiedOptions} -w "%{http_code}" "${url}" -o "${listCurlBuffer}" 2>/dev/null)
   fi
 
   case $url in

pihole

@@ -17,13 +17,16 @@ readonly PI_HOLE_SCRIPT_DIR="/opt/pihole"
 PI_HOLE_BIN_DIR="/usr/local/bin"
 
 readonly colfile="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
+# shellcheck source=./advanced/Scripts/COL_TABLE
 source "${colfile}"
 
 readonly utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+# shellcheck source=./advanced/Scripts/utils.sh
 source "${utilsfile}"
 
 # Source api functions
 readonly apifile="${PI_HOLE_SCRIPT_DIR}/api.sh"
+# shellcheck source=./advanced/Scripts/api.sh
 source "${apifile}"
 
 versionsfile="/etc/pihole/versions"
@@ -31,6 +34,7 @@ if [ -f "${versionsfile}" ]; then
     # Only source versionsfile if the file exits
     # fixes a warning during installation where versionsfile does not exist yet
     # but gravity calls `pihole -status` and thereby sourcing the file
+    # shellcheck source=/dev/null
     source "${versionsfile}"
 fi
 
@@ -427,6 +431,7 @@ piholeCheckoutFunc() {
       exit 0
     fi
 
+    #shellcheck source=./advanced/Scripts/piholeCheckout.sh
     source "${PI_HOLE_SCRIPT_DIR}"/piholeCheckout.sh
     shift
     checkout "$@"