Skip to content

Commit 76b2b69

Browse files
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>
1 parent b4cb892 commit 76b2b69

File tree

8 files changed

+76
-42
lines changed

8 files changed

+76
-42
lines changed

.codespell.cfg

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
[codespell]
22
# Third-party assets, minified files, translation files, and historical text
3-
skip = .git,include/vendor,include/fa,include/js,include/tabler,formats/default.format,*.po,*.pot,CHANGELOG,*.min.js,*.min.css
3+
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
44

5-
# Intentional words: Cacti variable conventions, SQL aliases, internal functions, RRD XML notation
6-
ignore-words-list = parms,parm,tht,ot,CurrOS,hasTables,pring,realy,realx,wheight,alues,alue
5+
# Intentional words: Cacti variable conventions, SQL aliases, internal functions,
6+
# RRD XML notation, Windows CLI flags (FO=TASKLIST /FO), UI terms
7+
ignore-words-list = parms,parm,tht,ot,CurrOS,hasTables,pring,realy,realx,wheight,alues,alue,FO,coner,sightly,stati,succesful,superfluos,Infomation
78

89
# Suppress progress output; only show errors
910
quiet-level = 2

.github/actionlint.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Disable shellcheck integration in actionlint.
2+
# The embedded shellcheck produces too many false positives on YAML run: blocks
3+
# (PHP code in single quotes, intentional word splitting, etc.).
4+
# Standalone shellcheck runs on actual .sh files in the quality-extended workflow.
5+
no-shellcheck: true

lib/html_utility.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1423,9 +1423,9 @@ function get_order_string_page() : string {
14231423
*
14241424
* @param string $regex The regular expression to validate.
14251425
*
1426-
* @return mixed Returns true if the regular expression is valid, otherwise returns an error message.
1426+
* @return true|string Returns true if the regular expression is valid, otherwise returns an error message string.
14271427
*/
1428-
function validate_is_regex(string $regex) : mixed {
1428+
function validate_is_regex(string $regex) : true|string {
14291429
if ($regex == '') {
14301430
return true;
14311431
}

scripts/diskfree.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@
2020
# | http://www.cacti.net/ |
2121
# +-------------------------------------------------------------------------+
2222

23-
df -k $1 | grep -v Filesystem| awk '{printf "megabytes:" $4 " percent:" int($5)}'
23+
df -k "$1" | grep -v Filesystem| awk '{printf "megabytes:" $4 " percent:" int($5)}'

tests/tools/check_all_pages.sh

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,16 @@ echo "---------------------------------------------------------------------"
4545
# ------------------------------------------------------------------------------
4646
# Check for MariaDB or MySQL
4747
# ------------------------------------------------------------------------------
48-
if [ $(which mariadb | wc -l) -gt 0 ]; then
48+
if [ "$(which mariadb | wc -l)" -gt 0 ]; then
4949
dbshell="mariadb"
50+
# shellcheck disable=SC2034 # dbdump/dbadmin reserved for callers sourcing this script
5051
dbdump="mariadb-dump"
5152
dbadmin="mariadb-admin"
5253
else
5354
dbshell="mysql"
55+
# shellcheck disable=SC2034
5456
dbdump="mysqldump"
57+
# shellcheck disable=SC2034
5558
dbadmin="mysqladmin"
5659
fi
5760

@@ -71,6 +74,7 @@ DBNAME="cacti";
7174
DBPASS="cactiuser";
7275
DBUSER="cactiuser";
7376
DBSLEEP=2
77+
# shellcheck disable=SC2034 # DBCLIENT printed in the values summary block via indirect expansion
7478
DBCLIENT=$($dbshell --version | awk '{print $3}')
7579

7680
# ------------------------------------------------------------------------------
@@ -86,6 +90,7 @@ if id www-data > /dev/null 2>&1; then
8690
WSACCESS="/var/log/apache2/access.log"
8791
fi
8892

93+
# shellcheck disable=SC2034 # WGET_OUTPUT captured to suppress output; result checked via $?
8994
WGET_OUTPUT=$(wget 2>&1);
9095
WGET_RESULT=$?
9196
if [ $WGET_RESULT -eq 127 ]; then
@@ -227,6 +232,7 @@ else
227232
fi
228233

229234
# --- Get the server version and dump the key variables
235+
# shellcheck disable=SC2086,SC2034 # $MYSQL_AUTH_USR word-split intentional; DBSERVER printed via indirect expansion
230236
DBSERVER=$($dbshell $MYSQL_AUTH_USR -e "SHOW GLOBAL VARIABLES LIKE 'version'" | grep -v Value | awk '{print $2}')
231237

232238
echo "---------------------------------------------------------------------"
@@ -258,6 +264,7 @@ echo "NOTE: Base Path is ${BASE_PATH}"
258264

259265
CACTI_LOG="${BASE_PATH}/log/cacti.log"
260266
CACTI_ERRLOG="${BASE_PATH}/log/cacti.stderr.log"
267+
# shellcheck disable=SC2034 # POLLER reserved for callers sourcing this script
261268
POLLER="${BASE_PATH}/poller.php"
262269

263270
# ------------------------------------------------------------------------------
@@ -274,12 +281,13 @@ fi
274281
# ------------------------------------------------------------------------------
275282
# Backup the error logs to capture what went wrong
276283
# ------------------------------------------------------------------------------
284+
# shellcheck disable=SC2329 # invoked indirectly via shutdown()
277285
save_log_files() {
278286
echo "---------------------------------------------------------------------"
279287
echo "NOTE: Saving All Log Files"
280288
echo "---------------------------------------------------------------------"
281289

282-
if [ $started == 1 ];then
290+
if [ "$started" == 1 ];then
283291
logBase="/tmp/check-all-pages/test.$(date +%s)"
284292
mkdir -p "$logBase"
285293

@@ -304,7 +312,7 @@ save_log_files() {
304312

305313
chmod a+r -R "${logBase}/"
306314

307-
if [ $DEBUG -eq 1 ];then
315+
if [ "$DEBUG" -eq 1 ];then
308316
echo "DEBUG: Dumping ${CACTI_LOG}"
309317
cat "$CACTI_LOG" "${logBase}/cacti.log"
310318
echo "DEBUG: Dumping ${CACTI_ERRLOG}"
@@ -323,38 +331,47 @@ save_log_files() {
323331
set_cacti_admin_password() {
324332
echo "NOTE: Setting Cacti admin password and unsetting forced password change"
325333

334+
# shellcheck disable=SC2086 # $MYSQL_AUTH_USR is intentionally word-split (multi-word option string)
326335
$dbshell $MYSQL_AUTH_USR -e "UPDATE user_auth SET password=SHA2('$WAPASS', 256) WHERE id = 1 ;" "$DBNAME"
336+
# shellcheck disable=SC2086
327337
$dbshell $MYSQL_AUTH_USR -e "UPDATE user_auth SET password_change='', must_change_password='' WHERE id = 1 ;" "$DBNAME"
338+
# shellcheck disable=SC2086
328339
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('secpass_forceold', '') ;" "$DBNAME"
329340
}
330341

331342
enable_log_validation() {
332343
echo "NOTE: Setting Cacti log validation to on to validate improperly validated variables"
333344

345+
# shellcheck disable=SC2086 # $MYSQL_AUTH_USR is intentionally word-split (multi-word option string)
334346
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_validation','on') ;" "$DBNAME"
335347
}
336348

349+
# shellcheck disable=SC2329 # available for callers; not invoked in this script path
337350
set_log_level_none() {
338351
echo "NOTE: Setting Cacti log verbosity to none"
339352

353+
# shellcheck disable=SC2086
340354
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '1') ;" "$DBNAME"
341355
}
342356

343357
set_log_level_normal() {
344358
echo "NOTE: Setting Cacti log verbosity to low"
345359

360+
# shellcheck disable=SC2086
346361
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '2') ;" "$DBNAME"
347362
}
348363

349364
set_log_level_debug() {
350365
echo "NOTE: Setting Cacti log verbosity to DEBUG"
351366

367+
# shellcheck disable=SC2086
352368
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('log_verbosity', '6') ;" "$DBNAME"
353369
}
354370

355371
set_stderr_logging() {
356372
echo "NOTE: Setting Cacti standard error log location"
357373

374+
# shellcheck disable=SC2086
358375
$dbshell $MYSQL_AUTH_USR -e "REPLACE INTO settings (name, value) VALUES ('path_stderrlog', '${CACTI_ERRLOG}');" "$DBNAME"
359376
}
360377

@@ -364,14 +381,16 @@ allow_index_following() {
364381
sed -i "s/<meta name='robots' content='noindex,nofollow'>//g" "$BASE_PATH/lib/html.php"
365382
}
366383

384+
# shellcheck disable=SC2329 # invoked via shutdown_handler/normal_shutdown trap handlers
367385
shutdown() {
368-
if [ $SHUTDOWN -eq 0 ]; then
386+
if [ "$SHUTDOWN" -eq 0 ]; then
369387
echo ""
370388
echo "NOTE: Process Ending. Cleaning up and Exiting."
371389

372390
save_log_files
373391

374392
# Get rid of any jobs
393+
# shellcheck disable=SC2046 # jobs -p produces one PID per word intentionally
375394
kill $(jobs -p) 2> /dev/null
376395

377396
if [ -f "$tmpFile1" ]; then
@@ -402,18 +421,20 @@ shutdown() {
402421
fi
403422
}
404423

424+
# shellcheck disable=SC2329 # invoked via trap on signals 1 2 3 6 14 15
405425
shutdown_handler() {
406426
shutdown
407427

408428
exit 1;
409429
}
410430

431+
# shellcheck disable=SC2329 # invoked via trap on EXIT (signal 0)
411432
normal_shutdown() {
412433
return=$?
413434

414435
shutdown
415436

416-
exit $return;
437+
exit "$return";
417438
}
418439

419440
# ------------------------------------------------------------------------------
@@ -423,29 +444,31 @@ capture_processes() {
423444
sleep_time=$1
424445

425446
while true; do
447+
# shellcheck disable=SC2129 # individual redirects preserve readability here
426448
echo "-------------------------------------------------" >> /tmp/check-all-output/topproc.out
427449
date >> /tmp/check-all-output/topproc.out
428450
echo "-------------------------------------------------" >> /tmp/check-all-output/topproc.out
451+
# shellcheck disable=SC2009 # pgrep lacks --sort/-rss; full ps pipeline required
429452
ps aux --sort -rss | grep -v gdm 2>/dev/null | head -5 >> /tmp/check-all-output/topproc.out
430-
sleep $sleep_time
453+
sleep "$sleep_time"
431454
done
432455
}
433456

434457
# ------------------------------------------------------------------------------
435458
# To make sure that the autopkgtest/CI sites store the information
436459
# ------------------------------------------------------------------------------
437-
trap 'shutdown_handler' 1 2 3 6 9 14 15
460+
trap 'shutdown_handler' 1 2 3 6 14 15
438461
trap 'normal_shutdown' 0
439462

440463
echo "NOTE: Current Directory is $(pwd)"
441464

442465
# ------------------------------------------------------------------------------
443466
# Zero out the log files
444467
# ------------------------------------------------------------------------------
445-
> "$CACTI_LOG"
446-
> "$CACTI_ERRLOG"
447-
> "$WSERROR"
448-
> "$WSACCESS"
468+
true > "$CACTI_LOG"
469+
true > "$CACTI_ERRLOG"
470+
true > "$WSERROR"
471+
true > "$WSACCESS"
449472
/bin/chown "$WSOWNER":"$WSOWNER" "$CACTI_LOG"
450473
/bin/chown "$WSOWNER":"$WSOWNER" "$CACTI_ERRLOG"
451474

@@ -460,21 +483,21 @@ allow_index_following
460483
# ------------------------------------------------------------------------------
461484
# Check the Apache Syntax and add the default site
462485
# ------------------------------------------------------------------------------
463-
if [ $DEBUG -eq 1 ]; then
486+
if [ "$DEBUG" -eq 1 ]; then
464487
echo "---------------------------------------------------------------------"
465488
echo "NOTE: Checking the Apache Config"
466489
echo "---------------------------------------------------------------------"
467490
apache2ctl -t
468491
fi
469492

470-
if [ -f "/usr/sbin/a2ensite" -a -f "/etc/apache2/sites-available/000-default.conf" ]; then
493+
if [ -f "/usr/sbin/a2ensite" ] && [ -f "/etc/apache2/sites-available/000-default.conf" ]; then
471494
echo "---------------------------------------------------------------------"
472495
echo "NOTE: Enabling the Apache Site for Debian/Ubuntu"
473496
echo "---------------------------------------------------------------------"
474497
/usr/sbin/a2ensite 000-default.conf
475498
fi
476499

477-
if [ $DEBUG -eq 1 ]; then
500+
if [ "$DEBUG" -eq 1 ]; then
478501
# ------------------------------------------------------------------------------
479502
# Check to see if apache2 is up and listening
480503
# ------------------------------------------------------------------------------
@@ -507,6 +530,7 @@ if [ $DEBUG -eq 1 ]; then
507530
echo "---------------------------------------------------------------------"
508531
echo "NOTE: Apache Process List"
509532
echo "---------------------------------------------------------------------"
533+
# shellcheck disable=SC2009 # grep -v grep pattern intentional; pgrep lacks -f equivalent here
510534
ps -ef | grep apache2 | grep -v grep
511535
fi
512536

@@ -520,7 +544,7 @@ started=1
520544
# ------------------------------------------------------------------------------
521545
# Make sure we get the magic, this is stored in the cookies for future use.
522546
# ------------------------------------------------------------------------------
523-
if [ $DEBUG -eq 1 ]; then
547+
if [ "$DEBUG" -eq 1 ]; then
524548
set_log_level_debug
525549
else
526550
set_log_level_normal
@@ -537,14 +561,14 @@ echo "---------------------------------------------------------------------"
537561
echo "NOTE: Saving Cookie Data"
538562
wget -q --keep-session-cookies --save-cookies "$cookieFile" --output-document="$tmpFile1" "$WEBHOST"/index.php >/dev/null 2>&1
539563

540-
if [ -f $tmpFile1 ]; then
541-
magic=$(grep "name='__csrf_magic' value=" $tmpFile1 | sed "s/.*__csrf_magic' value=\"//" | sed "s/\" \/>//")
564+
if [ -f "$tmpFile1" ]; then
565+
magic=$(grep "name='__csrf_magic' value=" "$tmpFile1" | sed "s/.*__csrf_magic' value=\"//" | sed "s/\" \/>//")
542566

543-
if [ $DEBUG -eq 1 ]; then
567+
if [ "$DEBUG" -eq 1 ]; then
544568
echo "---------------------------------------------------------------------"
545569
echo "NOTE: The CSRF Magic Token is"
546570
echo "---------------------------------------------------------------------"
547-
echo ${magic}
571+
echo "${magic}"
548572
fi
549573
else
550574
echo "---------------------------------------------------------------------"
@@ -556,13 +580,14 @@ fi
556580
postData="action=login&login_username=${WAUSER}&login_password=${WAPASS}&__csrf_magic=${magic}"
557581

558582
echo "NOTE: Logging into the Cacti User Interface"
583+
# shellcheck disable=SC2086 # $loadSaveCookie is intentionally word-split (multi-word option string)
559584
wget $loadSaveCookie --post-data="${postData}" --output-document="${tmpFile2}" "${WEBHOST}"/index.php >/dev/null 2>&1
560585

561-
if [ $DEBUG -eq 1 ]; then
586+
if [ "$DEBUG" -eq 1 ]; then
562587
echo "---------------------------------------------------------------------"
563588
echo "DEBUG: Output of index.php"
564589
echo "---------------------------------------------------------------------"
565-
cat ${tmpFile2}
590+
cat "${tmpFile2}"
566591

567592
progress=" --show-progress"
568593
else
@@ -572,16 +597,17 @@ fi
572597
# ------------------------------------------------------------------------------
573598
# Run vmstat in background at a user-configurable interval (VMSTAT)
574599
# ------------------------------------------------------------------------------
575-
if [ $VMSTAT -gt 0 ]; then
576-
vmstat --wide $VMSTAT > /tmp/check-all-output/vmstat.out &
600+
if [ "$VMSTAT" -gt 0 ]; then
601+
vmstat --wide "$VMSTAT" > /tmp/check-all-output/vmstat.out &
602+
# shellcheck disable=SC2034 # PIDS reserved for future job tracking
577603
PIDS="PIDS $!"
578604
fi
579605

580606
# ------------------------------------------------------------------------------
581607
# Show memory stats top memory consumers
582608
# ------------------------------------------------------------------------------
583-
if [ $PS -gt 0 ]; then
584-
capture_processes $PS &
609+
if [ "$PS" -gt 0 ]; then
610+
capture_processes "$PS" &
585611
fi
586612

587613
# ------------------------------------------------------------------------------
@@ -591,21 +617,22 @@ fi
591617
start_time=$(date +%s)
592618

593619
echo "NOTE: Recursively Checking all Base Pages - Note this will take several minutes!!!"
620+
# shellcheck disable=SC2086 # $loadSaveCookie and $progress are intentionally word-split (multi-word option strings)
594621
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
595622
error=$?
596623

597624
end_time=$(date +%s)
598-
total=$(($end_time-$start_time))
625+
total=$((end_time-start_time))
599626

600-
if [ $error -eq 8 ]; then
627+
if [ "$error" -eq 8 ]; then
601628
errors=$(grep -c "awaiting response... 404" "${logFile1}")
602629
echo "WARNING: $errors pages not found. This is not necessarily a bug"
603630
fi
604631

605632
# ------------------------------------------------------------------------------
606633
# Debug Errors if required
607634
# ------------------------------------------------------------------------------
608-
if [ $DEBUG -eq 1 ]; then
635+
if [ "$DEBUG" -eq 1 ]; then
609636
echo "---------------------------------------------------------------------"
610637
echo "DEBUG: Output of Wget Log file"
611638
echo "---------------------------------------------------------------------"
@@ -626,7 +653,7 @@ fi
626653

627654
checks=$(grep -c "HTTP" "$logFile1")
628655

629-
if [ $total -gt 0 ]; then
656+
if [ "$total" -gt 0 ]; then
630657
check_rate=$(echo "scale=2; $checks / $total" | bc)
631658
else
632659
check_rate="N/A"
@@ -656,7 +683,7 @@ echo "---------------------------------------------------------------------"
656683
# ------------------------------------------------------------------------------
657684
# Output vmstat statistics if requested
658685
# ------------------------------------------------------------------------------
659-
if [ $VMSTAT -gt 0 ]; then
686+
if [ "$VMSTAT" -gt 0 ]; then
660687
echo "NOTE: Output of vmstat"
661688
echo "---------------------------------------------------------------------"
662689
cat /tmp/check-all-output/vmstat.out

tests/tools/check_cli_version.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@
2323
SCRIPTPATH="$( cd "$(dirname "$0")" || exit ; pwd -P )"
2424
cd "${SCRIPTPATH}/../../" || exit
2525
FILES1=$(find cli -name \*.php | grep -v "index.php" | sort)
26-
FILES2=$(ls -1 poller*.php | egrep -v "(index.php|pollers.php)" | sort)
26+
FILES2=$(find . -maxdepth 1 -name 'poller*.php' | grep -E -v "(index.php|pollers.php)" | sort)
2727
FILES3="cactid.php cmd.php"
28-
WEBUSER=$(ps -ef | egrep '(httpd|apache2|apache)' | grep -v $(whoami) | grep -v root | head -n1 | awk '{print $1}')
28+
# shellcheck disable=SC2009 # pgrep lacks awk/user-filtering in one step; ps pipeline required
29+
WEBUSER=$(ps -ef | grep -E '(httpd|apache2|apache)' | grep -v "$(whoami)" | grep -v root | head -n1 | awk '{print $1}')
2930
PWD=$(pwd)
3031

3132
FAILED=0

0 commit comments

Comments
 (0)