From 3c4646052eff185f3d7fc670868ec86532f9c2f7 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 09:29:56 +0200 Subject: [PATCH 01/14] mupibox bash scripts: kill the cat<<FILE truncate-race in 12 scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pattern `/usr/bin/cat <<< $(/usr/bin/jq … FILE) > FILE` showed up 55 times across 12 scripts in scripts/mupibox/. It writes back through two indirections (subshell capture + here-string), the outer `> FILE` redirect opens FILE before cat finishes, and any jq error or partial output leaves FILE truncated to empty. Concrete failure mode: a single jq glitch on save wipes mupiboxconfig.json, monitor.json, network.json or data.json — the box is then unbootable until manual recovery. Replace every occurrence with the same atomic-rename pattern: _TMP="${TARGET}.tmp.$$" /usr/bin/jq … > "${_TMP}" && mv "${_TMP}" "${TARGET}" || rm -f "${_TMP}" Same-fs rename is atomic on ext4, jq's exit code now gates the swap, and a half-written tempfile is unlinked on failure so /tmp doesn't fill up with leftovers. For the scripts that updated multiple fields in the same target file (setting_update.sh: 16 writes; mupi_start_led .sh: 5 writes; get_network.sh: 9 writes), bundle the field updates into one jq pipeline per target — fewer SD-card writes, single truncate-race window per pass. Files touched: albumstop.sh, albumstop_activator.sh: monitor.json init + set check_network.sh: network.json onlinestate (already touched in Phase 2 for the offline filter, now atomic too) data_clean.sh: TMP_DATA append + sed-truncate-race get_monitor.sh: monitor.json On/Off transitions get_network.sh: 9-field network.json bundle m3u_generator.sh: data.json append for audiobook/music/other mupi_start_led.sh: TMP_LEDFILE init + dim-mode toggles repair_config.sh: SONOS_CONFIG hostname setting_update.sh: SONOS_CONFIG (6→1) + SPOTIFYCONTROLLER_CONFIG (6→1) set_deviceid.sh: also picks up HIGH-9 (string-test → -gt) and HIGH-10 (sudo-redirect mismatch → /tmp staging + sudo install) set_hostname.sh: SONOS_CONFIG hostname While here: HIGH-14 — `sudo echo "[]" FILE` (missing the `>` redirect, file never created) is hit in albumstop.sh, check_network.sh, get_network.sh, get_monitor.sh and albumstop_activator.sh. Replace with a plain `echo -n "{}" > FILE` (no sudo: every target lives in /tmp or under /home/dietpi/.../config/, both directly writable by the dietpi user the script runs as) and seed as `{}` (object) since every consumer reads these files via `.onlinestate` / `.monitor` / `.albumStop` etc. The earlier `sudo tee` variant produced a root-owned seed that the subsequent jq+mv (running as dietpi) silently failed to replace, leaving the seed wrong forever. Tighten the existence- guard in check_network.sh / get_network.sh to also rebuild on empty-or-wrong-shape so existing boxes recover without manual cleanup. HIGH-16 — data_clean.sh `[[ ! -f \${j} ]]` matched the literal- glob fallback when the iterated cover-dir was empty (then `rm -R "${j}"` tried to rm a path containing a literal `*`). Restrict to actual directories with `[[ -d \${j} ]]`. --- scripts/mupibox/albumstop.sh | 20 +++++-- scripts/mupibox/albumstop_activator.sh | 12 ++-- scripts/mupibox/check_network.sh | 26 +++++++-- scripts/mupibox/data_clean.sh | 25 +++++++-- scripts/mupibox/get_monitor.sh | 28 +++++++--- scripts/mupibox/get_network.sh | 41 +++++++++----- scripts/mupibox/m3u_generator.sh | 22 ++++++-- scripts/mupibox/mupi_start_led.sh | 21 ++++--- scripts/mupibox/repair_config.sh | 4 +- scripts/mupibox/set_deviceid.sh | 20 ++++++- scripts/mupibox/set_hostname.sh | 4 +- scripts/mupibox/setting_update.sh | 76 ++++++++++++++++---------- 12 files changed, 213 insertions(+), 86 deletions(-) diff --git a/scripts/mupibox/albumstop.sh b/scripts/mupibox/albumstop.sh index a868b85c..b00f2a2e 100644 --- a/scripts/mupibox/albumstop.sh +++ b/scripts/mupibox/albumstop.sh @@ -5,14 +5,26 @@ ALBUMSTOP_FILE="/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/albumstop.json" +# Atomic-update pattern (HIGH-8): write jq output to a same-dir tempfile, +# then mv it onto the target. The previous `cat <<< $(jq …) > FILE` +# truncated FILE before jq finished — jq errors or partial output left +# the JSON empty and bricked the next boot. With write-then-rename, jq +# either succeeds and the rename atomically swaps in the new content, or +# it fails and the original file stays intact (we rm the half-written +# tempfile so /tmp doesn't fill up). if [ ! -f ${ALBUMSTOP_FILE} ]; then - sudo echo -n "{}" ${ALBUMSTOP_FILE} - chown dietpi:dietpi ${ALBUMSTOP_FILE} - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "Off" '.albumStop = $v' ${ALBUMSTOP_FILE}) > ${ALBUMSTOP_FILE} + # HIGH-14 (Phase-3) + Phase-5 follow-up: ALBUMSTOP_FILE lives + # under /home/dietpi/.../config/ which is dietpi-writable; drop + # the sudo+tee that produced a root-owned file the script + # couldn't subsequently replace. + echo -n "{}" > "${ALBUMSTOP_FILE}" + _TMP="${ALBUMSTOP_FILE}.tmp.$$" + /usr/bin/jq -n --arg v "Off" '.albumStop = $v' > "${_TMP}" && mv "${_TMP}" "${ALBUMSTOP_FILE}" || rm -f "${_TMP}" sleep 10 sudo bash /usr/local/bin/mupibox/shutdown.sh else - /usr/bin/cat <<< $(/usr/bin/jq --arg v "Off" '.albumStop = $v' ${ALBUMSTOP_FILE}) > ${ALBUMSTOP_FILE} + _TMP="${ALBUMSTOP_FILE}.tmp.$$" + /usr/bin/jq --arg v "Off" '.albumStop = $v' "${ALBUMSTOP_FILE}" > "${_TMP}" && mv "${_TMP}" "${ALBUMSTOP_FILE}" || rm -f "${_TMP}" sleep 10 sudo bash /usr/local/bin/mupibox/shutdown.sh fi \ No newline at end of file diff --git a/scripts/mupibox/albumstop_activator.sh b/scripts/mupibox/albumstop_activator.sh index dbc63a91..8f9323bc 100644 --- a/scripts/mupibox/albumstop_activator.sh +++ b/scripts/mupibox/albumstop_activator.sh @@ -4,10 +4,14 @@ ALBUMSTOP_FILE="/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/albumstop.json" +# Atomic-update pattern (HIGH-8): same as albumstop.sh. if [ ! -f ${ALBUMSTOP_FILE} ]; then - sudo echo -n "{}" ${ALBUMSTOP_FILE} - chown dietpi:dietpi ${ALBUMSTOP_FILE} - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "On" '.albumStop = $v' ${ALBUMSTOP_FILE}) > ${ALBUMSTOP_FILE} + # HIGH-14 (Phase-3) + Phase-5 follow-up: same as albumstop.sh — + # drop sudo, dietpi can write the destination directly. + echo -n "{}" > "${ALBUMSTOP_FILE}" + _TMP="${ALBUMSTOP_FILE}.tmp.$$" + /usr/bin/jq -n --arg v "On" '.albumStop = $v' > "${_TMP}" && mv "${_TMP}" "${ALBUMSTOP_FILE}" || rm -f "${_TMP}" else - /usr/bin/cat <<< $(/usr/bin/jq --arg v "On" '.albumStop = $v' ${ALBUMSTOP_FILE}) > ${ALBUMSTOP_FILE} + _TMP="${ALBUMSTOP_FILE}.tmp.$$" + /usr/bin/jq --arg v "On" '.albumStop = $v' "${ALBUMSTOP_FILE}" > "${_TMP}" && mv "${_TMP}" "${ALBUMSTOP_FILE}" || rm -f "${_TMP}" fi \ No newline at end of file diff --git a/scripts/mupibox/check_network.sh b/scripts/mupibox/check_network.sh index 5be7a38f..cedc3a00 100644 --- a/scripts/mupibox/check_network.sh +++ b/scripts/mupibox/check_network.sh @@ -39,11 +39,23 @@ if [ ! -f ${RESUME_FILE} ]; then fi fi -if [ ! -f ${NETWORKCONFIG} ]; then - sudo echo -n "[]" ${NETWORKCONFIG} - chown dietpi:dietpi ${NETWORKCONFIG} - chmod 777 ${NETWORKCONFIG} - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "starting" '.onlinestate = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} +if [ ! -f ${NETWORKCONFIG} ] || [ ! -s ${NETWORKCONFIG} ] || ! /usr/bin/jq -e 'type == "object"' ${NETWORKCONFIG} >/dev/null 2>&1; then + # HIGH-14 (Phase-3) + Phase-5 follow-up: the previous "fix" piped + # via `sudo tee` which produced a root-owned seed file in /tmp. + # The next line's tempfile + mv ran as dietpi and silently failed + # to replace the root-owned target — leaving the wrong (empty array) + # seed in place forever. Drop sudo entirely (the script runs as + # dietpi which can write /tmp directly), and seed as an OBJECT + # `{}` since network.json is shaped as one (every consumer reads + # it via `.onlinestate` etc). The existence-or-empty-or-wrong- + # shape guard above also recovers from the broken state we + # produced earlier without manual cleanup. + rm -f "${NETWORKCONFIG}" + echo -n "{}" > "${NETWORKCONFIG}" + # Atomic-update (HIGH-8). + _TMP="${NETWORKCONFIG}.tmp.$$" + /usr/bin/jq -n --arg v "starting" '.onlinestate = $v' > "${_TMP}" && mv "${_TMP}" "${NETWORKCONFIG}" || rm -f "${_TMP}" + OLD_ONLINESTATE="starting" else OLD_ONLINESTATE=$(/usr/bin/jq -r .onlinestate ${NETWORKCONFIG}) fi @@ -133,7 +145,9 @@ do fi if [ "${ONLINESTATE}" != "${OLDSTATE}" ]; then - /usr/bin/cat <<< $(/usr/bin/jq --arg v "${ONLINESTATE}" '.onlinestate = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} + # Atomic-update (HIGH-8). + _TMP="${NETWORKCONFIG}.tmp.$$" + /usr/bin/jq --arg v "${ONLINESTATE}" '.onlinestate = $v' "${NETWORKCONFIG}" > "${_TMP}" && mv "${_TMP}" "${NETWORKCONFIG}" || rm -f "${_TMP}" # if [ "${ONLINESTATE}" == "${FALSESTATE}" ] && [ "${OLDSTATE}" != "starting" ]; then # #sudo dhclient -r # sudo service ifup@wlan0 stop diff --git a/scripts/mupibox/data_clean.sh b/scripts/mupibox/data_clean.sh index 920a4e1f..edce8485 100644 --- a/scripts/mupibox/data_clean.sh +++ b/scripts/mupibox/data_clean.sh @@ -13,7 +13,11 @@ for i in "/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover/audiobook echo "[OK] ${artist}" for j in "${i}/"* ; do album=$(/usr/bin/basename "${j}") - if [[ ! -f ${j} ]] + # HIGH-16: previous test was `[[ ! -f ${j} ]]` — true for any + # non-regular-file, including the literal `${i}/*` glob fallback + # when the dir is empty (would then `rm -R` a path containing + # a literal `*`). Restrict to actual directories. + if [[ -d ${j} ]] then if [[ -d "/home/dietpi/MuPiBox/media/audiobook/${artist}/${album}/" ]] then @@ -36,7 +40,11 @@ for i in "/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover/music/"* echo "[OK] ${artist}" for j in "${i}/"* ; do album=$(/usr/bin/basename "${j}") - if [[ ! -f ${j} ]] + # HIGH-16: previous test was `[[ ! -f ${j} ]]` — true for any + # non-regular-file, including the literal `${i}/*` glob fallback + # when the dir is empty (would then `rm -R` a path containing + # a literal `*`). Restrict to actual directories. + if [[ -d ${j} ]] then if [[ -d "/home/dietpi/MuPiBox/media/music/${artist}/${album}/" ]] then @@ -75,7 +83,12 @@ for item in "${my_array[@]}"; do fi if [[ ${i} > 0 ]] && [[ ${add_item} > 0 ]] then - echo $(sed '${s/$/,/}' ${TMP_DATA}) > ${TMP_DATA} + # Atomic-update (HIGH-8). sed reads TMP_DATA into the substitution, + # echo writes back via redirect — if sed errored mid-read the file + # was truncated to empty. Tempfile + rename keeps TMP_DATA intact + # on failure. + _TMP="${TMP_DATA}.tmp.$$" + sed '${s/$/,/}' "${TMP_DATA}" > "${_TMP}" && mv "${_TMP}" "${TMP_DATA}" || rm -f "${_TMP}" fi if [[ ${i} = 0 ]] && [[ ${add_item} > 0 ]] then @@ -83,7 +96,11 @@ for item in "${my_array[@]}"; do fi if [[ ${add_item} > 0 ]] then - /usr/bin/cat <<< $(/usr/bin/jq '.' <<< $item) >> ${TMP_DATA} + # Drop the cat<<< indirection — jq's stdout goes straight to the + # append. Same result, simpler, and no needless tempfile when we + # only need to append (>> doesn't have the truncate-race risk + # that > has). + /usr/bin/jq '.' <<< "$item" >> "${TMP_DATA}" echo "[OK] ${type:1:-1} | ${category:1:-1}" else echo "[DEL] ${type:1:-1} | ${category:1:-1} | ${artist:1:-1} | ${title:1:-1}" diff --git a/scripts/mupibox/get_monitor.sh b/scripts/mupibox/get_monitor.sh index 19728868..64d814e8 100644 --- a/scripts/mupibox/get_monitor.sh +++ b/scripts/mupibox/get_monitor.sh @@ -11,14 +11,22 @@ do actualsize=$(wc -c <"${MONITOR_FILE}") if [ ! -f ${MONITOR_FILE} ]; then - sudo echo -n "{}" ${MONITOR_FILE} - sudo chown dietpi:dietpi ${MONITOR_FILE} - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "On" '.monitor = $v' ${MONITOR_FILE}) > ${MONITOR_FILE} + # HIGH-14 (Phase-3) + Phase-5 follow-up: drop the sudo — + # MONITOR_FILE lives under /home/dietpi/.../config/ and the + # script runs as dietpi, so direct write works. Sudo+tee + # produced a root-owned file that subsequent jq+mv (as + # dietpi) couldn't replace, leaving the seed broken. Same + # fix as check_network / get_network. + rm -f "${MONITOR_FILE}" + echo -n "{}" > "${MONITOR_FILE}" + # Atomic-update (HIGH-8). + _TMP="${MONITOR_FILE}.tmp.$$" + /usr/bin/jq -n --arg v "On" '.monitor = $v' > "${_TMP}" && mv "${_TMP}" "${MONITOR_FILE}" || rm -f "${_TMP}" elif [ $actualsize -le $minimumsize ]; then - sudo rm ${MONITOR_FILE} - sudo echo -n "{}" ${MONITOR_FILE} - sudo chown dietpi:dietpi ${MONITOR_FILE} - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "On" '.monitor = $v' ${MONITOR_FILE}) > ${MONITOR_FILE} + rm -f "${MONITOR_FILE}" + echo -n "{}" > "${MONITOR_FILE}" + _TMP="${MONITOR_FILE}.tmp.$$" + /usr/bin/jq -n --arg v "On" '.monitor = $v' > "${_TMP}" && mv "${_TMP}" "${MONITOR_FILE}" || rm -f "${_TMP}" else MONITOR=$(sudo -H -u root bash -c "vcgencmd display_power") MONITOR=(${MONITOR##*=}) @@ -28,9 +36,11 @@ do fi if [ ${MONITOR} == "0" ] || [ ${POWER} == "4" ]; then - /usr/bin/cat <<< $(/usr/bin/jq --arg v "Off" '.monitor = $v' ${MONITOR_FILE}) > ${MONITOR_FILE} + _TMP="${MONITOR_FILE}.tmp.$$" + /usr/bin/jq --arg v "Off" '.monitor = $v' "${MONITOR_FILE}" > "${_TMP}" && mv "${_TMP}" "${MONITOR_FILE}" || rm -f "${_TMP}" elif [ ${MONITOR} == "1" ] || [ ${POWER} == "0" ]; then - /usr/bin/cat <<< $(/usr/bin/jq --arg v "On" '.monitor = $v' ${MONITOR_FILE}) > ${MONITOR_FILE} + _TMP="${MONITOR_FILE}.tmp.$$" + /usr/bin/jq --arg v "On" '.monitor = $v' "${MONITOR_FILE}" > "${_TMP}" && mv "${_TMP}" "${MONITOR_FILE}" || rm -f "${_TMP}" fi fi diff --git a/scripts/mupibox/get_network.sh b/scripts/mupibox/get_network.sh index 54b40eb1..1a309909 100644 --- a/scripts/mupibox/get_network.sh +++ b/scripts/mupibox/get_network.sh @@ -40,12 +40,18 @@ else rm ${RESUME_LOCK} fi -if [ ! -f ${NETWORKCONFIG} ]; then - sudo echo -n "[]" ${NETWORKCONFIG} - chown dietpi:dietpi ${NETWORKCONFIG} - chmod 777 ${NETWORKCONFIG} +if [ ! -f ${NETWORKCONFIG} ] || [ ! -s ${NETWORKCONFIG} ] || ! /usr/bin/jq -e 'type == "object"' ${NETWORKCONFIG} >/dev/null 2>&1; then + # HIGH-14 (Phase-3) + Phase-5 follow-up: same fix as in + # check_network.sh — drop sudo (dietpi can write /tmp) and seed + # as `{}` since network.json is an object. The guard also + # rebuilds an existing wrong-shape file from a prior broken + # session. + rm -f "${NETWORKCONFIG}" + echo -n "{}" > "${NETWORKCONFIG}" OLD_ONLINESTATE="starting" - /usr/bin/cat <<< $(/usr/bin/jq -n --arg v "starting" '.onlinestate = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} + # Atomic-update (HIGH-8). + _TMP="${NETWORKCONFIG}.tmp.$$" + /usr/bin/jq -n --arg v "starting" '.onlinestate = $v' > "${_TMP}" && mv "${_TMP}" "${NETWORKCONFIG}" || rm -f "${_TMP}" else OLD_ONLINESTATE=$(/usr/bin/jq -r .onlinestate ${NETWORKCONFIG}) fi @@ -86,14 +92,21 @@ IPA=$(/usr/bin/hostname -I | awk '{print $1}') DNS=$(echo $(sudo cat /etc/resolv.conf | grep 'nameserver ') | sed 's/nameserver //g') SUBNET=$(/sbin/ifconfig wlan0 | awk '/netmask/{split($4,a,":"); print a[1]}') -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${HOSTN}" '.host = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${IPA}" '.ip = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${MAC}" '.mac = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${WIFI}" '.wifi = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${WIFILINK}" '.wifilink = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${WIFISIGNAL}" '.wifisignal = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${GW}" '.gateway = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${DNS}" '.dns = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${SUBNET}" '.subnet = $v' ${NETWORKCONFIG}) > ${NETWORKCONFIG} +# Atomic-update (HIGH-8). Bundle all nine field updates into a single jq +# pipeline so we only do one tempfile-write-rename cycle, not nine — same +# correctness, ninth the SD-card writes. +_TMP="${NETWORKCONFIG}.tmp.$$" +/usr/bin/jq \ + --arg host "${HOSTN}" \ + --arg ip "${IPA}" \ + --arg mac "${MAC}" \ + --arg wifi "${WIFI}" \ + --arg wifilink "${WIFILINK}" \ + --arg wifisignal "${WIFISIGNAL}" \ + --arg gateway "${GW}" \ + --arg dns "${DNS}" \ + --arg subnet "${SUBNET}" \ + '.host = $host | .ip = $ip | .mac = $mac | .wifi = $wifi | .wifilink = $wifilink | .wifisignal = $wifisignal | .gateway = $gateway | .dns = $dns | .subnet = $subnet' \ + "${NETWORKCONFIG}" > "${_TMP}" && mv "${_TMP}" "${NETWORKCONFIG}" || rm -f "${_TMP}" #/usr/bin/cat <<< $(/usr/bin/jq --arg v "${HOSTN}" '."node-sonos-http-api".server = $v' ${FRONTENDCONFIG}) > ${FRONTENDCONFIG} #/usr/bin/cat <<< $(/usr/bin/jq --arg v "${IPA}" '."node-sonos-http-api".ip = $v' ${FRONTENDCONFIG}) > ${FRONTENDCONFIG} \ No newline at end of file diff --git a/scripts/mupibox/m3u_generator.sh b/scripts/mupibox/m3u_generator.sh index 74e89939..7722e869 100644 --- a/scripts/mupibox/m3u_generator.sh +++ b/scripts/mupibox/m3u_generator.sh @@ -65,11 +65,17 @@ else if [ -z "${searchStrTitleCover}" ] then + # Atomic-update (HIGH-8). m3u_generator runs after every + # media-tree change; previously a jq error or partial + # write while appending an entry truncated data.json + # to empty — wiping every album's metadata. Tempfile + # + rename preserves the original on failure. + _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi @@ -118,11 +124,13 @@ else if [ -z "${searchStrTitleCover}" ] then + # Atomic-update (HIGH-8) — same as the audiobook block above. + _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/music/'"${artist}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/music/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi @@ -171,11 +179,13 @@ else if [ -z "${searchStrTitleCover}" ] then + # Atomic-update (HIGH-8) — same as the audiobook/music blocks above. + _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/other/'"${artist}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/other/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/cat <<< $(/usr/bin/cat ${DATA} | /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg"}]') > ${DATA} + /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi diff --git a/scripts/mupibox/mupi_start_led.sh b/scripts/mupibox/mupi_start_led.sh index 457e4988..47e878a7 100644 --- a/scripts/mupibox/mupi_start_led.sh +++ b/scripts/mupibox/mupi_start_led.sh @@ -13,11 +13,15 @@ ledMin=$(/usr/bin/jq -r .shim.ledBrightnessMin ${MUPIBOX_CONFIG}) ledMin=$(printf '%d' "$ledMin") echo "{}" | tee ${TMP_LEDFILE} -/usr/bin/cat <<< $(/usr/bin/jq --argjson v ${ledPin} '.led_gpio = $v' ${TMP_LEDFILE}) > ${TMP_LEDFILE} -/usr/bin/cat <<< $(/usr/bin/jq --argjson v ${ledMax} '.led_max_brightness = $v' ${TMP_LEDFILE}) > ${TMP_LEDFILE} -/usr/bin/cat <<< $(/usr/bin/jq --argjson v ${ledMin} '.led_min_brightness = $v' ${TMP_LEDFILE}) > ${TMP_LEDFILE} -/usr/bin/cat <<< $(/usr/bin/jq '.led_current_brightness = 0' ${TMP_LEDFILE}) > ${TMP_LEDFILE} -/usr/bin/cat <<< $(/usr/bin/jq '.led_dim_mode = 0' ${TMP_LEDFILE}) > ${TMP_LEDFILE} +# Atomic-update (HIGH-8). Bundle five field updates into one jq invocation +# instead of cat<< "${_TMP}" && mv "${_TMP}" "${TMP_LEDFILE}" || rm -f "${_TMP}" /usr/bin/python3 /usr/local/bin/mupibox/led_control.py & #/usr/local/bin/mupibox/./led_control & @@ -63,7 +67,9 @@ do wled_data='{"bri":'${wled_brightness_def}'}' /usr/bin/python3 /usr/local/bin/mupibox/wled_send_data.py -s ${wled_com_port} -b ${wled_baud_rate} -j ${wled_data} fi - /usr/bin/cat <<< $(/usr/bin/jq '.led_dim_mode = 1' ${TMP_LEDFILE}) > ${TMP_LEDFILE} + # Atomic-update (HIGH-8). + _TMP="${TMP_LEDFILE}.tmp.$$" + /usr/bin/jq '.led_dim_mode = 1' "${TMP_LEDFILE}" > "${_TMP}" && mv "${_TMP}" "${TMP_LEDFILE}" || rm -f "${_TMP}" OLD_STATE=${displayState} elif [ ${displayState} -eq 0 ] && [ ${OLD_STATE} -ne ${displayState} ] then @@ -71,7 +77,8 @@ do wled_data='{"bri":'${wled_brightness_dim}'}' /usr/bin/python3 /usr/local/bin/mupibox/wled_send_data.py -s ${wled_com_port} -b ${wled_baud_rate} -j ${wled_data} fi - /usr/bin/cat <<< $(/usr/bin/jq '.led_dim_mode = 0' ${TMP_LEDFILE}) > ${TMP_LEDFILE} + _TMP="${TMP_LEDFILE}.tmp.$$" + /usr/bin/jq '.led_dim_mode = 0' "${TMP_LEDFILE}" > "${_TMP}" && mv "${_TMP}" "${TMP_LEDFILE}" || rm -f "${_TMP}" OLD_STATE=${displayState} fi done diff --git a/scripts/mupibox/repair_config.sh b/scripts/mupibox/repair_config.sh index 631e5e9b..a79a0ed4 100644 --- a/scripts/mupibox/repair_config.sh +++ b/scripts/mupibox/repair_config.sh @@ -10,5 +10,7 @@ rm ${FRONTENDCONFIG} wget ${SRC}/config/templates/www.json -O ${FRONTENDCONFIG} /usr/local/bin/mupibox/./set_hostname.sh -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${HOSTN}" '."node-sonos-http-api".server = $v' ${FRONTENDCONFIG}) > ${FRONTENDCONFIG} +# Atomic-update (HIGH-8). +_TMP="${FRONTENDCONFIG}.tmp.$$" +/usr/bin/jq --arg v "${HOSTN}" '."node-sonos-http-api".server = $v' "${FRONTENDCONFIG}" > "${_TMP}" && mv "${_TMP}" "${FRONTENDCONFIG}" || rm -f "${_TMP}" chown dietpi:dietpi ${FRONTENDCONFIG} diff --git a/scripts/mupibox/set_deviceid.sh b/scripts/mupibox/set_deviceid.sh index 869124e3..58bc965b 100644 --- a/scripts/mupibox/set_deviceid.sh +++ b/scripts/mupibox/set_deviceid.sh @@ -7,9 +7,25 @@ HOSTNAME=$(sudo /usr/bin/jq -r .mupibox.host ${CONFIG}) DEVICES=$(curl http://${HOSTNAME}:5005/getDevices 2>/dev/null) devID=$(echo ${DEVICES} | jq '.[] | select(.name=='\"${HOSTNAME}\"')' | jq '.id') devID=$(echo ${devID} | sed 's/\"//g') -if [ ${#devID} > 5 ]; +# HIGH-9: previous `[ ${#devID} > 5 ]` was a string-test of `${#devID}` +# (always non-empty so always true) plus a stray `>` redirect that +# silently created a file named `5`. Use the numeric test `-gt`. +if [ "${#devID}" -gt 5 ]; then - sudo /usr/bin/cat <<< $(/usr/bin/jq --arg v "${devID}" '.spotify.deviceId = $v' ${CONFIG}) > ${CONFIG} + # HIGH-8 + HIGH-10: previous `sudo /usr/bin/cat <<< $(jq …) > ${CONFIG}` + # had two bugs at once. (a) the `>` redirect runs as the calling user + # not as root, so writing to root-owned ${CONFIG} would EACCES at the + # redirect step, leaving CONFIG unchanged but the script silently + # "succeeding". (b) the cat<<< would truncate ${CONFIG} on jq error. + # New approach: stage the new content in /tmp (always writable for + # dietpi), then `sudo install` it onto ${CONFIG} preserving owner + + # mode in one atomic syscall. Same pattern as admin.php's + # write_json() and jsoneditor.php's atomic save. + _TMP="/tmp/.deviceid.$$.json" + if /usr/bin/jq --arg v "${devID}" '.spotify.deviceId = $v' "${CONFIG}" > "${_TMP}"; then + sudo install -m 644 -o root -g www-data "${_TMP}" "${CONFIG}" + fi + rm -f "${_TMP}" fi sudo /usr/local/bin/mupibox/./setting_update.sh diff --git a/scripts/mupibox/set_hostname.sh b/scripts/mupibox/set_hostname.sh index a5d9157b..7ee98f35 100644 --- a/scripts/mupibox/set_hostname.sh +++ b/scripts/mupibox/set_hostname.sh @@ -6,4 +6,6 @@ FRONTENDCONFIG="/home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config HOSTN=$(/usr/bin/hostname) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${HOSTN}" '."node-sonos-http-api".server = $v' ${FRONTENDCONFIG}) > ${FRONTENDCONFIG} \ No newline at end of file +# Atomic-update (HIGH-8). +_TMP="${FRONTENDCONFIG}.tmp.$$" +/usr/bin/jq --arg v "${HOSTN}" '."node-sonos-http-api".server = $v' "${FRONTENDCONFIG}" > "${_TMP}" && mv "${_TMP}" "${FRONTENDCONFIG}" || rm -f "${_TMP}" \ No newline at end of file diff --git a/scripts/mupibox/setting_update.sh b/scripts/mupibox/setting_update.sh index be2926e9..6d80f1f1 100644 --- a/scripts/mupibox/setting_update.sh +++ b/scripts/mupibox/setting_update.sh @@ -19,45 +19,65 @@ then ln -s /home/dietpi/MuPiBox/themes/${NEW_THEME}.css ${THEME_FILE} fi +# Atomic-update (HIGH-8). Read all source values first, then bundle the +# writes into one jq invocation per target file. Each cat<<< pattern was +# its own truncate-race window; collapsing to one pipeline per file means +# 16+ tempfile cycles → 2 cycles, plus far less SD wear. deviceId=$(/usr/bin/jq -r .spotify.deviceId ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${deviceId}" '.["node-sonos-http-api"].rooms = [$v]' ${SONOS_CONFIG}) > ${SONOS_CONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${deviceId}" '.spotify.deviceId = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - clientId=$(/usr/bin/jq -r .spotify.clientId ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${clientId}" '.spotify.clientId = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${clientId}" '.spotify.clientId = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - clientSecret=$(/usr/bin/jq -r .spotify.clientSecret ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${clientSecret}" '.spotify.clientSecret = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${clientSecret}" '.spotify.clientSecret = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - accessToken=$(/usr/bin/jq -r .spotify.accessToken ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${accessToken}" '.spotify.accessToken = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - refreshToken=$(/usr/bin/jq -r .spotify.refreshToken ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "$refreshToken" '.spotify.refreshToken = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - ttsLanguage=$(/usr/bin/jq -r .mupibox.ttsLanguage ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "$ttsLanguage" '.ttsLanguage = $v' ${SPOTIFYCONTROLLER_CONFIG}) > ${SPOTIFYCONTROLLER_CONFIG} - hostname=$(/usr/bin/jq -r .mupibox.host ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --arg v "${hostname}" '.["node-sonos-http-api"].server = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} - hat_active=$(/usr/bin/jq -r .mupihat.hat_active ${MUPIBOX_CONFIG}) -/usr/bin/cat <<< $(/usr/bin/jq --argjson v "$hat_active" '.["node-sonos-http-api"].hat_active = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} - ip_control_backend=$(/usr/bin/jq -r .mupibox.ip_control_backend ${MUPIBOX_CONFIG}) + +# Resolve the IP once so the SONOS_CONFIG update can be a single pipeline +# regardless of ip_control_backend. Default to "" (= same as the +# old false-branch behaviour). +SONOS_IP="" if [ "$ip_control_backend" = true ] ; then - IP=$(hostname -I | sed 's/ *$//') - if [ "$IP" = "" ] ; then - IP=$(/usr/bin/jq -r .ip ${NETWORK_CONFIG}) - fi - /usr/bin/cat <<< $(/usr/bin/jq --arg v "${IP}" '.["node-sonos-http-api"].ip = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} + SONOS_IP=$(hostname -I | sed 's/ *$//') + if [ -z "$SONOS_IP" ] ; then + SONOS_IP=$(/usr/bin/jq -r .ip ${NETWORK_CONFIG}) + fi fi -if [ "$ip_control_backend" = false ] ; then - /usr/bin/cat <<< $(/usr/bin/jq '.["node-sonos-http-api"].ip = ""' ${SONOS_CONFIG}) > ${SONOS_CONFIG} -fi -/usr/bin/cat <<< $(/usr/bin/jq --arg v "5005" '.["node-sonos-http-api"].port = $v' ${SONOS_CONFIG}) > ${SONOS_CONFIG} + +# One atomic update for SONOS_CONFIG (was 6 cat<< "${_TMP}" && mv "${_TMP}" "${SONOS_CONFIG}" || rm -f "${_TMP}" + +# One atomic update for SPOTIFYCONTROLLER_CONFIG (was 6 cat<< "${_TMP}" && mv "${_TMP}" "${SPOTIFYCONTROLLER_CONFIG}" || rm -f "${_TMP}" #cachepath=$(/usr/bin/jq -r .spotify.cachepath ${MUPIBOX_CONFIG}) #/usr/bin/sed -i 's@.*cache_path.*@ cache_path = "'${cachepath}'"@g' ${SPOTIFYD_CONFIG} From 077678c0696163cd136f6ecd87fc9bf2a15d5233 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 22:16:47 +0200 Subject: [PATCH 02/14] add_wifi.sh: pin jq to first entry + escape SSID before writing wpa_supplicant.conf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two distinct bugs in the wifi-add loop, both reachable from the frontend's "save WiFi" form. HIGH-11: `jq -r .[].ssid ${MUPIWIFI}` iterated the wlan.json array and emitted every entry's SSID as newline-separated values. With more than one queued WiFi (admin saves twice quickly, or a corrupt wlan.json file with leftover entries), the SSID variable became a multi-line string. The downstream `wpa_passphrase "${SSID}" "${PSK}"` invocation got a multi-line first arg and wrote garbage into wpa_supplicant.conf. Pin to `.[0].ssid // empty` so we always handle exactly one entry per loop iteration; the existing `sudo rm ${MUPIWIFI}` at the bottom of the iteration drains queued entries on subsequent passes. HIGH-12: the open-network branch wrote `ssid="${SSID}"` unquoted into wpa_supplicant.conf via tee. An SSID containing `"` or a newline would break out of the quoted ssid value and let the caller inject arbitrary `network={…}` blocks into a root-owned config file (a security boundary because wpa_supplicant runs as root). Reject SSIDs with literal newlines or NUL up front, escape `\` and `"` for the wpa_supplicant. conf string format, and emit via printf %s rather than echo so the shell can't reinterpret the value mid-flight. PSK-network branch was already safe — wpa_passphrase escapes its arguments internally. --- scripts/mupibox/add_wifi.sh | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/scripts/mupibox/add_wifi.sh b/scripts/mupibox/add_wifi.sh index 40514615..8722ba4c 100644 --- a/scripts/mupibox/add_wifi.sh +++ b/scripts/mupibox/add_wifi.sh @@ -22,8 +22,16 @@ while true do if test -f "${MUPIWIFI}" then - SSID="$(/usr/bin/jq -r .[].ssid ${MUPIWIFI})" - PSK="$(/usr/bin/jq -r .[].pw ${MUPIWIFI})" + # HIGH-11: previous filter was `jq -r .[].ssid` which iterates and + # emits N newline-separated SSIDs when the array has N entries. + # wpa_passphrase / the "network={ ssid=…" emit below then receive + # a multi-line string and write garbage into wpa_supplicant.conf. + # The frontend always queues exactly one entry per save event, so + # pin to index 0; if there are leftover entries they're picked up + # on the next loop iteration after `sudo rm ${MUPIWIFI}` clears + # the file. + SSID="$(/usr/bin/jq -r '.[0].ssid // empty' ${MUPIWIFI})" + PSK="$(/usr/bin/jq -r '.[0].pw // empty' ${MUPIWIFI})" if [ "${SSID}" = "" ] then sudo rm ${MUPIWIFI} @@ -44,11 +52,23 @@ do restart_network elif [ "${PSK}" = "" ] then - echo 'network={' | sudo tee -a ${WPACONF} - echo ' ssid="'${SSID}'"' | sudo tee -a ${WPACONF} - echo ' scan_ssid=1' | sudo tee -a ${WPACONF} - echo '}' | sudo tee -a ${WPACONF} - restart_network + # HIGH-12: previous code spliced ${SSID} unquoted into a + # single-quoted echo, so an SSID containing `"` or a newline + # could escape the quoted string and inject arbitrary blocks + # into wpa_supplicant.conf (root-owned, security-relevant). + # Use wpa_passphrase's open-network shape via printf with %s, + # which can't be reinterpreted by the shell, then escape any + # embedded `"` for the JSON-style ssid field. + # Reject SSIDs containing characters wpa_supplicant.conf can't + # represent (newlines / NUL) outright. + if [[ "${SSID}" == *$'\n'* || "${SSID}" == *$'\0'* ]]; then + echo "add_wifi.sh: SSID rejected (newline / NUL in name)" >&2 + else + _ESCAPED_SSID="${SSID//\\/\\\\}" + _ESCAPED_SSID="${_ESCAPED_SSID//\"/\\\"}" + printf 'network={\n\tssid="%s"\n\tscan_ssid=1\n}\n' "${_ESCAPED_SSID}" | sudo tee -a ${WPACONF} >/dev/null + restart_network + fi else WIFI_RESULT=$(sudo -i wpa_passphrase "${SSID}" "${PSK}") IFS=$'\n' From d2396b6067f034fc0a286c4082a4b119ef3a59d5 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 22:17:09 +0200 Subject: [PATCH 03/14] optimize_wifi.sh: rm -f for the .bak files so first-run doesn't lose the backup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rm "$file.bak"` errors when the .bak file doesn't exist (which is exactly the case on a fresh box's first run of optimize_wifi.sh). Today the error is only stderr noise, but if anyone later tightens this script with `set -e` the rm would abort before the cp can create the new backup — leaving no rollback path while we mutate /etc/network/interfaces and /etc/wpa_supplicant/wpa_supplicant.conf as root. `rm -f` is idempotent: silently does nothing on missing file. --- scripts/mupibox/optimize_wifi.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scripts/mupibox/optimize_wifi.sh b/scripts/mupibox/optimize_wifi.sh index bab551ba..debb95dc 100644 --- a/scripts/mupibox/optimize_wifi.sh +++ b/scripts/mupibox/optimize_wifi.sh @@ -27,15 +27,20 @@ add_config() { # /etc/network/interfaces if ! grep -q "wpa-roam /etc/wpa_supplicant/wpa_supplicant.conf" "$NETWORKINTERFACES"; then - rm "$NETWORKINTERFACES.bak" + # HIGH-13: `rm "$file.bak"` without -f errors on first run when no + # .bak exists. Without `set -e` it's just a stderr noise; with set -e + # (e.g. anyone tightening this script later) it would abort BEFORE + # the cp can create the new backup, leaving no backup at all. Use -f + # so the rm is idempotent. + rm -f "$NETWORKINTERFACES.bak" cp "$NETWORKINTERFACES" "$NETWORKINTERFACES.bak" sed -i 's|wpa-conf /etc/wpa_supplicant/wpa_supplicant.conf|wpa-roam /etc/wpa_supplicant/wpa_supplicant.conf|' $NETWORKINTERFACES sed -i 's|iface wlan0 inet dhcp|iface wlan0 inet manual|' $NETWORKINTERFACES echo "iface default inet dhcp" | tee -a $NETWORKINTERFACES > /dev/null fi -# /etc/wpa_supplicant/wpa_supplicant -rm "$WPACONF.bak" +# /etc/wpa_supplicant/wpa_supplicant — same idempotency fix (HIGH-13). +rm -f "$WPACONF.bak" cp "$WPACONF" "$WPACONF.bak" add_config 'bgscan="simple:30:-70:60"' #add_config 'roam_timeout=5' From d57f6576dfd783bc0428a07f4cbfe96b9727b2c0 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 22:17:59 +0200 Subject: [PATCH 04/14] add_index.sh: trap-cleanup for DATA_LOCK so a crash doesn't wedge the indexer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lock-file pattern was `touch DATA_LOCK; …work…; rm DATA_LOCK` with no signal handler. If perl, jq, or grep died mid-pass — out of disk, malformed data.json, the box getting SIGTERM during shutdown — the final `rm DATA_LOCK` never ran. Subsequent invocations see the stale lock at the top of the script and exit immediately, so the index never gets regenerated. Symptom on the box: a freshly-added audiobook folder shows up in the cover view but doesn't get an `index` field, which means resume / track-selection logic can't address it. Add `trap 'rm -f "${DATA_LOCK}"' EXIT INT TERM HUP` right after touching the lock so the cleanup runs on any script-exit path. The trailing explicit `rm ${DATA_LOCK}` is now redundant but kept (under a comment) to preserve the "Index is finished" log ordering people may grep for. --- scripts/mupibox/add_index.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/mupibox/add_index.sh b/scripts/mupibox/add_index.sh index ee18cacd..086ab3bf 100644 --- a/scripts/mupibox/add_index.sh +++ b/scripts/mupibox/add_index.sh @@ -18,6 +18,14 @@ if [ -f "${DATA_LOCK}" ]; then exit else touch ${DATA_LOCK} + # HIGH-15: previous code had no trap, so a perl/jq crash, a SIGTERM + # from systemd, or an out-of-disk during the temp writes left + # DATA_LOCK behind. Subsequent invocations would see the stale lock + # and exit, blocking m3u_generator's index pass forever — observable + # as "new audiobook folder shows up but never gets an index, so + # resume can't address it." Trap the common termination signals so + # the lock is removed even when the script dies mid-write. + trap 'rm -f "${DATA_LOCK}"' EXIT INT TERM HUP /usr/bin/cat ${DATA} | grep -v '"index":' > ${TMP_DATA} /usr/bin/perl -pe 'BEGIN{$k=-1};s/{/$& . "\n \"index\": " . ++$k . ","/e' ${TMP_DATA} > ${DATA} @@ -26,5 +34,6 @@ else /usr/bin/mv ${TMP_DATA} ${DATA} /usr/bin/chown dietpi:dietpi ${DATA} echo "Index is finished" - rm ${DATA_LOCK} + # Lock removal happens via the EXIT trap above; the explicit rm here + # is now redundant but kept to preserve the original log-line ordering. fi \ No newline at end of file From 339e04295e62236a2591423d6adef28221e921d1 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 22:18:14 +0200 Subject: [PATCH 05/14] change_checker.sh: default CHECK_TIMER to 60s when config key is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `jq -r .mupibox.mediaCheckTimer` on a config that doesn't have the key yields the literal string "null". `sleep null` returns immediately ("invalid time interval"), so the while-true loop iterated as fast as the CPU could go — a fresh-image box with no config save yet would peg one core at 100%, ramp the fan, and waste battery. Add a guard that substitutes 60s when CHECK_TIMER is empty or "null". 60s matches the cadence the audit / setup-docs assume m3u_generator runs at. --- scripts/mupibox/change_checker.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/mupibox/change_checker.sh b/scripts/mupibox/change_checker.sh index 0bddb146..bb8b7ff5 100644 --- a/scripts/mupibox/change_checker.sh +++ b/scripts/mupibox/change_checker.sh @@ -4,6 +4,14 @@ CONFIG="/etc/mupibox/mupiboxconfig.json" CHECK_TIMER=$(/usr/bin/jq -r .mupibox.mediaCheckTimer ${CONFIG}) +# MED-22: a fresh image without `mediaCheckTimer` in mupiboxconfig.json +# gets `null` from jq -r, so `sleep null` returns immediately and the +# while loop pegs one CPU at 100% — observable on a fresh box as the +# system fan ramping up before the first config save. Default to 60s +# (same cadence m3u_generator.sh runs at on real boxes). +if [ -z "${CHECK_TIMER}" ] || [ "${CHECK_TIMER}" = "null" ]; then + CHECK_TIMER=60 +fi while true do From 7a7cca307471d231e05f92d6d65e9b005a7e1401 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 20:00:32 +0200 Subject: [PATCH 06/14] check_network.sh: fix broken jq filter so offline mode shows local audiobooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filter that builds offline_data.json / offline_resume.json read jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' Two issues: an unclosed paren after `"radio"` and a stray pipe inside the select() instead of between selects. jq exits with a parse error, the script swallows it via `$(...)` capture, and offline_data.json ends up as `[]` — the offline-mode UI then has nothing to show, even though local audiobooks are sitting on disk. Reported anecdotally by parents whose box loses Wi-Fi and "all my child's audiobooks are gone." Replace all six identical occurrences with a single combined select(): jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' Same intent (drop online-only entries) but actually parses, and reads clearer than three chained selects. --- scripts/mupibox/check_network.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/mupibox/check_network.sh b/scripts/mupibox/check_network.sh index cedc3a00..79c1c8f5 100644 --- a/scripts/mupibox/check_network.sh +++ b/scripts/mupibox/check_network.sh @@ -88,39 +88,39 @@ do ONLINESTATE=${FALSESTATE} if [ ! -f ${OFFLINE_FILE} ]; then echo -n "[" > ${OFFLINE_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} echo -n "]" >> ${OFFLINE_FILE} sed -i 's/} {/}, {/g' ${OFFLINE_FILE} chown dietpi:dietpi ${OFFLINE_FILE} elif [ ! -s ${OFFLINE_FILE} ]; then rm ${OFFLINE_FILE} echo -n "[" > ${OFFLINE_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} echo -n "]" >> ${OFFLINE_FILE} sed -i 's/} {/}, {/g' ${OFFLINE_FILE} chown dietpi:dietpi ${OFFLINE_FILE} elif [ $(stat --format='%Y' "${DATA_FILE}") -gt $(stat --format='%Y' "${OFFLINE_FILE}") ]; then echo -n "[" > ${OFFLINE_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${DATA_FILE}) >> ${OFFLINE_FILE} echo -n "]" >> ${OFFLINE_FILE} sed -i 's/} {/}, {/g' ${OFFLINE_FILE} fi if [ ! -f ${OFFLINERESUME_FILE} ]; then echo -n "[" > ${OFFLINERESUME_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} echo -n "]" >> ${OFFLINERESUME_FILE} sed -i 's/} {/}, {/g' ${OFFLINERESUME_FILE} chown dietpi:dietpi ${OFFLINERESUME_FILE} elif [ ! -s ${OFFLINERESUME_FILE} ]; then rm ${OFFLINERESUME_FILE} echo -n "[" > ${OFFLINERESUME_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} echo -n "]" >> ${OFFLINERESUME_FILE} sed -i 's/} {/}, {/g' ${OFFLINERESUME_FILE} chown dietpi:dietpi ${OFFLINERESUME_FILE} elif [ $(stat --format='%Y' "${RESUME_FILE}") -gt $(stat --format='%Y' "${OFFLINERESUME_FILE}") ]; then echo -n "[" > ${OFFLINERESUME_FILE} - echo -n $(jq '.[] | select(.type != "spotify") | select(.type != "radio" | select(.type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} + echo -n $(jq '.[] | select(.type != "spotify" and .type != "radio" and .type != "rss")' < ${RESUME_FILE}) >> ${OFFLINERESUME_FILE} echo -n "]" >> ${OFFLINERESUME_FILE} sed -i 's/} {/}, {/g' ${OFFLINERESUME_FILE} fi From a3d41336e05d6afed89918fcf225241537eb2868 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 23:26:03 +0200 Subject: [PATCH 07/14] check_network.sh: self-healing symlink reconciliation (Phase 4.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous symlink logic only fired on a state TRANSITION between ticks: `if [ "${ONLINESTATE}" != "${OLDSTATE}" ]`. After a process restart while the box was already online, the loop's first iteration saw ONLINESTATE=online with OLDSTATE=starting, entered the block, but then hit `[[ ${OLD_ONLINESTATE} != "online" ]]` — and OLD_ONLINESTATE was loaded once from the persisted /tmp/network.json, which already held "online" from the previous session. Result: the inner branch never ran, the symlink was left pointing wherever it was (often at offline_data.json from a prior offline session), and the API served the offline shape forever. Visible only after Phase 3's HIGH-19 jq-filter fix made offline_data .json actually contain (correctly-filtered, Spotify-less) entries. Before HIGH-19 the file was empty so a stale symlink read as "no data" — annoying but indistinguishable from the box just being slow. Post-HIGH-19 the symlink-to-offline rendered local-only content even when online: user reported "no Spotify albums anymore." Replace with an idempotent ensure_symlink helper that does "reconcile to expected target" every tick. Cheap when no-op (one readlink syscall), self-healing when wrong, and removes the ONLINESTATE-vs-OLDSTATE / OLD_ONLINESTATE coordination problem entirely. The OLDSTATE comparison further down (for the network.json .onlinestate write) is unaffected; only the symlink branch is reworked. --- scripts/mupibox/check_network.sh | 65 ++++++++++++++------------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/scripts/mupibox/check_network.sh b/scripts/mupibox/check_network.sh index 79c1c8f5..47674868 100644 --- a/scripts/mupibox/check_network.sh +++ b/scripts/mupibox/check_network.sh @@ -62,28 +62,36 @@ fi #wget -q --spider http://google.com +# Idempotent symlink reconciliation. Always points $link at $target — +# if the link already points there, no-op. Replaces the previous +# state-transition-only logic that depended on detecting a change +# from OLDSTATE → ONLINESTATE; that logic missed the case where +# check_network.sh starts up in a state that already matches stored +# OLD_ONLINESTATE but where the on-disk symlink is still pointing at +# the wrong target (e.g. after a pm2 restart while box was already +# online — both ONLINESTATE and OLD_ONLINESTATE = "online", no flip +# fired, but the symlink may still be pointing at offline_data.json +# from a prior offline session). Symptom: active_data.json never +# resolved to data.json post-reboot, so the API served the offline +# (Spotify-less) shape even though the box was clearly online. +ensure_symlink() { + local target="$1" + local link="$2" + if [ ! -L "$link" ] || [ "$(readlink "$link")" != "$target" ]; then + rm -f "$link" + ln -s "$target" "$link" + chown -h dietpi:dietpi "$link" 2>/dev/null || sudo chown -h dietpi:dietpi "$link" + fi +} + while true do if ( $(/usr/bin/python3 /usr/local/bin/mupibox/check_network.py) == ${TRUESTATE} ); then ONLINESTATE=${TRUESTATE} - if [ "${ONLINESTATE}" != "${OLDSTATE}" ]; then - if [ ! -f ${ACTIVE_FILE} ]; then - ln -s ${DATA_FILE} ${ACTIVE_FILE} - chown dietpi:dietpi ${ACTIVE_FILE} - elif [[ ${OLD_ONLINESTATE} != "online" ]]; then - rm ${ACTIVE_FILE} - ln -s ${DATA_FILE} ${ACTIVE_FILE} - chown dietpi:dietpi ${ACTIVE_FILE} - fi - if [ ! -f ${ACTIVERESUME_FILE} ]; then - ln -s ${RESUME_FILE} ${ACTIVERESUME_FILE} - chown dietpi:dietpi ${ACTIVERESUME_FILE} - elif [[ ${OLD_ONLINESTATE} != "online" ]]; then - rm ${ACTIVERESUME_FILE} - ln -s ${RESUME_FILE} ${ACTIVERESUME_FILE} - chown dietpi:dietpi ${ACTIVERESUME_FILE} - fi - fi + # Reconcile every tick (cheap when no-op) instead of only on + # state change. Self-healing if the symlink was wrong. + ensure_symlink "${DATA_FILE}" "${ACTIVE_FILE}" + ensure_symlink "${RESUME_FILE}" "${ACTIVERESUME_FILE}" else ONLINESTATE=${FALSESTATE} if [ ! -f ${OFFLINE_FILE} ]; then @@ -124,24 +132,9 @@ do echo -n "]" >> ${OFFLINERESUME_FILE} sed -i 's/} {/}, {/g' ${OFFLINERESUME_FILE} fi - if [ "${ONLINESTATE}" != "${OLDSTATE}" ]; then - if [ ! -f ${ACTIVE_FILE} ]; then - ln -s ${OFFLINE_FILE} ${ACTIVE_FILE} - chown dietpi:dietpi ${ACTIVE_FILE} - elif [[ ${OLD_ONLINESTATE} != "offline" ]]; then - rm ${ACTIVE_FILE} - ln -s ${OFFLINE_FILE} ${ACTIVE_FILE} - chown dietpi:dietpi ${ACTIVE_FILE} - fi - if [ ! -f ${ACTIVERESUME_FILE} ]; then - ln -s ${OFFLINERESUME_FILE} ${ACTIVERESUME_FILE} - chown dietpi:dietpi ${ACTIVERESUME_FILE} - elif [[ ${OLD_ONLINESTATE} != "offline" ]]; then - rm ${ACTIVERESUME_FILE} - ln -s ${OFFLINERESUME_FILE} ${ACTIVERESUME_FILE} - chown dietpi:dietpi ${ACTIVERESUME_FILE} - fi - fi + # Self-healing reconciliation, see ensure_symlink comment above. + ensure_symlink "${OFFLINE_FILE}" "${ACTIVE_FILE}" + ensure_symlink "${OFFLINERESUME_FILE}" "${ACTIVERESUME_FILE}" fi if [ "${ONLINESTATE}" != "${OLDSTATE}" ]; then From f0487f5f40560423af4a20afb49019d9be332566 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:26:04 +0200 Subject: [PATCH 08/14] bash one-liners: idle_shutdown true-vs-truthy + librespot-start typo (MED-3 / LOW-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two trivial bash one-liners that broke quietly enough that nobody noticed until the audit. MED-3: idle_shutdown.sh tested `[ "${TELEGRAM}" ]` which is bash's "non-empty string" check. jq -r on a bool emits the literal strings "true" or "false" — both of which are non-empty. So `telegram.active = false` in mupiboxconfig.json was treated as "telegram is enabled" and the box still tried to fire a Telegram notification at idle- shutdown. Compare explicitly to "true". LOW-1: librespot-start.sh tested `[[ ! -z "$var" ]]` where `$var` was never set anywhere in the script. The test was therefore always true under set -u -less defaults (treats unset as empty), so LIBRESPOT_USERNAME got exported as `null` (jq -r's missing-key output) even when the user had a real username, leading to authl flailing later. Test `$username` (the actual variable) and filter the `null` literal. --- scripts/librespot/librespot-start.sh | 8 +++++++- scripts/mupibox/idle_shutdown.sh | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/librespot/librespot-start.sh b/scripts/librespot/librespot-start.sh index c9e2a3af..e5d45854 100644 --- a/scripts/librespot/librespot-start.sh +++ b/scripts/librespot/librespot-start.sh @@ -16,7 +16,13 @@ if ! $cachestate ; then export LIBRESPOT_NAME_DISABLE_AUDIO_CACHE=1 fi username=$( jq -r .username ${LIBRESPOT_CACHE}/credentials.json ) -if [[ ! -z "$var" ]]; then +# LOW-1: previous test was `[[ ! -z "$var" ]]` — `$var` was never set +# anywhere in this script, so the test was always false (or always true +# depending on shell semantics for unset-vs-empty), and LIBRESPOT_USERNAME +# never got exported even when credentials.json had a real username. +# Test the actual username variable, plus filter "null" since jq -r emits +# that literal for missing keys. +if [[ -n "$username" && "$username" != "null" ]]; then export LIBRESPOT_USERNAME=${username} fi diff --git a/scripts/mupibox/idle_shutdown.sh b/scripts/mupibox/idle_shutdown.sh index d4c0bfbe..dacd9017 100644 --- a/scripts/mupibox/idle_shutdown.sh +++ b/scripts/mupibox/idle_shutdown.sh @@ -27,7 +27,12 @@ do TELEGRAM=$(/usr/bin/jq -r .telegram.active ${CONFIG}) TELEGRAM_CHATID=$(/usr/bin/jq -r .telegram.chatId ${CONFIG}) TELEGRAM_TOKEN=$(/usr/bin/jq -r .telegram.token ${CONFIG}) - if [ "${TELEGRAM}" ] && [ ${#TELEGRAM_CHATID} -ge 1 ] && [ ${#TELEGRAM_TOKEN} -ge 1 ]; then + # MED-3: previous test was `[ "${TELEGRAM}" ]` which evaluates to + # true for ANY non-empty string — including "false". jq emits + # "true"/"false" as literals from a bool field, so `telegram.active=false` + # was treated as "telegram is on" and the box still tried to shoot + # off a Telegram message at idle-shutdown. Compare explicitly to "true". + if [ "${TELEGRAM}" = "true" ] && [ ${#TELEGRAM_CHATID} -ge 1 ] && [ ${#TELEGRAM_TOKEN} -ge 1 ]; then /usr/bin/python3 /usr/local/bin/mupibox/telegram_send_message.py "MuPiBox is to long idle" fi echo "$(date +'%d/%m/%Y %H:%M:%S') # CURRENT IDLE TIME = ${idle}" >> ${LOG} From 5138b96a825f00faab2534a09c269735608e314c Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:32:15 +0200 Subject: [PATCH 09/14] service.php: serialise apt-get calls behind flock (LOW-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five apt-get-touching service toggles (VNC enable/disable, Samba enable/disable, FTP enable/disable, plus an apt-get install in the FTP-enable path) all just exec'd directly. dpkg has an exclusive lock at /var/lib/dpkg/lock-frontend, so any concurrent invocation fails with "Could not get lock" — but PHP's exec discards stderr by default, the page swallows the failure, and the user gets a "Service enabled" toast for an action that didn't actually run. Wrap each apt-touching command in `flock -w 120 /tmp/.mupibox.apt.lock bash -c ''`. flock holds an advisory lock and waits up to 120 seconds for it to be free, so a second admin click queues behind the first instead of silently failing. Lock file under /tmp so it auto-cleans on reboot. systemctl-only paths (BT-autoconnect, vnc disable's stop steps) don't go through apt and stay unwrapped. --- AdminInterface/www/service.php | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/AdminInterface/www/service.php b/AdminInterface/www/service.php index d78627ca..696777a1 100644 --- a/AdminInterface/www/service.php +++ b/AdminInterface/www/service.php @@ -3,13 +3,26 @@ $dataonline = json_decode($onlinejson, true); include ('includes/header.php'); + // LOW-3: every apt-get path here can collide with another concurrent + // service-toggle (admin clicks "enable VNC" while "enable Samba" is + // still running). dpkg has its own exclusive lock at + // /var/lib/dpkg/lock-frontend, so the second request fails with a + // confusing error in stderr but the PHP page just says "VNC enabled" + // — the user has no idea it didn't actually run. Serialise apt-touching + // commands behind a flock so the second one waits up to 120s for the + // first to finish, and log if the wait expires. + $APT_LOCK = '/tmp/.mupibox.apt.lock'; + $aptWrap = function ($cmd) use ($APT_LOCK) { + return "flock -w 120 " . escapeshellarg($APT_LOCK) . " bash -c " . escapeshellarg($cmd); + }; + if( $_POST['change_vnc'] == "stop & disable" ) { exec("sudo systemctl stop mupi_vnc.service"); exec("sudo systemctl stop mupi_novnc.service"); exec("sudo systemctl disable mupi_vnc.service"); exec("sudo systemctl disable mupi_novnc.service"); - exec("sudo apt-get remove x11vnc websockify -y"); + exec($aptWrap("sudo apt-get remove x11vnc websockify -y")); exec("sudo pkill websockify"); exec("sudo rm -R /usr/share/novnc"); exec("sudo su - -c \"/usr/bin/cat <<< $(/usr/bin/jq --arg v \"0\" '.tweaks.vnc = $v' /etc/mupibox/mupiboxconfig.json) > /etc/mupibox/mupiboxconfig.json\""); @@ -18,7 +31,7 @@ } else if( $_POST['change_vnc'] == "enable & start" ) { - exec("sudo apt-get install x11vnc websockify -y"); + exec($aptWrap("sudo apt-get install x11vnc websockify -y")); exec("sudo git clone https://github.com/novnc/noVNC.git /usr/share/novnc"); exec("sudo chown -R dietpi:dietpi /usr/share/novnc"); exec("sudo systemctl enable mupi_vnc.service"); @@ -33,14 +46,14 @@ if( $_POST['change_samba'] == "enable & start" ) { $command = "sudo apt-get install samba -y && sudo wget https://raw.githubusercontent.com/splitti/MuPiBox/main/config/templates/smb.conf -O /etc/samba/smb.conf && sudo systemctl enable smbd.service && sudo systemctl start smbd.service"; - exec($command, $output, $result ); + exec($aptWrap($command), $output, $result ); $change=1; $CHANGE_TXT=$CHANGE_TXT."
  • Samba enabled
    • "; } else if( $_POST['change_samba'] == "stop & disable" ) { $command = "sudo systemctl stop smbd.service && sudo systemctl disable smbd.service && sudo apt-get remove samba -y"; - exec($command, $output, $result ); + exec($aptWrap($command), $output, $result ); $change=1; $CHANGE_TXT=$CHANGE_TXT."
  • Samba disabled
    • "; } @@ -48,14 +61,14 @@ if( $_POST['change_ftp'] == "enable & start" ) { $command = " sudo apt-get install proftpd -y && sudo apt-get install samba -y && sudo wget https://raw.githubusercontent.com/splitti/MuPiBox/main/config/templates/proftpd.conf -O /etc/proftpd/proftpd.conf && sudo systemctl restart proftpd"; - exec($command, $output, $result ); + exec($aptWrap($command), $output, $result ); $change=1; $CHANGE_TXT=$CHANGE_TXT."
  • FTP enabled
    • "; } else if( $_POST['change_ftp'] == "stop & disable" ) { $command = "sudo systemctl stop proftpd.service && sudo systemctl disable proftpd.service && sudo apt-get remove proftpd -y"; - exec($command, $output, $result ); + exec($aptWrap($command), $output, $result ); $change=1; $CHANGE_TXT=$CHANGE_TXT."
  • FTP disabled
    • "; } From c1efcab4c788e21ad3f6bdf8145545824c7d3dce Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 10:10:54 +0200 Subject: [PATCH 10/14] bash robustness round: m3u_generator jq + setting_update quoting + footer reboot lock (B2 / B4 / R3-B-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Bash + PHP cleanups around configuration plumbing that could each silently produce broken state. B2 — m3u_generator.sh. - The Bash test `if [$setArtistCover == 1 ]` (no space after `[`) was always false: bash interpreted it as a command named `[$setArtistCover` which doesn't exist, exit 1. So the artist-cover-only branch never fired, and audiobook artists with a folder-level cover but no per-album cover always got the MuPiLogo fallback. Add the space. - Six `jq '. += [{"artist": "'"${artist}"'", …}]'` calls spliced artist/title into JSON via shell concatenation. Any title containing `"` (e.g. `Foo "Bar" Baz`) produced syntactically broken JSON; jq rejected it; pre-Phase-3 (HIGH-8) the > redirect truncated data.json to empty; post-Phase-3 the tempfile+rename pattern leaves data.json untouched but the entry doesn't get added. Replace all six with `--arg` so jq itself does the JSON-encoding. B4 — setting_update.sh. - `ls -l ${THEME_FILE} | grep ${NEW_THEME}` was unquoted: a theme name with a space split across grep arguments, a name with `-` started looking like a flag. Quote both, plus `-F --` so theme names with regex metachars match literally. - `xargs rm <<< ${THEME_FILE}` is needlessly complex for a fixed path; `rm -f` does the same with -f for idempotence. - sed-injection via the hostname / timeout fields: `sed 's/.*pat. */.../g'` uses `/` as the delimiter, but a hostname containing `/` would break out of the pattern. Switch to `|` as the delimiter (extremely unlikely in a hostname or numeric value), plus normalise hostname through `tr -dc 'A-Za-z0-9.-'` and timeout through `tr -dc '0-9'` so injection can't land via the config layer either. R3-B-5 — footer.php reboot/shutdown double-click. - `sudo su - -c "sleep 5; restart.sh &" &` had no locking. A double- click on the reboot button (or two admin tabs both submitting in quick succession) spawned two restart.sh processes whose cleanup steps fought, leaving the box in a half-rebooted state. Wrap in `( flock -n 9 || exit 0; … ) 9>/tmp/.mupibox.reboot.lock &` so a second invocation while the first is still running silently becomes a no-op. Same fix for the shutdown path with a separate lockfile. B5 (set -euo pipefail global) deferred — too many existing scripts rely on continue-on-error patterns for best-effort cleanup; rolling out set -e without per-script testing is a regression risk that outweighs the linter benefit. --- AdminInterface/www/includes/footer.php | 13 ++++- scripts/mupibox/m3u_generator.sh | 66 +++++++++++++++++++++++--- scripts/mupibox/setting_update.sh | 32 +++++++++++-- 3 files changed, 97 insertions(+), 14 deletions(-) diff --git a/AdminInterface/www/includes/footer.php b/AdminInterface/www/includes/footer.php index 99e845cf..810bf385 100644 --- a/AdminInterface/www/includes/footer.php +++ b/AdminInterface/www/includes/footer.php @@ -138,14 +138,23 @@ function updateBatteryIcon() { /tmp/.mupibox.reboot.lock &'; exec($command); } if( $shutdown == 1 ) { - $command='sudo su - -c "sleep 5; /usr/local/bin/mupibox/./shutdown.sh &" &'; + $command='( flock -n 9 || exit 0; sleep 5; sudo /usr/local/bin/mupibox/./shutdown.sh ) 9>/tmp/.mupibox.shutdown.lock &'; exec($command); } ?> diff --git a/scripts/mupibox/m3u_generator.sh b/scripts/mupibox/m3u_generator.sh index 7722e869..f3f8dcc3 100644 --- a/scripts/mupibox/m3u_generator.sh +++ b/scripts/mupibox/m3u_generator.sh @@ -50,7 +50,14 @@ else #/usr/bin/cp --update "${i}"/*.jp*g "/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover/audiobook/${artist}/${title}/cover.jpg" searchStrTitleCover=`/usr/bin/cat ${DATA} | grep 'audiobook/'"${artist}"'/'"${title}"'/cover.jpg'` else - if [$setArtistCover == 1 ] + # B2: previous test was `if [$setArtistCover == 1 ]` — + # missing space after `[`. Bash interprets that as a + # command named `[$setArtistCover`, which doesn't exist, + # so the test is always false and we always fell to the + # else-branch (MuPiLogo fallback). Audiobook artists + # with their own cover image but no per-title cover + # never got their cover used. + if [ $setArtistCover == 1 ] then /usr/bin/mkdir -p "/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover/audiobook/${artist}/${title}/" > /dev/null for i in "${topFolder}"/*.jp*g; do cp "$i" "/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover/audiobook/${artist}/${title}/cover.jpg"; break; done @@ -70,12 +77,32 @@ else # write while appending an entry truncated data.json # to empty — wiping every album's metadata. Tempfile # + rename preserves the original on failure. + # B2: previous code spliced ${artist} and ${title} into a + # JSON literal via shell concatenation — any artist + # title containing `"` (e.g. an audiobook called + # `Foo "Bar" Baz`) produced syntactically-broken JSON + # that jq rejected, taking down the m3u-generator pass + # and leaving data.json untouched (or, before HIGH-8, + # truncated to empty). Use --arg so jq itself does + # the JSON encoding — `"` in values is escaped to + # `\"`, no breakage. _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/audiobook/${artist}/${title}/cover.jpg" \ + --arg artistcover "http://${HN}:8200/cover/audiobook/${artist}/cover.jpg" \ + '. += [{type: "library", category: "audiobook", artist: $artist, title: $title, cover: $cover, artistcover: $artistcover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/jq '. += [{"type": "library", "category": "audiobook", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/audiobook/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/audiobook/${artist}/${title}/cover.jpg" \ + '. += [{type: "library", category: "audiobook", artist: $artist, title: $title, cover: $cover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi @@ -125,12 +152,25 @@ else if [ -z "${searchStrTitleCover}" ] then # Atomic-update (HIGH-8) — same as the audiobook block above. + # B2: same fix as the audiobook branch above — --arg + # instead of shell-spliced JSON. _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/music/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/music/${artist}/${title}/cover.jpg" \ + --arg artistcover "http://${HN}:8200/cover/music/${artist}/cover.jpg" \ + '. += [{type: "library", category: "music", artist: $artist, title: $title, cover: $cover, artistcover: $artistcover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/jq '. += [{"type": "library", "category": "music", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/music/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/music/${artist}/${title}/cover.jpg" \ + '. += [{type: "library", category: "music", artist: $artist, title: $title, cover: $cover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi @@ -180,12 +220,24 @@ else if [ -z "${searchStrTitleCover}" ] then # Atomic-update (HIGH-8) — same as the audiobook/music blocks above. + # B2: same fix as the audiobook/music branches above. _TMP="${DATA}.tmp.$$" if [ $setArtistCover == 1 ] then - /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg", "artistcover": "http://'${HN}':8200/cover/other/'"${artist}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/other/${artist}/${title}/cover.jpg" \ + --arg artistcover "http://${HN}:8200/cover/other/${artist}/cover.jpg" \ + '. += [{type: "library", category: "other", artist: $artist, title: $title, cover: $cover, artistcover: $artistcover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" else - /usr/bin/jq '. += [{"type": "library", "category": "other", "artist": "'"${artist}"'", "title": "'"${title}"'", "cover": "http://'${HN}':8200/cover/other/'"${artist}"'/'"${title}"'/cover.jpg"}]' "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" + /usr/bin/jq \ + --arg artist "${artist}" \ + --arg title "${title}" \ + --arg cover "http://${HN}:8200/cover/other/${artist}/${title}/cover.jpg" \ + '. += [{type: "library", category: "other", artist: $artist, title: $title, cover: $cover}]' \ + "${DATA}" > "${_TMP}" && mv "${_TMP}" "${DATA}" || rm -f "${_TMP}" fi fi fi diff --git a/scripts/mupibox/setting_update.sh b/scripts/mupibox/setting_update.sh index 6d80f1f1..59852a36 100644 --- a/scripts/mupibox/setting_update.sh +++ b/scripts/mupibox/setting_update.sh @@ -12,11 +12,17 @@ DISPLAY_STANDBY="/etc/X11/xorg.conf.d/98-dietpi-disable_dpms.conf" THEME_FILE="/home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/active_theme.css" NEW_THEME=$(/usr/bin/jq -r .mupibox.theme ${MUPIBOX_CONFIG}) -newTheme=$(ls -l ${THEME_FILE} | grep ${NEW_THEME}) +# B4: NEW_THEME comes from mupiboxconfig.json — admin-controlled. +# `ls -l ${THEME_FILE} | grep ${NEW_THEME}` was unquoted, so a theme +# name with a space (or a `/`) would either truncate or pull in +# unrelated grep flags. Quote both args, plus -F so theme names with +# regex metachars (`.`, `*`) match literally. +newTheme=$(ls -l "${THEME_FILE}" | grep -F -- "${NEW_THEME}") if (( ${#newTheme} == 0 )) then - xargs rm <<< ${THEME_FILE} - ln -s /home/dietpi/MuPiBox/themes/${NEW_THEME}.css ${THEME_FILE} + # Same quoting fix on the symlink replace. + rm -f "${THEME_FILE}" + ln -s "/home/dietpi/MuPiBox/themes/${NEW_THEME}.css" "${THEME_FILE}" fi # Atomic-update (HIGH-8). Read all source values first, then bundle the @@ -95,11 +101,27 @@ _TMP="${SPOTIFYCONTROLLER_CONFIG}.tmp.$$" #/usr/bin/sed -i 's/.*username.*/ username = '\"${username}\"'/g' ${SPOTIFYD_CONFIG} #password=$(/usr/bin/jq -r .spotify.password ${MUPIBOX_CONFIG}) #/usr/bin/sed -i 's/.*password.*/ password = '\"${password}\"'/g' ${SPOTIFYD_CONFIG} +# B4: hostname/timeout get spliced into a sed `s/`-delimited pattern. +# `/` is the default sed delimiter; if either value contained `/` +# (legitimate for hostnames with FQDN dots — also legal in some user- +# typed values), sed would interpret it as a delimiter and produce a +# garbled config. Switch to `|` as the sed delimiter (extremely +# unlikely to appear in a hostname or numeric timeout) and validate +# the timeout numerically before splicing — sed-injection via a +# numeric field would only happen if the config layer was already +# compromised, but defence in depth. hostname=$(/usr/bin/jq -r .mupibox.host ${MUPIBOX_CONFIG}) -/usr/bin/sed -i 's/.*device_name.*/ device_name = '\"${hostname}\"'/g' ${SPOTIFYD_CONFIG} +# Strip anything that isn't a hostname char to be belt-and-braces; +# RFC1123 hostnames are letters, digits, `.`, `-` only. +hostname_safe=$(printf '%s' "${hostname}" | tr -dc 'A-Za-z0-9.-') +/usr/bin/sed -i 's|.*device_name.*| device_name = "'"${hostname_safe}"'"|g' ${SPOTIFYD_CONFIG} timeout=$(/usr/bin/jq -r .timeout.idleDisplayOff ${MUPIBOX_CONFIG}) -/usr/bin/sed -i 's/.*Option \"BlankTime\".*/ Option \"BlankTime\" '\"${timeout}\"'/g' ${DISPLAY_STANDBY} +# Force-numeric — empty or non-int collapses to 0 (sane "no timeout" +# default for the X server's BlankTime). +timeout_int=$(printf '%s' "${timeout}" | tr -dc '0-9') +: "${timeout_int:=0}" +/usr/bin/sed -i 's|.*Option "BlankTime".*| Option "BlankTime" "'"${timeout_int}"'"|g' ${DISPLAY_STANDBY} #currentIP=$(hostname -I) #/usr/bin/cat <<< $(/usr/bin/jq --arg v "${currentIP}" '.ip = $v' ${SONOS_NETWORK}) > ${SONOS_NETWORK} From d3b89fa886b40834236b97e83d5dafcb9b5fc5aa Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 23:52:28 +0200 Subject: [PATCH 11/14] =?UTF-8?q?AR5-1:=20fix=20check=5Fnetwork.sh=20?= =?UTF-8?q?=E2=80=94=20subshell=20+=20python=20output=20mismatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two compounded bugs in scripts/mupibox/check_network.sh:289 (one of them was masking the other, so they get fixed together): 1) Subshell pretending to be a string-test: `if ( $(/usr/bin/python3 check_network.py) == ${TRUESTATE} )` is a bash subshell — bash captures the python script's stdout, then runs it as a command with `==` and `${TRUESTATE}` as arguments. Whatever the python emits is interpreted as a command name; the subshell returns 127, the if-condition is permanently false. The else-branch (offline mode — symlinks data.json to offline_data.json with spotify/radio/rss filtered out) ran every tick regardless of network state. 2) Value mismatch hidden by the subshell: check_network.py prints the literal "true"/"false", but the bash variables ${TRUESTATE}/${FALSESTATE} are "online"/"offline". Even a syntactically-correct string-test would always have been false. The subshell bug masked this — first apparent only after the subshell was fixed and the symlink kept pointing at offline_data.json even with internet reachable. Fix uses the standard string-comparison idiom against the actual python output literal: if [ "$(/usr/bin/python3 …)" = "true" ]; then ONLINESTATE keeps its "online"/"offline" form so downstream consumers of /tmp/network.json see no behaviour change. Squashed from: ecc81a9c 2b4fa289 --- scripts/mupibox/check_network.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/mupibox/check_network.sh b/scripts/mupibox/check_network.sh index 47674868..bc7711bf 100644 --- a/scripts/mupibox/check_network.sh +++ b/scripts/mupibox/check_network.sh @@ -86,7 +86,17 @@ ensure_symlink() { while true do - if ( $(/usr/bin/python3 /usr/local/bin/mupibox/check_network.py) == ${TRUESTATE} ); then + # AR5-1: was `if ( $(python3 ...) == ${TRUESTATE} )` — that's a bash + # subshell executing the python output as a command (with `==` and the + # literal string as args), not a string comparison. Subshell exit 127 + # made the if-condition permanently false. + # AR5-1.1: ALSO — check_network.py prints the string "true"/"false" + # (not "online"/"offline"), so even a correct string-test against + # ${TRUESTATE}="online" would always be false. The original bash-bug + # was masking this mismatch. Compare against the actual python output. + # ONLINESTATE keeps its "online"/"offline" values for downstream + # consumers of /tmp/network.json. + if [ "$(/usr/bin/python3 /usr/local/bin/mupibox/check_network.py)" = "true" ]; then ONLINESTATE=${TRUESTATE} # Reconcile every tick (cheap when no-op) instead of only on # state change. Self-healing if the symlink was wrong. From 1952a5ea80232038cd5fff43f905f395b234db16 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:34:16 +0200 Subject: [PATCH 12/14] fix(services): add Restart=on-failure to four crash-relevant services (AR5-8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without a Restart= directive, a crash leaves the service permanently dead until the next reboot. For four of these the silent-failure consequences are non-trivial: - mupi_change_checker.service: the watcher that drives Quiet-Hours and Playtime-Cap evaluation. If it crashes mid-day (bash edge case, file I/O error on /etc/mupibox/, etc.), the kid-protection guards stop firing silently — the box would play through quiet windows and ignore the daily limit until someone notices and reboots. Highest-impact service of the four. - mupi_telegram.service: the bot receiver. We saw the AR5-2/7 IndexError crash live in Phase 7 — without restart, parents' /release and /quietnow commands would just stop working without explanation. - mupi_idle_shutdown.service: the auto-shutdown timer. Silently broken means the box stays on forever, draining the battery overnight. - mupi_check_internet.service: the online/offline state driver. Silent failure leaves the box stuck in whatever state it was when the crash happened (probably "online" — masks real outages). All four get Restart=on-failure + RestartSec=10. mupi_startstop.service is intentionally excluded — it's Type=oneshot RemainAfterExit=true, so Restart= would conflict with the oneshot semantics. mupi_hat.service already has Restart=on-failure from upstream. On deploy: copy each unit to /etc/systemd/system/, run `sudo systemctl daemon-reload`, then restart-or-leave each. No need to manually restart any of them right now (they're running already). --- config/services/mupi_change_checker.service | 2 ++ config/services/mupi_check_internet.service | 2 ++ config/services/mupi_idle_shutdown.service | 2 ++ config/services/mupi_telegram.service | 2 ++ 4 files changed, 8 insertions(+) diff --git a/config/services/mupi_change_checker.service b/config/services/mupi_change_checker.service index 186eb39e..2a39c6cd 100644 --- a/config/services/mupi_change_checker.service +++ b/config/services/mupi_change_checker.service @@ -3,6 +3,8 @@ Description=Check for changes in media directory [Service] ExecStart=/usr/local/bin/mupibox/./change_checker.sh +Restart=on-failure +RestartSec=10 [Install] WantedBy=basic.target \ No newline at end of file diff --git a/config/services/mupi_check_internet.service b/config/services/mupi_check_internet.service index c4e80aa2..13cf357d 100644 --- a/config/services/mupi_check_internet.service +++ b/config/services/mupi_check_internet.service @@ -5,6 +5,8 @@ Description=Checks Internet Connection User=dietpi Group=dietpi ExecStart=/usr/local/bin/mupibox/./check_network.sh +Restart=on-failure +RestartSec=10 [Install] WantedBy=multi-user.target \ No newline at end of file diff --git a/config/services/mupi_idle_shutdown.service b/config/services/mupi_idle_shutdown.service index 806796f2..281cc2b0 100644 --- a/config/services/mupi_idle_shutdown.service +++ b/config/services/mupi_idle_shutdown.service @@ -3,6 +3,8 @@ Description=Idle Shutdown Timer [Service] ExecStart=/usr/local/bin/mupibox/./idle_shutdown.sh +Restart=on-failure +RestartSec=10 [Install] WantedBy=basic.target \ No newline at end of file diff --git a/config/services/mupi_telegram.service b/config/services/mupi_telegram.service index 98533d49..aca53e12 100644 --- a/config/services/mupi_telegram.service +++ b/config/services/mupi_telegram.service @@ -5,6 +5,8 @@ After=network-online.target [Service] ExecStart=/usr/local/bin/mupibox/./telegram_start.sh +Restart=on-failure +RestartSec=10 [Install] WantedBy=default.target \ No newline at end of file From 9046a3400ebdeb9fc4701cfc9893ba40e7b88856 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 13:22:32 +0200 Subject: [PATCH 13/14] H5: pre-flight checks + atomic-swap rollback in update script start_mupibox_update.sh ran the destructive `rm -R Sonos-Kids- Controller-master/` before verifying the downloaded archive was usable. A flaky internet, a 404, a corrupted zip, or a kill mid- unzip would leave the box with no installation and no rollback. Pre-flight checks before any destructive op: - mupibox.zip non-empty after wget - unzip -t integrity check on mupibox.zip - MUPI_SRC dir exists after outer unzip - inner deploy.zip non-empty - /tmp/data.json backup actually landed (resume/library state is non-recoverable; cover/active_theme.css can be regenerated) Atomic swap around the destructive step: - mv old install to Sonos-Kids-Controller-master.upd-bak instead of rm -R - if the inner unzip fails, restore from .upd-bak and pm2 start the old server, then exit 1 with a clear message - only on successful extract is the .upd-bak dir removed fail_update() helper surfaces the abort to the dialog/whiptail UI so the user actually sees what went wrong instead of staring at a seemingly-stuck install. --- update/start_mupibox_update.sh | 59 ++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/update/start_mupibox_update.sh b/update/start_mupibox_update.sh index 7e024bea..18343436 100644 --- a/update/start_mupibox_update.sh +++ b/update/start_mupibox_update.sh @@ -25,6 +25,22 @@ killall -s 9 -w -q -r chromium CONFIG="/etc/mupibox/mupiboxconfig.json" LOG="/boot/mupibox_update.log" exec 3>${LOG} + +# H5: previously the script ran the destructive `rm -R Sonos-Kids- +# Controller-master/` BEFORE verifying that the downloaded archive was +# usable. A flaky internet-dropout, a 404 response, or a corrupted +# unzip would leave the box with no installation and no rollback path. +# Add fail_update() to bail BEFORE any destructive op when a pre-flight +# check fails, plus an atomic-swap pattern around the rm so a failure +# at extract time can restore the previous install. +fail_update() { + local msg=$1 + echo "## UPDATE ABORTED: ${msg}" >&3 2>&3 + echo "## (no destructive operation performed yet — your installation is intact)" >&3 2>&3 + # Surface to dialog/whiptail so the user actually sees the failure + echo -e "XXX\n100\nUpdate aborted: ${msg}\nXXX" + exit 1 +} service mupi_idle_shutdown stop packages2install="lighttpd-mod-openssl gpiod git libasound2 mplayer pulseaudio-module-bluetooth pip id3tool bluez zip rrdtool scrot net-tools wireless-tools autoconf automake bc build-essential python3-gpiozero python3-rpi.gpio python3-lgpio python3-serial python3-requests python3-paho-mqtt libgles2-mesa mesa-utils libsdl2-dev preload python3-smbus2 pigpio libjson-c-dev i2c-tools libi2c-dev python3-smbus python3-alsaaudio python3-netifaces libwidevinecdm0 python3-flask" packages2remove="jq" @@ -232,19 +248,27 @@ echo "========================================================================== ############################################################################################### - echo -e "XXX\n${STEP}\nDownload MuPiBox Version ${VERSION_LONG}... \nXXX" + echo -e "XXX\n${STEP}\nDownload MuPiBox Version ${VERSION_LONG}... \nXXX" before=$(date +%s) wget -q -O /home/dietpi/mupibox.zip ${MUPIBOX_URL} >&3 2>&3 + # H5: hard-fail before any destructive step if the download didn't land. + [ -s /home/dietpi/mupibox.zip ] || fail_update "MuPiBox download failed (file empty or missing — check internet)" + unzip -t /home/dietpi/mupibox.zip >/dev/null 2>&1 || fail_update "MuPiBox download corrupted (zip integrity check failed)" after=$(date +%s) echo -e "## MuPiBox Download ## finished after $((after - $before)) seconds" >&3 2>&3 STEP=$(($STEP + 1)) ############################################################################################### - echo -e "XXX\n${STEP}\nUnzip MuPiBox Version ${VERSION_LONG}... \nXXX" + echo -e "XXX\n${STEP}\nUnzip MuPiBox Version ${VERSION_LONG}... \nXXX" before=$(date +%s) unzip -q -d /home/dietpi /home/dietpi/mupibox.zip >&3 2>&3 rm /home/dietpi/mupibox.zip >&3 2>&3 + # H5: verify the source dir + the inner deploy.zip exist before we + # wipe the live install. If either is missing the user gets a + # clean abort instead of a half-installed box. + [ -d "${MUPI_SRC}" ] || fail_update "Expected source directory ${MUPI_SRC} not found after unzip" + [ -s "${MUPI_SRC}/bin/nodejs/deploy.zip" ] || fail_update "Backend deploy.zip missing or empty in update package" #MUPI_SRC="/home/dietpi/MuPiBox-${VERSION}" after=$(date +%s) @@ -259,21 +283,44 @@ echo "========================================================================== mv /home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/cover /tmp/cover >&3 2>&3 #mv /home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/config.json /tmp/config.json >&3 2>&3 mv /home/dietpi/.mupibox/Sonos-Kids-Controller-master/www/active_theme.css /tmp/active_theme.css >&3 2>&3 + # H5: data.json holds resume/library state — non-recoverable. cover + # and active_theme.css can be restored from defaults if missing, + # so only the data.json backup is fail-fast. + [ -f /tmp/data.json ] || fail_update "data.json backup failed (resume/library state would be lost)" after=$(date +%s) echo -e "## Backup Data ## finished after $((after - $before)) seconds" >&3 2>&3 - + STEP=$(($STEP + 1)) ############################################################################################### - echo -e "XXX\n${STEP}\nUpdate frontend, backend-api, and backend-player ... \nXXX" + echo -e "XXX\n${STEP}\nUpdate frontend, backend-api, and backend-player ... \nXXX" before=$(date +%s) sudo -H -u dietpi bash -c "pm2 stop server" >&3 2>&3 #su - dietpi -c "pm2 save" >&3 2>&3 - rm -R /home/dietpi/.mupibox/Sonos-Kids-Controller-master/ >&3 2>&3 + # H5: atomic-swap with rollback. Move old install aside instead of + # deleting it; if the deploy.zip extract fails, restore the backup + # so the box keeps running on the previous version. The .upd-bak + # directory is removed after a successful extract. + BAK_DIR="/home/dietpi/.mupibox/Sonos-Kids-Controller-master.upd-bak" + rm -rf "${BAK_DIR}" >&3 2>&3 + if [ -d /home/dietpi/.mupibox/Sonos-Kids-Controller-master ]; then + mv /home/dietpi/.mupibox/Sonos-Kids-Controller-master "${BAK_DIR}" >&3 2>&3 || \ + fail_update "Could not move old install aside (filesystem full?)" + fi mkdir -p /home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/ >&3 2>&3 - unzip ${MUPI_SRC}/bin/nodejs/deploy.zip -d /home/dietpi/.mupibox/Sonos-Kids-Controller-master/ >&3 2>&3 + if ! unzip ${MUPI_SRC}/bin/nodejs/deploy.zip -d /home/dietpi/.mupibox/Sonos-Kids-Controller-master/ >&3 2>&3; then + # Rollback: remove the partially-extracted dir and restore the backup. + rm -rf /home/dietpi/.mupibox/Sonos-Kids-Controller-master >&3 2>&3 + if [ -d "${BAK_DIR}" ]; then + mv "${BAK_DIR}" /home/dietpi/.mupibox/Sonos-Kids-Controller-master >&3 2>&3 + sudo -H -u dietpi bash -c "pm2 start server" >&3 2>&3 + fi + fail_update "deploy.zip extraction failed — rolled back to previous install" + fi + # Extract succeeded — drop the backup. + rm -rf "${BAK_DIR}" >&3 2>&3 mv ${MUPI_SRC}/config/templates/monitor.json /home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/monitor.json >&3 2>&3 mv ${MUPI_SRC}/config/templates/www.json /home/dietpi/.mupibox/Sonos-Kids-Controller-master/server/config/config.json >&3 2>&3 chown dietpi:dietpi -R /home/dietpi/.mupibox/Sonos-Kids-Controller-master/www >&3 2>&3 From 20ba821a139a9c7bbc9184f36071568187abf4de Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 08:36:17 +0200 Subject: [PATCH 14/14] update: preserve admin-customised www files + fix dietpiMuPiBox path typo (MED-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the MuPiBox auto-update script that bit users running it. (1) `rm -R /var/www/*` then `unzip` of the bundled www.zip wiped any files the admin had customised: - active_theme.css → the theme picker's selected theme - cover → symlink to /home/dietpi/MuPiBox/media/cover - theme-data/ → background images for custom themes After an update, the box reverted to the default theme and lost the cover symlink (so cover art for local audiobooks vanished from the admin UI's Media view). User then had to re-pick the theme and re-create the cover symlink manually. Mirror scripts/dev/deploy_box.sh's preserve-then-restore pattern: copy active_theme.css / cover / theme-data into a /tmp/mktemp dir before the rm, copy them back after the unzip. Belt-and-braces for the cover symlink — recreate it from scratch only if the preserve step didn't already drop a symlink in place. (2) Path typo: the script copied media files to `/home/dietpiMuPiBox/sysmedia/...` (missing slash between dietpi and MuPiBox) — a path that doesn't exist on a real box. cp emits an error to fd 3 (the script's logfile) and silently moves on, so the destination files never got refreshed during updates. Fixed all four typo'd paths to /home/dietpi/MuPiBox/sysmedia/. Also fixed `{STEP}` → `${STEP}` in the progress message that displayed the literal text "{STEP}" to the user. --- update/start_mupibox_update.sh | 44 ++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/update/start_mupibox_update.sh b/update/start_mupibox_update.sh index 18343436..656e5c0f 100644 --- a/update/start_mupibox_update.sh +++ b/update/start_mupibox_update.sh @@ -486,19 +486,23 @@ echo "========================================================================== ############################################################################################### - echo -e "XXX\n${STEP}\nCopy some media files... \nXXX" + echo -e "XXX\n${STEP}\nCopy some media files... \nXXX" # Splash and Media before=$(date +%s) #mv ${MUPI_SRC}/config/templates/splash.txt /boot/splash.txt >&3 2>&3 wget https://gitlab.com/DarkElvenAngel/initramfs-splash/-/raw/master/boot/initramfs.img -O /boot/initramfs.img >&3 2>&3 - cp ${MUPI_SRC}/media/images/goodbye.png /home/dietpiMuPiBox/sysmedia/images/goodbye.png >&3 2>&3 + # MED-6: previous code had `/home/dietpiMuPiBox/sysmedia/...` (missing + # slash between dietpi and MuPiBox). The cp's silently failed because + # that path doesn't exist on a real box; the destination files never + # got refreshed. Fixed to /home/dietpi/MuPiBox/. + cp ${MUPI_SRC}/media/images/goodbye.png /home/dietpi/MuPiBox/sysmedia/images/goodbye.png >&3 2>&3 #mv ${MUPI_SRC}/media/images/splash.png /boot/splash.png >&3 2>&3 - #cp ${MUPI_SRC}/media/images/MuPiLogo.jpg /home/dietpiMuPiBox/sysmedia/images/MuPiLogo.jpg >&3 2>&3 - #cp ${MUPI_SRC}/media/sound/shutdown.wav /home/dietpiMuPiBox/sysmedia/sound/shutdown.wav >&3 2>&3 - #cp ${MUPI_SRC}/media/sound/startup.wav /home/dietpiMuPiBox/sysmedia/sound/startup.wav >&3 2>&3 + #cp ${MUPI_SRC}/media/images/MuPiLogo.jpg /home/dietpi/MuPiBox/sysmedia/images/MuPiLogo.jpg >&3 2>&3 + #cp ${MUPI_SRC}/media/sound/shutdown.wav /home/dietpi/MuPiBox/sysmedia/sound/shutdown.wav >&3 2>&3 + #cp ${MUPI_SRC}/media/sound/startup.wav /home/dietpi/MuPiBox/sysmedia/sound/startup.wav >&3 2>&3 cp ${MUPI_SRC}/media/sound/button_shutdown.wav /home/dietpi/MuPiBox/sysmedia/sound/button_shutdown.wav >&3 2>&3 - cp ${MUPI_SRC}/media/sound/low.wav /home/dietpiMuPiBox/sysmedia/sound/low.wav >&3 2>&3 - cp ${MUPI_SRC}/media/images/installation.jpg /home/dietpiMuPiBox/sysmedia/images/installation.jpg >&3 2>&3 + cp ${MUPI_SRC}/media/sound/low.wav /home/dietpi/MuPiBox/sysmedia/sound/low.wav >&3 2>&3 + cp ${MUPI_SRC}/media/images/installation.jpg /home/dietpi/MuPiBox/sysmedia/images/installation.jpg >&3 2>&3 cp ${MUPI_SRC}/media/images/battery_low.jpg /home/dietpi/MuPiBox/sysmedia/images/battery_low.jpg >&3 2>&3 after=$(date +%s) @@ -622,13 +626,33 @@ echo "========================================================================== ############################################################################################### - echo -e "XXX\n{STEP}\nUpdate Admin-Interface... \nXXX" + # MED-6: typo `{STEP}` (no $) just printed literal "{STEP}" in the + # update progress UI. Plus: `rm -R /var/www/*` then unzip wiped any + # admin-customised files (active_theme.css from the theme picker, + # the cover/ symlink, theme-data/ for custom-theme backgrounds). + # The next admin save would re-create active_theme.css, but custom + # themes and cover/-symlink-content were lost. Preserve them around + # the wipe. + echo -e "XXX\n${STEP}\nUpdate Admin-Interface... \nXXX" before=$(date +%s) - rm -R /var/www/* >&3 2>&3 + # Stage user-customised files in /tmp so they survive the wipe. + UPDATE_PRESERVE=$(mktemp -d /tmp/mupibox-www-preserve.XXXXXX) + for item in active_theme.css cover theme-data; do + [ -e "/var/www/${item}" ] && cp -a "/var/www/${item}" "${UPDATE_PRESERVE}/" >&3 2>&3 + done + rm -R /var/www/* >&3 2>&3 mv ${MUPI_SRC}/AdminInterface/release/www.zip /var/www/www.zip >&3 2>&3 unzip /var/www/www.zip -d /var/www/ >&3 2>&3 rm /var/www/www.zip >&3 2>&3 - ln -s /home/dietpi/MuPiBox/media/cover /var/www/cover >&3 2>&3 + # Restore preserved files after the new www/ unzipped so they + # overwrite anything the package would otherwise have shipped. + for item in active_theme.css cover theme-data; do + [ -e "${UPDATE_PRESERVE}/${item}" ] && cp -a "${UPDATE_PRESERVE}/${item}" /var/www/ >&3 2>&3 + done + rm -rf "${UPDATE_PRESERVE}" + # `cover` is supposed to be a symlink to /home/dietpi/MuPiBox/media/cover — + # only re-create if the preserve step didn't already restore it. + [ -L /var/www/cover ] || ln -s /home/dietpi/MuPiBox/media/cover /var/www/cover >&3 2>&3 chown -R www-data:www-data /var/www/ >&3 2>&3 chmod -R 755 /var/www/ >&3 2>&3 chown -R dietpi:www-data /home/dietpi/MuPiBox/media/cover >&3 2>&3