From ef71f66029a7432cb708ea4c97ad34ba1f7984cd Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Tue, 10 Mar 2026 12:14:57 +0000 Subject: [PATCH] feat: add Telegram notification provider support - Updated API to support Telegram as a notification provider type. - Enhanced tests to cover Telegram provider creation, updates, and token handling. - Modified frontend forms to include Telegram-specific fields and validation. - Added localization strings for Telegram provider. - Implemented security measures to ensure bot tokens are not exposed in API responses. --- .../api/handlers/feature_flags_handler.go | 2 + ...notification_provider_discord_only_test.go | 2 +- .../handlers/notification_provider_handler.go | 8 +- .../notification_provider_handler_test.go | 87 ++ ...urity_notifications_final_blockers_test.go | 2 +- .../internal/notifications/feature_flags.go | 1 + backend/internal/notifications/router.go | 2 + .../enhanced_security_notification_service.go | 9 +- ...nced_security_notification_service_test.go | 6 +- .../internal/services/notification_service.go | 52 +- .../notification_service_discord_only_test.go | 2 +- .../notification_service_json_test.go | 159 ++- .../services/notification_service_test.go | 1 - docs/plans/archive/codeql_hardening_spec.md | 422 ++++++++ docs/plans/current_spec.md | 910 +++++++++++------- .../src/api/__tests__/notifications.test.ts | 3 +- frontend/src/api/notifications.test.ts | 47 +- frontend/src/api/notifications.ts | 4 +- ...SecurityNotificationSettingsModal.test.tsx | 2 +- frontend/src/locales/en/translation.json | 6 + frontend/src/pages/Notifications.tsx | 20 +- .../pages/__tests__/Notifications.test.tsx | 6 +- tests/fixtures/notifications.ts | 4 +- tests/settings/notifications-payload.spec.ts | 17 +- tests/settings/notifications.spec.ts | 4 +- .../telegram-notification-provider.spec.ts | 472 +++++++++ 26 files changed, 1884 insertions(+), 366 deletions(-) create mode 100644 docs/plans/archive/codeql_hardening_spec.md create mode 100644 tests/settings/telegram-notification-provider.spec.ts diff --git a/backend/internal/api/handlers/feature_flags_handler.go b/backend/internal/api/handlers/feature_flags_handler.go index f874b210..258bb1a8 100644 --- a/backend/internal/api/handlers/feature_flags_handler.go +++ b/backend/internal/api/handlers/feature_flags_handler.go @@ -33,6 +33,7 @@ var defaultFlags = []string{ "feature.notifications.service.email.enabled", "feature.notifications.service.gotify.enabled", "feature.notifications.service.webhook.enabled", + "feature.notifications.service.telegram.enabled", "feature.notifications.security_provider_events.enabled", // Blocker 3: Add security_provider_events gate } @@ -45,6 +46,7 @@ var defaultFlagValues = map[string]bool{ "feature.notifications.service.email.enabled": false, "feature.notifications.service.gotify.enabled": false, "feature.notifications.service.webhook.enabled": false, + "feature.notifications.service.telegram.enabled": false, "feature.notifications.security_provider_events.enabled": false, // Blocker 3: Default disabled for this stage } diff --git a/backend/internal/api/handlers/notification_provider_discord_only_test.go b/backend/internal/api/handlers/notification_provider_discord_only_test.go index 24826a83..f9f67d62 100644 --- a/backend/internal/api/handlers/notification_provider_discord_only_test.go +++ b/backend/internal/api/handlers/notification_provider_discord_only_test.go @@ -36,7 +36,7 @@ func TestDiscordOnly_CreateRejectsNonDiscord(t *testing.T) { {"webhook", "webhook", http.StatusCreated, ""}, {"gotify", "gotify", http.StatusCreated, ""}, {"slack", "slack", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, - {"telegram", "telegram", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, + {"telegram", "telegram", http.StatusCreated, ""}, {"generic", "generic", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, {"email", "email", http.StatusCreated, ""}, } diff --git a/backend/internal/api/handlers/notification_provider_handler.go b/backend/internal/api/handlers/notification_provider_handler.go index 2584b39f..acf7d694 100644 --- a/backend/internal/api/handlers/notification_provider_handler.go +++ b/backend/internal/api/handlers/notification_provider_handler.go @@ -110,7 +110,7 @@ func classifyProviderTestFailure(err error) (code string, category string, messa if statusMatch := providerStatusCodePattern.FindStringSubmatch(errText); len(statusMatch) == 2 { switch statusMatch[1] { case "401", "403": - return "PROVIDER_TEST_AUTH_REJECTED", "dispatch", "Provider rejected authentication. Verify your Gotify token" + return "PROVIDER_TEST_AUTH_REJECTED", "dispatch", "Provider rejected authentication. Verify your credentials" case "404": return "PROVIDER_TEST_ENDPOINT_NOT_FOUND", "dispatch", "Provider endpoint was not found. Verify the provider URL path" default: @@ -168,7 +168,7 @@ func (h *NotificationProviderHandler) Create(c *gin.Context) { } providerType := strings.ToLower(strings.TrimSpace(req.Type)) - if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" { + if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" && providerType != "telegram" { respondSanitizedProviderError(c, http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE", "validation", "Unsupported notification provider type") return } @@ -228,12 +228,12 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { } providerType := strings.ToLower(strings.TrimSpace(existing.Type)) - if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" { + if providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email" && providerType != "telegram" { respondSanitizedProviderError(c, http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE", "validation", "Unsupported notification provider type") return } - if providerType == "gotify" && strings.TrimSpace(req.Token) == "" { + if (providerType == "gotify" || providerType == "telegram") && strings.TrimSpace(req.Token) == "" { // Keep existing token if update payload omits token req.Token = existing.Token } diff --git a/backend/internal/api/handlers/notification_provider_handler_test.go b/backend/internal/api/handlers/notification_provider_handler_test.go index 1b6cffdb..e75de4ac 100644 --- a/backend/internal/api/handlers/notification_provider_handler_test.go +++ b/backend/internal/api/handlers/notification_provider_handler_test.go @@ -581,3 +581,90 @@ func TestNotificationProviderHandler_Test_NonEmail_StillRequiresProviderID(t *te _ = json.Unmarshal(w.Body.Bytes(), &resp) assert.Equal(t, "MISSING_PROVIDER_ID", resp["code"]) } + +func TestNotificationProviderHandler_Create_Telegram(t *testing.T) { + r, _ := setupNotificationProviderTest(t) + + payload := map[string]interface{}{ + "name": "My Telegram Bot", + "type": "telegram", + "url": "123456789", + "token": "bot123456789:ABCdefGHIjklMNOpqrSTUvwxYZ", + "template": "minimal", + } + body, _ := json.Marshal(payload) + req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusCreated, w.Code) + + var raw map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &raw)) + assert.Equal(t, "telegram", raw["type"]) + assert.Equal(t, true, raw["has_token"]) + // Token must never appear in response + assert.NotContains(t, w.Body.String(), "bot123456789:ABCdefGHIjklMNOpqrSTUvwxYZ") +} + +func TestNotificationProviderHandler_Update_TelegramTokenPreservation(t *testing.T) { + r, db := setupNotificationProviderTest(t) + + p := models.NotificationProvider{ + ID: "tg-preserve", + Name: "Telegram Bot", + Type: "telegram", + URL: "123456789", + Token: "original-bot-token", + } + require.NoError(t, db.Create(&p).Error) + + // Update without token — token should be preserved + payload := map[string]interface{}{ + "name": "Updated Telegram Bot", + "type": "telegram", + "url": "987654321", + } + body, _ := json.Marshal(payload) + req, _ := http.NewRequest("PUT", "/api/v1/notifications/providers/tg-preserve", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + + // Verify token was preserved in DB + var dbProvider models.NotificationProvider + require.NoError(t, db.Where("id = ?", "tg-preserve").First(&dbProvider).Error) + assert.Equal(t, "original-bot-token", dbProvider.Token) + assert.Equal(t, "987654321", dbProvider.URL) +} + +func TestNotificationProviderHandler_List_TelegramNeverExposesBotToken(t *testing.T) { + r, db := setupNotificationProviderTest(t) + + p := models.NotificationProvider{ + ID: "tg-secret", + Name: "Secret Telegram", + Type: "telegram", + URL: "123456789", + Token: "bot999:SECRETTOKEN", + } + require.NoError(t, db.Create(&p).Error) + + req, _ := http.NewRequest("GET", "/api/v1/notifications/providers", http.NoBody) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.NotContains(t, w.Body.String(), "bot999:SECRETTOKEN") + assert.NotContains(t, w.Body.String(), "api.telegram.org") + + var raw []map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &raw)) + require.Len(t, raw, 1) + assert.Equal(t, true, raw[0]["has_token"]) + _, hasTokenField := raw[0]["token"] + assert.False(t, hasTokenField, "raw token field must not appear in JSON response") +} diff --git a/backend/internal/api/handlers/security_notifications_final_blockers_test.go b/backend/internal/api/handlers/security_notifications_final_blockers_test.go index 1d42ade5..7aedf121 100644 --- a/backend/internal/api/handlers/security_notifications_final_blockers_test.go +++ b/backend/internal/api/handlers/security_notifications_final_blockers_test.go @@ -224,7 +224,7 @@ func TestFinalBlocker3_SupportedProviderTypes_UnsupportedTypesIgnored(t *testing db := SetupCompatibilityTestDB(t) // Create ONLY unsupported providers - unsupportedTypes := []string{"telegram", "generic"} + unsupportedTypes := []string{"pushover", "generic"} for _, providerType := range unsupportedTypes { provider := &models.NotificationProvider{ diff --git a/backend/internal/notifications/feature_flags.go b/backend/internal/notifications/feature_flags.go index 609fac7b..7a3a3405 100644 --- a/backend/internal/notifications/feature_flags.go +++ b/backend/internal/notifications/feature_flags.go @@ -6,5 +6,6 @@ const ( FlagEmailServiceEnabled = "feature.notifications.service.email.enabled" FlagGotifyServiceEnabled = "feature.notifications.service.gotify.enabled" FlagWebhookServiceEnabled = "feature.notifications.service.webhook.enabled" + FlagTelegramServiceEnabled = "feature.notifications.service.telegram.enabled" FlagSecurityProviderEventsEnabled = "feature.notifications.security_provider_events.enabled" ) diff --git a/backend/internal/notifications/router.go b/backend/internal/notifications/router.go index 4821ec44..a69f6cbd 100644 --- a/backend/internal/notifications/router.go +++ b/backend/internal/notifications/router.go @@ -23,6 +23,8 @@ func (r *Router) ShouldUseNotify(providerType string, flags map[string]bool) boo return flags[FlagGotifyServiceEnabled] case "webhook": return flags[FlagWebhookServiceEnabled] + case "telegram": + return flags[FlagTelegramServiceEnabled] default: return false } diff --git a/backend/internal/services/enhanced_security_notification_service.go b/backend/internal/services/enhanced_security_notification_service.go index a6495d2d..7efb7037 100644 --- a/backend/internal/services/enhanced_security_notification_service.go +++ b/backend/internal/services/enhanced_security_notification_service.go @@ -84,10 +84,11 @@ func (s *EnhancedSecurityNotificationService) getProviderAggregatedConfig() (*mo // Blocker 3: Filter for supported notify-only provider types (PR-1 scope) // All supported types are included in GET aggregation for configuration visibility supportedTypes := map[string]bool{ - "webhook": true, - "discord": true, - "slack": true, - "gotify": true, + "webhook": true, + "discord": true, + "slack": true, + "gotify": true, + "telegram": true, } filteredProviders := []models.NotificationProvider{} for _, p := range providers { diff --git a/backend/internal/services/enhanced_security_notification_service_test.go b/backend/internal/services/enhanced_security_notification_service_test.go index 76f2de1a..bfddff29 100644 --- a/backend/internal/services/enhanced_security_notification_service_test.go +++ b/backend/internal/services/enhanced_security_notification_service_test.go @@ -136,7 +136,7 @@ func TestGetProviderAggregatedConfig_FiltersSupportedTypes(t *testing.T) { {ID: "webhook", Type: "webhook", Enabled: true, NotifySecurityWAFBlocks: true}, {ID: "slack", Type: "slack", Enabled: true, NotifySecurityACLDenies: true}, {ID: "gotify", Type: "gotify", Enabled: true, NotifySecurityRateLimitHits: true}, - {ID: "unsupported", Type: "telegram", Enabled: true, NotifySecurityWAFBlocks: true}, // Should be filtered + {ID: "telegram", Type: "telegram", Enabled: true, NotifySecurityWAFBlocks: true}, // Telegram is now supported } for _, p := range providers { @@ -146,8 +146,8 @@ func TestGetProviderAggregatedConfig_FiltersSupportedTypes(t *testing.T) { // Test config, err := service.getProviderAggregatedConfig() require.NoError(t, err) - // Telegram is unsupported, so it shouldn't contribute to aggregation - assert.True(t, config.NotifyWAFBlocks, "Discord and webhook have WAF=true") + // All provider types including telegram contribute to aggregation + assert.True(t, config.NotifyWAFBlocks, "Discord, webhook, and telegram have WAF=true") assert.True(t, config.NotifyACLDenies, "Slack has ACL=true") assert.True(t, config.NotifyRateLimitHits, "Gotify has RateLimit=true") } diff --git a/backend/internal/services/notification_service.go b/backend/internal/services/notification_service.go index 92f7793f..7f1476e9 100644 --- a/backend/internal/services/notification_service.go +++ b/backend/internal/services/notification_service.go @@ -99,7 +99,7 @@ func validateDiscordProviderURL(providerType, rawURL string) error { // supportsJSONTemplates returns true if the provider type can use JSON templates func supportsJSONTemplates(providerType string) bool { switch strings.ToLower(providerType) { - case "webhook", "discord", "gotify", "slack", "generic": + case "webhook", "discord", "gotify", "slack", "generic", "telegram": return true default: return false @@ -108,7 +108,7 @@ func supportsJSONTemplates(providerType string) bool { func isSupportedNotificationProviderType(providerType string) bool { switch strings.ToLower(strings.TrimSpace(providerType)) { - case "discord", "email", "gotify", "webhook": + case "discord", "email", "gotify", "webhook", "telegram": return true default: return false @@ -125,6 +125,8 @@ func (s *NotificationService) isDispatchEnabled(providerType string) bool { return s.getFeatureFlagValue(notifications.FlagGotifyServiceEnabled, true) case "webhook": return s.getFeatureFlagValue(notifications.FlagWebhookServiceEnabled, true) + case "telegram": + return s.getFeatureFlagValue(notifications.FlagTelegramServiceEnabled, true) default: return false } @@ -447,9 +449,26 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti if _, hasMessage := jsonPayload["message"]; !hasMessage { return fmt.Errorf("gotify payload requires 'message' field") } + case "telegram": + // Telegram requires 'text' field for the message body + if _, hasText := jsonPayload["text"]; !hasText { + if messageValue, hasMessage := jsonPayload["message"]; hasMessage { + jsonPayload["text"] = messageValue + normalizedBody, marshalErr := json.Marshal(jsonPayload) + if marshalErr != nil { + return fmt.Errorf("failed to normalize telegram payload: %w", marshalErr) + } + body.Reset() + if _, writeErr := body.Write(normalizedBody); writeErr != nil { + return fmt.Errorf("failed to write normalized telegram payload: %w", writeErr) + } + } else { + return fmt.Errorf("telegram payload requires 'text' field") + } + } } - if providerType == "gotify" || providerType == "webhook" { + if providerType == "gotify" || providerType == "webhook" || providerType == "telegram" { headers := map[string]string{ "Content-Type": "application/json", "User-Agent": "Charon-Notify/1.0", @@ -459,14 +478,35 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti headers["X-Request-ID"] = ridStr } } + + dispatchURL := p.URL + if providerType == "gotify" { if strings.TrimSpace(p.Token) != "" { headers["X-Gotify-Key"] = strings.TrimSpace(p.Token) } } + if providerType == "telegram" { + decryptedToken := p.Token + dispatchURL = "https://api.telegram.org/bot" + decryptedToken + "/sendMessage" + + parsedURL, parseErr := neturl.Parse(dispatchURL) + if parseErr != nil || parsedURL.Hostname() != "api.telegram.org" { + return fmt.Errorf("telegram dispatch URL validation failed: invalid hostname") + } + + jsonPayload["chat_id"] = p.URL + updatedBody, marshalErr := json.Marshal(jsonPayload) + if marshalErr != nil { + return fmt.Errorf("failed to marshal telegram payload with chat_id: %w", marshalErr) + } + body.Reset() + body.Write(updatedBody) + } + if _, sendErr := s.httpWrapper.Send(ctx, notifications.HTTPWrapperRequest{ - URL: p.URL, + URL: dispatchURL, Headers: headers, Body: body.Bytes(), }); sendErr != nil { @@ -688,7 +728,7 @@ func (s *NotificationService) CreateProvider(provider *models.NotificationProvid return err } - if provider.Type != "gotify" { + if provider.Type != "gotify" && provider.Type != "telegram" { provider.Token = "" } @@ -724,7 +764,7 @@ func (s *NotificationService) UpdateProvider(provider *models.NotificationProvid return err } - if provider.Type == "gotify" { + if provider.Type == "gotify" || provider.Type == "telegram" { if strings.TrimSpace(provider.Token) == "" { provider.Token = existing.Token } diff --git a/backend/internal/services/notification_service_discord_only_test.go b/backend/internal/services/notification_service_discord_only_test.go index a43afd5b..9fb9b19b 100644 --- a/backend/internal/services/notification_service_discord_only_test.go +++ b/backend/internal/services/notification_service_discord_only_test.go @@ -22,7 +22,7 @@ func TestDiscordOnly_CreateProviderRejectsUnsupported(t *testing.T) { service := NewNotificationService(db, nil) - testCases := []string{"slack", "telegram", "generic"} + testCases := []string{"slack", "generic"} for _, providerType := range testCases { t.Run(providerType, func(t *testing.T) { diff --git a/backend/internal/services/notification_service_json_test.go b/backend/internal/services/notification_service_json_test.go index 7cf968c5..5d14bada 100644 --- a/backend/internal/services/notification_service_json_test.go +++ b/backend/internal/services/notification_service_json_test.go @@ -29,7 +29,7 @@ func TestSupportsJSONTemplates(t *testing.T) { {"slack", "slack", true}, {"gotify", "gotify", true}, {"generic", "generic", true}, - {"telegram", "telegram", false}, + {"telegram", "telegram", true}, {"unknown", "unknown", false}, {"WEBHOOK uppercase", "WEBHOOK", true}, {"Discord mixed case", "Discord", true}, @@ -500,3 +500,160 @@ func TestTestProvider_UsesJSONForSupportedServices(t *testing.T) { err = svc.TestProvider(provider) assert.NoError(t, err) } + +func TestSendJSONPayload_Telegram_ValidPayload(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.NotNil(t, payload["text"], "Telegram payload should have text field") + assert.NotNil(t, payload["chat_id"], "Telegram payload should have chat_id field") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Extract host from test server to override SSRF check + serverURL, _ := url.Parse(server.URL) + + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db, nil) + + // Override httpWrapper.Send to capture the request and forward to test server + origWrapper := svc.httpWrapper + _ = origWrapper + + provider := models.NotificationProvider{ + Type: "telegram", + URL: "123456789", + Token: "bot-test-token", + Template: "custom", + Config: `{"text": {{toJSON .Message}}, "title": {{toJSON .Title}}}`, + } + + data := map[string]any{ + "Message": "Test notification", + "Title": "Test", + } + + // We need to test the payload construction, not the actual HTTP call. + // The SSRF check validates hostname = api.telegram.org, so the httpWrapper.Send + // will be called with the constructed URL. We test validation logic directly. + sendErr := svc.sendJSONPayload(context.Background(), provider, data) + // The send will fail because api.telegram.org is not reachable in tests, + // but the payload construction and validation should succeed. + // Check that the error is a network error, not a validation error. + if sendErr != nil { + assert.NotContains(t, sendErr.Error(), "telegram payload requires") + assert.NotContains(t, sendErr.Error(), "telegram dispatch URL validation failed") + } + _ = serverURL +} + +func TestSendJSONPayload_Telegram_AutoMapMessageToText(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db, nil) + + provider := models.NotificationProvider{ + Type: "telegram", + URL: "123456789", + Token: "bot-test-token", + Template: "custom", + Config: `{"message": {{toJSON .Message}}, "title": {{toJSON .Title}}}`, + } + + data := map[string]any{ + "Message": "Test notification", + "Title": "Test", + } + + sendErr := svc.sendJSONPayload(context.Background(), provider, data) + // Should not fail with validation error — 'message' is auto-mapped to 'text' + if sendErr != nil { + assert.NotContains(t, sendErr.Error(), "telegram payload requires 'text' field") + } +} + +func TestSendJSONPayload_Telegram_MissingTextAndMessage(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db, nil) + + provider := models.NotificationProvider{ + Type: "telegram", + URL: "123456789", + Token: "bot-test-token", + Template: "custom", + Config: `{"title": {{toJSON .Title}}}`, + } + + data := map[string]any{ + "Title": "Test", + } + + sendErr := svc.sendJSONPayload(context.Background(), provider, data) + require.Error(t, sendErr) + assert.Contains(t, sendErr.Error(), "telegram payload requires 'text' field") +} + +func TestSendJSONPayload_Telegram_SSRFValidation(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db, nil) + + // Path traversal in token does NOT change hostname — Go's net/url.Parse + // keeps api.telegram.org as the host. The request reaches the real API + // (which rejects it), proving hostname validation cannot be bypassed. + provider := models.NotificationProvider{ + Type: "telegram", + URL: "123456789", + Token: "test-token/../../../evil.com/x", + Template: "custom", + Config: `{"text": {{toJSON .Message}}}`, + } + + data := map[string]any{ + "Message": "Test", + } + + sendErr := svc.sendJSONPayload(context.Background(), provider, data) + require.Error(t, sendErr) + // Hostname validation passes (host IS api.telegram.org), so error comes + // from the HTTP dispatch layer — proving SSRF check was not bypassed. + assert.NotContains(t, sendErr.Error(), "telegram dispatch URL validation failed") +} + +func TestSendJSONPayload_Telegram_401ErrorMessage(t *testing.T) { + // Use a webhook provider with a mock server returning 401 to verify + // that the dispatch path surfaces "provider returned status 401" in the error. + // Telegram cannot be tested this way because its SSRF check requires api.telegram.org. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer server.Close() + + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{}) + require.NoError(t, err) + + svc := NewNotificationService(db, nil) + + provider := models.NotificationProvider{ + Type: "webhook", + URL: server.URL, + Template: "custom", + Config: `{"message": {{toJSON .Message}}}`, + } + + data := map[string]any{ + "Message": "Test notification", + } + + sendErr := svc.sendJSONPayload(context.Background(), provider, data) + require.Error(t, sendErr) + assert.Contains(t, sendErr.Error(), "provider returned status 401") +} diff --git a/backend/internal/services/notification_service_test.go b/backend/internal/services/notification_service_test.go index b72dd6ad..03546cbd 100644 --- a/backend/internal/services/notification_service_test.go +++ b/backend/internal/services/notification_service_test.go @@ -1826,7 +1826,6 @@ func TestTestProvider_NotifyOnlyRejectsUnsupportedProvider(t *testing.T) { providerType string url string }{ - {"telegram", "telegram", "telegram://token@telegram?chats=123"}, {"slack", "slack", "https://hooks.slack.com/services/T/B/X"}, {"pushover", "pushover", "pushover://token@user"}, } diff --git a/docs/plans/archive/codeql_hardening_spec.md b/docs/plans/archive/codeql_hardening_spec.md new file mode 100644 index 00000000..fc2559d0 --- /dev/null +++ b/docs/plans/archive/codeql_hardening_spec.md @@ -0,0 +1,422 @@ +# Spec: Fix Remaining CodeQL Findings + Harden Local Security Scanning + +**Branch:** `feature/beta-release` +**PR:** #800 (Email Notifications) +**Date:** 2026-03-06 +**Status:** Planning + +--- + +## 1. Introduction + +Two CodeQL findings remain open in CI after the email-injection remediation in the prior commit (`ee224adc`), and local scanning does not block commits that would reproduce them. This spec covers the root-cause analysis, the precise code fixes, and the hardening of the local scanning pipeline so future regressions are caught before push. + +### Objectives + +1. Silence `go/email-injection` (CWE-640) in CI without weakening the existing multi-layer defence. +2. Silence `go/cookie-secure-not-set` (CWE-614) in CI for the intentional dev-mode loopback exception. +3. Ensure local scanning fails (exit-code 1) on Critical/High security findings of the same class before code reaches GitHub. + +--- + +## 2. Research Findings + +### 2.1 CWE-640 (`go/email-injection`) — Why It Is Still Flagged + +CodeQL's `go/email-injection` rule tracks untrusted input from HTTP sources to `smtp.SendMail` / `(*smtp.Client).Rcpt` sinks. It treats a **validator** (a function that returns an error if bad data is present) differently from a **sanitizer** (a function that transforms and strips the bad data). Only sanitizers break the taint flow; validators do not. + +The previous fix added `sanitizeForEmail()` in `notification_service.go` for `title` and `message`. It did **not** patch two other direct callers of `SendEmail`, both of which pass an HTTP-sourced `to` address without going through `notification_service.go` at all. + +#### Confirmed taint sinks (from `codeql-results-go.sarif`) + +| SARIF line | Function | Tainted argument | +|---|---|---| +| 365–367 | `(*MailService).SendEmail` | `[]string{toEnvelope}` in `smtp.SendMail` | +| ~530 | `(*MailService).sendSSL` | `toEnvelope` in `client.Rcpt(toEnvelope)` | +| ~583 | `(*MailService).sendSTARTTLS` | `toEnvelope` in `client.Rcpt(toEnvelope)` | + +CodeQL reports 4 untrusted taint paths converging on each sink. These correspond to: + +| Path # | Source file | Source expression | Route to sink | +|---|---|---|---| +| 1 | `backend/internal/api/handlers/settings_handler.go:637` | `req.To` (ShouldBindJSON, `binding:"required,email"`) | Direct `h.MailService.SendEmail(ctx, []string{req.To}, ...)` — bypasses `notification_service.go` entirely | +| 2 | `backend/internal/api/handlers/user_handler.go:597` | `userEmail` (HTTP request field) | `h.MailService.SendInvite(userEmail, ...)` → `mail_service.go:679` → `s.SendEmail(ctx, []string{email}, ...)` | +| 3 | `backend/internal/api/handlers/user_handler.go:1015` | `user.Email` (DB row, set from HTTP during registration) | Same `SendInvite` → `SendEmail` chain | +| 4 | `backend/internal/services/notification_service.go` | `rawRecipients` from `p.URL` (DB, admin-set) | `s.mailService.SendEmail(ctx, recipients, ...)` — CodeQL may trace DB values as tainted from prior HTTP writes | + +#### Why CodeQL does not recognise the existing safeguards as sanitisers + +``` +validateEmailRecipients() → ContainsAny check + mail.ParseAddress → error return (validator, not sanitizer) +parseEmailAddressForHeader → net/mail.ParseAddress → validator +rejectCRLF(toEnvelope) → ContainsAny("\r\n") → error → validator +``` + +CodeQL's taint model for Go requires the taint to be **transformed** (characters stripped or value replaced) before it considers the path neutralised. None of the helpers above strips CRLF — they gate on error returns. From CodeQL's perspective the original tainted bytes still flow into `smtp.SendMail`. + +#### Why adding `sanitizeForEmail()` to `settings_handler.go` alone is insufficient + +Even if added, `sanitizeForEmail()` calls `strings.ReplaceAll(s, "\r", "")` — stripping characters from an email address that contains `\r` would corrupt it. For recipient addresses, the correct model is to validate (which is already done) and suppress the residual finding with an annotated justification. + +### 2.2 CWE-614 (`go/cookie-secure-not-set`) — Why It Appeared as "New" + +**Only one `c.SetCookie` call exists** in production Go code: + +``` +backend/internal/api/handlers/auth_handler.go:152 +``` + +The finding is "new" because it was introduced when `setSecureCookie()` was refactored to support the loopback dev-mode exception (`secure = false` for local HTTP requests). Prior to that refactor, `secure` was always `true`. + +The `// codeql[go/cookie-secure-not-set]` suppression comment **is** present on `auth_handler.go:152`. However, the SARIF stored in the repository (`codeql-results-go.sarif`) shows the finding at **line 151** — a 1-line discrepancy caused by a subsequent commit that inserted the `// secure is intentionally false...` explanation comment, shifting the `c.SetCookie(` line from 151 → 152. + +The inline suppression **should** work now that it is on the correct line (152). However, inline suppressions are fragile under line-number churn. The robust fix is a `query-filter` in `.github/codeql/codeql-config.yml`, which targets the rule ID independent of line number. + +The `query-filters` section does not yet exist in the CodeQL config — only `paths-ignore` is used. This must be added for the first time. + +### 2.3 Local Scanning Gap + +The table below maps which security tools catch which findings locally. + +| Tool | Stage | Fails on High/Critical? | Catches CWE-640? | Catches CWE-614? | +|---|---|---|---|---| +| `golangci-lint-fast` (gosec G101,G110,G305,G401,G501-503) | commit (blocking) | ✅ | ❌ no rule | ❌ gosec has no Gin cookie rule | +| `go-vet` | commit (blocking) | ✅ | ❌ | ❌ | +| `security-scan.sh` (govulncheck) | manual | Warn only | ❌ (CVEs only) | ❌ | +| `semgrep-scan.sh` (auto config, `--error`) | **manual only** | ✅ if run | ✅ `p/golang` | ✅ `p/golang` | +| `codeql-go-scan` + `codeql-check-findings` | **manual only** | ✅ if run | ✅ | ✅ | +| `golangci-lint-full` | manual | ✅ if run | ❌ | ❌ | + +**The gap:** `semgrep-scan` already has `--error` (blocking) and covers both issue classes via `p/golang` and `p/owasp-top-ten`, but it runs as `stages: [manual]` only. Moving it to `pre-push` is the highest-leverage single change. + +gosec rule audit for missing coverage: + +| CWE | gosec rule | Covered by fast config? | +|---|---|---| +| CWE-614 cookie security | No standard gosec rule for Gin's `c.SetCookie` | ❌ | +| CWE-640 email injection | No gosec rule exists for SMTP injection | ❌ | +| CWE-89 SQL injection | G201, G202 — NOT in fast config | ❌ | +| CWE-22 path traversal | G305 — in fast config | ✅ | + +`semgrep` fills the gap for CWE-614 and CWE-640 where gosec has no rules. + +--- + +## 3. Technical Specifications + +### 3.1 Phase 1 — Harden Local Scanning + +#### 3.1.1 Move `semgrep-scan` to `pre-push` stage + +**File:** `/projects/Charon/.pre-commit-config.yaml` + +Locate the `semgrep-scan` hook entry (currently has `stages: [manual]`). Change the stage and name: + +```yaml + - id: semgrep-scan + name: Semgrep Security Scan (Blocking - pre-push) + entry: scripts/pre-commit-hooks/semgrep-scan.sh + language: script + pass_filenames: false + verbose: true + stages: [pre-push] +``` + +Rationale: `p/golang` includes: +- `go.cookie.security.insecure-cookie.insecure-cookie` → CWE-614 equivalent +- `go.lang.security.injection.tainted-smtp-email.tainted-smtp-email` → CWE-640 equivalent + +#### 3.1.2 Restrict semgrep to WARNING+ and use targeted config + +**File:** `/projects/Charon/scripts/pre-commit-hooks/semgrep-scan.sh` + +The current invocation uses `--config auto --error`. Two changes: +1. Change default config from `auto` → `p/golang`. `auto` fetches 1,000-3,000+ rules and takes 60-180s — too slow for a blocking pre-push hook. `p/golang` covers all Go-specific OWASP/CWE rules and completes in ~10-30s. +2. Add `--severity ERROR --severity WARNING` to filter out INFO-level noise (these are OR logic, both required for WARNING+): + +```bash +semgrep scan \ + --config "${SEMGREP_CONFIG_VALUE:-p/golang}" \ + --severity ERROR \ + --severity WARNING \ + --error \ + --exclude "frontend/node_modules" \ + --exclude "frontend/coverage" \ + --exclude "frontend/dist" \ + backend frontend/src .github/workflows +``` + +The `SEMGREP_CONFIG` env var can be overridden to `auto` or `p/golang p/owasp-top-ten` for a broader audit: `SEMGREP_CONFIG=auto git push`. + +#### 3.1.3 Add `make security-local` target + +**File:** `/projects/Charon/Makefile` + +Add after the `security-scan-deps` target: + +```make +security-local: ## Run local security scan (govulncheck + semgrep) +@echo "Running govulncheck..." +@./scripts/security-scan.sh +@echo "Running semgrep..." +@SEMGREP_CONFIG=p/golang ./scripts/pre-commit-hooks/semgrep-scan.sh +``` + +#### 3.1.4 Expand golangci-lint-fast gosec ruleset (Deferred) + +**Status: DEFERRED** — G201/G202 (SQL injection via format string / string concat) are candidates for the fast config but must be pre-validated against the existing codebase first. GORM's raw query DSL can produce false positives. Run the following before adding: + +```bash +cd backend && golangci-lint run --enable=gosec --disable-all --config .golangci-fast.yml ./... +``` + +…with G201/G202 temporarily uncommented. If zero false positives, add in a separate hardening commit. This is out of scope for this PR to avoid blocking PR #800. + +Note: CWE-614 and CWE-640 remain CodeQL/Semgrep territory — gosec has no rules for these. + +### 3.2 Phase 2 — Fix CWE-640 (`go/email-injection`) + +#### Strategy + +Add `// codeql[go/email-injection]` inline suppression annotations at all three sink sites in `mail_service.go`, with a structured justification comment immediately above each. This is the correct approach because: + +1. The actual runtime defence is already correct and comprehensive (4-layer defence). +2. The taint is a CodeQL false-positive caused by the tool not modelling validators as sanitisers. +3. Restructuring to call `strings.ReplaceAll` on email addresses would corrupt valid addresses. + +**The 4-layer defence that justifies these suppressions:** + +``` +Layer 1: HTTP boundary — gin binding:"required,email" validates RFC 5321 format; CRLF fails well-formedness +Layer 2: Service boundary — validateEmailRecipients() → ContainsAny("\r\n") error + net/mail.ParseAddress +Layer 3: Mail layer parse — parseEmailAddressForHeader() → net/mail.ParseAddress returns only .Address field +Layer 4: Pre-sink validation — rejectCRLF(toEnvelope) immediately before smtp.SendMail / client.Rcpt calls +``` + +#### 3.2.1 Suppress at `smtp.SendMail` sink + +**File:** `backend/internal/services/mail_service.go` (around line 367) + +Locate the default-encryption branch in `SendEmail`. Replace the `smtp.SendMail` call line: + +```go +default: +// toEnvelope passes through 4-layer CRLF defence: +// 1. gin binding:"required,email" at HTTP entry (CRLF invalid per RFC 5321) +// 2. validateEmailRecipients → ContainsAny("\r\n") + net/mail.ParseAddress +// 3. parseEmailAddressForHeader → net/mail.ParseAddress (returns .Address only) +// 4. rejectCRLF(toEnvelope) guard earlier in this function +// CodeQL does not model validators as sanitisers; suppression is correct here. +if err := smtp.SendMail(addr, auth, fromEnvelope, []string{toEnvelope}, msg); err != nil { // codeql[go/email-injection] +``` + +#### 3.2.2 Suppress at `client.Rcpt` in `sendSSL` + +**File:** `backend/internal/services/mail_service.go` (around line 530) + +Locate in `sendSSL`. Replace the `client.Rcpt` call line: + +```go +// toEnvelope validated by rejectCRLF + net/mail.ParseAddress before this call (see SendEmail). +if rcptErr := client.Rcpt(toEnvelope); rcptErr != nil { // codeql[go/email-injection] +return fmt.Errorf("RCPT TO failed: %w", rcptErr) +} +``` + +#### 3.2.3 Suppress at `client.Rcpt` in `sendSTARTTLS` + +**File:** `backend/internal/services/mail_service.go` (around line 583) + +Same pattern as sendSSL: + +```go +// toEnvelope validated by rejectCRLF + net/mail.ParseAddress before this call (see SendEmail). +if rcptErr := client.Rcpt(toEnvelope); rcptErr != nil { // codeql[go/email-injection] +return fmt.Errorf("RCPT TO failed: %w", rcptErr) +} +``` + +#### 3.2.4 Document safe call in `settings_handler.go` + +**File:** `backend/internal/api/handlers/settings_handler.go` (around line 637) + +Add a comment immediately above the `SendEmail` call — the sinks in mail_service.go are already annotated, so this is documentation only: + +```go +// req.To is validated as RFC 5321 email via gin binding:"required,email". +// SendEmail applies validateEmailRecipients + net/mail.ParseAddress + rejectCRLF as defence-in-depth. +// Suppression annotations are on the sinks in mail_service.go. +if err := h.MailService.SendEmail(c.Request.Context(), []string{req.To}, "Charon - Test Email", htmlBody); err != nil { +``` + +#### 3.2.5 Document safe calls in `user_handler.go` + +**File:** `backend/internal/api/handlers/user_handler.go` (lines ~597 and ~1015) + +Add the same explanatory comment above both `SendInvite` call sites: + +```go +// userEmail validated as RFC 5321 email format; suppression on mail_service.go sinks covers this path. +if err := h.MailService.SendInvite(userEmail, userToken, appName, baseURL); err != nil { +``` + +### 3.3 Phase 3 — Fix CWE-614 (`go/cookie-secure-not-set`) + +#### Strategy + +Two complementary changes: (1) add a `query-filters` exclusion in `.github/codeql/codeql-config.yml` which is robust to line-number churn, and (2) verify the inline `// codeql[go/cookie-secure-not-set]` annotation is correctly positioned. + +**Justification for exclusion:** + +The `secure` parameter in `setSecureCookie()` is `true` for **all** external production requests. It is `false` only when `isLocalRequest()` returns `true` — i.e., when the request comes from `127.x.x.x`, `::1`, or `localhost` over HTTP. In that scenario, browsers reject `Secure` cookies over non-TLS connections anyway, so setting `Secure: true` would silently break auth for local development. The conditional is tested and documented. + +#### 3.3.1 Skip `query-filters` approach — inline annotation is sufficient + +**Status: NOT IMPLEMENTING** — `query-filters` is a CodeQL query suite (`.qls`) concept, NOT a valid top-level key in GitHub's `codeql-config.yml`. Adding it risks silent failure or breaking the entire CodeQL analysis. The inline annotation at `auth_handler.go:152` is the documented mechanism and is already correct. No changes to `.github/codeql/codeql-config.yml` are needed for CWE-614. + +**Why inline annotation is sufficient and preferred:** It is scoped to the single intentional instance. Any future `c.SetCookie(...)` call without `Secure:true` anywhere else in the codebase will correctly flag. Global exclusion via config would silently hide future regressions. + +#### 3.3.1 (REFERENCE ONLY) Current valid `codeql-config.yml` structure + +```yaml +name: "Charon CodeQL Config" + +# Paths to ignore from all analysis (use sparingly - prefer query-filters for rule-level exclusions) +paths-ignore: + - "frontend/coverage/**" + - "frontend/dist/**" + - "playwright-report/**" + - "test-results/**" + - "coverage/**" +``` + +DO NOT add `query-filters:` — it is not supported. + - exclude: + id: go/cookie-secure-not-set + # Justified: setSecureCookie() in auth_handler.go intentionally sets Secure=false + # ONLY for local loopback (127.x.x.x / ::1 / localhost) HTTP requests. + # Browsers reject Secure cookies over HTTP regardless, so Secure=true would silently + # break local development auth. All external HTTPS flows always set Secure=true. + # Code: backend/internal/api/handlers/auth_handler.go → setSecureCookie() + # Tests: TestSetSecureCookie_HTTPS_Strict, TestSetSecureCookie_HTTP_Loopback_Insecure +``` + +#### 3.3.2 Verify inline suppression placement in `auth_handler.go` + +**File:** `backend/internal/api/handlers/auth_handler.go` (around line 152) + +Confirm the `// codeql[go/cookie-secure-not-set]` annotation is on the same line as `c.SetCookie(`. The code should read: + +```go +c.SetSameSite(sameSite) +// secure is intentionally false for local non-HTTPS loopback (development only). +c.SetCookie( // codeql[go/cookie-secure-not-set] +name, +value, +maxAge, +"/", +domain, +secure, +true, +) +``` + +The `query-filters` in §3.3.1 provides the primary fix. The inline annotation provides belt-and-suspenders coverage that survives if the config is ever reset. + +--- + +## 4. Implementation Plan + +### Phase 1 — Local Scanning (implement first to gate subsequent work) + +| Task | File | Change | Effort | +|---|---|---|---| +| P1-1 | `.pre-commit-config.yaml` | Change semgrep-scan stage from `[manual]` → `[pre-push]`, update name | XS | +| P1-2 | `scripts/pre-commit-hooks/semgrep-scan.sh` | Add `--severity ERROR --severity WARNING` flags, exclude generated dirs | XS | +| P1-3 | `Makefile` | Add `security-local` target | XS | +| P1-4 | `backend/.golangci-fast.yml` | Add G201, G202 to gosec includes | XS | + +### Phase 2 — CWE-640 Fix + +| Task | File | Change | Effort | +|---|---|---|---| +| P2-1 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on smtp.SendMail line + 4-layer defence comment | XS | +| P2-2 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on sendSSL client.Rcpt line | XS | +| P2-3 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on sendSTARTTLS client.Rcpt line | XS | +| P2-4 | `backend/internal/api/handlers/settings_handler.go` | Add explanatory comment above SendEmail call | XS | +| P2-5 | `backend/internal/api/handlers/user_handler.go` | Add explanatory comment above both SendInvite calls (~line 597, ~line 1015) | XS | + +### Phase 3 — CWE-614 Fix + +| Task | File | Change | Effort | +|---|---|---|---| +| P3-1 | `backend/internal/api/handlers/auth_handler.go` | Verify `// codeql[go/cookie-secure-not-set]` is on `c.SetCookie(` line (no codeql-config.yml changes needed) | XS | + +**Total estimated file changes: 6 files, all comment/config additions — no logic changes.** + +--- + +## 5. Acceptance Criteria + +### CI (CodeQL must pass with zero error-level findings) + +- [ ] `codeql.yml` CodeQL analysis (Go) passes with **0 blocking findings** +- [ ] `go/email-injection` is absent from the Go SARIF output +- [ ] `go/cookie-secure-not-set` is absent from the Go SARIF output + +### Local scanning + +- [ ] A `git push` with any `.go` file touched **blocks** if semgrep finds WARNING+ severity issues +- [ ] `pre-commit run semgrep-scan` on the current codebase exits 0 (no new findings) +- [ ] `make security-local` runs and exits 0 + +### Regression safety + +- [ ] `go test ./...` in `backend/` passes (all changes are comments/config — no test updates required) +- [ ] `golangci-lint run --config .golangci-fast.yml ./...` passes in `backend/` +- [ ] The existing runtime defence (rejectCRLF, validateEmailRecipients) is **unchanged** — confirmed by diff + +--- + +## 6. Commit Slicing Strategy + +### Decision: Single commit on `feature/beta-release` + +**Rationale:** All three phases are tightly related (one CI failure, two root findings, one local gap). All changes are additive (comments, config, no logic mutations). Splitting into multiple PRs would create an intermediate state where CI still fails and the local gap remains open. A single well-scoped commit keeps PR #800 atomic and reviewable. + +**Suggested commit message:** +``` +fix(security): suppress CodeQL false-positives for email-injection and cookie-secure + +CWE-640 (go/email-injection): Add // codeql[go/email-injection] annotations at all 3 +smtp sink sites in mail_service.go (smtp.SendMail, sendSSL client.Rcpt, sendSTARTTLS +client.Rcpt). The 4-layer defence (gin binding:"required,email", validateEmailRecipients, +net/mail.ParseAddress, rejectCRLF) is comprehensive; CodeQL's taint model does not +model validators as sanitisers, producing false-positive paths from +settings_handler.go:637 and user_handler.go invite flows that bypass +notification_service.go. + +CWE-614 (go/cookie-secure-not-set): Add query-filter to codeql-config.yml excluding +this rule with documented justification. setSecureCookie() correctly sets Secure=false +only for local loopback HTTP requests where the Secure attribute is browser-rejected. +All external HTTPS flows set Secure=true. + +Local scanning: Promote semgrep-scan from manual to pre-push stage so WARNING+ +severity findings block push. Addresses gap where CWE-614 and CWE-640 equivalents +are not covered by any blocking local scan tool. +``` + +**PR:** All changes target PR #800 directly. + +--- + +## 7. Risk and Rollback + +| Risk | Likelihood | Mitigation | +|---|---|---| +| Inline suppression ends up on wrong line after future rebase | Medium | `query-filters` in codeql-config.yml provides independent suppression independent of line numbers | +| `semgrep-scan` at `pre-push` produces false-positive blocking | Low | `--severity WARNING --error` limits to genuine findings; use `SEMGREP_CONFIG=p/golang` for targeted override | +| G201/G202 gosec rules trigger on existing legitimate code | Low | Run `golangci-lint run --config .golangci-fast.yml` locally before committing; suppress specific instances if needed | +| CodeQL `query-filters` YAML syntax changes in future GitHub CodeQL versions | Low | Inline `// codeql[...]` annotations serve as independent fallback | + +**Rollback:** All changes are additive config and comments. Reverting the commit restores the prior state exactly. No schema, API, or behaviour changes are made. diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index fc2559d0..57c12d69 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,422 +1,686 @@ -# Spec: Fix Remaining CodeQL Findings + Harden Local Security Scanning +# Telegram Notification Provider — Implementation Plan -**Branch:** `feature/beta-release` -**PR:** #800 (Email Notifications) -**Date:** 2026-03-06 -**Status:** Planning +**Date:** 2026-07-10 +**Author:** Planning Agent +**Confidence Score:** 92% (High — existing patterns well-established, Telegram Bot API straightforward) --- ## 1. Introduction -Two CodeQL findings remain open in CI after the email-injection remediation in the prior commit (`ee224adc`), and local scanning does not block commits that would reproduce them. This spec covers the root-cause analysis, the precise code fixes, and the hardening of the local scanning pipeline so future regressions are caught before push. +### Objective -### Objectives +Add Telegram as a first-class notification provider in Charon, following the established architecture used by Discord, Gotify, Email, and generic Webhook providers. -1. Silence `go/email-injection` (CWE-640) in CI without weakening the existing multi-layer defence. -2. Silence `go/cookie-secure-not-set` (CWE-614) in CI for the intentional dev-mode loopback exception. -3. Ensure local scanning fails (exit-code 1) on Critical/High security findings of the same class before code reaches GitHub. +### Goals + +- Users can configure a Telegram bot token and chat ID to receive notifications via Telegram +- All existing notification event types (proxy hosts, certs, uptime, security events) work with Telegram +- JSON template engine (minimal/detailed/custom) works with Telegram +- Feature flag allows enabling/disabling Telegram dispatch independently +- Token is treated as a secret (write-only, never exposed in API responses) +- Full test coverage: Go unit tests, Vitest frontend tests, Playwright E2E tests + +### Telegram Bot API Overview + +Telegram bots send messages via: + +``` +POST https://api.telegram.org/bot/sendMessage +Content-Type: application/json + +{ + "chat_id": "", + "text": "Hello world", + "parse_mode": "HTML" // optional: "HTML" or "MarkdownV2" +} +``` + +**Key design decisions:** +- **Token storage:** The bot token is stored in `NotificationProvider.Token` (`json:"-"`, encrypted at rest) — never in the URL field. This mirrors the Gotify pattern where secrets are separated from endpoints. +- **URL field:** Stores only the `chat_id` (e.g., `987654321`). At dispatch time, the full API URL is constructed dynamically: `https://api.telegram.org/bot` + decryptedToken + `/sendMessage`. The `chat_id` is passed in the POST body alongside the message text. This prevents token leakage via API responses since URL is `json:"url"`. +- **SSRF mitigation:** Before dispatching, validate that the constructed URL hostname is exactly `api.telegram.org`. This prevents SSRF if stored data is tampered with. +- **Dispatch path:** Uses `sendJSONPayload` → `httpWrapper.Send()` (same as Gotify), since both are token-based JSON POST providers +- **No schema migration needed:** The existing `NotificationProvider` model accommodates Telegram without changes + +> **Supervisor Review Note:** The original design embedded the bot token in the URL field (`https://api.telegram.org/bot/sendMessage?chat_id=`). This was rejected because the URL field is `json:"url"` — exposed in every API response. The token MUST only reside in the `Token` field (`json:"-"`). --- ## 2. Research Findings -### 2.1 CWE-640 (`go/email-injection`) — Why It Is Still Flagged +### Existing Architecture -CodeQL's `go/email-injection` rule tracks untrusted input from HTTP sources to `smtp.SendMail` / `(*smtp.Client).Rcpt` sinks. It treats a **validator** (a function that returns an error if bad data is present) differently from a **sanitizer** (a function that transforms and strips the bad data). Only sanitizers break the taint flow; validators do not. +The notification system follows a provider-based architecture: -The previous fix added `sanitizeForEmail()` in `notification_service.go` for `title` and `message`. It did **not** patch two other direct callers of `SendEmail`, both of which pass an HTTP-sourced `to` address without going through `notification_service.go` at all. +| Layer | File | Role | +|-------|------|------| +| Feature flags | `backend/internal/notifications/feature_flags.go` | Flag constants (`FlagXxxServiceEnabled`) | +| Feature flag handler | `backend/internal/api/handlers/feature_flags_handler.go` | DB-backed flags with defaults | +| Router | `backend/internal/notifications/router.go` | `ShouldUseNotify()` per-type dispatch | +| Service | `backend/internal/services/notification_service.go` | Core dispatch: `isSupportedNotificationProviderType()`, `isDispatchEnabled()`, `supportsJSONTemplates()`, `sendJSONPayload()`, `TestProvider()` | +| Handlers | `backend/internal/api/handlers/notification_provider_handler.go` | CRUD + type validation + token preservation | +| Model | `backend/internal/models/notification_provider.go` | GORM model with Token (json:"-"), HasToken | +| Frontend API | `frontend/src/api/notifications.ts` | `SUPPORTED_NOTIFICATION_PROVIDER_TYPES`, sanitization | +| Frontend UI | `frontend/src/pages/Notifications.tsx` | Provider form with conditional fields per type | +| i18n | `frontend/src/locales/en/translation.json` | Label strings | +| E2E fixtures | `tests/fixtures/notifications.ts` | `telegramProvider` **already defined** | -#### Confirmed taint sinks (from `codeql-results-go.sarif`) +### Existing Provider Addition Points (Switch Statements / Type Checks) -| SARIF line | Function | Tainted argument | -|---|---|---| -| 365–367 | `(*MailService).SendEmail` | `[]string{toEnvelope}` in `smtp.SendMail` | -| ~530 | `(*MailService).sendSSL` | `toEnvelope` in `client.Rcpt(toEnvelope)` | -| ~583 | `(*MailService).sendSTARTTLS` | `toEnvelope` in `client.Rcpt(toEnvelope)` | +Every location that checks provider types is listed below — all require a `"telegram"` case: -CodeQL reports 4 untrusted taint paths converging on each sink. These correspond to: +| # | File | Function/Line | Current Logic | +|---|------|---------------|---------------| +| 1 | `feature_flags.go` | Constants | Missing `FlagTelegramServiceEnabled` | +| 2 | `feature_flags_handler.go` | `defaultFlags` + `defaultFlagValues` | Missing telegram entry | +| 3 | `router.go` | `ShouldUseNotify()` switch | Missing `case "telegram"` | +| 4 | `notification_service.go` | `isSupportedNotificationProviderType()` | `case "discord", "email", "gotify", "webhook"` | +| 5 | `notification_service.go` | `isDispatchEnabled()` | switch with per-type flag checks | +| 6 | `notification_service.go` | `supportsJSONTemplates()` | `case "webhook", "discord", "gotify", "slack", "generic"` | +| 7 | `notification_service.go` | `sendJSONPayload()` — service-specific validation | Missing `case "telegram"` for payload validation | +| 8 | `notification_service.go` | `sendJSONPayload()` — dispatch branch | Gotify/webhook use `httpWrapper.Send()`; others use `ValidateExternalURL` + `SafeHTTPClient` | +| 9 | `notification_provider_handler.go` | `Create()` type guard | `providerType != "discord" && providerType != "gotify" && providerType != "webhook" && providerType != "email"` | +| 10 | `notification_provider_handler.go` | `Update()` type guard | Same pattern as Create | +| 11 | `notification_provider_handler.go` | `Update()` token preservation | `if providerType == "gotify" && strings.TrimSpace(req.Token) == ""` | +| 12 | `notifications.ts` | `SUPPORTED_NOTIFICATION_PROVIDER_TYPES` | `['discord', 'gotify', 'webhook', 'email']` | +| 13 | `notifications.ts` | `sanitizeProviderForWriteAction()` | Token handling only for `type !== 'gotify'` | +| 14 | `Notifications.tsx` | Type ` + {initialData?.has_token && ( +

+ {t('notificationProviders.gotifyTokenStored')} +

+ )} +

{t('notificationProviders.gotifyTokenWriteOnlyHint')}

+ +)} ``` -#### 3.2.5 Document safe calls in `user_handler.go` +#### URL Field Placeholder -**File:** `backend/internal/api/handlers/user_handler.go` (lines ~597 and ~1015) +For Telegram, the URL field stores the chat_id (not a full URL). Update the placeholder and label accordingly: -Add the same explanatory comment above both `SendInvite` call sites: - -```go -// userEmail validated as RFC 5321 email format; suppression on mail_service.go sinks covers this path. -if err := h.MailService.SendInvite(userEmail, userToken, appName, baseURL); err != nil { +```typescript +placeholder={ + isEmail ? 'user@example.com, admin@example.com' + : type === 'discord' ? 'https://discord.com/api/webhooks/...' + : type === 'gotify' ? 'https://gotify.example.com/message' + : isTelegram ? '987654321' + : 'https://example.com/webhook' +} ``` -### 3.3 Phase 3 — Fix CWE-614 (`go/cookie-secure-not-set`) +Update the label for the URL field when type is telegram: -#### Strategy - -Two complementary changes: (1) add a `query-filters` exclusion in `.github/codeql/codeql-config.yml` which is robust to line-number churn, and (2) verify the inline `// codeql[go/cookie-secure-not-set]` annotation is correctly positioned. - -**Justification for exclusion:** - -The `secure` parameter in `setSecureCookie()` is `true` for **all** external production requests. It is `false` only when `isLocalRequest()` returns `true` — i.e., when the request comes from `127.x.x.x`, `::1`, or `localhost` over HTTP. In that scenario, browsers reject `Secure` cookies over non-TLS connections anyway, so setting `Secure: true` would silently break auth for local development. The conditional is tested and documented. - -#### 3.3.1 Skip `query-filters` approach — inline annotation is sufficient - -**Status: NOT IMPLEMENTING** — `query-filters` is a CodeQL query suite (`.qls`) concept, NOT a valid top-level key in GitHub's `codeql-config.yml`. Adding it risks silent failure or breaking the entire CodeQL analysis. The inline annotation at `auth_handler.go:152` is the documented mechanism and is already correct. No changes to `.github/codeql/codeql-config.yml` are needed for CWE-614. - -**Why inline annotation is sufficient and preferred:** It is scoped to the single intentional instance. Any future `c.SetCookie(...)` call without `Secure:true` anywhere else in the codebase will correctly flag. Global exclusion via config would silently hide future regressions. - -#### 3.3.1 (REFERENCE ONLY) Current valid `codeql-config.yml` structure - -```yaml -name: "Charon CodeQL Config" - -# Paths to ignore from all analysis (use sparingly - prefer query-filters for rule-level exclusions) -paths-ignore: - - "frontend/coverage/**" - - "frontend/dist/**" - - "playwright-report/**" - - "test-results/**" - - "coverage/**" +```typescript +label={isTelegram ? t('notificationProviders.telegramChatId') : t('notificationProviders.url')} ``` -DO NOT add `query-filters:` — it is not supported. - - exclude: - id: go/cookie-secure-not-set - # Justified: setSecureCookie() in auth_handler.go intentionally sets Secure=false - # ONLY for local loopback (127.x.x.x / ::1 / localhost) HTTP requests. - # Browsers reject Secure cookies over HTTP regardless, so Secure=true would silently - # break local development auth. All external HTTPS flows always set Secure=true. - # Code: backend/internal/api/handlers/auth_handler.go → setSecureCookie() - # Tests: TestSetSecureCookie_HTTPS_Strict, TestSetSecureCookie_HTTP_Loopback_Insecure +#### Clear Token on Type Change + +Update the existing `useEffect` that clears `gotify_token`: + +```typescript +useEffect(() => { + if (type !== 'gotify' && type !== 'telegram') { + setValue('gotify_token', '', { shouldDirty: false, shouldTouch: false }); + } +}, [type, setValue]); ``` -#### 3.3.2 Verify inline suppression placement in `auth_handler.go` +### 3.8 Frontend — i18n Strings -**File:** `backend/internal/api/handlers/auth_handler.go` (around line 152) +**File:** `frontend/src/locales/en/translation.json` -Confirm the `// codeql[go/cookie-secure-not-set]` annotation is on the same line as `c.SetCookie(`. The code should read: +Add to the `notificationProviders` section: -```go -c.SetSameSite(sameSite) -// secure is intentionally false for local non-HTTPS loopback (development only). -c.SetCookie( // codeql[go/cookie-secure-not-set] -name, -value, -maxAge, -"/", -domain, -secure, -true, -) +```json +"telegram": "Telegram", +"telegramBotToken": "Bot Token", +"telegramBotTokenPlaceholder": "Enter your Telegram Bot Token", +"telegramChatId": "Chat ID", +"telegramChatIdPlaceholder": "987654321", +"telegramChatIdHelp": "Your Telegram chat, group, or channel ID. The bot token is stored securely and separately." ``` -The `query-filters` in §3.3.1 provides the primary fix. The inline annotation provides belt-and-suspenders coverage that survives if the config is ever reset. +### 3.9 API Contract (No Changes) + +The existing REST endpoints remain unchanged: + +| Method | Endpoint | Notes | +|--------|----------|-------| +| `GET` | `/api/notification-providers` | Returns all providers (token stripped) | +| `POST` | `/api/notification-providers` | Create — now accepts `type: "telegram"` | +| `PUT` | `/api/notification-providers/:id` | Update — token preserved if omitted | +| `DELETE` | `/api/notification-providers/:id` | Delete — no type-specific logic | +| `POST` | `/api/notification-providers/test` | Test — routes through `sendJSONPayload` | + +Request/response schemas are unchanged. The `type` field now accepts `"telegram"` in addition to existing values. --- ## 4. Implementation Plan -### Phase 1 — Local Scanning (implement first to gate subsequent work) +### Phase 1: Playwright E2E Tests (Test-First) -| Task | File | Change | Effort | -|---|---|---|---| -| P1-1 | `.pre-commit-config.yaml` | Change semgrep-scan stage from `[manual]` → `[pre-push]`, update name | XS | -| P1-2 | `scripts/pre-commit-hooks/semgrep-scan.sh` | Add `--severity ERROR --severity WARNING` flags, exclude generated dirs | XS | -| P1-3 | `Makefile` | Add `security-local` target | XS | -| P1-4 | `backend/.golangci-fast.yml` | Add G201, G202 to gosec includes | XS | +**Rationale:** Per project conventions — write feature behaviour tests first. -### Phase 2 — CWE-640 Fix +**New file:** `tests/settings/telegram-notification-provider.spec.ts` -| Task | File | Change | Effort | -|---|---|---|---| -| P2-1 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on smtp.SendMail line + 4-layer defence comment | XS | -| P2-2 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on sendSSL client.Rcpt line | XS | -| P2-3 | `backend/internal/services/mail_service.go` | Add `// codeql[go/email-injection]` on sendSTARTTLS client.Rcpt line | XS | -| P2-4 | `backend/internal/api/handlers/settings_handler.go` | Add explanatory comment above SendEmail call | XS | -| P2-5 | `backend/internal/api/handlers/user_handler.go` | Add explanatory comment above both SendInvite calls (~line 597, ~line 1015) | XS | +Modeled after `tests/settings/email-notification-provider.spec.ts`. -### Phase 3 — CWE-614 Fix +Test scenarios: +1. Create a Telegram provider (name, chat_id in URL field, bot token in token field, enable events) +2. Verify provider appears in the list +3. Edit the Telegram provider (change name, verify token preservation) +4. Test the Telegram provider (mock API returns 200) +5. Delete the Telegram provider +6. **Negative security test:** Verify `GET /api/notification-providers` does NOT expose the bot token in any response field +7. **Negative security test:** Verify bot token is NOT present in the URL field of the API response -| Task | File | Change | Effort | -|---|---|---|---| -| P3-1 | `backend/internal/api/handlers/auth_handler.go` | Verify `// codeql[go/cookie-secure-not-set]` is on `c.SetCookie(` line (no codeql-config.yml changes needed) | XS | +**Update file:** `tests/settings/notifications-payload.spec.ts` -**Total estimated file changes: 6 files, all comment/config additions — no logic changes.** +Add telegram to the payload matrix test scenarios. + +**E2E fixtures:** Update `telegramProvider` in `tests/fixtures/notifications.ts` — URL must contain only `chat_id`, token goes in the `token` field (see Research Findings section for updated fixture). + +### Phase 2: Backend Implementation + +**2A — Feature Flags (3 files)** + +| File | Change | +|------|--------| +| `backend/internal/notifications/feature_flags.go` | Add `FlagTelegramServiceEnabled` constant | +| `backend/internal/api/handlers/feature_flags_handler.go` | Add to `defaultFlags` + `defaultFlagValues` | +| `backend/internal/notifications/router.go` | Add `case "telegram"` to `ShouldUseNotify()` | + +**2B — Service Layer (1 file, 4 function changes)** + +| File | Function | Change | +|------|----------|--------| +| `notification_service.go` | `isSupportedNotificationProviderType()` | Add `"telegram"` to case | +| `notification_service.go` | `isDispatchEnabled()` | Add `case "telegram"` with flag check | +| `notification_service.go` | `supportsJSONTemplates()` | Add `"telegram"` to case | +| `notification_service.go` | `sendJSONPayload()` | Add telegram validation + dispatch branch | + +**2C — Handler Layer (1 file, 3 locations)** + +| File | Location | Change | +|------|----------|--------| +| `notification_provider_handler.go` | `Create()` type guard | Add `&& providerType != "telegram"` | +| `notification_provider_handler.go` | `Update()` type guard | Same | +| `notification_provider_handler.go` | `Update()` token preservation | Add `|| providerType == "telegram"` | + +### Phase 3: Frontend Implementation + +**3A — API Client (1 file)** + +| File | Change | +|------|--------| +| `frontend/src/api/notifications.ts` | Add `'telegram'` to `SUPPORTED_NOTIFICATION_PROVIDER_TYPES`, update token sanitization logic | + +**3B — Notifications Page (1 file)** + +| File | Change | +|------|--------| +| `frontend/src/pages/Notifications.tsx` | Add telegram to type select, token field conditional, URL placeholder, `normalizeProviderPayloadForSubmit()`, type-change useEffect | + +**3C — Localization (1 file)** + +| File | Change | +|------|--------| +| `frontend/src/locales/en/translation.json` | Add telegram-specific label strings | + +### Phase 4: Backend Tests + +| Test File | Changes | +|-----------|---------| +| `notification_service_test.go` | Update "rejects unsupported provider" test (remove telegram from unsupported list). Add telegram dispatch/integration tests. | +| `notification_service_json_test.go` | Add `TestSendJSONPayload_Telegram_*` tests: valid payload, missing text with message auto-map, missing both text and message, dispatch via httpWrapper, **SSRF hostname validation**, **401/403 error message** | +| `notification_provider_handler_test.go` | Add telegram to Create/Update happy path tests, token preservation test. **Add negative test: verify GET response does not contain bot token in URL field or response body** | +| `enhanced_security_notification_service_test.go` | Change telegram from "filtered" to "valid provider" in security dispatch tests | +| Router test (if exists) | Add telegram to `ShouldUseNotify()` tests | + +### Phase 5: Frontend Tests + +| Test File | Changes | +|-----------|---------| +| `frontend/src/api/notifications.test.ts` | Remove telegram rejection test, add telegram CRUD sanitization tests | +| `frontend/src/api/__tests__/notifications.test.ts` | Same changes (duplicate test location) | +| `frontend/src/pages/Notifications.test.tsx` | Add telegram form rendering tests (token field visibility, placeholder text) | + +### Phase 6: Integration, Documentation & Deployment + +- Verify E2E tests pass with Docker container +- Update `docs/features.md` with Telegram provider mention +- No `ARCHITECTURE.md` changes needed (same provider pattern) +- No database migration needed --- ## 5. Acceptance Criteria -### CI (CodeQL must pass with zero error-level findings) +### EARS Requirements -- [ ] `codeql.yml` CodeQL analysis (Go) passes with **0 blocking findings** -- [ ] `go/email-injection` is absent from the Go SARIF output -- [ ] `go/cookie-secure-not-set` is absent from the Go SARIF output +| ID | Requirement | +|----|-------------| +| T-01 | WHEN a user creates a notification provider with type "telegram", THE SYSTEM SHALL accept the provider and store it in the database | +| T-02 | WHEN a user provides a bot token for a Telegram provider, THE SYSTEM SHALL store it securely and never expose it in API responses | +| T-03 | WHEN a Telegram provider is enabled and a notification event fires, THE SYSTEM SHALL construct the Telegram API URL dynamically from the stored token (`https://api.telegram.org/bot` + token + `/sendMessage`), inject `chat_id` from the URL field into the POST body, and send the rendered template payload | +| T-04 | WHEN the rendered JSON payload contains a "message" field but not a "text" field, THE SYSTEM SHALL auto-map "message" to "text" for Telegram compatibility | +| T-05 | WHEN the Telegram feature flag is disabled, THE SYSTEM SHALL skip dispatch for all Telegram providers | +| T-06 | WHEN a user updates a Telegram provider without providing a token, THE SYSTEM SHALL preserve the existing stored token | +| T-07 | WHEN a user tests a Telegram provider, THE SYSTEM SHALL send a test notification through the standard sendJSONPayload path | +| T-08 | WHEN the frontend renders the provider form with type "telegram", THE SYSTEM SHALL display a bot token input field and a chat_id input field (with appropriate placeholder) | +| T-09 | WHEN dispatching a Telegram notification, THE SYSTEM SHALL validate that the constructed URL hostname is exactly `api.telegram.org` before sending (SSRF mitigation) | +| T-10 | WHEN a Telegram test request receives HTTP 401 or 403, THE SYSTEM SHALL return the error message "Provider rejected authentication. Verify your Telegram Bot Token" | +| T-11 | WHEN the API returns notification providers via GET, THE SYSTEM SHALL NOT include the bot token in the URL field or any other exposed response field | -### Local scanning +### Definition of Done -- [ ] A `git push` with any `.go` file touched **blocks** if semgrep finds WARNING+ severity issues -- [ ] `pre-commit run semgrep-scan` on the current codebase exits 0 (no new findings) -- [ ] `make security-local` runs and exits 0 - -### Regression safety - -- [ ] `go test ./...` in `backend/` passes (all changes are comments/config — no test updates required) -- [ ] `golangci-lint run --config .golangci-fast.yml ./...` passes in `backend/` -- [ ] The existing runtime defence (rejectCRLF, validateEmailRecipients) is **unchanged** — confirmed by diff +- [ ] All 16 code touchpoints updated (see section 2 table) +- [ ] E2E Playwright tests pass for Telegram CRUD + test send +- [ ] Backend unit tests cover: type registration, dispatch routing, payload validation (text field), token preservation, feature flag gating +- [ ] Frontend unit tests cover: type array acceptance, sanitization, form rendering +- [ ] `go test ./...` passes +- [ ] `npm test` passes +- [ ] `npx playwright test --project=firefox` passes +- [ ] `make lint-fast` passes (staticcheck) +- [ ] Coverage threshold maintained (85%+) +- [ ] GORM security scan passes (no model changes, but verify) +- [ ] Token never appears in API responses, logs, or frontend state +- [ ] Negative security tests pass (bot token not in GET response body or URL field) +- [ ] SSRF hostname validation test passes (only `api.telegram.org` allowed) +- [ ] Telegram 401/403 returns specific auth error message --- ## 6. Commit Slicing Strategy -### Decision: Single commit on `feature/beta-release` +### Decision: 2 PRs -**Rationale:** All three phases are tightly related (one CI failure, two root findings, one local gap). All changes are additive (comments, config, no logic mutations). Splitting into multiple PRs would create an intermediate state where CI still fails and the local gap remains open. A single well-scoped commit keeps PR #800 atomic and reviewable. +**Trigger reasons:** Changes span backend + frontend + E2E tests with independent functionality per layer. Splitting improves review quality and rollback safety. -**Suggested commit message:** -``` -fix(security): suppress CodeQL false-positives for email-injection and cookie-secure +### PR-1: Backend — Telegram Provider Support -CWE-640 (go/email-injection): Add // codeql[go/email-injection] annotations at all 3 -smtp sink sites in mail_service.go (smtp.SendMail, sendSSL client.Rcpt, sendSTARTTLS -client.Rcpt). The 4-layer defence (gin binding:"required,email", validateEmailRecipients, -net/mail.ParseAddress, rejectCRLF) is comprehensive; CodeQL's taint model does not -model validators as sanitisers, producing false-positive paths from -settings_handler.go:637 and user_handler.go invite flows that bypass -notification_service.go. +**Scope:** Feature flags, service layer, handler layer, all Go unit tests -CWE-614 (go/cookie-secure-not-set): Add query-filter to codeql-config.yml excluding -this rule with documented justification. setSecureCookie() correctly sets Secure=false -only for local loopback HTTP requests where the Secure attribute is browser-rejected. -All external HTTPS flows set Secure=true. +**Files changed:** +- `backend/internal/notifications/feature_flags.go` +- `backend/internal/api/handlers/feature_flags_handler.go` +- `backend/internal/notifications/router.go` +- `backend/internal/services/notification_service.go` +- `backend/internal/api/handlers/notification_provider_handler.go` +- `backend/internal/services/notification_service_test.go` +- `backend/internal/services/notification_service_json_test.go` +- `backend/internal/api/handlers/notification_provider_handler_test.go` +- `backend/internal/services/enhanced_security_notification_service_test.go` -Local scanning: Promote semgrep-scan from manual to pre-push stage so WARNING+ -severity findings block push. Addresses gap where CWE-614 and CWE-640 equivalents -are not covered by any blocking local scan tool. -``` +**Dependencies:** None (self-contained backend change) -**PR:** All changes target PR #800 directly. +**Validation gates:** +- `go test ./...` passes +- `make lint-fast` passes +- Coverage ≥ 85% +- GORM security scan passes + +**Rollback:** Revert PR — no DB migration to undo. + +### PR-2: Frontend + E2E — Telegram Provider UI + +**Scope:** Frontend API client, Notifications page, i18n strings, frontend unit tests, Playwright E2E tests + +**Files changed:** +- `frontend/src/api/notifications.ts` +- `frontend/src/pages/Notifications.tsx` +- `frontend/src/locales/en/translation.json` +- `frontend/src/api/notifications.test.ts` +- `frontend/src/api/__tests__/notifications.test.ts` +- `frontend/src/pages/Notifications.test.tsx` +- `tests/settings/telegram-notification-provider.spec.ts` (new) +- `tests/settings/notifications-payload.spec.ts` + +**Dependencies:** PR-1 must be merged first (backend must accept `type: "telegram"`) + +**Validation gates:** +- `npm test` passes +- `npm run type-check` passes +- `npx playwright test --project=firefox` passes +- Coverage ≥ 85% + +**Rollback:** Revert PR — frontend-only, no cascading effects. --- -## 7. Risk and Rollback +## 7. Risk Assessment -| Risk | Likelihood | Mitigation | -|---|---|---| -| Inline suppression ends up on wrong line after future rebase | Medium | `query-filters` in codeql-config.yml provides independent suppression independent of line numbers | -| `semgrep-scan` at `pre-push` produces false-positive blocking | Low | `--severity WARNING --error` limits to genuine findings; use `SEMGREP_CONFIG=p/golang` for targeted override | -| G201/G202 gosec rules trigger on existing legitimate code | Low | Run `golangci-lint run --config .golangci-fast.yml` locally before committing; suppress specific instances if needed | -| CodeQL `query-filters` YAML syntax changes in future GitHub CodeQL versions | Low | Inline `// codeql[...]` annotations serve as independent fallback | +| Risk | Likelihood | Impact | Mitigation | +|------|-----------|--------|------------| +| Telegram API rate limiting | Low | Medium | Use existing retry/timeout patterns from httpWrapper | +| Bot token exposure in responses/logs | Low | Critical | Token stored ONLY in `Token` field (`json:"-"`), never in URL field. URL field contains only `chat_id`. Negative security tests verify this invariant. | +| Template auto-mapping edge cases | Low | Low | Test with all three template types (minimal, detailed, custom) | +| URL validation rejects chat_id format | Low | Low | URL field now stores a chat_id string (not a full URL). Validation may need adjustment to accept non-URL values for telegram type. | +| SSRF via tampered stored data | Low | High | Dispatch-time validation ensures hostname is exactly `api.telegram.org`. Dedicated test covers this. | +| E2E test flakiness with mocked API | Low | Low | Existing route-mocking patterns are stable | -**Rollback:** All changes are additive config and comments. Reverting the commit restores the prior state exactly. No schema, API, or behaviour changes are made. +--- + +## 8. Complexity Estimates + +| Component | Estimate | Notes | +|-----------|----------|-------| +| Backend feature flags | S | 3 files, ~5 lines each | +| Backend service layer | M | 4 function changes + telegram validation block | +| Backend handler layer | S | 3 string-level changes | +| Frontend API client | S | 2 lines + sanitization tweak | +| Frontend UI | M | Template conditional, placeholder, useEffect updates | +| Frontend i18n | S | 4 strings | +| Backend tests | L | Multiple test files, new test functions, update existing assertions | +| Frontend tests | M | Update rejection tests, add rendering tests | +| E2E tests | M | New spec file modeled on existing email spec | +| **Total** | **M-L** | ~2-3 days of focused implementation | diff --git a/frontend/src/api/__tests__/notifications.test.ts b/frontend/src/api/__tests__/notifications.test.ts index 5339161a..2e0464df 100644 --- a/frontend/src/api/__tests__/notifications.test.ts +++ b/frontend/src/api/__tests__/notifications.test.ts @@ -54,7 +54,8 @@ describe('notifications api', () => { await expect(createProvider({ name: 'x', type: 'slack' })).rejects.toThrow('Unsupported notification provider type: slack') await expect(updateProvider('2', { name: 'updated', type: 'generic' })).rejects.toThrow('Unsupported notification provider type: generic') - await expect(testProvider({ id: '2', name: 'test', type: 'telegram' })).rejects.toThrow('Unsupported notification provider type: telegram') + await testProvider({ id: '2', name: 'test', type: 'telegram' }) + expect(client.post).toHaveBeenCalledWith('/notifications/providers/test', { id: '2', name: 'test', type: 'telegram' }) }) it('templates and previews use merged payloads', async () => { diff --git a/frontend/src/api/notifications.test.ts b/frontend/src/api/notifications.test.ts index ba89c79f..f0ca144c 100644 --- a/frontend/src/api/notifications.test.ts +++ b/frontend/src/api/notifications.test.ts @@ -119,7 +119,52 @@ describe('notifications api', () => { await expect(createProvider({ name: 'Bad', type: 'slack' })).rejects.toThrow('Unsupported notification provider type: slack') await expect(updateProvider('bad', { type: 'generic' })).rejects.toThrow('Unsupported notification provider type: generic') - await expect(testProvider({ id: 'bad', type: 'telegram' })).rejects.toThrow('Unsupported notification provider type: telegram') + }) + + it('supports telegram provider with token payload contract', async () => { + mockedClient.post.mockResolvedValue({ data: { id: 'tg1' } }) + mockedClient.put.mockResolvedValue({ data: { id: 'tg1' } }) + + await createProvider({ name: 'Telegram', type: 'telegram', gotify_token: 'bot123:ABC' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers', { + name: 'Telegram', + type: 'telegram', + token: 'bot123:ABC', + }) + + await updateProvider('tg1', { type: 'telegram', url: '987654321', gotify_token: 'newtoken' }) + expect(mockedClient.put).toHaveBeenCalledWith('/notifications/providers/tg1', { + type: 'telegram', + url: '987654321', + token: 'newtoken', + }) + + await updateProvider('tg1', { type: 'telegram', url: '987654321' }) + expect(mockedClient.put).toHaveBeenCalledWith('/notifications/providers/tg1', { + type: 'telegram', + url: '987654321', + }) + }) + + it('telegram preserves token in sanitization and strips gotify_token key', async () => { + mockedClient.post.mockResolvedValue({ data: { id: 'tg2' } }) + + await createProvider({ name: 'TG', type: 'telegram', token: 'direct-token', gotify_token: '' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers', { + name: 'TG', + type: 'telegram', + token: 'direct-token', + }) + }) + + it('telegram test/preview strips token from read-like actions', async () => { + mockedClient.post.mockResolvedValue({ data: { id: 'tg3' } }) + + await testProvider({ id: 'tg3', type: 'telegram', gotify_token: 'should-not-send' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers/test', { + id: 'tg3', + type: 'telegram', + }) }) it('fetches templates and previews provider payloads with data', async () => { diff --git a/frontend/src/api/notifications.ts b/frontend/src/api/notifications.ts index e4bbaee5..f51784c8 100644 --- a/frontend/src/api/notifications.ts +++ b/frontend/src/api/notifications.ts @@ -1,6 +1,6 @@ import client from './client'; -export const SUPPORTED_NOTIFICATION_PROVIDER_TYPES = ['discord', 'gotify', 'webhook', 'email'] as const; +export const SUPPORTED_NOTIFICATION_PROVIDER_TYPES = ['discord', 'gotify', 'webhook', 'email', 'telegram'] as const; export type SupportedNotificationProviderType = (typeof SUPPORTED_NOTIFICATION_PROVIDER_TYPES)[number]; const DEFAULT_PROVIDER_TYPE: SupportedNotificationProviderType = 'discord'; @@ -59,7 +59,7 @@ const sanitizeProviderForWriteAction = (data: Partial): Pa delete payload.gotify_token; - if (type !== 'gotify') { + if (type !== 'gotify' && type !== 'telegram') { delete payload.token; return payload; } diff --git a/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx b/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx index 25e3c437..d01d76f9 100644 --- a/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx +++ b/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx @@ -85,7 +85,7 @@ describe('Security Notification Settings on Notifications page', () => { await user.click(await screen.findByTestId('add-provider-btn')); const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement; - expect(Array.from(typeSelect.options).map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook', 'email']); + expect(Array.from(typeSelect.options).map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook', 'email', 'telegram']); expect(typeSelect.value).toBe('discord'); const webhookInput = screen.getByTestId('provider-url') as HTMLInputElement; diff --git a/frontend/src/locales/en/translation.json b/frontend/src/locales/en/translation.json index 7e282e39..d0f465c2 100644 --- a/frontend/src/locales/en/translation.json +++ b/frontend/src/locales/en/translation.json @@ -527,6 +527,12 @@ "gotifyTokenWriteOnlyHint": "Token is write-only and only sent on save.", "gotifyTokenStored": "Token saved. Leave blank to keep current token.", "gotifyTokenKeepPlaceholder": "Leave blank to keep current token", + "telegram": "Telegram", + "telegramBotToken": "Bot Token", + "telegramBotTokenPlaceholder": "Enter your Telegram Bot Token", + "telegramChatId": "Chat ID", + "telegramChatIdPlaceholder": "987654321", + "telegramChatIdHelp": "Your Telegram chat, group, or channel ID. The bot token is stored securely and separately.", "invalidUrl": "Please enter a valid URL starting with http:// or https://", "genericWebhook": "Generic Webhook", "customWebhook": "Custom Webhook (JSON)", diff --git a/frontend/src/pages/Notifications.tsx b/frontend/src/pages/Notifications.tsx index 1d16384f..1ab50472 100644 --- a/frontend/src/pages/Notifications.tsx +++ b/frontend/src/pages/Notifications.tsx @@ -22,7 +22,7 @@ const isSupportedProviderType = (providerType: string | undefined): providerType const supportsJSONTemplates = (providerType: string | undefined): boolean => { if (!providerType) return false; const t = providerType.toLowerCase(); - return t === 'discord' || t === 'gotify' || t === 'webhook'; + return t === 'discord' || t === 'gotify' || t === 'webhook' || t === 'telegram'; }; const isUnsupportedProviderType = (providerType: string | undefined): boolean => !isSupportedProviderType(providerType); @@ -42,7 +42,7 @@ const normalizeProviderPayloadForSubmit = (data: Partial): type, }; - if (type === 'gotify') { + if (type === 'gotify' || type === 'telegram') { const normalizedToken = typeof payload.gotify_token === 'string' ? payload.gotify_token.trim() : ''; if (normalizedToken.length > 0) { @@ -139,9 +139,10 @@ const ProviderForm: FC<{ const type = normalizeProviderType(watch('type')); const isGotify = type === 'gotify'; + const isTelegram = type === 'telegram'; const isEmail = type === 'email'; useEffect(() => { - if (type !== 'gotify') { + if (type !== 'gotify' && type !== 'telegram') { setValue('gotify_token', '', { shouldDirty: false, shouldTouch: false }); } }, [type, setValue]); @@ -196,12 +197,13 @@ const ProviderForm: FC<{ +
{isEmail && (

@@ -212,10 +214,10 @@ const ProviderForm: FC<{ id="provider-url" {...register('url', { required: isEmail ? false : (t('notificationProviders.urlRequired') as string), - validate: isEmail ? undefined : validateUrl, + validate: (isEmail || isTelegram) ? undefined : validateUrl, })} data-testid="provider-url" - placeholder={isEmail ? 'user@example.com, admin@example.com' : type === 'discord' ? 'https://discord.com/api/webhooks/...' : type === 'gotify' ? 'https://gotify.example.com/message' : 'https://example.com/webhook'} + placeholder={isEmail ? 'user@example.com, admin@example.com' : isTelegram ? '987654321' : type === 'discord' ? 'https://discord.com/api/webhooks/...' : type === 'gotify' ? 'https://gotify.example.com/message' : 'https://example.com/webhook'} className={`mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 dark:bg-gray-700 dark:border-gray-600 dark:text-white sm:text-sm ${errors.url ? 'border-red-500' : ''}`} aria-invalid={errors.url ? 'true' : 'false'} aria-describedby={isEmail ? 'email-recipients-help' : errors.url ? 'provider-url-error' : undefined} @@ -235,10 +237,10 @@ const ProviderForm: FC<{

)} - {isGotify && ( + {(isGotify || isTelegram) && (
diff --git a/frontend/src/pages/__tests__/Notifications.test.tsx b/frontend/src/pages/__tests__/Notifications.test.tsx index bbc123e0..64a7854f 100644 --- a/frontend/src/pages/__tests__/Notifications.test.tsx +++ b/frontend/src/pages/__tests__/Notifications.test.tsx @@ -14,7 +14,7 @@ vi.mock('react-i18next', () => ({ })) vi.mock('../../api/notifications', () => ({ - SUPPORTED_NOTIFICATION_PROVIDER_TYPES: ['discord', 'gotify', 'webhook', 'email'], + SUPPORTED_NOTIFICATION_PROVIDER_TYPES: ['discord', 'gotify', 'webhook', 'email', 'telegram'], getProviders: vi.fn(), createProvider: vi.fn(), updateProvider: vi.fn(), @@ -146,8 +146,8 @@ describe('Notifications', () => { const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement const options = Array.from(typeSelect.options) - expect(options).toHaveLength(4) - expect(options.map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook', 'email']) + expect(options).toHaveLength(5) + expect(options.map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook', 'email', 'telegram']) expect(typeSelect.disabled).toBe(false) }) diff --git a/tests/fixtures/notifications.ts b/tests/fixtures/notifications.ts index 395522da..100e84fe 100644 --- a/tests/fixtures/notifications.ts +++ b/tests/fixtures/notifications.ts @@ -32,6 +32,7 @@ export interface NotificationProviderConfig { url: string; config?: string; template?: string; + token?: string; enabled: boolean; notify_proxy_hosts: boolean; notify_certs: boolean; @@ -162,7 +163,8 @@ export const gotifyProvider: NotificationProviderConfig = { export const telegramProvider: NotificationProviderConfig = { name: generateProviderName('telegram'), type: 'telegram', - url: 'https://api.telegram.org/bot123456789:ABCdefGHIjklMNOpqrSTUvwxYZ/sendMessage?chat_id=987654321', + url: '987654321', + token: 'bot123456789:ABCdefGHIjklMNOpqrSTUvwxYZ', enabled: true, notify_proxy_hosts: true, notify_certs: true, diff --git a/tests/settings/notifications-payload.spec.ts b/tests/settings/notifications-payload.spec.ts index 3b33e393..148090d8 100644 --- a/tests/settings/notifications-payload.spec.ts +++ b/tests/settings/notifications-payload.spec.ts @@ -102,6 +102,11 @@ test.describe('Notifications Payload Matrix', () => { name: `webhook-matrix-${Date.now()}`, url: 'https://example.com/notify', }, + { + type: 'telegram', + name: `telegram-matrix-${Date.now()}`, + url: '987654321', + }, ] as const; for (const scenario of scenarios) { @@ -116,12 +121,16 @@ test.describe('Notifications Payload Matrix', () => { await page.getByTestId('provider-gotify-token').fill(' gotify-secret-token '); } + if (scenario.type === 'telegram') { + await page.getByTestId('provider-gotify-token').fill('bot123456789:ABCdefGHI'); + } + await page.getByTestId('provider-save-btn').click(); }); } await test.step('Verify payload contract per provider type', async () => { - expect(capturedCreatePayloads).toHaveLength(3); + expect(capturedCreatePayloads).toHaveLength(4); const discordPayload = capturedCreatePayloads.find((payload) => payload.type === 'discord'); expect(discordPayload).toBeTruthy(); @@ -137,6 +146,12 @@ test.describe('Notifications Payload Matrix', () => { expect(webhookPayload).toBeTruthy(); expect(webhookPayload?.token).toBeUndefined(); expect(typeof webhookPayload?.config).toBe('string'); + + const telegramPayload = capturedCreatePayloads.find((payload) => payload.type === 'telegram'); + expect(telegramPayload).toBeTruthy(); + expect(telegramPayload?.token).toBe('bot123456789:ABCdefGHI'); + expect(telegramPayload?.gotify_token).toBeUndefined(); + expect(telegramPayload?.url).toBe('987654321'); }); }); diff --git a/tests/settings/notifications.spec.ts b/tests/settings/notifications.spec.ts index c6ba22bf..0ec3238e 100644 --- a/tests/settings/notifications.spec.ts +++ b/tests/settings/notifications.spec.ts @@ -294,8 +294,8 @@ test.describe('Notification Providers', () => { await test.step('Verify provider type select contains supported options', async () => { const providerTypeSelect = page.getByTestId('provider-type'); - await expect(providerTypeSelect.locator('option')).toHaveCount(4); - await expect(providerTypeSelect.locator('option')).toHaveText(['Discord', 'Gotify', 'Generic Webhook', 'Email']); + await expect(providerTypeSelect.locator('option')).toHaveCount(5); + await expect(providerTypeSelect.locator('option')).toHaveText(['Discord', 'Gotify', 'Generic Webhook', 'Email', 'Telegram']); await expect(providerTypeSelect).toBeEnabled(); }); }); diff --git a/tests/settings/telegram-notification-provider.spec.ts b/tests/settings/telegram-notification-provider.spec.ts new file mode 100644 index 00000000..213079bb --- /dev/null +++ b/tests/settings/telegram-notification-provider.spec.ts @@ -0,0 +1,472 @@ +/** + * Telegram Notification Provider E2E Tests + * + * Tests the Telegram notification provider type. + * Covers form rendering, CRUD operations, payload contracts, + * token security, and validation behavior specific to the Telegram provider type. + */ + +import { test, expect, loginUser } from '../fixtures/auth-fixtures'; +import { waitForLoadingComplete, waitForAPIResponse } from '../utils/wait-helpers'; + +function generateProviderName(prefix: string = 'telegram-test'): string { + return `${prefix}-${Date.now()}`; +} + +test.describe('Telegram Notification Provider', () => { + test.beforeEach(async ({ page, adminUser }) => { + await loginUser(page, adminUser); + await waitForLoadingComplete(page); + await page.goto('/settings/notifications'); + await waitForLoadingComplete(page); + }); + + test.describe('Form Rendering', () => { + test('should show token field and chat ID placeholder when telegram type selected', async ({ page }) => { + await test.step('Open Add Provider form', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Select telegram provider type', async () => { + await page.getByTestId('provider-type').selectOption('telegram'); + }); + + await test.step('Verify token field is visible', async () => { + await expect(page.getByTestId('provider-gotify-token')).toBeVisible(); + }); + + await test.step('Verify token field label shows Bot Token', async () => { + const tokenLabel = page.getByText(/bot token/i); + await expect(tokenLabel.first()).toBeVisible(); + }); + + await test.step('Verify chat ID placeholder', async () => { + const urlInput = page.getByTestId('provider-url'); + await expect(urlInput).toHaveAttribute('placeholder', '987654321'); + }); + + await test.step('Verify Chat ID label replaces URL label', async () => { + const chatIdLabel = page.getByText(/chat id/i); + await expect(chatIdLabel.first()).toBeVisible(); + }); + + await test.step('Verify JSON template section is shown for telegram', async () => { + await expect(page.getByTestId('provider-config')).toBeVisible(); + }); + + await test.step('Verify save button is accessible', async () => { + const saveButton = page.getByTestId('provider-save-btn'); + await expect(saveButton).toBeVisible(); + await expect(saveButton).toBeEnabled(); + }); + }); + + test('should toggle form fields when switching between telegram and discord types', async ({ page }) => { + await test.step('Open Add Provider form', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Verify discord is default without token field', async () => { + await expect(page.getByTestId('provider-type')).toHaveValue('discord'); + await expect(page.getByTestId('provider-gotify-token')).toHaveCount(0); + }); + + await test.step('Switch to telegram and verify token field appears', async () => { + await page.getByTestId('provider-type').selectOption('telegram'); + await expect(page.getByTestId('provider-gotify-token')).toBeVisible(); + }); + + await test.step('Switch back to discord and verify token field hidden', async () => { + await page.getByTestId('provider-type').selectOption('discord'); + await expect(page.getByTestId('provider-gotify-token')).toHaveCount(0); + }); + }); + }); + + test.describe('CRUD Operations', () => { + test('should create telegram notification provider', async ({ page }) => { + const providerName = generateProviderName('tg-create'); + let capturedPayload: Record | null = null; + + await test.step('Mock create endpoint to capture payload', async () => { + const createdProviders: Array> = []; + + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'POST') { + const payload = (await request.postDataJSON()) as Record; + capturedPayload = payload; + const created = { id: 'tg-provider-1', ...payload }; + createdProviders.push(created); + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify(created), + }); + return; + } + + if (request.method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(createdProviders), + }); + return; + } + + await route.continue(); + }); + }); + + await test.step('Open form and select telegram type', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 }); + await page.getByTestId('provider-type').selectOption('telegram'); + }); + + await test.step('Fill telegram provider form', async () => { + await page.getByTestId('provider-name').fill(providerName); + await page.getByTestId('provider-url').fill('987654321'); + await page.getByTestId('provider-gotify-token').fill('bot123456789:ABCdefGHI'); + }); + + await test.step('Configure event notifications', async () => { + await page.getByTestId('notify-proxy-hosts').check(); + await page.getByTestId('notify-certs').check(); + }); + + await test.step('Save provider', async () => { + await page.getByTestId('provider-save-btn').click(); + }); + + await test.step('Verify provider appears in list', async () => { + const providerInList = page.getByText(providerName); + await expect(providerInList.first()).toBeVisible({ timeout: 10000 }); + }); + + await test.step('Verify outgoing payload contract', async () => { + expect(capturedPayload).toBeTruthy(); + expect(capturedPayload?.type).toBe('telegram'); + expect(capturedPayload?.name).toBe(providerName); + expect(capturedPayload?.url).toBe('987654321'); + expect(capturedPayload?.token).toBe('bot123456789:ABCdefGHI'); + expect(capturedPayload?.gotify_token).toBeUndefined(); + }); + }); + + test('should edit telegram notification provider and preserve token', async ({ page }) => { + let updatedPayload: Record | null = null; + + await test.step('Mock existing telegram provider', async () => { + let providers = [ + { + id: 'tg-edit-id', + name: 'Telegram Alerts', + type: 'telegram', + url: '987654321', + has_token: true, + enabled: true, + notify_proxy_hosts: true, + notify_certs: true, + notify_uptime: false, + }, + ]; + + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(providers), + }); + } else { + await route.continue(); + } + }); + + await page.route('**/api/v1/notifications/providers/*', async (route, request) => { + if (request.method() === 'PUT') { + updatedPayload = (await request.postDataJSON()) as Record; + providers = providers.map((p) => + p.id === 'tg-edit-id' ? { ...p, ...updatedPayload } : p + ); + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ success: true }), + }); + } else { + await route.continue(); + } + }); + }); + + await test.step('Reload to get mocked provider', async () => { + await page.reload(); + await waitForLoadingComplete(page); + }); + + await test.step('Verify telegram provider is displayed', async () => { + await expect(page.getByText('Telegram Alerts')).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Click edit on telegram provider', async () => { + const providerRow = page.getByTestId('provider-row-tg-edit-id'); + const sendTestButton = providerRow.getByRole('button', { name: /send test/i }); + await expect(sendTestButton).toBeVisible({ timeout: 5000 }); + await sendTestButton.focus(); + await page.keyboard.press('Tab'); + await page.keyboard.press('Enter'); + await expect(page.getByTestId('provider-name')).toBeVisible({ timeout: 5000 }); + }); + + await test.step('Verify form loads with telegram type', async () => { + await expect(page.getByTestId('provider-type')).toHaveValue('telegram'); + }); + + await test.step('Verify stored token indicator is shown', async () => { + await expect(page.getByTestId('gotify-token-stored-indicator')).toBeVisible(); + }); + + await test.step('Update name without changing token', async () => { + const nameInput = page.getByTestId('provider-name'); + await nameInput.clear(); + await nameInput.fill('Telegram Alerts v2'); + }); + + await test.step('Save changes', async () => { + const updateResponsePromise = waitForAPIResponse( + page, + /\/api\/v1\/notifications\/providers\/tg-edit-id/, + { status: 200 } + ); + const refreshResponsePromise = waitForAPIResponse( + page, + /\/api\/v1\/notifications\/providers$/, + { status: 200 } + ); + + await page.getByTestId('provider-save-btn').click(); + await updateResponsePromise; + await refreshResponsePromise; + }); + + await test.step('Verify update payload preserves token omission', async () => { + expect(updatedPayload).toBeTruthy(); + expect(updatedPayload?.type).toBe('telegram'); + expect(updatedPayload?.name).toBe('Telegram Alerts v2'); + expect(updatedPayload?.token).toBeUndefined(); + expect(updatedPayload?.gotify_token).toBeUndefined(); + }); + }); + + test('should test telegram notification provider', async ({ page }) => { + let testCalled = false; + + await test.step('Mock existing telegram provider and test endpoint', async () => { + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 'tg-test-id', + name: 'Telegram Test Provider', + type: 'telegram', + url: '987654321', + has_token: true, + enabled: true, + notify_proxy_hosts: true, + notify_certs: true, + notify_uptime: false, + }, + ]), + }); + } else { + await route.continue(); + } + }); + + await page.route('**/api/v1/notifications/providers/test', async (route, request) => { + if (request.method() === 'POST') { + testCalled = true; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ success: true }), + }); + } else { + await route.continue(); + } + }); + }); + + await test.step('Reload to get mocked provider', async () => { + await page.reload(); + await waitForLoadingComplete(page); + }); + + await test.step('Click Send Test on the provider', async () => { + const providerRow = page.getByTestId('provider-row-tg-test-id'); + const sendTestButton = providerRow.getByRole('button', { name: /send test/i }); + await expect(sendTestButton).toBeVisible({ timeout: 5000 }); + await sendTestButton.click(); + }); + + await test.step('Verify test was called', async () => { + expect(testCalled).toBe(true); + }); + }); + + test('should delete telegram notification provider', async ({ page }) => { + await test.step('Mock existing telegram provider', async () => { + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 'tg-delete-id', + name: 'Telegram To Delete', + type: 'telegram', + url: '987654321', + enabled: true, + }, + ]), + }); + } else { + await route.continue(); + } + }); + + await page.route('**/api/v1/notifications/providers/*', async (route, request) => { + if (request.method() === 'DELETE') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ success: true }), + }); + } else { + await route.continue(); + } + }); + }); + + await test.step('Reload to get mocked provider', async () => { + await page.reload(); + await waitForLoadingComplete(page); + }); + + await test.step('Verify telegram provider is displayed', async () => { + await expect(page.getByText('Telegram To Delete')).toBeVisible({ timeout: 10000 }); + }); + + await test.step('Delete provider', async () => { + page.on('dialog', async (dialog) => { + expect(dialog.type()).toBe('confirm'); + await dialog.accept(); + }); + + const deleteButton = page.getByRole('button', { name: /delete/i }) + .or(page.locator('button').filter({ has: page.locator('svg.lucide-trash2, svg[class*="trash"]') })); + await deleteButton.first().click(); + }); + + await test.step('Verify deletion feedback', async () => { + const successIndicator = page.locator('[data-testid="toast-success"]') + .or(page.getByRole('status').filter({ hasText: /deleted|removed/i })) + .or(page.getByText(/no.*providers/i)); + await expect(successIndicator.first()).toBeVisible({ timeout: 5000 }); + }); + }); + }); + + test.describe('Security', () => { + test('GET response should NOT expose bot token', async ({ page }) => { + let apiResponseBody: Array> | null = null; + + await test.step('Mock provider list with has_token flag', async () => { + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'GET') { + const body = [ + { + id: 'tg-sec-id', + name: 'Telegram Secure', + type: 'telegram', + url: '987654321', + has_token: true, + enabled: true, + notify_proxy_hosts: true, + notify_certs: true, + notify_uptime: false, + }, + ]; + apiResponseBody = body; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(body), + }); + } else { + await route.continue(); + } + }); + }); + + await test.step('Navigate to trigger GET', async () => { + await page.goto('/settings/notifications'); + await waitForLoadingComplete(page); + }); + + await test.step('Verify token is not in API response', async () => { + expect(apiResponseBody).toBeTruthy(); + const provider = apiResponseBody![0]; + expect(provider.token).toBeUndefined(); + expect(provider.gotify_token).toBeUndefined(); + const responseStr = JSON.stringify(provider); + expect(responseStr).not.toContain('bot123456789'); + expect(responseStr).not.toContain('ABCdefGHI'); + }); + }); + + test('bot token should NOT be present in URL field', async ({ page }) => { + await test.step('Mock provider with clean URL field', async () => { + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'GET') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 'tg-url-sec-id', + name: 'Telegram URL Check', + type: 'telegram', + url: '987654321', + has_token: true, + enabled: true, + }, + ]), + }); + } else { + await route.continue(); + } + }); + }); + + await test.step('Reload and verify URL field does not contain bot token', async () => { + await page.reload(); + await waitForLoadingComplete(page); + await expect(page.getByText('Telegram URL Check')).toBeVisible({ timeout: 5000 }); + + const providerRow = page.getByTestId('provider-row-tg-url-sec-id'); + const urlText = await providerRow.textContent(); + expect(urlText).not.toContain('bot123456789'); + expect(urlText).not.toContain('api.telegram.org'); + }); + }); + }); +});