diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 2148dab31..1d51f9d8d 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -458,6 +458,32 @@ TLS floor in the dynamic config. (`TestRootInstallServiceGrantsIcmpProbeCapability`) pins the pairing, and `docs/CONFIGURATION.md` documents the `systemctl edit` override for units written before the grant existed. + Unattended update execution must fail closed on service availability + (#1630): `scripts/pulse-auto-update.sh` `perform_update` must leave the + service running on every exit path when it was active before the attempt, + including the installer-exits-nonzero rollback branch. The generated + `pulse-update.service` gates on `ExecCondition=systemctl is-active`, so a + service left stopped also silently disables every future unattended run. + This is enforced by a `service_was_active`-guarded restart in each rollback + branch plus the `ensure_service_restarted` RETURN-trap backstop, and pinned + by `scripts/installtests/pulse_auto_update_test.go` + (`TestPerformUpdateRestartsServiceWhenInstallerFails`, + `TestEnsureServiceRestartedHonorsPriorServiceState`) and + `scripts/tests/test-pulse-auto-update.sh`. For the same reason, root + `install.sh` writes outside the hardened update unit's writable set + (`ProtectSystem=strict` leaves only the install dir, config dir and `/tmp` + writable) must be idempotent and non-fatal warnings rather than errexit + aborts: the `/bin/update` helper heredoc, the PATH appends to + `/etc/profile` and `/etc/bash.bashrc`, and the `/usr/local/bin/pulse` + convenience symlink (`install_binary_symlink`, which must also keep an + already-correct link without rewriting it). An abort in any of these kills + the installer after the new binary is installed and the service is + stopped, landing in the rollback branch above; transient read-only + remounts hit the same paths on unhardened installs. Pinned by + `scripts/installtests/root_install_sh_test.go` + (`TestRootInstallScriptUpdateHelperWriteIsNonFatalOnReadOnlyPath`, + `TestRootInstallScriptBinarySymlinkIsIdempotentAndNonFatal`) and + `scripts/tests/test-install-update-resilience.sh`. The top-level `install.sh` asset published on GitHub Releases must be the root Pulse SERVER installer (the LXC / systemd / Proxmox VE installer that accepts `--version vX.Y.Z`, `--rc`, `--stable`, and friends). The rendered diff --git a/install.sh b/install.sh index 7c24d8c3c..d7ea1f13c 100755 --- a/install.sh +++ b/install.sh @@ -3173,6 +3173,30 @@ download_release_archive() { return 0 } +# Create (or refresh) the convenience symlink to the installed binary. +# Never fatal and idempotent: the link path can be unwritable — the generated +# pulse-update.service runs the unattended updater with ProtectSystem=strict +# (only the install dir, config dir and /tmp are writable), and transient +# read-only filesystems hit the same error (#1630). The symlink is a +# nice-to-have; an existing correct link is kept and a failure only warns. +install_binary_symlink() { + local target="$1" + local link_path="$2" + + if [[ "$(readlink "$link_path" 2>/dev/null)" == "$target" ]]; then + print_success "Symlink already in place at $link_path" + return 0 + fi + + mkdir -p "$(dirname "$link_path")" 2>/dev/null || true + if ln -sf "$target" "$link_path" 2>/dev/null; then + print_success "Symlink created at $link_path" + else + print_warn "Could not create symlink at $link_path (read-only filesystem?); pulse remains available at $target" + fi + return 0 +} + install_pulse_archive() { local archive_path="$1" local expected_release="${2:-}" @@ -3263,10 +3287,8 @@ install_pulse_archive() { chown -R pulse:pulse "$INSTALL_DIR" rm -f "$INSTALL_DIR/bin/pulse.old" - mkdir -p "$(dirname "$BINARY_LINK_PATH")" - ln -sf "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH" print_success "Pulse binary installed to $INSTALL_DIR/bin/pulse" - print_success "Symlink created at $BINARY_LINK_PATH" + install_binary_symlink "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH" if [[ -f "$temp_extract/VERSION" ]]; then cp "$temp_extract/VERSION" "$INSTALL_DIR/VERSION" @@ -3730,8 +3752,7 @@ build_from_source() { fi done - mkdir -p "$(dirname "$BINARY_LINK_PATH")" - ln -sf "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH" + install_binary_symlink "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH" echo "$branch-$(git rev-parse --short HEAD)" > "$INSTALL_DIR/VERSION" echo "$branch" > "$BUILD_FROM_SOURCE_MARKER" @@ -3816,7 +3837,14 @@ setup_update_command() { local update_timer_path="${UPDATE_TIMER_PATH:-${PULSE_UPDATE_TIMER_PATH:-/etc/systemd/system/${service_name}-update.timer}}" local profile_path="${PULSE_PROFILE_PATH:-/etc/profile}" local bashrc_path="${PULSE_BASHRC_PATH:-/etc/bash.bashrc}" - cat > "$update_helper_path" </dev/null || true + if ! cat 2>/dev/null > "$update_helper_path" </dev/null || \ + print_warn "Could not mark $update_helper_path executable" # Ensure /usr/local/bin is in PATH for all users if ! grep -q '/usr/local/bin' "$profile_path" 2>/dev/null; then - echo 'export PATH="/usr/local/bin:$PATH"' >> "$profile_path" + echo 'export PATH="/usr/local/bin:$PATH"' 2>/dev/null >> "$profile_path" || \ + print_warn "Could not update PATH in $profile_path" fi # Also add to bash profile if it exists if [[ -f "$bashrc_path" ]] && ! grep -q '/usr/local/bin' "$bashrc_path" 2>/dev/null; then - echo 'export PATH="/usr/local/bin:$PATH"' >> "$bashrc_path" + echo 'export PATH="/usr/local/bin:$PATH"' 2>/dev/null >> "$bashrc_path" || \ + print_warn "Could not update PATH in $bashrc_path" fi } diff --git a/scripts/installtests/pulse_auto_update_test.go b/scripts/installtests/pulse_auto_update_test.go index 0fd6a44b7..23080c5fe 100644 --- a/scripts/installtests/pulse_auto_update_test.go +++ b/scripts/installtests/pulse_auto_update_test.go @@ -387,3 +387,141 @@ func TestInstalledBinaryIsPulseProGuard(t *testing.T) { } }) } + +// TestPerformUpdateRestartsServiceWhenInstallerFails asserts the #1630 +// guarantee: when the downloaded installer exits non-zero (for example a +// write to a read-only path aborting it under errexit) after it has already +// stopped the service, perform_update must restore the backup AND leave the +// service running. The generated pulse-update.service gates on +// ExecCondition=systemctl is-active, so a service left stopped would also +// disable every future unattended run. +func TestPerformUpdateRestartsServiceWhenInstallerFails(t *testing.T) { + script := ` +set -uo pipefail +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +GITHUB_REPO="rcourtman/Pulse" +INSTALL_DIR="$TMP/opt/pulse" +CONFIG_DIR="$TMP/etc/pulse" +mkdir -p "$INSTALL_DIR/bin" "$CONFIG_DIR" +printf 'v5.1.24\n' > "$INSTALL_DIR/VERSION" +printf '#!/usr/bin/env bash\necho v5.1.24\n' > "$INSTALL_DIR/bin/pulse" +chmod +x "$INSTALL_DIR/bin/pulse" +export INSTALL_DIR + +log() { echo "[$1] ${*:2}"; } +detect_service_name() { echo pulse; } +get_current_version() { tr -d '\r\n' < "$INSTALL_DIR/VERSION"; } +verify_release_signature() { return 0; } +sleep() { :; } + +# curl writes the installer / signature to the -o target; the fake installer +# fails outright, like the real one aborting on a read-only filesystem after +# it has stopped the service. +curl() { + local out="" prev="" arg + for arg in "$@"; do + if [[ "$prev" == "-o" ]]; then out="$arg"; fi + prev="$arg" + done + if [[ -n "$out" ]]; then + case "$out" in + *.sig.*) printf 'dummy-signature\n' > "$out" ;; + *) printf '#!/usr/bin/env bash\nexit 1\n' > "$out" ;; + esac + fi + return 0 +} + +# Service is running at the was-active capture, then down (the installer +# stopped it before failing) until an explicit start/restart. +IS_ACTIVE_CALLS=0 +SERVICE_UP="no" +STARTS=0 +systemctl() { + case "$1" in + is-active) + ((IS_ACTIVE_CALLS += 1)) + if (( IS_ACTIVE_CALLS == 1 )); then return 0; fi + [[ "$SERVICE_UP" == "yes" ]] && return 0 || return 1 + ;; + start|restart) + ((STARTS += 1)) + SERVICE_UP="yes" + return 0 + ;; + esac + return 1 +} +` + extractAutoUpdateFunction(t, "is_prerelease_tag") + ` +` + extractAutoUpdateFunction(t, "resolve_install_script_url") + ` +` + extractAutoUpdateFunction(t, "wait_for_service_active") + ` +` + extractAutoUpdateFunction(t, "ensure_service_restarted") + ` +` + extractAutoUpdateFunction(t, "perform_update") + ` +if perform_update v5.1.25; then + echo "RESULT:succeeded" +else + echo "RESULT:failed" +fi +echo "STARTS:$STARTS" +echo "SERVICE:$SERVICE_UP" +echo "VERSION:$(tr -d '\r\n' < "$INSTALL_DIR/VERSION")" +` + + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{"RESULT:failed", "SERVICE:yes", "VERSION:v5.1.24"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in perform_update installer-failure output:\n%s", want, got) + } + } + if strings.Contains(got, "STARTS:0") { + t.Fatalf("service was never restarted after installer failure:\n%s", got) + } +} + +// TestEnsureServiceRestartedHonorsPriorServiceState asserts the RETURN-trap +// backstop behind the #1630 fix: a service that was inactive before the +// update is left alone, and a service that was active is started again. +func TestEnsureServiceRestartedHonorsPriorServiceState(t *testing.T) { + script := ` +set -uo pipefail +SERVICE_UP="no" +STARTS=0 +systemctl() { + case "$1" in + is-active) [[ "$SERVICE_UP" == "yes" ]] && return 0 || return 1 ;; + start|restart) ((STARTS += 1)); SERVICE_UP="yes"; return 0 ;; + esac + return 1 +} +sleep() { :; } +log() { echo "[$1] ${*:2}"; } +` + extractAutoUpdateFunction(t, "wait_for_service_active") + ` +` + extractAutoUpdateFunction(t, "ensure_service_restarted") + ` +ensure_service_restarted pulse false || echo "INACTIVE_PATH_FAILED" +echo "STARTS_AFTER_INACTIVE:$STARTS" +ensure_service_restarted pulse true || echo "ACTIVE_PATH_FAILED" +echo "STARTS_AFTER_ACTIVE:$STARTS" +echo "SERVICE:$SERVICE_UP" +` + + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{"STARTS_AFTER_INACTIVE:0", "STARTS_AFTER_ACTIVE:1", "SERVICE:yes"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in ensure_service_restarted output:\n%s", want, got) + } + } + for _, reject := range []string{"INACTIVE_PATH_FAILED", "ACTIVE_PATH_FAILED"} { + if strings.Contains(got, reject) { + t.Fatalf("ensure_service_restarted must always return 0 (it runs in a RETURN trap under set -e):\n%s", got) + } + } +} diff --git a/scripts/installtests/root_install_sh_test.go b/scripts/installtests/root_install_sh_test.go index aabb7fef1..2d632ed50 100644 --- a/scripts/installtests/root_install_sh_test.go +++ b/scripts/installtests/root_install_sh_test.go @@ -604,8 +604,8 @@ func TestRootInstallScriptSupportsInstanceScopedServerInstalls(t *testing.T) { `UPDATE_SERVICE_PATH="${PULSE_UPDATE_SERVICE_PATH:-$(default_update_service_path_for_service "$SERVICE_NAME")}"`, `UPDATE_TIMER_PATH="${PULSE_UPDATE_TIMER_PATH:-$(default_update_timer_path_for_service "$SERVICE_NAME")}"`, `if [[ "$SERVICE_NAME_EXPLICIT" == "true" ]]; then`, - `mkdir -p "$(dirname "$BINARY_LINK_PATH")"`, - `ln -sf "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH"`, + `install_binary_symlink "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH"`, + `ln -sf "$target" "$link_path"`, `safe_systemctl enable "$update_timer_unit" || true`, `safe_systemctl start "$update_timer_unit" || true`, `Environment="PULSE_SERVICE_NAME=$service_name"`, @@ -1350,3 +1350,94 @@ func TestRootInstallServiceGrantsIcmpProbeCapability(t *testing.T) { t.Fatal("AmbientCapabilities=CAP_NET_RAW is not in the unit's security hardening block") } } + +// TestRootInstallScriptUpdateHelperWriteIsNonFatalOnReadOnlyPath asserts the +// #1630 guarantee: setup_update_command's write of the /bin/update helper +// (and its PATH appends) must not abort the installer under errexit when the +// destination is unwritable. The stock pulse-update.service runs the +// unattended updater with ProtectSystem=strict, leaving /bin and +// /usr/local/bin read-only; the old behavior killed the installer after the +// new binary was installed and the service stopped, and the auto-update +// rollback then left Pulse down. A regular file as the "parent directory" +// makes writes beneath it fail with ENOTDIR, which also fails when the test +// runs as root (unlike chmod 555). +func TestRootInstallScriptUpdateHelperWriteIsNonFatalOnReadOnlyPath(t *testing.T) { + script := ` +set -euo pipefail +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +touch "$TMP/blocker" +GITHUB_REPO="rcourtman/Pulse" +INSTALL_SIGNATURE_IDENTITY="pulse-installer" +INSTALL_SIGNATURE_NAMESPACE="pulse-install" +PINNED_RELEASE_SSH_PUBLIC_KEY="test-key" +print_warn() { echo "WARN: $*"; } +print_success() { echo "OK: $*"; } +release_signature_key_available() { :; } +require_release_signature_verifier() { :; } +verify_release_signature() { :; } +` + extractRootInstallShellFunction(t, "setup_update_command") + ` +UPDATE_HELPER_PATH="$TMP/blocker/update" \ +PULSE_PROFILE_PATH="$TMP/profile" \ +PULSE_BASHRC_PATH="$TMP/bashrc" \ +setup_update_command +echo "SURVIVED_UNWRITABLE" +UPDATE_HELPER_PATH="$TMP/bin/update" \ +PULSE_PROFILE_PATH="$TMP/profile" \ +PULSE_BASHRC_PATH="$TMP/bashrc" \ +setup_update_command +[[ -x "$TMP/bin/update" ]] && echo "HELPER_WRITTEN" +grep -q "Pulse update command" "$TMP/bin/update" && echo "HELPER_BODY_OK" +` + + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("setup_update_command aborted the installer on an unwritable helper path: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{"SURVIVED_UNWRITABLE", "WARN:", "HELPER_WRITTEN", "HELPER_BODY_OK"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in setup_update_command output:\n%s", want, got) + } + } +} + +// TestRootInstallScriptBinarySymlinkIsIdempotentAndNonFatal asserts the +// companion #1630 guarantee for the /usr/local/bin/pulse convenience +// symlink: an unwritable link path only warns, a writable one creates the +// link, and an already-correct link is kept without needing ln at all +// (the update-run case where the link survives but the fs is read-only). +func TestRootInstallScriptBinarySymlinkIsIdempotentAndNonFatal(t *testing.T) { + script := ` +set -euo pipefail +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +touch "$TMP/blocker" +mkdir -p "$TMP/bin" +touch "$TMP/bin/pulse-binary" +print_warn() { echo "WARN: $*"; } +print_success() { echo "OK: $*"; } +` + extractRootInstallShellFunction(t, "install_binary_symlink") + ` +install_binary_symlink "$TMP/bin/pulse-binary" "$TMP/blocker/pulse" +echo "SURVIVED_UNWRITABLE" +install_binary_symlink "$TMP/bin/pulse-binary" "$TMP/bin/pulse" +[[ "$(readlink "$TMP/bin/pulse")" == "$TMP/bin/pulse-binary" ]] && echo "LINK_CREATED" +ln() { echo "LN_CALLED_AGAIN"; return 1; } +install_binary_symlink "$TMP/bin/pulse-binary" "$TMP/bin/pulse" +echo "SURVIVED_EXISTING_LINK" +` + + out, err := exec.Command("bash", "-c", script).CombinedOutput() + if err != nil { + t.Fatalf("install_binary_symlink aborted under errexit: %v\n%s", err, out) + } + got := string(out) + for _, want := range []string{"SURVIVED_UNWRITABLE", "WARN:", "LINK_CREATED", "already in place", "SURVIVED_EXISTING_LINK"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in install_binary_symlink output:\n%s", want, got) + } + } + if strings.Contains(got, "LN_CALLED_AGAIN") { + t.Fatalf("install_binary_symlink should not invoke ln when the correct link already exists:\n%s", got) + } +} diff --git a/scripts/pulse-auto-update.sh b/scripts/pulse-auto-update.sh index 83a5c7636..6d7ee2321 100755 --- a/scripts/pulse-auto-update.sh +++ b/scripts/pulse-auto-update.sh @@ -346,6 +346,35 @@ wait_for_service_active() { return 1 } +# Guarantee the Pulse service is left running after an update attempt if (and +# only if) it was running beforehand. Invoked from perform_update's RETURN trap +# so that no exit path — present or future — can leave Pulse stopped (#1630: +# the install-failed rollback branch restored the backup but never restarted +# the service). This matters doubly because the generated pulse-update.service +# uses ExecCondition=systemctl is-active pulse.service: once Pulse is down, +# every subsequent timer run is skipped and the install stays down until +# someone intervenes. Always returns 0 (it runs under set -e in a trap). +ensure_service_restarted() { + local service_name=$1 + local service_was_active=$2 + + if [[ "$service_was_active" != "true" ]]; then + return 0 + fi + if systemctl is-active --quiet "$service_name" 2>/dev/null; then + return 0 + fi + + log warn "Pulse service is not active after update attempt; starting it" + systemctl start "$service_name" 2>/dev/null || true + if wait_for_service_active "$service_name" 20; then + log info "Pulse service is running again" + else + log error "Pulse service could not be started after update attempt; manual intervention required (systemctl start $service_name)" + fi + return 0 +} + perform_update() { local new_version=$1 local service_name=$(detect_service_name) @@ -360,6 +389,11 @@ perform_update() { service_was_active="true" fi + # Whatever way this function exits, never leave Pulse stopped when it was + # running before the update (#1630). Extended below once the installer + # tempfiles exist. + trap 'ensure_service_restarted "$service_name" "$service_was_active"' RETURN + # Refuse to install a prerelease via the unattended updater. The stable # channel must never cross onto a tag like v6.0.0-rc.2, even if every # caller above this point thought it was safe. @@ -406,7 +440,7 @@ perform_update() { installer_tmp=$(mktemp /tmp/pulse-update-installer.XXXXXX) signature_tmp=$(mktemp /tmp/pulse-update-installer.sig.XXXXXX) - trap 'rm -f "$installer_tmp" "$signature_tmp"' RETURN + trap 'rm -f "$installer_tmp" "$signature_tmp"; ensure_service_restarted "$service_name" "$service_was_active"' RETURN if ! curl -fsSL "$install_script_url" -o "$installer_tmp"; then log error "Failed to download installer from $install_script_url" @@ -510,10 +544,18 @@ perform_update() { if [[ -f "$backup_dir/VERSION" ]]; then cp -f "$backup_dir/VERSION" "$INSTALL_DIR/VERSION" fi - + + # Restart the restored binary if Pulse was running before the update. + # The installer stops the service before it can fail, so skipping this + # left Pulse down indefinitely (#1630); the RETURN trap above is the + # backstop if this path ever changes. + if [[ "$service_was_active" == "true" ]]; then + systemctl restart "$service_name" || true + fi + # Clean up backup rm -rf "$backup_dir" - + return 1 fi } diff --git a/scripts/tests/test-install-update-resilience.sh b/scripts/tests/test-install-update-resilience.sh index 17af0e208..bf92467bd 100755 --- a/scripts/tests/test-install-update-resilience.sh +++ b/scripts/tests/test-install-update-resilience.sh @@ -68,4 +68,68 @@ fi [[ "$START_ATTEMPTS" -ge 1 ]] || fail "should attempt an explicit restart, got $START_ATTEMPTS" [[ "$PRINT_BUF" == *"did not come back up"* ]] || fail "should surface a clear error, got: $PRINT_BUF" -echo "PASS: install.sh update-resilience helpers (#1323)" +# --- Cases 5+: read-only-filesystem resilience (#1630) ----------------------- +# The unattended updater runs the installer under pulse-update.service, whose +# ProtectSystem=strict leaves /bin and /usr/local/bin read-only; transient +# read-only remounts hit the same paths. Writes outside the install dir must +# be non-fatal: aborting used to kill the installer with errexit active after +# the new binary was installed and the service stopped. +TMPDIR_RO="$(mktemp -d)" +# A regular file as the "parent directory" makes any write beneath it fail +# with ENOTDIR — unlike chmod 555, this also fails when running as root. +touch "$TMPDIR_RO/blocker" + +# --- Case 5: setup_update_command must not abort the installer (errexit) ---- +out=$( + set -e + PRINT_BUF="" + UPDATE_HELPER_PATH="$TMPDIR_RO/blocker/update" + PULSE_PROFILE_PATH="$TMPDIR_RO/profile" + PULSE_BASHRC_PATH="$TMPDIR_RO/bashrc" + setup_update_command + printf '%s' "$PRINT_BUF" +) +rc=$? +[[ "$rc" -eq 0 ]] || fail "setup_update_command must not abort the installer when the helper path is unwritable (rc=$rc)" +[[ "$out" == *"WARN:"*"$TMPDIR_RO/blocker/update"* ]] || fail "should warn about the unwritable helper path, got: $out" + +# --- Case 6: setup_update_command still writes the helper when it can ------- +out=$( + set -e + PRINT_BUF="" + UPDATE_HELPER_PATH="$TMPDIR_RO/bin/update" + PULSE_PROFILE_PATH="$TMPDIR_RO/profile" + PULSE_BASHRC_PATH="$TMPDIR_RO/bashrc" + setup_update_command + printf '%s' "$PRINT_BUF" +) +rc=$? +[[ "$rc" -eq 0 ]] || fail "setup_update_command should succeed on a writable path (rc=$rc)" +[[ -x "$TMPDIR_RO/bin/update" ]] || fail "should create an executable update helper at a writable path" +grep -q "Pulse update command" "$TMPDIR_RO/bin/update" || fail "helper should contain the expected script body" + +# --- Case 7: install_binary_symlink is non-fatal on an unwritable path ------ +PRINT_BUF="" +install_binary_symlink "$TMPDIR_RO/bin/update" "$TMPDIR_RO/blocker/pulse" || \ + fail "install_binary_symlink must not fail on an unwritable link path" +[[ "$PRINT_BUF" == *"WARN:"* ]] || fail "should warn when the symlink cannot be created, got: $PRINT_BUF" + +# --- Case 8: install_binary_symlink creates and keeps the link --------------- +PRINT_BUF="" +install_binary_symlink "$TMPDIR_RO/bin/update" "$TMPDIR_RO/bin/pulse" || \ + fail "install_binary_symlink should succeed on a writable path" +[[ "$(readlink "$TMPDIR_RO/bin/pulse")" == "$TMPDIR_RO/bin/update" ]] || \ + fail "symlink should point at the installed binary" +# Idempotence: with the correct link already present it must not need ln at +# all (matches an update run where the link survives but the fs is read-only). +ln() { return 1; } +PRINT_BUF="" +install_binary_symlink "$TMPDIR_RO/bin/update" "$TMPDIR_RO/bin/pulse" || \ + fail "install_binary_symlink should succeed when the correct link already exists" +[[ "$PRINT_BUF" == *"already in place"* ]] || \ + fail "should recognise an existing correct link, got: $PRINT_BUF" +unset -f ln + +rm -rf "$TMPDIR_RO" + +echo "PASS: install.sh update-resilience helpers (#1323, #1630)" diff --git a/scripts/tests/test-pulse-auto-update.sh b/scripts/tests/test-pulse-auto-update.sh index fdbd1f31b..4347487cc 100755 --- a/scripts/tests/test-pulse-auto-update.sh +++ b/scripts/tests/test-pulse-auto-update.sh @@ -199,12 +199,170 @@ INSTALLER return "${status}" } +test_ensure_service_restarted_noops_when_service_was_inactive() { + ENSURE_TEST_STARTS=0 + systemctl() { + if [[ "$1" == "start" ]] || [[ "$1" == "restart" ]]; then + ((ENSURE_TEST_STARTS += 1)) + fi + return 1 + } + sleep() { :; } + + ensure_service_restarted pulse "false" || return 1 + if (( ENSURE_TEST_STARTS != 0 )); then + echo "expected no start attempts when the service was inactive before the update, got ${ENSURE_TEST_STARTS}" >&2 + return 1 + fi + return 0 +} + +test_ensure_service_restarted_starts_stopped_service() { + ENSURE_TEST_STARTS=0 + ENSURE_TEST_UP="no" + systemctl() { + case "$1" in + is-active) + [[ "$ENSURE_TEST_UP" == "yes" ]] && return 0 || return 1 + ;; + start|restart) + ((ENSURE_TEST_STARTS += 1)) + ENSURE_TEST_UP="yes" + return 0 + ;; + esac + return 1 + } + sleep() { :; } + + ensure_service_restarted pulse "true" || return 1 + if (( ENSURE_TEST_STARTS != 1 )); then + echo "expected exactly one start attempt, got ${ENSURE_TEST_STARTS}" >&2 + return 1 + fi + if [[ "$ENSURE_TEST_UP" != "yes" ]]; then + echo "expected the service to be running afterwards" >&2 + return 1 + fi + return 0 +} + +test_perform_update_restarts_service_when_installer_fails() { + # Regression for #1630: the installer stops the service and can then exit + # non-zero (e.g. a write to a read-only path aborts it). perform_update's + # rollback branch must restart Pulse — previously it restored the backup + # and returned 1 with the service left stopped, which also disabled every + # future timer run via pulse-update.service's ExecCondition. + local tmpdir + tmpdir="$(mktemp -d)" + local status=0 + # perform_update installs a RETURN trap referencing these; declare them here + # so the trap is safe under set -u if it surfaces in this calling scope. + local installer_tmp="" signature_tmp="" + local service_name="pulse" service_was_active="false" + + INSTALL_DIR="${tmpdir}/opt/pulse" + CONFIG_DIR="${tmpdir}/etc/pulse" + mkdir -p "${INSTALL_DIR}/bin" "${CONFIG_DIR}" + + printf 'v5.1.24\n' > "${INSTALL_DIR}/VERSION" + cat > "${INSTALL_DIR}/bin/pulse" <<'EOF' +#!/usr/bin/env bash +echo "v5.1.24" +EOF + chmod +x "${INSTALL_DIR}/bin/pulse" + + export INSTALL_DIR + + is_prerelease_tag() { return 1; } + detect_service_name() { echo "pulse"; } + resolve_install_script_url() { echo "http://localhost/install.sh"; } + verify_release_signature() { return 0; } + get_current_version() { tr -d '\r\n' < "${INSTALL_DIR}/VERSION"; } + + # curl writes the installer / signature to the -o target. The fake installer + # fails outright, like the /bin/update heredoc aborting on a read-only + # filesystem after the real installer has already stopped the service. + curl() { + local out="" prev="" + local arg + for arg in "$@"; do + if [[ "$prev" == "-o" ]]; then out="$arg"; fi + prev="$arg" + done + if [[ -n "$out" ]]; then + case "$out" in + *.sig.*) printf 'dummy-signature\n' > "$out" ;; + *) + cat > "$out" <<'INSTALLER' +#!/usr/bin/env bash +exit 1 +INSTALLER + ;; + esac + fi + return 0 + } + + # Service is running when perform_update checks (was-active capture), then + # down (the real installer stops it before failing) until start/restart. + AUTOUPDATE_TEST_IS_ACTIVE_CALLS=0 + AUTOUPDATE_TEST_UP="no" + AUTOUPDATE_TEST_STARTS=0 + systemctl() { + case "$1" in + is-active) + ((AUTOUPDATE_TEST_IS_ACTIVE_CALLS += 1)) + if (( AUTOUPDATE_TEST_IS_ACTIVE_CALLS == 1 )); then + return 0 + fi + [[ "$AUTOUPDATE_TEST_UP" == "yes" ]] && return 0 || return 1 + ;; + start|restart) + ((AUTOUPDATE_TEST_STARTS += 1)) + AUTOUPDATE_TEST_UP="yes" + return 0 + ;; + esac + return 1 + } + + sleep() { :; } + + if perform_update "v5.1.25"; then + echo "perform_update unexpectedly succeeded when the installer failed" >&2 + status=1 + fi + # perform_update installs a RETURN trap; clear it so it does not leak into + # subsequent function returns in this sourced test harness. + trap - RETURN 2>/dev/null || true + + if (( AUTOUPDATE_TEST_STARTS < 1 )); then + echo "expected the service to be restarted after installer failure, got ${AUTOUPDATE_TEST_STARTS} start attempts" >&2 + status=1 + fi + if [[ "$AUTOUPDATE_TEST_UP" != "yes" ]]; then + echo "expected the service to be running after the failed update" >&2 + status=1 + fi + if [[ "$(tr -d '\r\n' < "${INSTALL_DIR}/VERSION")" != "v5.1.24" ]]; then + echo "expected VERSION to remain v5.1.24 after failed install" >&2 + status=1 + fi + + rm -rf "${tmpdir}" + return "${status}" +} + main() { assert_success "wait_for_service_active retries until active" test_wait_for_service_active_succeeds_after_retry assert_success "wait_for_service_active times out when never active" test_wait_for_service_active_times_out_when_never_active assert_success "pick_highest_stable_tag ignores list order and prereleases" test_pick_highest_stable_tag_ignores_list_order_and_prereleases assert_success "get_latest_stable_version prefers highest version over created order" test_get_latest_stable_version_prefers_highest_over_created_order assert_success "perform_update restores backup when service stays down" test_perform_update_restores_backup_when_service_stays_down + assert_success "ensure_service_restarted no-ops when service was inactive" test_ensure_service_restarted_noops_when_service_was_inactive + assert_success "ensure_service_restarted starts a stopped service" test_ensure_service_restarted_starts_stopped_service + assert_success "perform_update restarts service when installer fails" test_perform_update_restarts_service_when_installer_fails if (( failures > 0 )); then echo "Total failures: ${failures}" >&2