fix: harden checklist selection parsing and clarify -f help text
This commit is contained in:
@@ -102,7 +102,7 @@ netupgrade [--help] [-f] [-y] [configfilename]
|
|||||||
Options:
|
Options:
|
||||||
|
|
||||||
- `--help`: show help
|
- `--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
|
- `-y`: pass non-interactive confirmation flags to supported package managers
|
||||||
- `configfilename`: path to a config file
|
- `configfilename`: path to a config file
|
||||||
|
|
||||||
|
|||||||
+31
-5
@@ -4,7 +4,7 @@ showHelp() {
|
|||||||
echo "netupgrade [--help] [-f] [-y] [configfilename]"
|
echo "netupgrade [--help] [-f] [-y] [configfilename]"
|
||||||
echo ""
|
echo ""
|
||||||
echo " --help Show this help message"
|
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 " -y No confirmation for supported package managers"
|
||||||
echo " configfilename Path to a cfg file"
|
echo " configfilename Path to a cfg file"
|
||||||
}
|
}
|
||||||
@@ -64,6 +64,20 @@ parseNode() {
|
|||||||
IFS=';' read -r -a NODE_FIELDS_REF <<< "${NODE_VALUE}"
|
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() {
|
runSSH() {
|
||||||
local HOST="${1}"
|
local HOST="${1}"
|
||||||
shift
|
shift
|
||||||
@@ -72,11 +86,18 @@ runSSH() {
|
|||||||
|
|
||||||
selectNodes(){
|
selectNodes(){
|
||||||
local -a OPTIONS=()
|
local -a OPTIONS=()
|
||||||
|
local -a SELECTED_ITEMS=()
|
||||||
local -i INDEX=1
|
local -i INDEX=1
|
||||||
local -i I=0
|
local -i I=0
|
||||||
local -a FIELDS=()
|
local -a FIELDS=()
|
||||||
local DESC=""
|
local DESC=""
|
||||||
local FIELD=""
|
local FIELD=""
|
||||||
|
local SEL=""
|
||||||
|
local DEFAULT_STATE="OFF"
|
||||||
|
|
||||||
|
if [ "${FULL}" -eq 1 ]; then
|
||||||
|
DEFAULT_STATE="ON"
|
||||||
|
fi
|
||||||
|
|
||||||
for NODE in "${NODES[@]}"; do
|
for NODE in "${NODES[@]}"; do
|
||||||
FIELDS=()
|
FIELDS=()
|
||||||
@@ -90,7 +111,7 @@ selectNodes(){
|
|||||||
DESC="${DESC}|${FIELD%%:*}"
|
DESC="${DESC}|${FIELD%%:*}"
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
OPTIONS+=("${INDEX}:${FIELDS[0]}" "${FIELDS[1]} [${DESC}]" "${FULL}")
|
OPTIONS+=("${INDEX}:${FIELDS[0]}" "${FIELDS[1]} [${DESC}]" "${DEFAULT_STATE}")
|
||||||
INDEX+=1
|
INDEX+=1
|
||||||
done
|
done
|
||||||
if ! SEL=$(whiptail --title "NetUpgrade" --checklist "" 0 0 0 \
|
if ! SEL=$(whiptail --title "NetUpgrade" --checklist "" 0 0 0 \
|
||||||
@@ -98,7 +119,12 @@ selectNodes(){
|
|||||||
3>&1 1>&2 2>&3); then
|
3>&1 1>&2 2>&3); then
|
||||||
exit 0
|
exit 0
|
||||||
fi
|
fi
|
||||||
if [ ${#SEL} == 0 ]; then
|
if [ -z "${SEL}" ]; then
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
parseSelection "${SEL}" SELECTED_ITEMS
|
||||||
|
if [ ${#SELECTED_ITEMS[@]} -eq 0 ]; then
|
||||||
exit 0
|
exit 0
|
||||||
fi
|
fi
|
||||||
|
|
||||||
@@ -108,12 +134,12 @@ selectNodes(){
|
|||||||
touch "${LOGFILENAME}"
|
touch "${LOGFILENAME}"
|
||||||
local RESULT="\n"
|
local RESULT="\n"
|
||||||
|
|
||||||
for ITM in ${SEL}; do
|
for ITM in "${SELECTED_ITEMS[@]}"; do
|
||||||
INDEX=1
|
INDEX=1
|
||||||
for NODE in "${NODES[@]}"; do
|
for NODE in "${NODES[@]}"; do
|
||||||
FIELDS=()
|
FIELDS=()
|
||||||
parseNode "${NODE}" FIELDS
|
parseNode "${NODE}" FIELDS
|
||||||
if [ "${ITM}" = "\"${INDEX}:${FIELDS[0]}\"" ]; then
|
if [ "${ITM}" = "${INDEX}:${FIELDS[0]}" ]; then
|
||||||
for ((I = 2; I < ${#FIELDS[@]}; ++I)); do
|
for ((I = 2; I < ${#FIELDS[@]}; ++I)); do
|
||||||
if runCmd "${FIELDS[0]}" "${FIELDS[1]}" "${FIELDS[I]}"; then
|
if runCmd "${FIELDS[0]}" "${FIELDS[1]}" "${FIELDS[I]}"; then
|
||||||
RESULT+="Ok: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[I]}\n"
|
RESULT+="Ok: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[I]}\n"
|
||||||
|
|||||||
@@ -90,7 +90,6 @@ Supported action types currently include:
|
|||||||
|
|
||||||
### Short term
|
### Short term
|
||||||
- Tackle the next hardening work as small, reviewable commits instead of one broad patch
|
- 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
|
- 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 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
|
- 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
|
- 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
|
- 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
|
- 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
|
## Change guidance
|
||||||
- Preserve backward compatibility for existing config files where possible
|
- 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
|
- Be cautious with changes to remote command construction, as quoting changes can introduce regressions
|
||||||
|
|
||||||
## Suggested review focus for future changes
|
## 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
|
- 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
|
- Correctness of remote command execution
|
||||||
- Safe quoting and shell expansion behavior
|
- Safe quoting and shell expansion behavior
|
||||||
- Compatibility of config format with existing user setups
|
- Compatibility of config format with existing user setups
|
||||||
|
|||||||
Reference in New Issue
Block a user