From 1b221cca7188d5b393240d02b4b16a2e955b19c8 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 9 Nov 2025 08:31:12 +0000 Subject: [PATCH] feat: Add configurable allowlist for webhook private IP targets (addresses #673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow homelab users to send webhooks to internal services while maintaining security defaults. Changes: - Add webhookAllowedPrivateCIDRs field to SystemSettings (persistent config) - Implement CIDR parsing and validation in NotificationManager - Convert ValidateWebhookURL to instance method to access allowlist - Add UI controls in System Settings for configuring trusted CIDR ranges - Maintain strict security by default (block all private IPs) - Keep localhost, link-local, and cloud metadata services blocked regardless of allowlist - Re-validate on both config save and webhook delivery (DNS rebinding protection) - Add comprehensive tests for CIDR parsing and IP matching Backend: - UpdateAllowedPrivateCIDRs() parses comma-separated CIDRs with validation - Support for bare IPs (auto-converts to /32 or /128) - Thread-safe allowlist updates with RWMutex - Logging when allowlist is updated or used - Validation errors prevent invalid CIDRs from being saved Frontend: - New "Webhook Security" section in System Settings - Input field with examples and helpful placeholder text - Real-time unsaved changes tracking - Loads and saves allowlist via system settings API Security: - Default behavior unchanged (all private IPs blocked) - Explicit opt-in required via configuration - Localhost (127/8) always blocked - Link-local (169.254/16) always blocked - Cloud metadata services always blocked - DNS resolution checked at both save and send time Testing: - Tests for CIDR parsing (valid/invalid inputs) - Tests for IP allowlist matching - Tests for bare IP address handling - Tests for security boundaries (localhost, link-local remain blocked) Related to #673 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/components/Settings/Settings.tsx | 54 ++++++ frontend-modern/src/types/config.ts | 1 + internal/api/notifications.go | 4 +- internal/api/router.go | 9 + internal/api/system_settings.go | 19 ++ internal/config/persistence.go | 1 + internal/notifications/notifications.go | 100 +++++++++- .../notifications/webhook_allowlist_test.go | 171 ++++++++++++++++++ internal/notifications/webhook_enhanced.go | 4 +- 9 files changed, 352 insertions(+), 11 deletions(-) create mode 100644 internal/notifications/webhook_allowlist_test.go diff --git a/frontend-modern/src/components/Settings/Settings.tsx b/frontend-modern/src/components/Settings/Settings.tsx index c678a9d..44e3332 100644 --- a/frontend-modern/src/components/Settings/Settings.tsx +++ b/frontend-modern/src/components/Settings/Settings.tsx @@ -644,6 +644,9 @@ const Settings: Component = (props) => { const [allowEmbedding, setAllowEmbedding] = createSignal(false); const [allowedEmbedOrigins, setAllowedEmbedOrigins] = createSignal(''); + // Webhook security settings + const [webhookAllowedPrivateCIDRs, setWebhookAllowedPrivateCIDRs] = createSignal(''); + // Update settings const [versionInfo, setVersionInfo] = createSignal(null); const [updateInfo, setUpdateInfo] = createSignal(null); @@ -1659,6 +1662,8 @@ const Settings: Component = (props) => { // Load embedding settings setAllowEmbedding(systemSettings.allowEmbedding ?? false); setAllowedEmbedOrigins(systemSettings.allowedEmbedOrigins || ''); + // Load webhook security settings + setWebhookAllowedPrivateCIDRs(systemSettings.webhookAllowedPrivateCIDRs || ''); setTemperatureMonitoringEnabled( typeof systemSettings.temperatureMonitoringEnabled === 'boolean' ? systemSettings.temperatureMonitoringEnabled @@ -1775,6 +1780,7 @@ const Settings: Component = (props) => { backupPollingInterval: backupPollingInterval(), allowEmbedding: allowEmbedding(), allowedEmbedOrigins: allowedEmbedOrigins(), + webhookAllowedPrivateCIDRs: webhookAllowedPrivateCIDRs(), }); } @@ -3759,6 +3765,54 @@ const Settings: Component = (props) => { + {/* Webhook Security Settings */} +
+

+ + + + Webhook Security +

+
+
+ +

+ By default, webhooks to private IP addresses are blocked for + security. Enter trusted CIDR ranges to allow webhooks to internal + services (leave empty to block all private IPs). +

+ { + setWebhookAllowedPrivateCIDRs(e.currentTarget.value); + setHasUnsavedChanges(true); + }} + placeholder="192.168.1.0/24, 10.0.0.0/8" + class="w-full px-3 py-1.5 text-sm border rounded-lg border-gray-300 dark:border-gray-600 bg-white dark:bg-gray-800" + /> +

+ Example: 192.168.1.0/24,10.0.0.0/8 allows webhooks to + these private networks. Localhost and cloud metadata services + remain blocked. +

+
+
+
+ = 16 && second <= 31 } +// UpdateAllowedPrivateCIDRs parses and updates the list of allowed private CIDR ranges for webhooks +func (n *NotificationManager) UpdateAllowedPrivateCIDRs(cidrsString string) error { + n.allowedPrivateMu.Lock() + defer n.allowedPrivateMu.Unlock() + + // Clear existing allowlist + n.allowedPrivateNets = nil + + // Empty string means no allowlist (block all private IPs) + if cidrsString == "" { + log.Info().Msg("Webhook private IP allowlist cleared - all private IPs blocked") + return nil + } + + // Parse comma-separated CIDRs + cidrs := strings.Split(cidrsString, ",") + var parsedNets []*net.IPNet + + for _, cidr := range cidrs { + cidr = strings.TrimSpace(cidr) + if cidr == "" { + continue + } + + // Support bare IPs by adding /32 or /128 + if !strings.Contains(cidr, "/") { + ip := net.ParseIP(cidr) + if ip == nil { + return fmt.Errorf("invalid IP address: %s", cidr) + } + if ip.To4() != nil { + cidr = cidr + "/32" + } else { + cidr = cidr + "/128" + } + } + + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + return fmt.Errorf("invalid CIDR range %s: %w", cidr, err) + } + + parsedNets = append(parsedNets, ipNet) + } + + n.allowedPrivateNets = parsedNets + log.Info(). + Str("cidrs", cidrsString). + Int("count", len(parsedNets)). + Msg("Webhook private IP allowlist updated") + + return nil +} + +// isIPInAllowlist checks if an IP is in the configured allowlist +func (n *NotificationManager) isIPInAllowlist(ip net.IP) bool { + n.allowedPrivateMu.RLock() + defer n.allowedPrivateMu.RUnlock() + + // No allowlist means block all private IPs + if len(n.allowedPrivateNets) == 0 { + return false + } + + // Check if IP is in any allowed range + for _, ipNet := range n.allowedPrivateNets { + if ipNet.Contains(ip) { + return true + } + } + + return false +} + // addWebhookDelivery adds a webhook delivery record to the history func (n *NotificationManager) addWebhookDelivery(delivery WebhookDelivery) { n.mu.Lock() diff --git a/internal/notifications/webhook_allowlist_test.go b/internal/notifications/webhook_allowlist_test.go new file mode 100644 index 0000000..bcf8c3e --- /dev/null +++ b/internal/notifications/webhook_allowlist_test.go @@ -0,0 +1,171 @@ +package notifications + +import ( + "net" + "testing" +) + +func TestUpdateAllowedPrivateCIDRs(t *testing.T) { + nm := NewNotificationManager("") + + tests := []struct { + name string + cidrs string + wantErr bool + }{ + { + name: "empty string clears allowlist", + cidrs: "", + wantErr: false, + }, + { + name: "single valid CIDR", + cidrs: "192.168.1.0/24", + wantErr: false, + }, + { + name: "multiple valid CIDRs", + cidrs: "192.168.1.0/24,10.0.0.0/8", + wantErr: false, + }, + { + name: "CIDR with spaces", + cidrs: "192.168.1.0/24, 10.0.0.0/8", + wantErr: false, + }, + { + name: "bare IPv4 address", + cidrs: "192.168.1.1", + wantErr: false, + }, + { + name: "bare IPv6 address", + cidrs: "fe80::1", + wantErr: false, + }, + { + name: "invalid CIDR", + cidrs: "not-a-cidr", + wantErr: true, + }, + { + name: "invalid IP address", + cidrs: "999.999.999.999", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := nm.UpdateAllowedPrivateCIDRs(tt.cidrs) + if (err != nil) != tt.wantErr { + t.Errorf("UpdateAllowedPrivateCIDRs() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestIsIPInAllowlist(t *testing.T) { + nm := NewNotificationManager("") + + // Set up allowlist + err := nm.UpdateAllowedPrivateCIDRs("192.168.1.0/24,10.0.0.0/8") + if err != nil { + t.Fatalf("Failed to setup allowlist: %v", err) + } + + tests := []struct { + name string + ip string + expected bool + }{ + { + name: "IP in first CIDR range", + ip: "192.168.1.100", + expected: true, + }, + { + name: "IP in second CIDR range", + ip: "10.5.10.20", + expected: true, + }, + { + name: "IP not in any range", + ip: "172.16.1.1", + expected: false, + }, + { + name: "IP at network boundary", + ip: "192.168.1.0", + expected: true, + }, + { + name: "IP at broadcast boundary", + ip: "192.168.1.255", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if ip == nil { + t.Fatalf("Invalid test IP: %s", tt.ip) + } + result := nm.isIPInAllowlist(ip) + if result != tt.expected { + t.Errorf("isIPInAllowlist(%s) = %v, expected %v", tt.ip, result, tt.expected) + } + }) + } +} + +func TestIsIPInAllowlistEmptyList(t *testing.T) { + nm := NewNotificationManager("") + + // Empty allowlist should block all IPs + ip := net.ParseIP("192.168.1.1") + if nm.isIPInAllowlist(ip) { + t.Error("Empty allowlist should block all private IPs") + } +} + +func TestValidateWebhookURLWithAllowlist(t *testing.T) { + nm := NewNotificationManager("") + + // Test without allowlist (should block private IPs) + err := nm.ValidateWebhookURL("http://192.168.1.100/webhook") + if err == nil { + t.Error("Expected error for private IP without allowlist") + } + + // Set up allowlist + err = nm.UpdateAllowedPrivateCIDRs("192.168.1.0/24") + if err != nil { + t.Fatalf("Failed to setup allowlist: %v", err) + } + + // Should now allow the private IP in the allowlist + err = nm.ValidateWebhookURL("http://192.168.1.100/webhook") + if err != nil { + t.Errorf("Expected no error for private IP in allowlist, got: %v", err) + } + + // Should still block private IPs not in allowlist + err = nm.ValidateWebhookURL("http://10.0.0.1/webhook") + if err == nil { + t.Error("Expected error for private IP not in allowlist") + } + + // Should always block localhost regardless of allowlist + err = nm.ValidateWebhookURL("http://localhost/webhook") + if err == nil { + t.Error("Expected error for localhost even with allowlist") + } + + // Should always block link-local regardless of allowlist + err = nm.ValidateWebhookURL("http://169.254.169.254/webhook") + if err == nil { + t.Error("Expected error for link-local even with allowlist") + } +} diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index 60f43ea..36b9e73 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -401,7 +401,7 @@ func (n *NotificationManager) sendWebhookOnceWithResponse(webhook EnhancedWebhoo req.Header.Set("User-Agent", "Pulse-Monitoring/2.0") // Send request with secure client - client := createSecureWebhookClient(WebhookTimeout) + client := n.createSecureWebhookClient(WebhookTimeout) resp, err := client.Do(req) if err != nil { @@ -600,7 +600,7 @@ func (n *NotificationManager) TestEnhancedWebhook(webhook EnhancedWebhookConfig) req.Header.Set("User-Agent", "Pulse-Monitoring/2.0 (Test)") // Send with shorter timeout for testing - client := createSecureWebhookClient(WebhookTestTimeout) + client := n.createSecureWebhookClient(WebhookTestTimeout) resp, err := client.Do(req) if err != nil {