mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
courtmanr@gmail.com
5c1c89a363
Adversarial review of9db25ba60found four residual defects in the auto-update asset install, plus a doc line it left contradicting itself. - install_auto_update_assets copied the bundled helper into the staged mktemp file with an unchecked cp, and both call sites invoke the function under `if !`, which suppresses errexit for its whole body. A failed copy (ENOSPC, EIO) fell through to configure_auto_update_script_repo, whose awk emits a lone GITHUB_REPO= line for empty input, so a shebang-less one-line stub replaced the working helper with a "script" that only ever exits 0 - silently disabling unattended updates. Check the cp, and refuse the swap unless the staged helper is non-empty and starts with #!. - Both units were rendered with a bare truncating `cat > "$unit"` whose status was never checked, and the function's last statement is safe_systemctl daemon-reload, which returns 0 by design. A failing write truncated a working unit and still reported success. Render each unit to ${path}.tmp and commit it with a checked rename, so a failure leaves the installed unit byte-identical. - The widened ReadWritePaths could not reach deployed boxes: the unit that grants the write access is itself the file that has to be rewritten, and on an existing install the sandbox running the installer excludes /etc/systemd/system and /usr/local/bin (EROFS). The Go update pipeline cannot carry it either - pulse.service runs as User=pulse with its own ProtectSystem=strict over the install and config dirs only. So probe each destination directory up front and, when one is blocked, re-exec this already-signature-verified installer through systemd-run with a new internal --repair-auto-update-units entry point: PID 1 forks the transient unit, so it starts in the host mount namespace instead of inheriting the sandbox. The installer is copied into the install dir first because the calling unit's PrivateTmp=yes hides its /tmp copy from PID 1. The escape needs root and systemd-run, and never recurses. - Keep the ReadWritePaths entries as directory grants: every write now commits with a rename from a sibling staging file, and rename needs write access on the containing directory, so the file-level entries systemd would otherwise accept cannot work. Document the tradeoff in the unit and the subsystem contract instead. The deployment-installability contract still claimed the update sandbox leaves "only the install dir, config dir and /tmp" writable, which the paragraph the same file gained in9db25ba60contradicts; the same stale rationale had been copied into two test comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
138 lines
6.1 KiB
Bash
Executable File
138 lines
6.1 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
#
|
|
# Tests the interactive-update service-resilience helpers in install.sh (#1323):
|
|
# after an update that stopped a running Pulse, the installer must verify the
|
|
# service came back up, retry one explicit start, and surface a clear error
|
|
# instead of silently leaving Pulse stopped (common on unprivileged LXC where
|
|
# the installer's restart silently fails).
|
|
set -uo pipefail
|
|
|
|
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
|
INSTALL_SCRIPT="${ROOT_DIR}/install.sh"
|
|
|
|
# Sourcing relies on the BASH_SOURCE guard so main() does not run on import.
|
|
source "${INSTALL_SCRIPT}"
|
|
set +e # this test drives helpers that intentionally return non-zero
|
|
|
|
SERVICE_NAME="pulse"
|
|
|
|
# --- Deterministic stubs: no real sleeps, no real systemctl/timeout ---------
|
|
sleep() { :; }
|
|
timeout() { shift; "$@"; } # drop the duration; run the wrapped (stubbed) command
|
|
|
|
SYSTEMCTL_ACTIVE="no"
|
|
START_ATTEMPTS=0
|
|
systemctl() {
|
|
case "$1" in
|
|
is-active) [[ "$SYSTEMCTL_ACTIVE" == "yes" ]] && return 0 || return 1 ;;
|
|
stop) SYSTEMCTL_ACTIVE="no"; return 0 ;;
|
|
start) START_ATTEMPTS=$((START_ATTEMPTS + 1)); return 0 ;;
|
|
*) return 0 ;;
|
|
esac
|
|
}
|
|
safe_systemctl() { systemctl "$@"; }
|
|
|
|
PRINT_BUF=""
|
|
print_info() { PRINT_BUF+="INFO:$*"$'\n'; }
|
|
print_warn() { PRINT_BUF+="WARN:$*"$'\n'; }
|
|
print_error() { PRINT_BUF+="ERROR:$*"$'\n'; }
|
|
print_success() { PRINT_BUF+="OK:$*"$'\n'; }
|
|
|
|
fail() { echo "FAIL: $*" >&2; exit 1; }
|
|
|
|
# --- Case 1: stop_pulse_for_update records whether Pulse was running --------
|
|
SYSTEMCTL_ACTIVE="yes"; PULSE_WAS_ACTIVE="false"
|
|
stop_pulse_for_update
|
|
[[ "$PULSE_WAS_ACTIVE" == "true" ]] || fail "should record was-active=true when running"
|
|
[[ "$SYSTEMCTL_ACTIVE" == "no" ]] || fail "should stop the service"
|
|
|
|
SYSTEMCTL_ACTIVE="no"; PULSE_WAS_ACTIVE="true"
|
|
stop_pulse_for_update
|
|
[[ "$PULSE_WAS_ACTIVE" == "false" ]] || fail "should record was-active=false when not running"
|
|
|
|
# --- Case 2: no-op when Pulse was not running before the update ------------
|
|
PULSE_WAS_ACTIVE="false"; PRINT_BUF=""
|
|
ensure_pulse_running_after_update || fail "should no-op (succeed) when was-active=false"
|
|
[[ -z "$PRINT_BUF" ]] || fail "should be silent when was-active=false, got: $PRINT_BUF"
|
|
|
|
# --- Case 3: service comes back up -> success, flag consumed ----------------
|
|
PULSE_WAS_ACTIVE="true"; SYSTEMCTL_ACTIVE="yes"; PRINT_BUF=""
|
|
ensure_pulse_running_after_update || fail "should succeed when the service is active"
|
|
[[ "$PULSE_WAS_ACTIVE" == "false" ]] || fail "should consume the was-active flag"
|
|
|
|
# --- Case 4: service stays down -> retries once, then a clear error ---------
|
|
PULSE_WAS_ACTIVE="true"; SYSTEMCTL_ACTIVE="no"; START_ATTEMPTS=0; PRINT_BUF=""
|
|
if ensure_pulse_running_after_update; then
|
|
fail "should return non-zero when the service will not come up"
|
|
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"
|
|
|
|
# --- Cases 5+: read-only-filesystem resilience (#1630) -----------------------
|
|
# The unattended updater runs the installer under pulse-update.service, whose
|
|
# ProtectSystem=strict grants ReadWritePaths only for the install dir, config
|
|
# dir, /tmp and the auto-update helper and unit directories — so /bin (the
|
|
# default update-helper path) and /etc/profile stay read-only; transient
|
|
# read-only remounts hit the same paths. Writes outside the writable set 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)"
|