diff --git a/Dockerfile b/Dockerfile index cf33365c..c423e6dd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -486,8 +486,7 @@ COPY scripts/ /app/scripts/ RUN chmod +x /app/scripts/db-recovery.sh # Set default environment variables -ENV GODEBUG=netdns=go \ - CHARON_ENV=production \ +ENV CHARON_ENV=production \ CHARON_DB_PATH=/app/data/charon.db \ CHARON_FRONTEND_DIR=/app/frontend/dist \ CHARON_CADDY_ADMIN_API=http://localhost:2019 \ diff --git a/backend/internal/api/handlers/notification_provider_handler.go b/backend/internal/api/handlers/notification_provider_handler.go index c39929cb..cd956891 100644 --- a/backend/internal/api/handlers/notification_provider_handler.go +++ b/backend/internal/api/handlers/notification_provider_handler.go @@ -48,7 +48,7 @@ func (h *NotificationProviderHandler) Create(c *gin.Context) { if err := h.service.CreateProvider(&provider); err != nil { // If it's a validation error from template parsing, return 400 - if strings.Contains(err.Error(), "invalid custom template") || strings.Contains(err.Error(), "rendered template") || strings.Contains(err.Error(), "failed to parse template") || strings.Contains(err.Error(), "failed to render template") { + if isProviderValidationError(err) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } @@ -75,7 +75,7 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { provider.ID = id if err := h.service.UpdateProvider(&provider); err != nil { - if strings.Contains(err.Error(), "invalid custom template") || strings.Contains(err.Error(), "rendered template") || strings.Contains(err.Error(), "failed to parse template") || strings.Contains(err.Error(), "failed to render template") { + if isProviderValidationError(err) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } @@ -88,6 +88,19 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { c.JSON(http.StatusOK, provider) } +func isProviderValidationError(err error) bool { + if err == nil { + return false + } + + errMsg := err.Error() + return strings.Contains(errMsg, "invalid custom template") || + strings.Contains(errMsg, "rendered template") || + strings.Contains(errMsg, "failed to parse template") || + strings.Contains(errMsg, "failed to render template") || + strings.Contains(errMsg, "invalid Discord webhook URL") +} + func (h *NotificationProviderHandler) Delete(c *gin.Context) { if !requireAdmin(c) { return diff --git a/backend/internal/api/handlers/notification_provider_handler_test.go b/backend/internal/api/handlers/notification_provider_handler_test.go index 77a1f950..39a05de9 100644 --- a/backend/internal/api/handlers/notification_provider_handler_test.go +++ b/backend/internal/api/handlers/notification_provider_handler_test.go @@ -232,3 +232,37 @@ func TestNotificationProviderHandler_Preview(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) } + +func TestNotificationProviderHandler_CreateRejectsDiscordIPHost(t *testing.T) { + r, _ := setupNotificationProviderTest(t) + + provider := models.NotificationProvider{ + Name: "Discord IP", + Type: "discord", + URL: "https://203.0.113.10/api/webhooks/123456/token_abc", + } + body, _ := json.Marshal(provider) + req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body)) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "invalid Discord webhook URL") + assert.Contains(t, w.Body.String(), "IP address hosts are not allowed") +} + +func TestNotificationProviderHandler_CreateAcceptsDiscordHostname(t *testing.T) { + r, _ := setupNotificationProviderTest(t) + + provider := models.NotificationProvider{ + Name: "Discord Host", + Type: "discord", + URL: "https://discord.com/api/webhooks/123456/token_abc", + } + body, _ := json.Marshal(provider) + req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body)) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusCreated, w.Code) +} diff --git a/backend/internal/services/notification_service.go b/backend/internal/services/notification_service.go index 69bbabca..996f1c99 100644 --- a/backend/internal/services/notification_service.go +++ b/backend/internal/services/notification_service.go @@ -34,6 +34,11 @@ func NewNotificationService(db *gorm.DB) *NotificationService { var discordWebhookRegex = regexp.MustCompile(`^https://discord(?:app)?\.com/api/webhooks/(\d+)/([a-zA-Z0-9_-]+)`) +var allowedDiscordWebhookHosts = map[string]struct{}{ + "discord.com": {}, + "canary.discord.com": {}, +} + func normalizeURL(serviceType, rawURL string) string { if serviceType == "discord" { matches := discordWebhookRegex.FindStringSubmatch(rawURL) @@ -46,33 +51,42 @@ func normalizeURL(serviceType, rawURL string) string { return rawURL } -func canonicalizeDiscordWebhookURL(providerType, rawURL string) string { - if !strings.EqualFold(providerType, "discord") { - return rawURL - } - +func validateDiscordWebhookURL(rawURL string) error { parsedURL, err := neturl.Parse(rawURL) if err != nil { - return rawURL + return fmt.Errorf("invalid Discord webhook URL: failed to parse URL; use the HTTPS webhook URL provided by Discord") + } + + if strings.EqualFold(parsedURL.Scheme, "discord") { + return nil } if !strings.EqualFold(parsedURL.Scheme, "https") { - return rawURL + return fmt.Errorf("invalid Discord webhook URL: URL must use HTTPS and the hostname URL provided by Discord") } - hostname := parsedURL.Hostname() - if net.ParseIP(hostname) == nil { - return rawURL + hostname := strings.ToLower(parsedURL.Hostname()) + if hostname == "" { + return fmt.Errorf("invalid Discord webhook URL: missing hostname; use the HTTPS webhook URL provided by Discord") } - port := parsedURL.Port() - if port == "" || port == "443" { - parsedURL.Host = "discord.com" - } else { - parsedURL.Host = net.JoinHostPort("discord.com", port) + if net.ParseIP(hostname) != nil { + return fmt.Errorf("invalid Discord webhook URL: IP address hosts are not allowed; use the hostname URL provided by Discord (discord.com or canary.discord.com)") } - return parsedURL.String() + if _, ok := allowedDiscordWebhookHosts[hostname]; !ok { + return fmt.Errorf("invalid Discord webhook URL: host must be discord.com or canary.discord.com; use the hostname URL provided by Discord") + } + + return nil +} + +func validateDiscordProviderURL(providerType, rawURL string) error { + if !strings.EqualFold(providerType, "discord") { + return nil + } + + return validateDiscordWebhookURL(rawURL) } // supportsJSONTemplates returns true if the provider type can use JSON templates @@ -196,6 +210,12 @@ func (s *NotificationService) SendExternal(ctx context.Context, eventType, title // In production it defaults to shoutrrr.Send. var shoutrrrSendFunc = shoutrrr.Send +// webhookDoRequestFunc is a test hook for outbound JSON webhook requests. +// In production it defaults to (*http.Client).Do. +var webhookDoRequestFunc = func(client *http.Client, req *http.Request) (*http.Response, error) { + return client.Do(req) +} + func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.NotificationProvider, data map[string]any) error { // Built-in templates const minimalTemplate = `{"message": {{toJSON .Message}}, "title": {{toJSON .Title}}, "time": {{toJSON .Time}}, "event": {{toJSON .EventType}}}` @@ -234,7 +254,11 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti // Additionally, we apply `isValidRedirectURL` as a barrier-guard style predicate. // CodeQL recognizes this pattern as a sanitizer for untrusted URL values, while // the real SSRF protection remains `security.ValidateExternalURL`. - webhookURL := canonicalizeDiscordWebhookURL(p.Type, p.URL) + if err := validateDiscordProviderURL(p.Type, p.URL); err != nil { + return err + } + + webhookURL := p.URL if !isValidRedirectURL(webhookURL) { return fmt.Errorf("invalid webhook url") @@ -322,81 +346,7 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti network.WithAllowLocalhost(), // Allow localhost for testing ) - // Resolve the hostname to an explicit IP and construct the request URL using the - // resolved IP. This prevents direct user-controlled hostnames from being used - // as the request's destination (SSRF mitigation) and helps CodeQL validate the - // sanitisation performed by security.ValidateExternalURL. - // - // NOTE (security): The following mitigations are intentionally applied to - // reduce SSRF/request-forgery risk: - // - security.ValidateExternalURL enforces http(s) schemes and rejects private IPs - // (except explicit localhost for testing) after DNS resolution. - // - We perform an additional DNS resolution here and choose a non-private - // IP to use as the TCP destination to avoid direct hostname-based routing. - // - We set the request's `Host` header to the original hostname so virtual - // hosting works while the actual socket connects to a resolved IP. - // - The HTTP client disables automatic redirects and has a short timeout. - // Together these steps make the request destination unambiguous and prevent - // accidental requests to internal networks. If your threat model requires - // stricter controls, consider an explicit allowlist of webhook hostnames. - // Re-parse the validated URL string to get hostname for DNS lookup. - // This uses the sanitized string rather than the original tainted input. - validatedURL, _ := neturl.Parse(validatedURLStr) - - // Normalize scheme to a constant value derived from an allowlisted set. - // This avoids propagating the original input string directly into request construction. - var safeScheme string - switch validatedURL.Scheme { - case "http": - safeScheme = "http" - case "https": - safeScheme = "https" - default: - return fmt.Errorf("invalid webhook url: unsupported scheme") - } - ips, err := net.LookupIP(validatedURL.Hostname()) - if err != nil || len(ips) == 0 { - return fmt.Errorf("failed to resolve webhook host: %w", err) - } - // If hostname is local loopback, accept loopback addresses; otherwise pick - // the first non-private IP (security.ValidateExternalURL already ensured these - // are not private, but check again defensively). - var selectedIP net.IP - for _, ip := range ips { - if validatedURL.Hostname() == "localhost" || validatedURL.Hostname() == "127.0.0.1" || validatedURL.Hostname() == "::1" { - selectedIP = ip - break - } - if !isPrivateIP(ip) { - selectedIP = ip - break - } - } - if selectedIP == nil { - return fmt.Errorf("failed to find non-private IP for webhook host: %s", validatedURL.Hostname()) - } - - port := validatedURL.Port() - if port == "" { - if safeScheme == "https" { - port = "443" - } else { - port = "80" - } - } - // Construct a safe URL using the resolved IP:port for the Host component, - // while preserving the original path and query from the validated URL. - // This makes the destination hostname unambiguously an IP that we resolved - // and prevents accidental requests to private/internal addresses. - // Using validatedURL (derived from validatedURLStr) breaks the CodeQL taint chain. - safeURL := &neturl.URL{ - Scheme: safeScheme, - Host: net.JoinHostPort(selectedIP.String(), port), - Path: validatedURL.Path, - RawQuery: validatedURL.RawQuery, - } - - req, err := http.NewRequestWithContext(ctx, "POST", safeURL.String(), &body) + req, err := http.NewRequestWithContext(ctx, "POST", validatedURLStr, &body) if err != nil { return fmt.Errorf("failed to create webhook request: %w", err) } @@ -407,22 +357,15 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti req.Header.Set("X-Request-ID", ridStr) } } - // Preserve original hostname for virtual host (Host header) - // Using validatedURL.Host ensures we're using the sanitized value. - req.Host = validatedURL.Host - - // We validated the URL and resolved the hostname to an explicit IP above. - // The request uses the resolved IP (selectedIP) and we also set the - // Host header to the original hostname, so virtual-hosting works while - // preventing requests to private or otherwise disallowed addresses. - // This mitigates SSRF and addresses the CodeQL request-forgery rule. + // Safe: URL validated by security.ValidateExternalURL() which validates URL + // format/scheme and blocks private/reserved destinations through DNS+dial-time checks. // Safe: URL validated by security.ValidateExternalURL() which: // 1. Validates URL format and scheme (HTTPS required in production) // 2. Resolves DNS and blocks private/reserved IPs (RFC 1918, loopback, link-local) // 3. Uses ssrfSafeDialer for connection-time IP revalidation (TOCTOU protection) // 4. No redirect following allowed // See: internal/security/url_validator.go - resp, err := client.Do(req) + resp, err := webhookDoRequestFunc(client, req) if err != nil { return fmt.Errorf("failed to send webhook: %w", err) } @@ -459,6 +402,10 @@ func isValidRedirectURL(rawURL string) bool { } func (s *NotificationService) TestProvider(provider models.NotificationProvider) error { + if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil { + return err + } + if supportsJSONTemplates(provider.Type) && provider.Template != "" { data := map[string]any{ "Title": "Test Notification", @@ -574,6 +521,10 @@ func (s *NotificationService) ListProviders() ([]models.NotificationProvider, er } func (s *NotificationService) CreateProvider(provider *models.NotificationProvider) error { + if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil { + return err + } + // Validate custom template before creating if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" { // Provide a minimal preview payload @@ -586,6 +537,10 @@ func (s *NotificationService) CreateProvider(provider *models.NotificationProvid } func (s *NotificationService) UpdateProvider(provider *models.NotificationProvider) error { + if err := validateDiscordProviderURL(provider.Type, provider.URL); err != nil { + return err + } + // Validate custom template before saving if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" { payload := map[string]any{"Title": "Preview", "Message": "Preview", "Time": time.Now().Format(time.RFC3339), "EventType": "preview"} diff --git a/backend/internal/services/notification_service_json_test.go b/backend/internal/services/notification_service_json_test.go index 75dff222..ce195519 100644 --- a/backend/internal/services/notification_service_json_test.go +++ b/backend/internal/services/notification_service_json_test.go @@ -43,6 +43,91 @@ func TestSupportsJSONTemplates(t *testing.T) { } } +func TestSendJSONPayload_DiscordIPHostRejected(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) + + svc := NewNotificationService(db) + + provider := models.NotificationProvider{ + Type: "discord", + URL: "https://203.0.113.10/api/webhooks/123456/token_abc", + Template: "custom", + Config: `{"content": {{toJSON .Message}}, "username": "Charon"}`, + } + + data := map[string]any{ + "Message": "Test notification", + "Title": "Test", + "Time": time.Now().Format(time.RFC3339), + } + + err = svc.sendJSONPayload(context.Background(), provider, data) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid Discord webhook URL") + assert.Contains(t, err.Error(), "IP address hosts are not allowed") +} + +func TestValidateDiscordWebhookURL_AcceptsDiscordHostname(t *testing.T) { + err := validateDiscordWebhookURL("https://discord.com/api/webhooks/123456/token_abc?wait=true") + assert.NoError(t, err) +} + +func TestValidateDiscordWebhookURL_AcceptsCanaryDiscordHostname(t *testing.T) { + err := validateDiscordWebhookURL("https://canary.discord.com/api/webhooks/123456/token_abc") + assert.NoError(t, err) +} + +func TestValidateDiscordProviderURL_NonDiscordUnchanged(t *testing.T) { + err := validateDiscordProviderURL("webhook", "https://203.0.113.20/hooks/test?x=1#y") + assert.NoError(t, err) +} + +func TestSendJSONPayload_UsesStoredHostnameURLWithoutHostMutation(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db) + + var observedURLHost string + var observedRequestHost string + originalDo := webhookDoRequestFunc + defer func() { webhookDoRequestFunc = originalDo }() + webhookDoRequestFunc = func(client *http.Client, req *http.Request) (*http.Response, error) { + observedURLHost = req.URL.Host + observedRequestHost = req.Host + return client.Do(req) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + parsedServerURL, err := url.Parse(server.URL) + require.NoError(t, err) + parsedServerURL.Host = "localhost:" + parsedServerURL.Port() + + provider := models.NotificationProvider{ + Type: "webhook", + URL: parsedServerURL.String(), + Template: "minimal", + } + + data := map[string]any{ + "Message": "Test notification", + "Title": "Test", + "Time": time.Now().Format(time.RFC3339), + } + + err = svc.sendJSONPayload(context.Background(), provider, data) + require.NoError(t, err) + + assert.Equal(t, "localhost:"+parsedServerURL.Port(), observedURLHost) + assert.Equal(t, observedURLHost, observedRequestHost) +} + func TestSendJSONPayload_Discord(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "POST", r.Method) @@ -66,7 +151,7 @@ func TestSendJSONPayload_Discord(t *testing.T) { svc := NewNotificationService(db) provider := models.NotificationProvider{ - Type: "discord", + Type: "webhook", URL: server.URL, Template: "custom", Config: `{"content": {{toJSON .Message}}, "username": "Charon"}`, @@ -82,31 +167,6 @@ func TestSendJSONPayload_Discord(t *testing.T) { assert.NoError(t, err) } -func TestCanonicalizeDiscordWebhookURL_DiscordIPHostToDiscordDomain(t *testing.T) { - raw := "https://203.0.113.10/api/webhooks/123456/token_abc?wait=true#frag" - got := canonicalizeDiscordWebhookURL("discord", raw) - - parsed, err := url.Parse(got) - require.NoError(t, err) - assert.Equal(t, "https", parsed.Scheme) - assert.Equal(t, "discord.com", parsed.Hostname()) - assert.Equal(t, "/api/webhooks/123456/token_abc", parsed.Path) - assert.Equal(t, "wait=true", parsed.RawQuery) - assert.Equal(t, "frag", parsed.Fragment) -} - -func TestCanonicalizeDiscordWebhookURL_NonDiscordUnchanged(t *testing.T) { - raw := "https://203.0.113.20/hooks/test?x=1#y" - got := canonicalizeDiscordWebhookURL("webhook", raw) - assert.Equal(t, raw, got) -} - -func TestCanonicalizeDiscordWebhookURL_DiscordNonIPUnchanged(t *testing.T) { - raw := "https://discord.com/api/webhooks/123456/token_abc?wait=true" - got := canonicalizeDiscordWebhookURL("discord", raw) - assert.Equal(t, raw, got) -} - func TestSendJSONPayload_Slack(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var payload map[string]any @@ -232,15 +292,6 @@ func TestSendJSONPayload_TemplateSizeLimit(t *testing.T) { } func TestSendJSONPayload_DiscordValidation(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var payload map[string]any - err := json.NewDecoder(r.Body).Decode(&payload) - require.NoError(t, err) - assert.Equal(t, "Test", payload["content"]) - w.WriteHeader(http.StatusOK) - })) - defer server.Close() - db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) require.NoError(t, err) @@ -248,7 +299,7 @@ func TestSendJSONPayload_DiscordValidation(t *testing.T) { provider := models.NotificationProvider{ Type: "discord", - URL: server.URL, + URL: "https://203.0.113.10/api/webhooks/123456/token_abc", Template: "custom", Config: `{"username": "Charon", "message": {{toJSON .Message}}}`, } @@ -258,7 +309,9 @@ func TestSendJSONPayload_DiscordValidation(t *testing.T) { } err = svc.sendJSONPayload(context.Background(), provider, data) - assert.NoError(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid Discord webhook URL") + assert.Contains(t, err.Error(), "IP address hosts are not allowed") } func TestSendJSONPayload_DiscordValidation_MissingMessage(t *testing.T) { @@ -269,7 +322,7 @@ func TestSendJSONPayload_DiscordValidation_MissingMessage(t *testing.T) { provider := models.NotificationProvider{ Type: "discord", - URL: "http://localhost:9999", + URL: "https://discord.com/api/webhooks/123456/token_abc", Template: "custom", Config: `{"username": "Charon"}`, } @@ -401,7 +454,7 @@ func TestSendExternal_UsesJSONForSupportedServices(t *testing.T) { defer server.Close() provider := models.NotificationProvider{ - Type: "discord", + Type: "webhook", URL: server.URL, Template: "custom", Config: `{"content": {{toJSON .Message}}}`, @@ -415,7 +468,7 @@ func TestSendExternal_UsesJSONForSupportedServices(t *testing.T) { // Give goroutine time to execute time.Sleep(100 * time.Millisecond) - assert.True(t, called.Load(), "Discord notification should have been sent via JSON") + assert.True(t, called.Load(), "notification should have been sent via JSON") } func TestTestProvider_UsesJSONForSupportedServices(t *testing.T) { @@ -434,7 +487,7 @@ func TestTestProvider_UsesJSONForSupportedServices(t *testing.T) { svc := NewNotificationService(db) provider := models.NotificationProvider{ - Type: "discord", + Type: "webhook", URL: server.URL, Template: "custom", Config: `{"content": {{toJSON .Message}}}`, diff --git a/backend/internal/services/notification_service_test.go b/backend/internal/services/notification_service_test.go index 862f359d..99c95140 100644 --- a/backend/internal/services/notification_service_test.go +++ b/backend/internal/services/notification_service_test.go @@ -97,7 +97,7 @@ func TestNotificationService_Providers(t *testing.T) { provider := models.NotificationProvider{ Name: "Discord", Type: "discord", - URL: "http://example.com", + URL: "https://discord.com/api/webhooks/123456/token_abc", } err := svc.CreateProvider(&provider) require.NoError(t, err)