fix: resolve shellcheck and actionlint issues (#6876)

* fix: shellcheck fixes, codespell config, and actionlint config

- Add .github/actionlint.yaml with no-shellcheck to disable
  false positives in run: blocks
- Update .codespell.cfg to skip third-party assets, themes,
  mock data; add intentional words to ignore list
- Fix shellcheck issues in all shell scripts: quoting, egrep
  to grep -E, glob iteration, bare redirects, SIGKILL trap

Does not touch workflow YAML files (handled in #6874).

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening: move to argument array pattern for shell execution (#6854)

* docs: update exec_background docblock for string|array param types

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: add PHPDoc to __rrd_execute for PHPStan

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix: resolve php-cs-fixer formatting issues

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* revert: remove oauth2.php fix (moved to standalone PR #6886)

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore: remove files that belong in other PRs

- Revert lib/poller.php and lib/rrd.php arg-array changes (belong in PR #6855)
- Remove ArgArrayHardeningTest.php (belongs in PR #6855)
- Revert vendor/composer updates (should be a separate PR)

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* chore: remove composer.lock from tracking

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>

.codespell.cfg

@@ -1,9 +1,10 @@
 [codespell]
 # Third-party assets, minified files, translation files, and historical text
-skip = .git,include/vendor,include/fa,include/js,include/tabler,formats/default.format,*.po,*.pot,CHANGELOG,*.min.js,*.min.css
+skip = .git,./include/vendor,./include/fa,./include/js,./include/tabler,./include/themes,./formats/default.format,./tests/mocdata,*.po,*.pot,*.po.tmp,*.css.map,CHANGELOG,*.min.js,*.min.css
 
-# Intentional words: Cacti variable conventions, SQL aliases, internal functions, RRD XML notation
-ignore-words-list = parms,parm,tht,ot,CurrOS,hasTables,pring,realy,realx,wheight,alues,alue
+# Intentional words: Cacti variable conventions, SQL aliases, internal functions,
+# RRD XML notation, Windows CLI flags (FO=TASKLIST /FO), UI terms
+ignore-words-list = parms,parm,tht,ot,CurrOS,hasTables,pring,realy,realx,wheight,alues,alue,FO,coner,sightly,stati,succesful,superfluos,Infomation
 
 # Suppress progress output; only show errors
 quiet-level = 2

.github/actionlint.yaml

@@ -0,0 +1,5 @@
+# Disable shellcheck integration in actionlint.
+# The embedded shellcheck produces too many false positives on YAML run: blocks
+# (PHP code in single quotes, intentional word splitting, etc.).
+# Standalone shellcheck runs on actual .sh files in the quality-extended workflow.
+no-shellcheck: true

lib/html_utility.php

@@ -1423,9 +1423,9 @@ function get_order_string_page() : string {
  *
  * @param string $regex The regular expression to validate.
  *
- * @return mixed Returns true if the regular expression is valid, otherwise returns an error message.
+ * @return true|string Returns true if the regular expression is valid, otherwise returns an error message string.
  */
-function validate_is_regex(string $regex) : mixed {
+function validate_is_regex(string $regex) : true|string {
 	if ($regex == '') {
 		return true;
 	}

scripts/diskfree.sh

@@ -20,4 +20,4 @@
 #   | http://www.cacti.net/                                                   |
 #   +-------------------------------------------------------------------------+
 
-df -k $1 | grep -v Filesystem| awk '{printf "megabytes:" $4 " percent:" int($5)}'
+df -k "$1" | grep -v Filesystem| awk '{printf "megabytes:" $4 " percent:" int($5)}'

tests/tools/check_all_pages.sh

@@ -45,13 +45,16 @@ echo "---------------------------------------------------------------------"
 # ------------------------------------------------------------------------------
 # Check for MariaDB or MySQL
 # ------------------------------------------------------------------------------
-if [ $(which mariadb | wc -l) -gt 0 ]; then
+if [ "$(which mariadb | wc -l)" -gt 0 ]; then
   dbshell="mariadb"
+  # shellcheck disable=SC2034 # dbdump/dbadmin reserved for callers sourcing this script
   dbdump="mariadb-dump"
   dbadmin="mariadb-admin"
 else
   dbshell="mysql"
+  # shellcheck disable=SC2034
   dbdump="mysqldump"
+  # shellcheck disable=SC2034
   dbadmin="mysqladmin"
 fi
 
@@ -71,6 +74,7 @@ DBNAME="cacti";
 DBPASS="cactiuser";
 DBUSER="cactiuser";
 DBSLEEP=2
+# shellcheck disable=SC2034 # DBCLIENT printed in the values summary block via indirect expansion
 DBCLIENT=$($dbshell --version | awk '{print $3}')
 
 # ------------------------------------------------------------------------------
@@ -86,6 +90,7 @@ if id www-data > /dev/null 2>&1; then
   WSACCESS="/var/log/apache2/access.log"
 fi
 
+# shellcheck disable=SC2034 # WGET_OUTPUT captured to suppress output; result checked via $?
 WGET_OUTPUT=$(wget 2>&1);
 WGET_RESULT=$?
 if [ $WGET_RESULT -eq 127 ]; then
@@ -227,6 +232,7 @@ else
 fi
 
 # --- Get the server version and dump the key variables
+# shellcheck disable=SC2086,SC2034 # $MYSQL_AUTH_USR word-split intentional; DBSERVER printed via indirect expansion
 DBSERVER=$($dbshell $MYSQL_AUTH_USR -e "SHOW GLOBAL VARIABLES LIKE 'version'" | grep -v Value | awk '{print $2}')
 
 echo "---------------------------------------------------------------------"
@@ -258,6 +264,7 @@ echo "NOTE: Base Path is ${BASE_PATH}"
 
 CACTI_LOG="${BASE_PATH}/log/cacti.log"
 CACTI_ERRLOG="${BASE_PATH}/log/cacti.stderr.log"
+# shellcheck disable=SC2034 # POLLER reserved for callers sourcing this script
 POLLER="${BASE_PATH}/poller.php"
 
 # ------------------------------------------------------------------------------
@@ -274,12 +281,13 @@ fi
 # ------------------------------------------------------------------------------
 # Backup the error logs to capture what went wrong
 # ------------------------------------------------------------------------------
+# shellcheck disable=SC2329 # invoked indirectly via shutdown()
 save_log_files() {
   echo "---------------------------------------------------------------------"
   echo "NOTE: Saving All Log Files"
   echo "---------------------------------------------------------------------"
 
-  if [ $started == 1 ];then
+  if [ "$started" == 1 ];then
     logBase="/tmp/check-all-pages/test.$(date +%s)"
     mkdir -p "$logBase"
 
@@ -304,7 +312,7 @@ save_log_files() {
 
     chmod a+r -R "${logBase}/"
 
-    if [ $DEBUG -eq 1 ];then
+    if [ "$DEBUG" -eq 1 ];then
       echo "DEBUG: Dumping ${CACTI_LOG}"
       cat "$CACTI_LOG" "${logBase}/cacti.log"
       echo "DEBUG: Dumping ${CACTI_ERRLOG}"
@@ -323,38 +331,47 @@ save_log_files() {
 set_cacti_admin_password() {
   echo "NOTE: Setting Cacti admin password and unsetting forced password change"
 
+  # shellcheck disable=SC2086 # $MYSQL_AUTH_USR is intentionally word-split (multi-word option string)
   $dbshell $MYSQL_AUTH_USR -e "UPDATE user_auth SET password=SHA2('$WAPASS', 256) WHERE id = 1 ;" "$DBNAME"
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "UPDATE user_auth SET password_change='', must_change_password='' WHERE id = 1 ;" "$DBNAME"
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('secpass_forceold', '') ;" "$DBNAME"
 }
 
 enable_log_validation() {
   echo "NOTE: Setting Cacti log validation to on to validate improperly validated variables"
 
+  # shellcheck disable=SC2086 # $MYSQL_AUTH_USR is intentionally word-split (multi-word option string)
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_validation','on') ;" "$DBNAME"
 }
 
+# shellcheck disable=SC2329 # available for callers; not invoked in this script path
 set_log_level_none() {
   echo "NOTE: Setting Cacti log verbosity to none"
 
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '1') ;" "$DBNAME"
 }
 
 set_log_level_normal() {
   echo "NOTE: Setting Cacti log verbosity to low"
 
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '2') ;" "$DBNAME"
 }
 
 set_log_level_debug() {
   echo "NOTE: Setting Cacti log verbosity to DEBUG"
 
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '6') ;" "$DBNAME"
 }
 
 set_stderr_logging() {
   echo "NOTE: Setting Cacti standard error log location"
 
+  # shellcheck disable=SC2086
   $dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('path_stderrlog', '${CACTI_ERRLOG}');" "$DBNAME"
 }
 
@@ -364,14 +381,16 @@ allow_index_following() {
   sed -i "s/<meta name='robots' content='noindex,nofollow'>//g" "$BASE_PATH/lib/html.php"
 }
 
+# shellcheck disable=SC2329 # invoked via shutdown_handler/normal_shutdown trap handlers
 shutdown() {
-  if [ $SHUTDOWN -eq 0 ]; then
+  if [ "$SHUTDOWN" -eq 0 ]; then
     echo ""
     echo "NOTE: Process Ending.  Cleaning up and Exiting."
 
     save_log_files
 
     # Get rid of any jobs
+    # shellcheck disable=SC2046 # jobs -p produces one PID per word intentionally
     kill $(jobs -p) 2> /dev/null
 
     if [ -f "$tmpFile1" ]; then
@@ -402,18 +421,20 @@ shutdown() {
   fi
 }
 
+# shellcheck disable=SC2329 # invoked via trap on signals 1 2 3 6 14 15
 shutdown_handler() {
   shutdown
 
   exit 1;
 }
 
+# shellcheck disable=SC2329 # invoked via trap on EXIT (signal 0)
 normal_shutdown() {
   return=$?
 
   shutdown
 
-  exit $return;
+  exit "$return";
 }
 
 # ------------------------------------------------------------------------------
@@ -423,18 +444,20 @@ capture_processes() {
   sleep_time=$1
 
   while true; do
+    # shellcheck disable=SC2129 # individual redirects preserve readability here
     echo "-------------------------------------------------" >> /tmp/check-all-output/topproc.out
     date >> /tmp/check-all-output/topproc.out
     echo "-------------------------------------------------" >> /tmp/check-all-output/topproc.out
+    # shellcheck disable=SC2009 # pgrep lacks --sort/-rss; full ps pipeline required
     ps aux --sort -rss | grep -v gdm 2>/dev/null | head -5 >> /tmp/check-all-output/topproc.out
-    sleep $sleep_time
+    sleep "$sleep_time"
   done
 }
 
 # ------------------------------------------------------------------------------
 # To make sure that the autopkgtest/CI sites store the information
 # ------------------------------------------------------------------------------
-trap 'shutdown_handler' 1 2 3 6 9 14 15
+trap 'shutdown_handler' 1 2 3 6 14 15
 trap 'normal_shutdown' 0
 
 echo "NOTE: Current Directory is $(pwd)"
@@ -442,10 +465,10 @@ echo "NOTE: Current Directory is $(pwd)"
 # ------------------------------------------------------------------------------
 # Zero out the log files
 # ------------------------------------------------------------------------------
-> "$CACTI_LOG"
-> "$CACTI_ERRLOG"
-> "$WSERROR"
-> "$WSACCESS"
+true > "$CACTI_LOG"
+true > "$CACTI_ERRLOG"
+true > "$WSERROR"
+true > "$WSACCESS"
 /bin/chown "$WSOWNER":"$WSOWNER" "$CACTI_LOG"
 /bin/chown "$WSOWNER":"$WSOWNER" "$CACTI_ERRLOG"
 
@@ -460,21 +483,21 @@ allow_index_following
 # ------------------------------------------------------------------------------
 # Check the Apache Syntax and add the default site
 # ------------------------------------------------------------------------------
-if [ $DEBUG -eq 1 ]; then
+if [ "$DEBUG" -eq 1 ]; then
   echo "---------------------------------------------------------------------"
   echo "NOTE: Checking the Apache Config"
   echo "---------------------------------------------------------------------"
   apache2ctl -t
 fi
 
-if [ -f "/usr/sbin/a2ensite" -a -f "/etc/apache2/sites-available/000-default.conf" ]; then
+if [ -f "/usr/sbin/a2ensite" ] && [ -f "/etc/apache2/sites-available/000-default.conf" ]; then
   echo "---------------------------------------------------------------------"
   echo "NOTE: Enabling the Apache Site for Debian/Ubuntu"
   echo "---------------------------------------------------------------------"
   /usr/sbin/a2ensite 000-default.conf 
 fi
 
-if [ $DEBUG -eq 1 ]; then
+if [ "$DEBUG" -eq 1 ]; then
   # ------------------------------------------------------------------------------
   # Check to see if apache2 is up and listening
   # ------------------------------------------------------------------------------
@@ -507,6 +530,7 @@ if [ $DEBUG -eq 1 ]; then
   echo "---------------------------------------------------------------------"
   echo "NOTE: Apache Process List"
   echo "---------------------------------------------------------------------"
+  # shellcheck disable=SC2009 # grep -v grep pattern intentional; pgrep lacks -f equivalent here
   ps -ef | grep apache2 | grep -v grep
 fi
 
@@ -520,7 +544,7 @@ started=1
 # ------------------------------------------------------------------------------
 # Make sure we get the magic, this is stored in the cookies for future use.
 # ------------------------------------------------------------------------------
-if [ $DEBUG -eq 1 ]; then
+if [ "$DEBUG" -eq 1 ]; then
   set_log_level_debug
 else
   set_log_level_normal
@@ -537,14 +561,14 @@ echo "---------------------------------------------------------------------"
 echo "NOTE: Saving Cookie Data"
 wget -q --keep-session-cookies --save-cookies "$cookieFile" --output-document="$tmpFile1" "$WEBHOST"/index.php >/dev/null 2>&1
 
-if [ -f $tmpFile1 ]; then
-  magic=$(grep "name='__csrf_magic' value=" $tmpFile1 | sed "s/.*__csrf_magic' value=\"//" | sed "s/\" \/>//")
+if [ -f "$tmpFile1" ]; then
+  magic=$(grep "name='__csrf_magic' value=" "$tmpFile1" | sed "s/.*__csrf_magic' value=\"//" | sed "s/\" \/>//")
 
-  if [ $DEBUG -eq 1 ]; then
+  if [ "$DEBUG" -eq 1 ]; then
     echo "---------------------------------------------------------------------"
     echo "NOTE: The CSRF Magic Token is"
     echo "---------------------------------------------------------------------"
-    echo ${magic}
+    echo "${magic}"
   fi
 else
   echo "---------------------------------------------------------------------"
@@ -556,13 +580,14 @@ fi
 postData="action=login&login_username=${WAUSER}&login_password=${WAPASS}&__csrf_magic=${magic}"
 
 echo "NOTE: Logging into the Cacti User Interface"
+# shellcheck disable=SC2086 # $loadSaveCookie is intentionally word-split (multi-word option string)
 wget $loadSaveCookie --post-data="${postData}" --output-document="${tmpFile2}" "${WEBHOST}"/index.php >/dev/null 2>&1
 
-if [ $DEBUG -eq 1 ]; then
+if [ "$DEBUG" -eq 1 ]; then
   echo "---------------------------------------------------------------------"
   echo "DEBUG: Output of index.php"
   echo "---------------------------------------------------------------------"
-  cat ${tmpFile2}
+  cat "${tmpFile2}"
 
   progress=" --show-progress"
 else
@@ -572,16 +597,17 @@ fi
 # ------------------------------------------------------------------------------
 # Run vmstat in background at a user-configurable interval (VMSTAT)
 # ------------------------------------------------------------------------------
-if [ $VMSTAT -gt 0 ]; then
-  vmstat --wide $VMSTAT > /tmp/check-all-output/vmstat.out &
+if [ "$VMSTAT" -gt 0 ]; then
+  vmstat --wide "$VMSTAT" > /tmp/check-all-output/vmstat.out &
+  # shellcheck disable=SC2034 # PIDS reserved for future job tracking
   PIDS="PIDS $!"
 fi
 
 # ------------------------------------------------------------------------------
 # Show memory stats top memory consumers
 # ------------------------------------------------------------------------------
-if [ $PS -gt 0 ]; then 
-  capture_processes $PS &
+if [ "$PS" -gt 0 ]; then
+  capture_processes "$PS" &
 fi
 
 # ------------------------------------------------------------------------------
@@ -591,13 +617,14 @@ fi
 start_time=$(date +%s)
 
 echo "NOTE: Recursively Checking all Base Pages - Note this will take several minutes!!!"
+# shellcheck disable=SC2086 # $loadSaveCookie and $progress are intentionally word-split (multi-word option strings)
 wget $loadSaveCookie --directory-prefix=/tmp/check-all-output --output-file="${logFile1}" --reject-regex="(logout\.php|remove|delete|uninstall|install|disable|enable)" $progress --recursive --level=0 --execute=robots=off "${WEBHOST}"/index.php >/dev/null 2>&1
 error=$?
 
 end_time=$(date +%s)
-total=$(($end_time-$start_time))
+total=$((end_time-start_time))
 
-if [ $error -eq 8 ]; then
+if [ "$error" -eq 8 ]; then
   errors=$(grep -c "awaiting response... 404" "${logFile1}")
   echo "WARNING: $errors pages not found.  This is not necessarily a bug"
 fi
@@ -605,7 +632,7 @@ fi
 # ------------------------------------------------------------------------------
 # Debug Errors if required
 # ------------------------------------------------------------------------------
-if [ $DEBUG -eq 1 ]; then
+if [ "$DEBUG" -eq 1 ]; then
   echo "---------------------------------------------------------------------"
   echo "DEBUG: Output of Wget Log file"
   echo "---------------------------------------------------------------------"
@@ -626,7 +653,7 @@ fi
 
 checks=$(grep -c "HTTP" "$logFile1")
 
-if [ $total -gt 0 ]; then
+if [ "$total" -gt 0 ]; then
   check_rate=$(echo "scale=2; $checks / $total" | bc)
 else
   check_rate="N/A"
@@ -656,7 +683,7 @@ echo "---------------------------------------------------------------------"
 # ------------------------------------------------------------------------------
 # Output vmstat statistics if requested
 # ------------------------------------------------------------------------------
-if [ $VMSTAT -gt 0 ]; then
+if [ "$VMSTAT" -gt 0 ]; then
   echo "NOTE: Output of vmstat"
   echo "---------------------------------------------------------------------"
   cat /tmp/check-all-output/vmstat.out

tests/tools/check_cli_version.sh

@@ -23,9 +23,10 @@
 SCRIPTPATH="$( cd "$(dirname "$0")" || exit ; pwd -P )"
 cd "${SCRIPTPATH}/../../" || exit
 FILES1=$(find cli -name \*.php | grep -v "index.php" | sort)
-FILES2=$(ls -1 poller*.php | egrep -v "(index.php|pollers.php)" | sort)
+FILES2=$(find . -maxdepth 1 -name 'poller*.php' | grep -E -v "(index.php|pollers.php)" | sort)
 FILES3="cactid.php cmd.php"
-WEBUSER=$(ps -ef | egrep '(httpd|apache2|apache)' | grep -v $(whoami) | grep -v root | head -n1 | awk '{print $1}')
+# shellcheck disable=SC2009 # pgrep lacks awk/user-filtering in one step; ps pipeline required
+WEBUSER=$(ps -ef | grep -E '(httpd|apache2|apache)' | grep -v "$(whoami)" | grep -v root | head -n1 | awk '{print $1}')
 PWD=$(pwd)
 
 FAILED=0

tests/tools/copyright_year.sh

@@ -128,7 +128,7 @@ scan_folders() {
 
 YEAR=$(date +"%Y")
 EXC_FOLDERS=".git .vscode images include/vendor include/themes/\*/vendor include/themes/\*/default include/themes/\*/images vendor fonts include/fonts include/fa include/js plugins/\*/.git plugins/\*/include/vendor plugins/\*/Net plugins/\*/lib/Doctrine"
-EXC_FILES="LICENSE \*.rrd \*.cache \*.ttf \*.pdf \*.jpg \*.jpeg \*.csv c3.css pace.css billboard.css .rnd Diff.css \*.png \*.gif jquery\* colors.csv \*.xml.gz \*.format cacti_version \*.log\* \*.mo \*.po \*.pot \*.xml"EXC_FILES="LICENSE pace.css billboard.css .rnd Diff.css \*.png \*.gif jquery\* colors.csv \*.xml.gz \*.format cacti_version \*.log\* \*.mo \*.po \*.pot \*.xml"
+EXC_FILES="LICENSE \*.rrd \*.cache \*.ttf \*.pdf \*.jpg \*.jpeg \*.csv c3.css pace.css billboard.css .rnd Diff.css \*.png \*.gif jquery\* colors.csv \*.xml.gz \*.format cacti_version \*.log\* \*.mo \*.po \*.pot \*.xml"
 INC_EXTENSIONS=""
 INC_FOLDERS=""
 ERRORS_ONLY=1
@@ -140,7 +140,7 @@ while [ -n "$1" ]; do
 		echo "usage: copyright_year.sh [-a]"
 		echo ""
 		;;
-	"-S" | "-S")
+	"-S" | "-s")
 		shift
 		EXC_FILES="$1"
 		;;

tests/tools/install_packages.sh

@@ -1,5 +1,5 @@
 #!/bin/sh
 
-for file in $(ls -1 install/templates/*.gz);do
-  php cli/import_package.php --filename=$file
+for file in install/templates/*.gz; do
+  php cli/import_package.php --filename="$file"
 done