Merge pull request #763 from BastilleBSD/shellcheck

shellcheck: general linting and github action

.github/workflows/shellcheck.yml

@@ -3,7 +3,7 @@ name: ShellCheck Linting
 on:
   pull_request:
     branches:
-      - main
+      - master
 
 jobs:
   lint:
@@ -15,8 +15,18 @@ jobs:
         uses: actions/checkout@v4
 
       - name: Run ShellCheck
-        uses: ludeeus/action-shellcheckudeeus/[email protected]
+        uses: ludeeus/[email protected]
+        env:
+          # Excluding SC3043: In POSIX sh, 'local' is undefined. Ignoring because local is a built-in command in FreeBSD
+          # Excluding SC2154: Variable is referenced but not assigned. Because we include files in the scripts
+          # Excluding SC3037: In POSIX sh, echo flags are undefined. Ignoring temporarily until we decide to keep it or
+          #                   use printf instead
+          # Excluding SC2155: Declare and assign separately to avoid masking return values.
+          # Excluding SC2124: Assigning an array to a string! Check instead if this is a false positive or if there is
+          #                   a better way to do it.
+          SHELLCHECK_OPTS: -e SC3043 -e SC2154 -e SC3037 -e SC2155 -e SC2124
         with:
+          severity: warning
           scandir: "./usr/local/share/bastille"
           additional_files: "./usr/local/bin/bastille"
-          ignore_paths: "./usr/local/share/bastille/templates"
+          ignore_paths: "./usr/local/share/bastille/templates ./usr/local/share/bastille/colors.pre.sh"

README.md

@@ -97,7 +97,7 @@ Use "bastille command -h|--help" for more information about a command.
 
 ```
 
-## 0.10-beta
+## 0.12-beta
 This document outlines the basic usage of the Bastille container management
 framework. This release is still considered beta.
 

usr/local/share/bastille/bootstrap.sh

@@ -216,6 +216,8 @@ bootstrap_release() {
     if [ -f "${bastille_releasesdir}/${RELEASE}/COPYRIGHT" ]; then
         ## check distfiles list and skip existing cached files
         bastille_bootstrap_archives=$(echo "${bastille_bootstrap_archives}" | sed "s/base//")
+        # TODO check how to handle this
+        # shellcheck disable=SC2010
         bastille_cached_files=$(ls "${bastille_cachedir}/${RELEASE}" | grep -v "MANIFEST" | tr -d ".txz")
         for distfile in ${bastille_cached_files}; do
             bastille_bootstrap_archives=$(echo "${bastille_bootstrap_archives}" | sed "s/${distfile}//")
@@ -452,7 +454,7 @@ HW_MACHINE_ARCH=$(sysctl hw.machine_arch | awk '{ print $2 }')
 
 # bootstrapping from aarch64/arm64 Debian or Ubuntu require a different value for ARCH
 # create a new variable
-if [ "${HW_MACHINE_ARCH}" == "aarch64" ]; then
+if [ "${HW_MACHINE_ARCH}" = "aarch64" ]; then
     HW_MACHINE_ARCH_LINUX="arm64"
 else
     HW_MACHINE_ARCH_LINUX=${HW_MACHINE_ARCH}

usr/local/share/bastille/clone.sh

@@ -58,6 +58,7 @@ validate_ip() {
     if [ -n "${ip6}" ]; then
         info "Valid: (${ip6})."
         IPX_ADDR="ip6.addr"
+        # shellcheck disable=SC2034
         IP6_MODE="new"
     else
         local IFS
@@ -104,8 +105,8 @@ update_jailconf_vnet() {
     bastille_jail_rc_conf="${bastille_jailsdir}/${NEWNAME}/root/etc/rc.conf"
 
     # Determine number of containers and define an uniq_epair
-    local list_jails_num=$(bastille list jails | wc -l | awk '{print $1}')
-    local num_range=$(expr "${list_jails_num}" + 1)
+    local list_jails_num="$(bastille list jails | wc -l | awk '{print $1}')"
+    local num_range="$(expr "${list_jails_num}" + 1)"
     jail_list=$(bastille list jail)
     for _num in $(seq 0 "${num_range}"); do
         if [ -n "${jail_list}" ]; then
@@ -125,7 +126,7 @@ update_jailconf_vnet() {
     sed -i '' "s|ifconfig_e0b_bastille.*_name|ifconfig_e0b_${uniq_epair}_name|" "${bastille_jail_rc_conf}"
 
     # If 0.0.0.0 set DHCP, else set static IP address
-    if [ "${IP}" == "0.0.0.0" ]; then
+    if [ "${IP}" = "0.0.0.0" ]; then
         sysrc -f "${bastille_jail_rc_conf}" ifconfig_vnet0="SYNCDHCP"
     else
         sysrc -f "${bastille_jail_rc_conf}" ifconfig_vnet0="inet ${IP}"

usr/local/share/bastille/common.sh

@@ -47,7 +47,7 @@ enable_color() {
 
 # If "NO_COLOR" environment variable is present, or we aren't speaking to a
 # tty, disable output colors.
-if [ -z "${NO_COLOR}" -a -t 1 ]; then
+if [ -z "${NO_COLOR}" ] && [ -t 1 ]; then
     enable_color
 fi
 
@@ -77,9 +77,9 @@ generate_vnet_jail_netblock() {
     ## determine number of containers + 1
     ## iterate num and grep all jail configs
     ## define uniq_epair
-    local jail_list=$(bastille list jails)
+    local jail_list="$(bastille list jails)"
     if [ -n "${jail_list}" ]; then
-        local list_jails_num=$(echo "${jail_list}" | wc -l | awk '{print $1}')
+        local list_jails_num="$(echo "${jail_list}" | wc -l | awk '{print $1}')"
         local num_range=$((list_jails_num + 1))
         for _num in $(seq 0 "${num_range}"); do
             if ! grep -q "e[0-9]b_bastille${_num}" "${bastille_jailsdir}"/*/jail.conf; then

usr/local/share/bastille/convert.sh

@@ -114,7 +114,7 @@ start_convert() {
         HASPORTS=$(grep -w ${bastille_releasesdir}/${RELEASE}/usr/ports ${bastille_jailsdir}/${TARGET}/fstab)
 
         if [ -n "${RELEASE}" ]; then
-            cd "${bastille_jailsdir}/${TARGET}/root"
+            cd "${bastille_jailsdir}/${TARGET}/root" || error_exit "Failed to change directory to ${bastille_jailsdir}/${TARGET}/root"
 
             # Work with the symlinks
             convert_symlinks
@@ -149,6 +149,8 @@ fi
 # Be interactive here since this cannot be easily undone
 while :; do
     error_notify "Warning: container conversion from thin to thick can't be undone!"
+    # shellcheck disable=SC2162
+    # shellcheck disable=SC3045
     read -p "Do you really wish to convert '${TARGET}' into a thick container? [y/N]:" yn
     case ${yn} in
     [Yy]) start_convert;;

usr/local/share/bastille/create.sh

@@ -60,7 +60,7 @@ running_jail() {
 
 validate_name() {
     local NAME_VERIFY=${NAME}
-    local NAME_SANITY=$(echo "${NAME_VERIFY}" | tr -c -d 'a-zA-Z0-9-_')
+    local NAME_SANITY="$(echo "${NAME_VERIFY}" | tr -c -d 'a-zA-Z0-9-_')"
     if [ -n "$(echo "${NAME_SANITY}" | awk "/^[-_].*$/" )" ]; then
         error_exit "Container names may not begin with (-|_) characters!"
     elif [ "${NAME_VERIFY}" != "${NAME_SANITY}" ]; then
@@ -123,7 +123,7 @@ validate_ips() {
 }
 
 validate_netif() {
-    local LIST_INTERFACES=$(ifconfig -l)
+    local LIST_INTERFACES="$(ifconfig -l)"
     if echo "${LIST_INTERFACES} VNET" | grep -qwo "${INTERFACE}"; then
         info "Valid: (${INTERFACE})."
     else
@@ -253,7 +253,7 @@ post_create_jail() {
 
     # Using relative paths here.
     # MAKE SURE WE'RE IN THE RIGHT PLACE.
-    cd "${bastille_jail_path}"
+    cd "${bastille_jail_path}" || error_exit "Failed to change directory."
     echo
 
     if [ ! -f "${bastille_jail_conf}" ]; then
@@ -292,7 +292,9 @@ create_jail() {
     bastille_jail_fstab="${bastille_jailsdir}/${NAME}/fstab"  ## file
     bastille_jail_conf="${bastille_jailsdir}/${NAME}/jail.conf"  ## file
     bastille_jail_log="${bastille_logsdir}/${NAME}_console.log"  ## file
+    # shellcheck disable=SC2034
     bastille_jail_rc_conf="${bastille_jailsdir}/${NAME}/root/etc/rc.conf" ## file
+    # shellcheck disable=SC2034
     bastille_jail_resolv_conf="${bastille_jailsdir}/${NAME}/root/etc/resolv.conf" ## file
 
     if [ ! -d "${bastille_jailsdir}/${NAME}" ]; then
@@ -409,8 +411,10 @@ create_jail() {
                         info "Creating a clonejail...\n"
                         ## clone the release base to the new basejail
                         SNAP_NAME="bastille-clone-$(date +%Y-%m-%d-%H%M%S)"
+                        # shellcheck disable=SC2140
                         zfs snapshot "${bastille_zfs_zpool}/${bastille_zfs_prefix}/releases/${RELEASE}"@"${SNAP_NAME}"
 
+                        # shellcheck disable=SC2140
                         zfs clone -p "${bastille_zfs_zpool}/${bastille_zfs_prefix}/releases/${RELEASE}"@"${SNAP_NAME}" \
                         "${bastille_zfs_zpool}/${bastille_zfs_prefix}/jails/${NAME}/root"
 
@@ -425,16 +429,20 @@ create_jail() {
 
                         ## take a temp snapshot of the base release
                         SNAP_NAME="bastille-$(date +%Y-%m-%d-%H%M%S)"
+                        # shellcheck disable=SC2140
                         zfs snapshot "${bastille_zfs_zpool}/${bastille_zfs_prefix}/releases/${RELEASE}"@"${SNAP_NAME}"
 
                         ## replicate the release base to the new thickjail and set the default mountpoint
+                        # shellcheck disable=SC2140
                         zfs send -R "${bastille_zfs_zpool}/${bastille_zfs_prefix}/releases/${RELEASE}"@"${SNAP_NAME}" | \
                         zfs receive "${bastille_zfs_zpool}/${bastille_zfs_prefix}/jails/${NAME}/root"
                         zfs set ${ZFS_OPTIONS} mountpoint=none "${bastille_zfs_zpool}/${bastille_zfs_prefix}/jails/${NAME}/root"
                         zfs inherit mountpoint "${bastille_zfs_zpool}/${bastille_zfs_prefix}/jails/${NAME}/root"
 
                         ## cleanup temp snapshots initially
+                        # shellcheck disable=SC2140
                         zfs destroy "${bastille_zfs_zpool}/${bastille_zfs_prefix}/releases/${RELEASE}"@"${SNAP_NAME}"
+                        # shellcheck disable=SC2140
                         zfs destroy "${bastille_zfs_zpool}/${bastille_zfs_prefix}/jails/${NAME}/root"@"${SNAP_NAME}"
                     fi
 
@@ -608,7 +616,9 @@ esac
 bastille_root_check
 
 if echo "$3" | grep '@'; then
+    # shellcheck disable=SC2034
     BASTILLE_JAIL_IP=$(echo "$3" | awk -F@ '{print $2}')
+    # shellcheck disable=SC2034
     BASTILLE_JAIL_INTERFACES=$( echo "$3" | awk -F@ '{print $1}')
 fi
 
@@ -691,7 +701,7 @@ while [ $# -gt 0 ]; do
             VNET_JAIL_BRIDGE="1"
             shift
             ;;
-        -*|--*)
+        --*|-*)
             error_notify "Unknown Option."
             usage
             ;;

usr/local/share/bastille/destroy.sh

@@ -172,7 +172,7 @@ destroy_rel() {
             if [ "${FORCE}" = "1" ]; then
                 ## remove cache on force
                 if [ -d "${bastille_cachedir}/${TARGET}" ]; then
-                    rm -rf "${bastille_cachedir}/${TARGET}"
+                    rm -rf "${bastille_cachedir:?}/${TARGET}"
                 fi
             fi
             echo

usr/local/share/bastille/edit.sh

@@ -51,6 +51,7 @@ fi
 bastille_root_check
 
 if [ -z "${EDITOR}" ]; then
+    # shellcheck disable=SC2209
     EDITOR=vi
 fi
 

usr/local/share/bastille/export.sh

@@ -76,6 +76,7 @@ bastille_root_check
 zfs_enable_check() {
     # Temporarily disable ZFS so we can create a standard backup archive
     if checkyesno bastille_zfs_enable; then
+        # shellcheck disable=SC2034
         bastille_zfs_enable="NO"
     fi
 }
@@ -135,7 +136,7 @@ if [ -n "${bastille_export_options}" ]; then
             --verbose)
                 OPT_ZSEND="-Rv"
                 shift;;
-            -*|--*) error_notify "Unknown Option."
+            --*|-*) error_notify "Unknown Option."
                 usage;;
         esac
     done
@@ -185,7 +186,7 @@ else
                 TARGET="${2}"
                 shift
                 ;;
-            -*|--*)
+            --*|-*)
                 error_notify "Unknown Option."
                 usage
                 ;;
@@ -208,12 +209,16 @@ if [ "${COMP_OPTION}" -gt "1" ]; then
     error_exit "Error: Only one compression format can be used during export."
 fi
 
-if [ -n "${TXZ_EXPORT}" -o -n "${TGZ_EXPORT}" ] && [ -n "${SAFE_EXPORT}" ]; then
+if { [ -n "${TXZ_EXPORT}" ] || [ -n "${TGZ_EXPORT}" ]; } && [ -n "${SAFE_EXPORT}" ]; then
     error_exit "Error: Simple archive modes with safe ZFS export can't be used together."
 fi
 
 if ! checkyesno bastille_zfs_enable; then
-    if [ -n "${XZ_EXPORT}" -o -n "${GZIP_EXPORT}" -o -n "${RAW_EXPORT}" -o -n "${SAFE_EXPORT}" -o "${OPT_ZSEND}" = "-Rv" ]; then
+    if [ -n "${XZ_EXPORT}" ] ||
+       [ -n "${GZIP_EXPORT}" ] ||
+       [ -n "${RAW_EXPORT}" ] ||
+       [ -n "${SAFE_EXPORT}" ] ||
+       [ "${OPT_ZSEND}" = "-Rv" ]; then
         error_exit "Options --xz, --gz, --raw, --safe, --verbose are valid for ZFS configured systems only."
     fi
 fi
@@ -270,7 +275,7 @@ export_check() {
             EXPORT_AS="Exporting"
         fi
 
-        if [ "${FILE_EXT}" = ".xz" -o "${FILE_EXT}" = ".gz" -o "${FILE_EXT}" = "" ]; then
+        if [ "${FILE_EXT}" = ".xz" ] || [ "${FILE_EXT}" = ".gz" ] || [ "${FILE_EXT}" = "" ]; then
             EXPORT_TYPE="image"
         else
             EXPORT_TYPE="archive"
@@ -360,12 +365,13 @@ jail_export() {
         fi
     fi
 
+    # shellcheck disable=SC2181
     if [ "$?" -ne 0 ]; then
         error_exit "Failed to export '${TARGET}' container."
     else
         if [ -z "${USER_EXPORT}" ]; then
             # Generate container checksum file
-            cd "${bastille_backupsdir}"
+            cd "${bastille_backupsdir}" || error_exit "Failed to change directory."
             sha256 -q "${TARGET}_${DATE}${FILE_EXT}" > "${TARGET}_${DATE}.sha256"
             info "Exported '${bastille_backupsdir}/${TARGET}_${DATE}${FILE_EXT}' successfully."
         fi

usr/local/share/bastille/import.sh

@@ -79,7 +79,7 @@ while [ $# -gt 0 ]; do
             TARGET="${2}"
             shift
             ;;
-        -*|--*)
+        --*|-*)
             error_notify "Unknown Option."
             usage
             ;;
@@ -281,7 +281,7 @@ EOF
         >> "${bastille_jailsdir}/${TARGET_TRIM}/fstab"
 
         # Work with the symlinks
-        cd "${bastille_jailsdir}/${TARGET_TRIM}/root"
+        cd "${bastille_jailsdir}/${TARGET_TRIM}/root" || error_exit "Failed to change directory."
         update_symlinks
     else
         # Generate new empty fstab file
@@ -324,7 +324,7 @@ update_config() {
     >> "${bastille_jailsdir}/${TARGET_TRIM}/fstab"
 
     # Work with the symlinks
-    cd "${bastille_jailsdir}/${TARGET_TRIM}/root"
+    cd "${bastille_jailsdir}/${TARGET_TRIM}/root" || error_exit "Failed to change directory."
     update_symlinks
 }
 
@@ -377,7 +377,7 @@ update_symlinks() {
     for _link in ${SYMLINKS}; do
         if [ -L "${_link}" ]; then
             ln -sf /.bastille/${_link} ${_link}
-        elif [ "${ALLOW_EMPTY_DIRS_TO_BE_SYMLINKED:-0}" = "1" -a -d "${_link}" ]; then
+        elif [ "${ALLOW_EMPTY_DIRS_TO_BE_SYMLINKED:-0}" = "1" ] && [ -d "${_link}" ]; then
             # -F will enforce that the directory is empty and replaced by the symlink
             ln -sfF /.bastille/${_link} ${_link} || EXIT_CODE=$?
             if [ "${EXIT_CODE:-0}" != "0" ]; then