From 3fe795985024477acd60f5b9f95e869ae4c269f7 Mon Sep 17 00:00:00 2001 From: MatMoul Date: Sun, 26 Apr 2026 00:36:01 +0200 Subject: [PATCH] fix: avoid sed interpolation when prepending log summary Use a temporary file to write the summary header and existing log content, then replace the original log atomically. This removes the dependency on sed for summary insertion and avoids unsafe string interpolation. --- bin/netupgrade | 22 ++++++++++++++++++---- state.md | 5 ++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/bin/netupgrade b/bin/netupgrade index cfc8425..cc99a1b 100755 --- a/bin/netupgrade +++ b/bin/netupgrade @@ -10,7 +10,7 @@ showHelp() { } checkDependencies() { - local -a REQUIRED_CMDS=(ssh whiptail sed tee rm touch) + local -a REQUIRED_CMDS=(ssh whiptail cat tee rm touch) local -a MISSING_CMDS=() local CMD @@ -84,6 +84,22 @@ runSSH() { ssh "${SSH_USER}@${HOST}" "$@" } +prependLogSummary() { + local SUMMARY_CONTENT="${1}" + local TEMP_LOGFILENAME="${LOGFILENAME}.tmp" + + { + echo "Results :" + echo "---------" + echo "" + echo -e "${SUMMARY_CONTENT}" + echo "" + echo "" + cat "${LOGFILENAME}" + } > "${TEMP_LOGFILENAME}" + mv "${TEMP_LOGFILENAME}" "${LOGFILENAME}" +} + selectNodes(){ local -a OPTIONS=() local -a SELECTED_ITEMS=() @@ -152,9 +168,7 @@ selectNodes(){ done done - sed -i "1s/^/${RESULT//\//\\\/}\n\n\n\n/" "${LOGFILENAME}" - sed -i "1s/^/---------\n\n/" "${LOGFILENAME}" - sed -i "1s/^/Results :\n/" "${LOGFILENAME}" + prependLogSummary "${RESULT}" if [ -n "${LOGVIEWER}" ]; then "${LOGVIEWER}" "${LOGFILENAME}" else diff --git a/state.md b/state.md index 0279557..4af6450 100644 --- a/state.md +++ b/state.md @@ -68,7 +68,7 @@ Supported action types currently include: - 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`) +- Required runtime dependencies are now checked at startup (`ssh`, `whiptail`, `cat`, `tee`, `rm`, `touch`) - Log viewing no longer depends strictly on `nano`; the script now falls back to `$EDITOR`, then `nano`, `vi`, or `less` - If no supported log viewer is available, execution continues and the log path is shown - The workflow is highly interactive and not well suited for automation @@ -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 -- 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 @@ -121,6 +120,7 @@ Supported action types currently include: - 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 +- Log summary generation no longer uses `sed -i` interpolation; the script now writes a temporary file with the summary header plus the existing log content and replaces the original log atomically ## Change guidance - Preserve backward compatibility for existing config files where possible @@ -130,7 +130,6 @@ 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 -- 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