From c3dffff5e46b0604e8b4695f1eaa32e073c670f2 Mon Sep 17 00:00:00 2001 From: henrygd Date: Tue, 7 Apr 2026 16:08:28 -0400 Subject: [PATCH] hub: prevent non-admin users from sending test alerts to internal hosts --- internal/alerts/alerts_api.go | 57 ++++++++ internal/alerts/alerts_api_test.go | 133 ++++++++++++++++++ internal/alerts/alerts_test_helpers.go | 4 + internal/hub/api_test.go | 25 ---- .../routes/settings/notifications.tsx | 47 ++++--- 5 files changed, 220 insertions(+), 46 deletions(-) diff --git a/internal/alerts/alerts_api.go b/internal/alerts/alerts_api.go index 7138f537..8c9e92ef 100644 --- a/internal/alerts/alerts_api.go +++ b/internal/alerts/alerts_api.go @@ -3,7 +3,11 @@ package alerts import ( "database/sql" "errors" + "net" "net/http" + "net/url" + "slices" + "strings" "github.com/pocketbase/dbx" "github.com/pocketbase/pocketbase/core" @@ -127,9 +131,62 @@ func (am *AlertManager) SendTestNotification(e *core.RequestEvent) error { if err != nil || data.URL == "" { return e.BadRequestError("URL is required", err) } + // Only allow admins to send test notifications to internal URLs + if !e.Auth.IsSuperuser() && e.Auth.GetString("role") != "admin" { + internalURL, err := isInternalURL(data.URL) + if err != nil { + return e.BadRequestError(err.Error(), nil) + } + if internalURL { + return e.ForbiddenError("Only admins can send to internal destinations", nil) + } + } err = am.SendShoutrrrAlert(data.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()}) } return e.JSON(200, map[string]bool{"err": false}) } + +// isInternalURL checks if the given shoutrrr URL points to an internal destination (localhost or private IP) +func isInternalURL(rawURL string) (bool, error) { + parsedURL, err := url.Parse(rawURL) + if err != nil { + return false, err + } + + host := parsedURL.Hostname() + if host == "" { + return false, nil + } + + if strings.EqualFold(host, "localhost") { + return true, nil + } + + if ip := net.ParseIP(host); ip != nil { + return isInternalIP(ip), nil + } + + // Some Shoutrrr URLs use the host position for service identifiers rather than a + // network hostname (for example, discord://token@webhookid). Restrict DNS lookups + // to names that look like actual hostnames so valid service URLs keep working. + if !strings.Contains(host, ".") { + return false, nil + } + + ips, err := net.LookupIP(host) + if err != nil { + return false, nil + } + + if slices.ContainsFunc(ips, isInternalIP) { + return true, nil + } + + return false, nil +} + +func isInternalIP(ip net.IP) bool { + return ip.IsPrivate() || ip.IsLoopback() || ip.IsUnspecified() +} diff --git a/internal/alerts/alerts_api_test.go b/internal/alerts/alerts_api_test.go index 954fb456..dccb667f 100644 --- a/internal/alerts/alerts_api_test.go +++ b/internal/alerts/alerts_api_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/henrygd/beszel/internal/alerts" beszelTests "github.com/henrygd/beszel/internal/tests" pbTests "github.com/pocketbase/pocketbase/tests" @@ -28,6 +29,31 @@ func jsonReader(v any) io.Reader { return bytes.NewReader(data) } +func TestIsInternalURL(t *testing.T) { + testCases := []struct { + name string + url string + internal bool + }{ + {name: "loopback ipv4", url: "generic://127.0.0.1", internal: true}, + {name: "localhost hostname", url: "generic://localhost", internal: true}, + {name: "localhost hostname", url: "generic+http://localhost/api/v1/postStuff", internal: true}, + {name: "localhost hostname", url: "generic+http://127.0.0.1:8080/api/v1/postStuff", internal: true}, + {name: "localhost hostname", url: "generic+https://beszel.dev/api/v1/postStuff", internal: false}, + {name: "public ipv4", url: "generic://8.8.8.8", internal: false}, + {name: "token style service url", url: "discord://abc123@123456789", internal: false}, + {name: "single label service url", url: "slack://token@team/channel", internal: false}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + internal, err := alerts.IsInternalURL(testCase.url) + assert.NoError(t, err) + assert.Equal(t, testCase.internal, internal) + }) + } +} + func TestUserAlertsApi(t *testing.T) { hub, _ := beszelTests.NewTestHub(t.TempDir()) defer hub.Cleanup() @@ -366,3 +392,110 @@ func TestUserAlertsApi(t *testing.T) { scenario.Test(t) } } +func TestSendTestNotification(t *testing.T) { + hub, user := beszelTests.GetHubWithUser(t) + defer hub.Cleanup() + + userToken, err := user.NewAuthToken() + + adminUser, err := beszelTests.CreateUserWithRole(hub, "admin@example.com", "password123", "admin") + assert.NoError(t, err, "Failed to create admin user") + adminUserToken, err := adminUser.NewAuthToken() + + superuser, err := beszelTests.CreateSuperuser(hub, "superuser@example.com", "password123") + assert.NoError(t, err, "Failed to create superuser") + superuserToken, err := superuser.NewAuthToken() + assert.NoError(t, err, "Failed to create superuser auth token") + + testAppFactory := func(t testing.TB) *pbTests.TestApp { + return hub.TestApp + } + + scenarios := []beszelTests.ApiScenario{ + { + Name: "POST /test-notification - no auth should fail", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + ExpectedStatus: 401, + ExpectedContent: []string{"requires valid"}, + TestAppFactory: testAppFactory, + Body: jsonReader(map[string]any{ + "url": "generic://127.0.0.1", + }), + }, + { + Name: "POST /test-notification - with external auth should succeed", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + TestAppFactory: testAppFactory, + Headers: map[string]string{ + "Authorization": userToken, + }, + Body: jsonReader(map[string]any{ + "url": "generic://8.8.8.8", + }), + ExpectedStatus: 200, + ExpectedContent: []string{"\"err\":"}, + }, + { + Name: "POST /test-notification - local url with user auth should fail", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + TestAppFactory: testAppFactory, + Headers: map[string]string{ + "Authorization": userToken, + }, + Body: jsonReader(map[string]any{ + "url": "generic://localhost:8010", + }), + ExpectedStatus: 403, + ExpectedContent: []string{"Only admins"}, + }, + { + Name: "POST /test-notification - internal url with user auth should fail", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + TestAppFactory: testAppFactory, + Headers: map[string]string{ + "Authorization": userToken, + }, + Body: jsonReader(map[string]any{ + "url": "generic+http://192.168.0.5", + }), + ExpectedStatus: 403, + ExpectedContent: []string{"Only admins"}, + }, + { + Name: "POST /test-notification - internal url with admin auth should succeed", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + TestAppFactory: testAppFactory, + Headers: map[string]string{ + "Authorization": adminUserToken, + }, + Body: jsonReader(map[string]any{ + "url": "generic://127.0.0.1", + }), + ExpectedStatus: 200, + ExpectedContent: []string{"\"err\":"}, + }, + { + Name: "POST /test-notification - internal url with superuser auth should succeed", + Method: http.MethodPost, + URL: "/api/beszel/test-notification", + TestAppFactory: testAppFactory, + Headers: map[string]string{ + "Authorization": superuserToken, + }, + Body: jsonReader(map[string]any{ + "url": "generic://127.0.0.1", + }), + ExpectedStatus: 200, + ExpectedContent: []string{"\"err\":"}, + }, + } + + for _, scenario := range scenarios { + scenario.Test(t) + } +} diff --git a/internal/alerts/alerts_test_helpers.go b/internal/alerts/alerts_test_helpers.go index f87cd5d6..6567f76a 100644 --- a/internal/alerts/alerts_test_helpers.go +++ b/internal/alerts/alerts_test_helpers.go @@ -95,3 +95,7 @@ func (am *AlertManager) RestorePendingStatusAlerts() error { func (am *AlertManager) SetAlertTriggered(alert CachedAlertData, triggered bool) error { return am.setAlertTriggered(alert, triggered) } + +func IsInternalURL(rawURL string) (bool, error) { + return isInternalURL(rawURL) +} diff --git a/internal/hub/api_test.go b/internal/hub/api_test.go index 8c8e3bd5..7981dfd6 100644 --- a/internal/hub/api_test.go +++ b/internal/hub/api_test.go @@ -66,31 +66,6 @@ func TestApiRoutesAuthentication(t *testing.T) { scenarios := []beszelTests.ApiScenario{ // Auth Protected Routes - Should require authentication - { - Name: "POST /test-notification - no auth should fail", - Method: http.MethodPost, - URL: "/api/beszel/test-notification", - ExpectedStatus: 401, - ExpectedContent: []string{"requires valid"}, - TestAppFactory: testAppFactory, - Body: jsonReader(map[string]any{ - "url": "generic://127.0.0.1", - }), - }, - { - Name: "POST /test-notification - with auth should succeed", - Method: http.MethodPost, - URL: "/api/beszel/test-notification", - TestAppFactory: testAppFactory, - Headers: map[string]string{ - "Authorization": userToken, - }, - Body: jsonReader(map[string]any{ - "url": "generic://127.0.0.1", - }), - ExpectedStatus: 200, - ExpectedContent: []string{"sending message"}, - }, { Name: "GET /config-yaml - no auth should fail", Method: http.MethodGet, diff --git a/internal/site/src/components/routes/settings/notifications.tsx b/internal/site/src/components/routes/settings/notifications.tsx index 8f570719..3650d24b 100644 --- a/internal/site/src/components/routes/settings/notifications.tsx +++ b/internal/site/src/components/routes/settings/notifications.tsx @@ -15,6 +15,7 @@ import { isAdmin, pb } from "@/lib/api" import type { UserSettings } from "@/types" import { saveSettings } from "./layout" import { QuietHours } from "./quiet-hours" +import type { ClientResponseError } from "pocketbase" interface ShoutrrrUrlCardProps { url: string @@ -59,10 +60,10 @@ const SettingsNotificationsPage = ({ userSettings }: { userSettings: UserSetting try { const parsedData = v.parse(NotificationSchema, { emails, webhooks }) await saveSettings(parsedData) - } catch (e: any) { + } catch (e: unknown) { toast({ title: t`Failed to save settings`, - description: e.message, + description: (e as Error).message, variant: "destructive", }) } @@ -136,12 +137,7 @@ const SettingsNotificationsPage = ({ userSettings }: { userSettings: UserSetting

-