From 6dc43d298c607c215580d5683d6fbaa74ef8a54c Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:49:36 +0100 Subject: [PATCH] Stage server update archives on the install filesystem, not /tmp The server installer downloaded the release archive and extracted it under /tmp. Debian 13 and many LXC templates mount /tmp as tmpfs, whose pages are charged to the container's memory cgroup. A v6.5.0 archive is ~280 MB and extracts to ~750 MB, so an unattended update on a 1 GiB LXC filled the cgroup with ~820 MB of shmem; the kernel OOM-killed the container's systemd and Proxmox stopped the container mid-update. Download and extract into owner-only hidden directories under the install directory instead. It is always writable from the hardened pulse-update.service sandbox, it is where the staged binary already lives, and the disk headroom preflight now measures the filesystem the update actually stages on. The Go updater already prefers its data directory over /tmp for the same reason. Unlike /tmp, the install filesystem keeps an interrupted run's staging across reboots. Staging setup now removes stale staging directories that match only the exact owner-only names, never symlinks or files, and only once their ctime is over six hours old: install.sh has no update lock, so age is what proves no live run still owns them. Every recursive pulse chown of the install directory now goes through one helper that skips staging, so the payload root still reads is never handed to the service account and the ~750 MB tree is not rewritten for nothing. The "Failed to print table: Broken pipe" line in the report was systemctl list-unit-files | grep -q in v6.5.0's service discovery; main already reads that inventory in full, and it was not the failure. Contract-Neutral: Defect fix inside the existing private server-installer inputs and update-space boundaries; staging moves from tmpfs-prone /tmp to the install filesystem with no public contract change --- install.sh | 78 +++++++++- scripts/installtests/root_install_sh_test.go | 135 +++++++++++++++++- scripts/tests/test_server_installer_stop.py | 15 ++ .../test_server_installer_temporary_inputs.py | 18 ++- 4 files changed, 234 insertions(+), 12 deletions(-) mode change 100755 => 100644 install.sh diff --git a/install.sh b/install.sh old mode 100755 new mode 100644 index d1ee9ab5ef..24f406711c --- a/install.sh +++ b/install.sh @@ -107,6 +107,7 @@ BUILD_FROM_SOURCE_MARKER="$INSTALL_DIR/BUILD_FROM_SOURCE" DETECTED_CTID="" UPDATE_MIN_TEMP_FREE_BYTES=$((900 * 1024 * 1024)) UPDATE_MIN_INSTALL_FREE_BYTES=$((256 * 1024 * 1024)) +SERVER_STAGING_STALE_MINUTES=360 CONFIG_BACKUP_MIN_EXTRA_BYTES=$((64 * 1024 * 1024)) release_signature_key_available() { @@ -3291,8 +3292,62 @@ create_temp_archive_path() { # Reserving then unlinking a mktemp file does not reserve the .tar.gz or # sidecar name. Keep both downloads inside one owner-only directory. + # A hidden staging prefix still yields the canonical archive file name. + local archive_name="${prefix##*/}" + archive_name="${archive_name#.}" temp_dir=$(mktemp -d "${prefix}-XXXXXX") || return 1 - printf '%s/%s.tar.gz\n' "$temp_dir" "${prefix##*/}" + printf '%s/%s.tar.gz\n' "$temp_dir" "$archive_name" +} + +# Server archives are staged and extracted on the install filesystem. /tmp is +# tmpfs on Debian 13 and many LXC templates, where staged bytes count against +# the container's memory limit rather than its disk (#2785). +prepare_server_staging_parent() { + if ! mkdir -p -- "$INSTALL_DIR"; then + print_error "Could not prepare $INSTALL_DIR for staging the Pulse update; Pulse has not been stopped" + return 1 + fi + sweep_stale_server_staging + printf '%s\n' "$INSTALL_DIR" +} + +# Owner-only staging names created under $INSTALL_DIR by create_temp_archive_path +# and install_pulse_archive (mktemp's six-character suffix). +is_server_staging_name() { + case "$1" in + .pulse-extract-??????|.pulse-v*-linux-*-??????) return 0 ;; + esac + return 1 +} + +# Unlike /tmp, the install filesystem keeps an interrupted run's staging +# (~1 GB) across reboots. No installer lock exists, so remove only real +# directories (never symlinks or files) unchanged for longer than any live run +# takes. ctime, unlike mtime, cannot be carried back by extraction. +sweep_stale_server_staging() { + local entry="" + for entry in "$INSTALL_DIR"/.pulse-extract-* "$INSTALL_DIR"/.pulse-v*-linux-*; do + [[ -d "$entry" && ! -L "$entry" ]] || continue + is_server_staging_name "${entry##*/}" || continue + if [[ -n "$(find "$entry" -maxdepth 0 -cmin "+${SERVER_STAGING_STALE_MINUTES}" -print 2>/dev/null)" ]]; then + rm -rf -- "$entry" 2>/dev/null || print_warn "Could not remove stale update staging at $entry" + fi + done + return 0 +} + +# Recursive pulse ownership for the install directory, excluding root-consumed +# update staging so the extracted payload and archive are never handed to the +# service account while the installer still reads them. +chown_install_dir_for_pulse() { + local entry="" rc=0 + chown pulse:pulse "$INSTALL_DIR" || rc=1 + for entry in "$INSTALL_DIR"/* "$INSTALL_DIR"/.[!.]* "$INSTALL_DIR"/..?*; do + [[ -e "$entry" || -L "$entry" ]] || continue + is_server_staging_name "${entry##*/}" && continue + chown -R pulse:pulse "$entry" || rc=1 + done + return "$rc" } cleanup_temp_archive_path() { @@ -3536,7 +3591,9 @@ install_pulse_archive() { expected_release=$(infer_release_from_archive_name "$archive_path" 2>/dev/null || true) fi - if ! temp_extract=$(mktemp -d /tmp/pulse-extract-XXXXXX); then + local staging_parent="" + if ! staging_parent=$(prepare_server_staging_parent) || + ! temp_extract=$(mktemp -d "$staging_parent/.pulse-extract-XXXXXX"); then print_error "Could not prepare archive extraction; Pulse has not been stopped" return 1 fi @@ -3614,7 +3671,7 @@ install_pulse_archive() { install_additional_agent_binaries "$expected_release" "$temp_extract" deploy_agent_scripts "$temp_extract" - chown -R pulse:pulse "$INSTALL_DIR" + chown_install_dir_for_pulse print_success "Pulse binary installed to $INSTALL_DIR/bin/pulse" install_binary_symlink "$INSTALL_DIR/bin/pulse" "$BINARY_LINK_PATH" @@ -3653,8 +3710,15 @@ download_pulse() { local pulse_arch="" local archive_from_temp=false local inferred_release="" + local staging_parent="" - if ! ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"; then + # The archive download and its extraction live on the install + # filesystem, never /tmp. Debian 13 and many LXC templates mount /tmp + # as tmpfs, which the kernel charges to the container's memory limit: + # a ~280 MB archive plus its ~750 MB extracted tree OOM-killed a 1 GiB + # LXC's init mid-update and Proxmox stopped the container (#2785). + if ! staging_parent=$(prepare_server_staging_parent) || + ! ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"; then # The configuration snapshot taken earlier is not needed: the update # never reached staging. Remove it so a retry loop on a low-space # filesystem does not accumulate backups and worsen the shortage @@ -3696,7 +3760,7 @@ download_pulse() { print_info "Downloading Pulse..." resolve_target_release - archive_path=$(create_temp_archive_path "/tmp/pulse-${LATEST_RELEASE}-linux-${pulse_arch}") || { + archive_path=$(create_temp_archive_path "$staging_parent/.pulse-${LATEST_RELEASE}-linux-${pulse_arch}") || { print_error "Failed to create temporary archive path" exit 1 } @@ -4083,7 +4147,7 @@ build_from_source() { echo "$branch-$source_revision" > "$INSTALL_DIR/VERSION" echo "$branch" > "$BUILD_FROM_SOURCE_MARKER" - chown -R pulse:pulse "$INSTALL_DIR" 2>/dev/null || true + chown_install_dir_for_pulse 2>/dev/null || true chown pulse:pulse "$BUILD_FROM_SOURCE_MARKER" 2>/dev/null || true cd "$original_dir" >/dev/null 2>&1 || true @@ -4104,7 +4168,7 @@ setup_directories() { # Set permissions (preserve existing files) # Use chown without -R on CONFIG_DIR to avoid changing existing file permissions chown pulse:pulse "$CONFIG_DIR" - chown -R pulse:pulse "$INSTALL_DIR" + chown_install_dir_for_pulse chmod 700 "$CONFIG_DIR" # Ensure critical config files retain proper permissions if they exist diff --git a/scripts/installtests/root_install_sh_test.go b/scripts/installtests/root_install_sh_test.go index 12da4f886b..b6d051f1f1 100644 --- a/scripts/installtests/root_install_sh_test.go +++ b/scripts/installtests/root_install_sh_test.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "regexp" + "sort" "strings" "testing" "time" @@ -78,7 +79,7 @@ func TestRootInstallScriptArchiveSupportContract(t *testing.T) { `ensure_update_disk_headroom()`, `UPDATE_MIN_TEMP_FREE_BYTES=$((900 * 1024 * 1024))`, `UPDATE_MIN_INSTALL_FREE_BYTES=$((256 * 1024 * 1024))`, - `ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`, + `ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`, `prefetch_pulse_archive_for_container()`, `download_release_archive()`, `install_pulse_archive()`, @@ -111,7 +112,7 @@ func TestRootInstallSystemdServiceExposesOnlyPulseOwnedSubscriptionCLIHome(t *te func TestRootInstallScriptStagesUpdateBeforeStoppingService(t *testing.T) { downloadPulse := extractRootInstallShellFunction(t, "download_pulse") orderedSteps := []string{ - `ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`, + `ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`, `download_release_archive "$LATEST_RELEASE" "$pulse_arch" "$archive_path"`, `run_upgrade_readiness_preflight "$CURRENT_VERSION" "$expected_release"`, `install_pulse_archive "$archive_path" "$expected_release"`, @@ -536,7 +537,7 @@ func TestRootInstallScriptDiscardsUnneededConfigBackup(t *testing.T) { // backups while the filesystem is short of space (#2127). func TestRootInstallScriptDiscardsBackupWhenStagingHeadroomFails(t *testing.T) { downloadPulse := extractRootInstallShellFunction(t, "download_pulse") - headroom := strings.Index(downloadPulse, `ensure_update_disk_headroom "/tmp" "$INSTALL_DIR"`) + headroom := strings.Index(downloadPulse, `ensure_update_disk_headroom "$staging_parent" "$INSTALL_DIR"`) discard := strings.Index(downloadPulse, `discard_last_config_backup`) if headroom < 0 || discard < 0 { t.Fatalf("download_pulse must check staging headroom and discard the unneeded config backup (headroom=%d discard=%d)", headroom, discard) @@ -2591,3 +2592,131 @@ func TestRootInstallResetBoundsEverySystemdObservationAndMutation(t *testing.T) } } } + +// Server update staging lives under the install directory (#2785), which keeps +// an interrupted run's ~1 GB extract across reboots. The sweep removes only +// owner-only staging directories older than any live run, never symlinks, +// files, look-alike names or anything else in the install directory. +func TestRootInstallScriptSweepsOnlyStaleServerStaging(t *testing.T) { + installDir := filepath.Join(t.TempDir(), "install") + outside := filepath.Join(t.TempDir(), "outside") + for _, dir := range []string{ + filepath.Join(installDir, "bin"), + filepath.Join(installDir, ".pulse-extract-AbC123", "bin"), + filepath.Join(installDir, ".pulse-v6.6.0-linux-amd64-XyZ789"), + filepath.Join(installDir, ".pulse-extract-toolong1"), + outside, + } { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + } + files := []string{ + filepath.Join(installDir, ".pulse-extract-AbC123", "bin", "pulse"), + filepath.Join(installDir, ".pulse-v6.6.0-linux-amd64-XyZ789", "pulse-v6.6.0-linux-amd64.tar.gz"), + filepath.Join(installDir, ".pulse-extract-file01"), + filepath.Join(installDir, "bin", "pulse"), + filepath.Join(outside, "keep"), + } + for _, file := range files { + if err := os.WriteFile(file, []byte("fixture"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(outside, filepath.Join(installDir, ".pulse-extract-link01")); err != nil { + t.Fatal(err) + } + + sweep := func(minutes string) { + t.Helper() + script := ` + set -euo pipefail + print_warn() { echo "$*" >&2; } + INSTALL_DIR="$1" + SERVER_STAGING_STALE_MINUTES="$2" +` + extractRootInstallShellFunction(t, "is_server_staging_name") + ` +` + extractRootInstallShellFunction(t, "sweep_stale_server_staging") + ` + sweep_stale_server_staging + ` + if out, err := exec.Command("bash", "-c", script, "bash", installDir, minutes).CombinedOutput(); err != nil { + t.Fatalf("sweep: %v\n%s", err, out) + } + } + + // A live run's staging is fresh: the production age keeps it. + sweep("360") + for _, file := range files { + if _, err := os.Stat(file); err != nil { + t.Fatalf("fresh staging or unrelated file removed: %s", file) + } + } + + time.Sleep(1100 * time.Millisecond) + sweep("0") + for _, removed := range []string{".pulse-extract-AbC123", ".pulse-v6.6.0-linux-amd64-XyZ789"} { + if _, err := os.Lstat(filepath.Join(installDir, removed)); !os.IsNotExist(err) { + t.Fatalf("stale staging %s was not removed (err=%v)", removed, err) + } + } + for _, kept := range []string{".pulse-extract-toolong1", ".pulse-extract-file01", ".pulse-extract-link01", "bin/pulse"} { + if _, err := os.Lstat(filepath.Join(installDir, kept)); err != nil { + t.Fatalf("sweep removed non-staging entry %s: %v", kept, err) + } + } + if _, err := os.Stat(filepath.Join(outside, "keep")); err != nil { + t.Fatalf("sweep followed a staging-named symlink: %v", err) + } +} + +// The recursive pulse ownership pass must not hand root-consumed update +// staging (verified archive, extracted payload) to the service account, and +// every recursive install-dir chown goes through that one helper. +func TestRootInstallScriptPulseOwnershipExcludesServerStaging(t *testing.T) { + content, err := os.ReadFile(filepath.Join("..", "..", "install.sh")) + if err != nil { + t.Fatal(err) + } + if regexp.MustCompile(`chown -R [^\n]*"\$INSTALL_DIR"`).Match(content) { + t.Fatal(`install.sh must use chown_install_dir_for_pulse, not chown -R on "$INSTALL_DIR"`) + } + + installDir := filepath.Join(t.TempDir(), "install") + for _, dir := range []string{"bin", "scripts", ".pulse-extract-AbC123", ".pulse-v6.6.0-linux-amd64-XyZ789", ".hidden"} { + if err := os.MkdirAll(filepath.Join(installDir, dir), 0o700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(installDir, "VERSION"), []byte("v6.6.0\n"), 0o600); err != nil { + t.Fatal(err) + } + calls := filepath.Join(t.TempDir(), "calls") + script := ` + set -euo pipefail + INSTALL_DIR="$1" + CALLS="$2" + chown() { printf '%s\n' "$*" >> "$CALLS"; } +` + extractRootInstallShellFunction(t, "is_server_staging_name") + ` +` + extractRootInstallShellFunction(t, "chown_install_dir_for_pulse") + ` + chown_install_dir_for_pulse + ` + if out, err := exec.Command("bash", "-c", script, "bash", installDir, calls).CombinedOutput(); err != nil { + t.Fatalf("chown_install_dir_for_pulse: %v\n%s", err, out) + } + recorded, err := os.ReadFile(calls) + if err != nil { + t.Fatal(err) + } + got := strings.Split(strings.TrimSpace(string(recorded)), "\n") + want := []string{ + "pulse:pulse " + installDir, + "-R pulse:pulse " + filepath.Join(installDir, "VERSION"), + "-R pulse:pulse " + filepath.Join(installDir, "bin"), + "-R pulse:pulse " + filepath.Join(installDir, "scripts"), + "-R pulse:pulse " + filepath.Join(installDir, ".hidden"), + } + sort.Strings(got) + sort.Strings(want) + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("ownership calls:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) + } +} diff --git a/scripts/tests/test_server_installer_stop.py b/scripts/tests/test_server_installer_stop.py index f0d7e2fd5e..155d04c156 100644 --- a/scripts/tests/test_server_installer_stop.py +++ b/scripts/tests/test_server_installer_stop.py @@ -231,6 +231,8 @@ class ServerInstallerStopTest(unittest.TestCase): "originals": {str(p.relative_to(fixture)): b for p, b in originals.items()}, "calls": (fixture / "calls").read_text() if (fixture / "calls").exists() else "", "archive_retained": (fixture / "archive.tgz").exists(), + "temporary_paths": (fixture / "temporary-paths").read_text().splitlines() if (fixture / "temporary-paths").exists() else [], + "install_dir": installed, } def test_refused_replacements_preserve_installed_bytes_and_modes(self): @@ -353,6 +355,19 @@ class ServerInstallerStopTest(unittest.TestCase): self.assertEqual(observation["temporary_paths_remaining"], []) self.assertNotIn("start pulse-custom", calls) + def test_archive_extraction_stages_on_the_install_filesystem(self): + # /tmp is tmpfs on Debian 13 and many LXC templates: an extracted + # release there is charged to the container's memory limit and + # OOM-killed a 1 GiB LXC mid-update (#2785). + for flow in ("archive", "local_archive"): + with self.subTest(flow=flow): + observation = self.exercise(REPLACEMENT, flow, "active") + self.assertEqual(observation["result"].returncode, 0, observation["result"].stderr) + extracts = [Path(p) for p in observation["temporary_paths"] if "pulse-extract-" in p] + self.assertEqual(len(extracts), 1, observation["temporary_paths"]) + self.assertEqual(extracts[0].parent, observation["install_dir"]) + self.assertEqual(observation["temporary_paths_remaining"], []) + def test_all_manual_update_flows_leave_service_running_when_staging_fails(self): for flow in ("version", "update", "reinstall"): with self.subTest(flow=flow): diff --git a/scripts/tests/test_server_installer_temporary_inputs.py b/scripts/tests/test_server_installer_temporary_inputs.py index e54c6c3896..5da82fbdc9 100644 --- a/scripts/tests/test_server_installer_temporary_inputs.py +++ b/scripts/tests/test_server_installer_temporary_inputs.py @@ -26,6 +26,7 @@ mktemp() { [[ "$FAULT" != allocation ]] || return 1 local template="${@: -1}" created local -a args=("$@") + printf '%s\n' "$template" >> "$FIXTURE/templates" args[${#args[@]}-1]="$FIXTURE/shared/${template##*/}" created=$(command mktemp "${args[@]}") || return 1 printf '%s\n' "$created" >> "$FIXTURE/created" @@ -66,7 +67,7 @@ BUILD_FROM_SOURCE_MARKER="$INSTALL_DIR/BUILD_FROM_SOURCE" CURRENT_VERSION=v6.5.0 LATEST_RELEASE=v6.6.0 detect_pulse_architecture() { echo amd64; } -ensure_update_disk_headroom() { return 0; } +ensure_update_disk_headroom() { printf '%s\n' "$1" > "$FIXTURE/headroom-path"; } resolve_target_release() { :; } download_release_archive() { printf '%s\n' "$3" > "$FIXTURE/archive-path" @@ -204,7 +205,8 @@ class ServerInstallerTemporaryInputsTest(unittest.TestCase): result = subprocess.run(command, env=env, text=True, capture_output=True, timeout=15) observed = {} - for name in ("archive-path", "returned", "inferred", "calls", "transport", "directory-modes"): + for name in ("archive-path", "returned", "inferred", "calls", "transport", "directory-modes", + "templates", "headroom-path"): p = fixture / name observed[name] = p.read_text().splitlines() if p.exists() else [] archive_path = Path(observed["archive-path"][0]) if observed["archive-path"] else None @@ -258,6 +260,18 @@ class ServerInstallerTemporaryInputsTest(unittest.TestCase): self.assertEqual(o["shared_entries"], [], o) self.assertEqual(o["calls"], ["INSTALL"] if fault in ("", "archive_admission") else []) + def test_download_stages_archive_on_the_install_filesystem(self): + # /tmp is tmpfs on Debian 13 and many LXC templates, where a staged + # release is charged to the container's memory limit (#2785). + r, o = self.exercise("download") + self.assertEqual(r.returncode, 0, r.stderr) + self.assertEqual(len(o["templates"]), 1, o) + template = Path(o["templates"][0]) + self.assertEqual(template.parent.name, "install", o) + self.assertEqual(template.name, ".pulse-v6.6.0-linux-amd64-XXXXXX") + self.assertEqual(Path(o["headroom-path"][0]), template.parent) + self.assertEqual(Path(o["archive-path"][0]).name, "pulse-v6.6.0-linux-amd64.tar.gz") + def test_explicit_archive_and_sidecar_are_never_owned_or_deleted(self): for fault in ("", "readiness", "archive_admission"): with self.subTest(fault=fault):