test: expand cmd and agent update coverage

cmd/pulse-agent/health_test.go

@@ -0,0 +1,53 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/rs/zerolog"
+)
+
+func TestHealthHandler_HealthzAlwaysOK(t *testing.T) {
+	var ready atomic.Bool
+	h := healthHandler(&ready)
+
+	req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String())
+	}
+}
+
+func TestHealthHandler_ReadyzDependsOnReadyFlag(t *testing.T) {
+	var ready atomic.Bool
+	h := healthHandler(&ready)
+
+	req := httptest.NewRequest(http.MethodGet, "/readyz", nil)
+
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if rec.Code != http.StatusServiceUnavailable {
+		t.Fatalf("expected 503, got %d", rec.Code)
+	}
+
+	ready.Store(true)
+	rec = httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", rec.Code)
+	}
+}
+
+func TestRunAsWindowsService_Stub(t *testing.T) {
+	ran, err := runAsWindowsService(Config{}, zerolog.Nop())
+	if err != nil {
+		t.Fatalf("err = %v", err)
+	}
+	if ran {
+		t.Fatalf("expected ran=false on non-windows")
+	}
+}

cmd/pulse-agent/main.go

@@ -258,7 +258,7 @@ func cleanupDockerAgent(agent *dockeragent.Agent, logger *zerolog.Logger) {
 	}
 }
 
-func startHealthServer(ctx context.Context, addr string, ready *atomic.Bool, logger *zerolog.Logger) {
+func healthHandler(ready *atomic.Bool) http.Handler {
 	mux := http.NewServeMux()
 
 	// Liveness probe - always returns 200 if server is running
@@ -280,10 +280,13 @@ func startHealthServer(ctx context.Context, addr string, ready *atomic.Bool, log
 
 	// Prometheus metrics
 	mux.Handle("/metrics", promhttp.Handler())
+	return mux
+}
 
+func startHealthServer(ctx context.Context, addr string, ready *atomic.Bool, logger *zerolog.Logger) {
 	srv := &http.Server{
 		Addr:         addr,
-		Handler:      mux,
+		Handler:      healthHandler(ready),
 		ReadTimeout:  5 * time.Second,
 		WriteTimeout: 10 * time.Second,
 		IdleTimeout:  30 * time.Second,

cmd/pulse-agent/main_test.go

@@ -143,6 +143,55 @@ func TestGatherTags(t *testing.T) {
 	}
 }
 
+func TestGatherCSV(t *testing.T) {
+	tests := []struct {
+		name     string
+		env      string
+		flags    []string
+		expected []string
+	}{
+		{"empty", "", nil, []string{}},
+		{"env only", "a,b", nil, []string{"a", "b"}},
+		{"env trims", " a , b ", nil, []string{"a", "b"}},
+		{"flags only", "", []string{"x", " y "}, []string{"x", "y"}},
+		{"both", "a", []string{"b"}, []string{"a", "b"}},
+		{"filters empties", "a,,", []string{"", "b", "  "}, []string{"a", "b"}},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := gatherCSV(tc.env, tc.flags)
+			if !reflect.DeepEqual(got, tc.expected) {
+				t.Fatalf("expected %v, got %v", tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestDefaultInt(t *testing.T) {
+	tests := []struct {
+		name     string
+		value    string
+		fallback int
+		expected int
+	}{
+		{"empty uses fallback", "", 5, 5},
+		{"whitespace uses fallback", "   ", 5, 5},
+		{"valid int", "12", 5, 12},
+		{"invalid uses fallback", "nope", 5, 5},
+		{"leading whitespace", " 7", 5, 7},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := defaultInt(tc.value, tc.fallback)
+			if got != tc.expected {
+				t.Fatalf("expected %d, got %d", tc.expected, got)
+			}
+		})
+	}
+}
+
 func TestParseLogLevel(t *testing.T) {
 	tests := []struct {
 		name      string

cmd/pulse/auto_import_test.go

@@ -0,0 +1,83 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/config"
+)
+
+func TestShouldAutoImport(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PULSE_DATA_DIR", dir)
+
+	// No env vars => false.
+	t.Setenv("PULSE_INIT_CONFIG_DATA", "")
+	t.Setenv("PULSE_INIT_CONFIG_FILE", "")
+	if shouldAutoImport() {
+		t.Fatalf("expected false when no env vars set")
+	}
+
+	// Env var set => true.
+	t.Setenv("PULSE_INIT_CONFIG_DATA", "anything")
+	if !shouldAutoImport() {
+		t.Fatalf("expected true when init config env var set")
+	}
+
+	// Existing config should disable auto-import.
+	if err := os.WriteFile(filepath.Join(dir, "nodes.enc"), []byte("exists"), 0o600); err != nil {
+		t.Fatalf("write nodes.enc: %v", err)
+	}
+	if shouldAutoImport() {
+		t.Fatalf("expected false when nodes.enc exists")
+	}
+}
+
+func TestPerformAutoImport_ValidPayload(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PULSE_DATA_DIR", dir)
+
+	pass := "test-passphrase"
+	cp := config.NewConfigPersistence(dir)
+	exported, err := cp.ExportConfig(pass)
+	if err != nil {
+		t.Fatalf("ExportConfig: %v", err)
+	}
+
+	t.Setenv("PULSE_INIT_CONFIG_PASSPHRASE", pass)
+	t.Setenv("PULSE_INIT_CONFIG_DATA", exported)
+	t.Setenv("PULSE_INIT_CONFIG_FILE", "")
+
+	if err := performAutoImport(); err != nil {
+		t.Fatalf("performAutoImport: %v", err)
+	}
+
+	if _, err := os.Stat(filepath.Join(dir, "nodes.enc")); err != nil {
+		t.Fatalf("expected nodes.enc to exist: %v", err)
+	}
+}
+
+func TestPerformAutoImport_MissingPassphrase(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PULSE_DATA_DIR", dir)
+	t.Setenv("PULSE_INIT_CONFIG_PASSPHRASE", "")
+	t.Setenv("PULSE_INIT_CONFIG_DATA", "data")
+	t.Setenv("PULSE_INIT_CONFIG_FILE", "")
+
+	if err := performAutoImport(); err == nil {
+		t.Fatalf("expected error")
+	}
+}
+
+func TestPerformAutoImport_MissingData(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("PULSE_DATA_DIR", dir)
+	t.Setenv("PULSE_INIT_CONFIG_PASSPHRASE", "pass")
+	t.Setenv("PULSE_INIT_CONFIG_DATA", "")
+	t.Setenv("PULSE_INIT_CONFIG_FILE", "")
+
+	if err := performAutoImport(); err == nil {
+		t.Fatalf("expected error")
+	}
+}

cmd/pulse/config.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"bufio"
-	"encoding/base64"
 	"fmt"
 	"io"
 	"net/http"
@@ -253,19 +252,17 @@ var configAutoImportCmd = &cobra.Command{
 				return fmt.Errorf("configuration response from URL was empty")
 			}
 
-			trimmed := strings.TrimSpace(string(body))
-			if decoded, err := base64.StdEncoding.DecodeString(trimmed); err == nil {
-				encryptedData = string(decoded)
-			} else {
-				encryptedData = string(body)
+			payload, err := normalizeImportPayload(body)
+			if err != nil {
+				return err
 			}
+			encryptedData = payload
 		} else if configData != "" {
-			// Decode base64 if needed
-			if decoded, err := base64.StdEncoding.DecodeString(configData); err == nil {
-				encryptedData = string(decoded)
-			} else {
-				encryptedData = configData
+			payload, err := normalizeImportPayload([]byte(configData))
+			if err != nil {
+				return err
 			}
+			encryptedData = payload
 		}
 
 		// Load configuration path

cmd/pulse/config_test.go

@@ -0,0 +1,27 @@
+package main
+
+import (
+	"testing"
+)
+
+func TestGetPassphrase_FromEnv(t *testing.T) {
+	t.Setenv("PULSE_PASSPHRASE", "from-env")
+	passphrase = ""
+	t.Cleanup(func() { passphrase = "" })
+
+	got := getPassphrase("ignored", false)
+	if got != "from-env" {
+		t.Fatalf("got %q", got)
+	}
+}
+
+func TestGetPassphrase_FromFlag(t *testing.T) {
+	t.Setenv("PULSE_PASSPHRASE", "")
+	passphrase = "from-flag"
+	t.Cleanup(func() { passphrase = "" })
+
+	got := getPassphrase("ignored", false)
+	if got != "from-flag" {
+		t.Fatalf("got %q", got)
+	}
+}

cmd/pulse/import_payload.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"encoding/base64"
+	"fmt"
+	"strings"
+)
+
+func normalizeImportPayload(raw []byte) (string, error) {
+	trimmed := strings.TrimSpace(string(raw))
+	if trimmed == "" {
+		return "", fmt.Errorf("configuration payload is empty")
+	}
+
+	// 1) If it's base64, keep as-is (this is what ConfigPersistence.ImportConfig expects).
+	// 2) If it's base64-of-base64 (common when passing through systems that base64-encode values),
+	//    unwrap one layer.
+	if decoded, err := base64.StdEncoding.DecodeString(trimmed); err == nil {
+		decodedTrimmed := strings.TrimSpace(string(decoded))
+		if looksLikeBase64(decodedTrimmed) {
+			return decodedTrimmed, nil
+		}
+		return trimmed, nil
+	}
+
+	// Otherwise treat it as raw encrypted bytes and base64-encode it.
+	return base64.StdEncoding.EncodeToString(raw), nil
+}
+
+func looksLikeBase64(s string) bool {
+	if s == "" {
+		return false
+	}
+	// Allow whitespace that often appears in wrapped output.
+	compact := strings.Map(func(r rune) rune {
+		switch r {
+		case '\n', '\r', '\t', ' ':
+			return -1
+		default:
+			return r
+		}
+	}, s)
+
+	if compact == "" || len(compact)%4 != 0 {
+		return false
+	}
+	for i := 0; i < len(compact); i++ {
+		c := compact[i]
+		isAlphaNum := (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')
+		if isAlphaNum || c == '+' || c == '/' || c == '=' {
+			continue
+		}
+		return false
+	}
+	return true
+}

cmd/pulse/import_payload_test.go

@@ -0,0 +1,56 @@
+package main
+
+import (
+	"encoding/base64"
+	"testing"
+)
+
+func TestNormalizeImportPayload_Base64Passthrough(t *testing.T) {
+	raw := []byte(" Zm9vYmFy \n") // base64("foobar")
+	out, err := normalizeImportPayload(raw)
+	if err != nil {
+		t.Fatalf("err = %v", err)
+	}
+	if out != "Zm9vYmFy" {
+		t.Fatalf("out = %q", out)
+	}
+}
+
+func TestNormalizeImportPayload_Base64OfBase64Unwrap(t *testing.T) {
+	inner := "Zm9vYmFy" // base64("foobar")
+	outer := base64.StdEncoding.EncodeToString([]byte(inner))
+	out, err := normalizeImportPayload([]byte(outer))
+	if err != nil {
+		t.Fatalf("err = %v", err)
+	}
+	if out != inner {
+		t.Fatalf("out = %q, want %q", out, inner)
+	}
+}
+
+func TestNormalizeImportPayload_RawBytesGetEncoded(t *testing.T) {
+	raw := []byte{0x00, 0x01, 0x02, 0xff, 0x10}
+	out, err := normalizeImportPayload(raw)
+	if err != nil {
+		t.Fatalf("err = %v", err)
+	}
+	decoded, err := base64.StdEncoding.DecodeString(out)
+	if err != nil {
+		t.Fatalf("DecodeString: %v", err)
+	}
+	if string(decoded) != string(raw) {
+		t.Fatalf("roundtrip mismatch")
+	}
+}
+
+func TestLooksLikeBase64(t *testing.T) {
+	if looksLikeBase64("") {
+		t.Fatalf("empty should be false")
+	}
+	if !looksLikeBase64("Zm9vYmFy") {
+		t.Fatalf("expected base64 true")
+	}
+	if looksLikeBase64("not-base64!!!") {
+		t.Fatalf("expected base64 false")
+	}
+}

cmd/pulse/main.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"encoding/base64"
 	"fmt"
 	"net/http"
 	"os"
@@ -179,7 +178,7 @@ func runServer() {
 		return nil
 	}
 	router = api.NewRouter(cfg, reloadableMonitor.GetMonitor(), wsHub, reloadFunc, Version)
-	
+
 	// Inject resource store into monitor for WebSocket broadcasts
 	// This must be done after router creation since resourceHandlers is created in NewRouter
 	router.SetMonitor(reloadableMonitor.GetMonitor())
@@ -352,14 +351,17 @@ func performAutoImport() error {
 		if err != nil {
 			return fmt.Errorf("failed to read config file: %w", err)
 		}
-		encryptedData = string(data)
-	} else if configData != "" {
-		// Try to decode base64 if it looks encoded
-		if decoded, err := base64.StdEncoding.DecodeString(configData); err == nil {
-			encryptedData = string(decoded)
-		} else {
-			encryptedData = configData
+		payload, err := normalizeImportPayload(data)
+		if err != nil {
+			return err
 		}
+		encryptedData = payload
+	} else if configData != "" {
+		payload, err := normalizeImportPayload([]byte(configData))
+		if err != nil {
+			return err
+		}
+		encryptedData = payload
 	} else {
 		return fmt.Errorf("no config data provided")
 	}

internal/agentbinaries/extract_test.go

@@ -0,0 +1,98 @@
+package agentbinaries
+
+import (
+	"archive/tar"
+	"compress/gzip"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestExtractHostAgentBinaries_ExtractsAndSymlinks(t *testing.T) {
+	tmpDir := t.TempDir()
+	archivePath := filepath.Join(tmpDir, "bundle.tar.gz")
+	targetDir := filepath.Join(tmpDir, "out")
+
+	content := []byte("binary-content")
+
+	f, err := os.Create(archivePath)
+	if err != nil {
+		t.Fatalf("create archive: %v", err)
+	}
+	gz := gzip.NewWriter(f)
+	tw := tar.NewWriter(gz)
+
+	writeFile := func(name string, mode int64, data []byte) {
+		hdr := &tar.Header{
+			Name: name,
+			Mode: mode,
+			Size: int64(len(data)),
+		}
+		if err := tw.WriteHeader(hdr); err != nil {
+			t.Fatalf("WriteHeader(%s): %v", name, err)
+		}
+		if _, err := tw.Write(data); err != nil {
+			t.Fatalf("Write(%s): %v", name, err)
+		}
+	}
+
+	writeSymlink := func(name, target string) {
+		hdr := &tar.Header{
+			Name:     name,
+			Typeflag: tar.TypeSymlink,
+			Linkname: target,
+			Mode:     0o777,
+		}
+		if err := tw.WriteHeader(hdr); err != nil {
+			t.Fatalf("WriteHeader(%s symlink): %v", name, err)
+		}
+	}
+
+	// Relevant binary under bin/
+	writeFile("bin/pulse-host-agent-linux-amd64", 0o644, content)
+	// Irrelevant entries should be ignored
+	writeFile("bin/not-a-host-agent", 0o644, []byte("ignore"))
+	writeFile("docs/readme.txt", 0o644, []byte("ignore"))
+	// Symlink entry should be created as-is
+	writeSymlink("bin/pulse-host-agent-linux-amd64-link", "pulse-host-agent-linux-amd64")
+
+	if err := tw.Close(); err != nil {
+		t.Fatalf("tar close: %v", err)
+	}
+	if err := gz.Close(); err != nil {
+		t.Fatalf("gzip close: %v", err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatalf("file close: %v", err)
+	}
+
+	if err := extractHostAgentBinaries(archivePath, targetDir); err != nil {
+		t.Fatalf("extractHostAgentBinaries: %v", err)
+	}
+
+	extracted := filepath.Join(targetDir, "pulse-host-agent-linux-amd64")
+	info, err := os.Stat(extracted)
+	if err != nil {
+		t.Fatalf("stat extracted: %v", err)
+	}
+	if info.Mode()&0o111 == 0 {
+		t.Fatalf("expected executable permissions, got %v", info.Mode())
+	}
+
+	got, err := os.ReadFile(extracted)
+	if err != nil {
+		t.Fatalf("read extracted: %v", err)
+	}
+	if string(got) != string(content) {
+		t.Fatalf("content mismatch")
+	}
+
+	linkPath := filepath.Join(targetDir, "pulse-host-agent-linux-amd64-link")
+	target, err := os.Readlink(linkPath)
+	if err != nil {
+		t.Fatalf("readlink: %v", err)
+	}
+	if target != "pulse-host-agent-linux-amd64" {
+		t.Fatalf("link target = %q", target)
+	}
+}

internal/agentupdate/update.go

@@ -64,6 +64,8 @@ type Updater struct {
 	cfg    Config
 	client *http.Client
 	logger zerolog.Logger
+
+	performUpdateFn func(context.Context) error
 }
 
 // New creates a new Updater with the given configuration.
@@ -84,7 +86,7 @@ func New(cfg Config) *Updater {
 		},
 	}
 
-	return &Updater{
+	u := &Updater{
 		cfg: cfg,
 		client: &http.Client{
 			Transport: transport,
@@ -92,6 +94,8 @@ func New(cfg Config) *Updater {
 		},
 		logger: logger,
 	}
+	u.performUpdateFn = u.performUpdate
+	return u
 }
 
 // RunLoop starts the update check loop. It blocks until the context is cancelled.
@@ -179,7 +183,7 @@ func (u *Updater) CheckAndUpdate(ctx context.Context) {
 		Str("availableVersion", serverVersion).
 		Msg("New agent version available, performing self-update")
 
-	if err := u.performUpdate(ctx); err != nil {
+	if err := u.performUpdateFn(ctx); err != nil {
 		u.logger.Error().Err(err).Msg("Failed to self-update agent")
 		return
 	}

internal/agentupdate/update_http_test.go

@@ -0,0 +1,136 @@
+package agentupdate
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestUpdater_getServerVersion_SetsAuthHeaders(t *testing.T) {
+	var sawAuth bool
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/agent/version" {
+			t.Fatalf("unexpected path: %s", r.URL.Path)
+		}
+		if r.Header.Get("X-API-Token") != "token" {
+			t.Fatalf("X-API-Token = %q", r.Header.Get("X-API-Token"))
+		}
+		if r.Header.Get("Authorization") != "Bearer token" {
+			t.Fatalf("Authorization = %q", r.Header.Get("Authorization"))
+		}
+		sawAuth = true
+		_ = json.NewEncoder(w).Encode(map[string]string{"version": "1.2.3"})
+	}))
+	defer srv.Close()
+
+	u := New(Config{
+		PulseURL:       srv.URL,
+		APIToken:       "token",
+		CurrentVersion: "1.0.0",
+		CheckInterval:  time.Minute,
+	})
+
+	v, err := u.getServerVersion(context.Background())
+	if err != nil {
+		t.Fatalf("getServerVersion: %v", err)
+	}
+	if v != "1.2.3" || !sawAuth {
+		t.Fatalf("unexpected version=%q sawAuth=%v", v, sawAuth)
+	}
+}
+
+func TestUpdater_getServerVersion_BadStatus(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+
+	u := New(Config{PulseURL: srv.URL, CurrentVersion: "1.0.0"})
+	_, err := u.getServerVersion(context.Background())
+	if err == nil || !strings.Contains(err.Error(), "status") {
+		t.Fatalf("expected status error, got: %v", err)
+	}
+}
+
+func TestUpdater_getServerVersion_BadJSON(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte("{not-json"))
+	}))
+	defer srv.Close()
+
+	u := New(Config{PulseURL: srv.URL, CurrentVersion: "1.0.0"})
+	_, err := u.getServerVersion(context.Background())
+	if err == nil || !strings.Contains(err.Error(), "decode") {
+		t.Fatalf("expected decode error, got: %v", err)
+	}
+}
+
+func TestUpdater_CheckAndUpdate_EarlyReturns(t *testing.T) {
+	u := New(Config{Disabled: true})
+	u.performUpdateFn = func(ctx context.Context) error {
+		t.Fatalf("performUpdate should not be called")
+		return nil
+	}
+	u.CheckAndUpdate(context.Background())
+
+	u = New(Config{CurrentVersion: "dev"})
+	u.performUpdateFn = func(ctx context.Context) error {
+		t.Fatalf("performUpdate should not be called")
+		return nil
+	}
+	u.CheckAndUpdate(context.Background())
+
+	u = New(Config{CurrentVersion: "1.0.0", PulseURL: ""})
+	u.performUpdateFn = func(ctx context.Context) error {
+		t.Fatalf("performUpdate should not be called")
+		return nil
+	}
+	u.CheckAndUpdate(context.Background())
+}
+
+func TestUpdater_CheckAndUpdate_VersionComparePaths(t *testing.T) {
+	tests := []struct {
+		name          string
+		current       string
+		server        string
+		expectUpdate  bool
+		expectNoError bool
+	}{
+		{"up-to-date", "1.0.0", "1.0.0", false, true},
+		{"server-older", "1.0.1", "1.0.0", false, true},
+		{"server-dev", "1.0.0", "dev", false, true},
+		{"server-newer", "1.0.0", "1.0.1", true, true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var called bool
+
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				_ = json.NewEncoder(w).Encode(map[string]string{"version": tc.server})
+			}))
+			defer srv.Close()
+
+			u := New(Config{
+				PulseURL:       srv.URL,
+				AgentName:      "pulse-agent",
+				CurrentVersion: tc.current,
+				CheckInterval:  time.Minute,
+			})
+			u.performUpdateFn = func(ctx context.Context) error {
+				called = true
+				return nil
+			}
+
+			u.CheckAndUpdate(context.Background())
+
+			if called != tc.expectUpdate {
+				t.Fatalf("performUpdate called=%v, want %v", called, tc.expectUpdate)
+			}
+		})
+	}
+}