From ba10da1b9f13455f2879336445102a45eb6cb07b Mon Sep 17 00:00:00 2001 From: henrygd Date: Wed, 1 Apr 2026 16:30:45 -0400 Subject: [PATCH] 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 | 39 +++--- internal/hub/api_test.go | 184 ++++++++++++++++++++++++++--- internal/hub/hub.go | 3 - internal/hub/server_development.go | 2 - internal/hub/systems/system.go | 19 ++- 5 files changed, 205 insertions(+), 42 deletions(-) diff --git a/internal/hub/api.go b/internal/hub/api.go index 9d0fc3f4..4c1085a8 100644 --- a/internal/hub/api.go +++ b/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 == "" { - return e.JSON(http.StatusBadRequest, map[string]string{"error": "system and container parameters are required"}) - } - if !containerIDPattern.MatchString(containerID) { - return e.JSON(http.StatusBadRequest, map[string]string{"error": "invalid container parameter"}) + if systemID == "" || containerID == "" || !containerIDPattern.MatchString(containerID) { + return e.BadRequestError("Invalid system or container parameter", nil) } system, err := h.sm.GetSystem(systemID) - if err != nil { - return e.JSON(http.StatusNotFound, map[string]string{"error": "system not found"}) + if err != nil || !system.HasUser(e.App, e.Auth.Id) { + 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 { - return e.JSON(http.StatusNotFound, map[string]string{"error": "system not found"}) + if err != nil || !system.HasUser(e.App, e.Auth.Id) { + 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"}) diff --git a/internal/hub/api_test.go b/internal/hub/api_test.go index 70d68296..df7cdd80 100644 --- a/internal/hub/api_test.go +++ b/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() - require.NoError(t, err, "Failed to create auth token") - - // Create test system for user-alerts endpoints + // Create test system 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 { diff --git a/internal/hub/hub.go b/internal/hub/hub.go index 0c5c222e..badca8c4 100644 --- a/internal/hub/hub.go +++ b/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} diff --git a/internal/hub/server_development.go b/internal/hub/server_development.go index 0a39df7a..b32dc66a 100644 --- a/internal/hub/server_development.go +++ b/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", diff --git a/internal/hub/systems/system.go b/internal/hub/systems/system.go index e57b0d39..28f357a3 100644 --- a/internal/hub/systems/system.go +++ b/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) { - record, err := sys.manager.hub.FindRecordById("systems", sys.Id) +func (sys *System) getRecord(app core.App) (*core.Record, error) { + 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 }