Fix doubled auto-update schedule and unfreeze sandboxed refreshes

Residual auto-update defects found while triaging #1643 and #1637
(the primary regression was fixed in 806cbe83d):

- The generated pulse-update.timer carried both OnCalendar=daily and
  OnCalendar=02:00, so with RandomizedDelaySec=4h every box attempted
  two updates per day (00:00-04:00 and 02:00-06:00 windows). Keep the
  single documented 02:00 schedule.
- The generated pulse-update.service sandbox (ProtectSystem=strict)
  excluded the helper and unit directories from ReadWritePaths, so the
  unattended path could never refresh /usr/local/bin/pulse-auto-update.sh
  or rewrite the units - updater fixes only reached boxes via manual
  installs. Grant the sandbox write access to both directories on
  purpose.
- Because the unattended path replaces the helper bash is currently
  executing, stage the new helper next to its destination and swap it
  in with an atomic rename only after repo configuration succeeds. A
  failed download or configure now leaves the previously working helper
  in place instead of rm -f'ing it out from under the enabled timer's
  ExecStart.
- Delete scripts/systemd/pulse-update.{service,timer}: orphaned
  reference copies that had drifted from the units install.sh actually
  generates and were referenced by nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs/release-control/v6/internal/subsystems/deployment-installability.md

@@ -1896,8 +1896,25 @@ already-installed auto-update assets unconditionally via
 helper script and rewriting the units so a version-pinned helper from a
 previous major (which never selects newer releases and reports "Already
 running latest version" forever) cannot survive an upgrade — while leaving
-`system.json` and the timer's enabled/started state untouched. The
-rendered-unit execution, refresh-behavior, and call-site wiring tests in
+`system.json` and the timer's enabled/started state untouched. The rendered
+timer must schedule exactly one update attempt per day — a single
+`OnCalendar` entry at 02:00 with the 4h randomized spread — because a second
+`OnCalendar=daily` line silently doubled every box's daily update attempts
+(issue #1643). The rendered service sandbox must keep the helper's directory
+and the unit directory inside `ReadWritePaths`: unattended updates run
+`install.sh` (and therefore `refresh_auto_updates`) under that very sandbox,
+and a sandbox that excludes those paths freezes every installed helper and
+unit at whatever a manual install last wrote (issue #1637 triage). Because
+the unattended path replaces the helper script that bash is currently
+executing, `install_auto_update_assets` must stage the new helper in the
+destination directory and swap it in with an atomic same-filesystem rename
+only after repo configuration succeeds; no failure path may delete or
+truncate a previously working helper (the old `rm -f` on configure failure
+left the enabled timer with a dangling `ExecStart`). The generated units are
+the only unit source — no checked-in reference copies of
+`pulse-update.service` / `pulse-update.timer` may exist to drift from the
+heredocs. The rendered-unit execution, schedule, sandbox-writability,
+failure-preservation, refresh-behavior, and call-site wiring tests in
 `scripts/installtests/root_install_sh_test.go` are the owned proof surface
 for these invariants.
 That same server-installer uninstall must also leave no legacy companion

install.sh

@@ -4080,24 +4080,46 @@ install_auto_update_assets() {
     local update_timer_path="${UPDATE_TIMER_PATH:-${PULSE_UPDATE_TIMER_PATH:-/etc/systemd/system/${service_name}-update.timer}}"
     local update_timer_unit
     update_timer_unit="$(basename "$update_timer_path")"
+    local auto_update_bin_dir update_unit_dir update_timer_dir
+    auto_update_bin_dir="$(dirname "$auto_update_dest")"
+    update_unit_dir="$(dirname "$update_service_path")"
+    update_timer_dir="$(dirname "$update_timer_path")"
+    local unit_write_dirs="$update_unit_dir"
+    if [[ "$update_timer_dir" != "$update_unit_dir" ]]; then
+        unit_write_dirs="$unit_write_dirs $update_timer_dir"
+    fi
 
-    # Copy auto-update script if it exists in the release
+    # Stage the helper next to its destination and only swap it in once it is
+    # fully configured. During unattended updates install.sh runs under the
+    # pulse-update.service sandbox, where the file being replaced is the very
+    # script bash is executing — the same-directory mv keeps the swap an
+    # atomic rename — and a failed download or configure must leave the
+    # previously working helper in place rather than deleting it out from
+    # under the timer's ExecStart.
+    local staged_helper
+    if ! staged_helper=$(mktemp "${auto_update_dest}.staged.XXXXXX"); then
+        print_warn "Cannot write to ${auto_update_bin_dir} to stage the auto-update helper."
+        return 1
+    fi
     if [[ -f "$install_dir/scripts/pulse-auto-update.sh" ]]; then
-        cp "$install_dir/scripts/pulse-auto-update.sh" "$auto_update_dest"
-        chmod +x "$auto_update_dest"
+        # Copy auto-update script if it exists in the release
+        cp "$install_dir/scripts/pulse-auto-update.sh" "$staged_helper"
     else
         print_info "Downloading auto-update script..."
-        if ! download_auto_update_script; then
+        if ! AUTO_UPDATE_DEST="$staged_helper" download_auto_update_script; then
             print_warn "Could not download the auto-update helper after multiple attempts."
+            rm -f "$staged_helper"
             return 1
         fi
     fi
 
-    if ! configure_auto_update_script_repo "$auto_update_dest"; then
+    if ! configure_auto_update_script_repo "$staged_helper"; then
         print_warn "Could not configure the auto-update helper for the selected release repo."
-        rm -f "$auto_update_dest"
+        rm -f "$staged_helper"
         return 1
     fi
+    chmod +x "$staged_helper"
+    mv "$staged_helper" "$auto_update_dest"
 
     # Install systemd timer and service. The heredoc is unquoted so the
     # installer substitutes paths/names; the rendered unit must contain no
@@ -4129,7 +4151,10 @@ Environment="PULSE_UPDATE_TIMER_UNIT=$update_timer_unit"
 PrivateTmp=yes
 ProtectHome=yes
 ProtectSystem=strict
-ReadWritePaths=$install_dir $config_dir /tmp
+# The helper and unit directories are writable on purpose: the installer this
+# service runs refreshes the auto-update helper and rewrites these units, and
+# without write access updater fixes only ever reach boxes via manual installs.
+ReadWritePaths=$install_dir $config_dir /tmp $auto_update_bin_dir $unit_write_dirs
 PrivateNetwork=no
 Nice=10
 
@@ -4145,8 +4170,10 @@ After=network-online.target
 Wants=network-online.target
 
 [Timer]
-OnCalendar=daily
-OnCalendar=02:00
+# One update attempt per day, landing in the 02:00-06:00 window. A second
+# OnCalendar=daily line here used to double the schedule with an extra
+# midnight trigger (issue #1643).
+OnCalendar=*-*-* 02:00:00
 RandomizedDelaySec=4h
 Persistent=true
 AccuracySec=1h
@@ -4210,7 +4237,7 @@ setup_auto_updates() {
     # Start the timer
     safe_systemctl start "$update_timer_unit" || true
 
-    print_success "Automatic updates enabled (daily check with 2-6 hour random delay)"
+    print_success "Automatic updates enabled (daily check between 02:00 and 06:00)"
 }
 
 # Updates and reinstalls only run setup_auto_updates when the user opts in,

scripts/installtests/root_install_sh_test.go

@@ -1441,3 +1441,196 @@ echo "SURVIVED_EXISTING_LINK"
 		t.Fatalf("install_binary_symlink should not invoke ln when the correct link already exists:\n%s", got)
 	}
 }
+
+// renderAutoUpdateUnits runs install_auto_update_assets against a seeded
+// release helper and returns the rendered service and timer unit contents.
+func renderAutoUpdateUnits(t *testing.T) (string, string, string, string) {
+	t.Helper()
+
+	tmpDir := t.TempDir()
+	configDir := filepath.Join(tmpDir, "config")
+	installDir := filepath.Join(tmpDir, "install")
+	autoUpdateSrc := filepath.Join(installDir, "scripts", "pulse-auto-update.sh")
+	autoUpdateDest, servicePath, timerPath := prepareAutoUpdatePaths(t, tmpDir)
+
+	if err := os.MkdirAll(configDir, 0755); err != nil {
+		t.Fatalf("mkdir config dir: %v", err)
+	}
+	if err := os.MkdirAll(filepath.Dir(autoUpdateSrc), 0755); err != nil {
+		t.Fatalf("mkdir auto-update src dir: %v", err)
+	}
+	if err := os.WriteFile(autoUpdateSrc, []byte("#!/usr/bin/env bash\n"), 0755); err != nil {
+		t.Fatalf("write auto-update src: %v", err)
+	}
+
+	script := `
+		CONFIG_DIR="` + configDir + `"
+		INSTALL_DIR="` + installDir + `"
+		PULSE_AUTO_UPDATE_DEST="` + autoUpdateDest + `"
+		PULSE_UPDATE_SERVICE_PATH="` + servicePath + `"
+		PULSE_UPDATE_TIMER_PATH="` + timerPath + `"
+		GITHUB_REPO="rcourtman/Pulse"
+		print_info() { :; }
+		print_warn() { :; }
+		safe_systemctl() { :; }
+` + extractRootInstallShellFunction(t, "repo_web_url") + `
+` + extractRootInstallShellFunction(t, "configure_auto_update_script_repo") + `
+` + extractRootInstallShellFunction(t, "install_auto_update_assets") + `
+		install_auto_update_assets
+	`
+
+	out, err := exec.Command("bash", "-c", script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("bash: %v\n%s", err, out)
+	}
+
+	serviceBytes, err := os.ReadFile(servicePath)
+	if err != nil {
+		t.Fatalf("read rendered service unit: %v", err)
+	}
+	timerBytes, err := os.ReadFile(timerPath)
+	if err != nil {
+		t.Fatalf("read rendered timer unit: %v", err)
+	}
+	return string(serviceBytes), string(timerBytes), autoUpdateDest, servicePath
+}
+
+// Regression test for the doubled auto-update schedule (issue #1643): the
+// rendered timer carried both OnCalendar=daily and OnCalendar=02:00, so with
+// RandomizedDelaySec=4h every box attempted two updates per day, one in the
+// 00:00-04:00 window and one in 02:00-06:00. Exactly one OnCalendar line may
+// survive, and it must be the documented 02:00 schedule.
+func TestAutoUpdateTimerSchedulesSingleDailyRun(t *testing.T) {
+	_, timer, _, _ := renderAutoUpdateUnits(t)
+
+	var schedules []string
+	for _, line := range strings.Split(timer, "\n") {
+		if strings.HasPrefix(strings.TrimSpace(line), "OnCalendar=") {
+			schedules = append(schedules, strings.TrimSpace(line))
+		}
+	}
+	if len(schedules) != 1 {
+		t.Fatalf("rendered timer must hold exactly one OnCalendar line, got %d:\n%s", len(schedules), timer)
+	}
+	if schedules[0] != "OnCalendar=*-*-* 02:00:00" {
+		t.Fatalf("rendered timer schedule = %q, want the documented 02:00 daily run:\n%s", schedules[0], timer)
+	}
+	if !strings.Contains(timer, "RandomizedDelaySec=4h\n") {
+		t.Fatalf("rendered timer lost the 4h random spread:\n%s", timer)
+	}
+}
+
+// Regression test for the sandbox that froze updater fixes (issue #1637
+// triage): pulse-update.service runs install.sh with ProtectSystem=strict and
+// ReadWritePaths that excluded the helper and unit directories, so
+// refresh_auto_updates could never replace /usr/local/bin/pulse-auto-update.sh
+// or rewrite the units during an unattended update — helper fixes only reached
+// boxes via manual installs. The rendered sandbox must grant write access to
+// both directories.
+func TestAutoUpdateServiceSandboxAllowsHelperAndUnitRefresh(t *testing.T) {
+	service, _, autoUpdateDest, servicePath := renderAutoUpdateUnits(t)
+
+	var rwLine string
+	for _, line := range strings.Split(service, "\n") {
+		if strings.HasPrefix(line, "ReadWritePaths=") {
+			rwLine = line
+		}
+	}
+	if rwLine == "" {
+		t.Fatalf("rendered service unit lost its ReadWritePaths line:\n%s", service)
+	}
+	paths := strings.Fields(strings.TrimPrefix(rwLine, "ReadWritePaths="))
+	want := map[string]bool{
+		filepath.Dir(autoUpdateDest): false,
+		filepath.Dir(servicePath):    false,
+	}
+	for _, p := range paths {
+		if _, ok := want[p]; ok {
+			want[p] = true
+		}
+	}
+	for dir, found := range want {
+		if !found {
+			t.Fatalf("ReadWritePaths %q is missing %q; unattended refreshes cannot write there:\n%s", rwLine, dir, service)
+		}
+	}
+}
+
+// Regression test for the destructive failure path in
+// install_auto_update_assets (issue #1637 triage): a failed
+// configure_auto_update_script_repo used to rm -f the installed helper,
+// leaving the still-enabled timer with a dangling ExecStart. The helper is now
+// staged in its destination directory and swapped in with an atomic rename
+// only after configuration succeeds, so any failure must leave the previously
+// working helper untouched, the units unwritten, and no staging litter behind.
+func TestInstallAutoUpdateAssetsKeepsWorkingHelperWhenConfigureFails(t *testing.T) {
+	tmpDir := t.TempDir()
+	configDir := filepath.Join(tmpDir, "config")
+	installDir := filepath.Join(tmpDir, "install")
+	autoUpdateSrc := filepath.Join(installDir, "scripts", "pulse-auto-update.sh")
+	autoUpdateDest, servicePath, timerPath := prepareAutoUpdatePaths(t, tmpDir)
+
+	if err := os.MkdirAll(configDir, 0755); err != nil {
+		t.Fatalf("mkdir config dir: %v", err)
+	}
+	if err := os.MkdirAll(filepath.Dir(autoUpdateSrc), 0755); err != nil {
+		t.Fatalf("mkdir auto-update src dir: %v", err)
+	}
+	if err := os.WriteFile(autoUpdateSrc, []byte("#!/usr/bin/env bash\necho new-helper\n"), 0755); err != nil {
+		t.Fatalf("write auto-update src: %v", err)
+	}
+	workingHelper := "#!/usr/bin/env bash\necho working-helper\n"
+	if err := os.WriteFile(autoUpdateDest, []byte(workingHelper), 0755); err != nil {
+		t.Fatalf("write installed helper: %v", err)
+	}
+
+	// The failing configure stub is defined after the extracted functions so
+	// it overrides the real implementation.
+	script := `
+		CONFIG_DIR="` + configDir + `"
+		INSTALL_DIR="` + installDir + `"
+		PULSE_AUTO_UPDATE_DEST="` + autoUpdateDest + `"
+		PULSE_UPDATE_SERVICE_PATH="` + servicePath + `"
+		PULSE_UPDATE_TIMER_PATH="` + timerPath + `"
+		GITHUB_REPO="rcourtman/Pulse"
+		print_info() { :; }
+		print_warn() { :; }
+		safe_systemctl() { :; }
+` + extractRootInstallShellFunction(t, "repo_web_url") + `
+` + extractRootInstallShellFunction(t, "install_auto_update_assets") + `
+		configure_auto_update_script_repo() { return 1; }
+		if install_auto_update_assets; then
+			echo "UNEXPECTED_SUCCESS"
+		else
+			echo "FAILED_AS_EXPECTED"
+		fi
+	`
+
+	out, err := exec.Command("bash", "-c", script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("bash: %v\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "FAILED_AS_EXPECTED") {
+		t.Fatalf("install_auto_update_assets should report failure when configure fails:\n%s", out)
+	}
+
+	helper, err := os.ReadFile(autoUpdateDest)
+	if err != nil {
+		t.Fatalf("installed helper is gone after failed configure: %v", err)
+	}
+	if string(helper) != workingHelper {
+		t.Fatalf("failed configure replaced the working helper:\n%s", helper)
+	}
+	if _, err := os.Stat(servicePath); !os.IsNotExist(err) {
+		t.Fatalf("failed configure still rewrote the service unit (stat err %v)", err)
+	}
+	entries, err := os.ReadDir(filepath.Dir(autoUpdateDest))
+	if err != nil {
+		t.Fatalf("read helper dir: %v", err)
+	}
+	for _, entry := range entries {
+		if strings.Contains(entry.Name(), ".staged.") {
+			t.Fatalf("failed configure left staging litter behind: %s", entry.Name())
+		}
+	}
+}

scripts/systemd/pulse-update.service

@@ -1,35 +0,0 @@
-[Unit]
-Description=Automatic Pulse update check and install
-Documentation=https://github.com/rcourtman/Pulse
-After=network-online.target
-Wants=network-online.target
-
-[Service]
-Type=oneshot
-# Run as root to allow service restart
-User=root
-Group=root
-# Skip auto-update run unless a supported Pulse service is active
-ExecCondition=/bin/sh -c 'systemctl is-active --quiet pulse || systemctl is-active --quiet pulse-backend'
-# Use the update script
-ExecStart=/usr/local/bin/pulse-auto-update.sh
-# Restart policy for the update service itself
-Restart=no
-# Timeout for the update process (10 minutes should be plenty)
-TimeoutStartSec=600
-# Log to journal
-StandardOutput=journal
-StandardError=journal
-SyslogIdentifier=pulse-update
-# Security hardening
-PrivateTmp=yes
-ProtectHome=yes
-ProtectSystem=strict
-ReadWritePaths=/opt/pulse /etc/pulse /tmp
-# Network access needed for GitHub
-PrivateNetwork=no
-# Nice level to run updates at lower priority
-Nice=10
-
-[Install]
-WantedBy=multi-user.target

scripts/systemd/pulse-update.timer

@@ -1,19 +0,0 @@
-[Unit]
-Description=Daily check for Pulse updates
-Documentation=https://github.com/rcourtman/Pulse
-After=network-online.target
-Wants=network-online.target
-
-[Timer]
-# Run daily at 2 AM with a random delay up to 4 hours
-# This spreads the load on GitHub and prevents all instances updating at once
-OnCalendar=daily
-OnCalendar=02:00
-RandomizedDelaySec=4h
-# Persist the last trigger time during downtime
-Persistent=true
-# Ensure we run if the system was off when scheduled
-AccuracySec=1h
-
-[Install]
-WantedBy=timers.target