Fix system path in notification urls

- Updated AlertManager to replace direct app references with a hub interface.
- Changed AlertManager.app to AlertManager.hub
- Add tests for MakeLink

beszel/internal/alerts/alerts.go

@@ -15,8 +15,13 @@ import (
 	"github.com/pocketbase/pocketbase/tools/mailer"
 )
 
+type hubLike interface {
+	core.App
+	MakeLink(parts ...string) string
+}
+
 type AlertManager struct {
-	app           core.App
+	hub           hubLike
 	alertQueue    chan alertTask
 	stopChan      chan struct{}
 	pendingAlerts sync.Map
@@ -79,9 +84,9 @@ var supportsTitle = map[string]struct{}{
 }
 
 // NewAlertManager creates a new AlertManager instance.
-func NewAlertManager(app core.App) *AlertManager {
+func NewAlertManager(app hubLike) *AlertManager {
 	am := &AlertManager{
-		app:        app,
+		hub:        app,
 		alertQueue: make(chan alertTask),
 		stopChan:   make(chan struct{}),
 	}
@@ -91,7 +96,7 @@ func NewAlertManager(app core.App) *AlertManager {
 
 func (am *AlertManager) SendAlert(data AlertMessageData) error {
 	// get user settings
-	record, err := am.app.FindFirstRecordByFilter(
+	record, err := am.hub.FindFirstRecordByFilter(
 		"user_settings", "user={:user}",
 		dbx.Params{"user": data.UserID},
 	)
@@ -104,12 +109,12 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
 		Webhooks: []string{},
 	}
 	if err := record.UnmarshalJSONField("settings", &userAlertSettings); err != nil {
-		am.app.Logger().Error("Failed to unmarshal user settings", "err", err.Error())
+		am.hub.Logger().Error("Failed to unmarshal user settings", "err", err)
 	}
 	// send alerts via webhooks
 	for _, webhook := range userAlertSettings.Webhooks {
 		if err := am.SendShoutrrrAlert(webhook, data.Title, data.Message, data.Link, data.LinkText); err != nil {
-			am.app.Logger().Error("Failed to send shoutrrr alert", "err", err.Error())
+			am.hub.Logger().Error("Failed to send shoutrrr alert", "err", err)
 		}
 	}
 	// send alerts via email
@@ -125,15 +130,15 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
 		Subject: data.Title,
 		Text:    data.Message + fmt.Sprintf("\n\n%s", data.Link),
 		From: mail.Address{
-			Address: am.app.Settings().Meta.SenderAddress,
-			Name:    am.app.Settings().Meta.SenderName,
+			Address: am.hub.Settings().Meta.SenderAddress,
+			Name:    am.hub.Settings().Meta.SenderName,
 		},
 	}
-	err = am.app.NewMailClient().Send(&message)
+	err = am.hub.NewMailClient().Send(&message)
 	if err != nil {
 		return err
 	}
-	am.app.Logger().Info("Sent email alert", "to", message.To, "subj", message.Subject)
+	am.hub.Logger().Info("Sent email alert", "to", message.To, "subj", message.Subject)
 	return nil
 }
 
@@ -183,9 +188,9 @@ func (am *AlertManager) SendShoutrrrAlert(notificationUrl, title, message, link,
 	err = shoutrrr.Send(parsedURL.String(), message)
 
 	if err == nil {
-		am.app.Logger().Info("Sent shoutrrr alert", "title", title)
+		am.hub.Logger().Info("Sent shoutrrr alert", "title", title)
 	} else {
-		am.app.Logger().Error("Error sending shoutrrr alert", "err", err.Error())
+		am.hub.Logger().Error("Error sending shoutrrr alert", "err", err)
 		return err
 	}
 	return nil
@@ -201,7 +206,7 @@ func (am *AlertManager) SendTestNotification(e *core.RequestEvent) error {
 	if url == "" {
 		return e.JSON(200, map[string]string{"err": "URL is required"})
 	}
-	err := am.SendShoutrrrAlert(url, "Test Alert", "This is a notification from Beszel.", am.app.Settings().Meta.AppURL, "View Beszel")
+	err := am.SendShoutrrrAlert(url, "Test Alert", "This is a notification from Beszel.", am.hub.Settings().Meta.AppURL, "View Beszel")
 	if err != nil {
 		return e.JSON(200, map[string]string{"err": err.Error()})
 	}

beszel/internal/alerts/alerts_status.go

@@ -2,7 +2,6 @@ package alerts
 
 import (
 	"fmt"
-	"net/url"
 	"strings"
 	"time"
 
@@ -87,7 +86,7 @@ func (am *AlertManager) HandleStatusAlerts(newStatus string, systemRecord *core.
 
 // getSystemStatusAlerts retrieves all "Status" alert records for a given system ID.
 func (am *AlertManager) getSystemStatusAlerts(systemID string) ([]*core.Record, error) {
-	alertRecords, err := am.app.FindAllRecords("alerts", dbx.HashExp{
+	alertRecords, err := am.hub.FindAllRecords("alerts", dbx.HashExp{
 		"system": systemID,
 		"name":   "Status",
 	})
@@ -130,7 +129,7 @@ func (am *AlertManager) handleSystemUp(systemName string, alertRecords []*core.R
 		}
 		// No alert scheduled for this record, send "up" alert
 		if err := am.sendStatusAlert("up", systemName, alertRecord); err != nil {
-			am.app.Logger().Error("Failed to send alert", "err", err.Error())
+			am.hub.Logger().Error("Failed to send alert", "err", err)
 		}
 	}
 }
@@ -147,7 +146,7 @@ func (am *AlertManager) sendStatusAlert(alertStatus string, systemName string, a
 	title := fmt.Sprintf("Connection to %s is %s %v", systemName, alertStatus, emoji)
 	message := strings.TrimSuffix(title, emoji)
 
-	if errs := am.app.ExpandRecord(alertRecord, []string{"user"}, nil); len(errs) > 0 {
+	if errs := am.hub.ExpandRecord(alertRecord, []string{"user"}, nil); len(errs) > 0 {
 		return errs["user"]
 	}
 	user := alertRecord.ExpandedOne("user")
@@ -159,7 +158,7 @@ func (am *AlertManager) sendStatusAlert(alertStatus string, systemName string, a
 		UserID:   user.Id,
 		Title:    title,
 		Message:  message,
-		Link:     am.app.Settings().Meta.AppURL + "/system/" + url.PathEscape(systemName),
+		Link:     am.hub.MakeLink("system", systemName),
 		LinkText: "View " + systemName,
 	})
 }

beszel/internal/alerts/alerts_system.go

@@ -3,7 +3,6 @@ package alerts
 import (
 	"beszel/internal/entities/system"
 	"fmt"
-	"net/url"
 	"strings"
 	"time"
 
@@ -15,7 +14,7 @@ import (
 )
 
 func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *system.CombinedData) error {
-	alertRecords, err := am.app.FindAllRecords("alerts",
+	alertRecords, err := am.hub.FindAllRecords("alerts",
 		dbx.NewExp("system={:system}", dbx.Params{"system": systemRecord.Id}),
 	)
 	if err != nil || len(alertRecords) == 0 {
@@ -101,7 +100,7 @@ func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *syst
 		Created types.DateTime `db:"created"`
 	}{}
 
-	err = am.app.DB().
+	err = am.hub.DB().
 		Select("stats", "created").
 		From("system_stats").
 		Where(dbx.NewExp(
@@ -271,12 +270,12 @@ func (am *AlertManager) sendSystemAlert(alert SystemAlertData) {
 	body := fmt.Sprintf("%s averaged %.2f%s for the previous %v %s.", alert.descriptor, alert.val, alert.unit, alert.min, minutesLabel)
 
 	alert.alertRecord.Set("triggered", alert.triggered)
-	if err := am.app.Save(alert.alertRecord); err != nil {
-		// app.Logger().Error("failed to save alert record", "err", err.Error())
+	if err := am.hub.Save(alert.alertRecord); err != nil {
+		// app.Logger().Error("failed to save alert record", "err", err)
 		return
 	}
 	// expand the user relation and send the alert
-	if errs := am.app.ExpandRecord(alert.alertRecord, []string{"user"}, nil); len(errs) > 0 {
+	if errs := am.hub.ExpandRecord(alert.alertRecord, []string{"user"}, nil); len(errs) > 0 {
 		// app.Logger().Error("failed to expand user relation", "errs", errs)
 		return
 	}
@@ -285,7 +284,7 @@ func (am *AlertManager) sendSystemAlert(alert SystemAlertData) {
 			UserID:   user.Id,
 			Title:    subject,
 			Message:  body,
-			Link:     am.app.Settings().Meta.AppURL + "/system/" + url.PathEscape(systemName),
+			Link:     am.hub.MakeLink("system", systemName),
 			LinkText: "View " + systemName,
 		})
 	}

beszel/internal/hub/hub.go

@@ -58,7 +58,6 @@ func GetEnv(key string) (value string, exists bool) {
 }
 
 func (h *Hub) StartHub() error {
-
 	h.App.OnServe().BindFunc(func(e *core.ServeEvent) error {
 		// initialize settings / collections
 		if err := h.initialize(e); err != nil {
@@ -158,7 +157,7 @@ func (h *Hub) initialize(e *core.ServeEvent) error {
 	return nil
 }
 
-// startServer starts the server for the Beszel (not PocketBase)
+// startServer sets up the server for Beszel
 func (h *Hub) startServer(se *core.ServeEvent) error {
 	// TODO: exclude dev server from production binary
 	switch h.IsDev() {
@@ -242,8 +241,8 @@ func (h *Hub) registerApiRoutes(se *core.ServeEvent) error {
 }
 
 // generates key pair if it doesn't exist and returns signer
-func (h *Hub) GetSSHKey() (ssh.Signer, error) {
-	privateKeyPath := path.Join(h.DataDir(), "id_ed25519")
+func (h *Hub) GetSSHKey(dataDir string) (ssh.Signer, error) {
+	privateKeyPath := path.Join(dataDir, "id_ed25519")
 
 	// check if the key pair already exists
 	existingKey, err := os.ReadFile(privateKeyPath)
@@ -255,6 +254,9 @@ func (h *Hub) GetSSHKey() (ssh.Signer, error) {
 		pubKeyBytes := ssh.MarshalAuthorizedKey(private.PublicKey())
 		h.pubKey = strings.TrimSuffix(string(pubKeyBytes), "\n")
 		return private, nil
+	} else if !os.IsNotExist(err) {
+		// File exists but couldn't be read for some other reason
+		return nil, fmt.Errorf("failed to read %s: %w", privateKeyPath, err)
 	}
 
 	// Generate the Ed25519 key pair
@@ -285,3 +287,16 @@ func (h *Hub) GetSSHKey() (ssh.Signer, error) {
 
 	return sshPrivate, err
 }
+
+// MakeLink formats a link with the app URL and path segments.
+// Only path segments should be provided.
+func (h *Hub) MakeLink(parts ...string) string {
+	base := strings.TrimSuffix(h.Settings().Meta.AppURL, "/")
+	for _, part := range parts {
+		if part == "" {
+			continue
+		}
+		base = fmt.Sprintf("%s/%s", base, url.PathEscape(part))
+	}
+	return base
+}

beszel/internal/hub/hub_test.go

@@ -0,0 +1,257 @@
+//go:build testing
+// +build testing
+
+package hub
+
+import (
+	"testing"
+
+	"crypto/ed25519"
+	"encoding/pem"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/pocketbase/pocketbase"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/crypto/ssh"
+)
+
+func getTestHub() *Hub {
+	app := pocketbase.New()
+	return NewHub(app)
+}
+
+func TestMakeLink(t *testing.T) {
+	hub := getTestHub()
+
+	tests := []struct {
+		name     string
+		appURL   string
+		parts    []string
+		expected string
+	}{
+		{
+			name:     "no parts, no trailing slash in AppURL",
+			appURL:   "http://localhost:8090",
+			parts:    []string{},
+			expected: "http://localhost:8090",
+		},
+		{
+			name:     "no parts, with trailing slash in AppURL",
+			appURL:   "http://localhost:8090/",
+			parts:    []string{},
+			expected: "http://localhost:8090", // TrimSuffix should handle the trailing slash
+		},
+		{
+			name:     "one part",
+			appURL:   "http://example.com",
+			parts:    []string{"one"},
+			expected: "http://example.com/one",
+		},
+		{
+			name:     "multiple parts",
+			appURL:   "http://example.com",
+			parts:    []string{"alpha", "beta", "gamma"},
+			expected: "http://example.com/alpha/beta/gamma",
+		},
+		{
+			name:     "parts with spaces needing escaping",
+			appURL:   "http://example.com",
+			parts:    []string{"path with spaces", "another part"},
+			expected: "http://example.com/path%20with%20spaces/another%20part",
+		},
+		{
+			name:     "parts with slashes needing escaping",
+			appURL:   "http://example.com",
+			parts:    []string{"a/b", "c"},
+			expected: "http://example.com/a%2Fb/c", // url.PathEscape escapes '/'
+		},
+		{
+			name:     "AppURL with subpath, no trailing slash",
+			appURL:   "http://localhost/sub",
+			parts:    []string{"resource"},
+			expected: "http://localhost/sub/resource",
+		},
+		{
+			name:     "AppURL with subpath, with trailing slash",
+			appURL:   "http://localhost/sub/",
+			parts:    []string{"item"},
+			expected: "http://localhost/sub/item",
+		},
+		{
+			name:     "empty parts in the middle",
+			appURL:   "http://localhost",
+			parts:    []string{"first", "", "third"},
+			expected: "http://localhost/first/third",
+		},
+		{
+			name:     "leading and trailing empty parts",
+			appURL:   "http://localhost",
+			parts:    []string{"", "path", ""},
+			expected: "http://localhost/path",
+		},
+		{
+			name:     "parts with various special characters",
+			appURL:   "https://test.dev/",
+			parts:    []string{"p@th?", "key=value&"},
+			expected: "https://test.dev/p@th%3F/key=value&",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Store original app URL and restore it after the test
+			originalAppURL := hub.Settings().Meta.AppURL
+			hub.Settings().Meta.AppURL = tt.appURL
+			defer func() { hub.Settings().Meta.AppURL = originalAppURL }()
+
+			got := hub.MakeLink(tt.parts...)
+			assert.Equal(t, tt.expected, got, "MakeLink generated URL does not match expected")
+		})
+	}
+}
+
+func TestGetSSHKey(t *testing.T) {
+	hub := getTestHub()
+
+	// Test Case 1: Key generation (no existing key)
+	t.Run("KeyGeneration", func(t *testing.T) {
+		tempDir := t.TempDir()
+
+		// Ensure pubKey is initially empty or different to ensure GetSSHKey sets it
+		hub.pubKey = ""
+
+		signer, err := hub.GetSSHKey(tempDir)
+		assert.NoError(t, err, "GetSSHKey should not error when generating a new key")
+		assert.NotNil(t, signer, "GetSSHKey should return a non-nil signer")
+
+		// Check if private key file was created
+		privateKeyPath := filepath.Join(tempDir, "id_ed25519")
+		info, err := os.Stat(privateKeyPath)
+		assert.NoError(t, err, "Private key file should be created")
+		assert.False(t, info.IsDir(), "Private key path should be a file, not a directory")
+
+		// Check if h.pubKey was set
+		assert.NotEmpty(t, hub.pubKey, "h.pubKey should be set after key generation")
+		assert.True(t, strings.HasPrefix(hub.pubKey, "ssh-ed25519 "), "h.pubKey should start with 'ssh-ed25519 '")
+
+		// Verify the generated private key is parsable
+		keyData, err := os.ReadFile(privateKeyPath)
+		require.NoError(t, err)
+		_, err = ssh.ParsePrivateKey(keyData)
+		assert.NoError(t, err, "Generated private key should be parsable by ssh.ParsePrivateKey")
+	})
+
+	// Test Case 2: Existing key
+	t.Run("ExistingKey", func(t *testing.T) {
+		tempDir := t.TempDir()
+
+		// Manually create a valid key pair for the test
+		rawPubKey, rawPrivKey, err := ed25519.GenerateKey(nil)
+		require.NoError(t, err, "Failed to generate raw ed25519 key pair for pre-existing key test")
+
+		// Marshal the private key into OpenSSH PEM format
+		pemBlock, err := ssh.MarshalPrivateKey(rawPrivKey, "")
+		require.NoError(t, err, "Failed to marshal private key to PEM block for pre-existing key test")
+
+		privateKeyBytes := pem.EncodeToMemory(pemBlock)
+		require.NotNil(t, privateKeyBytes, "PEM encoded private key bytes should not be nil")
+
+		privateKeyPath := filepath.Join(tempDir, "id_ed25519")
+		err = os.WriteFile(privateKeyPath, privateKeyBytes, 0600)
+		require.NoError(t, err, "Failed to write pre-existing private key")
+
+		// Determine the expected public key string
+		sshPubKey, err := ssh.NewPublicKey(rawPubKey)
+		require.NoError(t, err)
+		expectedPubKeyStr := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(sshPubKey)))
+
+		// Reset h.pubKey to ensure it's set by GetSSHKey from the file
+		hub.pubKey = ""
+
+		signer, err := hub.GetSSHKey(tempDir)
+		assert.NoError(t, err, "GetSSHKey should not error when reading an existing key")
+		assert.NotNil(t, signer, "GetSSHKey should return a non-nil signer for an existing key")
+
+		// Check if h.pubKey was set correctly to the public key from the file
+		assert.Equal(t, expectedPubKeyStr, hub.pubKey, "h.pubKey should match the existing public key")
+
+		// Verify the signer's public key matches the original public key
+		signerPubKey := signer.PublicKey()
+		marshaledSignerPubKey := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(signerPubKey)))
+		assert.Equal(t, expectedPubKeyStr, marshaledSignerPubKey, "Signer's public key should match the existing public key")
+	})
+
+	// Test Case 3: Error cases
+	t.Run("ErrorCases", func(t *testing.T) {
+		tests := []struct {
+			name       string
+			setupFunc  func(dir string) error
+			errorCheck func(t *testing.T, err error)
+		}{
+			{
+				name: "CorruptedKey",
+				setupFunc: func(dir string) error {
+					return os.WriteFile(filepath.Join(dir, "id_ed25519"), []byte("this is not a valid SSH key"), 0600)
+				},
+				errorCheck: func(t *testing.T, err error) {
+					assert.Error(t, err)
+					assert.Contains(t, err.Error(), "ssh: no key found")
+				},
+			},
+			{
+				name: "PermissionDenied",
+				setupFunc: func(dir string) error {
+					// Create the key file
+					keyPath := filepath.Join(dir, "id_ed25519")
+					if err := os.WriteFile(keyPath, []byte("dummy content"), 0600); err != nil {
+						return err
+					}
+					// Make it read-only (can't be opened for writing in case a new key needs to be written)
+					return os.Chmod(keyPath, 0400)
+				},
+				errorCheck: func(t *testing.T, err error) {
+					// On read-only key, the parser will attempt to parse it and fail with "ssh: no key found"
+					assert.Error(t, err)
+				},
+			},
+			{
+				name: "EmptyFile",
+				setupFunc: func(dir string) error {
+					// Create an empty file
+					return os.WriteFile(filepath.Join(dir, "id_ed25519"), []byte{}, 0600)
+				},
+				errorCheck: func(t *testing.T, err error) {
+					assert.Error(t, err)
+					// The error from attempting to parse an empty file
+					assert.Contains(t, err.Error(), "ssh: no key found")
+				},
+			},
+		}
+
+		for _, tc := range tests {
+			t.Run(tc.name, func(t *testing.T) {
+				tempDir := t.TempDir()
+
+				// Setup the test case
+				err := tc.setupFunc(tempDir)
+				require.NoError(t, err, "Setup failed")
+
+				// Reset h.pubKey before each test case
+				hub.pubKey = ""
+
+				// Attempt to get SSH key
+				_, err = hub.GetSSHKey(tempDir)
+
+				// Verify the error
+				tc.errorCheck(t, err)
+
+				// Check that pubKey was not set in error cases
+				assert.Empty(t, hub.pubKey, "h.pubKey should not be set if there was an error")
+			})
+		}
+	})
+}

beszel/internal/hub/systems/systems.go

@@ -46,7 +46,7 @@ type System struct {
 
 type hubLike interface {
 	core.App
-	GetSSHKey() (ssh.Signer, error)
+	GetSSHKey(dataDir string) (ssh.Signer, error)
 	HandleSystemAlerts(systemRecord *core.Record, data *system.CombinedData) error
 	HandleStatusAlerts(status string, systemRecord *core.Record) error
 }
@@ -362,7 +362,7 @@ func (sys *System) fetchDataFromAgent() (*system.CombinedData, error) {
 
 // createSSHClientConfig initializes the ssh config for the system manager
 func (sm *SystemManager) createSSHClientConfig() error {
-	privateKey, err := sm.hub.GetSSHKey()
+	privateKey, err := sm.hub.GetSSHKey(sm.hub.DataDir())
 	if err != nil {
 		return err
 	}