From 702f933bd6a9e29f9d95e04a2575e37fb723302a Mon Sep 17 00:00:00 2001 From: Stephen Shaw Date: Tue, 28 Apr 2026 14:49:22 -0600 Subject: [PATCH 1/2] entrypoint: patch RoonServer/start.sh tar flags for upgrade compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In-container Roon Server upgrades are silently failing for users with restricted-chown host setups (userns-remap, NFS root_squash, TrueNAS / Synology bind mounts with strict ACLs). Initial install works because entrypoint.sh extracts with `tar xjf ... --no-same-permissions --no-same-owner`, but RoonServer's bundled start.sh handles in-container upgrades with bare `tar xf "$PKG"`. The build tarballs ship with files owned by uid 1001 (the Azure Pipelines build agent), so when start.sh runs as root and tries to chown extracted files to 1001 on a host that rejects foreign-uid chown, tar exits non-zero and start.sh's `|| doerror` reports "ERROR: failed to tar xf $PKG". The user is left on the old version with no obvious diagnostic. The proper fix is upstream — start.sh should use the same flags entrypoint.sh uses. While that's pending, patch the on-disk start.sh from our entrypoint to inject the missing flags. Idempotent: skipped when the flag is already present, so it survives upstream's eventual fix without conflict, and re-runs after every successful upgrade (which restores start.sh from the new build tarball). Tests: pre-seed an existing install with an upstream-shape start.sh, run the entrypoint, assert the patched line is present and that the flag count is exactly 1 (not double-applied). Second test pre-seeds with an already-patched start.sh and asserts the log message is absent (idempotency on second pass). Smoke suite: 56 → 60 checks. --- entrypoint.sh | 27 ++++++++++++++++++++ tests/smoke.sh | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/entrypoint.sh b/entrypoint.sh index fae4eff..07db64c 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -119,6 +119,33 @@ if [ "$NEEDS_INSTALL" = true ]; then echo "RoonServer installed successfully." fi +# --- Patch start.sh for in-container update durability --------------------- +# RoonServer's start.sh handles in-container upgrades by extracting a new +# build tarball into RoonServer/.update.tmp/. The bundled script uses bare +# `tar xf "$PKG"` (no --no-same-owner / --no-same-permissions), and the +# build tarballs store files as uid 1001 (the Azure Pipelines build agent). +# That combination fails on hosts with restricted chown semantics — userns- +# remapped Docker daemons, NFS mounts with root_squash, TrueNAS bind +# mounts with strict ACLs — because tar tries to chown extracted files +# to a uid the host filesystem rejects, exits non-zero, and start.sh's +# `|| doerror` catches the failure as "ERROR: failed to tar xf $PKG". +# Result: initial install succeeds (entrypoint extract uses the right +# flags); auto-updates silently fail. +# +# Fix the running on-disk start.sh to mirror our entrypoint extract flags. +# Idempotent: skip when the flag is already present (avoids double-patches +# on restart, and makes upstream's eventual fix a no-op here). Re-runs on +# every container start so each upgrade — which replaces start.sh with +# the unpatched upstream version — gets re-patched on the next restart. + +START_SH="${ROON_APP_DIR}/RoonServer/start.sh" +if [ -f "$START_SH" ] && ! grep -q -- "--no-same-owner" "$START_SH"; then + if sed -i 's|tar xf "$PKG"|tar xf --no-same-owner --no-same-permissions "$PKG"|' "$START_SH" \ + && grep -q -- "--no-same-owner" "$START_SH"; then + echo "Patched RoonServer/start.sh for in-container update compatibility." + fi +fi + # --- Final state log -------------------------------------------------------- # Line format is contract-ish: runtime tests grep for "^Branch: production" # and "^Branch: earlyaccess". Don't change the prefix or spacing without diff --git a/tests/smoke.sh b/tests/smoke.sh index 204c674..5ca3373 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -254,6 +254,74 @@ check "multi-line VERSION: last line wins (detects production)" \ check "image has no USER directive (runs as root)" \ sh -c '[ -z "$(docker inspect --format "{{.Config.User}}" "$1")" ]' _ "$IMAGE" +# In-container update compatibility: entrypoint must patch start.sh's bare +# `tar xf` to use --no-same-owner --no-same-permissions, otherwise upgrades +# fail on userns-remap / NFS-root_squash / strict-ACL hosts because tar +# tries to chown extracted files to the build-agent uid (1001) and exits +# non-zero on chown failure. +# +# Pre-seed a fake RoonServer install with start.sh containing the upstream +# bare-tar pattern, plus a Server/RoonServer launcher and VERSION file so +# the entrypoint takes the "no reinstall needed" path. Replace the +# entrypoint's exec target with `:` (the no-op shell builtin) by also +# providing a fake start.sh that just exits — but we want to inspect +# start.sh after the patch step, so we use --entrypoint sh to bypass +# entrypoint.sh entirely on the second run and just read the file. +PATCH_TMP=$(mktemp -d) +docker run --rm --entrypoint sh -v "$PATCH_TMP:/Roon" "$IMAGE" -c ' +mkdir -p /Roon/app/RoonServer/Server +cat > /Roon/app/RoonServer/start.sh <<'\''EOF'\'' +#!/bin/bash +if [ "$1" = "--update" ]; then + PKG="$2"; ROOTDIR="$3" + tar xf "$PKG" -C "$ROOTDIR/.update.tmp" +fi +exit 0 +EOF +chmod +x /Roon/app/RoonServer/start.sh +touch /Roon/app/RoonServer/Server/RoonServer +printf "100\nproduction\n" > /Roon/app/RoonServer/VERSION +' >/dev/null + +# Run the entrypoint — should detect existing install (no reinstall), +# patch start.sh, then exec it (which exits 0 thanks to our fake script). +PATCH_LOG=$(docker run --rm -e ROON_INSTALL_BRANCH=production -v "$PATCH_TMP:/Roon" "$IMAGE" 2>&1) || true + +# Read the (possibly-patched) start.sh from inside the container so the +# test doesn't depend on host file-sharing config (Docker Desktop on +# macOS doesn't share /var/folders by default). +PATCHED_SH=$(docker run --rm --entrypoint cat -v "$PATCH_TMP:/Roon" "$IMAGE" /Roon/app/RoonServer/start.sh 2>&1) +rm -rf "$PATCH_TMP" 2>/dev/null || true + +check "start.sh patch: tar invocation gets --no-same-owner + --no-same-permissions" \ + sh -c 'echo "$1" | grep -qF "tar xf --no-same-owner --no-same-permissions \"\$PKG\""' _ "$PATCHED_SH" +check "start.sh patch: idempotent (only one set of flags after patching)" \ + sh -c '[ "$(echo "$1" | grep -c -- --no-same-owner)" -eq 1 ]' _ "$PATCHED_SH" +check "start.sh patch: log message is emitted on patching" \ + sh -c 'echo "$1" | grep -q "Patched RoonServer/start.sh"' _ "$PATCH_LOG" + +# Idempotency check on a second run: PATCH_TMP was deleted, so re-seed +# with an already-patched start.sh and verify the entrypoint is a no-op. +PATCH_TMP2=$(mktemp -d) +docker run --rm --entrypoint sh -v "$PATCH_TMP2:/Roon" "$IMAGE" -c ' +mkdir -p /Roon/app/RoonServer/Server +cat > /Roon/app/RoonServer/start.sh <<'\''EOF'\'' +#!/bin/bash +if [ "$1" = "--update" ]; then + tar xf --no-same-owner --no-same-permissions "$PKG" -C "$ROOTDIR/.update.tmp" +fi +exit 0 +EOF +chmod +x /Roon/app/RoonServer/start.sh +touch /Roon/app/RoonServer/Server/RoonServer +printf "100\nproduction\n" > /Roon/app/RoonServer/VERSION +' >/dev/null +PATCH2_LOG=$(docker run --rm -e ROON_INSTALL_BRANCH=production -v "$PATCH_TMP2:/Roon" "$IMAGE" 2>&1) || true +rm -rf "$PATCH_TMP2" 2>/dev/null || true + +check "start.sh patch: skipped (no log) when --no-same-owner already present" \ + sh -c '! echo "$1" | grep -q "Patched RoonServer/start.sh"' _ "$PATCH2_LOG" + # Startup banner is always emitted (regardless of branch resolution outcome) check "startup banner always logged" \ sh -c 'echo "$1" | grep -q "^Roon Docker image "' _ "$UNSET_OUTPUT" From 2d0afd6b3672802377dbda5d8904ce59c1517771 Mon Sep 17 00:00:00 2001 From: Stephen Shaw Date: Tue, 28 Apr 2026 14:57:59 -0600 Subject: [PATCH 2/2] review: tighten patch guard + assert idempotent run exit code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from PR review on #20: - entrypoint.sh: patch guard now requires BOTH --no-same-owner AND --no-same-permissions before considering start.sh already patched. A partial upstream fix (one flag added, the other missing) would otherwise leave the file half-fixed with our patch silently skipped. Post-sed verification checks both flags before logging success. - smoke.sh: assert exit==0 on the second-run idempotency check. The previous version let \`|| true\` mask container startup failures, which would have made the negative "no log" assertion pass for the wrong reason. Now: capture exit code, assert it's 0, then assert the log is absent — so the negative assertion only triggers when the container actually ran cleanly. --- entrypoint.sh | 9 +++++++-- tests/smoke.sh | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 07db64c..bae696d 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -139,9 +139,14 @@ fi # the unpatched upstream version — gets re-patched on the next restart. START_SH="${ROON_APP_DIR}/RoonServer/start.sh" -if [ -f "$START_SH" ] && ! grep -q -- "--no-same-owner" "$START_SH"; then +# Guard checks for BOTH flags so a partial upstream fix (e.g., owner added +# but not permissions) still triggers the full patch attempt. Post-sed we +# also verify both flags are present before logging success. +if [ -f "$START_SH" ] && ! { grep -q -- "--no-same-owner" "$START_SH" \ + && grep -q -- "--no-same-permissions" "$START_SH"; }; then if sed -i 's|tar xf "$PKG"|tar xf --no-same-owner --no-same-permissions "$PKG"|' "$START_SH" \ - && grep -q -- "--no-same-owner" "$START_SH"; then + && grep -q -- "--no-same-owner" "$START_SH" \ + && grep -q -- "--no-same-permissions" "$START_SH"; then echo "Patched RoonServer/start.sh for in-container update compatibility." fi fi diff --git a/tests/smoke.sh b/tests/smoke.sh index 5ca3373..192009d 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -316,10 +316,15 @@ chmod +x /Roon/app/RoonServer/start.sh touch /Roon/app/RoonServer/Server/RoonServer printf "100\nproduction\n" > /Roon/app/RoonServer/VERSION ' >/dev/null -PATCH2_LOG=$(docker run --rm -e ROON_INSTALL_BRANCH=production -v "$PATCH_TMP2:/Roon" "$IMAGE" 2>&1) || true +PATCH2_EXIT=0 +PATCH2_LOG=$(docker run --rm -e ROON_INSTALL_BRANCH=production -v "$PATCH_TMP2:/Roon" "$IMAGE" 2>&1) || PATCH2_EXIT=$? rm -rf "$PATCH_TMP2" 2>/dev/null || true -check "start.sh patch: skipped (no log) when --no-same-owner already present" \ +# Guard the negative assertion below: if the container failed to start +# at all, the "no log" assertion would pass for the wrong reason. +check "start.sh patch: idempotent run exits cleanly" \ + test "$PATCH2_EXIT" -eq 0 +check "start.sh patch: skipped (no log) when both flags already present" \ sh -c '! echo "$1" | grep -q "Patched RoonServer/start.sh"' _ "$PATCH2_LOG" # Startup banner is always emitted (regardless of branch resolution outcome)