diff --git a/Dockerfile b/Dockerfile index 92e82e0..f90b202 100644 --- a/Dockerfile +++ b/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 diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 8286143..c8e3be8 100644 --- a/cmd/agent/main.go +++ b/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") diff --git a/internal/update/manager.go b/internal/update/manager.go index 54e66ec..72bbd83 100644 --- a/internal/update/manager.go +++ b/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 diff --git a/internal/update/manager_test.go b/internal/update/manager_test.go index 6fbe994..405bd14 100644 --- a/internal/update/manager_test.go +++ b/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" { diff --git a/scripts/serve.sh b/scripts/serve.sh index d1de001..3e48bac 100755 --- a/scripts/serve.sh +++ b/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}"