From da421873b48341d6140d711a065da1b56449c7d0 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 21:52:33 +0000 Subject: [PATCH 01/20] agent: export secure binary upgrade installer --- pkg/agent/agentbinary/upgrade.go | 399 ++++++++++++++++++++++++++ pkg/agent/agentbinary/upgrade_test.go | 208 ++++++++++++++ pkg/agent/internal/utilio/io.go | 6 +- 3 files changed, 610 insertions(+), 3 deletions(-) create mode 100644 pkg/agent/agentbinary/upgrade.go create mode 100644 pkg/agent/agentbinary/upgrade_test.go diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go new file mode 100644 index 000000000..5f1b340f2 --- /dev/null +++ b/pkg/agent/agentbinary/upgrade.go @@ -0,0 +1,399 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package agentbinary + +import ( + "archive/tar" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/Azure/unbounded/pkg/agent/goalstates" + "github.com/Azure/unbounded/pkg/agent/internal/utilio" +) + +const ( + defaultMaxArchiveBytes = 256 << 20 + defaultMaxBinaryBytes = 256 << 20 +) + +// SecureInstallOptions configures a verified agent release archive install. +type SecureInstallOptions struct { + DownloadURL string + ExpectedSHA256 string + ExpectedMember string + Mode os.FileMode + MaxArchiveBytes int64 + MaxExtractedBytes int64 + HTTPClient *http.Client +} + +// SwitchResult describes a completed blue-green binary switch. +type SwitchResult struct { + PreviousPath string + CurrentPath string +} + +// SecureInstallAndSwitch downloads and verifies an HTTPS release archive, +// installs its only member into the inactive slot, and atomically updates the +// last-good and current links. +func SecureInstallAndSwitch( + ctx context.Context, + log *slog.Logger, + paths goalstates.AgentUpgradePaths, + opts SecureInstallOptions, +) (SwitchResult, error) { + if log == nil { + return SwitchResult{}, fmt.Errorf("logger is nil") + } + + parsedURL, err := validateSecureDownloadURL(opts.DownloadURL) + if err != nil { + return SwitchResult{}, err + } + + expectedDigest, err := parseSHA256(opts.ExpectedSHA256) + if err != nil { + return SwitchResult{}, err + } + + opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) + if opts.ExpectedMember == "" || filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { + return SwitchResult{}, fmt.Errorf("expected archive member must be a base name") + } + + if opts.Mode == 0 { + opts.Mode = daemonBinaryMode + } + + if opts.MaxArchiveBytes <= 0 { + opts.MaxArchiveBytes = defaultMaxArchiveBytes + } + + if opts.MaxExtractedBytes <= 0 { + opts.MaxExtractedBytes = defaultMaxBinaryBytes + } + + opts.HTTPClient = secureHTTPClient(opts.HTTPClient) + + previousPath, err := executablePath(paths.CurrentPath) + if err != nil { + return SwitchResult{}, fmt.Errorf("resolve current agent binary: %w", err) + } + + targetPath := paths.BluePath + if previousPath == paths.BluePath { + targetPath = paths.GreenPath + } + + archivePath, err := downloadVerifiedArchive(ctx, opts.HTTPClient, parsedURL, expectedDigest, opts.MaxArchiveBytes) + if err != nil { + return SwitchResult{}, err + } + defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup + + // The inactive slot may still be last-good. Protect the verified running + // binary before replacing that slot. + if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { + return SwitchResult{}, fmt.Errorf("protect current agent as last-good: %w", err) + } + + if err := extractOnlyArchiveMember(archivePath, targetPath, opts); err != nil { + return SwitchResult{}, err + } + + if err := verifyQuiet(ctx, targetPath); err != nil { + return SwitchResult{}, err + } + + if err := utilio.UpdateSymlink(paths.CurrentPath, targetPath); err != nil { + return SwitchResult{}, fmt.Errorf("update current agent symlink: %w", err) + } + + log.Info("staged upgraded agent binary", + "url", RedactedURL(parsedURL), + "previous", previousPath, + "current", targetPath, + ) + + return SwitchResult{PreviousPath: previousPath, CurrentPath: targetPath}, nil +} + +// RedactedURL removes query and fragment data that may contain credentials. +func RedactedURL(parsedURL *url.URL) string { + redacted := *parsedURL + redacted.RawQuery = "" + redacted.Fragment = "" + + return redacted.String() +} + +func validateSecureDownloadURL(rawURL string) (*url.URL, error) { + parsedURL, err := url.ParseRequestURI(strings.TrimSpace(rawURL)) + if err != nil { + return nil, fmt.Errorf("invalid download URL") + } + + if parsedURL.Scheme != "https" || parsedURL.Host == "" || parsedURL.User != nil || parsedURL.Fragment != "" { + return nil, fmt.Errorf("download URL must be an HTTPS URL without user information") + } + + return parsedURL, nil +} + +func parseSHA256(value string) ([sha256.Size]byte, error) { + var expected [sha256.Size]byte + + value = strings.TrimSpace(value) + value = strings.TrimPrefix(value, "sha256:") + + decoded, err := hex.DecodeString(value) + if err != nil || len(decoded) != sha256.Size { + return expected, fmt.Errorf("expected SHA-256 must be exactly 64 hexadecimal characters") + } + + copy(expected[:], decoded) + + return expected, nil +} + +func secureHTTPClient(base *http.Client) *http.Client { + if base == nil { + base = &http.Client{Timeout: 10 * time.Minute} + } + + client := *base + originalCheckRedirect := client.CheckRedirect + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if req.URL.Scheme != "https" { + return fmt.Errorf("redirect to non-HTTPS URL is not allowed") + } + + if originalCheckRedirect != nil { + return originalCheckRedirect(req, via) + } + + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + + return nil + } + + return &client +} + +func downloadVerifiedArchive( + ctx context.Context, + client *http.Client, + parsedURL *url.URL, + expected [sha256.Size]byte, + maxBytes int64, +) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, parsedURL.String(), http.NoBody) + if err != nil { + return "", fmt.Errorf("create agent archive request: %w", err) + } + + resp, err := client.Do(req) + if err != nil { + if ctx.Err() != nil { + return "", fmt.Errorf("download agent archive from %s: %w", RedactedURL(parsedURL), ctx.Err()) + } + // Redirect and transport errors can contain credential-bearing URLs. + return "", fmt.Errorf("download agent archive from %s failed", RedactedURL(parsedURL)) + } + defer resp.Body.Close() //nolint:errcheck // response body cleanup + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("download agent archive from %s: HTTP status %d", RedactedURL(parsedURL), resp.StatusCode) + } + + if resp.ContentLength > maxBytes { + return "", fmt.Errorf("agent archive exceeds %d-byte limit", maxBytes) + } + + temp, err := os.CreateTemp("", "agent-upgrade-*.tar.gz") + if err != nil { + return "", fmt.Errorf("create temporary agent archive: %w", err) + } + + path := temp.Name() + ok := false + + defer func() { + temp.Close() //nolint:errcheck // best effort cleanup after an earlier failure + + if !ok { + os.Remove(path) //nolint:errcheck // best effort temporary file cleanup + } + }() + + hasher := sha256.New() + + n, err := io.Copy(io.MultiWriter(temp, hasher), io.LimitReader(resp.Body, maxBytes+1)) + if err != nil { + return "", fmt.Errorf("read agent archive: %w", err) + } + + if n > maxBytes { + return "", fmt.Errorf("agent archive exceeds %d-byte limit", maxBytes) + } + + if !equalDigest(hasher.Sum(nil), expected[:]) { + return "", fmt.Errorf("agent archive SHA-256 does not match expected digest") + } + + if err := temp.Close(); err != nil { + return "", fmt.Errorf("close temporary agent archive: %w", err) + } + + ok = true + + return path, nil +} + +func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstallOptions) (err error) { + archive, err := os.Open(archivePath) + if err != nil { + return fmt.Errorf("open agent archive: %w", err) + } + + defer func() { + if closeErr := archive.Close(); closeErr != nil && err == nil { + err = closeErr + } + }() + + gz, err := gzip.NewReader(archive) + if err != nil { + return fmt.Errorf("decompress agent archive: %w", err) + } + defer gz.Close() //nolint:errcheck // extraction reports read errors + + found := false + decompressed := &countingReader{reader: io.LimitReader(gz, 2*opts.MaxExtractedBytes+1)} + + tarReader := tar.NewReader(decompressed) + for { + header, nextErr := tarReader.Next() + if errors.Is(nextErr, io.EOF) { + break + } + + if nextErr != nil { + return fmt.Errorf("read agent archive: %w", nextErr) + } + + if !safeArchiveName(header.Name) { + return fmt.Errorf("agent archive contains unsafe member name %q", header.Name) + } + + if header.Name != opts.ExpectedMember { + return fmt.Errorf("agent archive contains unexpected member %q", header.Name) + } + + if found { + return fmt.Errorf("agent archive contains duplicate member %q", opts.ExpectedMember) + } + + if header.Typeflag != tar.TypeReg || header.Size <= 0 || header.Size > opts.MaxExtractedBytes { + return fmt.Errorf("agent archive member %q is not a valid bounded regular file", opts.ExpectedMember) + } + + if err := utilio.InstallFileWithLimitedSize(targetPath, tarReader, opts.Mode, opts.MaxExtractedBytes); err != nil { + return fmt.Errorf("install upgraded agent binary: %w", err) + } + + found = true + } + + if decompressed.count > 2*opts.MaxExtractedBytes { + return fmt.Errorf("decompressed agent archive exceeds %d-byte limit", 2*opts.MaxExtractedBytes) + } + + if !found { + return fmt.Errorf("agent archive does not contain expected member %q", opts.ExpectedMember) + } + + return nil +} + +func safeArchiveName(name string) bool { + return name != "" && + !filepath.IsAbs(name) && + filepath.Clean(name) == name && + !strings.Contains(name, `\`) && + !strings.HasPrefix(name, ".."+string(filepath.Separator)) +} + +func executablePath(path string) (string, error) { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", err + } + + info, err := os.Stat(resolved) + if err != nil { + return "", err + } + + if !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 { + return "", fmt.Errorf("%s is not a regular executable file", path) + } + + return resolved, nil +} + +func verifyQuiet(ctx context.Context, path string) error { + verifyCtx, cancel := context.WithTimeout(ctx, verifyTimeout) + defer cancel() + + cmd := exec.CommandContext(verifyCtx, path, "version") + cmd.Stdout = io.Discard + + cmd.Stderr = io.Discard + if err := cmd.Run(); err != nil { + return fmt.Errorf("verify upgraded agent binary: %w", err) + } + + return nil +} + +type countingReader struct { + reader io.Reader + count int64 +} + +func (r *countingReader) Read(data []byte) (int, error) { + n, err := r.reader.Read(data) + r.count += int64(n) + + return n, err +} + +func equalDigest(actual, expected []byte) bool { + if len(actual) != len(expected) { + return false + } + + var different byte + for i := range actual { + different |= actual[i] ^ expected[i] + } + + return different == 0 +} diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go new file mode 100644 index 000000000..024f93904 --- /dev/null +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -0,0 +1,208 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package agentbinary + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "crypto/sha256" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Azure/unbounded/pkg/agent/goalstates" +) + +func TestSecureInstallAndSwitch(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + result, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz?sig=secret", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + Mode: 0o755, + MaxArchiveBytes: 1 << 20, + MaxExtractedBytes: 1 << 20, + HTTPClient: server.Client(), + }) + if err != nil { + t.Fatalf("SecureInstallAndSwitch: %v", err) + } + + if result.PreviousPath != paths.BluePath || result.CurrentPath != paths.GreenPath { + t.Fatalf("result = %#v", result) + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) +} + +func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + tests := map[string]SecureInstallOptions{ + "HTTP URL": { + DownloadURL: "http://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + }, + "invalid digest": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: "bad", + ExpectedMember: "custom-agent", + }, + "nested member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "bin/custom-agent", + }, + } + for name, opts := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + if _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + }) + } +} + +func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + payload := secureUpgradeArchive(t, "other-agent", []byte("binary")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + HTTPClient: server.Client(), + }) + if err == nil || !strings.Contains(err.Error(), "unexpected member") { + t.Fatalf("error = %v", err) + } +} + +func TestRedactedURL(t *testing.T) { + t.Parallel() + + parsed, err := url.Parse("https://example.com/agent.tar.gz?sig=secret") + if err != nil { + t.Fatalf("parse URL: %v", err) + } + + if got := RedactedURL(parsed); got != "https://example.com/agent.tar.gz" { + t.Fatalf("RedactedURL = %q", got) + } +} + +func secureUpgradeTestPaths(t *testing.T) goalstates.AgentUpgradePaths { + t.Helper() + dir := t.TempDir() + paths := goalstates.AgentUpgradePaths{ + BinaryPath: filepath.Join(dir, "agent"), + BluePath: filepath.Join(dir, "agent-blue"), + GreenPath: filepath.Join(dir, "agent-green"), + CurrentPath: filepath.Join(dir, "agent-current"), + LastGoodPath: filepath.Join(dir, "agent-last-good"), + SignalPath: filepath.Join(dir, "agent-signal"), + CurrentTargetPath: filepath.Join(dir, "agent-blue"), + } + + return paths +} + +func secureUpgradeArchive(t *testing.T, name string, body []byte) []byte { + t.Helper() + + var archive bytes.Buffer + + gz := gzip.NewWriter(&archive) + + tarWriter := tar.NewWriter(gz) + if err := tarWriter.WriteHeader(&tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatalf("write tar header: %v", err) + } + + if _, err := io.Copy(tarWriter, bytes.NewReader(body)); err != nil { + t.Fatalf("write tar body: %v", err) + } + + if err := tarWriter.Close(); err != nil { + t.Fatalf("close tar: %v", err) + } + + if err := gz.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + + return archive.Bytes() +} + +func assertSecureUpgradeLink(t *testing.T, path, want string) { + t.Helper() + + got, err := filepath.EvalSymlinks(path) + if err != nil { + t.Fatalf("resolve %s: %v", path, err) + } + + if got != want { + t.Fatalf("resolved %s = %s, want %s", path, got, want) + } +} diff --git a/pkg/agent/internal/utilio/io.go b/pkg/agent/internal/utilio/io.go index 5573bcefb..ed107dde6 100644 --- a/pkg/agent/internal/utilio/io.go +++ b/pkg/agent/internal/utilio/io.go @@ -22,14 +22,14 @@ var errFileTooLarge = errors.New("file exceeds maximum allowed size") // NOTE: we assume the filename is trusted and cleaned without path traversal characters. func InstallFile(filename string, r io.Reader, perm os.FileMode) error { const maxFileSize = 1 * 1024 * 1024 * 1024 // 1 GiB - return installFileWithLimitedSize(filename, r, perm, maxFileSize) + return InstallFileWithLimitedSize(filename, r, perm, maxFileSize) } -// installFileWithLimitedSize streams content to local file with limited size and specified permissions. +// InstallFileWithLimitedSize streams content to a local file with limited size and specified permissions. // It ensures that the target directory exists and handles the file writing atomically. // // NOTE: we assume the filename is trusted and cleaned without path traversal characters. -func installFileWithLimitedSize(filename string, r io.Reader, perm os.FileMode, maxBytes int64) error { +func InstallFileWithLimitedSize(filename string, r io.Reader, perm os.FileMode, maxBytes int64) error { if maxBytes <= 0 { return fmt.Errorf("invalid maxBytes: %d", maxBytes) } From 5e0aab6faf4b3e49718a2d9b1585f0a1b2ac6cc5 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 21:53:26 +0000 Subject: [PATCH 02/20] agent: expose secure install validation --- pkg/agent/agentbinary/upgrade.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index 5f1b340f2..f03923d52 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -47,6 +47,24 @@ type SwitchResult struct { CurrentPath string } +// ValidateSecureInstallOptions validates caller-provided secure install inputs. +func ValidateSecureInstallOptions(opts SecureInstallOptions) error { + if _, err := validateSecureDownloadURL(opts.DownloadURL); err != nil { + return err + } + + if _, err := parseSHA256(opts.ExpectedSHA256); err != nil { + return err + } + + expectedMember := strings.TrimSpace(opts.ExpectedMember) + if expectedMember == "" || filepath.Base(expectedMember) != expectedMember { + return fmt.Errorf("expected archive member must be a base name") + } + + return nil +} + // SecureInstallAndSwitch downloads and verifies an HTTPS release archive, // installs its only member into the inactive slot, and atomically updates the // last-good and current links. @@ -60,6 +78,10 @@ func SecureInstallAndSwitch( return SwitchResult{}, fmt.Errorf("logger is nil") } + if err := ValidateSecureInstallOptions(opts); err != nil { + return SwitchResult{}, err + } + parsedURL, err := validateSecureDownloadURL(opts.DownloadURL) if err != nil { return SwitchResult{}, err @@ -71,9 +93,6 @@ func SecureInstallAndSwitch( } opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) - if opts.ExpectedMember == "" || filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { - return SwitchResult{}, fmt.Errorf("expected archive member must be a base name") - } if opts.Mode == 0 { opts.Mode = daemonBinaryMode From 8173c7ee7a8c96190eea8fa9776a239c4e7b5a7c Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 22:07:13 +0000 Subject: [PATCH 03/20] agent: harden reusable upgrade API --- pkg/agent/agentbinary/agentbinary.go | 8 +- pkg/agent/agentbinary/upgrade.go | 55 +++++- pkg/agent/agentbinary/upgrade_test.go | 242 ++++++++++++++++++++++++-- 3 files changed, 284 insertions(+), 21 deletions(-) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index b7b26e985..b2834e811 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// Package agentbinary installs unbounded-agent binaries from release archives. +// Package agentbinary installs and switches verified agent binaries from release archives. package agentbinary import ( @@ -26,7 +26,9 @@ const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 // InstallFromTarGz downloads a remote .tar.gz archive and installs binaryName -// from it to targetPath. +// from it to targetPath. It is retained for the legacy Unbounded upgrade +// contract, which does not provide an archive digest. New managed upgrade +// callers should use SecureInstallAndSwitch. func InstallFromTarGz(ctx context.Context, downloadURL, targetPath, binaryName string, perm os.FileMode) error { parsedURL, err := url.Parse(downloadURL) if err != nil { @@ -85,6 +87,8 @@ func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error } // InstallAndSwitchFromTarGz installs the next agent binary and switches daemon links. +// It is retained for the legacy Unbounded upgrade contract. New managed upgrade +// callers should use SecureInstallAndSwitch. func InstallAndSwitchFromTarGz(ctx context.Context, downloadURL string, paths goalstates.AgentUpgradePaths, perm os.FileMode) error { targetPath := paths.NextTargetPath() if err := InstallFromTarGz(ctx, downloadURL, targetPath, goalstates.AgentUpgradeBinaryName, perm); err != nil { diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index f03923d52..6fbf41098 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -13,6 +13,7 @@ import ( "fmt" "io" "log/slog" + "math" "net/http" "net/url" "os" @@ -21,7 +22,6 @@ import ( "strings" "time" - "github.com/Azure/unbounded/pkg/agent/goalstates" "github.com/Azure/unbounded/pkg/agent/internal/utilio" ) @@ -30,6 +30,15 @@ const ( defaultMaxBinaryBytes = 256 << 20 ) +// Layout describes caller-owned blue-green agent binary paths. +type Layout struct { + BinaryPath string + BluePath string + GreenPath string + CurrentPath string + LastGoodPath string +} + // SecureInstallOptions configures a verified agent release archive install. type SecureInstallOptions struct { DownloadURL string @@ -62,6 +71,44 @@ func ValidateSecureInstallOptions(opts SecureInstallOptions) error { return fmt.Errorf("expected archive member must be a base name") } + if opts.Mode != 0 && opts.Mode.Perm() != opts.Mode { + return fmt.Errorf("agent binary mode must contain permission bits only") + } + + if opts.MaxArchiveBytes < 0 || opts.MaxExtractedBytes < 0 { + return fmt.Errorf("agent archive size limits must not be negative") + } + + if opts.MaxExtractedBytes > math.MaxInt64/2 { + return fmt.Errorf("maximum extracted size is too large") + } + + return nil +} + +// ValidateLayout verifies that all binary paths are clean, absolute, and distinct. +func ValidateLayout(paths Layout) error { + values := []string{ + paths.BinaryPath, + paths.BluePath, + paths.GreenPath, + paths.CurrentPath, + paths.LastGoodPath, + } + + seen := make(map[string]struct{}, len(values)) + for _, value := range values { + if value == "" || !filepath.IsAbs(value) || filepath.Clean(value) != value { + return fmt.Errorf("invalid agent binary path %q", value) + } + + if _, ok := seen[value]; ok { + return fmt.Errorf("duplicate agent binary path %q", value) + } + + seen[value] = struct{}{} + } + return nil } @@ -71,13 +118,17 @@ func ValidateSecureInstallOptions(opts SecureInstallOptions) error { func SecureInstallAndSwitch( ctx context.Context, log *slog.Logger, - paths goalstates.AgentUpgradePaths, + paths Layout, opts SecureInstallOptions, ) (SwitchResult, error) { if log == nil { return SwitchResult{}, fmt.Errorf("logger is nil") } + if err := ValidateLayout(paths); err != nil { + return SwitchResult{}, err + } + if err := ValidateSecureInstallOptions(opts); err != nil { return SwitchResult{}, err } diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index 024f93904..9691a7855 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -18,8 +18,6 @@ import ( "path/filepath" "strings" "testing" - - "github.com/Azure/unbounded/pkg/agent/goalstates" ) func TestSecureInstallAndSwitch(t *testing.T) { @@ -95,6 +93,18 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "bin/custom-agent", }, + "invalid mode": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + Mode: os.ModeSetuid | 0o755, + }, + "negative size": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + MaxArchiveBytes: -1, + }, } for name, opts := range tests { t.Run(name, func(t *testing.T) { @@ -107,6 +117,20 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { } } +func TestValidateLayout(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := ValidateLayout(paths); err != nil { + t.Fatalf("ValidateLayout: %v", err) + } + + paths.LastGoodPath = paths.CurrentPath + if err := ValidateLayout(paths); err == nil { + t.Fatal("ValidateLayout duplicate path error = nil") + } +} + func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { t.Parallel() @@ -138,6 +162,160 @@ func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { } } +func TestSecureInstallAndSwitchPreservesCurrentOnVerificationFailures(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + binary []byte + digest string + }{ + "digest mismatch": { + binary: []byte("#!/bin/sh\nexit 0\n"), + digest: strings.Repeat("0", 64), + }, + "candidate version failure": { + binary: []byte("#!/bin/sh\nexit 42\n"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchive(t, "custom-agent", tt.binary) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := tt.digest + if digest == "" { + sum := sha256.Sum256(payload) + digest = fmt.Sprintf("%x", sum) + } + + _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz?sig=secret", + ExpectedSHA256: digest, + ExpectedMember: "custom-agent", + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + + if strings.Contains(err.Error(), "secret") { + t.Fatalf("error leaked URL query: %v", err) + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) + }) + } +} + +func TestSecureInstallAndSwitchEnforcesSizeLimits(t *testing.T) { + t.Parallel() + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) + digest := sha256.Sum256(payload) + + tests := map[string]SecureInstallOptions{ + "compressed": { + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + MaxArchiveBytes: int64(len(payload) - 1), + MaxExtractedBytes: 1 << 20, + }, + "extracted": { + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + MaxArchiveBytes: 1 << 20, + MaxExtractedBytes: 4, + }, + } + for name, opts := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + opts.DownloadURL = server.URL + "/agent.tar.gz" + + opts.HTTPClient = server.Client() + if _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + }) + } +} + +func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { + t.Parallel() + + tests := map[string][]secureTarMember{ + "unsafe": {{name: "../custom-agent", body: []byte("binary")}}, + "duplicate": { + {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, + {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, + }, + } + for name, members := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchiveWithMembers(t, members) + digest := sha256.Sum256(payload) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + }) + } +} + +func TestSecureInstallAndSwitchRejectsHTTPRedirect(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + insecure := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("not reached")) + })) + t.Cleanup(insecure.Close) + + secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + http.Redirect(w, request, insecure.URL, http.StatusFound) + })) + t.Cleanup(secure.Close) + + _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: secure.URL + "/agent.tar.gz", + ExpectedSHA256: strings.Repeat("0", 64), + ExpectedMember: "custom-agent", + HTTPClient: secure.Client(), + }) + if err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) +} + func TestRedactedURL(t *testing.T) { t.Parallel() @@ -151,36 +329,66 @@ func TestRedactedURL(t *testing.T) { } } -func secureUpgradeTestPaths(t *testing.T) goalstates.AgentUpgradePaths { +func secureUpgradeTestPaths(t *testing.T) Layout { t.Helper() dir := t.TempDir() - paths := goalstates.AgentUpgradePaths{ - BinaryPath: filepath.Join(dir, "agent"), - BluePath: filepath.Join(dir, "agent-blue"), - GreenPath: filepath.Join(dir, "agent-green"), - CurrentPath: filepath.Join(dir, "agent-current"), - LastGoodPath: filepath.Join(dir, "agent-last-good"), - SignalPath: filepath.Join(dir, "agent-signal"), - CurrentTargetPath: filepath.Join(dir, "agent-blue"), + paths := Layout{ + BinaryPath: filepath.Join(dir, "agent"), + BluePath: filepath.Join(dir, "agent-blue"), + GreenPath: filepath.Join(dir, "agent-green"), + CurrentPath: filepath.Join(dir, "agent-current"), + LastGoodPath: filepath.Join(dir, "agent-last-good"), } return paths } +func secureUpgradeReadyPaths(t *testing.T) Layout { + t.Helper() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good: %v", err) + } + + return paths +} + +type secureTarMember struct { + name string + body []byte +} + func secureUpgradeArchive(t *testing.T, name string, body []byte) []byte { t.Helper() + return secureUpgradeArchiveWithMembers(t, []secureTarMember{{name: name, body: body}}) +} + +func secureUpgradeArchiveWithMembers(t *testing.T, members []secureTarMember) []byte { + t.Helper() + var archive bytes.Buffer gz := gzip.NewWriter(&archive) tarWriter := tar.NewWriter(gz) - if err := tarWriter.WriteHeader(&tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg}); err != nil { - t.Fatalf("write tar header: %v", err) - } - - if _, err := io.Copy(tarWriter, bytes.NewReader(body)); err != nil { - t.Fatalf("write tar body: %v", err) + for _, member := range members { + if err := tarWriter.WriteHeader(&tar.Header{Name: member.name, Mode: 0o755, Size: int64(len(member.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatalf("write tar header: %v", err) + } + + if _, err := io.Copy(tarWriter, bytes.NewReader(member.body)); err != nil { + t.Fatalf("write tar body: %v", err) + } } if err := tarWriter.Close(); err != nil { From 72a217985a700c1f3043dc84d5493c95f360cb5c Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 22:36:31 +0000 Subject: [PATCH 04/20] agent: use secure installer for managed upgrades --- cmd/agent/internal/daemon/agentupgrade.go | 56 +++++++++++++++++-- .../internal/daemon/agentupgrade_test.go | 16 ++++-- .../daemon/controller_machineoperation.go | 6 +- cmd/agent/internal/daemon/controller_test.go | 12 +++- cmd/agent/internal/daemon/nodeoperator.go | 6 +- .../app/machine_operation_aliases.go | 13 ++++- .../app/machine_operation_create.go | 10 +++- .../app/machine_operation_e2e_test.go | 4 ++ .../app/machine_operation_test.go | 27 ++++++++- designs/agent-upgrade.md | 27 +++++---- .../guides/operations/agent-operations.md | 9 +-- docs/content/guides/operations/automation.md | 2 + docs/content/reference/cli.md | 7 ++- docs/content/reference/machina-crd.md | 2 +- 14 files changed, 154 insertions(+), 43 deletions(-) diff --git a/cmd/agent/internal/daemon/agentupgrade.go b/cmd/agent/internal/daemon/agentupgrade.go index bc011d389..40ab15e07 100644 --- a/cmd/agent/internal/daemon/agentupgrade.go +++ b/cmd/agent/internal/daemon/agentupgrade.go @@ -18,9 +18,15 @@ import ( const ( agentUpgradeDownloadURLParameter = "downloadURL" + agentUpgradeSHA256Parameter = "sha256" agentUpgradeBinaryMode = 0o755 ) +type agentUpgradeRequest struct { + downloadURL string + sha256 string +} + // agentUpgradeSignal is the JSON payload for pending and failure signals. type agentUpgradeSignal struct { OperationName string `json:"operationName"` @@ -42,15 +48,32 @@ type fileAgentUpgradeSignalOperator struct { path string } -func agentUpgradeDownloadURL(parameters map[string]string) (string, error) { - downloadURL := strings.TrimSpace(parameters[agentUpgradeDownloadURLParameter]) - if downloadURL == "" { - return "", fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) +func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest, error) { + request := agentUpgradeRequest{ + downloadURL: strings.TrimSpace(parameters[agentUpgradeDownloadURLParameter]), + sha256: strings.TrimSpace(parameters[agentUpgradeSHA256Parameter]), + } + if request.downloadURL == "" { + return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) + } + + if request.sha256 == "" { + return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeSHA256Parameter) } - return downloadURL, nil + if err := agentbinary.ValidateSecureInstallOptions(agentbinary.SecureInstallOptions{ + DownloadURL: request.downloadURL, + ExpectedSHA256: request.sha256, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + }); err != nil { + return agentUpgradeRequest{}, err + } + + return request, nil } +// upgradeDaemonBinary retains the legacy Unbounded download contract for +// compatibility tests. Managed MachineOperations use upgradeDaemonBinarySecure. func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, downloadURL string) error { paths, err := goalstates.ResolvedAgentUpgradePaths() if err != nil { @@ -71,6 +94,29 @@ func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, downloadURL stri return nil } +func upgradeDaemonBinarySecure(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { + paths, err := goalstates.ResolvedAgentUpgradePaths() + if err != nil { + return fmt.Errorf("resolve current daemon binary symlink: %w", err) + } + + layout := agentbinary.Layout{ + BinaryPath: paths.BinaryPath, + BluePath: paths.BluePath, + GreenPath: paths.GreenPath, + CurrentPath: paths.CurrentPath, + LastGoodPath: paths.LastGoodPath, + } + _, err = agentbinary.SecureInstallAndSwitch(ctx, log, layout, agentbinary.SecureInstallOptions{ + DownloadURL: request.downloadURL, + ExpectedSHA256: request.sha256, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + Mode: agentUpgradeBinaryMode, + }) + + return err +} + func newAgentUpgradeSignalOperator() (agentUpgradeSignalOperator, error) { paths, err := goalstates.ResolvedAgentUpgradePaths() if err != nil { diff --git a/cmd/agent/internal/daemon/agentupgrade_test.go b/cmd/agent/internal/daemon/agentupgrade_test.go index db2656877..f84a21d8b 100644 --- a/cmd/agent/internal/daemon/agentupgrade_test.go +++ b/cmd/agent/internal/daemon/agentupgrade_test.go @@ -25,18 +25,26 @@ import ( "github.com/Azure/unbounded/pkg/agent/goalstates" ) -func TestAgentUpgradeDownloadURL(t *testing.T) { +func TestParseAgentUpgradeRequest(t *testing.T) { t.Parallel() - downloadURL, err := agentUpgradeDownloadURL(map[string]string{ + request, err := parseAgentUpgradeRequest(map[string]string{ agentUpgradeDownloadURLParameter: " https://example.com/agent.tar.gz ", + agentUpgradeSHA256Parameter: " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ", }) require.NoError(t, err) - assert.Equal(t, "https://example.com/agent.tar.gz", downloadURL) + assert.Equal(t, "https://example.com/agent.tar.gz", request.downloadURL) + assert.Equal(t, testAgentUpgradeSHA256, request.sha256) - _, err = agentUpgradeDownloadURL(nil) + _, err = parseAgentUpgradeRequest(nil) require.Error(t, err) assert.Contains(t, err.Error(), agentUpgradeDownloadURLParameter) + + _, err = parseAgentUpgradeRequest(map[string]string{ + agentUpgradeDownloadURLParameter: "https://example.com/agent.tar.gz", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), agentUpgradeSHA256Parameter) } func TestAgentUpgradeSignalOperator_RecordFailure(t *testing.T) { diff --git a/cmd/agent/internal/daemon/controller_machineoperation.go b/cmd/agent/internal/daemon/controller_machineoperation.go index ed2f631da..1c826dd47 100644 --- a/cmd/agent/internal/daemon/controller_machineoperation.go +++ b/cmd/agent/internal/daemon/controller_machineoperation.go @@ -59,14 +59,14 @@ func (t *machineOperationTarget) reconcileAgentUpgrade(ctx context.Context, stor return ctrl.Result{}, err } - downloadURL, err := agentUpgradeDownloadURL(op.Parameters) + request, err := parseAgentUpgradeRequest(op.Parameters) if err != nil { return ctrl.Result{}, store.Finish(ctx, op, daemon.MachineOperationResult[int64]{Phase: v1alpha3.OperationPhaseFailed, Reason: "InvalidParameters", Message: err.Error()}) } - t.log.Info("staging AgentUpgrade binary", "operation", op.Name, "url", downloadURL) + t.log.Info("staging AgentUpgrade binary", "operation", op.Name) - if err := t.nodeOperator.StageAgentUpgrade(ctx, t.log, downloadURL); err != nil { + if err := t.nodeOperator.StageAgentUpgrade(ctx, t.log, request); err != nil { return finishFailedMachineOperation(ctx, store, op, err) } diff --git a/cmd/agent/internal/daemon/controller_test.go b/cmd/agent/internal/daemon/controller_test.go index 0a4c60cab..694264817 100644 --- a/cmd/agent/internal/daemon/controller_test.go +++ b/cmd/agent/internal/daemon/controller_test.go @@ -26,6 +26,8 @@ import ( "github.com/Azure/unbounded/pkg/agent/goalstates" ) +const testAgentUpgradeSHA256 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + type fakeNodeOperator struct { active *ActiveMachine findErr error @@ -46,6 +48,7 @@ type fakeNodeOperator struct { stageUpgradeCalled bool stageUpgradeURL string + stageUpgradeSHA256 string stageUpgradeErr error restartAgentCalled bool @@ -91,9 +94,10 @@ func (op *fakeNodeOperator) RepaveNode( return op.repaveErr } -func (op *fakeNodeOperator) StageAgentUpgrade(_ context.Context, _ *slog.Logger, downloadURL string) error { +func (op *fakeNodeOperator) StageAgentUpgrade(_ context.Context, _ *slog.Logger, request agentUpgradeRequest) error { op.stageUpgradeCalled = true - op.stageUpgradeURL = downloadURL + op.stageUpgradeURL = request.downloadURL + op.stageUpgradeSHA256 = request.sha256 return op.stageUpgradeErr } @@ -238,6 +242,7 @@ func TestReconcileAgentUpgrade_Complete(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } @@ -250,6 +255,7 @@ func TestReconcileAgentUpgrade_Complete(t *testing.T) { require.NoError(t, err) assert.True(t, op.stageUpgradeCalled) assert.Equal(t, "https://example.com/unbounded-agent.tar.gz", op.stageUpgradeURL) + assert.Equal(t, testAgentUpgradeSHA256, op.stageUpgradeSHA256) assert.True(t, op.restartAgentCalled) var updated v1alpha3.MachineOperation @@ -307,6 +313,7 @@ func TestReconcileAgentUpgrade_Failed(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } @@ -336,6 +343,7 @@ func TestReconcileAgentUpgrade_RestartFailureFailsOperation(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } diff --git a/cmd/agent/internal/daemon/nodeoperator.go b/cmd/agent/internal/daemon/nodeoperator.go index 165b58afd..ec1602223 100644 --- a/cmd/agent/internal/daemon/nodeoperator.go +++ b/cmd/agent/internal/daemon/nodeoperator.go @@ -56,7 +56,7 @@ type nodeOperator interface { // not perform Kubernetes eviction or CNI-specific dataplane cleanup itself. RepaveNode(context.Context, *slog.Logger, *ActiveMachine, *provision.UnboundedAgentConfig) error // StageAgentUpgrade stages a new host-side agent binary. - StageAgentUpgrade(context.Context, *slog.Logger, string) error + StageAgentUpgrade(context.Context, *slog.Logger, agentUpgradeRequest) error // RestartAgentDaemon restarts the host-side agent daemon after an upgrade // operation has been recorded as complete. RestartAgentDaemon(context.Context, *slog.Logger) error @@ -252,8 +252,8 @@ func (nspawnNodeOperator) RepaveNode( return nil } -func (nspawnNodeOperator) StageAgentUpgrade(ctx context.Context, log *slog.Logger, downloadURL string) error { - return upgradeDaemonBinary(ctx, log, downloadURL) +func (nspawnNodeOperator) StageAgentUpgrade(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { + return upgradeDaemonBinarySecure(ctx, log, request) } func (nspawnNodeOperator) RestartAgentDaemon(ctx context.Context, log *slog.Logger) error { diff --git a/cmd/kubectl-unbounded/app/machine_operation_aliases.go b/cmd/kubectl-unbounded/app/machine_operation_aliases.go index 9fed2402b..66780a889 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_aliases.go +++ b/cmd/kubectl-unbounded/app/machine_operation_aliases.go @@ -71,7 +71,10 @@ func newMachinePowerOnCommand(rt *machineCommandRuntime) *cobra.Command { } func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { - var downloadURL string + var ( + downloadURL string + sha256Digest string + ) cmd := newMachineOperationAliasCommand( rt, @@ -81,7 +84,8 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { "agent-upgrade", ) - cmd.Flags().StringVar(&downloadURL, "download-url", "", "URL of the unbounded-agent release tarball") + cmd.Flags().StringVar(&downloadURL, "download-url", "", "HTTPS URL of the unbounded-agent release tarball") + cmd.Flags().StringVar(&sha256Digest, "sha256", "", "SHA-256 digest of the release tarball") oldRunE := cmd.RunE cmd.RunE = func(cmd *cobra.Command, args []string) error { @@ -89,8 +93,13 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { return fmt.Errorf("--download-url is required") } + if sha256Digest == "" { + return fmt.Errorf("--sha256 is required") + } + cmd.SetContext(context.WithValue(cmd.Context(), machineOperationParametersKey{}, map[string]string{ "downloadURL": downloadURL, + "sha256": sha256Digest, })) return oldRunE(cmd, args) diff --git a/cmd/kubectl-unbounded/app/machine_operation_create.go b/cmd/kubectl-unbounded/app/machine_operation_create.go index e3be20d0c..9a73020f0 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_create.go +++ b/cmd/kubectl-unbounded/app/machine_operation_create.go @@ -226,8 +226,14 @@ func (o *machineOperationCreateOptions) validate() error { return err } - if o.kind == v1alpha3.OperationAgentUpgrade && parameters["downloadURL"] == "" { - return fmt.Errorf("AgentUpgrade requires --param downloadURL=") + if o.kind == v1alpha3.OperationAgentUpgrade { + if parameters["downloadURL"] == "" { + return fmt.Errorf("AgentUpgrade requires --param downloadURL=") + } + + if parameters["sha256"] == "" { + return fmt.Errorf("AgentUpgrade requires --param sha256=") + } } return nil diff --git a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go index db8039c88..df5eca36f 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go @@ -48,6 +48,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { "--kind", string(v1alpha3.OperationAgentUpgrade), "--machine", "worker-01", "--param", "downloadURL=https://example.com/agent.tar.gz", + "--param", "sha256="+testAgentUpgradeSHA256, "--ttl", "900", ) require.NoError(t, err) @@ -57,6 +58,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { require.Nil(t, op.Spec.MachineSelector) require.Equal(t, v1alpha3.OperationAgentUpgrade, op.Spec.OperationKind) require.Equal(t, "https://example.com/agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) require.NotNil(t, op.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(900), *op.Spec.TTLSecondsAfterFinished) }) @@ -103,6 +105,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { "machine", "agent-upgrade", "worker-01", "--operation-name", "worker-01-agent-upgrade", "--download-url", "https://example.com/new-agent.tar.gz", + "--sha256", testAgentUpgradeSHA256, "--wait=false", ) require.NoError(t, err) @@ -111,6 +114,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { require.Equal(t, "worker-01", op.Spec.MachineRef) require.Equal(t, v1alpha3.OperationAgentUpgrade, op.Spec.OperationKind) require.Equal(t, "https://example.com/new-agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) require.NotNil(t, op.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(defaultTTLSeconds), *op.Spec.TTLSecondsAfterFinished) require.Len(t, op.OwnerReferences, 1) diff --git a/cmd/kubectl-unbounded/app/machine_operation_test.go b/cmd/kubectl-unbounded/app/machine_operation_test.go index 87325d2c4..9ca1ad604 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_test.go @@ -23,6 +23,8 @@ import ( v1alpha3 "github.com/Azure/unbounded/api/machina/v1alpha3" ) +const testAgentUpgradeSHA256 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + func TestBuildMachineOperationWithMachineRef(t *testing.T) { t.Parallel() @@ -76,7 +78,7 @@ func TestBuildMachineOperationParameters(t *testing.T) { name: "upgrade-worker-01", kind: v1alpha3.OperationAgentUpgrade, machine: "worker-01", - parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz", "sha256=" + testAgentUpgradeSHA256}, output: operationOutputName, dryRun: dryRunNone, } @@ -86,6 +88,7 @@ func TestBuildMachineOperationParameters(t *testing.T) { op, err := opts.build() require.NoError(t, err) require.Equal(t, "https://example.com/agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) } func TestValidateMachineOperationRequiresTarget(t *testing.T) { @@ -119,6 +122,23 @@ func TestValidateAgentUpgradeRequiresDownloadURL(t *testing.T) { require.Contains(t, err.Error(), "downloadURL") } +func TestValidateAgentUpgradeRequiresSHA256(t *testing.T) { + t.Parallel() + + opts := &machineOperationCreateOptions{ + name: "upgrade-worker-01", + kind: v1alpha3.OperationAgentUpgrade, + machine: "worker-01", + parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + output: operationOutputName, + dryRun: dryRunNone, + } + + err := opts.validate() + require.Error(t, err) + require.Contains(t, err.Error(), "sha256") +} + func TestValidateWaitRejectsStructuredOutput(t *testing.T) { t.Parallel() @@ -173,6 +193,7 @@ func TestMachineOperationCreateCommandDryRunYAML(t *testing.T) { "--kind", string(v1alpha3.OperationAgentUpgrade), "--machine", "worker-01", "--param", "downloadURL=https://example.com/agent.tar.gz", + "--param", "sha256="+testAgentUpgradeSHA256, "--ttl", "900", "--dry-run=client", "-o", "yaml", @@ -183,6 +204,7 @@ func TestMachineOperationCreateCommandDryRunYAML(t *testing.T) { require.Contains(t, out, "operationKind: AgentUpgrade") require.Contains(t, out, "machineRef: worker-01") require.Contains(t, out, "downloadURL: https://example.com/agent.tar.gz") + require.Contains(t, out, "sha256: "+testAgentUpgradeSHA256) require.Contains(t, out, "ttlSecondsAfterFinished: 900") } @@ -214,7 +236,7 @@ func TestMachineOperationCreateSmokeCreatesMachineRefOperation(t *testing.T) { name: "upgrade-worker-01", kind: v1alpha3.OperationAgentUpgrade, machine: "worker-01", - parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz", "sha256=" + testAgentUpgradeSHA256}, ttlSeconds: 900, output: operationOutputName, dryRun: dryRunNone, @@ -233,6 +255,7 @@ func TestMachineOperationCreateSmokeCreatesMachineRefOperation(t *testing.T) { require.Nil(t, got.Spec.MachineSelector) require.Equal(t, v1alpha3.OperationAgentUpgrade, got.Spec.OperationKind) require.Equal(t, "https://example.com/agent.tar.gz", got.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, got.Spec.Parameters["sha256"]) require.NotNil(t, got.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(900), *got.Spec.TTLSecondsAfterFinished) } diff --git a/designs/agent-upgrade.md b/designs/agent-upgrade.md index 3dd2181d3..7aa9ac974 100644 --- a/designs/agent-upgrade.md +++ b/designs/agent-upgrade.md @@ -62,7 +62,7 @@ Pending MachineOperation v Validate parameters | - +-- missing downloadURL -------------------------> Failed + +-- missing URL/digest or non-HTTPS URL ---------> Failed | v Mark InProgress @@ -96,18 +96,20 @@ Old process exits, new daemon starts ## Staging and switching -The daemon reads `spec.parameters["downloadURL"]` from the -`MachineOperation`. It logs the URL, resolves the current binary target, and -calls `agentbinary.InstallAndSwitchFromTarGz`. +The daemon reads `spec.parameters["downloadURL"]` and +`spec.parameters["sha256"]` from the `MachineOperation`, resolves the current +binary target, and calls `agentbinary.SecureInstallAndSwitch`. Logs and errors +omit URL query and fragment data. -`InstallAndSwitchFromTarGz` performs the upgrade as one logical operation: +`SecureInstallAndSwitch` performs the upgrade as one logical operation: -1. Download the tarball. -2. Extract the `unbounded-agent` entry into `NextTargetPath()`. -3. Reject an empty agent entry. -4. Run `unbounded-agent version` against the staged binary. -5. Update `LastGoodPath` to the previous `CurrentTargetPath`. -6. Update `CurrentPath` to the staged binary. +1. Require an HTTPS URL and an exact compressed-archive SHA-256. +2. Download the tarball within the configured size bound. +3. Require the archive to contain only the exact `unbounded-agent` entry. +4. Bound decompression and atomically install the inactive slot. +5. Run `unbounded-agent version` against the staged binary without exposing output. +6. Protect the running binary through `LastGoodPath` before replacing an inactive slot. +7. Atomically update `CurrentPath` to the staged binary. Symlink replacement uses `renameio.Symlink` through `utilio`, so each link is replaced atomically after parent directory creation. @@ -166,7 +168,8 @@ startup signal path. | Failure | Operation status | Binary state | |---------|------------------|--------------| -| Missing `downloadURL` | `Failed`, `InvalidParameters` | No link changes. | +| Missing `downloadURL` or `sha256` | `Failed`, `InvalidParameters` | No link changes. | +| Non-HTTPS URL or digest mismatch | `Failed`, `InvalidParameters` or `ExecutionFailed` | No current link change. | | Download or extraction failure | `Failed`, `ExecutionFailed` | No link changes after failure. | | Empty archive entry | `Failed`, `ExecutionFailed` | No link changes after failure. | | Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current and last-good remain unchanged. | diff --git a/docs/content/guides/operations/agent-operations.md b/docs/content/guides/operations/agent-operations.md index 393ff6828..f0c05aa52 100644 --- a/docs/content/guides/operations/agent-operations.md +++ b/docs/content/guides/operations/agent-operations.md @@ -10,8 +10,8 @@ handled by the agent itself. ## AgentUpgrade Replaces the host agent binary using blue-green staging with automatic rollback. -The operation requires a `downloadURL` parameter pointing to an agent release -tarball. +The operation requires a `downloadURL` parameter pointing to an HTTPS agent +release tarball and a `sha256` parameter containing the compressed archive digest. ```yaml apiVersion: unbounded-cloud.io/v1alpha3 @@ -23,6 +23,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` ```bash @@ -44,8 +45,8 @@ active at a time. **Staging:** -1. Downloads the release tarball from `downloadURL`. -2. Extracts the agent binary into the inactive slot. +1. Downloads the release tarball from `downloadURL` and verifies `sha256`. +2. Requires the bounded archive to contain only the exact `unbounded-agent` member. 3. Runs `unbounded-agent version` against the staged binary as a binary validation check. If this fails, the operation is marked `Failed` and the current binary is unchanged. diff --git a/docs/content/guides/operations/automation.md b/docs/content/guides/operations/automation.md index ae2efac7e..da7d1beff 100644 --- a/docs/content/guides/operations/automation.md +++ b/docs/content/guides/operations/automation.md @@ -139,6 +139,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-v1.2.0-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ttlSecondsAfterFinished: 7200 EOF @@ -170,6 +171,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-v1.2.0-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` ### Node Upgrade via Recreation diff --git a/docs/content/reference/cli.md b/docs/content/reference/cli.md index ff06f9758..3da8b07a5 100644 --- a/docs/content/reference/cli.md +++ b/docs/content/reference/cli.md @@ -319,7 +319,7 @@ Create a `MachineOperation`. | `--kind` | string | Operation kind: `NodeReboot`, `AgentUpgrade`, `AgentReset`, `HostReboot`, `HostPowerOff`, `HostPowerOn`, or `HostReplace` | | `--machine` or `--selector` | string | Target one Machine by name or select Machines by label selector | -`AgentUpgrade` also requires `--param downloadURL=`. +`AgentUpgrade` also requires `--param downloadURL=` and `--param sha256=`. #### Optional Flags @@ -372,7 +372,8 @@ Upgrade the agent: kubectl unbounded machine operation create upgrade-worker-01 \ --kind AgentUpgrade \ --machine worker-01 \ - --param downloadURL=https://example.com/unbounded-agent-linux-amd64.tar.gz + --param downloadURL=https://example.com/unbounded-agent-linux-amd64.tar.gz \ + --param sha256=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` Selector support is implemented at the CRD level. Agent operations support @@ -411,7 +412,7 @@ name, default `--ttl 300`, and `--wait=true`. | `kubectl unbounded machine host-reboot NAME` | `HostReboot` | Reboots or power-cycles the host through the owning backend. | | `kubectl unbounded machine power-off NAME` | `HostPowerOff` | Powers off the host. | | `kubectl unbounded machine power-on NAME` | `HostPowerOn` | Powers on the host. | -| `kubectl unbounded machine agent-upgrade NAME --download-url URL` | `AgentUpgrade` | Upgrades the host-side agent binary. | +| `kubectl unbounded machine agent-upgrade NAME --download-url URL --sha256 DIGEST` | `AgentUpgrade` | Upgrades the host-side agent binary. | | `kubectl unbounded machine agent-reset NAME --force` | `AgentReset` | Removes the agent and managed resources from the host. Requires confirmation unless `--force` is set. | | `kubectl unbounded machine replace NAME --force` | `HostReplace` | Destructively replaces the host. Requires confirmation unless `--force` is set. | diff --git a/docs/content/reference/machina-crd.md b/docs/content/reference/machina-crd.md index 9f3f90527..beebe2153 100644 --- a/docs/content/reference/machina-crd.md +++ b/docs/content/reference/machina-crd.md @@ -196,7 +196,7 @@ spec: | `status.targets` | []TargetStatus | No | Per-Machine target status snapshot used by host operation controllers. | | `status.conditions` | []Condition | No | Operation conditions. `Completed` tracks terminal state. `BootLoaderDownloaded=True` is latched by metalman when a target first downloads the initial PXE boot loader, usually over TFTP. `BootImageWritten` starts as `Unknown` for metalman `HostReplace`, transitions to `False` when the PXE installer requests `disk.img.gz`, and transitions to `True` when the existing `/pxe/disable` completion signal is received. `CloudInitDone` starts as `Unknown`, transitions to `False` when first-boot cloud-init starts, and transitions to `True` on final cloud-init success or `False` with reason `Failed` and a summarized error when cloud-init reports a failure. | -`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL`. The URL must point to an `unbounded-agent` release tarball; the agent stages it as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. +`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL` and `spec.parameters.sha256`. The URL must be HTTPS and point to an `unbounded-agent` release tarball; `sha256` is the expected digest of the compressed archive. The agent verifies the archive, stages its exact `unbounded-agent` member as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. The Azure VM provider handles: From 75333be34462a8743ee739480733d61ab7c98bc3 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 22:42:51 +0000 Subject: [PATCH 05/20] agent: remove duplicate upgrade staging path --- cmd/agent/internal/daemon/agentupgrade.go | 22 -- .../internal/daemon/agentupgrade_test.go | 293 +----------------- 2 files changed, 4 insertions(+), 311 deletions(-) diff --git a/cmd/agent/internal/daemon/agentupgrade.go b/cmd/agent/internal/daemon/agentupgrade.go index 40ab15e07..b2a3de143 100644 --- a/cmd/agent/internal/daemon/agentupgrade.go +++ b/cmd/agent/internal/daemon/agentupgrade.go @@ -72,28 +72,6 @@ func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest return request, nil } -// upgradeDaemonBinary retains the legacy Unbounded download contract for -// compatibility tests. Managed MachineOperations use upgradeDaemonBinarySecure. -func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, downloadURL string) error { - paths, err := goalstates.ResolvedAgentUpgradePaths() - if err != nil { - return fmt.Errorf("resolve current daemon binary symlink: %w", err) - } - - targetPath := paths.NextTargetPath() - if err := agentbinary.InstallAndSwitchFromTarGz(ctx, downloadURL, paths, agentUpgradeBinaryMode); err != nil { - return err - } - - log.Info("staged upgraded daemon binary", - "url", downloadURL, - "previous", paths.CurrentTargetPath, - "current", targetPath, - ) - - return nil -} - func upgradeDaemonBinarySecure(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { paths, err := goalstates.ResolvedAgentUpgradePaths() if err != nil { diff --git a/cmd/agent/internal/daemon/agentupgrade_test.go b/cmd/agent/internal/daemon/agentupgrade_test.go index f84a21d8b..25dd0fa4e 100644 --- a/cmd/agent/internal/daemon/agentupgrade_test.go +++ b/cmd/agent/internal/daemon/agentupgrade_test.go @@ -4,24 +4,13 @@ package daemon import ( - "archive/tar" - "bytes" - "compress/gzip" - "context" - "fmt" - "io" - "log/slog" - "net/http" - "net/http/httptest" "os" "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/Azure/unbounded/pkg/agent/agentbinary" "github.com/Azure/unbounded/pkg/agent/goalstates" ) @@ -47,11 +36,10 @@ func TestParseAgentUpgradeRequest(t *testing.T) { assert.Contains(t, err.Error(), agentUpgradeSHA256Parameter) } -func TestAgentUpgradeSignalOperator_RecordFailure(t *testing.T) { +func TestAgentUpgradeSignalOperatorRecordFailure(t *testing.T) { t.Parallel() - dir := t.TempDir() - signalPath := filepath.Join(dir, "agent-upgrade-signal") + signalPath := filepath.Join(t.TempDir(), "agent-upgrade-signal") signals := newAgentUpgradeSignalOperatorForPath(signalPath) require.NoError(t, signals.RecordPending("op-1", 7)) @@ -62,11 +50,10 @@ func TestAgentUpgradeSignalOperator_RecordFailure(t *testing.T) { assert.JSONEq(t, `{"operationName":"op-1","observedMachineGeneration":7,"failureMessage":"rolled back"}`, string(data)) } -func TestAgentUpgradeSignalOperator_ReadRejectsNonJSON(t *testing.T) { +func TestAgentUpgradeSignalOperatorReadRejectsNonJSON(t *testing.T) { t.Parallel() - dir := t.TempDir() - signalPath := filepath.Join(dir, "agent-upgrade-signal") + signalPath := filepath.Join(t.TempDir(), "agent-upgrade-signal") signals := newAgentUpgradeSignalOperatorForPath(signalPath) require.NoError(t, os.WriteFile(signalPath, []byte("op-1\n"), 0o600)) @@ -75,169 +62,6 @@ func TestAgentUpgradeSignalOperator_ReadRejectsNonJSON(t *testing.T) { assert.Contains(t, err.Error(), "decode AgentUpgrade signal") } -func TestUpgradeDaemonBinary(t *testing.T) { - dir := t.TempDir() - legacyPath := filepath.Join(dir, "unbounded-agent") - currentPath := filepath.Join(dir, "unbounded-agent-current") - lastGoodPath := filepath.Join(dir, "unbounded-agent-last-good") - bluePath := filepath.Join(dir, "unbounded-agent-blue") - greenPath := filepath.Join(dir, "unbounded-agent-green") - - require.NoError(t, os.WriteFile(legacyPath, []byte("legacy"), 0o755)) - require.NoError(t, os.Symlink(legacyPath, currentPath)) - - t.Setenv(goalstates.EnvDaemonBinary, legacyPath) - t.Setenv(goalstates.EnvDaemonBinaryCurrent, currentPath) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, lastGoodPath) - t.Setenv(goalstates.EnvDaemonBinaryBlue, bluePath) - t.Setenv(goalstates.EnvDaemonBinaryGreen, greenPath) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/gzip") - w.WriteHeader(http.StatusOK) - require.NoError(t, writeAgentArchive(w, agentArchiveScript("new-agent-binary", 0))) - })) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - target, err := filepath.EvalSymlinks(currentPath) - require.NoError(t, err) - assert.Equal(t, bluePath, target) - - lastGoodTarget, err := filepath.EvalSymlinks(lastGoodPath) - require.NoError(t, err) - assert.Equal(t, legacyPath, lastGoodTarget) - - newData, err := os.ReadFile(bluePath) - require.NoError(t, err) - assert.Equal(t, agentArchiveScript("new-agent-binary", 0), newData) -} - -func TestUpgradeDaemonBinary_AlternatesFromBlueToGreen(t *testing.T) { - dir := t.TempDir() - currentPath := filepath.Join(dir, "unbounded-agent-current") - lastGoodPath := filepath.Join(dir, "unbounded-agent-last-good") - bluePath := filepath.Join(dir, "unbounded-agent-blue") - greenPath := filepath.Join(dir, "unbounded-agent-green") - - require.NoError(t, os.WriteFile(bluePath, []byte("blue"), 0o755)) - require.NoError(t, os.Symlink(bluePath, currentPath)) - - t.Setenv(goalstates.EnvDaemonBinaryCurrent, currentPath) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, lastGoodPath) - t.Setenv(goalstates.EnvDaemonBinaryBlue, bluePath) - t.Setenv(goalstates.EnvDaemonBinaryGreen, greenPath) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - require.NoError(t, writeAgentArchive(w, agentArchiveScript("green", 0))) - })) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - target, err := filepath.EvalSymlinks(currentPath) - require.NoError(t, err) - assert.Equal(t, greenPath, target) - - lastGoodTarget, err := filepath.EvalSymlinks(lastGoodPath) - require.NoError(t, err) - assert.Equal(t, bluePath, lastGoodTarget) -} - -func TestUpgradeDaemonBinary_SequentialSuccesses(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 0)}, - {binary: agentArchiveScript("agent-b", 0)}, - }) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - assertSymlinkTarget(t, paths.current, paths.green) - assertSymlinkTarget(t, paths.lastGood, paths.blue) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 0))) - assertFileContent(t, paths.green, string(agentArchiveScript("agent-b", 0))) -} - -func TestUpgradeDaemonBinary_SequentialSuccessThenFailure(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 0)}, - {status: http.StatusInternalServerError}, - }) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 0))) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_SequentialFailureThenFailure(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {status: http.StatusInternalServerError}, - {status: http.StatusInternalServerError}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.legacy) - assert.NoFileExists(t, paths.lastGood) - assert.NoFileExists(t, paths.blue) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_SequentialFailureThenSuccess(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {status: http.StatusInternalServerError}, - {binary: agentArchiveScript("agent-b", 0)}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-b", 0))) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_RejectsBrokenBinary(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 42)}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.legacy) - assert.NoFileExists(t, paths.lastGood) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 42))) - assert.NoFileExists(t, paths.green) -} - -func TestDownloadAgentBinaryFromTarGz_RejectsUnsupportedScheme(t *testing.T) { - t.Parallel() - - err := agentbinary.InstallFromTarGz(context.Background(), "file:///tmp/unbounded-agent.tar.gz", filepath.Join(t.TempDir(), "agent"), goalstates.AgentUpgradeBinaryName, 0o755) - require.Error(t, err) - assert.Contains(t, err.Error(), "unsupported agent download URL scheme") -} - func TestAgentUpgradePathsInitialDaemonBinaryTarget(t *testing.T) { t.Parallel() @@ -269,112 +93,3 @@ func TestAgentUpgradePathsInitialDaemonBinaryTarget(t *testing.T) { _, err = paths.InitialDaemonBinaryTarget() require.Error(t, err) } - -type daemonBinaryTestPaths struct { - legacy string - current string - lastGood string - blue string - green string -} - -func setupDaemonBinaryTest(t *testing.T) daemonBinaryTestPaths { - t.Helper() - - paths := setupDaemonBinaryTestWithoutLinks(t) - require.NoError(t, os.WriteFile(paths.legacy, []byte("legacy"), 0o755)) - require.NoError(t, os.Symlink(paths.legacy, paths.current)) - - return paths -} - -func setupDaemonBinaryTestWithoutLinks(t *testing.T) daemonBinaryTestPaths { - t.Helper() - - dir := t.TempDir() - paths := daemonBinaryTestPaths{ - legacy: filepath.Join(dir, "unbounded-agent"), - current: filepath.Join(dir, "unbounded-agent-current"), - lastGood: filepath.Join(dir, "unbounded-agent-last-good"), - blue: filepath.Join(dir, "unbounded-agent-blue"), - green: filepath.Join(dir, "unbounded-agent-green"), - } - - t.Setenv(goalstates.EnvDaemonBinary, paths.legacy) - t.Setenv(goalstates.EnvDaemonBinaryCurrent, paths.current) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, paths.lastGood) - t.Setenv(goalstates.EnvDaemonBinaryBlue, paths.blue) - t.Setenv(goalstates.EnvDaemonBinaryGreen, paths.green) - - return paths -} - -type archiveResponse struct { - binary []byte - status int -} - -func newAgentArchiveSequenceServer(t *testing.T, responses []archiveResponse) *httptest.Server { - t.Helper() - - next := 0 - - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - require.Less(t, next, len(responses)) - response := responses[next] - next++ - - if response.status != 0 { - http.Error(w, "failed", response.status) - return - } - - w.Header().Set("Content-Type", "application/gzip") - require.NoError(t, writeAgentArchive(w, response.binary)) - })) -} - -func assertSymlinkTarget(t *testing.T, linkPath, expectedTarget string) { - t.Helper() - - target, err := filepath.EvalSymlinks(linkPath) - require.NoError(t, err) - assert.Equal(t, expectedTarget, target) -} - -func assertFileContent(t *testing.T, path, expected string) { - t.Helper() - - data, err := os.ReadFile(path) - require.NoError(t, err) - assert.Equal(t, expected, string(data)) -} - -func writeAgentArchive(w io.Writer, binary []byte) error { - gz := gzip.NewWriter(w) - defer gz.Close() - - tw := tar.NewWriter(gz) - defer tw.Close() - - header := &tar.Header{ - Name: "unbounded-agent", - Mode: 0o755, - Size: int64(len(binary)), - } - if err := tw.WriteHeader(header); err != nil { - return err - } - - _, err := io.Copy(tw, bytes.NewReader(binary)) - - return err -} - -func agentArchiveScript(version string, exitCode int) []byte { - return []byte(fmt.Sprintf("#!/bin/sh\nprintf '%%s\\n' %s\nexit %d\n", shellQuote(version), exitCode)) -} - -func shellQuote(value string) string { - return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" -} From 10ab536796c4adf61cef008f853306475ebe0549 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Thu, 6 Aug 2026 22:50:35 +0000 Subject: [PATCH 06/20] agent: address secure installer review --- pkg/agent/agentbinary/upgrade.go | 92 ++++++++++++++++----------- pkg/agent/agentbinary/upgrade_test.go | 10 ++- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index 6fbf41098..e9bcf3bd5 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -56,34 +56,70 @@ type SwitchResult struct { CurrentPath string } +type normalizedSecureInstallOptions struct { + options SecureInstallOptions + parsedURL *url.URL + expectedDigest [sha256.Size]byte +} + // ValidateSecureInstallOptions validates caller-provided secure install inputs. func ValidateSecureInstallOptions(opts SecureInstallOptions) error { - if _, err := validateSecureDownloadURL(opts.DownloadURL); err != nil { - return err + _, err := normalizeSecureInstallOptions(opts) + + return err +} + +func normalizeSecureInstallOptions(opts SecureInstallOptions) (normalizedSecureInstallOptions, error) { + parsedURL, err := validateSecureDownloadURL(opts.DownloadURL) + if err != nil { + return normalizedSecureInstallOptions{}, err } - if _, err := parseSHA256(opts.ExpectedSHA256); err != nil { - return err + expectedDigest, err := parseSHA256(opts.ExpectedSHA256) + if err != nil { + return normalizedSecureInstallOptions{}, err } - expectedMember := strings.TrimSpace(opts.ExpectedMember) - if expectedMember == "" || filepath.Base(expectedMember) != expectedMember { - return fmt.Errorf("expected archive member must be a base name") + opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) + if opts.ExpectedMember == "" || filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { + return normalizedSecureInstallOptions{}, fmt.Errorf("expected archive member must be an exact base name without a path prefix") } if opts.Mode != 0 && opts.Mode.Perm() != opts.Mode { - return fmt.Errorf("agent binary mode must contain permission bits only") + return normalizedSecureInstallOptions{}, fmt.Errorf("agent binary mode must contain permission bits only") } if opts.MaxArchiveBytes < 0 || opts.MaxExtractedBytes < 0 { - return fmt.Errorf("agent archive size limits must not be negative") + return normalizedSecureInstallOptions{}, fmt.Errorf("agent archive size limits must not be negative") + } + + if opts.MaxArchiveBytes > math.MaxInt64-1 { + return normalizedSecureInstallOptions{}, fmt.Errorf("maximum archive size is too large") } if opts.MaxExtractedBytes > math.MaxInt64/2 { - return fmt.Errorf("maximum extracted size is too large") + return normalizedSecureInstallOptions{}, fmt.Errorf("maximum extracted size is too large") } - return nil + if opts.Mode == 0 { + opts.Mode = daemonBinaryMode + } + + if opts.MaxArchiveBytes == 0 { + opts.MaxArchiveBytes = defaultMaxArchiveBytes + } + + if opts.MaxExtractedBytes == 0 { + opts.MaxExtractedBytes = defaultMaxBinaryBytes + } + + opts.HTTPClient = secureHTTPClient(opts.HTTPClient) + + return normalizedSecureInstallOptions{ + options: opts, + parsedURL: parsedURL, + expectedDigest: expectedDigest, + }, nil } // ValidateLayout verifies that all binary paths are clean, absolute, and distinct. @@ -114,7 +150,8 @@ func ValidateLayout(paths Layout) error { // SecureInstallAndSwitch downloads and verifies an HTTPS release archive, // installs its only member into the inactive slot, and atomically updates the -// last-good and current links. +// last-good and current links. The archive member must exactly equal the +// configured base name; path prefixes such as "./" are intentionally rejected. func SecureInstallAndSwitch( ctx context.Context, log *slog.Logger, @@ -129,35 +166,14 @@ func SecureInstallAndSwitch( return SwitchResult{}, err } - if err := ValidateSecureInstallOptions(opts); err != nil { - return SwitchResult{}, err - } - - parsedURL, err := validateSecureDownloadURL(opts.DownloadURL) - if err != nil { - return SwitchResult{}, err - } - - expectedDigest, err := parseSHA256(opts.ExpectedSHA256) + normalized, err := normalizeSecureInstallOptions(opts) if err != nil { return SwitchResult{}, err } - opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) - - if opts.Mode == 0 { - opts.Mode = daemonBinaryMode - } - - if opts.MaxArchiveBytes <= 0 { - opts.MaxArchiveBytes = defaultMaxArchiveBytes - } - - if opts.MaxExtractedBytes <= 0 { - opts.MaxExtractedBytes = defaultMaxBinaryBytes - } - - opts.HTTPClient = secureHTTPClient(opts.HTTPClient) + opts = normalized.options + parsedURL := normalized.parsedURL + expectedDigest := normalized.expectedDigest previousPath, err := executablePath(paths.CurrentPath) if err != nil { @@ -218,7 +234,7 @@ func validateSecureDownloadURL(rawURL string) (*url.URL, error) { } if parsedURL.Scheme != "https" || parsedURL.Host == "" || parsedURL.User != nil || parsedURL.Fragment != "" { - return nil, fmt.Errorf("download URL must be an HTTPS URL without user information") + return nil, fmt.Errorf("download URL must use HTTPS, include a host, omit user information, and omit fragments") } return parsedURL, nil diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index 9691a7855..d15a3a55d 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "log/slog" + "math" "net/http" "net/http/httptest" "net/url" @@ -105,6 +106,12 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { ExpectedMember: "custom-agent", MaxArchiveBytes: -1, }, + "archive size overflow": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + MaxArchiveBytes: math.MaxInt64, + }, } for name, opts := range tests { t.Run(name, func(t *testing.T) { @@ -257,7 +264,8 @@ func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { t.Parallel() tests := map[string][]secureTarMember{ - "unsafe": {{name: "../custom-agent", body: []byte("binary")}}, + "unsafe": {{name: "../custom-agent", body: []byte("binary")}}, + "path prefixed": {{name: "./custom-agent", body: []byte("binary")}}, "duplicate": { {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, From d68bbe5c44454d43bcfc025f2074d3adc8535f9d Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 00:02:13 +0000 Subject: [PATCH 07/20] test: serve agent upgrades over verified HTTPS --- hack/agent/e2e-kind/e2e.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 88926806a..9da4688e0 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -62,6 +62,7 @@ import re import secrets import shutil +import ssl import subprocess import sys import textwrap @@ -1178,10 +1179,31 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp """Serve *tarball* to the VM, create AgentUpgrade, and wait for it.""" runner_ip = VM_GATEWAY - agent_url = f"http://{runner_ip}:{SERVE_PORT}/{tarball.name}" - log(f"Starting HTTP file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") + agent_url = f"https://{runner_ip}:{SERVE_PORT}/{tarball.name}" + digest = hashlib.sha256(tarball.read_bytes()).hexdigest() + cert_path = tarball.parent / "agent-upgrade-e2e.crt" + key_path = tarball.parent / "agent-upgrade-e2e.key" + run([ + "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", + "-subj", f"/CN={runner_ip}", "-addext", f"subjectAltName=IP:{runner_ip}", + "-keyout", str(key_path), "-out", str(cert_path), + ]) + scp_cmd(str(cert_path), f"{SSH_TARGET}:/tmp/agent-upgrade-e2e.crt") + ssh_cmd( + "if command -v update-ca-certificates >/dev/null 2>&1; then " + "sudo cp /tmp/agent-upgrade-e2e.crt /usr/local/share/ca-certificates/agent-upgrade-e2e.crt && " + "sudo update-ca-certificates >/dev/null; " + "else sudo cp /tmp/agent-upgrade-e2e.crt /etc/pki/ca-trust/source/anchors/agent-upgrade-e2e.crt && " + "sudo update-ca-trust; fi" + ) + ssh_cmd("sudo systemctl restart unbounded-agent-daemon.service") + wait_for_daemon_active() + log(f"Starting HTTPS file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") handler = _make_handler(str(tarball.parent)) httpd = HTTPServer((runner_ip, SERVE_PORT), handler) + tls_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + tls_context.load_cert_chain(certfile=cert_path, keyfile=key_path) + httpd.socket = tls_context.wrap_socket(httpd.socket, server_side=True) server_thread = Thread(target=httpd.serve_forever, daemon=True) server_thread.start() try: @@ -1193,7 +1215,7 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp operation_name, AGENT_MACHINE_NAME, "AgentUpgrade", - parameters={"downloadURL": agent_url}, + parameters={"downloadURL": agent_url, "sha256": digest}, ) if expect_complete: return wait_for_machine_operation_complete(operation_name) From 10efc812e8171fafb9bddccbc9f3fd591670602f Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 00:12:19 +0000 Subject: [PATCH 08/20] test: reuse agent upgrade e2e certificate --- hack/agent/e2e-kind/e2e.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 9da4688e0..3be820805 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -1183,21 +1183,24 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp digest = hashlib.sha256(tarball.read_bytes()).hexdigest() cert_path = tarball.parent / "agent-upgrade-e2e.crt" key_path = tarball.parent / "agent-upgrade-e2e.key" - run([ - "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", - "-subj", f"/CN={runner_ip}", "-addext", f"subjectAltName=IP:{runner_ip}", - "-keyout", str(key_path), "-out", str(cert_path), - ]) - scp_cmd(str(cert_path), f"{SSH_TARGET}:/tmp/agent-upgrade-e2e.crt") - ssh_cmd( - "if command -v update-ca-certificates >/dev/null 2>&1; then " - "sudo cp /tmp/agent-upgrade-e2e.crt /usr/local/share/ca-certificates/agent-upgrade-e2e.crt && " - "sudo update-ca-certificates >/dev/null; " - "else sudo cp /tmp/agent-upgrade-e2e.crt /etc/pki/ca-trust/source/anchors/agent-upgrade-e2e.crt && " - "sudo update-ca-trust; fi" - ) - ssh_cmd("sudo systemctl restart unbounded-agent-daemon.service") - wait_for_daemon_active() + if not cert_path.exists() or not key_path.exists(): + run([ + "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", + "-subj", f"/CN={runner_ip}", "-addext", f"subjectAltName=IP:{runner_ip}", + "-keyout", str(key_path), "-out", str(cert_path), + ]) + scp_cmd(str(cert_path), f"{SSH_TARGET}:/tmp/agent-upgrade-e2e.crt") + ssh_cmd( + "if command -v update-ca-certificates >/dev/null 2>&1; then " + "sudo cp /tmp/agent-upgrade-e2e.crt /usr/local/share/ca-certificates/agent-upgrade-e2e.crt && " + "sudo update-ca-certificates >/dev/null; " + "else sudo cp /tmp/agent-upgrade-e2e.crt /etc/pki/ca-trust/source/anchors/agent-upgrade-e2e.crt && " + "sudo update-ca-trust; fi" + ) + # Go loads system roots lazily and may cache them for the process lifetime. + # Restart once after installing the test CA, before any upgrade candidate runs. + ssh_cmd("sudo systemctl restart unbounded-agent-daemon.service") + wait_for_daemon_active() log(f"Starting HTTPS file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") handler = _make_handler(str(tarball.parent)) httpd = HTTPServer((runner_ip, SERVE_PORT), handler) From 7e07b9ae81398e24d43456ac3b36248bf2a51969 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 00:20:52 +0000 Subject: [PATCH 09/20] test: update secure upgrade failure assertion --- hack/agent/e2e-kind/e2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 3be820805..171984602 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -3715,7 +3715,7 @@ def validate_agent_upgrade_rollback() -> None: broken_tarball, broken_operation_name, expect_complete=False) broken_status = broken_operation.get("status", {}) log(f"Broken AgentUpgrade failure reason: {broken_status.get('reason')!r}") - if "verify agent binary" not in broken_status.get("message", ""): + if "verify upgraded agent binary" not in broken_status.get("message", ""): die(f"unexpected broken AgentUpgrade failure message: {broken_status.get('message')!r}") if read_daemon_current_target() != previous_good: die("broken AgentUpgrade changed current daemon binary symlink") From da53b6da31fb550313de76bd128343c5e37ca4db Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 00:32:00 +0000 Subject: [PATCH 10/20] test: isolate agent upgrade start limits --- hack/agent/e2e-kind/e2e.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 171984602..eeb3b2445 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -1212,6 +1212,10 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp try: log(f"Verifying VM can reach agent upgrade URL: {agent_url}") ssh_cmd(f"curl -fsSL --connect-timeout 10 -o /dev/null {agent_url}") + # Each scenario intentionally restarts or fails the daemon. Isolate its + # systemd start-limit budget so the candidate under test gets the + # configured retries before recovery runs. + ssh_cmd("sudo systemctl reset-failed unbounded-agent-daemon.service") run_quiet([KUBECTL, "delete", _machine_operation_resource(), operation_name, "--ignore-not-found"], check=False) create_machine_operation( From adeb4e787ced64a051fbb5228870155f2842e24a Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 18:54:32 +0000 Subject: [PATCH 11/20] agent: preserve rollback state during secure upgrades --- .../app/machine_operation_create.go | 2 +- designs/agent-upgrade.md | 8 +- hack/agent/e2e-kind/e2e.py | 8 +- pkg/agent/agentbinary/upgrade.go | 102 ++++++++++++++++-- pkg/agent/agentbinary/upgrade_test.go | 61 +++++++++++ pkg/agent/internal/utilio/io.go | 3 +- pkg/agent/internal/utilio/io_test.go | 19 ++++ 7 files changed, 185 insertions(+), 18 deletions(-) create mode 100644 pkg/agent/internal/utilio/io_test.go diff --git a/cmd/kubectl-unbounded/app/machine_operation_create.go b/cmd/kubectl-unbounded/app/machine_operation_create.go index 9a73020f0..a29145015 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_create.go +++ b/cmd/kubectl-unbounded/app/machine_operation_create.go @@ -228,7 +228,7 @@ func (o *machineOperationCreateOptions) validate() error { if o.kind == v1alpha3.OperationAgentUpgrade { if parameters["downloadURL"] == "" { - return fmt.Errorf("AgentUpgrade requires --param downloadURL=") + return fmt.Errorf("AgentUpgrade requires --param downloadURL=") } if parameters["sha256"] == "" { diff --git a/designs/agent-upgrade.md b/designs/agent-upgrade.md index 7aa9ac974..1b58b1d34 100644 --- a/designs/agent-upgrade.md +++ b/designs/agent-upgrade.md @@ -108,7 +108,7 @@ omit URL query and fragment data. 3. Require the archive to contain only the exact `unbounded-agent` entry. 4. Bound decompression and atomically install the inactive slot. 5. Run `unbounded-agent version` against the staged binary without exposing output. -6. Protect the running binary through `LastGoodPath` before replacing an inactive slot. +6. If the inactive slot is last-good, protect the running binary through `LastGoodPath` before replacing it; otherwise defer the last-good update until candidate verification succeeds. 7. Atomically update `CurrentPath` to the staged binary. Symlink replacement uses `renameio.Symlink` through `utilio`, so each link is @@ -170,9 +170,9 @@ startup signal path. |---------|------------------|--------------| | Missing `downloadURL` or `sha256` | `Failed`, `InvalidParameters` | No link changes. | | Non-HTTPS URL or digest mismatch | `Failed`, `InvalidParameters` or `ExecutionFailed` | No current link change. | -| Download or extraction failure | `Failed`, `ExecutionFailed` | No link changes after failure. | -| Empty archive entry | `Failed`, `ExecutionFailed` | No link changes after failure. | -| Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current and last-good remain unchanged. | +| Download or extraction failure | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | +| Empty archive entry | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | +| Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current remains unchanged. A distinct last-good target remains unchanged. | | Restart command fails | `Failed` | Signal is cleared. Links may already point to the staged binary. | | Upgraded daemon fails under systemd | `Failed`, `DaemonFailed` | Recovery restores current to last-good. | diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index eeb3b2445..a4f5648ac 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -1175,15 +1175,20 @@ def wait_for_daemon_active(timeout_secs: int = 180) -> None: die(f"Timed out waiting for daemon to become active; last status={last_status!r}") +_agent_upgrade_certificate_ready = False + + def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_complete: bool = True) -> dict[str, Any]: """Serve *tarball* to the VM, create AgentUpgrade, and wait for it.""" + global _agent_upgrade_certificate_ready + runner_ip = VM_GATEWAY agent_url = f"https://{runner_ip}:{SERVE_PORT}/{tarball.name}" digest = hashlib.sha256(tarball.read_bytes()).hexdigest() cert_path = tarball.parent / "agent-upgrade-e2e.crt" key_path = tarball.parent / "agent-upgrade-e2e.key" - if not cert_path.exists() or not key_path.exists(): + if not _agent_upgrade_certificate_ready: run([ "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", "-subj", f"/CN={runner_ip}", "-addext", f"subjectAltName=IP:{runner_ip}", @@ -1201,6 +1206,7 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp # Restart once after installing the test CA, before any upgrade candidate runs. ssh_cmd("sudo systemctl restart unbounded-agent-daemon.service") wait_for_daemon_active() + _agent_upgrade_certificate_ready = True log(f"Starting HTTPS file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") handler = _make_handler(str(tarball.parent)) httpd = HTTPServer((runner_ip, SERVE_PORT), handler) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index e9bcf3bd5..f48d37571 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -32,6 +32,7 @@ const ( // Layout describes caller-owned blue-green agent binary paths. type Layout struct { + // BinaryPath is an optional compatibility path included in collision validation. BinaryPath string BluePath string GreenPath string @@ -81,7 +82,8 @@ func normalizeSecureInstallOptions(opts SecureInstallOptions) (normalizedSecureI } opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) - if opts.ExpectedMember == "" || filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { + if opts.ExpectedMember == "" || opts.ExpectedMember == "." || opts.ExpectedMember == ".." || + filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { return normalizedSecureInstallOptions{}, fmt.Errorf("expected archive member must be an exact base name without a path prefix") } @@ -122,15 +124,18 @@ func normalizeSecureInstallOptions(opts SecureInstallOptions) (normalizedSecureI }, nil } -// ValidateLayout verifies that all binary paths are clean, absolute, and distinct. +// ValidateLayout verifies that all required binary paths are clean, absolute, +// and distinct. It also validates BinaryPath when provided. func ValidateLayout(paths Layout) error { values := []string{ - paths.BinaryPath, paths.BluePath, paths.GreenPath, paths.CurrentPath, paths.LastGoodPath, } + if paths.BinaryPath != "" { + values = append(values, paths.BinaryPath) + } seen := make(map[string]struct{}, len(values)) for _, value := range values { @@ -180,8 +185,13 @@ func SecureInstallAndSwitch( return SwitchResult{}, fmt.Errorf("resolve current agent binary: %w", err) } + currentIsBlue, err := pathResolvesTo(paths.BluePath, previousPath) + if err != nil { + return SwitchResult{}, fmt.Errorf("resolve blue agent binary: %w", err) + } + targetPath := paths.BluePath - if previousPath == paths.BluePath { + if currentIsBlue { targetPath = paths.GreenPath } @@ -191,10 +201,25 @@ func SecureInstallAndSwitch( } defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup - // The inactive slot may still be last-good. Protect the verified running - // binary before replacing that slot. - if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { - return SwitchResult{}, fmt.Errorf("protect current agent as last-good: %w", err) + lastGoodTarget, err := filepath.EvalSymlinks(paths.LastGoodPath) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return SwitchResult{}, fmt.Errorf("resolve last-good agent binary: %w", err) + } + + canonicalTargetDir, err := filepath.EvalSymlinks(filepath.Dir(targetPath)) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return SwitchResult{}, fmt.Errorf("resolve inactive agent binary directory: %w", err) + } + + // Protect last-good before replacing the inactive slot only when that slot + // contains last-good. Otherwise, preserve the existing rollback target until + // the candidate has been staged and verified. A missing target directory + // cannot contain a last-good binary. + lastGoodProtected := err == nil && lastGoodTarget == filepath.Join(canonicalTargetDir, filepath.Base(targetPath)) + if lastGoodProtected { + if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { + return SwitchResult{}, fmt.Errorf("protect current agent as last-good: %w", err) + } } if err := extractOnlyArchiveMember(archivePath, targetPath, opts); err != nil { @@ -205,6 +230,12 @@ func SecureInstallAndSwitch( return SwitchResult{}, err } + if !lastGoodProtected { + if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { + return SwitchResult{}, fmt.Errorf("update last-good agent symlink: %w", err) + } + } + if err := utilio.UpdateSymlink(paths.CurrentPath, targetPath); err != nil { return SwitchResult{}, fmt.Errorf("update current agent symlink: %w", err) } @@ -220,6 +251,10 @@ func SecureInstallAndSwitch( // RedactedURL removes query and fragment data that may contain credentials. func RedactedURL(parsedURL *url.URL) string { + if parsedURL == nil { + return "" + } + redacted := *parsedURL redacted.RawQuery = "" redacted.Fragment = "" @@ -371,6 +406,14 @@ func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstall defer gz.Close() //nolint:errcheck // extraction reports read errors found := false + stagedPath := "" + + defer func() { + if stagedPath != "" { + os.Remove(stagedPath) //nolint:errcheck // best effort staged file cleanup + } + }() + decompressed := &countingReader{reader: io.LimitReader(gz, 2*opts.MaxExtractedBytes+1)} tarReader := tar.NewReader(decompressed) @@ -400,13 +443,31 @@ func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstall return fmt.Errorf("agent archive member %q is not a valid bounded regular file", opts.ExpectedMember) } - if err := utilio.InstallFileWithLimitedSize(targetPath, tarReader, opts.Mode, opts.MaxExtractedBytes); err != nil { - return fmt.Errorf("install upgraded agent binary: %w", err) + if mkdirErr := os.MkdirAll(filepath.Dir(targetPath), 0o750); mkdirErr != nil { + return fmt.Errorf("create agent binary directory: %w", mkdirErr) + } + + staged, createErr := os.CreateTemp(filepath.Dir(targetPath), ".agent-upgrade-*") + if createErr != nil { + return fmt.Errorf("create staged agent binary: %w", createErr) + } + + stagedPath = staged.Name() + if closeErr := staged.Close(); closeErr != nil { + return fmt.Errorf("close staged agent binary: %w", closeErr) + } + + if err := utilio.InstallFileWithLimitedSize(stagedPath, tarReader, opts.Mode, opts.MaxExtractedBytes); err != nil { + return fmt.Errorf("stage upgraded agent binary: %w", err) } found = true } + if _, err := io.Copy(io.Discard, decompressed); err != nil { + return fmt.Errorf("finish reading agent archive: %w", err) + } + if decompressed.count > 2*opts.MaxExtractedBytes { return fmt.Errorf("decompressed agent archive exceeds %d-byte limit", 2*opts.MaxExtractedBytes) } @@ -415,17 +476,36 @@ func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstall return fmt.Errorf("agent archive does not contain expected member %q", opts.ExpectedMember) } + if err := os.Rename(stagedPath, targetPath); err != nil { + return fmt.Errorf("install upgraded agent binary: %w", err) + } + + stagedPath = "" + return nil } func safeArchiveName(name string) bool { - return name != "" && + return name != "" && name != "." && name != ".." && !filepath.IsAbs(name) && filepath.Clean(name) == name && !strings.Contains(name, `\`) && !strings.HasPrefix(name, ".."+string(filepath.Separator)) } +func pathResolvesTo(path, expected string) (bool, error) { + resolved, err := filepath.EvalSymlinks(path) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + + if err != nil { + return false, err + } + + return resolved == expected, nil +} + func executablePath(path string) (string, error) { resolved, err := filepath.EvalSymlinks(path) if err != nil { diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index d15a3a55d..be1cca33f 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -94,6 +94,16 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "bin/custom-agent", }, + "dot member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: ".", + }, + "dot-dot member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "..", + }, "invalid mode": { DownloadURL: "https://example.com/agent.tar.gz", ExpectedSHA256: strings.Repeat("a", 64), @@ -132,6 +142,11 @@ func TestValidateLayout(t *testing.T) { t.Fatalf("ValidateLayout: %v", err) } + paths.BinaryPath = "" + if err := ValidateLayout(paths); err != nil { + t.Fatalf("ValidateLayout without optional BinaryPath: %v", err) + } + paths.LastGoodPath = paths.CurrentPath if err := ValidateLayout(paths); err == nil { t.Fatal("ValidateLayout duplicate path error = nil") @@ -220,6 +235,44 @@ func TestSecureInstallAndSwitchPreservesCurrentOnVerificationFailures(t *testing } } +func TestSecureInstallAndSwitchPreservesDistinctLastGoodOnCandidateFailure(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + if err := os.WriteFile(paths.BinaryPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write distinct last-good: %v", err) + } + + if err := os.Remove(paths.LastGoodPath); err != nil { + t.Fatalf("remove last-good link: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink distinct last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 42\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("SecureInstallAndSwitch error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BinaryPath) +} + func TestSecureInstallAndSwitchEnforcesSizeLimits(t *testing.T) { t.Parallel() @@ -293,6 +346,10 @@ func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + + if _, statErr := os.Stat(paths.GreenPath); !os.IsNotExist(statErr) { + t.Fatalf("inactive slot changed on invalid archive: %v", statErr) + } }) } } @@ -327,6 +384,10 @@ func TestSecureInstallAndSwitchRejectsHTTPRedirect(t *testing.T) { func TestRedactedURL(t *testing.T) { t.Parallel() + if got := RedactedURL(nil); got != "" { + t.Fatalf("RedactedURL(nil) = %q", got) + } + parsed, err := url.Parse("https://example.com/agent.tar.gz?sig=secret") if err != nil { t.Fatalf("parse URL: %v", err) diff --git a/pkg/agent/internal/utilio/io.go b/pkg/agent/internal/utilio/io.go index ed107dde6..f38ea0ec2 100644 --- a/pkg/agent/internal/utilio/io.go +++ b/pkg/agent/internal/utilio/io.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math" "os" "path/filepath" @@ -30,7 +31,7 @@ func InstallFile(filename string, r io.Reader, perm os.FileMode) error { // // NOTE: we assume the filename is trusted and cleaned without path traversal characters. func InstallFileWithLimitedSize(filename string, r io.Reader, perm os.FileMode, maxBytes int64) error { - if maxBytes <= 0 { + if maxBytes <= 0 || maxBytes == math.MaxInt64 { return fmt.Errorf("invalid maxBytes: %d", maxBytes) } diff --git a/pkg/agent/internal/utilio/io_test.go b/pkg/agent/internal/utilio/io_test.go new file mode 100644 index 000000000..afd3474e2 --- /dev/null +++ b/pkg/agent/internal/utilio/io_test.go @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package utilio + +import ( + "math" + "strings" + "testing" +) + +func TestInstallFileWithLimitedSizeRejectsOverflowingLimit(t *testing.T) { + t.Parallel() + + path := t.TempDir() + "/installed" + if err := InstallFileWithLimitedSize(path, strings.NewReader("content"), 0o600, math.MaxInt64); err == nil { + t.Fatal("InstallFileWithLimitedSize error = nil") + } +} From 461e7390991b0cceed44d555628afc7487dac5b8 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 18:57:32 +0000 Subject: [PATCH 12/20] agent: assume secure installer dependencies are valid --- pkg/agent/agentbinary/upgrade.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index f48d37571..7d9127178 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -163,10 +163,6 @@ func SecureInstallAndSwitch( paths Layout, opts SecureInstallOptions, ) (SwitchResult, error) { - if log == nil { - return SwitchResult{}, fmt.Errorf("logger is nil") - } - if err := ValidateLayout(paths); err != nil { return SwitchResult{}, err } From 075694a4c5213ef90d0a6966b8b89647118045c4 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 19:22:10 +0000 Subject: [PATCH 13/20] agent: reuse bounded archive upgrade flow --- cmd/agent/internal/daemon/agentupgrade.go | 11 +- .../internal/daemon/agentupgrade_test.go | 8 +- cmd/agent/internal/daemon/nodeoperator.go | 2 +- .../app/machine_operation_aliases.go | 14 +- .../app/machine_operation_create.go | 10 +- .../app/machine_operation_e2e_test.go | 7 +- .../app/machine_operation_test.go | 7 +- designs/agent-upgrade.md | 16 +- .../guides/operations/agent-operations.md | 6 +- docs/content/reference/cli.md | 4 +- docs/content/reference/machina-crd.md | 2 +- hack/agent/e2e-kind/e2e.py | 36 +--- pkg/agent/agentbinary/agentbinary.go | 103 +++++------- pkg/agent/agentbinary/agentbinary_test.go | 34 ++++ pkg/agent/agentbinary/upgrade.go | 158 +++++++++++------- pkg/agent/agentbinary/upgrade_test.go | 78 +++++---- 16 files changed, 262 insertions(+), 234 deletions(-) diff --git a/cmd/agent/internal/daemon/agentupgrade.go b/cmd/agent/internal/daemon/agentupgrade.go index b2a3de143..a1ca805dd 100644 --- a/cmd/agent/internal/daemon/agentupgrade.go +++ b/cmd/agent/internal/daemon/agentupgrade.go @@ -57,11 +57,7 @@ func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) } - if request.sha256 == "" { - return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeSHA256Parameter) - } - - if err := agentbinary.ValidateSecureInstallOptions(agentbinary.SecureInstallOptions{ + if err := agentbinary.ValidateInstallOptions(agentbinary.InstallOptions{ DownloadURL: request.downloadURL, ExpectedSHA256: request.sha256, ExpectedMember: goalstates.AgentUpgradeBinaryName, @@ -72,7 +68,7 @@ func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest return request, nil } -func upgradeDaemonBinarySecure(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { +func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { paths, err := goalstates.ResolvedAgentUpgradePaths() if err != nil { return fmt.Errorf("resolve current daemon binary symlink: %w", err) @@ -85,11 +81,12 @@ func upgradeDaemonBinarySecure(ctx context.Context, log *slog.Logger, request ag CurrentPath: paths.CurrentPath, LastGoodPath: paths.LastGoodPath, } - _, err = agentbinary.SecureInstallAndSwitch(ctx, log, layout, agentbinary.SecureInstallOptions{ + _, err = agentbinary.InstallAndSwitchFromTarGzWithOptions(ctx, log, layout, agentbinary.InstallOptions{ DownloadURL: request.downloadURL, ExpectedSHA256: request.sha256, ExpectedMember: goalstates.AgentUpgradeBinaryName, Mode: agentUpgradeBinaryMode, + ExactMember: true, }) return err diff --git a/cmd/agent/internal/daemon/agentupgrade_test.go b/cmd/agent/internal/daemon/agentupgrade_test.go index 25dd0fa4e..bf52cebb9 100644 --- a/cmd/agent/internal/daemon/agentupgrade_test.go +++ b/cmd/agent/internal/daemon/agentupgrade_test.go @@ -29,11 +29,11 @@ func TestParseAgentUpgradeRequest(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), agentUpgradeDownloadURLParameter) - _, err = parseAgentUpgradeRequest(map[string]string{ - agentUpgradeDownloadURLParameter: "https://example.com/agent.tar.gz", + request, err = parseAgentUpgradeRequest(map[string]string{ + agentUpgradeDownloadURLParameter: "http://example.com/agent.tar.gz", }) - require.Error(t, err) - assert.Contains(t, err.Error(), agentUpgradeSHA256Parameter) + require.NoError(t, err) + assert.Empty(t, request.sha256) } func TestAgentUpgradeSignalOperatorRecordFailure(t *testing.T) { diff --git a/cmd/agent/internal/daemon/nodeoperator.go b/cmd/agent/internal/daemon/nodeoperator.go index ec1602223..e56804ae9 100644 --- a/cmd/agent/internal/daemon/nodeoperator.go +++ b/cmd/agent/internal/daemon/nodeoperator.go @@ -253,7 +253,7 @@ func (nspawnNodeOperator) RepaveNode( } func (nspawnNodeOperator) StageAgentUpgrade(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { - return upgradeDaemonBinarySecure(ctx, log, request) + return upgradeDaemonBinary(ctx, log, request) } func (nspawnNodeOperator) RestartAgentDaemon(ctx context.Context, log *slog.Logger) error { diff --git a/cmd/kubectl-unbounded/app/machine_operation_aliases.go b/cmd/kubectl-unbounded/app/machine_operation_aliases.go index 66780a889..e68636f3b 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_aliases.go +++ b/cmd/kubectl-unbounded/app/machine_operation_aliases.go @@ -84,8 +84,8 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { "agent-upgrade", ) - cmd.Flags().StringVar(&downloadURL, "download-url", "", "HTTPS URL of the unbounded-agent release tarball") - cmd.Flags().StringVar(&sha256Digest, "sha256", "", "SHA-256 digest of the release tarball") + cmd.Flags().StringVar(&downloadURL, "download-url", "", "HTTP or HTTPS URL of the unbounded-agent release tarball") + cmd.Flags().StringVar(&sha256Digest, "sha256", "", "Optional SHA-256 digest of the release tarball") oldRunE := cmd.RunE cmd.RunE = func(cmd *cobra.Command, args []string) error { @@ -93,14 +93,12 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { return fmt.Errorf("--download-url is required") } - if sha256Digest == "" { - return fmt.Errorf("--sha256 is required") + parameters := map[string]string{"downloadURL": downloadURL} + if sha256Digest != "" { + parameters["sha256"] = sha256Digest } - cmd.SetContext(context.WithValue(cmd.Context(), machineOperationParametersKey{}, map[string]string{ - "downloadURL": downloadURL, - "sha256": sha256Digest, - })) + cmd.SetContext(context.WithValue(cmd.Context(), machineOperationParametersKey{}, parameters)) return oldRunE(cmd, args) } diff --git a/cmd/kubectl-unbounded/app/machine_operation_create.go b/cmd/kubectl-unbounded/app/machine_operation_create.go index a29145015..e3be20d0c 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_create.go +++ b/cmd/kubectl-unbounded/app/machine_operation_create.go @@ -226,14 +226,8 @@ func (o *machineOperationCreateOptions) validate() error { return err } - if o.kind == v1alpha3.OperationAgentUpgrade { - if parameters["downloadURL"] == "" { - return fmt.Errorf("AgentUpgrade requires --param downloadURL=") - } - - if parameters["sha256"] == "" { - return fmt.Errorf("AgentUpgrade requires --param sha256=") - } + if o.kind == v1alpha3.OperationAgentUpgrade && parameters["downloadURL"] == "" { + return fmt.Errorf("AgentUpgrade requires --param downloadURL=") } return nil diff --git a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go index df5eca36f..ea0cf76e3 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go @@ -104,8 +104,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { ctx, rt, "machine", "agent-upgrade", "worker-01", "--operation-name", "worker-01-agent-upgrade", - "--download-url", "https://example.com/new-agent.tar.gz", - "--sha256", testAgentUpgradeSHA256, + "--download-url", "http://example.com/new-agent.tar.gz", "--wait=false", ) require.NoError(t, err) @@ -113,8 +112,8 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { assertMachineOperation(t, ctx, c, "worker-01-agent-upgrade", func(op v1alpha3.MachineOperation) { require.Equal(t, "worker-01", op.Spec.MachineRef) require.Equal(t, v1alpha3.OperationAgentUpgrade, op.Spec.OperationKind) - require.Equal(t, "https://example.com/new-agent.tar.gz", op.Spec.Parameters["downloadURL"]) - require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) + require.Equal(t, "http://example.com/new-agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.NotContains(t, op.Spec.Parameters, "sha256") require.NotNil(t, op.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(defaultTTLSeconds), *op.Spec.TTLSecondsAfterFinished) require.Len(t, op.OwnerReferences, 1) diff --git a/cmd/kubectl-unbounded/app/machine_operation_test.go b/cmd/kubectl-unbounded/app/machine_operation_test.go index 9ca1ad604..4b2220d43 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_test.go @@ -129,14 +129,13 @@ func TestValidateAgentUpgradeRequiresSHA256(t *testing.T) { name: "upgrade-worker-01", kind: v1alpha3.OperationAgentUpgrade, machine: "worker-01", - parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + parameterArgs: []string{"downloadURL=http://example.com/agent.tar.gz"}, output: operationOutputName, dryRun: dryRunNone, } - err := opts.validate() - require.Error(t, err) - require.Contains(t, err.Error(), "sha256") + // SHA-256 is optional when the operation source is trusted. + require.NoError(t, opts.validate()) } func TestValidateWaitRejectsStructuredOutput(t *testing.T) { diff --git a/designs/agent-upgrade.md b/designs/agent-upgrade.md index 1b58b1d34..39e49a75f 100644 --- a/designs/agent-upgrade.md +++ b/designs/agent-upgrade.md @@ -62,7 +62,7 @@ Pending MachineOperation v Validate parameters | - +-- missing URL/digest or non-HTTPS URL ---------> Failed + +-- missing/invalid HTTP(S) URL -----------------> Failed | v Mark InProgress @@ -96,14 +96,14 @@ Old process exits, new daemon starts ## Staging and switching -The daemon reads `spec.parameters["downloadURL"]` and +The daemon reads `spec.parameters["downloadURL"]` and the optional `spec.parameters["sha256"]` from the `MachineOperation`, resolves the current -binary target, and calls `agentbinary.SecureInstallAndSwitch`. Logs and errors -omit URL query and fragment data. +binary target, and calls `agentbinary.InstallAndSwitchFromTarGzWithOptions`. +Logs and errors omit URL query and fragment data. -`SecureInstallAndSwitch` performs the upgrade as one logical operation: +`InstallAndSwitchFromTarGzWithOptions` performs the upgrade as one logical operation: -1. Require an HTTPS URL and an exact compressed-archive SHA-256. +1. Require an HTTP or HTTPS URL and verify the compressed-archive SHA-256 when provided. 2. Download the tarball within the configured size bound. 3. Require the archive to contain only the exact `unbounded-agent` entry. 4. Bound decompression and atomically install the inactive slot. @@ -168,8 +168,8 @@ startup signal path. | Failure | Operation status | Binary state | |---------|------------------|--------------| -| Missing `downloadURL` or `sha256` | `Failed`, `InvalidParameters` | No link changes. | -| Non-HTTPS URL or digest mismatch | `Failed`, `InvalidParameters` or `ExecutionFailed` | No current link change. | +| Missing `downloadURL` | `Failed`, `InvalidParameters` | No link changes. | +| Unsupported URL or digest mismatch | `Failed`, `InvalidParameters` or `ExecutionFailed` | No current link change. | | Download or extraction failure | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | | Empty archive entry | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | | Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current remains unchanged. A distinct last-good target remains unchanged. | diff --git a/docs/content/guides/operations/agent-operations.md b/docs/content/guides/operations/agent-operations.md index f0c05aa52..0e708c6f5 100644 --- a/docs/content/guides/operations/agent-operations.md +++ b/docs/content/guides/operations/agent-operations.md @@ -10,8 +10,8 @@ handled by the agent itself. ## AgentUpgrade Replaces the host agent binary using blue-green staging with automatic rollback. -The operation requires a `downloadURL` parameter pointing to an HTTPS agent -release tarball and a `sha256` parameter containing the compressed archive digest. +The operation requires a `downloadURL` parameter pointing to an HTTP or HTTPS +agent release tarball. An optional `sha256` parameter verifies the compressed archive digest. ```yaml apiVersion: unbounded-cloud.io/v1alpha3 @@ -45,7 +45,7 @@ active at a time. **Staging:** -1. Downloads the release tarball from `downloadURL` and verifies `sha256`. +1. Downloads the release tarball from `downloadURL` and verifies `sha256` when provided. 2. Requires the bounded archive to contain only the exact `unbounded-agent` member. 3. Runs `unbounded-agent version` against the staged binary as a binary validation check. If this fails, the operation is marked `Failed` and the diff --git a/docs/content/reference/cli.md b/docs/content/reference/cli.md index 3da8b07a5..7d75d8779 100644 --- a/docs/content/reference/cli.md +++ b/docs/content/reference/cli.md @@ -319,7 +319,7 @@ Create a `MachineOperation`. | `--kind` | string | Operation kind: `NodeReboot`, `AgentUpgrade`, `AgentReset`, `HostReboot`, `HostPowerOff`, `HostPowerOn`, or `HostReplace` | | `--machine` or `--selector` | string | Target one Machine by name or select Machines by label selector | -`AgentUpgrade` also requires `--param downloadURL=` and `--param sha256=`. +`AgentUpgrade` requires `--param downloadURL=` and optionally accepts `--param sha256=`. #### Optional Flags @@ -412,7 +412,7 @@ name, default `--ttl 300`, and `--wait=true`. | `kubectl unbounded machine host-reboot NAME` | `HostReboot` | Reboots or power-cycles the host through the owning backend. | | `kubectl unbounded machine power-off NAME` | `HostPowerOff` | Powers off the host. | | `kubectl unbounded machine power-on NAME` | `HostPowerOn` | Powers on the host. | -| `kubectl unbounded machine agent-upgrade NAME --download-url URL --sha256 DIGEST` | `AgentUpgrade` | Upgrades the host-side agent binary. | +| `kubectl unbounded machine agent-upgrade NAME --download-url URL [--sha256 DIGEST]` | `AgentUpgrade` | Upgrades the host-side agent binary. | | `kubectl unbounded machine agent-reset NAME --force` | `AgentReset` | Removes the agent and managed resources from the host. Requires confirmation unless `--force` is set. | | `kubectl unbounded machine replace NAME --force` | `HostReplace` | Destructively replaces the host. Requires confirmation unless `--force` is set. | diff --git a/docs/content/reference/machina-crd.md b/docs/content/reference/machina-crd.md index beebe2153..1ff031a8b 100644 --- a/docs/content/reference/machina-crd.md +++ b/docs/content/reference/machina-crd.md @@ -196,7 +196,7 @@ spec: | `status.targets` | []TargetStatus | No | Per-Machine target status snapshot used by host operation controllers. | | `status.conditions` | []Condition | No | Operation conditions. `Completed` tracks terminal state. `BootLoaderDownloaded=True` is latched by metalman when a target first downloads the initial PXE boot loader, usually over TFTP. `BootImageWritten` starts as `Unknown` for metalman `HostReplace`, transitions to `False` when the PXE installer requests `disk.img.gz`, and transitions to `True` when the existing `/pxe/disable` completion signal is received. `CloudInitDone` starts as `Unknown`, transitions to `False` when first-boot cloud-init starts, and transitions to `True` on final cloud-init success or `False` with reason `Failed` and a summarized error when cloud-init reports a failure. | -`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL` and `spec.parameters.sha256`. The URL must be HTTPS and point to an `unbounded-agent` release tarball; `sha256` is the expected digest of the compressed archive. The agent verifies the archive, stages its exact `unbounded-agent` member as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. +`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL`. The URL may use HTTP or HTTPS and must point to an `unbounded-agent` release tarball. The optional `spec.parameters.sha256` is the expected digest of the compressed archive. The agent verifies the digest when provided, stages the archive's exact `unbounded-agent` member as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. The Azure VM provider handles: diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index a4f5648ac..44e73902b 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -62,7 +62,6 @@ import re import secrets import shutil -import ssl import subprocess import sys import textwrap @@ -1175,44 +1174,15 @@ def wait_for_daemon_active(timeout_secs: int = 180) -> None: die(f"Timed out waiting for daemon to become active; last status={last_status!r}") -_agent_upgrade_certificate_ready = False - - def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_complete: bool = True) -> dict[str, Any]: """Serve *tarball* to the VM, create AgentUpgrade, and wait for it.""" - global _agent_upgrade_certificate_ready - runner_ip = VM_GATEWAY - agent_url = f"https://{runner_ip}:{SERVE_PORT}/{tarball.name}" + agent_url = f"http://{runner_ip}:{SERVE_PORT}/{tarball.name}" digest = hashlib.sha256(tarball.read_bytes()).hexdigest() - cert_path = tarball.parent / "agent-upgrade-e2e.crt" - key_path = tarball.parent / "agent-upgrade-e2e.key" - if not _agent_upgrade_certificate_ready: - run([ - "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1", - "-subj", f"/CN={runner_ip}", "-addext", f"subjectAltName=IP:{runner_ip}", - "-keyout", str(key_path), "-out", str(cert_path), - ]) - scp_cmd(str(cert_path), f"{SSH_TARGET}:/tmp/agent-upgrade-e2e.crt") - ssh_cmd( - "if command -v update-ca-certificates >/dev/null 2>&1; then " - "sudo cp /tmp/agent-upgrade-e2e.crt /usr/local/share/ca-certificates/agent-upgrade-e2e.crt && " - "sudo update-ca-certificates >/dev/null; " - "else sudo cp /tmp/agent-upgrade-e2e.crt /etc/pki/ca-trust/source/anchors/agent-upgrade-e2e.crt && " - "sudo update-ca-trust; fi" - ) - # Go loads system roots lazily and may cache them for the process lifetime. - # Restart once after installing the test CA, before any upgrade candidate runs. - ssh_cmd("sudo systemctl restart unbounded-agent-daemon.service") - wait_for_daemon_active() - _agent_upgrade_certificate_ready = True - log(f"Starting HTTPS file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") + log(f"Starting HTTP file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") handler = _make_handler(str(tarball.parent)) httpd = HTTPServer((runner_ip, SERVE_PORT), handler) - tls_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) - tls_context.load_cert_chain(certfile=cert_path, keyfile=key_path) - httpd.socket = tls_context.wrap_socket(httpd.socket, server_side=True) server_thread = Thread(target=httpd.serve_forever, daemon=True) server_thread.start() try: @@ -3725,7 +3695,7 @@ def validate_agent_upgrade_rollback() -> None: broken_tarball, broken_operation_name, expect_complete=False) broken_status = broken_operation.get("status", {}) log(f"Broken AgentUpgrade failure reason: {broken_status.get('reason')!r}") - if "verify upgraded agent binary" not in broken_status.get("message", ""): + if "verify agent binary" not in broken_status.get("message", ""): die(f"unexpected broken AgentUpgrade failure message: {broken_status.get('message')!r}") if read_daemon_current_target() != previous_good: die("broken AgentUpgrade changed current daemon binary symlink") diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index b2834e811..f8f2275a6 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// Package agentbinary installs and switches verified agent binaries from release archives. +// Package agentbinary installs and switches agent binaries from release archives. package agentbinary import ( @@ -9,7 +9,6 @@ import ( "errors" "fmt" "log/slog" - "net/url" "os" "os/exec" "path/filepath" @@ -25,45 +24,14 @@ const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 -// InstallFromTarGz downloads a remote .tar.gz archive and installs binaryName -// from it to targetPath. It is retained for the legacy Unbounded upgrade -// contract, which does not provide an archive digest. New managed upgrade -// callers should use SecureInstallAndSwitch. +// InstallFromTarGz downloads a bounded HTTP or HTTPS .tar.gz archive, installs +// binaryName to targetPath, and verifies the installed binary. func InstallFromTarGz(ctx context.Context, downloadURL, targetPath, binaryName string, perm os.FileMode) error { - parsedURL, err := url.Parse(downloadURL) - if err != nil { - return fmt.Errorf("parse download URL %q: %w", downloadURL, err) - } - - if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { - return fmt.Errorf("unsupported agent download URL scheme %q", parsedURL.Scheme) - } - - for tarFile, err := range utilio.DecompressTarGzFromRemote(ctx, downloadURL) { - if err != nil { - return err - } - - if filepath.Base(tarFile.Name) != binaryName { - continue - } - - if tarFile.Size == 0 { - return fmt.Errorf("agent binary %q in archive %q is empty", binaryName, downloadURL) - } - - if err := utilio.InstallFile(targetPath, tarFile.Body, perm); err != nil { - return fmt.Errorf("install %s from %q: %w", binaryName, downloadURL, err) - } - - if err := Verify(ctx, targetPath); err != nil { - return err - } - - return nil - } - - return fmt.Errorf("agent binary %q not found in archive %q", binaryName, downloadURL) + return installFromTarGz(ctx, targetPath, InstallOptions{ + DownloadURL: downloadURL, + ExpectedMember: binaryName, + Mode: perm, + }) } // InstallFromFile installs a local agent binary to targetPath. @@ -87,23 +55,20 @@ func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error } // InstallAndSwitchFromTarGz installs the next agent binary and switches daemon links. -// It is retained for the legacy Unbounded upgrade contract. New managed upgrade -// callers should use SecureInstallAndSwitch. func InstallAndSwitchFromTarGz(ctx context.Context, downloadURL string, paths goalstates.AgentUpgradePaths, perm os.FileMode) error { - targetPath := paths.NextTargetPath() - if err := InstallFromTarGz(ctx, downloadURL, targetPath, goalstates.AgentUpgradeBinaryName, perm); err != nil { - return fmt.Errorf("install upgraded daemon binary to %s: %w", targetPath, err) - } - - if err := utilio.UpdateSymlink(paths.LastGoodPath, paths.CurrentTargetPath); err != nil { - return fmt.Errorf("update last-good daemon symlink: %w", err) - } - - if err := utilio.UpdateSymlink(paths.CurrentPath, targetPath); err != nil { - return fmt.Errorf("update current daemon symlink: %w", err) - } - - return nil + _, err := InstallAndSwitchFromTarGzWithOptions(ctx, slog.Default(), Layout{ + BinaryPath: paths.BinaryPath, + BluePath: paths.BluePath, + GreenPath: paths.GreenPath, + CurrentPath: paths.CurrentPath, + LastGoodPath: paths.LastGoodPath, + }, InstallOptions{ + DownloadURL: downloadURL, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + Mode: perm, + }) + + return err } // EnsureDaemonBinaryLinks initializes daemon current, last-good, and @@ -181,7 +146,12 @@ func Verify(ctx context.Context, path string) error { defer cancel() for { - output, err := exec.CommandContext(verifyCtx, path, "version").CombinedOutput() + output := cappedOutput{remaining: 4 << 10} + cmd := exec.CommandContext(verifyCtx, path, "version") + cmd.Stdout = &output + cmd.Stderr = &output + + err := cmd.Run() if err == nil { return nil } @@ -195,7 +165,7 @@ func Verify(ctx context.Context, path string) error { } } - details := strings.TrimSpace(string(output)) + details := strings.TrimSpace(output.String()) if details != "" { return fmt.Errorf("verify agent binary %s: %w: %s", path, err, details) } @@ -203,3 +173,20 @@ func Verify(ctx context.Context, path string) error { return fmt.Errorf("verify agent binary %s: %w", path, err) } } + +type cappedOutput struct { + strings.Builder + remaining int +} + +func (w *cappedOutput) Write(data []byte) (int, error) { + originalLength := len(data) + if len(data) > w.remaining { + data = data[:w.remaining] + } + + _, _ = w.Builder.Write(data) + w.remaining -= len(data) + + return originalLength, nil +} diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index 7623070fe..3929b97b1 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -58,6 +58,40 @@ func TestInstallFromTarGzVerifiesInstalledBinary(t *testing.T) { } } +func TestInstallAndSwitchFromTarGz(t *testing.T) { + t.Parallel() + + paths := setupDaemonBinaryTestPaths(t) + + release := testAgentScript("release", 0) + if err := os.WriteFile(paths.BinaryPath, testAgentScript("current", 0), 0o755); err != nil { + t.Fatalf("write current binary: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current binary: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good binary: %v", err) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if err := writeTestAgentArchive(w, release); err != nil { + t.Errorf("write archive: %v", err) + } + })) + t.Cleanup(server.Close) + + if err := InstallAndSwitchFromTarGz(t.Context(), server.URL, paths, 0o755); err != nil { + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) + } + + assertSymlinkTarget(t, paths.CurrentPath, paths.BluePath) + assertSymlinkTarget(t, paths.LastGoodPath, paths.BinaryPath) + assertFileContent(t, paths.BluePath, string(release)) +} + func TestInstallFromTarGzRejectsUnsupportedScheme(t *testing.T) { t.Parallel() diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index 7d9127178..ae62dc941 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -17,7 +17,6 @@ import ( "net/http" "net/url" "os" - "os/exec" "path/filepath" "strings" "time" @@ -40,8 +39,8 @@ type Layout struct { LastGoodPath string } -// SecureInstallOptions configures a verified agent release archive install. -type SecureInstallOptions struct { +// InstallOptions configures a bounded agent release archive install. +type InstallOptions struct { DownloadURL string ExpectedSHA256 string ExpectedMember string @@ -49,6 +48,7 @@ type SecureInstallOptions struct { MaxArchiveBytes int64 MaxExtractedBytes int64 HTTPClient *http.Client + ExactMember bool } // SwitchResult describes a completed blue-green binary switch. @@ -57,50 +57,56 @@ type SwitchResult struct { CurrentPath string } -type normalizedSecureInstallOptions struct { - options SecureInstallOptions +type normalizedInstallOptions struct { + options InstallOptions parsedURL *url.URL expectedDigest [sha256.Size]byte + verifyDigest bool } -// ValidateSecureInstallOptions validates caller-provided secure install inputs. -func ValidateSecureInstallOptions(opts SecureInstallOptions) error { - _, err := normalizeSecureInstallOptions(opts) +// ValidateInstallOptions validates caller-provided install inputs. +func ValidateInstallOptions(opts InstallOptions) error { + _, err := normalizeInstallOptions(opts) return err } -func normalizeSecureInstallOptions(opts SecureInstallOptions) (normalizedSecureInstallOptions, error) { - parsedURL, err := validateSecureDownloadURL(opts.DownloadURL) +func normalizeInstallOptions(opts InstallOptions) (normalizedInstallOptions, error) { + parsedURL, err := validateDownloadURL(opts.DownloadURL) if err != nil { - return normalizedSecureInstallOptions{}, err + return normalizedInstallOptions{}, err } - expectedDigest, err := parseSHA256(opts.ExpectedSHA256) - if err != nil { - return normalizedSecureInstallOptions{}, err + var expectedDigest [sha256.Size]byte + + verifyDigest := strings.TrimSpace(opts.ExpectedSHA256) != "" + if verifyDigest { + expectedDigest, err = parseSHA256(opts.ExpectedSHA256) + if err != nil { + return normalizedInstallOptions{}, err + } } opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) if opts.ExpectedMember == "" || opts.ExpectedMember == "." || opts.ExpectedMember == ".." || filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { - return normalizedSecureInstallOptions{}, fmt.Errorf("expected archive member must be an exact base name without a path prefix") + return normalizedInstallOptions{}, fmt.Errorf("expected archive member must be an exact base name without a path prefix") } if opts.Mode != 0 && opts.Mode.Perm() != opts.Mode { - return normalizedSecureInstallOptions{}, fmt.Errorf("agent binary mode must contain permission bits only") + return normalizedInstallOptions{}, fmt.Errorf("agent binary mode must contain permission bits only") } if opts.MaxArchiveBytes < 0 || opts.MaxExtractedBytes < 0 { - return normalizedSecureInstallOptions{}, fmt.Errorf("agent archive size limits must not be negative") + return normalizedInstallOptions{}, fmt.Errorf("agent archive size limits must not be negative") } if opts.MaxArchiveBytes > math.MaxInt64-1 { - return normalizedSecureInstallOptions{}, fmt.Errorf("maximum archive size is too large") + return normalizedInstallOptions{}, fmt.Errorf("maximum archive size is too large") } if opts.MaxExtractedBytes > math.MaxInt64/2 { - return normalizedSecureInstallOptions{}, fmt.Errorf("maximum extracted size is too large") + return normalizedInstallOptions{}, fmt.Errorf("maximum extracted size is too large") } if opts.Mode == 0 { @@ -115,12 +121,13 @@ func normalizeSecureInstallOptions(opts SecureInstallOptions) (normalizedSecureI opts.MaxExtractedBytes = defaultMaxBinaryBytes } - opts.HTTPClient = secureHTTPClient(opts.HTTPClient) + opts.HTTPClient = boundedHTTPClient(opts.HTTPClient) - return normalizedSecureInstallOptions{ + return normalizedInstallOptions{ options: opts, parsedURL: parsedURL, expectedDigest: expectedDigest, + verifyDigest: verifyDigest, }, nil } @@ -153,21 +160,49 @@ func ValidateLayout(paths Layout) error { return nil } -// SecureInstallAndSwitch downloads and verifies an HTTPS release archive, -// installs its only member into the inactive slot, and atomically updates the -// last-good and current links. The archive member must exactly equal the -// configured base name; path prefixes such as "./" are intentionally rejected. -func SecureInstallAndSwitch( +func installFromTarGz(ctx context.Context, targetPath string, opts InstallOptions) error { + normalized, err := normalizeInstallOptions(opts) + if err != nil { + return err + } + + opts = normalized.options + + archivePath, err := downloadArchive( + ctx, + opts.HTTPClient, + normalized.parsedURL, + normalized.expectedDigest, + normalized.verifyDigest, + opts.MaxArchiveBytes, + ) + if err != nil { + return err + } + defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup + + if err := extractOnlyArchiveMember(archivePath, targetPath, opts); err != nil { + return err + } + + return Verify(ctx, targetPath) +} + +// InstallAndSwitchFromTarGzWithOptions downloads a bounded HTTP or HTTPS release +// archive, installs the configured member into the inactive slot, and atomically +// updates the last-good and current links. When ExactMember is set, the archive +// must contain only the exact configured base name. +func InstallAndSwitchFromTarGzWithOptions( ctx context.Context, log *slog.Logger, paths Layout, - opts SecureInstallOptions, + opts InstallOptions, ) (SwitchResult, error) { if err := ValidateLayout(paths); err != nil { return SwitchResult{}, err } - normalized, err := normalizeSecureInstallOptions(opts) + normalized, err := normalizeInstallOptions(opts) if err != nil { return SwitchResult{}, err } @@ -175,6 +210,7 @@ func SecureInstallAndSwitch( opts = normalized.options parsedURL := normalized.parsedURL expectedDigest := normalized.expectedDigest + verifyDigest := normalized.verifyDigest previousPath, err := executablePath(paths.CurrentPath) if err != nil { @@ -191,7 +227,7 @@ func SecureInstallAndSwitch( targetPath = paths.GreenPath } - archivePath, err := downloadVerifiedArchive(ctx, opts.HTTPClient, parsedURL, expectedDigest, opts.MaxArchiveBytes) + archivePath, err := downloadArchive(ctx, opts.HTTPClient, parsedURL, expectedDigest, verifyDigest, opts.MaxArchiveBytes) if err != nil { return SwitchResult{}, err } @@ -222,7 +258,7 @@ func SecureInstallAndSwitch( return SwitchResult{}, err } - if err := verifyQuiet(ctx, targetPath); err != nil { + if err := Verify(ctx, targetPath); err != nil { return SwitchResult{}, err } @@ -258,14 +294,18 @@ func RedactedURL(parsedURL *url.URL) string { return redacted.String() } -func validateSecureDownloadURL(rawURL string) (*url.URL, error) { +func validateDownloadURL(rawURL string) (*url.URL, error) { parsedURL, err := url.ParseRequestURI(strings.TrimSpace(rawURL)) if err != nil { return nil, fmt.Errorf("invalid download URL") } - if parsedURL.Scheme != "https" || parsedURL.Host == "" || parsedURL.User != nil || parsedURL.Fragment != "" { - return nil, fmt.Errorf("download URL must use HTTPS, include a host, omit user information, and omit fragments") + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return nil, fmt.Errorf("unsupported agent download URL scheme %q", parsedURL.Scheme) + } + + if parsedURL.Host == "" || parsedURL.User != nil || parsedURL.Fragment != "" { + return nil, fmt.Errorf("download URL must include a host, omit user information, and omit fragments") } return parsedURL, nil @@ -287,7 +327,7 @@ func parseSHA256(value string) ([sha256.Size]byte, error) { return expected, nil } -func secureHTTPClient(base *http.Client) *http.Client { +func boundedHTTPClient(base *http.Client) *http.Client { if base == nil { base = &http.Client{Timeout: 10 * time.Minute} } @@ -295,8 +335,9 @@ func secureHTTPClient(base *http.Client) *http.Client { client := *base originalCheckRedirect := client.CheckRedirect client.CheckRedirect = func(req *http.Request, via []*http.Request) error { - if req.URL.Scheme != "https" { - return fmt.Errorf("redirect to non-HTTPS URL is not allowed") + if (req.URL.Scheme != "http" && req.URL.Scheme != "https") || req.URL.Host == "" || + req.URL.User != nil || req.URL.Fragment != "" { + return fmt.Errorf("redirect URL must use HTTP or HTTPS, include a host, omit user information, and omit fragments") } if originalCheckRedirect != nil { @@ -313,11 +354,12 @@ func secureHTTPClient(base *http.Client) *http.Client { return &client } -func downloadVerifiedArchive( +func downloadArchive( ctx context.Context, client *http.Client, parsedURL *url.URL, expected [sha256.Size]byte, + verifyDigest bool, maxBytes int64, ) (string, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, parsedURL.String(), http.NoBody) @@ -370,7 +412,7 @@ func downloadVerifiedArchive( return "", fmt.Errorf("agent archive exceeds %d-byte limit", maxBytes) } - if !equalDigest(hasher.Sum(nil), expected[:]) { + if verifyDigest && !equalDigest(hasher.Sum(nil), expected[:]) { return "", fmt.Errorf("agent archive SHA-256 does not match expected digest") } @@ -383,7 +425,7 @@ func downloadVerifiedArchive( return path, nil } -func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstallOptions) (err error) { +func extractOnlyArchiveMember(archivePath, targetPath string, opts InstallOptions) (err error) { archive, err := os.Open(archivePath) if err != nil { return fmt.Errorf("open agent archive: %w", err) @@ -423,14 +465,19 @@ func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstall return fmt.Errorf("read agent archive: %w", nextErr) } - if !safeArchiveName(header.Name) { + memberName := filepath.Clean(header.Name) + if !safeArchiveName(header.Name, opts.ExactMember) { return fmt.Errorf("agent archive contains unsafe member name %q", header.Name) } - if header.Name != opts.ExpectedMember { + if opts.ExactMember && header.Name != opts.ExpectedMember { return fmt.Errorf("agent archive contains unexpected member %q", header.Name) } + if !opts.ExactMember && filepath.Base(memberName) != opts.ExpectedMember { + continue + } + if found { return fmt.Errorf("agent archive contains duplicate member %q", opts.ExpectedMember) } @@ -481,12 +528,14 @@ func extractOnlyArchiveMember(archivePath, targetPath string, opts SecureInstall return nil } -func safeArchiveName(name string) bool { - return name != "" && name != "." && name != ".." && - !filepath.IsAbs(name) && - filepath.Clean(name) == name && - !strings.Contains(name, `\`) && - !strings.HasPrefix(name, ".."+string(filepath.Separator)) +func safeArchiveName(name string, exact bool) bool { + cleaned := filepath.Clean(name) + if name == "" || cleaned == "." || cleaned == ".." || filepath.IsAbs(name) || + strings.Contains(name, `\`) || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) { + return false + } + + return !exact || cleaned == name } func pathResolvesTo(path, expected string) (bool, error) { @@ -520,21 +569,6 @@ func executablePath(path string) (string, error) { return resolved, nil } -func verifyQuiet(ctx context.Context, path string) error { - verifyCtx, cancel := context.WithTimeout(ctx, verifyTimeout) - defer cancel() - - cmd := exec.CommandContext(verifyCtx, path, "version") - cmd.Stdout = io.Discard - - cmd.Stderr = io.Discard - if err := cmd.Run(); err != nil { - return fmt.Errorf("verify upgraded agent binary: %w", err) - } - - return nil -} - type countingReader struct { reader io.Reader count int64 diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index be1cca33f..54d643787 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -21,7 +21,7 @@ import ( "testing" ) -func TestSecureInstallAndSwitch(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptions(t *testing.T) { t.Parallel() paths := secureUpgradeTestPaths(t) @@ -45,17 +45,18 @@ func TestSecureInstallAndSwitch(t *testing.T) { digest := sha256.Sum256(payload) - result, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + result, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz?sig=secret", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, Mode: 0o755, MaxArchiveBytes: 1 << 20, MaxExtractedBytes: 1 << 20, HTTPClient: server.Client(), }) if err != nil { - t.Fatalf("SecureInstallAndSwitch: %v", err) + t.Fatalf("InstallAndSwitchFromTarGzWithOptions: %v", err) } if result.PreviousPath != paths.BluePath || result.CurrentPath != paths.GreenPath { @@ -66,7 +67,7 @@ func TestSecureInstallAndSwitch(t *testing.T) { assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) } -func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsRejectsInvalidInputs(t *testing.T) { t.Parallel() paths := secureUpgradeTestPaths(t) @@ -78,16 +79,18 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { t.Fatalf("symlink current: %v", err) } - tests := map[string]SecureInstallOptions{ - "HTTP URL": { - DownloadURL: "http://example.com/agent.tar.gz", + tests := map[string]InstallOptions{ + "unsupported URL": { + DownloadURL: "ftp://example.com/agent.tar.gz", ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "custom-agent", + ExactMember: true, }, "invalid digest": { DownloadURL: "https://example.com/agent.tar.gz", ExpectedSHA256: "bad", ExpectedMember: "custom-agent", + ExactMember: true, }, "nested member": { DownloadURL: "https://example.com/agent.tar.gz", @@ -108,18 +111,21 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { DownloadURL: "https://example.com/agent.tar.gz", ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "custom-agent", + ExactMember: true, Mode: os.ModeSetuid | 0o755, }, "negative size": { DownloadURL: "https://example.com/agent.tar.gz", ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "custom-agent", + ExactMember: true, MaxArchiveBytes: -1, }, "archive size overflow": { DownloadURL: "https://example.com/agent.tar.gz", ExpectedSHA256: strings.Repeat("a", 64), ExpectedMember: "custom-agent", + ExactMember: true, MaxArchiveBytes: math.MaxInt64, }, } @@ -127,8 +133,8 @@ func TestSecureInstallAndSwitchRejectsInvalidInputs(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - if _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, opts); err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + if _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") } }) } @@ -153,7 +159,7 @@ func TestValidateLayout(t *testing.T) { } } -func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnexpectedMember(t *testing.T) { t.Parallel() paths := secureUpgradeTestPaths(t) @@ -173,10 +179,11 @@ func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { digest := sha256.Sum256(payload) - _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, HTTPClient: server.Client(), }) if err == nil || !strings.Contains(err.Error(), "unexpected member") { @@ -184,7 +191,7 @@ func TestSecureInstallAndSwitchRejectsUnexpectedMember(t *testing.T) { } } -func TestSecureInstallAndSwitchPreservesCurrentOnVerificationFailures(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsPreservesCurrentOnVerificationFailures(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -215,14 +222,15 @@ func TestSecureInstallAndSwitchPreservesCurrentOnVerificationFailures(t *testing digest = fmt.Sprintf("%x", sum) } - _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz?sig=secret", ExpectedSHA256: digest, ExpectedMember: "custom-agent", + ExactMember: true, HTTPClient: server.Client(), }) if err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") } if strings.Contains(err.Error(), "secret") { @@ -235,7 +243,7 @@ func TestSecureInstallAndSwitchPreservesCurrentOnVerificationFailures(t *testing } } -func TestSecureInstallAndSwitchPreservesDistinctLastGoodOnCandidateFailure(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsPreservesDistinctLastGoodOnCandidateFailure(t *testing.T) { t.Parallel() paths := secureUpgradeReadyPaths(t) @@ -259,36 +267,39 @@ func TestSecureInstallAndSwitchPreservesDistinctLastGoodOnCandidateFailure(t *te digest := sha256.Sum256(payload) - _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, HTTPClient: server.Client(), }) if err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BinaryPath) } -func TestSecureInstallAndSwitchEnforcesSizeLimits(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsEnforcesSizeLimits(t *testing.T) { t.Parallel() payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) digest := sha256.Sum256(payload) - tests := map[string]SecureInstallOptions{ + tests := map[string]InstallOptions{ "compressed": { ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, MaxArchiveBytes: int64(len(payload) - 1), MaxExtractedBytes: 1 << 20, }, "extracted": { ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, MaxArchiveBytes: 1 << 20, MaxExtractedBytes: 4, }, @@ -304,8 +315,8 @@ func TestSecureInstallAndSwitchEnforcesSizeLimits(t *testing.T) { opts.DownloadURL = server.URL + "/agent.tar.gz" opts.HTTPClient = server.Client() - if _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, opts); err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + if _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) @@ -313,7 +324,7 @@ func TestSecureInstallAndSwitchEnforcesSizeLimits(t *testing.T) { } } -func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnsafeAndDuplicateMembers(t *testing.T) { t.Parallel() tests := map[string][]secureTarMember{ @@ -335,14 +346,15 @@ func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { })) t.Cleanup(server.Close) - _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, HTTPClient: server.Client(), }) if err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) @@ -354,12 +366,13 @@ func TestSecureInstallAndSwitchRejectsUnsafeAndDuplicateMembers(t *testing.T) { } } -func TestSecureInstallAndSwitchRejectsHTTPRedirect(t *testing.T) { +func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { t.Parallel() paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) insecure := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte("not reached")) + _, _ = w.Write(payload) })) t.Cleanup(insecure.Close) @@ -368,17 +381,20 @@ func TestSecureInstallAndSwitchRejectsHTTPRedirect(t *testing.T) { })) t.Cleanup(secure.Close) - _, err := SecureInstallAndSwitch(t.Context(), slog.Default(), paths, SecureInstallOptions{ + digest := sha256.Sum256(payload) + + _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: secure.URL + "/agent.tar.gz", - ExpectedSHA256: strings.Repeat("0", 64), + ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", + ExactMember: true, HTTPClient: secure.Client(), }) - if err == nil { - t.Fatal("SecureInstallAndSwitch error = nil") + if err != nil { + t.Fatalf("InstallAndSwitchFromTarGzWithOptions: %v", err) } - assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) } func TestRedactedURL(t *testing.T) { From b1d0a98cb9d16fadef274a334b94616fc13627ab Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 20:04:40 +0000 Subject: [PATCH 14/20] agent: simplify archive upgrade API --- cmd/agent/internal/daemon/agentupgrade.go | 10 +----- designs/agent-upgrade.md | 4 +-- pkg/agent/agentbinary/agentbinary.go | 17 --------- pkg/agent/agentbinary/agentbinary_test.go | 13 ++++++- pkg/agent/agentbinary/upgrade.go | 27 +++++---------- pkg/agent/agentbinary/upgrade_test.go | 42 +++++++++++------------ 6 files changed, 45 insertions(+), 68 deletions(-) diff --git a/cmd/agent/internal/daemon/agentupgrade.go b/cmd/agent/internal/daemon/agentupgrade.go index a1ca805dd..2fe9d7aae 100644 --- a/cmd/agent/internal/daemon/agentupgrade.go +++ b/cmd/agent/internal/daemon/agentupgrade.go @@ -57,14 +57,6 @@ func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) } - if err := agentbinary.ValidateInstallOptions(agentbinary.InstallOptions{ - DownloadURL: request.downloadURL, - ExpectedSHA256: request.sha256, - ExpectedMember: goalstates.AgentUpgradeBinaryName, - }); err != nil { - return agentUpgradeRequest{}, err - } - return request, nil } @@ -81,7 +73,7 @@ func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, request agentUpg CurrentPath: paths.CurrentPath, LastGoodPath: paths.LastGoodPath, } - _, err = agentbinary.InstallAndSwitchFromTarGzWithOptions(ctx, log, layout, agentbinary.InstallOptions{ + _, err = agentbinary.InstallAndSwitchFromTarGz(ctx, log, layout, agentbinary.InstallOptions{ DownloadURL: request.downloadURL, ExpectedSHA256: request.sha256, ExpectedMember: goalstates.AgentUpgradeBinaryName, diff --git a/designs/agent-upgrade.md b/designs/agent-upgrade.md index 39e49a75f..c69a23dcf 100644 --- a/designs/agent-upgrade.md +++ b/designs/agent-upgrade.md @@ -98,10 +98,10 @@ Old process exits, new daemon starts The daemon reads `spec.parameters["downloadURL"]` and the optional `spec.parameters["sha256"]` from the `MachineOperation`, resolves the current -binary target, and calls `agentbinary.InstallAndSwitchFromTarGzWithOptions`. +binary target, and calls `agentbinary.InstallAndSwitchFromTarGz`. Logs and errors omit URL query and fragment data. -`InstallAndSwitchFromTarGzWithOptions` performs the upgrade as one logical operation: +`InstallAndSwitchFromTarGz` performs the upgrade as one logical operation: 1. Require an HTTP or HTTPS URL and verify the compressed-archive SHA-256 when provided. 2. Download the tarball within the configured size bound. diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index f8f2275a6..6c0d5970a 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -54,23 +54,6 @@ func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error return nil } -// InstallAndSwitchFromTarGz installs the next agent binary and switches daemon links. -func InstallAndSwitchFromTarGz(ctx context.Context, downloadURL string, paths goalstates.AgentUpgradePaths, perm os.FileMode) error { - _, err := InstallAndSwitchFromTarGzWithOptions(ctx, slog.Default(), Layout{ - BinaryPath: paths.BinaryPath, - BluePath: paths.BluePath, - GreenPath: paths.GreenPath, - CurrentPath: paths.CurrentPath, - LastGoodPath: paths.LastGoodPath, - }, InstallOptions{ - DownloadURL: downloadURL, - ExpectedMember: goalstates.AgentUpgradeBinaryName, - Mode: perm, - }) - - return err -} - // EnsureDaemonBinaryLinks initializes daemon current, last-good, and // compatibility binary links. func EnsureDaemonBinaryLinks(ctx context.Context, log *slog.Logger, paths goalstates.AgentUpgradePaths) error { diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index 3929b97b1..f9ee8a29c 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -83,7 +83,18 @@ func TestInstallAndSwitchFromTarGz(t *testing.T) { })) t.Cleanup(server.Close) - if err := InstallAndSwitchFromTarGz(t.Context(), server.URL, paths, 0o755); err != nil { + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), Layout{ + BinaryPath: paths.BinaryPath, + BluePath: paths.BluePath, + GreenPath: paths.GreenPath, + CurrentPath: paths.CurrentPath, + LastGoodPath: paths.LastGoodPath, + }, InstallOptions{ + DownloadURL: server.URL, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + Mode: 0o755, + }) + if err != nil { t.Fatalf("InstallAndSwitchFromTarGz: %v", err) } diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index ae62dc941..1b9454550 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -64,13 +64,6 @@ type normalizedInstallOptions struct { verifyDigest bool } -// ValidateInstallOptions validates caller-provided install inputs. -func ValidateInstallOptions(opts InstallOptions) error { - _, err := normalizeInstallOptions(opts) - - return err -} - func normalizeInstallOptions(opts InstallOptions) (normalizedInstallOptions, error) { parsedURL, err := validateDownloadURL(opts.DownloadURL) if err != nil { @@ -131,9 +124,7 @@ func normalizeInstallOptions(opts InstallOptions) (normalizedInstallOptions, err }, nil } -// ValidateLayout verifies that all required binary paths are clean, absolute, -// and distinct. It also validates BinaryPath when provided. -func ValidateLayout(paths Layout) error { +func validateLayout(paths Layout) error { values := []string{ paths.BluePath, paths.GreenPath, @@ -188,17 +179,17 @@ func installFromTarGz(ctx context.Context, targetPath string, opts InstallOption return Verify(ctx, targetPath) } -// InstallAndSwitchFromTarGzWithOptions downloads a bounded HTTP or HTTPS release +// InstallAndSwitchFromTarGz downloads a bounded HTTP or HTTPS release // archive, installs the configured member into the inactive slot, and atomically // updates the last-good and current links. When ExactMember is set, the archive // must contain only the exact configured base name. -func InstallAndSwitchFromTarGzWithOptions( +func InstallAndSwitchFromTarGz( ctx context.Context, log *slog.Logger, paths Layout, opts InstallOptions, ) (SwitchResult, error) { - if err := ValidateLayout(paths); err != nil { + if err := validateLayout(paths); err != nil { return SwitchResult{}, err } @@ -273,7 +264,7 @@ func InstallAndSwitchFromTarGzWithOptions( } log.Info("staged upgraded agent binary", - "url", RedactedURL(parsedURL), + "url", redactedURL(parsedURL), "previous", previousPath, "current", targetPath, ) @@ -282,7 +273,7 @@ func InstallAndSwitchFromTarGzWithOptions( } // RedactedURL removes query and fragment data that may contain credentials. -func RedactedURL(parsedURL *url.URL) string { +func redactedURL(parsedURL *url.URL) string { if parsedURL == nil { return "" } @@ -370,15 +361,15 @@ func downloadArchive( resp, err := client.Do(req) if err != nil { if ctx.Err() != nil { - return "", fmt.Errorf("download agent archive from %s: %w", RedactedURL(parsedURL), ctx.Err()) + return "", fmt.Errorf("download agent archive from %s: %w", redactedURL(parsedURL), ctx.Err()) } // Redirect and transport errors can contain credential-bearing URLs. - return "", fmt.Errorf("download agent archive from %s failed", RedactedURL(parsedURL)) + return "", fmt.Errorf("download agent archive from %s failed", redactedURL(parsedURL)) } defer resp.Body.Close() //nolint:errcheck // response body cleanup if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("download agent archive from %s: HTTP status %d", RedactedURL(parsedURL), resp.StatusCode) + return "", fmt.Errorf("download agent archive from %s: HTTP status %d", redactedURL(parsedURL), resp.StatusCode) } if resp.ContentLength > maxBytes { diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index 54d643787..68c608d34 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -45,7 +45,7 @@ func TestInstallAndSwitchFromTarGzWithOptions(t *testing.T) { digest := sha256.Sum256(payload) - result, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + result, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz?sig=secret", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", @@ -56,7 +56,7 @@ func TestInstallAndSwitchFromTarGzWithOptions(t *testing.T) { HTTPClient: server.Client(), }) if err != nil { - t.Fatalf("InstallAndSwitchFromTarGzWithOptions: %v", err) + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) } if result.PreviousPath != paths.BluePath || result.CurrentPath != paths.GreenPath { @@ -133,8 +133,8 @@ func TestInstallAndSwitchFromTarGzWithOptionsRejectsInvalidInputs(t *testing.T) t.Run(name, func(t *testing.T) { t.Parallel() - if _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, opts); err == nil { - t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") + if _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") } }) } @@ -144,17 +144,17 @@ func TestValidateLayout(t *testing.T) { t.Parallel() paths := secureUpgradeTestPaths(t) - if err := ValidateLayout(paths); err != nil { + if err := validateLayout(paths); err != nil { t.Fatalf("ValidateLayout: %v", err) } paths.BinaryPath = "" - if err := ValidateLayout(paths); err != nil { + if err := validateLayout(paths); err != nil { t.Fatalf("ValidateLayout without optional BinaryPath: %v", err) } paths.LastGoodPath = paths.CurrentPath - if err := ValidateLayout(paths); err == nil { + if err := validateLayout(paths); err == nil { t.Fatal("ValidateLayout duplicate path error = nil") } } @@ -179,7 +179,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnexpectedMember(t *testing. digest := sha256.Sum256(payload) - _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", @@ -222,7 +222,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsPreservesCurrentOnVerificationFailu digest = fmt.Sprintf("%x", sum) } - _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz?sig=secret", ExpectedSHA256: digest, ExpectedMember: "custom-agent", @@ -230,7 +230,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsPreservesCurrentOnVerificationFailu HTTPClient: server.Client(), }) if err == nil { - t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") + t.Fatal("InstallAndSwitchFromTarGz error = nil") } if strings.Contains(err.Error(), "secret") { @@ -267,7 +267,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsPreservesDistinctLastGoodOnCandidat digest := sha256.Sum256(payload) - _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", @@ -275,7 +275,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsPreservesDistinctLastGoodOnCandidat HTTPClient: server.Client(), }) if err == nil { - t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") + t.Fatal("InstallAndSwitchFromTarGz error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) @@ -315,8 +315,8 @@ func TestInstallAndSwitchFromTarGzWithOptionsEnforcesSizeLimits(t *testing.T) { opts.DownloadURL = server.URL + "/agent.tar.gz" opts.HTTPClient = server.Client() - if _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, opts); err == nil { - t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") + if _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) @@ -346,7 +346,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnsafeAndDuplicateMembers(t })) t.Cleanup(server.Close) - _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: server.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", @@ -354,7 +354,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnsafeAndDuplicateMembers(t HTTPClient: server.Client(), }) if err == nil { - t.Fatal("InstallAndSwitchFromTarGzWithOptions error = nil") + t.Fatal("InstallAndSwitchFromTarGz error = nil") } assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) @@ -383,7 +383,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { digest := sha256.Sum256(payload) - _, err := InstallAndSwitchFromTarGzWithOptions(t.Context(), slog.Default(), paths, InstallOptions{ + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ DownloadURL: secure.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", @@ -391,7 +391,7 @@ func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { HTTPClient: secure.Client(), }) if err != nil { - t.Fatalf("InstallAndSwitchFromTarGzWithOptions: %v", err) + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) } assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) @@ -400,8 +400,8 @@ func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { func TestRedactedURL(t *testing.T) { t.Parallel() - if got := RedactedURL(nil); got != "" { - t.Fatalf("RedactedURL(nil) = %q", got) + if got := redactedURL(nil); got != "" { + t.Fatalf("redactedURL(nil) = %q", got) } parsed, err := url.Parse("https://example.com/agent.tar.gz?sig=secret") @@ -409,7 +409,7 @@ func TestRedactedURL(t *testing.T) { t.Fatalf("parse URL: %v", err) } - if got := RedactedURL(parsed); got != "https://example.com/agent.tar.gz" { + if got := redactedURL(parsed); got != "https://example.com/agent.tar.gz" { t.Fatalf("RedactedURL = %q", got) } } From 427e5341eaa5305fe0f57fdbf89c1afc0cbb57bf Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 20:31:16 +0000 Subject: [PATCH 15/20] agent: remove unused archive install API --- pkg/agent/agentbinary/agentbinary.go | 10 ---------- pkg/agent/agentbinary/agentbinary_test.go | 12 ++++++++++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index 6c0d5970a..a74dfdfa7 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -24,16 +24,6 @@ const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 -// InstallFromTarGz downloads a bounded HTTP or HTTPS .tar.gz archive, installs -// binaryName to targetPath, and verifies the installed binary. -func InstallFromTarGz(ctx context.Context, downloadURL, targetPath, binaryName string, perm os.FileMode) error { - return installFromTarGz(ctx, targetPath, InstallOptions{ - DownloadURL: downloadURL, - ExpectedMember: binaryName, - Mode: perm, - }) -} - // InstallFromFile installs a local agent binary to targetPath. func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error) { source, err := os.Open(sourcePath) diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index f9ee8a29c..7aeb908f1 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -45,7 +45,11 @@ func TestInstallFromTarGzVerifiesInstalledBinary(t *testing.T) { targetPath := filepath.Join(t.TempDir(), "unbounded-agent") - err := InstallFromTarGz(context.Background(), server.URL, targetPath, "unbounded-agent", 0o755) + err := installFromTarGz(context.Background(), targetPath, InstallOptions{ + DownloadURL: server.URL, + ExpectedMember: "unbounded-agent", + Mode: 0o755, + }) if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -106,7 +110,11 @@ func TestInstallAndSwitchFromTarGz(t *testing.T) { func TestInstallFromTarGzRejectsUnsupportedScheme(t *testing.T) { t.Parallel() - err := InstallFromTarGz(context.Background(), "file:///tmp/unbounded-agent.tar.gz", filepath.Join(t.TempDir(), "agent"), "unbounded-agent", 0o755) + err := installFromTarGz(context.Background(), filepath.Join(t.TempDir(), "agent"), InstallOptions{ + DownloadURL: "file:///tmp/unbounded-agent.tar.gz", + ExpectedMember: "unbounded-agent", + Mode: 0o755, + }) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported agent download URL scheme") } From 3d9e93b1b8a3b7ca279e64f3609a4f1815ed1c0a Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 20:33:28 +0000 Subject: [PATCH 16/20] docs: clarify agent archive size units --- pkg/agent/agentbinary/upgrade.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index 1b9454550..dae41836e 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -25,8 +25,8 @@ import ( ) const ( - defaultMaxArchiveBytes = 256 << 20 - defaultMaxBinaryBytes = 256 << 20 + defaultMaxArchiveBytes = 256 << 20 // 256 MiB + defaultMaxBinaryBytes = 256 << 20 // 256 MiB ) // Layout describes caller-owned blue-green agent binary paths. From 9b4b5f1e0e2605de1d94effe1ea4f8e0eb9ea878 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 20:36:41 +0000 Subject: [PATCH 17/20] agent: make local binary install internal --- pkg/agent/agentbinary/agentbinary.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index a74dfdfa7..c89572dcd 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -24,8 +24,7 @@ const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 -// InstallFromFile installs a local agent binary to targetPath. -func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error) { +func installFromFile(sourcePath, targetPath string, perm os.FileMode) (err error) { source, err := os.Open(sourcePath) if err != nil { return fmt.Errorf("open %s: %w", sourcePath, err) @@ -106,7 +105,7 @@ func initialDaemonBinaryTarget(paths goalstates.AgentUpgradePaths) (string, erro return target, nil } - if err := InstallFromFile(paths.BinaryPath, paths.BluePath, daemonBinaryMode); err != nil { + if err := installFromFile(paths.BinaryPath, paths.BluePath, daemonBinaryMode); err != nil { return "", err } From 89ee9389ed76a1a493b791abfe04cc9ffdacefae Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 20:51:20 +0000 Subject: [PATCH 18/20] agent: address archive installer review --- pkg/agent/agentbinary/agentbinary.go | 6 +++- pkg/agent/agentbinary/agentbinary_test.go | 15 ++++++++++ pkg/agent/agentbinary/upgrade.go | 20 ++++++++----- pkg/agent/agentbinary/upgrade_test.go | 34 ++++++++++++++++++++--- 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index c89572dcd..988809061 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -20,7 +20,10 @@ import ( "github.com/Azure/unbounded/pkg/agent/internal/utilio" ) -const verifyTimeout = 30 * time.Second +const ( + verifyTimeout = 30 * time.Second + verifyWaitDelay = time.Second +) const daemonBinaryMode os.FileMode = 0o755 @@ -122,6 +125,7 @@ func Verify(ctx context.Context, path string) error { cmd := exec.CommandContext(verifyCtx, path, "version") cmd.Stdout = &output cmd.Stderr = &output + cmd.WaitDelay = verifyWaitDelay err := cmd.Run() if err == nil { diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index 7aeb908f1..c73747dff 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -17,6 +17,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -119,6 +120,20 @@ func TestInstallFromTarGzRejectsUnsupportedScheme(t *testing.T) { assert.Contains(t, err.Error(), "unsupported agent download URL scheme") } +func TestVerifyBoundsInheritedOutputWait(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "agent") + if err := os.WriteFile(path, []byte("#!/bin/sh\n(sleep 5) &\nexit 42\n"), 0o755); err != nil { + t.Fatalf("write agent: %v", err) + } + + start := time.Now() + err := Verify(t.Context(), path) + require.Error(t, err) + assert.Less(t, time.Since(start), 3*time.Second) +} + func TestEnsureDaemonBinaryLinks_InitializesFromBlue(t *testing.T) { paths := setupDaemonBinaryTestPaths(t) require.NoError(t, os.WriteFile(paths.BluePath, []byte("blue"), 0o755)) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index dae41836e..b3911778a 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -41,7 +41,9 @@ type Layout struct { // InstallOptions configures a bounded agent release archive install. type InstallOptions struct { - DownloadURL string + DownloadURL string + // ExpectedSHA256 may be empty only when the caller trusts both the archive + // source and the transport path. ExpectedSHA256 string ExpectedMember string Mode os.FileMode @@ -326,17 +328,21 @@ func boundedHTTPClient(base *http.Client) *http.Client { client := *base originalCheckRedirect := client.CheckRedirect client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if originalCheckRedirect != nil { + if err := originalCheckRedirect(req, via); err != nil { + return err + } + } else if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + if (req.URL.Scheme != "http" && req.URL.Scheme != "https") || req.URL.Host == "" || req.URL.User != nil || req.URL.Fragment != "" { return fmt.Errorf("redirect URL must use HTTP or HTTPS, include a host, omit user information, and omit fragments") } - if originalCheckRedirect != nil { - return originalCheckRedirect(req, via) - } - - if len(via) >= 10 { - return fmt.Errorf("stopped after 10 redirects") + if len(via) > 0 && via[0].URL.Scheme == "https" && req.URL.Scheme != "https" { + return fmt.Errorf("HTTPS download cannot redirect to HTTP") } return nil diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index 68c608d34..029374de4 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -376,19 +376,18 @@ func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { })) t.Cleanup(insecure.Close) - secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { http.Redirect(w, request, insecure.URL, http.StatusFound) })) - t.Cleanup(secure.Close) + t.Cleanup(redirector.Close) digest := sha256.Sum256(payload) _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ - DownloadURL: secure.URL + "/agent.tar.gz", + DownloadURL: redirector.URL + "/agent.tar.gz", ExpectedSHA256: fmt.Sprintf("%x", digest), ExpectedMember: "custom-agent", ExactMember: true, - HTTPClient: secure.Client(), }) if err != nil { t.Fatalf("InstallAndSwitchFromTarGz: %v", err) @@ -397,6 +396,33 @@ func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) } +func TestInstallAndSwitchFromTarGzRejectsHTTPSDowngrade(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + insecure := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("not reached")) + })) + t.Cleanup(insecure.Close) + + secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + http.Redirect(w, request, insecure.URL, http.StatusFound) + })) + t.Cleanup(secure.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: secure.URL + "/agent.tar.gz", + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: secure.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) +} + func TestRedactedURL(t *testing.T) { t.Parallel() From 48b083b71770eda1c17160201639c263488b6267 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 21:24:11 +0000 Subject: [PATCH 19/20] agent: harden upgrade path alias handling --- pkg/agent/agentbinary/upgrade.go | 95 +++++++++++++++++++++++---- pkg/agent/agentbinary/upgrade_test.go | 55 ++++++++++++++++ 2 files changed, 138 insertions(+), 12 deletions(-) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go index b3911778a..e2952efbb 100644 --- a/pkg/agent/agentbinary/upgrade.go +++ b/pkg/agent/agentbinary/upgrade.go @@ -143,11 +143,16 @@ func validateLayout(paths Layout) error { return fmt.Errorf("invalid agent binary path %q", value) } - if _, ok := seen[value]; ok { + canonical, err := canonicalPathEntry(value) + if err != nil { + return fmt.Errorf("resolve agent binary path %q: %w", value, err) + } + + if _, ok := seen[canonical]; ok { return fmt.Errorf("duplicate agent binary path %q", value) } - seen[value] = struct{}{} + seen[canonical] = struct{}{} } return nil @@ -226,21 +231,14 @@ func InstallAndSwitchFromTarGz( } defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup - lastGoodTarget, err := filepath.EvalSymlinks(paths.LastGoodPath) - if err != nil && !errors.Is(err, os.ErrNotExist) { + lastGoodProtected, err := symlinkReferencesPath(paths.LastGoodPath, targetPath) + if err != nil { return SwitchResult{}, fmt.Errorf("resolve last-good agent binary: %w", err) } - canonicalTargetDir, err := filepath.EvalSymlinks(filepath.Dir(targetPath)) - if err != nil && !errors.Is(err, os.ErrNotExist) { - return SwitchResult{}, fmt.Errorf("resolve inactive agent binary directory: %w", err) - } - // Protect last-good before replacing the inactive slot only when that slot // contains last-good. Otherwise, preserve the existing rollback target until - // the candidate has been staged and verified. A missing target directory - // cannot contain a last-good binary. - lastGoodProtected := err == nil && lastGoodTarget == filepath.Join(canonicalTargetDir, filepath.Base(targetPath)) + // the candidate has been staged and verified. if lastGoodProtected { if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { return SwitchResult{}, fmt.Errorf("protect current agent as last-good: %w", err) @@ -535,6 +533,79 @@ func safeArchiveName(name string, exact bool) bool { return !exact || cleaned == name } +func canonicalPathEntry(path string) (string, error) { + dir := filepath.Dir(path) + missing := make([]string, 0) + + for { + resolved, err := filepath.EvalSymlinks(dir) + if err == nil { + for i := len(missing) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, missing[i]) + } + + return filepath.Join(resolved, filepath.Base(path)), nil + } + + if !errors.Is(err, os.ErrNotExist) { + return "", err + } + + parent := filepath.Dir(dir) + if parent == dir { + return "", err + } + + missing = append(missing, filepath.Base(dir)) + dir = parent + } +} + +func symlinkReferencesPath(linkPath, targetPath string) (bool, error) { + targetEntry, err := canonicalPathEntry(targetPath) + if err != nil { + return false, err + } + + resolved, err := filepath.EvalSymlinks(linkPath) + if err == nil { + return resolved == targetEntry, nil + } + + if !errors.Is(err, os.ErrNotExist) { + return false, err + } + + info, lstatErr := os.Lstat(linkPath) + if errors.Is(lstatErr, os.ErrNotExist) { + return false, nil + } + + if lstatErr != nil { + return false, lstatErr + } + + if info.Mode()&os.ModeSymlink == 0 { + return false, nil + } + + intended, err := os.Readlink(linkPath) + if err != nil { + return false, err + } + + if !filepath.IsAbs(intended) { + intended = filepath.Join(filepath.Dir(linkPath), intended) + } + + intendedEntry, err := canonicalPathEntry(filepath.Clean(intended)) + if err != nil { + return false, err + } + + return intendedEntry == targetEntry, nil +} + func pathResolvesTo(path, expected string) (bool, error) { resolved, err := filepath.EvalSymlinks(path) if errors.Is(err, os.ErrNotExist) { diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go index 029374de4..79fa83896 100644 --- a/pkg/agent/agentbinary/upgrade_test.go +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -159,6 +159,29 @@ func TestValidateLayout(t *testing.T) { } } +func TestValidateLayoutRejectsAliasedEntries(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + + realDir := filepath.Join(t.TempDir(), "real") + if err := os.Mkdir(realDir, 0o750); err != nil { + t.Fatalf("create real directory: %v", err) + } + + aliasDir := filepath.Join(filepath.Dir(realDir), "alias") + if err := os.Symlink(realDir, aliasDir); err != nil { + t.Fatalf("create directory alias: %v", err) + } + + paths.BluePath = filepath.Join(realDir, "agent") + paths.GreenPath = filepath.Join(aliasDir, "agent") + + if err := validateLayout(paths); err == nil { + t.Fatal("validateLayout aliased path error = nil") + } +} + func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnexpectedMember(t *testing.T) { t.Parallel() @@ -243,6 +266,38 @@ func TestInstallAndSwitchFromTarGzWithOptionsPreservesCurrentOnVerificationFailu } } +func TestInstallAndSwitchFromTarGzProtectsDanglingLastGood(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + if err := os.Remove(paths.LastGoodPath); err != nil { + t.Fatalf("remove last-good link: %v", err) + } + + if err := os.Symlink(paths.GreenPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink dangling last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 42\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) +} + func TestInstallAndSwitchFromTarGzWithOptionsPreservesDistinctLastGoodOnCandidateFailure(t *testing.T) { t.Parallel() From 082aaddcb6c32ae6a1b0a34623a047380afa9449 Mon Sep 17 00:00:00 2001 From: Baichao He Date: Fri, 7 Aug 2026 21:36:46 +0000 Subject: [PATCH 20/20] agent: suppress candidate verification output --- pkg/agent/agentbinary/agentbinary.go | 36 ++--------------------- pkg/agent/agentbinary/agentbinary_test.go | 3 +- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index 988809061..7aca898a5 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -12,7 +12,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" "syscall" "time" @@ -20,10 +19,7 @@ import ( "github.com/Azure/unbounded/pkg/agent/internal/utilio" ) -const ( - verifyTimeout = 30 * time.Second - verifyWaitDelay = time.Second -) +const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 @@ -121,13 +117,7 @@ func Verify(ctx context.Context, path string) error { defer cancel() for { - output := cappedOutput{remaining: 4 << 10} - cmd := exec.CommandContext(verifyCtx, path, "version") - cmd.Stdout = &output - cmd.Stderr = &output - cmd.WaitDelay = verifyWaitDelay - - err := cmd.Run() + err := exec.CommandContext(verifyCtx, path, "version").Run() if err == nil { return nil } @@ -141,28 +131,6 @@ func Verify(ctx context.Context, path string) error { } } - details := strings.TrimSpace(output.String()) - if details != "" { - return fmt.Errorf("verify agent binary %s: %w: %s", path, err, details) - } - return fmt.Errorf("verify agent binary %s: %w", path, err) } } - -type cappedOutput struct { - strings.Builder - remaining int -} - -func (w *cappedOutput) Write(data []byte) (int, error) { - originalLength := len(data) - if len(data) > w.remaining { - data = data[:w.remaining] - } - - _, _ = w.Builder.Write(data) - w.remaining -= len(data) - - return originalLength, nil -} diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index c73747dff..cd6f48c1b 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -124,13 +124,14 @@ func TestVerifyBoundsInheritedOutputWait(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "agent") - if err := os.WriteFile(path, []byte("#!/bin/sh\n(sleep 5) &\nexit 42\n"), 0o755); err != nil { + if err := os.WriteFile(path, []byte("#!/bin/sh\nprintf 'candidate-secret\\n' >&2\n(sleep 5) &\nexit 42\n"), 0o755); err != nil { t.Fatalf("write agent: %v", err) } start := time.Now() err := Verify(t.Context(), path) require.Error(t, err) + assert.NotContains(t, err.Error(), "candidate-secret") assert.Less(t, time.Since(start), 3*time.Second) }