From c8ff52339f722dbfef76513a1def80a93f6dbd84 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 2 Aug 2026 18:50:21 +0100 Subject: [PATCH] Rotate configuration backups instead of growing without bound Every unattended update created another full config snapshot under config-backups (or next to the config dir) and nothing ever pruned old ones, so small root filesystems filled up within days (#1646, reported on the hardened-unit fallback path where snapshots land under the install dir). backup_existing now keeps the five newest snapshots and removes the rest after each successful copy. Contract-Neutral: installer config backup rotation; shell-only fix, no runtime contract --- install.sh | 29 +++++++ scripts/installtests/root_install_sh_test.go | 81 ++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/install.sh b/install.sh index 2c619c76b..21b723ae7 100755 --- a/install.sh +++ b/install.sh @@ -2845,6 +2845,34 @@ create_user() { fi } +prune_config_backups() { + # Unattended updates create one snapshot per run; without rotation they + # fill small root filesystems (issue #1646). Keep the newest snapshots + # and delete the rest. Snapshot names embed a sortable timestamp, so a + # lexicographic sort orders them oldest-first. + local backup_parent="$1" + local snapshot_prefix="$2" + local keep="${CONFIG_BACKUP_KEEP_COUNT:-5}" + local backups=() + local entry + + while IFS= read -r entry; do + [[ -n "$entry" ]] && backups+=("$entry") + done < <(find "$backup_parent" -maxdepth 1 -mindepth 1 -type d -name "${snapshot_prefix}*" 2>/dev/null | sort) + + local excess=$(( ${#backups[@]} - keep )) + if (( excess <= 0 )); then + return 0 + fi + + local i + for (( i = 0; i < excess; i++ )); do + if ! rm -rf -- "${backups[$i]}"; then + print_warn "Could not remove old configuration backup ${backups[$i]}" + fi + done +} + backup_existing() { if [[ -d "$CONFIG_DIR" ]]; then print_info "Backing up existing configuration..." @@ -2863,6 +2891,7 @@ backup_existing() { rm -rf "$backup_dir" return 1 fi + prune_config_backups "$backup_parent" "$(basename "$CONFIG_DIR").backup." fi } diff --git a/scripts/installtests/root_install_sh_test.go b/scripts/installtests/root_install_sh_test.go index 5c3e58c92..8b0c902cc 100644 --- a/scripts/installtests/root_install_sh_test.go +++ b/scripts/installtests/root_install_sh_test.go @@ -326,6 +326,87 @@ func TestRootInstallScriptConfigBackupCleansPartialCopy(t *testing.T) { } } +// Issue #1646: unattended updates created one config snapshot per run under +// the hardened-unit fallback directory and never pruned old ones, so small +// root filesystems filled up. backup_existing must rotate snapshots after a +// successful copy. +func TestRootInstallScriptConfigBackupRotatesOldSnapshots(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores write bits, so the read-only fallback path cannot be simulated") + } + configDir := t.TempDir() + installDir := t.TempDir() + backupParent := filepath.Join(installDir, "config-backups") + if err := os.MkdirAll(backupParent, 0755); err != nil { + t.Fatalf("mkdir backup parent: %v", err) + } + prefix := filepath.Base(configDir) + ".backup." + oldStamps := []string{"20260701-020000", "20260702-020000", "20260703-020000", "20260704-020000", "20260705-020000"} + for _, stamp := range oldStamps { + if err := os.MkdirAll(filepath.Join(backupParent, prefix+stamp), 0755); err != nil { + t.Fatalf("mkdir old snapshot: %v", err) + } + } + + script := ` + set -euo pipefail + print_error() { :; } + print_info() { :; } + print_warn() { :; } + CONFIG_DIR="$CONFIG_DIR_UNDER_TEST" + INSTALL_DIR="$INSTALL_DIR_UNDER_TEST" + CONFIG_BACKUP_MIN_EXTRA_BYTES=0 +` + extractRootInstallShellFunction(t, "bytes_to_human") + ` +` + extractRootInstallShellFunction(t, "get_available_bytes_for_path") + ` +` + extractRootInstallShellFunction(t, "get_directory_size_bytes") + ` +` + extractRootInstallShellFunction(t, "ensure_config_backup_headroom") + ` +` + extractRootInstallShellFunction(t, "prune_config_backups") + ` +` + extractRootInstallShellFunction(t, "backup_existing") + ` + date() { printf '20260706-020000\n'; } + chmod a-w "$(dirname "$CONFIG_DIR_UNDER_TEST")" 2>/dev/null || true + trap 'chmod u+w "$(dirname "$CONFIG_DIR_UNDER_TEST")" 2>/dev/null || true' EXIT + backup_existing + ` + + cmd := exec.Command("bash", "-c", script) + cmd.Env = append(os.Environ(), + "CONFIG_DIR_UNDER_TEST="+configDir, + "INSTALL_DIR_UNDER_TEST="+installDir, + ) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("bash: %v\n%s", err, out) + } + + entries, err := os.ReadDir(backupParent) + if err != nil { + t.Fatalf("read backup parent: %v", err) + } + names := make([]string, 0, len(entries)) + for _, entry := range entries { + names = append(names, entry.Name()) + } + if len(names) != 5 { + t.Fatalf("expected 5 snapshots after rotation, got %d: %v", len(names), names) + } + for _, gone := range []string{prefix + "20260701-020000"} { + for _, name := range names { + if name == gone { + t.Fatalf("expected oldest snapshot %s to be pruned, still present: %v", gone, names) + } + } + } + found := false + for _, name := range names { + if name == prefix+"20260706-020000" { + found = true + } + } + if !found { + t.Fatalf("expected fresh snapshot to exist, got %v", names) + } +} + func TestRootInstallScriptV5ToV6PreflightWarnsWhenAgentScopeMissing(t *testing.T) { configDir := t.TempDir() if err := os.WriteFile(filepath.Join(configDir, "api_tokens.json"), []byte(`[{"id":"tok-1","name":"admin","hash":"hash","scopes":["settings:read"]}]`), 0600); err != nil {