heartbeat: tweaks and tests (#1729)

internal/hub/heartbeat/heartbeat.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"time"
@@ -76,8 +77,9 @@ type Heartbeat struct {
 // New creates a Heartbeat if configuration is present.
 // Returns nil if HEARTBEAT_URL is not set (feature disabled).
 func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat {
-	url, ok := getEnv("HEARTBEAT_URL")
+	url, _ := getEnv("HEARTBEAT_URL")
-	if !ok || url == "" {
+	url = strings.TrimSpace(url)
+	if app == nil || url == "" {
 		return nil
 	}
 
@@ -88,10 +90,10 @@ func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat {
 		}
 	}
 
-	method := "POST"
+	method := http.MethodPost
 	if v, ok := getEnv("HEARTBEAT_METHOD"); ok {
 		v = strings.ToUpper(strings.TrimSpace(v))
-		if v == "GET" || v == "HEAD" {
+		if v == http.MethodGet || v == http.MethodHead {
 			method = v
 		}
 	}
@@ -110,8 +112,9 @@ func New(app core.App, getEnv func(string) (string, bool)) *Heartbeat {
 // Start begins the heartbeat loop. It blocks and should be called in a goroutine.
 // The loop runs until the provided stop channel is closed.
 func (hb *Heartbeat) Start(stop <-chan struct{}) {
+	sanitizedURL := sanitizeHeartbeatURL(hb.config.URL)
 	hb.app.Logger().Info("Heartbeat enabled",
-		"url", hb.config.URL,
+		"url", sanitizedURL,
 		"interval", fmt.Sprintf("%ds", hb.config.Interval),
 		"method", hb.config.Method,
 	)
@@ -143,23 +146,25 @@ func (hb *Heartbeat) GetConfig() Config {
 }
 
 func (hb *Heartbeat) send() error {
-	payload, err := hb.buildPayload()
+	var req *http.Request
-	if err != nil {
+	var err error
-		hb.app.Logger().Error("Heartbeat: failed to build payload", "err", err)
+	method := normalizeMethod(hb.config.Method)
-		return err
+
+	if method == http.MethodGet || method == http.MethodHead {
+		req, err = http.NewRequest(method, hb.config.URL, nil)
+	} else {
+		payload, payloadErr := hb.buildPayload()
+		if payloadErr != nil {
+			hb.app.Logger().Error("Heartbeat: failed to build payload", "err", payloadErr)
+			return payloadErr
 		}
 
-	var req *http.Request
-
-	if hb.config.Method == "GET" || hb.config.Method == "HEAD" {
-		req, err = http.NewRequest(hb.config.Method, hb.config.URL, nil)
-	} else {
 		body, jsonErr := json.Marshal(payload)
 		if jsonErr != nil {
 			hb.app.Logger().Error("Heartbeat: failed to marshal payload", "err", jsonErr)
 			return jsonErr
 		}
-		req, err = http.NewRequest("POST", hb.config.URL, bytes.NewReader(body))
+		req, err = http.NewRequest(http.MethodPost, hb.config.URL, bytes.NewReader(body))
 		if err == nil {
 			req.Header.Set("Content-Type", "application/json")
 		}
@@ -174,14 +179,14 @@ func (hb *Heartbeat) send() error {
 
 	resp, err := hb.client.Do(req)
 	if err != nil {
-		hb.app.Logger().Error("Heartbeat: request failed", "url", hb.config.URL, "err", err)
+		hb.app.Logger().Error("Heartbeat: request failed", "url", sanitizeHeartbeatURL(hb.config.URL), "err", err)
 		return err
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode >= 400 {
 		hb.app.Logger().Warn("Heartbeat: non-success response",
-			"url", hb.config.URL,
+			"url", sanitizeHeartbeatURL(hb.config.URL),
 			"status", resp.StatusCode,
 		)
 		return fmt.Errorf("heartbeat endpoint returned status %d", resp.StatusCode)
@@ -220,10 +225,12 @@ func (hb *Heartbeat) buildPayload() (*Payload, error) {
 
 	// Get names of down systems.
 	var downSystems []SystemInfo
+	if summary.Down > 0 {
 		err = db.NewQuery("SELECT id, name, host FROM systems WHERE status = 'down'").All(&downSystems)
 		if err != nil {
 			return nil, fmt.Errorf("query down systems: %w", err)
 		}
+	}
 
 	// Get triggered alerts with system names.
 	var triggeredAlerts []struct {
@@ -278,3 +285,19 @@ func (hb *Heartbeat) buildPayload() (*Payload, error) {
 		Version:   beszel.Version,
 	}, nil
 }
+
+func normalizeMethod(method string) string {
+	upper := strings.ToUpper(strings.TrimSpace(method))
+	if upper == http.MethodGet || upper == http.MethodHead || upper == http.MethodPost {
+		return upper
+	}
+	return http.MethodPost
+}
+
+func sanitizeHeartbeatURL(rawURL string) string {
+	parsed, err := url.Parse(strings.TrimSpace(rawURL))
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return "<invalid-url>"
+	}
+	return parsed.Scheme + "://" + parsed.Host
+}

internal/hub/heartbeat/heartbeat_test.go

@@ -0,0 +1,258 @@
+//go:build testing
+// +build testing
+
+package heartbeat_test
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/henrygd/beszel/internal/hub/heartbeat"
+	beszeltests "github.com/henrygd/beszel/internal/tests"
+	"github.com/pocketbase/pocketbase/core"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNew(t *testing.T) {
+	t.Run("returns nil when app is missing", func(t *testing.T) {
+		hb := heartbeat.New(nil, envGetter(map[string]string{
+			"HEARTBEAT_URL": "https://heartbeat.example.com/ping",
+		}))
+		assert.Nil(t, hb)
+	})
+
+	t.Run("returns nil when URL is missing", func(t *testing.T) {
+		app := newTestHub(t)
+		hb := heartbeat.New(app.App, func(string) (string, bool) {
+			return "", false
+		})
+		assert.Nil(t, hb)
+	})
+
+	t.Run("parses and normalizes config values", func(t *testing.T) {
+		app := newTestHub(t)
+		env := map[string]string{
+			"HEARTBEAT_URL":      "  https://heartbeat.example.com/ping  ",
+			"HEARTBEAT_INTERVAL": "90",
+			"HEARTBEAT_METHOD":   "head",
+		}
+		getEnv := func(key string) (string, bool) {
+			v, ok := env[key]
+			return v, ok
+		}
+
+		hb := heartbeat.New(app.App, getEnv)
+		require.NotNil(t, hb)
+		cfg := hb.GetConfig()
+		assert.Equal(t, "https://heartbeat.example.com/ping", cfg.URL)
+		assert.Equal(t, 90, cfg.Interval)
+		assert.Equal(t, http.MethodHead, cfg.Method)
+	})
+}
+
+func TestSendGETDoesNotRequireAppOrDB(t *testing.T) {
+	app := newTestHub(t)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodGet, r.Method)
+		assert.Equal(t, "Beszel-Heartbeat", r.Header.Get("User-Agent"))
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+
+	hb := heartbeat.New(app.App, envGetter(map[string]string{
+		"HEARTBEAT_URL":    server.URL,
+		"HEARTBEAT_METHOD": "GET",
+	}))
+	require.NotNil(t, hb)
+
+	require.NoError(t, hb.Send())
+}
+
+func TestSendReturnsErrorOnHTTPFailureStatus(t *testing.T) {
+	app := newTestHub(t)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	hb := heartbeat.New(app.App, envGetter(map[string]string{
+		"HEARTBEAT_URL":    server.URL,
+		"HEARTBEAT_METHOD": "GET",
+	}))
+	require.NotNil(t, hb)
+
+	err := hb.Send()
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "heartbeat endpoint returned status 500")
+}
+
+func TestSendPOSTBuildsExpectedStatuses(t *testing.T) {
+	tests := []struct {
+		name           string
+		setup          func(t *testing.T, app *beszeltests.TestHub, user *core.Record)
+		expectStatus   string
+		expectMsgPart  string
+		expectDown     int
+		expectAlerts   int
+		expectTotal    int
+		expectUp       int
+		expectPaused   int
+		expectPending  int
+		expectDownSumm int
+	}{
+		{
+			name: "error when at least one system is down",
+			setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) {
+				downSystem := createTestSystem(t, app, user.Id, "db-1", "10.0.0.1", "down")
+				_ = createTestSystem(t, app, user.Id, "web-1", "10.0.0.2", "up")
+				createTriggeredAlert(t, app, user.Id, downSystem.Id, "CPU", 95)
+			},
+			expectStatus:   "error",
+			expectMsgPart:  "1 system(s) down",
+			expectDown:     1,
+			expectAlerts:   1,
+			expectTotal:    2,
+			expectUp:       1,
+			expectDownSumm: 1,
+		},
+		{
+			name: "warn when only alerts are triggered",
+			setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) {
+				system := createTestSystem(t, app, user.Id, "api-1", "10.1.0.1", "up")
+				createTriggeredAlert(t, app, user.Id, system.Id, "CPU", 90)
+			},
+			expectStatus:   "warn",
+			expectMsgPart:  "1 alert(s) triggered",
+			expectDown:     0,
+			expectAlerts:   1,
+			expectTotal:    1,
+			expectUp:       1,
+			expectDownSumm: 0,
+		},
+		{
+			name: "ok when no down systems and no alerts",
+			setup: func(t *testing.T, app *beszeltests.TestHub, user *core.Record) {
+				_ = createTestSystem(t, app, user.Id, "node-1", "10.2.0.1", "up")
+				_ = createTestSystem(t, app, user.Id, "node-2", "10.2.0.2", "paused")
+				_ = createTestSystem(t, app, user.Id, "node-3", "10.2.0.3", "pending")
+			},
+			expectStatus:   "ok",
+			expectMsgPart:  "All systems operational",
+			expectDown:     0,
+			expectAlerts:   0,
+			expectTotal:    3,
+			expectUp:       1,
+			expectPaused:   1,
+			expectPending:  1,
+			expectDownSumm: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			app := newTestHub(t)
+			user := createTestUser(t, app)
+			tt.setup(t, app, user)
+
+			type requestCapture struct {
+				method      string
+				userAgent   string
+				contentType string
+				payload     heartbeat.Payload
+			}
+
+			captured := make(chan requestCapture, 1)
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				defer r.Body.Close()
+				body, err := io.ReadAll(r.Body)
+				require.NoError(t, err)
+
+				var payload heartbeat.Payload
+				require.NoError(t, json.Unmarshal(body, &payload))
+				captured <- requestCapture{
+					method:      r.Method,
+					userAgent:   r.Header.Get("User-Agent"),
+					contentType: r.Header.Get("Content-Type"),
+					payload:     payload,
+				}
+				w.WriteHeader(http.StatusNoContent)
+			}))
+			defer server.Close()
+
+			hb := heartbeat.New(app.App, envGetter(map[string]string{
+				"HEARTBEAT_URL":    server.URL,
+				"HEARTBEAT_METHOD": "POST",
+			}))
+			require.NotNil(t, hb)
+			require.NoError(t, hb.Send())
+
+			req := <-captured
+			assert.Equal(t, http.MethodPost, req.method)
+			assert.Equal(t, "Beszel-Heartbeat", req.userAgent)
+			assert.Equal(t, "application/json", req.contentType)
+
+			assert.Equal(t, tt.expectStatus, req.payload.Status)
+			assert.Contains(t, req.payload.Msg, tt.expectMsgPart)
+			assert.Equal(t, tt.expectDown, len(req.payload.Down))
+			assert.Equal(t, tt.expectAlerts, len(req.payload.Alerts))
+			assert.Equal(t, tt.expectTotal, req.payload.Systems.Total)
+			assert.Equal(t, tt.expectUp, req.payload.Systems.Up)
+			assert.Equal(t, tt.expectDownSumm, req.payload.Systems.Down)
+			assert.Equal(t, tt.expectPaused, req.payload.Systems.Paused)
+			assert.Equal(t, tt.expectPending, req.payload.Systems.Pending)
+		})
+	}
+}
+
+func newTestHub(t *testing.T) *beszeltests.TestHub {
+	t.Helper()
+	app, err := beszeltests.NewTestHub(t.TempDir())
+	require.NoError(t, err)
+	t.Cleanup(app.Cleanup)
+	return app
+}
+
+func createTestUser(t *testing.T, app *beszeltests.TestHub) *core.Record {
+	t.Helper()
+	user, err := beszeltests.CreateUser(app.App, "admin@example.com", "password123")
+	require.NoError(t, err)
+	return user
+}
+
+func createTestSystem(t *testing.T, app *beszeltests.TestHub, userID, name, host, status string) *core.Record {
+	t.Helper()
+	system, err := beszeltests.CreateRecord(app.App, "systems", map[string]any{
+		"name":   name,
+		"host":   host,
+		"port":   "45876",
+		"users":  []string{userID},
+		"status": status,
+	})
+	require.NoError(t, err)
+	return system
+}
+
+func createTriggeredAlert(t *testing.T, app *beszeltests.TestHub, userID, systemID, name string, threshold float64) *core.Record {
+	t.Helper()
+	alert, err := beszeltests.CreateRecord(app.App, "alerts", map[string]any{
+		"name":      name,
+		"system":    systemID,
+		"user":      userID,
+		"value":     threshold,
+		"min":       0,
+		"triggered": true,
+	})
+	require.NoError(t, err)
+	return alert
+}
+
+func envGetter(values map[string]string) func(string) (string, bool) {
+	return func(key string) (string, bool) {
+		v, ok := values[key]
+		return v, ok
+	}
+}

internal/hub/hub.go

@@ -419,10 +419,13 @@ func (h *Hub) getUniversalToken(e *core.RequestEvent) error {
 
 // getHeartbeatStatus returns current heartbeat configuration and whether it's enabled
 func (h *Hub) getHeartbeatStatus(e *core.RequestEvent) error {
+	if e.Auth.GetString("role") != "admin" {
+		return e.ForbiddenError("Requires admin role", nil)
+	}
 	if h.hb == nil {
 		return e.JSON(http.StatusOK, map[string]any{
 			"enabled": false,
-			"msg":     "Set BESZEL_HUB_HEARTBEAT_URL to enable outbound heartbeat monitoring",
+			"msg":     "Set HEARTBEAT_URL to enable outbound heartbeat monitoring",
 		})
 	}
 	cfg := h.hb.GetConfig()
@@ -436,9 +439,12 @@ func (h *Hub) getHeartbeatStatus(e *core.RequestEvent) error {
 
 // testHeartbeat triggers a single heartbeat ping and returns the result
 func (h *Hub) testHeartbeat(e *core.RequestEvent) error {
+	if e.Auth.GetString("role") != "admin" {
+		return e.ForbiddenError("Requires admin role", nil)
+	}
 	if h.hb == nil {
 		return e.JSON(http.StatusOK, map[string]any{
-			"err": "Heartbeat not configured. Set BESZEL_HUB_HEARTBEAT_URL environment variable.",
+			"err": "Heartbeat not configured. Set HEARTBEAT_URL environment variable.",
 		})
 	}
 	if err := h.hb.Send(); err != nil {

internal/hub/hub_test.go

@@ -362,6 +362,58 @@ func TestApiRoutesAuthentication(t *testing.T) {
 			ExpectedContent: []string{"test-system"},
 			TestAppFactory:  testAppFactory,
 		},
+		{
+			Name:            "GET /heartbeat-status - no auth should fail",
+			Method:          http.MethodGet,
+			URL:             "/api/beszel/heartbeat-status",
+			ExpectedStatus:  401,
+			ExpectedContent: []string{"requires valid"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /heartbeat-status - with user auth should fail",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/heartbeat-status",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{"Requires admin role"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /heartbeat-status - with admin auth should succeed",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/heartbeat-status",
+			Headers: map[string]string{
+				"Authorization": adminUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{`"enabled":false`},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /test-heartbeat - with user auth should fail",
+			Method: http.MethodPost,
+			URL:    "/api/beszel/test-heartbeat",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  403,
+			ExpectedContent: []string{"Requires admin role"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /test-heartbeat - with admin auth should report disabled state",
+			Method: http.MethodPost,
+			URL:    "/api/beszel/test-heartbeat",
+			Headers: map[string]string{
+				"Authorization": adminUserToken,
+			},
+			ExpectedStatus:  200,
+			ExpectedContent: []string{"Heartbeat not configured"},
+			TestAppFactory:  testAppFactory,
+		},
 		{
 			Name:            "GET /universal-token - no auth should fail",
 			Method:          http.MethodGet,

internal/site/src/components/routes/settings/heartbeat.tsx

@@ -136,7 +136,7 @@ export default function HeartbeatSettings() {
 					<Separator />
 
 					<div>
-						<h4 className="text-base font-medium mb-1">
+						<h4 className="text-base font-medium mb-2">
 							<Trans>Payload format</Trans>
 						</h4>
 						<p className="text-sm text-muted-foreground leading-relaxed mb-2">
@@ -155,30 +155,20 @@ export default function HeartbeatSettings() {
 					</div>
 				</div>
 			) : (
-				<div className="space-y-4">
+				<div className="grid gap-4">
-					<p className="text-sm text-muted-foreground leading-relaxed">
-						<Trans>Heartbeat monitoring is not configured.</Trans>
-					</p>
 					<div>
-						<h4 className="text-base font-medium mb-1">
-							<Trans>Configuration</Trans>
-						</h4>
 						<p className="text-sm text-muted-foreground leading-relaxed mb-3">
 							<Trans>Set the following environment variables on your Beszel hub to enable heartbeat monitoring:</Trans>
 						</p>
-						<div className="grid gap-2">
+						<div className="grid gap-2.5">
 							<EnvVarItem
-								name="BESZEL_HUB_HEARTBEAT_URL"
+								name="HEARTBEAT_URL"
 								description={t`Endpoint URL to ping (required)`}
 								example="https://uptime.betterstack.com/api/v1/heartbeat/xxxx"
 							/>
+							<EnvVarItem name="HEARTBEAT_INTERVAL" description={t`Seconds between pings (default: 60)`} example="60" />
 							<EnvVarItem
-								name="BESZEL_HUB_HEARTBEAT_INTERVAL"
+								name="HEARTBEAT_METHOD"
-								description={t`Seconds between pings (default: 60)`}
-								example="60"
-							/>
-							<EnvVarItem
-								name="BESZEL_HUB_HEARTBEAT_METHOD"
 								description={t`HTTP method: POST, GET, or HEAD (default: POST)`}
 								example="POST"
 							/>
@@ -204,10 +194,10 @@ function ConfigItem({ label, value, mono }: { label: string; value: string; mono
 
 function EnvVarItem({ name, description, example }: { name: string; description: string; example: string }) {
 	return (
-		<div className="bg-muted/50 rounded-md px-3 py-2">
+		<div className="bg-muted/50 rounded-md px-3 py-2 grid gap-1.5">
-			<code className="text-sm font-mono text-primary">{name}</code>
+			<code className="text-sm font-mono text-primary font-medium leading-tight">{name}</code>
-			<p className="text-sm text-muted-foreground mt-0.5">{description}</p>
+			<p className="text-sm text-muted-foreground">{description}</p>
-			<p className="text-xs text-muted-foreground mt-0.5">
+			<p className="text-xs text-muted-foreground">
 				<Trans>Example:</Trans> <code className="font-mono">{example}</code>
 			</p>
 		</div>