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):