fix: stage binary updates, delegate swap to systemd

Dockerfile

@@ -25,7 +25,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
     && useradd --system --no-create-home --shell /usr/sbin/nologin hyperstack-agent
 
 WORKDIR /app
-COPY --from=builder /app/hyperstack-agent /usr/local/bin/hyperstack-agent
+COPY --from=builder /app/hyperstack-agent /opt/hyperstack-agent/bin/hyperstack-agent
 
 ENV AGENT_VERSION=${VERSION} \
     AGENT_BUILD_DATE=${DATE} \
@@ -41,7 +41,7 @@ COPY scripts /scripts
 RUN chmod 0755 /scripts/*.sh
 USER hyperstack-agent
 
-CMD ["/usr/local/bin/hyperstack-agent"]
+CMD ["/opt/hyperstack-agent/bin/hyperstack-agent"]
 
 FROM runtime-base AS agent
 USER hyperstack-agent

cmd/agent/main.go

@@ -250,18 +250,18 @@ func main() {
 
 			if restartRelease != nil && updater != nil && !externalShutdown {
 				if err := updater.PromoteRelease(executablePath, restartRelease); err != nil {
-					slog.Error("self-update promote failed", "version", restartRelease.Version, "error", err)
-					os.Exit(1)
-				}
-				slog.Info("restarting agent after self-update", "version", restartRelease.Version)
-				if err := update.RestartProcess(executablePath); err != nil {
-					slog.Error("self-update restart failed", "error", err)
-					os.Exit(1)
+					slog.Error("self-update stage failed", "version", restartRelease.Version, "error", err)
+					_ = os.Remove(restartRelease.StagedPath)
+				} else {
+					slog.Info("self-update staged; systemd will swap on restart", "version", restartRelease.Version)
+					slog.Info("Hyperstack agent shutdown complete")
+					// Exit 57: Self-update staged, systemd should restart to apply the staged binary swap
+					os.Exit(57)
 				}
 			} else if restartRelease != nil {
 				_ = os.Remove(restartRelease.StagedPath)
 				if externalShutdown {
-					slog.Info("self-update skipped because shutdown was requested", "version", restartRelease.Version)
+					slog.Info("self-update discarded due to shutdown request", "version", restartRelease.Version)
 				}
 			}
 			slog.Info("Hyperstack agent shutdown complete")

internal/update/manager.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"hash"
 	"io"
-	"log/slog"
 	"net/http"
 	"net/url"
 	"os"
@@ -128,6 +127,25 @@ func (m *Manager) DownloadRelease(ctx context.Context, release *Release, current
 	}
 	defer func() { _ = resp.Body.Close() }()
 
+	// Handle redirects (3xx) by following Location header, since the HTTP client
+	// has CheckRedirect disabled to prevent auto-following in Check().
+	if resp.StatusCode >= 300 && resp.StatusCode < 400 {
+		location := resp.Header.Get("Location")
+		if location == "" {
+			return fmt.Errorf("binary download returned redirect %s with no Location header", resp.Status)
+		}
+		// Follow the redirect with a new request
+		redirectReq, err := http.NewRequestWithContext(ctx, http.MethodGet, location, nil)
+		if err != nil {
+			return err
+		}
+		resp, err = m.Client.Do(redirectReq)
+		if err != nil {
+			return err
+		}
+		defer func() { _ = resp.Body.Close() }()
+	}
+
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("binary download returned status %s", resp.Status)
 	}
@@ -203,14 +221,66 @@ func (m *Manager) PromoteRelease(currentPath string, release *Release) error {
 		return errors.New("release staged path is required")
 	}
 
-	if err := atomicSwap(currentPath, release.StagedPath); err != nil {
-		return err
+	// Stage the new binary as .new for ExecStartPre to swap on next restart.
+	// ExecStartPre will verify the binary, swap it atomically, and restore on failure.
+	stagedPath := currentPath + ".new"
+	if err := os.Rename(release.StagedPath, stagedPath); err != nil {
+		// If rename fails due to cross-device link (e.g., /tmp on different device),
+		// fall back to copying the file.
+		if !isExdev(err) {
+			return err
+		}
+		// Copy staged binary to .new
+		src, err := os.Open(release.StagedPath)
+		if err != nil {
+			return err
+		}
+		defer func() { _ = src.Close() }()
+
+		dst, err := os.Create(stagedPath) /* #nosec G304 -- stagedPath is currentPath + ".new" from os.Executable(), not user-supplied */
+		if err != nil {
+			return err
+		}
+		defer func() { _ = dst.Close() }()
+
+		if _, err := io.Copy(dst, src); err != nil {
+			_ = os.Remove(stagedPath)
+			return err
+		}
+		if err := dst.Sync(); err != nil {
+			_ = os.Remove(stagedPath)
+			return err
+		}
+		// Make the staged binary executable
+		if err := os.Chmod(stagedPath, 0o755); err != nil { /* #nosec G302 -- binary must be world-readable/executable */
+			_ = os.Remove(stagedPath)
+			return err
+		}
+		_ = os.Remove(release.StagedPath)
 	}
 
 	m.CurrentVersion = release.Version
 	return nil
 }
 
+// isExdev checks if an error is EXDEV (cross-device link).
+func isExdev(err error) bool {
+	if err == nil {
+		return false
+	}
+	// Check for syscall.EXDEV directly
+	if errno, ok := err.(syscall.Errno); ok {
+		return errno == syscall.EXDEV
+	}
+	// Check in os.LinkError
+	if linkErr, ok := err.(*os.LinkError); ok {
+		if errno, ok := linkErr.Err.(syscall.Errno); ok {
+			return errno == syscall.EXDEV
+		}
+	}
+	return strings.Contains(err.Error(), "cross-device link") || strings.Contains(err.Error(), "invalid cross-device")
+}
+
 // resolveExecPath resolves symlinks and verifies the result is an absolute path.
 // Both RestartProcess and smokeTestBinary call this before any exec so that
 // the executed path is a concrete, fully-resolved value rather than a raw
@@ -226,42 +296,6 @@ func resolveExecPath(p string) (string, error) {
 	return resolved, nil
 }
 
-// RestartProcess replaces the current process image with the binary at
-// currentPath using a clean exec(2). The path is resolved through symlinks
-// before the exec so the kernel receives a concrete, absolute path.
-func RestartProcess(currentPath string) error {
-	resolved, err := resolveExecPath(currentPath)
-	if err != nil {
-		return err
-	}
-	return syscall.Exec(resolved, os.Args, os.Environ()) /* #nosec G204 G702 -- resolved is the symlink-evaluated, absolute-asserted current executable path */
-}
-
-func atomicSwap(currentPath, newPath string) error {
-	backup := currentPath + ".bak"
-	_ = os.Remove(backup)
-	if err := copyFile(currentPath, backup); err != nil {
-		return err
-	}
-	// os.Rename is atomic on the same filesystem. When the staged binary lives
-	// in os.TempDir() and the install dir is on a different device (common in
-	// Docker / systemd setups), Rename returns EXDEV. Fall back to a copy+remove
-	// so the promote step still succeeds across filesystem boundaries.
-	if err := os.Rename(newPath, currentPath); err != nil {
-		// Cross-device rename (EXDEV) fallback: copyFile opens the destination
-		// with O_TRUNC, so currentPath is zeroed the moment the copy begins.
-		// If the copy fails, restore from the backup made above to avoid leaving
-		// the agent with a truncated (unlaunchable) binary.
-		if err2 := copyFile(newPath, currentPath); err2 != nil {
-			_ = copyFile(backup, currentPath) // best-effort restore
-			_ = os.Remove(newPath)
-			return err2
-		}
-		_ = os.Remove(newPath)
-	}
-	return nil
-}
-
 // Fix 2: verifyDigest now fails closed — an empty digest is treated as an
 // error rather than silently skipping verification. This prevents a stripped
 // or missing Hyperstack-Agent-Digest header from allowing an unverified binary
@@ -337,44 +371,6 @@ func smokeTestBinary(binaryPath string) error {
 	return err
 }
 
-func copyFile(src, dst string) error {
-	srcRoot, err := os.OpenRoot(filepath.Dir(src))
-	if err != nil {
-		return err
-	}
-	defer func() { _ = srcRoot.Close() }()
-	in, err := srcRoot.Open(filepath.Base(src))
-	if err != nil {
-		return err
-	}
-	defer func() { _ = in.Close() }()
-
-	info, err := in.Stat()
-	if err != nil {
-		return err
-	}
-
-	dstRoot, err := os.OpenRoot(filepath.Dir(dst))
-	if err != nil {
-		return err
-	}
-	defer func() { _ = dstRoot.Close() }()
-	out, err := dstRoot.OpenFile(filepath.Base(dst), os.O_CREATE|os.O_TRUNC|os.O_WRONLY, info.Mode())
-	if err != nil {
-		return err
-	}
-	defer func() {
-		if err := out.Close(); err != nil {
-			slog.Debug("copyFile: close destination failed", "dst", dst, "error", err)
-		}
-	}()
-
-	if _, err := io.Copy(out, in); err != nil {
-		return err
-	}
-	return out.Sync()
-}
-
 // Fix 5: isHigherVersion treats a non-parseable current version (e.g. "dev",
 // a git SHA, or any non-semver string) as "unknown / dev build" and returns
 // false without an error. This prevents the update check loop from spamming

internal/update/manager_test.go

@@ -112,20 +112,22 @@ func TestManagerDownloadReleaseAndPromote(t *testing.T) {
 		t.Fatalf("PromoteRelease() error = %v", err)
 	}
 
-	got, err := os.ReadFile(currentPath)
+	// New behavior: PromoteRelease stages to .new for systemd to swap, doesn't replace in-place
+	staged, err := os.ReadFile(currentPath + ".new")
 	if err != nil {
-		t.Fatalf("ReadFile(currentPath) error = %v", err)
+		t.Fatalf("ReadFile(staged) error = %v", err)
 	}
-	if len(got) == 0 {
-		t.Fatal("current binary is empty")
+	if len(staged) == 0 {
+		t.Fatal("staged binary is empty")
 	}
 
-	backup, err := os.ReadFile(currentPath + ".bak")
+	// Current binary should still be the old one
+	got, err := os.ReadFile(currentPath)
 	if err != nil {
-		t.Fatalf("ReadFile(backup) error = %v", err)
+		t.Fatalf("ReadFile(currentPath) error = %v", err)
 	}
-	if string(backup) != "old-binary" {
-		t.Fatalf("backup binary = %q, want %q", string(backup), "old-binary")
+	if string(got) != "old-binary" {
+		t.Fatalf("current binary = %q, want %q (should be unchanged until systemd swaps)", string(got), "old-binary")
 	}
 
 	if manager.CurrentVersion != "2.0.0" {

scripts/serve.sh

@@ -6,7 +6,7 @@ tmpdir="$(mktemp -d)"
 cd "$tmpdir"
 
 echo "ok" > healthz
-cp /usr/local/bin/hyperstack-agent download
+cp /opt/hyperstack-agent/bin/hyperstack-agent download
 
 sha256=$(sha256sum download | cut -d' ' -f1)
 digest="sha256:${sha256}"