diff --git a/README.md b/README.md index 32b5a55..3991004 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,8 @@ host;display-name;action1;action2;... Example: ```bash README.md +SSH_USER="root" + NODES=( "192.168.1.10;web-01;apt;reboot" "192.168.1.11;db-01;apt;cmd:systemctl restart postgresql" @@ -106,7 +108,8 @@ Options: ## Notes -- SSH connections currently use `root@host` +- SSH connections use `root@host` by default and can be changed with `SSH_USER` in the config file +- `cmd:` is executed through a remote shell, so shell operators such as pipes, redirections, `&&`, and `||` are supported - The tool is interactive and intended for manual administration workflows - After execution, the log file is opened with `$EDITOR` when available, otherwise with `nano`, `vi`, or `less` - If no supported log viewer is available, the script keeps running and prints the log file path diff --git a/bin/netupgrade b/bin/netupgrade index 516e31f..7c2310f 100755 --- a/bin/netupgrade +++ b/bin/netupgrade @@ -58,23 +58,37 @@ loadConfig(){ fi } +parseNode() { + local NODE_VALUE="${1}" + local -n NODE_FIELDS_REF="${2}" + IFS=';' read -r -a NODE_FIELDS_REF <<< "${NODE_VALUE}" +} + +runSSH() { + local HOST="${1}" + shift + ssh "${SSH_USER}@${HOST}" "$@" +} + selectNodes(){ local -a OPTIONS=() local -i INDEX=1 + local -i I=0 + local -a FIELDS=() + local DESC="" + local FIELD="" + for NODE in "${NODES[@]}"; do - # shellcheck disable=SC2206 - local FIELDS=(${NODE//;/ }) - local DESKSKIP=0 - local DESC="" - for FIELD in "${FIELDS[@]}"; do - if [ ${DESKSKIP} -gt 1 ]; then - if [ "${DESC}" == "" ]; then - DESC="${FIELD/:*/}" - else - DESC="${DESC}|${FIELD/:*/}" - fi + FIELDS=() + DESC="" + parseNode "${NODE}" FIELDS + for ((I = 2; I < ${#FIELDS[@]}; ++I)); do + FIELD="${FIELDS[I]}" + if [ -z "${DESC}" ]; then + DESC="${FIELD%%:*}" + else + DESC="${DESC}|${FIELD%%:*}" fi - DESKSKIP=$(( DESKSKIP + 1)) done OPTIONS+=("${INDEX}:${FIELDS[0]}" "${FIELDS[1]} [${DESC}]" "${FULL}") INDEX+=1 @@ -97,14 +111,14 @@ selectNodes(){ for ITM in ${SEL}; do INDEX=1 for NODE in "${NODES[@]}"; do - # shellcheck disable=SC2206 - local FIELDS=(${NODE//;/ }) + FIELDS=() + parseNode "${NODE}" FIELDS 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" + if runCmd "${FIELDS[0]}" "${FIELDS[1]}" "${FIELDS[I]}"; then + RESULT+="Ok: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[I]}\n" else - RESULT+="Error: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[${I}]}\n" + RESULT+="Error: ${FIELDS[1]} @ ${FIELDS[0]} : ${FIELDS[I]}\n" fi done fi @@ -127,136 +141,157 @@ selectNodes(){ echo -e "${RESULT}" } -runCmd() { #$1=host $2=name #3=cmd - local -r HOST=${1} - local -r NAME=${2} - local -r CMD=${3//:*} - local -r CMDVAL=${3//*:} +runCmd() { # $1=host $2=name $3=cmd + local -r HOST="${1}" + local -r NAME="${2}" + local -r ACTION="${3}" + local -r CMD="${ACTION%%:*}" + local -r CMDVAL="${ACTION#*:}" local -i ERROR=0 - echo "${NAME} @ ${HOST} : ${CMD}" | tee -a "${LOGFILENAME}" - local TITLELENGTH=$((${#NAME} + ${#HOST} + ${#CMD} + 6)) + local TITLELENGTH=0 local SUBTITLE="-----------------------------------------------------------------------------" - echo ${SUBTITLE:0:${TITLELENGTH}} | tee -a "${LOGFILENAME}" + local YESARG="" + + echo "${NAME} @ ${HOST} : ${CMD}" | tee -a "${LOGFILENAME}" + TITLELENGTH=$((${#NAME} + ${#HOST} + ${#CMD} + 6)) + echo "${SUBTITLE:0:${TITLELENGTH}}" | tee -a "${LOGFILENAME}" date +'%Y-%m-%d %H:%M:%S %A' | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" set -o pipefail - local YESARG="" + case ${CMD} in reboot) - ssh root@"${HOST}" reboot | tee -a "${LOGFILENAME}" + if ! runSSH "${HOST}" reboot | tee -a "${LOGFILENAME}"; then + ERROR=1 + fi ;; apt) - if [ ${YES} == 1 ]; then + if [ "${YES}" -eq 1 ]; then YESARG="-y" fi echo "apt-get ${YESARG} update" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" apt-get ${YESARG} update | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" apt-get ${YESARG} update | tee -a "${LOGFILENAME}"; then ERROR=1 else echo "" | tee -a "${LOGFILENAME}" echo "apt-get ${YESARG} dist-upgrade" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" apt-get ${YESARG} dist-upgrade | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" apt-get ${YESARG} dist-upgrade | tee -a "${LOGFILENAME}"; then ERROR=1 fi echo "" | tee -a "${LOGFILENAME}" echo "apt-get ${YESARG} autoremove" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" apt-get ${YESARG} autoremove | tee -a "${LOGFILENAME}" + runSSH "${HOST}" apt-get ${YESARG} autoremove | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" echo "apt-get ${YESARG} autoclean" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" apt-get ${YESARG} autoclean | tee -a "${LOGFILENAME}" + runSSH "${HOST}" apt-get ${YESARG} autoclean | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" echo "apt-get ${YESARG} clean" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" apt-get ${YESARG} clean | tee -a "${LOGFILENAME}" + runSSH "${HOST}" apt-get ${YESARG} clean | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" echo "apt-get ${YESARG} purge" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" apt-get ${YESARG} purge | tee -a "${LOGFILENAME}" + runSSH "${HOST}" apt-get ${YESARG} purge | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" fi ;; yum) - if [ ${YES} == 1 ]; then + if [ "${YES}" -eq 1 ]; then YESARG="-y" fi echo "yum ${YESARG} update" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" yum ${YESARG} update | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" yum ${YESARG} update | tee -a "${LOGFILENAME}"; then ERROR=1 fi ;; pkg) - if [ ${YES} == 1 ]; then + if [ "${YES}" -eq 1 ]; then YESARG="-y" fi echo "pkg ${YESARG} update" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" pkg ${YESARG} update | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" pkg ${YESARG} update | tee -a "${LOGFILENAME}"; then ERROR=1 else echo "" | tee -a "${LOGFILENAME}" echo "pkg upgrade ${YESARG}" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" pkg upgrade ${YESARG} | tee -a "${LOGFILENAME}"; then - ERROR=1 + if ! runSSH "${HOST}" pkg upgrade ${YESARG} | tee -a "${LOGFILENAME}"; then + ERROR=1 fi echo "" | tee -a "${LOGFILENAME}" echo "pkg autoremove ${YESARG}" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" pkg autoremove ${YESARG} | tee -a "${LOGFILENAME}" + runSSH "${HOST}" pkg autoremove ${YESARG} | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" echo "pkg clean ${YESARG}" | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" pkg clean ${YESARG} | tee -a "${LOGFILENAME}" + runSSH "${HOST}" pkg clean ${YESARG} | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" fi ;; pacman) - if [ ${YES} == 1 ]; then + if [ "${YES}" -eq 1 ]; then YESARG="--noconfirm" fi echo "pacman -Sy ${YESARG} archlinux-keyring" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" pacman -Sy ${YESARG} archlinux-keyring | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" pacman -Sy ${YESARG} archlinux-keyring | tee -a "${LOGFILENAME}"; then ERROR=1 fi echo "pacman -Syu ${YESARG}" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" pacman -Syu ${YESARG} | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" pacman -Syu ${YESARG} | tee -a "${LOGFILENAME}"; then + ERROR=1 + fi + echo "pacman orphan cleanup" | tee -a "${LOGFILENAME}" + if ! runSSH "${HOST}" sh -c 'yesarg="$1"; orphans=$(pacman -Qqtd 2>/dev/null || true); if [ -n "$orphans" ]; then pacman -Rns $yesarg $orphans; fi' sh "${YESARG}" | tee -a "${LOGFILENAME}"; then + ERROR=1 + fi + echo "pacman -Sc ${YESARG}" | tee -a "${LOGFILENAME}" + if ! runSSH "${HOST}" pacman -Sc ${YESARG} | tee -a "${LOGFILENAME}"; then ERROR=1 fi - # shellcheck disable=SC2046 - ssh root@"${HOST}" pacman -Rns $(pacman -Qqtd) ${YESARG} | tee -a "${LOGFILENAME}" - ssh root@"${HOST}" pacman -Sc ${YESARG} | tee -a "${LOGFILENAME}" ;; apk) - if [ ${YES} == 1 ]; then + if [ "${YES}" -eq 1 ]; then YESARG="-y" fi echo "apk update" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" apk update | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" apk update | tee -a "${LOGFILENAME}"; then ERROR=1 fi echo "apk upgrade" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" apk upgrade | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" apk upgrade | tee -a "${LOGFILENAME}"; then ERROR=1 fi ;; cmd) echo "cmd: ${CMDVAL}" | tee -a "${LOGFILENAME}" - # shellcheck disable=SC2029 - if ! ssh root@"${HOST}" "${CMDVAL}" | tee -a "${LOGFILENAME}"; then + if ! runSSH "${HOST}" sh -c "${CMDVAL}" | tee -a "${LOGFILENAME}"; then ERROR=1 fi ;; docker-stacks) - echo "docker stacks update" | tee -a "${LOGFILENAME}" - echo "for each" | tee -a "${LOGFILENAME}" - echo " docker compose pull; docker compose up -d" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" 'for dir in '"${CMDVAL}"'/*; do (cd "${dir}"; docker compose pull; docker compose up -d); done; docker image prune -f' | tee -a "${LOGFILENAME}"; then - ERROR=1 - fi - echo "docker image prune -a -f" | tee -a "${LOGFILENAME}" - if ! ssh root@"${HOST}" docker image prune -f | tee -a "${LOGFILENAME}"; then + echo "docker stacks update in ${CMDVAL}" | tee -a "${LOGFILENAME}" + if ! runSSH "${HOST}" sh -s -- "${CMDVAL}" <<'EOF' | tee -a "${LOGFILENAME}" +stack_root="$1" + +for dir in "$stack_root"/*; do + [ -d "$dir" ] || continue + ( + cd "$dir" || exit 1 + docker compose pull + docker compose up -d + ) || exit 1 +done + +docker image prune -f +EOF + then ERROR=1 fi ;; - *) echo "Error: Command ${CMD} unknown" | tee -a "${LOGFILENAME}";; + *) + echo "Error: Command ${CMD} unknown" | tee -a "${LOGFILENAME}" + ERROR=1 + ;; esac echo "" | tee -a "${LOGFILENAME}" echo "" | tee -a "${LOGFILENAME}" - if [ ${ERROR} == 1 ]; then + if [ "${ERROR}" -eq 1 ]; then return 1 fi } @@ -267,6 +302,7 @@ declare -i FULL=0 declare CONFIGFILENAME="${HOME}/.config/netupgrade/index.cfg" declare LOGFILENAME="${HOME}/netupgrade.log" declare LOGVIEWER="" +declare SSH_USER="root" declare -a NODES=() while [[ ${#} -gt 0 ]]; do diff --git a/state.md b/state.md index 0f0eb04..914d27b 100644 --- a/state.md +++ b/state.md @@ -20,7 +20,7 @@ The tool: 2. Expects a `NODES` array populated with entries formatted like: `host;display-name;action1;action2;...` 3. Displays an interactive multi-select checklist using `whiptail` -4. Executes the selected actions on each selected host through `ssh root@host` +4. Executes the selected actions on each selected host through SSH, using `root@host` by default or `SSH_USER@host` when configured 5. Writes execution logs to `~/netupgrade.log` 6. Opens the log with `$EDITOR` when available, otherwise `nano`, `vi`, or `less`; if none is available, it prints the log path, then optionally removes the log file @@ -57,18 +57,15 @@ Supported action types currently include: - The configuration format and supported actions are now documented in more detail ### 2. Shell robustness concerns -- Node parsing relies on replacing `;` with spaces and re-splitting: - this is fragile if values contain spaces or special characters -- Quoting is inconsistent across the script - Config files are sourced directly, which is flexible but implies arbitrary code execution -- The script does not use a stricter shell safety baseline +- The script still has some quoting-sensitive areas and does not use a stricter shell safety baseline +- The `NODES` parsing was hardened to split on `;` with `IFS`/`read -r -a`, which now preserves spaces in action values such as `cmd:...` ### 3. Remote execution correctness and safety -- SSH user is hardcoded to `root` -- `cmd:<...>` intentionally allows arbitrary remote command execution, which should be treated as a powerful unsafe feature -- Some remote command constructions are brittle -- The `pacman` orphan-removal command appears incorrect because command substitution is evaluated locally instead of remotely -- The `docker-stacks` remote loop should be reviewed carefully for quoting and path safety +- SSH execution now goes through a dedicated `runSSH` helper and the SSH user is configurable via `SSH_USER`, defaulting to `root` +- `cmd:<...>` intentionally allows arbitrary remote command execution and is now executed through a remote shell, which improves support for shell operators but remains a powerful unsafe feature +- The `pacman` orphan-removal command was corrected so orphan detection happens on the remote host instead of locally +- The `docker-stacks` remote loop was rewritten to pass the stack root as an argument to a remote shell script, improving quoting and path handling ### 4. UX and dependency issues - Required runtime dependencies are now checked at startup (`ssh`, `whiptail`, `sed`, `tee`, `rm`, `touch`) @@ -92,10 +89,9 @@ Supported action types currently include: ## Recommended direction ### Short term -- Correct the remote command bugs, especially for `pacman` -- Improve parsing of node entries to avoid whitespace-splitting issues -- Review `docker-stacks` remote command quoting and behavior -- Consider making SSH user, log path, and editor configurable +- Review the remaining quoting-sensitive areas, especially around remote shell command construction +- Consider making log path and editor configurable +- Revisit the log summary insertion method, which still relies on `sed -i` string interpolation ### Medium term - Improve parsing of node entries to avoid whitespace-splitting issues @@ -116,6 +112,11 @@ Supported action types currently include: - Log viewer selection was made more flexible: `$EDITOR` is preferred, then `nano`, `vi`, or `less` - `nano` is no longer a strict runtime dependency - The unsupported `-b` option remains unimplemented and is no longer shown in help output +- `NODES` parsing was hardened to preserve spaces in action values by splitting on `;` with `IFS` and `read -r -a` +- SSH calls were centralized through a `runSSH` helper and `SSH_USER` is now configurable, defaulting to `root` +- The `pacman` orphan cleanup now runs entirely on the remote host instead of evaluating orphan detection locally +- 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 ## Change guidance - Preserve backward compatibility for existing config files where possible