From 46f42c88933914c589fc2c919d6d2e15352b465f Mon Sep 17 00:00:00 2001
From: MatMoul <matmoul@gmail.com>
Date: Sun, 26 Apr 2026 00:33:27 +0200
Subject: [PATCH] fix: harden checklist selection parsing and clarify -f help
 text

---
 README.md      |  2 +-
 bin/netupgrade | 36 +++++++++++++++++++++++++++++++-----
 state.md       |  5 +++--
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/README.md b/README.md
index 3991004..ef0f856 100644
--- a/README.md
+++ b/README.md
@@ -102,7 +102,7 @@ netupgrade [--help] [-f] [-y] [configfilename]
 Options:
 
 - `--help`: show help
-- `-f`: preselect all nodes
+- `-f`: preselect all nodes in the interactive checklist
 - `-y`: pass non-interactive confirmation flags to supported package managers
 - `configfilename`: path to a config file
 
diff --git a/bin/netupgrade b/bin/netupgrade
index 7c2310f..cfc8425 100755
--- a/bin/netupgrade
+++ b/bin/netupgrade
@@ -4,7 +4,7 @@ showHelp() {
 	echo "netupgrade [--help] [-f] [-y] [configfilename]"
 	echo ""
 	echo "  --help         Show this help message"
-	echo "  -f             Select all nodes"
+	echo "  -f             Preselect all nodes in the checklist"
 	echo "  -y             No confirmation for supported package managers"
 	echo "  configfilename Path to a cfg file"
 }
@@ -64,6 +64,20 @@ parseNode() {
 	IFS=';' read -r -a NODE_FIELDS_REF <<< "${NODE_VALUE}"
 }
 
+parseSelection() {
+	local SELECTION_RAW="${1}"
+	local -n SELECTION_REF="${2}"
+	local ITEM_INDEX=0
+
+	SELECTION_REF=()
+	read -r -a SELECTION_REF <<< "${SELECTION_RAW}"
+
+	for ITEM_INDEX in "${!SELECTION_REF[@]}"; do
+		SELECTION_REF[ITEM_INDEX]="${SELECTION_REF[ITEM_INDEX]%\"}"
+		SELECTION_REF[ITEM_INDEX]="${SELECTION_REF[ITEM_INDEX]#\"}"
+	done
+}
+
 runSSH() {
 	local HOST="${1}"
 	shift
@@ -72,11 +86,18 @@ runSSH() {
 
 selectNodes(){
 	local -a OPTIONS=()
+	local -a SELECTED_ITEMS=()
 	local -i INDEX=1
 	local -i I=0
 	local -a FIELDS=()
 	local DESC=""
 	local FIELD=""
+	local SEL=""
+	local DEFAULT_STATE="OFF"
+
+	if [ "${FULL}" -eq 1 ]; then
+		DEFAULT_STATE="ON"
+	fi
 
 	for NODE in "${NODES[@]}"; do
 		FIELDS=()
@@ -90,7 +111,7 @@ selectNodes(){
 				DESC="${DESC}|${FIELD%%:*}"
 			fi
 		done
-		OPTIONS+=("${INDEX}:${FIELDS[0]}" "${FIELDS[1]} [${DESC}]" "${FULL}")
+		OPTIONS+=("${INDEX}:${FIELDS[0]}" "${FIELDS[1]} [${DESC}]" "${DEFAULT_STATE}")
 		INDEX+=1
 	done
 	if ! SEL=$(whiptail --title "NetUpgrade" --checklist "" 0 0 0 \
@@ -98,7 +119,12 @@ selectNodes(){
 		3>&1 1>&2 2>&3); then
 		exit 0
 	fi
-	if [ ${#SEL} == 0 ]; then
+	if [ -z "${SEL}" ]; then
+		exit 0
+	fi
+
+	parseSelection "${SEL}" SELECTED_ITEMS
+	if [ ${#SELECTED_ITEMS[@]} -eq 0 ]; then
 		exit 0
 	fi
 
@@ -108,12 +134,12 @@ selectNodes(){
 	touch "${LOGFILENAME}"
 	local RESULT="\n"
 
-	for ITM in ${SEL}; do
+	for ITM in "${SELECTED_ITEMS[@]}"; do
 		INDEX=1
 		for NODE in "${NODES[@]}"; do
 			FIELDS=()
 			parseNode "${NODE}" FIELDS
-			if [ "${ITM}" = "\"${INDEX}:${FIELDS[0]}\"" ]; then
+			if [ "${ITM}" = "${INDEX}:${FIELDS[0]}" ]; then
 				for ((I = 2; I < ${#FIELDS[@]}; ++I)); do
 					if runCmd "${FIELDS[0]}" "${FIELDS[1]}" "${FIELDS[I]}"; then
 						RESULT+="Ok: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[I]}\n"
diff --git a/state.md b/state.md
index d57e925..0279557 100644
--- a/state.md
+++ b/state.md
@@ -90,7 +90,6 @@ Supported action types currently include:
 
 ### Short term
 - Tackle the next hardening work as small, reviewable commits instead of one broad patch
-- First focus on the interactive selection flow: make `whiptail` defaults explicit with `ON`/`OFF` and harden parsing of selected items
 - Revisit the log summary insertion method, which still relies on `sed -i` string interpolation
 - Review package-manager cleanup steps that look incorrect or misleading, such as `apt-get purge` without arguments and the current `apk` `-y` handling
 - Review the remaining quoting-sensitive areas, especially around remote shell command construction
@@ -120,6 +119,8 @@ Supported action types currently include:
 - The `docker-stacks` action was rewritten to use a remote shell script with the stack directory passed as an argument
 - Unknown actions and reboot SSH failures now propagate error status more consistently
 - A focused code review identified the next recommended work items and suggested splitting them into separate commits rather than combining them in one larger hardening change
+- `whiptail` checklist defaults are now passed explicitly as `ON`/`OFF`, and selected items are parsed through a dedicated helper instead of relying on raw shell word splitting
+- The CLI help and README now clarify that `-f` preselects all nodes in the interactive checklist
 
 ## Change guidance
 - Preserve backward compatibility for existing config files where possible
@@ -129,8 +130,8 @@ Supported action types currently include:
 - Be cautious with changes to remote command construction, as quoting changes can introduce regressions
 
 ## Suggested review focus for future changes
-- `whiptail` selection handling, including explicit default states and robust parsing of selected values
 - Safe log summary generation without in-place `sed` interpolation of arbitrary text
+- Correctness of package-manager cleanup commands and confirmation flags
 - Correctness of remote command execution
 - Safe quoting and shell expansion behavior
 - Compatibility of config format with existing user setups