hub: add additional validation checks for custom api routes

- Validate the user is assigned to system in authenticated routes where
the user passes in system ID. This protects against a somewhat
impractical scenario where an authenticated user cracks a random 15
character alphanumeric ID of a system that doesn't belong to them via
web API.
- Validate that systemd service exists in database before requesting
service details from agent. This protects against authenticated users
getting unit properties of services that aren't explicitly monitored.
- Refactor responses in authenticated routes to prevent enumeration of
other users' random 15 char system IDs.

internal/hub/api.go

@@ -3,6 +3,7 @@ package hub
 import (
 	"context"
 	"net/http"
+	"regexp"
 	"strings"
 	"time"
 
@@ -25,6 +26,8 @@ type UpdateInfo struct {
 	Url       string `json:"url"`
 }
 
+var containerIDPattern = regexp.MustCompile(`^[a-fA-F0-9]{12,64}$`)
+
 // Middleware to allow only admin role users
 var requireAdminRole = customAuthMiddleware(func(e *core.RequestEvent) bool {
 	return e.Auth.GetString("role") == "admin"
@@ -303,21 +306,18 @@ func (h *Hub) containerRequestHandler(e *core.RequestEvent, fetchFunc func(*syst
 	systemID := e.Request.URL.Query().Get("system")
 	containerID := e.Request.URL.Query().Get("container")
 
-	if systemID == "" || containerID == "" {
+	if systemID == "" || containerID == "" || !containerIDPattern.MatchString(containerID) {
-		return e.JSON(http.StatusBadRequest, map[string]string{"error": "system and container parameters are required"})
+		return e.BadRequestError("Invalid system or container parameter", nil)
-	}
-	if !containerIDPattern.MatchString(containerID) {
-		return e.JSON(http.StatusBadRequest, map[string]string{"error": "invalid container parameter"})
 	}
 
 	system, err := h.sm.GetSystem(systemID)
-	if err != nil {
+	if err != nil || !system.HasUser(e.App, e.Auth.Id) {
-		return e.JSON(http.StatusNotFound, map[string]string{"error": "system not found"})
+		return e.NotFoundError("", nil)
 	}
 
 	data, err := fetchFunc(system, containerID)
 	if err != nil {
-		return e.JSON(http.StatusNotFound, map[string]string{"error": err.Error()})
+		return e.InternalServerError("", err)
 	}
 
 	return e.JSON(http.StatusOK, map[string]string{responseKey: data})
@@ -343,15 +343,23 @@ func (h *Hub) getSystemdInfo(e *core.RequestEvent) error {
 	serviceName := query.Get("service")
 
 	if systemID == "" || serviceName == "" {
-		return e.JSON(http.StatusBadRequest, map[string]string{"error": "system and service parameters are required"})
+		return e.BadRequestError("Invalid system or service parameter", nil)
 	}
 	system, err := h.sm.GetSystem(systemID)
+	if err != nil || !system.HasUser(e.App, e.Auth.Id) {
+		return e.NotFoundError("", nil)
+	}
+	// verify service exists before fetching details
+	_, err = e.App.FindFirstRecordByFilter("systemd_services", "system = {:system} && name = {:name}", dbx.Params{
+		"system": systemID,
+		"name":   serviceName,
+	})
 	if err != nil {
-		return e.JSON(http.StatusNotFound, map[string]string{"error": "system not found"})
+		return e.NotFoundError("", err)
 	}
 	details, err := system.FetchSystemdInfoFromAgent(serviceName)
 	if err != nil {
-		return e.JSON(http.StatusNotFound, map[string]string{"error": err.Error()})
+		return e.InternalServerError("", err)
 	}
 	e.Response.Header().Set("Cache-Control", "public, max-age=60")
 	return e.JSON(http.StatusOK, map[string]any{"details": details})
@@ -362,17 +370,16 @@ func (h *Hub) getSystemdInfo(e *core.RequestEvent) error {
 func (h *Hub) refreshSmartData(e *core.RequestEvent) error {
 	systemID := e.Request.URL.Query().Get("system")
 	if systemID == "" {
-		return e.JSON(http.StatusBadRequest, map[string]string{"error": "system parameter is required"})
+		return e.BadRequestError("Invalid system parameter", nil)
 	}
 
 	system, err := h.sm.GetSystem(systemID)
-	if err != nil {
+	if err != nil || !system.HasUser(e.App, e.Auth.Id) {
-		return e.JSON(http.StatusNotFound, map[string]string{"error": "system not found"})
+		return e.NotFoundError("", nil)
 	}
 
-	// Fetch and save SMART devices
 	if err := system.FetchAndSaveSmartDevices(); err != nil {
-		return e.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
+		return e.InternalServerError("", err)
 	}
 
 	return e.JSON(http.StatusOK, map[string]string{"status": "ok"})

internal/hub/api_test.go

@@ -3,6 +3,7 @@ package hub_test
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"testing"
@@ -25,14 +26,17 @@ func jsonReader(v any) io.Reader {
 }
 
 func TestApiRoutesAuthentication(t *testing.T) {
-	hub, _ := beszelTests.NewTestHub(t.TempDir())
+	hub, user := beszelTests.GetHubWithUser(t)
 	defer hub.Cleanup()
 
-	hub.StartHub()
+	userToken, err := user.NewAuthToken()
+	require.NoError(t, err, "Failed to create auth token")
 
 	// Create test user and get auth token
-	user, err := beszelTests.CreateUser(hub, "testuser@example.com", "password123")
+	user2, err := beszelTests.CreateUser(hub, "testuser@example.com", "password123")
 	require.NoError(t, err, "Failed to create test user")
+	user2Token, err := user2.NewAuthToken()
+	require.NoError(t, err, "Failed to create user2 auth token")
 
 	adminUser, err := beszelTests.CreateUserWithRole(hub, "admin@example.com", "password123", "admin")
 	require.NoError(t, err, "Failed to create admin user")
@@ -42,10 +46,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 	require.NoError(t, err, "Failed to create readonly user")
 	readOnlyUserToken, err := readOnlyUser.NewAuthToken()
 
-	userToken, err := user.NewAuthToken()
+	// Create test system
-	require.NoError(t, err, "Failed to create auth token")
-
-	// Create test system for user-alerts endpoints
 	system, err := beszelTests.CreateRecord(hub, "systems", map[string]any{
 		"name":  "test-system",
 		"users": []string{user.Id},
@@ -215,13 +216,13 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"system parameter is required"},
+			ExpectedContent: []string{"Invalid", "system", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
 		{
 			Name:   "POST /smart/refresh - with readonly auth should fail",
 			Method: http.MethodPost,
-			URL:    "/api/beszel/smart/refresh",
+			URL:    fmt.Sprintf("/api/beszel/smart/refresh?system=%s", system.Id),
 			Headers: map[string]string{
 				"Authorization": readOnlyUserToken,
 			},
@@ -229,6 +230,28 @@ func TestApiRoutesAuthentication(t *testing.T) {
 			ExpectedContent: []string{"The authorized record is not allowed to perform this action."},
 			TestAppFactory:  testAppFactory,
 		},
+		{
+			Name:   "POST /smart/refresh - non-user system should fail",
+			Method: http.MethodPost,
+			URL:    fmt.Sprintf("/api/beszel/smart/refresh?system=%s", system.Id),
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "POST /smart/refresh - good user should pass validation",
+			Method: http.MethodPost,
+			URL:    fmt.Sprintf("/api/beszel/smart/refresh?system=%s", system.Id),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  500,
+			ExpectedContent: []string{"Something went wrong while processing your request."},
+			TestAppFactory:  testAppFactory,
+		},
 		{
 			Name:            "POST /user-alerts - no auth should fail",
 			Method:          http.MethodPost,
@@ -300,20 +323,42 @@ func TestApiRoutesAuthentication(t *testing.T) {
 		{
 			Name:            "GET /containers/logs - no auth should fail",
 			Method:          http.MethodGet,
-			URL:             "/api/beszel/containers/logs?system=test-system&container=test-container",
+			URL:             "/api/beszel/containers/logs?system=test-system&container=abababababab",
 			ExpectedStatus:  401,
 			ExpectedContent: []string{"requires valid"},
 			TestAppFactory:  testAppFactory,
 		},
+		{
+			Name:            "GET /containers/logs - request for valid non-user system should fail",
+			Method:          http.MethodGet,
+			URL:             fmt.Sprintf("/api/beszel/containers/logs?system=%s&container=abababababab", system.Id),
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+		},
+		{
+			Name:            "GET /containers/info - request for valid non-user system should fail",
+			Method:          http.MethodGet,
+			URL:             fmt.Sprintf("/api/beszel/containers/info?system=%s&container=abababababab", system.Id),
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+		},
 		{
 			Name:   "GET /containers/logs - with auth but missing system param should fail",
 			Method: http.MethodGet,
-			URL:    "/api/beszel/containers/logs?container=test-container",
+			URL:    "/api/beszel/containers/logs?container=abababababab",
 			Headers: map[string]string{
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"system and container parameters are required"},
+			ExpectedContent: []string{"Invalid", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
 		{
@@ -324,7 +369,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"system and container parameters are required"},
+			ExpectedContent: []string{"Invalid", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
 		{
@@ -335,7 +380,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  404,
-			ExpectedContent: []string{"system not found"},
+			ExpectedContent: []string{"The requested resource wasn't found."},
 			TestAppFactory:  testAppFactory,
 		},
 		{
@@ -346,7 +391,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"invalid container parameter"},
+			ExpectedContent: []string{"Invalid", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
 		{
@@ -357,7 +402,7 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"invalid container parameter"},
+			ExpectedContent: []string{"Invalid", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
 		{
@@ -368,9 +413,114 @@ func TestApiRoutesAuthentication(t *testing.T) {
 				"Authorization": userToken,
 			},
 			ExpectedStatus:  400,
-			ExpectedContent: []string{"invalid container parameter"},
+			ExpectedContent: []string{"Invalid", "parameter"},
 			TestAppFactory:  testAppFactory,
 		},
+		{
+			Name:   "GET /containers/logs - good user should pass validation",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/containers/logs?system=" + system.Id + "&container=0123456789ab",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  500,
+			ExpectedContent: []string{"Something went wrong while processing your request."},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /containers/info - good user should pass validation",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/containers/info?system=" + system.Id + "&container=0123456789ab",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  500,
+			ExpectedContent: []string{"Something went wrong while processing your request."},
+			TestAppFactory:  testAppFactory,
+		},
+		// /systemd routes
+		{
+			Name:            "GET /systemd/info - no auth should fail",
+			Method:          http.MethodGet,
+			URL:             fmt.Sprintf("/api/beszel/systemd/info?system=%s&service=nginx.service", system.Id),
+			ExpectedStatus:  401,
+			ExpectedContent: []string{"requires valid"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:            "GET /systemd/info - request for valid non-user system should fail",
+			Method:          http.MethodGet,
+			URL:             fmt.Sprintf("/api/beszel/systemd/info?system=%s&service=nginx.service", system.Id),
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+			Headers: map[string]string{
+				"Authorization": user2Token,
+			},
+		},
+		{
+			Name:   "GET /systemd/info - with auth but missing system param should fail",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/systemd/info?service=nginx.service",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  400,
+			ExpectedContent: []string{"Invalid", "parameter"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /systemd/info - with auth but missing service param should fail",
+			Method: http.MethodGet,
+			URL:    fmt.Sprintf("/api/beszel/systemd/info?system=%s", system.Id),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  400,
+			ExpectedContent: []string{"Invalid", "parameter"},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /systemd/info - with auth but invalid system should fail",
+			Method: http.MethodGet,
+			URL:    "/api/beszel/systemd/info?system=invalid-system&service=nginx.service",
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /systemd/info - service not in systemd_services collection should fail",
+			Method: http.MethodGet,
+			URL:    fmt.Sprintf("/api/beszel/systemd/info?system=%s&service=notregistered.service", system.Id),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  404,
+			ExpectedContent: []string{"The requested resource wasn't found."},
+			TestAppFactory:  testAppFactory,
+		},
+		{
+			Name:   "GET /systemd/info - with auth and existing service record should pass validation",
+			Method: http.MethodGet,
+			URL:    fmt.Sprintf("/api/beszel/systemd/info?system=%s&service=nginx.service", system.Id),
+			Headers: map[string]string{
+				"Authorization": userToken,
+			},
+			ExpectedStatus:  500,
+			ExpectedContent: []string{"Something went wrong while processing your request."},
+			TestAppFactory:  testAppFactory,
+			BeforeTestFunc: func(t testing.TB, app *pbTests.TestApp, e *core.ServeEvent) {
+				beszelTests.CreateRecord(app, "systemd_services", map[string]any{
+					"system": system.Id,
+					"name":   "nginx.service",
+					"state":  0,
+					"sub":    1,
+				})
+			},
+		},
 
 		// Auth Optional Routes - Should work without authentication
 		{

internal/hub/hub.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"os"
 	"path"
-	"regexp"
 	"strings"
 
 	"github.com/henrygd/beszel/internal/alerts"
@@ -38,8 +37,6 @@ type Hub struct {
 	appURL string
 }
 
-var containerIDPattern = regexp.MustCompile(`^[a-fA-F0-9]{12,64}$`)
-
 // NewHub creates a new Hub instance with default configuration
 func NewHub(app core.App) *Hub {
 	hub := &Hub{App: app}

internal/hub/server_development.go

@@ -5,7 +5,6 @@ package hub
 import (
 	"fmt"
 	"io"
-	"log/slog"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -62,7 +61,6 @@ func (rm *responseModifier) modifyHTML(html string) string {
 
 // startServer sets up the development server for Beszel
 func (h *Hub) startServer(se *core.ServeEvent) error {
-	slog.Info("starting server", "appURL", h.appURL)
 	proxy := httputil.NewSingleHostReverseProxy(&url.URL{
 		Scheme: "http",
 		Host:   "localhost:5173",

internal/hub/systems/system.go

@@ -8,6 +8,7 @@ import (
 	"hash/fnv"
 	"math/rand"
 	"net"
+	"slices"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -184,7 +185,7 @@ func (sys *System) handlePaused() {
 
 // createRecords updates the system record and adds system_stats and container_stats records
 func (sys *System) createRecords(data *system.CombinedData) (*core.Record, error) {
-	systemRecord, err := sys.getRecord()
+	systemRecord, err := sys.getRecord(sys.manager.hub)
 	if err != nil {
 		return nil, err
 	}
@@ -343,8 +344,8 @@ func createContainerRecords(app core.App, data []*container.Stats, systemId stri
 
 // getRecord retrieves the system record from the database.
 // If the record is not found, it removes the system from the manager.
-func (sys *System) getRecord() (*core.Record, error) {
+func (sys *System) getRecord(app core.App) (*core.Record, error) {
-	record, err := sys.manager.hub.FindRecordById("systems", sys.Id)
+	record, err := app.FindRecordById("systems", sys.Id)
 	if err != nil || record == nil {
 		_ = sys.manager.RemoveSystem(sys.Id)
 		return nil, err
@@ -352,6 +353,16 @@ func (sys *System) getRecord() (*core.Record, error) {
 	return record, nil
 }
 
+// HasUser checks if the given user ID is in the system's users list.
+func (sys *System) HasUser(app core.App, userID string) bool {
+	record, err := sys.getRecord(app)
+	if err != nil {
+		return false
+	}
+	users := record.GetStringSlice("users")
+	return slices.Contains(users, userID)
+}
+
 // setDown marks a system as down in the database.
 // It takes the original error that caused the system to go down and returns any error
 // encountered during the process of updating the system status.
@@ -359,7 +370,7 @@ func (sys *System) setDown(originalError error) error {
 	if sys.Status == down || sys.Status == paused {
 		return nil
 	}
-	record, err := sys.getRecord()
+	record, err := sys.getRecord(sys.manager.hub)
 	if err != nil {
 		return err
 	}