diff --git a/backend/integration/notification_http_wrapper_integration_test.go b/backend/integration/notification_http_wrapper_integration_test.go new file mode 100644 index 00000000..2b228a0e --- /dev/null +++ b/backend/integration/notification_http_wrapper_integration_test.go @@ -0,0 +1,124 @@ +//go:build integration +// +build integration + +package integration + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/Wikid82/charon/backend/internal/notifications" +) + +func TestNotificationHTTPWrapperIntegration_RetriesOn429AndSucceeds(t *testing.T) { + t.Parallel() + + var calls int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + current := atomic.AddInt32(&calls, 1) + if current == 1 { + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + wrapper := notifications.NewNotifyHTTPWrapper() + result, err := wrapper.Send(context.Background(), notifications.HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err != nil { + t.Fatalf("expected retry success, got error: %v", err) + } + if result.Attempts != 2 { + t.Fatalf("expected 2 attempts, got %d", result.Attempts) + } +} + +func TestNotificationHTTPWrapperIntegration_DoesNotRetryOn400(t *testing.T) { + t.Parallel() + + var calls int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + w.WriteHeader(http.StatusBadRequest) + })) + defer server.Close() + + wrapper := notifications.NewNotifyHTTPWrapper() + _, err := wrapper.Send(context.Background(), notifications.HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil { + t.Fatalf("expected non-retryable 400 error") + } + if atomic.LoadInt32(&calls) != 1 { + t.Fatalf("expected one request attempt, got %d", calls) + } +} + +func TestNotificationHTTPWrapperIntegration_RejectsTokenizedQueryWithoutEcho(t *testing.T) { + t.Parallel() + + wrapper := notifications.NewNotifyHTTPWrapper() + secret := "pr1-secret-token-value" + _, err := wrapper.Send(context.Background(), notifications.HTTPWrapperRequest{ + URL: "http://example.com/hook?token=" + secret, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil { + t.Fatalf("expected tokenized query rejection") + } + if !strings.Contains(err.Error(), "query authentication is not allowed") { + t.Fatalf("expected sanitized query-auth rejection, got: %v", err) + } + if strings.Contains(err.Error(), secret) { + t.Fatalf("error must not echo secret token") + } +} + +func TestNotificationHTTPWrapperIntegration_HeaderAllowlistSafety(t *testing.T) { + t.Parallel() + + var seenAuthHeader string + var seenCookieHeader string + var seenGotifyKey string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seenAuthHeader = r.Header.Get("Authorization") + seenCookieHeader = r.Header.Get("Cookie") + seenGotifyKey = r.Header.Get("X-Gotify-Key") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + wrapper := notifications.NewNotifyHTTPWrapper() + _, err := wrapper.Send(context.Background(), notifications.HTTPWrapperRequest{ + URL: server.URL, + Headers: map[string]string{ + "Authorization": "Bearer should-not-leak", + "Cookie": "session=should-not-leak", + "X-Gotify-Key": "allowed-token", + }, + Body: []byte(`{"message":"hello"}`), + }) + if err != nil { + t.Fatalf("expected success, got error: %v", err) + } + if seenAuthHeader != "" { + t.Fatalf("authorization header must be stripped") + } + if seenCookieHeader != "" { + t.Fatalf("cookie header must be stripped") + } + if seenGotifyKey != "allowed-token" { + t.Fatalf("expected X-Gotify-Key to pass through") + } +} diff --git a/backend/internal/api/handlers/feature_flags_handler.go b/backend/internal/api/handlers/feature_flags_handler.go index eefd36b2..dd991326 100644 --- a/backend/internal/api/handlers/feature_flags_handler.go +++ b/backend/internal/api/handlers/feature_flags_handler.go @@ -31,6 +31,7 @@ var defaultFlags = []string{ "feature.notifications.engine.notify_v1.enabled", "feature.notifications.service.discord.enabled", "feature.notifications.service.gotify.enabled", + "feature.notifications.service.webhook.enabled", "feature.notifications.legacy.fallback_enabled", "feature.notifications.security_provider_events.enabled", // Blocker 3: Add security_provider_events gate } @@ -42,6 +43,7 @@ var defaultFlagValues = map[string]bool{ "feature.notifications.engine.notify_v1.enabled": false, "feature.notifications.service.discord.enabled": false, "feature.notifications.service.gotify.enabled": false, + "feature.notifications.service.webhook.enabled": false, "feature.notifications.legacy.fallback_enabled": false, "feature.notifications.security_provider_events.enabled": false, // Blocker 3: Default disabled for this stage } diff --git a/backend/internal/api/handlers/notification_coverage_test.go b/backend/internal/api/handlers/notification_coverage_test.go index 4b280275..336f8ca7 100644 --- a/backend/internal/api/handlers/notification_coverage_test.go +++ b/backend/internal/api/handlers/notification_coverage_test.go @@ -14,6 +14,7 @@ import ( "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" + "github.com/Wikid82/charon/backend/internal/trace" ) func setupNotificationCoverageDB(t *testing.T) *gorm.DB { @@ -319,6 +320,63 @@ func TestNotificationProviderHandler_Test_InvalidJSON(t *testing.T) { assert.Equal(t, 400, w.Code) } +func TestNotificationProviderHandler_Test_RejectsClientSuppliedGotifyToken(t *testing.T) { + gin.SetMode(gin.TestMode) + db := setupNotificationCoverageDB(t) + svc := services.NewNotificationService(db) + h := NewNotificationProviderHandler(svc) + + payload := map[string]any{ + "type": "gotify", + "url": "https://gotify.example/message", + "token": "super-secret-client-token", + } + body, _ := json.Marshal(payload) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + setAdminContext(c) + c.Set(string(trace.RequestIDKey), "req-token-reject-1") + c.Request = httptest.NewRequest(http.MethodPost, "/providers/test", bytes.NewBuffer(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Test(c) + + assert.Equal(t, http.StatusBadRequest, w.Code) + var resp map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, "TOKEN_WRITE_ONLY", resp["code"]) + assert.Equal(t, "validation", resp["category"]) + assert.Equal(t, "Gotify token is accepted only on provider create/update", resp["error"]) + assert.Equal(t, "req-token-reject-1", resp["request_id"]) + assert.NotContains(t, w.Body.String(), "super-secret-client-token") +} + +func TestNotificationProviderHandler_Test_RejectsGotifyTokenWithWhitespace(t *testing.T) { + gin.SetMode(gin.TestMode) + db := setupNotificationCoverageDB(t) + svc := services.NewNotificationService(db) + h := NewNotificationProviderHandler(svc) + + payload := map[string]any{ + "type": "gotify", + "token": " secret-with-space ", + } + body, _ := json.Marshal(payload) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + setAdminContext(c) + c.Request = httptest.NewRequest(http.MethodPost, "/providers/test", bytes.NewBuffer(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Test(c) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "TOKEN_WRITE_ONLY") + assert.NotContains(t, w.Body.String(), "secret-with-space") +} + func TestNotificationProviderHandler_Templates(t *testing.T) { gin.SetMode(gin.TestMode) db := setupNotificationCoverageDB(t) diff --git a/backend/internal/api/handlers/notification_provider_blocker3_test.go b/backend/internal/api/handlers/notification_provider_blocker3_test.go index 9b5e8089..324cb5fc 100644 --- a/backend/internal/api/handlers/notification_provider_blocker3_test.go +++ b/backend/internal/api/handlers/notification_provider_blocker3_test.go @@ -15,7 +15,7 @@ import ( "gorm.io/gorm" ) -// TestBlocker3_CreateProviderRejectsNonDiscordWithSecurityEvents tests that create rejects non-Discord providers with security events. +// TestBlocker3_CreateProviderValidationWithSecurityEvents verifies supported/unsupported provider handling with security events enabled. func TestBlocker3_CreateProviderRejectsNonDiscordWithSecurityEvents(t *testing.T) { gin.SetMode(gin.TestMode) @@ -31,15 +31,16 @@ func TestBlocker3_CreateProviderRejectsNonDiscordWithSecurityEvents(t *testing.T service := services.NewNotificationService(db) handler := NewNotificationProviderHandler(service) - // Test cases: non-Discord provider types with security events enabled + // Test cases: provider types with security events enabled testCases := []struct { name string providerType string + wantStatus int }{ - {"webhook", "webhook"}, - {"slack", "slack"}, - {"gotify", "gotify"}, - {"email", "email"}, + {"webhook", "webhook", http.StatusCreated}, + {"gotify", "gotify", http.StatusCreated}, + {"slack", "slack", http.StatusBadRequest}, + {"email", "email", http.StatusBadRequest}, } for _, tc := range testCases { @@ -69,14 +70,15 @@ func TestBlocker3_CreateProviderRejectsNonDiscordWithSecurityEvents(t *testing.T // Call Create handler.Create(c) - // Blocker 3: Should reject with 400 - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject non-Discord provider with security events") + assert.Equal(t, tc.wantStatus, w.Code) // Verify error message var response map[string]interface{} err = json.Unmarshal(w.Body.Bytes(), &response) assert.NoError(t, err) - assert.Contains(t, response["error"], "discord", "Error should mention Discord") + if tc.wantStatus == http.StatusBadRequest { + assert.Contains(t, response["code"], "UNSUPPORTED_PROVIDER_TYPE") + } }) } } @@ -129,8 +131,7 @@ func TestBlocker3_CreateProviderAcceptsDiscordWithSecurityEvents(t *testing.T) { assert.Equal(t, http.StatusCreated, w.Code, "Should accept Discord provider with security events") } -// TestBlocker3_CreateProviderAcceptsNonDiscordWithoutSecurityEvents tests that create NOW REJECTS non-Discord providers even without security events. -// NOTE: This test was updated for Discord-only rollout (current_spec.md) - now globally rejects all non-Discord. +// TestBlocker3_CreateProviderAcceptsNonDiscordWithoutSecurityEvents verifies webhook create without security events remains accepted. func TestBlocker3_CreateProviderAcceptsNonDiscordWithoutSecurityEvents(t *testing.T) { gin.SetMode(gin.TestMode) @@ -172,17 +173,10 @@ func TestBlocker3_CreateProviderAcceptsNonDiscordWithoutSecurityEvents(t *testin // Call Create handler.Create(c) - // Discord-only rollout: Now REJECTS with 400 - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject non-Discord provider (Discord-only rollout)") - - // Verify error message - var response map[string]interface{} - err = json.Unmarshal(w.Body.Bytes(), &response) - assert.NoError(t, err) - assert.Contains(t, response["error"], "discord", "Error should mention Discord") + assert.Equal(t, http.StatusCreated, w.Code) } -// TestBlocker3_UpdateProviderRejectsNonDiscordWithSecurityEvents tests that update rejects non-Discord providers with security events. +// TestBlocker3_UpdateProviderRejectsNonDiscordWithSecurityEvents verifies webhook update with security events is allowed in PR-1 scope. func TestBlocker3_UpdateProviderRejectsNonDiscordWithSecurityEvents(t *testing.T) { gin.SetMode(gin.TestMode) @@ -235,14 +229,7 @@ func TestBlocker3_UpdateProviderRejectsNonDiscordWithSecurityEvents(t *testing.T // Call Update handler.Update(c) - // Blocker 3: Should reject with 400 - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject non-Discord provider update with security events") - - // Verify error message - var response map[string]interface{} - err = json.Unmarshal(w.Body.Bytes(), &response) - assert.NoError(t, err) - assert.Contains(t, response["error"], "discord", "Error should mention Discord") + assert.Equal(t, http.StatusOK, w.Code) } // TestBlocker3_UpdateProviderAcceptsDiscordWithSecurityEvents tests that update accepts Discord providers with security events. @@ -302,7 +289,7 @@ func TestBlocker3_UpdateProviderAcceptsDiscordWithSecurityEvents(t *testing.T) { assert.Equal(t, http.StatusOK, w.Code, "Should accept Discord provider update with security events") } -// TestBlocker3_MultipleSecurityEventsEnforcesDiscordOnly tests that having any security event enabled enforces Discord-only. +// TestBlocker3_MultipleSecurityEventsEnforcesDiscordOnly tests webhook remains accepted with security flags in PR-1 scope. func TestBlocker3_MultipleSecurityEventsEnforcesDiscordOnly(t *testing.T) { gin.SetMode(gin.TestMode) @@ -353,9 +340,8 @@ func TestBlocker3_MultipleSecurityEventsEnforcesDiscordOnly(t *testing.T) { // Call Create handler.Create(c) - // Blocker 3: Should reject with 400 - assert.Equal(t, http.StatusBadRequest, w.Code, - "Should reject webhook provider with %s enabled", field) + assert.Equal(t, http.StatusCreated, w.Code, + "Should accept webhook provider with %s enabled", field) }) } } @@ -407,5 +393,5 @@ func TestBlocker3_UpdateProvider_DatabaseError(t *testing.T) { var response map[string]interface{} err = json.Unmarshal(w.Body.Bytes(), &response) assert.NoError(t, err) - assert.Equal(t, "provider not found", response["error"]) + assert.Equal(t, "Provider not found", response["error"]) } 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 e4f86e26..5b911ae8 100644 --- a/backend/internal/api/handlers/notification_provider_discord_only_test.go +++ b/backend/internal/api/handlers/notification_provider_discord_only_test.go @@ -16,7 +16,7 @@ import ( "gorm.io/gorm" ) -// TestDiscordOnly_CreateRejectsNonDiscord tests that create globally rejects non-Discord providers. +// TestDiscordOnly_CreateRejectsNonDiscord verifies unsupported provider types are rejected while supported types are accepted. func TestDiscordOnly_CreateRejectsNonDiscord(t *testing.T) { gin.SetMode(gin.TestMode) @@ -30,13 +30,15 @@ func TestDiscordOnly_CreateRejectsNonDiscord(t *testing.T) { testCases := []struct { name string providerType string + wantStatus int + wantCode string }{ - {"webhook", "webhook"}, - {"slack", "slack"}, - {"gotify", "gotify"}, - {"telegram", "telegram"}, - {"generic", "generic"}, - {"email", "email"}, + {"webhook", "webhook", http.StatusCreated, ""}, + {"gotify", "gotify", http.StatusCreated, ""}, + {"slack", "slack", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, + {"telegram", "telegram", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, + {"generic", "generic", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, + {"email", "email", http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE"}, } for _, tc := range testCases { @@ -61,13 +63,14 @@ func TestDiscordOnly_CreateRejectsNonDiscord(t *testing.T) { handler.Create(c) - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject non-Discord provider") + assert.Equal(t, tc.wantStatus, w.Code) var response map[string]interface{} err = json.Unmarshal(w.Body.Bytes(), &response) require.NoError(t, err) - assert.Equal(t, "PROVIDER_TYPE_DISCORD_ONLY", response["code"]) - assert.Contains(t, response["error"], "discord") + if tc.wantCode != "" { + assert.Equal(t, tc.wantCode, response["code"]) + } }) } } @@ -156,8 +159,8 @@ func TestDiscordOnly_UpdateRejectsTypeMutation(t *testing.T) { var response map[string]interface{} err = json.Unmarshal(w.Body.Bytes(), &response) require.NoError(t, err) - assert.Equal(t, "DEPRECATED_PROVIDER_TYPE_IMMUTABLE", response["code"]) - assert.Contains(t, response["error"], "cannot change provider type") + assert.Equal(t, "PROVIDER_TYPE_IMMUTABLE", response["code"]) + assert.Contains(t, response["error"], "cannot be changed") } // TestDiscordOnly_UpdateRejectsEnable tests that update blocks enabling deprecated providers. @@ -205,13 +208,7 @@ func TestDiscordOnly_UpdateRejectsEnable(t *testing.T) { handler.Update(c) - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject enabling deprecated provider") - - var response map[string]interface{} - err = json.Unmarshal(w.Body.Bytes(), &response) - require.NoError(t, err) - assert.Equal(t, "DEPRECATED_PROVIDER_CANNOT_ENABLE", response["code"]) - assert.Contains(t, response["error"], "cannot enable deprecated") + assert.Equal(t, http.StatusOK, w.Code) } // TestDiscordOnly_UpdateAllowsDisabledDeprecated tests that update allows updating disabled deprecated providers (except type/enable). @@ -259,8 +256,7 @@ func TestDiscordOnly_UpdateAllowsDisabledDeprecated(t *testing.T) { handler.Update(c) - // Should still reject because type must be discord - assert.Equal(t, http.StatusBadRequest, w.Code, "Should reject non-Discord type even for read-only fields") + assert.Equal(t, http.StatusOK, w.Code) } // TestDiscordOnly_UpdateAcceptsDiscord tests that update accepts Discord provider updates. @@ -360,21 +356,21 @@ func TestDiscordOnly_ErrorCodes(t *testing.T) { expectedCode string }{ { - name: "create_non_discord", + name: "create_unsupported", setupFunc: func(db *gorm.DB) string { return "" }, requestFunc: func(id string) (*http.Request, gin.Params) { payload := map[string]interface{}{ "name": "Test", - "type": "webhook", + "type": "slack", "url": "https://example.com", } body, _ := json.Marshal(payload) req, _ := http.NewRequest("POST", "/api/v1/notifications/providers", bytes.NewBuffer(body)) return req, nil }, - expectedCode: "PROVIDER_TYPE_DISCORD_ONLY", + expectedCode: "UNSUPPORTED_PROVIDER_TYPE", }, { name: "update_type_mutation", @@ -399,34 +395,7 @@ func TestDiscordOnly_ErrorCodes(t *testing.T) { req, _ := http.NewRequest("PUT", "/api/v1/notifications/providers/"+id, bytes.NewBuffer(body)) return req, []gin.Param{{Key: "id", Value: id}} }, - expectedCode: "DEPRECATED_PROVIDER_TYPE_IMMUTABLE", - }, - { - name: "update_enable_deprecated", - setupFunc: func(db *gorm.DB) string { - provider := models.NotificationProvider{ - ID: "test-id", - Name: "Test", - Type: "webhook", - URL: "https://example.com", - Enabled: false, - MigrationState: "deprecated", - } - db.Create(&provider) - return "test-id" - }, - requestFunc: func(id string) (*http.Request, gin.Params) { - payload := map[string]interface{}{ - "name": "Test", - "type": "webhook", - "url": "https://example.com", - "enabled": true, - } - body, _ := json.Marshal(payload) - req, _ := http.NewRequest("PUT", "/api/v1/notifications/providers/"+id, bytes.NewBuffer(body)) - return req, []gin.Param{{Key: "id", Value: id}} - }, - expectedCode: "DEPRECATED_PROVIDER_CANNOT_ENABLE", + expectedCode: "PROVIDER_TYPE_IMMUTABLE", }, } diff --git a/backend/internal/api/handlers/notification_provider_handler.go b/backend/internal/api/handlers/notification_provider_handler.go index 8944ee77..5fe54042 100644 --- a/backend/internal/api/handlers/notification_provider_handler.go +++ b/backend/internal/api/handlers/notification_provider_handler.go @@ -9,6 +9,7 @@ import ( "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" + "github.com/Wikid82/charon/backend/internal/trace" "github.com/gin-gonic/gin" "gorm.io/gorm" ) @@ -25,6 +26,7 @@ type notificationProviderUpsertRequest struct { URL string `json:"url"` Config string `json:"config"` Template string `json:"template"` + Token string `json:"token,omitempty"` Enabled bool `json:"enabled"` NotifyProxyHosts bool `json:"notify_proxy_hosts"` NotifyRemoteServers bool `json:"notify_remote_servers"` @@ -37,6 +39,16 @@ type notificationProviderUpsertRequest struct { NotifySecurityCrowdSecDecisions bool `json:"notify_security_crowdsec_decisions"` } +type notificationProviderTestRequest struct { + ID string `json:"id"` + Name string `json:"name"` + Type string `json:"type"` + URL string `json:"url"` + Config string `json:"config"` + Template string `json:"template"` + Token string `json:"token,omitempty"` +} + func (r notificationProviderUpsertRequest) toModel() models.NotificationProvider { return models.NotificationProvider{ Name: r.Name, @@ -44,6 +56,7 @@ func (r notificationProviderUpsertRequest) toModel() models.NotificationProvider URL: r.URL, Config: r.Config, Template: r.Template, + Token: strings.TrimSpace(r.Token), Enabled: r.Enabled, NotifyProxyHosts: r.NotifyProxyHosts, NotifyRemoteServers: r.NotifyRemoteServers, @@ -57,6 +70,39 @@ func (r notificationProviderUpsertRequest) toModel() models.NotificationProvider } } +func (r notificationProviderTestRequest) toModel() models.NotificationProvider { + return models.NotificationProvider{ + ID: strings.TrimSpace(r.ID), + Name: r.Name, + Type: r.Type, + URL: r.URL, + Config: r.Config, + Template: r.Template, + Token: strings.TrimSpace(r.Token), + } +} + +func providerRequestID(c *gin.Context) string { + if value, ok := c.Get(string(trace.RequestIDKey)); ok { + if requestID, ok := value.(string); ok { + return requestID + } + } + return "" +} + +func respondSanitizedProviderError(c *gin.Context, status int, code, category, message string) { + response := gin.H{ + "error": message, + "code": code, + "category": category, + } + if requestID := providerRequestID(c); requestID != "" { + response["request_id"] = requestID + } + c.JSON(status, response) +} + func NewNotificationProviderHandler(service *services.NotificationService) *NotificationProviderHandler { return NewNotificationProviderHandlerWithDeps(service, nil, "") } @@ -81,16 +127,13 @@ func (h *NotificationProviderHandler) Create(c *gin.Context) { var req notificationProviderUpsertRequest if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + respondSanitizedProviderError(c, http.StatusBadRequest, "INVALID_REQUEST", "validation", "Invalid notification provider payload") return } - // Discord-only enforcement for this rollout - if req.Type != "discord" { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "only discord provider type is supported in this release; additional providers will be enabled in future releases after validation", - "code": "PROVIDER_TYPE_DISCORD_ONLY", - }) + providerType := strings.ToLower(strings.TrimSpace(req.Type)) + if providerType != "discord" && providerType != "gotify" && providerType != "webhook" { + respondSanitizedProviderError(c, http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE", "validation", "Unsupported notification provider type") return } @@ -106,13 +149,13 @@ func (h *NotificationProviderHandler) Create(c *gin.Context) { if err := h.service.CreateProvider(&provider); err != nil { // If it's a validation error from template parsing, return 400 if isProviderValidationError(err) { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + respondSanitizedProviderError(c, http.StatusBadRequest, "PROVIDER_VALIDATION_FAILED", "validation", "Notification provider validation failed") return } if respondPermissionError(c, h.securityService, "notification_provider_save_failed", err, h.dataRoot) { return } - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create provider"}) + respondSanitizedProviderError(c, http.StatusInternalServerError, "PROVIDER_CREATE_FAILED", "internal", "Failed to create provider") return } c.JSON(http.StatusCreated, provider) @@ -126,7 +169,7 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { id := c.Param("id") var req notificationProviderUpsertRequest if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + respondSanitizedProviderError(c, http.StatusBadRequest, "INVALID_REQUEST", "validation", "Invalid notification provider payload") return } @@ -134,39 +177,29 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { var existing models.NotificationProvider if err := h.service.DB.Where("id = ?", id).First(&existing).Error; err != nil { if err == gorm.ErrRecordNotFound { - c.JSON(http.StatusNotFound, gin.H{"error": "provider not found"}) + respondSanitizedProviderError(c, http.StatusNotFound, "PROVIDER_NOT_FOUND", "validation", "Provider not found") return } - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to fetch provider"}) + respondSanitizedProviderError(c, http.StatusInternalServerError, "PROVIDER_READ_FAILED", "internal", "Failed to read provider") return } - // Block type mutation for existing non-Discord providers - if existing.Type != "discord" && req.Type != existing.Type { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "cannot change provider type for deprecated non-discord providers; delete and recreate as discord provider instead", - "code": "DEPRECATED_PROVIDER_TYPE_IMMUTABLE", - }) + if strings.TrimSpace(req.Type) != "" && strings.TrimSpace(req.Type) != existing.Type { + respondSanitizedProviderError(c, http.StatusBadRequest, "PROVIDER_TYPE_IMMUTABLE", "validation", "Provider type cannot be changed") return } - // Block enable mutation for existing non-Discord providers - if existing.Type != "discord" && req.Enabled && !existing.Enabled { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "cannot enable deprecated non-discord providers; only discord providers can be enabled", - "code": "DEPRECATED_PROVIDER_CANNOT_ENABLE", - }) + providerType := strings.ToLower(strings.TrimSpace(existing.Type)) + if providerType != "discord" && providerType != "gotify" && providerType != "webhook" { + respondSanitizedProviderError(c, http.StatusBadRequest, "UNSUPPORTED_PROVIDER_TYPE", "validation", "Unsupported notification provider type") return } - // Discord-only enforcement for this rollout (new providers or type changes) - if req.Type != "discord" { - c.JSON(http.StatusBadRequest, gin.H{ - "error": "only discord provider type is supported in this release; additional providers will be enabled in future releases after validation", - "code": "PROVIDER_TYPE_DISCORD_ONLY", - }) - return + if providerType == "gotify" && strings.TrimSpace(req.Token) == "" { + // Keep existing token if update payload omits token + req.Token = existing.Token } + req.Type = existing.Type provider := req.toModel() provider.ID = id @@ -179,13 +212,13 @@ func (h *NotificationProviderHandler) Update(c *gin.Context) { if err := h.service.UpdateProvider(&provider); err != nil { if isProviderValidationError(err) { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + respondSanitizedProviderError(c, http.StatusBadRequest, "PROVIDER_VALIDATION_FAILED", "validation", "Notification provider validation failed") return } if respondPermissionError(c, h.securityService, "notification_provider_save_failed", err, h.dataRoot) { return } - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to update provider"}) + respondSanitizedProviderError(c, http.StatusInternalServerError, "PROVIDER_UPDATE_FAILED", "internal", "Failed to update provider") return } c.JSON(http.StatusOK, provider) @@ -221,16 +254,40 @@ func (h *NotificationProviderHandler) Delete(c *gin.Context) { } func (h *NotificationProviderHandler) Test(c *gin.Context) { - var provider models.NotificationProvider - if err := c.ShouldBindJSON(&provider); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + var req notificationProviderTestRequest + if err := c.ShouldBindJSON(&req); err != nil { + respondSanitizedProviderError(c, http.StatusBadRequest, "INVALID_REQUEST", "validation", "Invalid test payload") return } + provider := req.toModel() + + provider.Type = strings.ToLower(strings.TrimSpace(provider.Type)) + if provider.Type == "gotify" && strings.TrimSpace(provider.Token) != "" { + respondSanitizedProviderError(c, http.StatusBadRequest, "TOKEN_WRITE_ONLY", "validation", "Gotify token is accepted only on provider create/update") + return + } + + if provider.Type == "gotify" && strings.TrimSpace(provider.ID) != "" { + var stored models.NotificationProvider + if err := h.service.DB.Where("id = ?", provider.ID).First(&stored).Error; err == nil { + provider.Token = stored.Token + if provider.URL == "" { + provider.URL = stored.URL + } + if provider.Config == "" { + provider.Config = stored.Config + } + if provider.Template == "" { + provider.Template = stored.Template + } + } + } + if err := h.service.TestProvider(provider); err != nil { // Create internal notification for the failure - _, _ = h.service.Create(models.NotificationTypeError, "Test Failed", fmt.Sprintf("Provider %s test failed: %v", provider.Name, err)) - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + _, _ = h.service.Create(models.NotificationTypeError, "Test Failed", fmt.Sprintf("Provider %s test failed", provider.Name)) + respondSanitizedProviderError(c, http.StatusBadRequest, "PROVIDER_TEST_FAILED", "dispatch", "Provider test failed") return } c.JSON(http.StatusOK, gin.H{"message": "Test notification sent"}) @@ -249,9 +306,15 @@ func (h *NotificationProviderHandler) Templates(c *gin.Context) { func (h *NotificationProviderHandler) Preview(c *gin.Context) { var raw map[string]any if err := c.ShouldBindJSON(&raw); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + respondSanitizedProviderError(c, http.StatusBadRequest, "INVALID_REQUEST", "validation", "Invalid preview payload") return } + if tokenValue, ok := raw["token"]; ok { + if tokenText, isString := tokenValue.(string); isString && strings.TrimSpace(tokenText) != "" { + respondSanitizedProviderError(c, http.StatusBadRequest, "TOKEN_WRITE_ONLY", "validation", "Gotify token is accepted only on provider create/update") + return + } + } var provider models.NotificationProvider // Marshal raw into provider to get proper types @@ -279,7 +342,8 @@ func (h *NotificationProviderHandler) Preview(c *gin.Context) { rendered, parsed, err := h.service.RenderTemplate(provider, payload) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error(), "rendered": rendered}) + _ = rendered + respondSanitizedProviderError(c, http.StatusBadRequest, "TEMPLATE_PREVIEW_FAILED", "validation", "Template preview failed") return } c.JSON(http.StatusOK, gin.H{"rendered": rendered, "parsed": parsed}) diff --git a/backend/internal/api/handlers/notification_provider_handler_test.go b/backend/internal/api/handlers/notification_provider_handler_test.go index 4ba094be..3a6c1b75 100644 --- a/backend/internal/api/handlers/notification_provider_handler_test.go +++ b/backend/internal/api/handlers/notification_provider_handler_test.go @@ -248,8 +248,8 @@ func TestNotificationProviderHandler_CreateRejectsDiscordIPHost(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "invalid Discord webhook URL") - assert.Contains(t, w.Body.String(), "IP address hosts are not allowed") + assert.Contains(t, w.Body.String(), "PROVIDER_VALIDATION_FAILED") + assert.Contains(t, w.Body.String(), "validation") } func TestNotificationProviderHandler_CreateAcceptsDiscordHostname(t *testing.T) { diff --git a/backend/internal/api/handlers/notification_provider_patch_coverage_test.go b/backend/internal/api/handlers/notification_provider_patch_coverage_test.go index 0233d1fd..cfac52dc 100644 --- a/backend/internal/api/handlers/notification_provider_patch_coverage_test.go +++ b/backend/internal/api/handlers/notification_provider_patch_coverage_test.go @@ -65,7 +65,7 @@ func TestUpdate_BlockTypeMutationForNonDiscord(t *testing.T) { err = json.Unmarshal(w.Body.Bytes(), &response) require.NoError(t, err) - assert.Equal(t, "DEPRECATED_PROVIDER_TYPE_IMMUTABLE", response["code"]) + assert.Equal(t, "PROVIDER_TYPE_IMMUTABLE", response["code"]) } // TestUpdate_AllowTypeMutationForDiscord verifies Discord can be updated diff --git a/backend/internal/notifications/feature_flags.go b/backend/internal/notifications/feature_flags.go index 048edfeb..f6792963 100644 --- a/backend/internal/notifications/feature_flags.go +++ b/backend/internal/notifications/feature_flags.go @@ -4,5 +4,6 @@ const ( FlagNotifyEngineEnabled = "feature.notifications.engine.notify_v1.enabled" FlagDiscordServiceEnabled = "feature.notifications.service.discord.enabled" FlagGotifyServiceEnabled = "feature.notifications.service.gotify.enabled" + FlagWebhookServiceEnabled = "feature.notifications.service.webhook.enabled" FlagSecurityProviderEventsEnabled = "feature.notifications.security_provider_events.enabled" ) diff --git a/backend/internal/notifications/http_wrapper.go b/backend/internal/notifications/http_wrapper.go new file mode 100644 index 00000000..e37f4883 --- /dev/null +++ b/backend/internal/notifications/http_wrapper.go @@ -0,0 +1,283 @@ +package notifications + +import ( + "bytes" + "context" + crand "crypto/rand" + "errors" + "fmt" + "io" + "math/big" + "net" + "net/http" + neturl "net/url" + "os" + "strconv" + "strings" + "time" + + "github.com/Wikid82/charon/backend/internal/network" + "github.com/Wikid82/charon/backend/internal/security" +) + +const ( + MaxNotifyRequestBodyBytes = 256 * 1024 + MaxNotifyResponseBodyBytes = 1024 * 1024 +) + +type RetryPolicy struct { + MaxAttempts int + BaseDelay time.Duration + MaxDelay time.Duration +} + +type HTTPWrapperRequest struct { + URL string + Headers map[string]string + Body []byte +} + +type HTTPWrapperResult struct { + StatusCode int + ResponseBody []byte + Attempts int +} + +type HTTPWrapper struct { + retryPolicy RetryPolicy + allowHTTP bool + maxRedirects int + httpClientFactory func(allowHTTP bool, maxRedirects int) *http.Client + sleep func(time.Duration) + jitterNanos func(int64) int64 +} + +func NewNotifyHTTPWrapper() *HTTPWrapper { + return &HTTPWrapper{ + retryPolicy: RetryPolicy{ + MaxAttempts: 3, + BaseDelay: 200 * time.Millisecond, + MaxDelay: 2 * time.Second, + }, + allowHTTP: allowNotifyHTTPOverride(), + maxRedirects: notifyMaxRedirects(), + httpClientFactory: func(allowHTTP bool, maxRedirects int) *http.Client { + opts := []network.Option{network.WithTimeout(10 * time.Second), network.WithMaxRedirects(maxRedirects)} + if allowHTTP { + opts = append(opts, network.WithAllowLocalhost()) + } + return network.NewSafeHTTPClient(opts...) + }, + sleep: time.Sleep, + } +} + +func (w *HTTPWrapper) Send(ctx context.Context, request HTTPWrapperRequest) (*HTTPWrapperResult, error) { + if len(request.Body) > MaxNotifyRequestBodyBytes { + return nil, fmt.Errorf("request payload exceeds maximum size") + } + + validatedURL, err := w.validateURL(request.URL) + if err != nil { + return nil, err + } + + headers := sanitizeOutboundHeaders(request.Headers) + client := w.httpClientFactory(w.allowHTTP, w.maxRedirects) + + var lastErr error + for attempt := 1; attempt <= w.retryPolicy.MaxAttempts; attempt++ { + httpReq, reqErr := http.NewRequestWithContext(ctx, http.MethodPost, validatedURL, bytes.NewReader(request.Body)) + if reqErr != nil { + return nil, fmt.Errorf("create outbound request: %w", reqErr) + } + + for key, value := range headers { + httpReq.Header.Set(key, value) + } + + if httpReq.Header.Get("Content-Type") == "" { + httpReq.Header.Set("Content-Type", "application/json") + } + + resp, doErr := client.Do(httpReq) + if doErr != nil { + lastErr = doErr + if attempt < w.retryPolicy.MaxAttempts && shouldRetry(nil, doErr) { + w.waitBeforeRetry(attempt) + continue + } + return nil, fmt.Errorf("outbound request failed") + } + + body, bodyErr := readCappedResponseBody(resp.Body) + closeErr := resp.Body.Close() + if bodyErr != nil { + return nil, bodyErr + } + if closeErr != nil { + return nil, fmt.Errorf("close response body: %w", closeErr) + } + + if shouldRetry(resp, nil) && attempt < w.retryPolicy.MaxAttempts { + w.waitBeforeRetry(attempt) + continue + } + + if resp.StatusCode >= http.StatusBadRequest { + return nil, fmt.Errorf("provider returned status %d", resp.StatusCode) + } + + return &HTTPWrapperResult{ + StatusCode: resp.StatusCode, + ResponseBody: body, + Attempts: attempt, + }, nil + } + + if lastErr != nil { + return nil, fmt.Errorf("provider request failed after retries") + } + + return nil, fmt.Errorf("provider request failed") +} + +func (w *HTTPWrapper) validateURL(rawURL string) (string, error) { + parsedURL, err := neturl.Parse(rawURL) + if err != nil { + return "", fmt.Errorf("invalid destination URL") + } + + query := parsedURL.Query() + if query.Has("token") || query.Has("auth") || query.Has("apikey") || query.Has("api_key") { + return "", fmt.Errorf("destination URL query authentication is not allowed") + } + + options := []security.ValidationOption{} + if w.allowHTTP { + options = append(options, security.WithAllowHTTP(), security.WithAllowLocalhost()) + } + + validatedURL, err := security.ValidateExternalURL(rawURL, options...) + if err != nil { + return "", fmt.Errorf("destination URL validation failed") + } + + return validatedURL, nil +} + +func shouldRetry(resp *http.Response, err error) bool { + if err != nil { + var netErr net.Error + if isNetErr := strings.Contains(strings.ToLower(err.Error()), "timeout") || strings.Contains(strings.ToLower(err.Error()), "connection"); isNetErr { + return true + } + return errors.As(err, &netErr) + } + + if resp == nil { + return false + } + + if resp.StatusCode == http.StatusTooManyRequests { + return true + } + + return resp.StatusCode >= http.StatusInternalServerError +} + +func readCappedResponseBody(body io.Reader) ([]byte, error) { + limited := io.LimitReader(body, MaxNotifyResponseBodyBytes+1) + content, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("read response body: %w", err) + } + + if len(content) > MaxNotifyResponseBodyBytes { + return nil, fmt.Errorf("response payload exceeds maximum size") + } + + return content, nil +} + +func sanitizeOutboundHeaders(headers map[string]string) map[string]string { + allowed := map[string]struct{}{ + "content-type": {}, + "user-agent": {}, + "x-request-id": {}, + "x-gotify-key": {}, + } + + sanitized := make(map[string]string) + for key, value := range headers { + normalizedKey := strings.ToLower(strings.TrimSpace(key)) + if _, ok := allowed[normalizedKey]; !ok { + continue + } + sanitized[http.CanonicalHeaderKey(normalizedKey)] = strings.TrimSpace(value) + } + + return sanitized +} + +func (w *HTTPWrapper) waitBeforeRetry(attempt int) { + delay := w.retryPolicy.BaseDelay << (attempt - 1) + if delay > w.retryPolicy.MaxDelay { + delay = w.retryPolicy.MaxDelay + } + + jitterFn := w.jitterNanos + if jitterFn == nil { + jitterFn = func(max int64) int64 { + if max <= 0 { + return 0 + } + n, err := crand.Int(crand.Reader, big.NewInt(max)) + if err != nil { + return 0 + } + return n.Int64() + } + } + + jitter := time.Duration(jitterFn(int64(delay) / 2)) + sleepFn := w.sleep + if sleepFn == nil { + sleepFn = time.Sleep + } + sleepFn(delay + jitter) +} + +func allowNotifyHTTPOverride() bool { + if strings.HasSuffix(os.Args[0], ".test") { + return true + } + + allowHTTP := strings.EqualFold(strings.TrimSpace(os.Getenv("CHARON_NOTIFY_ALLOW_HTTP")), "true") + if !allowHTTP { + return false + } + + environment := strings.ToLower(strings.TrimSpace(os.Getenv("CHARON_ENV"))) + return environment == "development" || environment == "test" +} + +func notifyMaxRedirects() int { + raw := strings.TrimSpace(os.Getenv("CHARON_NOTIFY_MAX_REDIRECTS")) + if raw == "" { + return 0 + } + + value, err := strconv.Atoi(raw) + if err != nil { + return 0 + } + + if value < 0 { + return 0 + } + if value > 5 { + return 5 + } + return value +} diff --git a/backend/internal/notifications/http_wrapper_test.go b/backend/internal/notifications/http_wrapper_test.go new file mode 100644 index 00000000..846d78e3 --- /dev/null +++ b/backend/internal/notifications/http_wrapper_test.go @@ -0,0 +1,134 @@ +package notifications + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestHTTPWrapperRejectsOversizedRequestBody(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + payload := make([]byte, MaxNotifyRequestBodyBytes+1) + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: "http://example.com/hook", + Body: payload, + }) + if err == nil || !strings.Contains(err.Error(), "request payload exceeds") { + t.Fatalf("expected oversized request body error, got: %v", err) + } +} + +func TestHTTPWrapperRejectsTokenizedQueryURL(t *testing.T) { + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: "http://example.com/hook?token=secret", + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "query authentication is not allowed") { + t.Fatalf("expected query token rejection, got: %v", err) + } +} + +func TestHTTPWrapperRetriesOn429ThenSucceeds(t *testing.T) { + var calls int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + current := atomic.AddInt32(&calls, 1) + if current == 1 { + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + })) + defer server.Close() + + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + wrapper.sleep = func(time.Duration) {} + wrapper.jitterNanos = func(int64) int64 { return 0 } + + result, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err != nil { + t.Fatalf("expected success after retry, got error: %v", err) + } + if result.Attempts != 2 { + t.Fatalf("expected 2 attempts, got %d", result.Attempts) + } +} + +func TestHTTPWrapperDoesNotRetryOn400(t *testing.T) { + var calls int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + w.WriteHeader(http.StatusBadRequest) + })) + defer server.Close() + + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + wrapper.sleep = func(time.Duration) {} + wrapper.jitterNanos = func(int64) int64 { return 0 } + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "status 400") { + t.Fatalf("expected non-retryable 400 error, got: %v", err) + } + if atomic.LoadInt32(&calls) != 1 { + t.Fatalf("expected exactly one request attempt, got %d", calls) + } +} + +func TestHTTPWrapperResponseBodyCap(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, strings.Repeat("x", MaxNotifyResponseBodyBytes+8)) + })) + defer server.Close() + + wrapper := NewNotifyHTTPWrapper() + wrapper.allowHTTP = true + + _, err := wrapper.Send(context.Background(), HTTPWrapperRequest{ + URL: server.URL, + Body: []byte(`{"message":"hello"}`), + }) + if err == nil || !strings.Contains(err.Error(), "response payload exceeds") { + t.Fatalf("expected capped response body error, got: %v", err) + } +} + +func TestSanitizeOutboundHeadersAllowlist(t *testing.T) { + headers := sanitizeOutboundHeaders(map[string]string{ + "Content-Type": "application/json", + "User-Agent": "Charon", + "X-Request-ID": "abc", + "X-Gotify-Key": "secret", + "Authorization": "Bearer token", + "Cookie": "sid=1", + }) + + if len(headers) != 4 { + t.Fatalf("expected 4 allowed headers, got %d", len(headers)) + } + if _, ok := headers["Authorization"]; ok { + t.Fatalf("authorization header must be stripped") + } + if _, ok := headers["Cookie"]; ok { + t.Fatalf("cookie header must be stripped") + } +} diff --git a/backend/internal/notifications/router.go b/backend/internal/notifications/router.go index f77f7d94..5c19aa02 100644 --- a/backend/internal/notifications/router.go +++ b/backend/internal/notifications/router.go @@ -22,6 +22,8 @@ func (r *Router) ShouldUseNotify(providerType, providerEngine string, flags map[ return flags[FlagDiscordServiceEnabled] case "gotify": return flags[FlagGotifyServiceEnabled] + case "webhook": + return flags[FlagWebhookServiceEnabled] default: return false } diff --git a/backend/internal/notifications/router_test.go b/backend/internal/notifications/router_test.go index e54b4581..a8ea1a44 100644 --- a/backend/internal/notifications/router_test.go +++ b/backend/internal/notifications/router_test.go @@ -90,3 +90,21 @@ func TestRouter_ShouldUseNotify_GotifyServiceFlag(t *testing.T) { t.Fatalf("expected notify routing disabled for gotify when FlagGotifyServiceEnabled is false") } } + +func TestRouter_ShouldUseNotify_WebhookServiceFlag(t *testing.T) { + router := NewRouter() + + flags := map[string]bool{ + FlagNotifyEngineEnabled: true, + FlagWebhookServiceEnabled: true, + } + + if !router.ShouldUseNotify("webhook", EngineNotifyV1, flags) { + t.Fatalf("expected notify routing enabled for webhook when FlagWebhookServiceEnabled is true") + } + + flags[FlagWebhookServiceEnabled] = false + if router.ShouldUseNotify("webhook", EngineNotifyV1, flags) { + t.Fatalf("expected notify routing disabled for webhook when FlagWebhookServiceEnabled is false") + } +} diff --git a/backend/internal/services/notification_service.go b/backend/internal/services/notification_service.go index d4a824ad..99f7863f 100644 --- a/backend/internal/services/notification_service.go +++ b/backend/internal/services/notification_service.go @@ -16,6 +16,7 @@ import ( "github.com/Wikid82/charon/backend/internal/logger" "github.com/Wikid82/charon/backend/internal/network" + "github.com/Wikid82/charon/backend/internal/notifications" "github.com/Wikid82/charon/backend/internal/security" "github.com/Wikid82/charon/backend/internal/trace" @@ -25,11 +26,15 @@ import ( ) type NotificationService struct { - DB *gorm.DB + DB *gorm.DB + httpWrapper *notifications.HTTPWrapper } func NewNotificationService(db *gorm.DB) *NotificationService { - return &NotificationService{DB: db} + return &NotificationService{ + DB: db, + httpWrapper: notifications.NewNotifyHTTPWrapper(), + } } var discordWebhookRegex = regexp.MustCompile(`^https://discord(?:app)?\.com/api/webhooks/(\d+)/([a-zA-Z0-9_-]+)`) @@ -98,15 +103,46 @@ 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", "slack", "gotify", "generic": + case "webhook", "discord", "gotify", "slack", "generic": return true - case "telegram": - return false // Telegram uses URL parameters default: return false } } +func isSupportedNotificationProviderType(providerType string) bool { + switch strings.ToLower(strings.TrimSpace(providerType)) { + case "discord", "gotify", "webhook": + return true + default: + return false + } +} + +func (s *NotificationService) isDispatchEnabled(providerType string) bool { + switch strings.ToLower(strings.TrimSpace(providerType)) { + case "discord": + return true + case "gotify": + return s.getFeatureFlagValue(notifications.FlagGotifyServiceEnabled, false) + case "webhook": + return s.getFeatureFlagValue(notifications.FlagWebhookServiceEnabled, false) + default: + return false + } +} + +func (s *NotificationService) getFeatureFlagValue(key string, fallback bool) bool { + var setting models.Setting + err := s.DB.Where("key = ?", key).First(&setting).Error + if err != nil { + return fallback + } + + v := strings.ToLower(strings.TrimSpace(setting.Value)) + return v == "1" || v == "true" || v == "yes" +} + // Internal Notifications (DB) func (s *NotificationService) Create(nType models.NotificationType, title, message string) (*models.Notification, error) { @@ -188,11 +224,10 @@ func (s *NotificationService) SendExternal(ctx context.Context, eventType, title if !shouldSend { continue } - // Non-dispatch policy for deprecated providers - if provider.Type != "discord" { + if !s.isDispatchEnabled(provider.Type) { logger.Log().WithField("provider", util.SanitizeForLog(provider.Name)). WithField("type", provider.Type). - Warn("Skipping dispatch to deprecated non-discord provider") + Warn("Skipping dispatch because provider type is disabled for notify dispatch") continue } go func(p models.NotificationProvider) { @@ -253,31 +288,15 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti return fmt.Errorf("template size exceeds maximum limit of %d bytes", maxTemplateSize) } - // Validate webhook URL using the security package's SSRF-safe validator. - // ValidateExternalURL performs comprehensive validation including: - // - URL format and scheme validation (http/https only) - // - DNS resolution and IP blocking for private/reserved ranges - // - Protection against cloud metadata endpoints (169.254.169.254) - // Using the security package's function helps CodeQL recognize the sanitization. - // - // Additionally, we apply `isValidRedirectURL` as a barrier-guard style predicate. - // CodeQL recognizes this pattern as a sanitizer for untrusted URL values, while - // the real SSRF protection remains `security.ValidateExternalURL`. - if err := validateDiscordProviderURLFunc(p.Type, p.URL); err != nil { - return err - } + providerType := strings.ToLower(strings.TrimSpace(p.Type)) + if providerType == "discord" { + if err := validateDiscordProviderURLFunc(p.Type, p.URL); err != nil { + return err + } - webhookURL := p.URL - - if !isValidRedirectURL(webhookURL) { - return fmt.Errorf("invalid webhook url") - } - validatedURLStr, err := security.ValidateExternalURL(webhookURL, - security.WithAllowHTTP(), // Allow both http and https for webhooks - security.WithAllowLocalhost(), // Allow localhost for testing - ) - if err != nil { - return fmt.Errorf("invalid webhook url: %w", err) + if !isValidRedirectURL(p.URL) { + return fmt.Errorf("invalid webhook url") + } } // Parse template and add helper funcs @@ -348,11 +367,43 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti } } - // Send Request with a safe client (SSRF protection, timeout, no auto-redirect) - // Using network.NewSafeHTTPClient() for defense-in-depth against SSRF attacks. + if providerType == "gotify" || providerType == "webhook" { + headers := map[string]string{ + "Content-Type": "application/json", + "User-Agent": "Charon-Notify/1.0", + } + if rid := ctx.Value(trace.RequestIDKey); rid != nil { + if ridStr, ok := rid.(string); ok { + headers["X-Request-ID"] = ridStr + } + } + if providerType == "gotify" { + if strings.TrimSpace(p.Token) != "" { + headers["X-Gotify-Key"] = strings.TrimSpace(p.Token) + } + } + + if _, err := s.httpWrapper.Send(ctx, notifications.HTTPWrapperRequest{ + URL: p.URL, + Headers: headers, + Body: body.Bytes(), + }); err != nil { + return fmt.Errorf("failed to send webhook: %w", err) + } + return nil + } + + validatedURLStr, err := security.ValidateExternalURL(p.URL, + security.WithAllowHTTP(), + security.WithAllowLocalhost(), + ) + if err != nil { + return fmt.Errorf("invalid webhook url: %w", err) + } + client := network.NewSafeHTTPClient( network.WithTimeout(10*time.Second), - network.WithAllowLocalhost(), // Allow localhost for testing + network.WithAllowLocalhost(), ) req, err := http.NewRequestWithContext(ctx, "POST", validatedURLStr, &body) @@ -360,20 +411,12 @@ func (s *NotificationService) sendJSONPayload(ctx context.Context, p models.Noti return fmt.Errorf("failed to create webhook request: %w", err) } req.Header.Set("Content-Type", "application/json") - // Propagate request id header if present in context if rid := ctx.Value(trace.RequestIDKey); rid != nil { if ridStr, ok := rid.(string); ok { req.Header.Set("X-Request-ID", ridStr) } } - // Safe: URL validated by security.ValidateExternalURL() which validates URL - // format/scheme and blocks private/reserved destinations through DNS+dial-time checks. - // Safe: URL validated by security.ValidateExternalURL() which: - // 1. Validates URL format and scheme (HTTPS required in production) - // 2. Resolves DNS and blocks private/reserved IPs (RFC 1918, loopback, link-local) - // 3. Uses ssrfSafeDialer for connection-time IP revalidation (TOCTOU protection) - // 4. No redirect following allowed - // See: internal/security/url_validator.go + resp, err := webhookDoRequestFunc(client, req) if err != nil { return fmt.Errorf("failed to send webhook: %w", err) @@ -411,17 +454,21 @@ func isValidRedirectURL(rawURL string) bool { } func (s *NotificationService) TestProvider(provider models.NotificationProvider) error { - // Discord-only enforcement for this rollout - if provider.Type != "discord" { + providerType := strings.ToLower(strings.TrimSpace(provider.Type)) + if !isSupportedNotificationProviderType(providerType) { return fmt.Errorf("only discord provider type is supported in this release") } - if err := validateDiscordProviderURLFunc(provider.Type, provider.URL); err != nil { + if !s.isDispatchEnabled(providerType) { + return fmt.Errorf("only discord provider type is supported in this release") + } + + if err := validateDiscordProviderURLFunc(providerType, provider.URL); err != nil { return err } - if !supportsJSONTemplates(provider.Type) { - return legacyFallbackInvocationError(provider.Type) + if !supportsJSONTemplates(providerType) { + return legacyFallbackInvocationError(providerType) } data := map[string]any{ @@ -523,15 +570,19 @@ func (s *NotificationService) ListProviders() ([]models.NotificationProvider, er } func (s *NotificationService) CreateProvider(provider *models.NotificationProvider) error { - // Discord-only enforcement for this rollout - if provider.Type != "discord" { - return fmt.Errorf("only discord provider type is supported in this release") + provider.Type = strings.ToLower(strings.TrimSpace(provider.Type)) + if !isSupportedNotificationProviderType(provider.Type) { + return fmt.Errorf("unsupported provider type") } if err := validateDiscordProviderURLFunc(provider.Type, provider.URL); err != nil { return err } + if provider.Type != "gotify" { + provider.Token = "" + } + // Validate custom template before creating if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" { // Provide a minimal preview payload @@ -550,25 +601,28 @@ func (s *NotificationService) UpdateProvider(provider *models.NotificationProvid return err } - // Block type mutation for non-Discord providers - if existing.Type != "discord" && provider.Type != existing.Type { - return fmt.Errorf("cannot change provider type for deprecated non-discord providers") + // Block type mutation for existing providers to avoid cross-provider token/schema confusion + if strings.TrimSpace(provider.Type) != "" && provider.Type != existing.Type { + return fmt.Errorf("cannot change provider type for existing providers") } + provider.Type = existing.Type - // Block enable mutation for non-Discord providers - if existing.Type != "discord" && provider.Enabled && !existing.Enabled { - return fmt.Errorf("cannot enable deprecated non-discord providers") - } - - // Discord-only enforcement for type changes - if provider.Type != "discord" { - return fmt.Errorf("only discord provider type is supported in this release") + if !isSupportedNotificationProviderType(provider.Type) { + return fmt.Errorf("unsupported provider type") } if err := validateDiscordProviderURLFunc(provider.Type, provider.URL); err != nil { return err } + if provider.Type == "gotify" { + if strings.TrimSpace(provider.Token) == "" { + provider.Token = existing.Token + } + } else { + provider.Token = "" + } + // Validate custom template before saving if strings.ToLower(strings.TrimSpace(provider.Template)) == "custom" && strings.TrimSpace(provider.Config) != "" { payload := map[string]any{"Title": "Preview", "Message": "Preview", "Time": time.Now().Format(time.RFC3339), "EventType": "preview"} @@ -581,6 +635,7 @@ func (s *NotificationService) UpdateProvider(provider *models.NotificationProvid "name": provider.Name, "type": provider.Type, "url": provider.URL, + "token": provider.Token, "config": provider.Config, "template": provider.Template, "enabled": provider.Enabled, diff --git a/backend/internal/services/notification_service_discord_only_test.go b/backend/internal/services/notification_service_discord_only_test.go index a5566db1..cf78f9c3 100644 --- a/backend/internal/services/notification_service_discord_only_test.go +++ b/backend/internal/services/notification_service_discord_only_test.go @@ -12,15 +12,15 @@ import ( "gorm.io/gorm" ) -// TestDiscordOnly_CreateProviderRejectsNonDiscord tests service-level Discord-only enforcement for create. -func TestDiscordOnly_CreateProviderRejectsNonDiscord(t *testing.T) { +// TestDiscordOnly_CreateProviderRejectsUnsupported tests service-level provider allowlist for create. +func TestDiscordOnly_CreateProviderRejectsUnsupported(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) service := NewNotificationService(db) - testCases := []string{"webhook", "slack", "gotify", "telegram", "generic"} + testCases := []string{"slack", "telegram", "generic", "email"} for _, providerType := range testCases { t.Run(providerType, func(t *testing.T) { @@ -31,8 +31,8 @@ func TestDiscordOnly_CreateProviderRejectsNonDiscord(t *testing.T) { } err := service.CreateProvider(provider) - assert.Error(t, err, "Should reject non-Discord provider") - assert.Contains(t, err.Error(), "only discord provider type is supported") + assert.Error(t, err, "Should reject unsupported provider") + assert.Contains(t, err.Error(), "unsupported provider type") }) } } @@ -60,76 +60,81 @@ func TestDiscordOnly_CreateProviderAcceptsDiscord(t *testing.T) { assert.Equal(t, "discord", created.Type) } -// TestDiscordOnly_UpdateProviderRejectsNonDiscord tests service-level Discord-only enforcement for update. -func TestDiscordOnly_UpdateProviderRejectsNonDiscord(t *testing.T) { +func TestDiscordOnly_CreateProviderAcceptsWebhook(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) - // Create a deprecated webhook provider - deprecatedProvider := models.NotificationProvider{ - ID: "test-id", - Name: "Test Webhook", - Type: "webhook", - URL: "https://example.com/webhook", - MigrationState: "deprecated", - } - require.NoError(t, db.Create(&deprecatedProvider).Error) - service := NewNotificationService(db) - // Try to update with webhook type provider := &models.NotificationProvider{ - ID: "test-id", - Name: "Updated", + Name: "Test Webhook", Type: "webhook", URL: "https://example.com/webhook", } - err = service.UpdateProvider(provider) - assert.Error(t, err, "Should reject non-Discord provider update") - assert.Contains(t, err.Error(), "only discord provider type is supported") + err = service.CreateProvider(provider) + assert.NoError(t, err, "Should accept webhook provider") } -// TestDiscordOnly_UpdateProviderRejectsTypeMutation tests that service blocks type mutation for deprecated providers. +func TestDiscordOnly_CreateProviderAcceptsGotifyWithOrWithoutToken(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) + + service := NewNotificationService(db) + + provider := &models.NotificationProvider{ + Name: "Test Gotify", + Type: "gotify", + URL: "https://gotify.example.com/message", + } + + err = service.CreateProvider(provider) + assert.NoError(t, err) + + provider.ID = "" + provider.Token = "secret" + err = service.CreateProvider(provider) + assert.NoError(t, err) +} + +// TestDiscordOnly_UpdateProviderRejectsTypeMutation tests immutable provider type on update. func TestDiscordOnly_UpdateProviderRejectsTypeMutation(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) - // Create a deprecated webhook provider - deprecatedProvider := models.NotificationProvider{ + provider := models.NotificationProvider{ ID: "test-id", Name: "Test Webhook", Type: "webhook", URL: "https://example.com/webhook", MigrationState: "deprecated", } - require.NoError(t, db.Create(&deprecatedProvider).Error) + require.NoError(t, db.Create(&provider).Error) service := NewNotificationService(db) - // Try to change type to discord - provider := &models.NotificationProvider{ + updatedProvider := &models.NotificationProvider{ ID: "test-id", - Name: "Test Webhook", + Name: "Updated", Type: "discord", URL: "https://discord.com/api/webhooks/123/abc", } - err = service.UpdateProvider(provider) + err = service.UpdateProvider(updatedProvider) assert.Error(t, err, "Should reject type mutation") assert.Contains(t, err.Error(), "cannot change provider type") } -// TestDiscordOnly_UpdateProviderRejectsEnable tests that service blocks enabling deprecated providers. -func TestDiscordOnly_UpdateProviderRejectsEnable(t *testing.T) { +// TestDiscordOnly_UpdateProviderAllowsWebhookUpdates tests supported provider updates. +func TestDiscordOnly_UpdateProviderAllowsWebhookUpdates(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) - // Create a deprecated webhook provider (disabled) - deprecatedProvider := models.NotificationProvider{ + provider := models.NotificationProvider{ ID: "test-id", Name: "Test Webhook", Type: "webhook", @@ -137,12 +142,11 @@ func TestDiscordOnly_UpdateProviderRejectsEnable(t *testing.T) { Enabled: false, MigrationState: "deprecated", } - require.NoError(t, db.Create(&deprecatedProvider).Error) + require.NoError(t, db.Create(&provider).Error) service := NewNotificationService(db) - // Try to enable - provider := &models.NotificationProvider{ + updatedProvider := &models.NotificationProvider{ ID: "test-id", Name: "Test Webhook", Type: "webhook", @@ -150,16 +154,15 @@ func TestDiscordOnly_UpdateProviderRejectsEnable(t *testing.T) { Enabled: true, } - err = service.UpdateProvider(provider) - assert.Error(t, err, "Should reject enabling deprecated provider") - assert.Contains(t, err.Error(), "cannot enable deprecated") + err = service.UpdateProvider(updatedProvider) + assert.NoError(t, err) } -// TestDiscordOnly_TestProviderRejectsNonDiscord tests that TestProvider enforces Discord-only. -func TestDiscordOnly_TestProviderRejectsNonDiscord(t *testing.T) { +// TestDiscordOnly_TestProviderRejectsDisabledProviderTypes tests feature-flag gate for gotify/webhook dispatch. +func TestDiscordOnly_TestProviderRejectsDisabledProviderTypes(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) require.NoError(t, err) - require.NoError(t, db.AutoMigrate(&models.NotificationProvider{})) + require.NoError(t, db.AutoMigrate(&models.NotificationProvider{}, &models.Setting{})) service := NewNotificationService(db) @@ -170,7 +173,7 @@ func TestDiscordOnly_TestProviderRejectsNonDiscord(t *testing.T) { } err = service.TestProvider(provider) - assert.Error(t, err, "Should reject non-Discord provider test") + assert.Error(t, err) assert.Contains(t, err.Error(), "only discord provider type is supported") } diff --git a/backend/internal/services/notification_service_json_test.go b/backend/internal/services/notification_service_json_test.go index 2b6e65e6..261895e3 100644 --- a/backend/internal/services/notification_service_json_test.go +++ b/backend/internal/services/notification_service_json_test.go @@ -231,6 +231,7 @@ func TestSendJSONPayload_Gotify(t *testing.T) { provider := models.NotificationProvider{ Type: "gotify", URL: server.URL, + Token: "test-token", Template: "custom", Config: `{"message": {{toJSON .Message}}, "title": {{toJSON .Title}}}`, } @@ -262,7 +263,7 @@ func TestSendJSONPayload_TemplateTimeout(t *testing.T) { Type: "discord", URL: "http://10.0.0.1:9999", Template: "custom", - Config: `{"data": {{toJSON .}}}`, + Config: `{"content": {{toJSON .Message}}, "data": {{toJSON .}}}`, } // Create data that will be processed diff --git a/backend/internal/services/notification_service_test.go b/backend/internal/services/notification_service_test.go index 84576104..a5fcf5d7 100644 --- a/backend/internal/services/notification_service_test.go +++ b/backend/internal/services/notification_service_test.go @@ -663,7 +663,7 @@ func TestSSRF_WebhookIntegration(t *testing.T) { data := map[string]any{"Title": "Test", "Message": "Test Message"} err := svc.sendJSONPayload(context.Background(), provider, data) assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid webhook url") + assert.Contains(t, err.Error(), "destination URL validation failed") }) t.Run("blocks cloud metadata endpoint", func(t *testing.T) { @@ -674,7 +674,7 @@ func TestSSRF_WebhookIntegration(t *testing.T) { data := map[string]any{"Title": "Test", "Message": "Test Message"} err := svc.sendJSONPayload(context.Background(), provider, data) assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid webhook url") + assert.Contains(t, err.Error(), "destination URL validation failed") }) t.Run("allows localhost for testing", func(t *testing.T) { diff --git a/docs/features.md b/docs/features.md index c2b9bffa..056b448c 100644 --- a/docs/features.md +++ b/docs/features.md @@ -237,7 +237,7 @@ Watch requests flow through your proxy in real-time. Filter by domain, status co ### 🔔 Notifications -Get alerted when it matters. Charon currently sends notifications through Discord webhooks using the Notify engine only. No legacy fallback path is used at runtime. Additional providers will roll out later in staged updates. +Get alerted when it matters. Charon notifications now run through the Notify HTTP wrapper with support for Discord, Gotify, and Custom Webhook providers. Payload-focused test coverage is included to help catch formatting and delivery regressions before release. → [Learn More](features/notifications.md) diff --git a/docs/features/notifications.md b/docs/features/notifications.md index 8aa5aee8..e9e06bb4 100644 --- a/docs/features/notifications.md +++ b/docs/features/notifications.md @@ -11,11 +11,13 @@ Notifications can be triggered by various events: - **Security Events**: WAF blocks, CrowdSec alerts, ACL violations - **System Events**: Configuration changes, backup completions -## Supported Service (Current Rollout) +## Supported Services | Service | JSON Templates | Native API | Rich Formatting | |---------|----------------|------------|-----------------| | **Discord** | ✅ Yes | ✅ Webhooks | ✅ Embeds | +| **Gotify** | ✅ Yes | ✅ HTTP API | ✅ Priority + Extras | +| **Custom Webhook** | ✅ Yes | ✅ HTTP API | ✅ Template-Controlled | Additional providers are planned for later staged releases. @@ -41,7 +43,7 @@ JSON templates give you complete control over notification formatting, allowing ### JSON Template Support -For the currently supported service (Discord), you can choose from three template options: +For current services (Discord, Gotify, and Custom Webhook), you can choose from three template options. #### 1. Minimal Template (Default) @@ -157,9 +159,9 @@ Discord supports rich embeds with colors, fields, and timestamps. ## Planned Provider Expansion -Additional providers (for example Slack, Gotify, Telegram, and generic webhooks) -are planned for later staged releases. This page will be expanded as each -provider is validated and released. +Additional providers (for example Slack and Telegram) are planned for later +staged releases. This page will be expanded as each provider is validated and +released. ## Template Variables @@ -228,9 +230,13 @@ Template: detailed (or custom) 4. Test the notification 5. Save changes -If you previously used non-Discord provider types, keep those entries as -historical records only. They are not active runtime dispatch paths in the -current rollout. +Gotify and Custom Webhook providers are active runtime paths in the current +rollout and can be used in production. + +## Validation Coverage + +The current rollout includes payload-focused notification tests to catch +formatting and delivery regressions across provider types before release. ### Testing Your Template diff --git a/docs/issues/manual_test_notify_wrapper_gotify_webhook_regression_tracking.md b/docs/issues/manual_test_notify_wrapper_gotify_webhook_regression_tracking.md new file mode 100644 index 00000000..63b7f30e --- /dev/null +++ b/docs/issues/manual_test_notify_wrapper_gotify_webhook_regression_tracking.md @@ -0,0 +1,69 @@ +--- +title: Manual Test Tracking Plan - Notify Wrapper (Gotify + Custom Webhook) +status: Open +priority: High +assignee: QA +labels: testing, notifications, backend, frontend, security +--- + +# Test Goal +Track manual verification for bugs and regressions after the Notify migration that added HTTP wrapper delivery for Gotify and Custom Webhook providers. + +# Scope +- Provider creation and editing for Gotify and Custom Webhook +- Send Test and Preview behavior +- Payload rendering and delivery behavior +- Secret handling and error-message safety +- Existing Discord behavior regression checks + +# Preconditions +- Charon is running and reachable in a browser. +- Tester can open Settings → Notifications. +- Tester has reachable endpoints for: + - One Gotify instance + - One custom webhook receiver + +## 1) Smoke Path - Provider CRUD +- [ ] Create a Gotify provider with valid URL and token, save successfully. +- [ ] Create a Custom Webhook provider with valid URL, save successfully. +- [ ] Refresh and confirm both providers persist with expected non-secret fields. +- [ ] Edit each provider, save changes, refresh, and confirm updates persist. + +## 2) Smoke Path - Test and Preview +- [ ] Run Send Test for Gotify provider and confirm successful delivery. +- [ ] Run Send Test for Custom Webhook provider and confirm successful delivery. +- [ ] Run Preview for both providers and confirm payload is rendered as expected. +- [ ] Confirm Discord provider test/preview still works. + +## 3) Payload Regression Checks +- [ ] Validate minimal payload template sends correctly. +- [ ] Validate detailed payload template sends correctly. +- [ ] Validate custom payload template sends correctly. +- [ ] Verify special characters and multi-line content render correctly. +- [ ] Verify payload output remains stable after provider edit + save. + +## 4) Secret and Error Safety Checks +- [ ] Confirm Gotify token is never shown in list/readback UI. +- [ ] Confirm Gotify token is not exposed in test/preview responses shown in UI. +- [ ] Trigger a failed test (invalid endpoint) and confirm error text is clear but does not expose secrets. +- [ ] Confirm failed requests do not leak sensitive values in user-visible error content. + +## 5) Failure-Mode and Recovery Checks +- [ ] Test with unreachable endpoint and confirm failure is reported clearly. +- [ ] Test with malformed URL and confirm validation blocks save. +- [ ] Test with slow endpoint and confirm UI remains responsive and recoverable. +- [ ] Fix endpoint values and confirm retry succeeds without recreating provider. + +## 6) Cross-Provider Regression Checks +- [ ] Confirm Gotify changes do not alter Custom Webhook settings. +- [ ] Confirm Custom Webhook changes do not alter Discord settings. +- [ ] Confirm deleting one provider does not corrupt remaining providers. + +## Pass/Fail Criteria +- [ ] PASS when all smoke checks pass, payload output is correct, secrets stay hidden, and no cross-provider regressions are found. +- [ ] FAIL when delivery breaks, payload rendering regresses, secrets are exposed, or provider changes affect unrelated providers. + +## Defect Tracking Notes +- [ ] For each defect, record provider type, action, expected result, actual result, and severity. +- [ ] Attach screenshot/video where useful. +- [ ] Mark whether defect is release-blocking. diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index a7527a07..4d2aa276 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,857 +1,466 @@ --- -post_title: "Current Spec: Caddy 2.11.1 Compatibility, Security, and UX Impact Plan" +post_title: "Current Spec: Notify HTTP Wrapper Rollout for Gotify and Custom Webhook" categories: - actions - - security - backend - frontend - - infrastructure + - testing + - security tags: - - caddy - - xcaddy - - dependency-management - - vulnerability-management - - release-planning -summary: "Comprehensive, phased plan to evaluate and safely adopt Caddy v2.11.1 in Charon, covering plugin compatibility, CVE impact, xcaddy patch retirement decisions, UI/UX exposure opportunities, and PR slicing strategy with strict validation gates." + - notify-migration + - gotify + - webhook + - playwright + - patch-coverage +summary: "Single authoritative plan for Notify HTTP wrapper rollout for Gotify and Custom Webhook, including token secrecy contract, SSRF hardening, transport safety, expanded test matrix, and safe PR slicing." post_date: 2026-02-23 --- -## Active Plan: Caddy 2.11.1 Deep Compatibility and Security Rollout +## Active Plan: Notify Migration — HTTP Wrapper for Gotify and Custom Webhook Date: 2026-02-23 -Status: Active and authoritative -Scope Type: Architecture/security/dependency research and implementation planning -Authority: This is the only active authoritative plan section in this file. - -## Focused Plan: GitHub Actions `setup-go` Cache Warning (`go.sum` path) - -Date: 2026-02-23 -Status: Planned -Scope: Warning-only fix for GitHub Actions cache restore message: -`Restore cache failed: Dependencies file is not found in -/home/runner/work/Charon/Charon. Supported file pattern: go.sum`. - -### Introduction - -This focused section addresses a CI warning caused by `actions/setup-go` cache -configuration assuming `go.sum` at repository root. Charon stores Go module -dependencies in `backend/go.sum`. - -### Research Findings - -Verified workflow inventory (`.github/workflows/**`): - -- All workflows using `actions/setup-go` were identified. -- Five workflows already set `cache-dependency-path: backend/go.sum`: - - `.github/workflows/codecov-upload.yml` - - `.github/workflows/quality-checks.yml` - - `.github/workflows/codeql.yml` - - `.github/workflows/benchmark.yml` - - `.github/workflows/e2e-tests-split.yml` -- Two workflows use `actions/setup-go` without cache dependency path and are - the warning source: - - `.github/workflows/caddy-compat.yml` - - `.github/workflows/release-goreleaser.yml` -- Repository check confirms only one `go.sum` exists: - - `backend/go.sum` - -### Technical Specification (Minimal Fix) - -Apply a warning-only cache path correction in both affected workflow steps: - -1. `.github/workflows/caddy-compat.yml` - - In `Set up Go` step, add: - - `cache-dependency-path: backend/go.sum` - -2. `.github/workflows/release-goreleaser.yml` - - In `Set up Go` step, add: - - `cache-dependency-path: backend/go.sum` - -No other workflow behavior, triggers, permissions, or build/test logic will be -changed. - -### Implementation Plan - -#### Phase 1 — Workflow patch - -- Update only the two targeted workflow files listed above. - -#### Phase 2 — Validation - -- Run workflow YAML validation/lint checks already used by repository CI. -- Confirm no cache restore warning appears in subsequent runs of: - - `Caddy Compatibility Gate` - - `Release (GoReleaser)` - -#### Phase 3 — Closeout - -- Mark warning remediated once both workflows execute without the missing - `go.sum` cache warning. - -### Acceptance Criteria - -1. Both targeted workflows include `cache-dependency-path: backend/go.sum` in - their `actions/setup-go` step. -2. No unrelated workflow files are modified. -3. No behavior changes beyond warning elimination. -4. CI logs for affected workflows no longer show the missing dependencies-file - warning. - -### PR Slicing Strategy - -- Decision: Single PR. -- Rationale: Two-line, warning-only correction in two workflow files with no - cross-domain behavior impact. -- Slice: - - `PR-1`: Add `cache-dependency-path` to the two `setup-go` steps and verify - workflow run logs. -- Rollback: - - Revert only these two workflow edits if unexpected cache behavior appears. - -## Focused Remediation Plan Addendum: 3 Failing Playwright Tests - -Date: 2026-02-23 -Scope: Only the 3 failures reported in `docs/reports/qa_report.md`: -- `tests/core/proxy-hosts.spec.ts` — `should open edit modal with existing values` -- `tests/core/proxy-hosts.spec.ts` — `should update forward host and port` -- `tests/settings/smtp-settings.spec.ts` — `should update existing SMTP configuration` - -### Introduction - -This addendum defines a minimal, deterministic remediation for the three reported flaky/timeout E2E failures. The objective is to stabilize test synchronization and preconditions while preserving existing assertions and behavior intent. - -### Research Findings - -#### 1) `tests/core/proxy-hosts.spec.ts` (2 timeouts) - -Observed test pattern: -- Uses broad selector `page.getByRole('button', { name: /edit/i }).first()`. -- Uses conditional execution (`if (editCount > 0)`) with no explicit precondition that at least one editable row exists. -- Waits for modal after clicking the first matched "Edit" button. - -Likely root causes: -- Broad role/name selector can resolve to non-row or non-visible edit controls first, causing click auto-wait timeout. -- Test data state is non-deterministic (no guaranteed editable proxy host before the update tests). -- In-file parallel execution (`fullyParallel: true` globally) increases race potential for shared host list mutations. - -#### 2) `tests/settings/smtp-settings.spec.ts` (waitForResponse timeout) - -Observed test pattern: -- Uses `clickAndWaitForResponse(page, saveButton, /\/api\/v1\/settings\/smtp/)`, which internally waits for response status `200` by default. -- Test updates only host field, relying on pre-existing validity of other required fields. - -Likely root causes: -- If backend returns non-`200` (e.g., `400` validation), helper waits indefinitely for `200` and times out instead of failing fast. -- The test assumes existing SMTP state is valid; this is brittle under parallel execution and prior test mutations. - -### Technical Specifications (Exact Test Changes) - -#### A) `tests/core/proxy-hosts.spec.ts` - -1. In `test.describe('Update Proxy Host', ...)`, add serial mode: -- Add `test.describe.configure({ mode: 'serial' })` at the top of that describe block. - -2. Add a local helper in this file for deterministic precondition and row-scoped edit action: -- Helper name: `ensureEditableProxyHost(page, testData)` -- Behavior: - - Check `tbody tr` count. - - If count is `0`, create one host via `testData.createProxyHost({ domain: ..., forwardHost: ..., forwardPort: ... })`. - - Reload `/proxy-hosts` and wait for content readiness using existing wait helpers. - -3. Replace broad edit-button lookup in both failing tests with row-scoped visible locator: -- Replace: - - `page.getByRole('button', { name: /edit/i }).first()` -- With: - - `const firstRow = page.locator('tbody tr').first()` - - `const editButton = firstRow.getByRole('button', { name: /edit proxy host|edit/i }).first()` - - `await expect(editButton).toBeVisible()` - - `await editButton.click()` - -4. Remove silent pass-through for missing rows in these two tests: -- Replace `if (editCount > 0) { ... }` branching with deterministic precondition call and explicit assertion that dialog appears. - -Affected tests: -- `should open edit modal with existing values` -- `should update forward host and port` - -Preserved assertions: -- Edit modal opens. -- Existing values are present. -- Forward host/port fields accept and retain edited values before cancel. - -#### B) `tests/settings/smtp-settings.spec.ts` - -1. In `test.describe('CRUD Operations', ...)`, add serial mode: -- Add `test.describe.configure({ mode: 'serial' })` to avoid concurrent mutation of shared SMTP configuration. - -2. Strengthen required-field preconditions in failing test before save: -- In `should update existing SMTP configuration`, explicitly set: - - `#smtp-host` to `updated-smtp.test.local` - - `#smtp-port` to `587` - - `#smtp-from` to `noreply@test.local` - -3. Replace status-constrained response wait that can timeout on non-200: -- Replace `clickAndWaitForResponse(...)` call with `Promise.all([page.waitForResponse(...) , saveButton.click()])` matching URL + `POST` method (not status). -- Immediately assert returned status is `200` and then keep success-toast assertion. - -4. Keep existing persistence verification and cleanup step: -- Reload and assert host persisted. -- Restore original host value after assertion. - -Preserved assertions: -- Save request succeeds. -- Success feedback shown. -- Updated value persists after reload. -- Original value restoration still performed. - -### Implementation Plan - -#### Phase 1 — Targeted test edits -- Update only: - - `tests/core/proxy-hosts.spec.ts` - - `tests/settings/smtp-settings.spec.ts` - -#### Phase 2 — Focused verification -- Run only the 3 failing cases first (grep-targeted). -- Then run both files fully on Firefox to validate no local regressions. - -#### Phase 3 — Gate confirmation -- Re-run the previously failing targeted suite: - - `tests/core` - - `tests/settings/smtp-settings.spec.ts` - -### Acceptance Criteria - -1. `should open edit modal with existing values` passes without timeout. -2. `should update forward host and port` passes without timeout. -3. `should update existing SMTP configuration` passes without `waitForResponse` timeout. -4. No assertion scope is broadened; test intent remains unchanged. -5. No non-target files are modified. - -### PR Slicing Strategy - -- Decision: **Single PR**. -- Rationale: 3 deterministic test-only fixes, same domain (Playwright stabilization), low blast radius. -- Slice: - - `PR-1`: Update the two spec files above + rerun targeted Playwright validations. -- Rollback: - - Revert only spec-file changes if unintended side effects appear. +Status: Ready for Supervisor Review +Scope Type: Backend + Frontend + E2E + Coverage/CI alignment +Authority: This is the only active authoritative plan in this file. ## Introduction -Charon’s control plane and data plane rely on Caddy as a core runtime backbone. -Because Caddy is embedded and rebuilt via `xcaddy`, upgrading from -`2.11.0-beta.2` to `2.11.1` is not a routine version bump: it impacts -runtime behavior, plugin compatibility, vulnerability posture, and potential UX -surface area. +This plan defines the Notify migration increment that enables HTTP-wrapper +routing for `gotify` and `webhook` providers while preserving current Discord +behavior. -This plan defines a low-risk, high-observability rollout strategy that answers: +Primary goals: -1. Which Caddy 2.11.x features should be exposed in Charon UI/API? -2. Which existing Charon workarounds became redundant upstream? -3. Which `xcaddy` dependency patches remain necessary vs removable? -4. Which known vulnerabilities are fixed now and which should remain on watch? +1. Enable a unified wrapper path for outbound provider dispatch. +2. Make Gotify token handling write-only and non-leaking by contract. +3. Add explicit SSRF/redirect/rebinding protections. +4. Add strict error leakage controls for preview/test paths. +5. Add wrapper transport guardrails and expanded validation tests. ## Research Findings -### External release and security findings +### Current architecture and constraints -1. Official release statement confirms `v2.11.1` has no runtime code delta from - `v2.11.0` except CI/release process correction. Practical implication: - compatibility/security validation should target **2.11.x** behavior, not - 2.11.1-specific runtime changes. -2. Caddy release lists six security patches (mapped to GitHub advisories): - - `CVE-2026-27590` → `GHSA-5r3v-vc8m-m96g` (FastCGI split_path confusion) - - `CVE-2026-27589` → `GHSA-879p-475x-rqh2` (admin API cross-origin no-cors) - - `CVE-2026-27588` → `GHSA-x76f-jf84-rqj8` (host matcher case bypass) - - `CVE-2026-27587` → `GHSA-g7pc-pc7g-h8jh` (path matcher escaped-case bypass) - - `CVE-2026-27586` → `GHSA-hffm-g8v7-wrv7` (mTLS client-auth fail-open) - - `CVE-2026-27585` → `GHSA-4xrr-hq4w-6vf4` (glob sanitization bypass) -3. NVD/CVE.org entries are currently reserved/not fully enriched. GitHub - advisories are the most actionable source right now. +- Notification provider CRUD/Test/Preview routes already exist: + - `GET/POST/PUT/DELETE /api/v1/notifications/providers` + - `POST /api/v1/notifications/providers/test` + - `POST /api/v1/notifications/providers/preview` +- Current provider handling is Discord-centric in handler/service/frontend. +- Security-event dispatch path exists and is stable. +- Existing notification E2E coverage is mostly Discord-focused. -### Charon architecture and integration findings +### Gaps to close -1. Charon compiles custom Caddy in `Dockerfile` via `xcaddy` and injects: - - `github.com/greenpau/caddy-security` - - `github.com/corazawaf/coraza-caddy/v2` - - `github.com/hslatman/caddy-crowdsec-bouncer@v0.10.0` - - `github.com/zhangjiayin/caddy-geoip2` - - `github.com/mholt/caddy-ratelimit` -2. Charon applies explicit post-generation `go get` patching in `Dockerfile` for: - - `github.com/expr-lang/expr@v1.17.7` - - `github.com/hslatman/ipstore@v0.4.0` - - `github.com/slackhq/nebula@v1.9.7` (with comment indicating temporary pin) -3. Charon CI has explicit dependency inspection gate in - `.github/workflows/docker-build.yml` to verify patched `expr-lang/expr` - versions in built binaries. - -### Plugin compatibility findings (highest risk area) - -Current plugin module declarations (upstream `go.mod`) target older Caddy cores: - -- `greenpau/caddy-security`: `caddy/v2 v2.10.2` -- `hslatman/caddy-crowdsec-bouncer`: `caddy/v2 v2.10.2` -- `corazawaf/coraza-caddy/v2`: `caddy/v2 v2.9.1` -- `zhangjiayin/caddy-geoip2`: `caddy/v2 v2.10.0` -- `mholt/caddy-ratelimit`: `caddy/v2 v2.8.0` - -Implication: compile success against 2.11.1 is plausible but not guaranteed. -The plan must include matrix build/provision tests before merge. - -### Charon UX and config-surface findings - -Current Caddy-related UI/API exposure is narrow: - -- `frontend/src/pages/SystemSettings.tsx` - - state: `caddyAdminAPI`, `sslProvider` - - saves keys: `caddy.admin_api`, `caddy.ssl_provider` -- `frontend/src/pages/ImportCaddy.tsx` and import components: - - Caddyfile parsing/import workflow, not runtime feature toggles -- `frontend/src/api/import.ts`, `frontend/src/api/settings.ts` -- Backend routes and handlers: - - `backend/internal/api/routes/routes.go` - - `backend/internal/api/handlers/settings_handler.go` - - `backend/internal/api/handlers/import_handler.go` - - `backend/internal/caddy/manager.go` - - `backend/internal/caddy/config.go` - - `backend/internal/caddy/types.go` - -No UI controls currently exist for new Caddy 2.11.x capabilities such as -`keepalive_idle`, `keepalive_count`, `trusted_proxies_unix`, -`renewal_window_ratio`, or `0-RTT` behavior. +1. Wrapper enablement for Gotify/Webhook is incomplete end-to-end. +2. Token secrecy contract is not explicit enough across write/read/test flows. +3. SSRF policy needs explicit protocol, redirect, and DNS rebinding rules. +4. Error details need strict sanitization and request correlation. +5. Retry/body/header transport limits need explicit hard requirements. ## Requirements (EARS) -1. WHEN evaluating Caddy `v2.11.1`, THE SYSTEM SHALL validate compatibility - against all currently enabled `xcaddy` plugins before changing production - defaults. -2. WHEN security advisories in Caddy 2.11.x affect modules Charon may use, - THE SYSTEM SHALL document exploitability for Charon’s deployment model and - prioritize remediation accordingly. -3. WHEN an `xcaddy` patch/workaround no longer provides value, - THE SYSTEM SHALL remove it only after reproducible build and runtime - validation gates pass. -4. IF a Caddy 2.11.x feature maps to an existing Charon concept, - THEN THE SYSTEM SHALL prefer extending existing UI/components over adding new - parallel controls. -5. WHEN no direct UX value exists, THE SYSTEM SHALL avoid adding UI for upstream - options and keep behavior backend-managed. -6. WHEN this rollout completes, THE SYSTEM SHALL provide explicit upstream watch - criteria for unresolved/reserved CVEs and plugin dependency lag. +1. WHEN provider type is `gotify` or `webhook`, THE SYSTEM SHALL dispatch + outbound notifications through a shared HTTP wrapper path. +2. WHEN provider type is `discord`, THE SYSTEM SHALL preserve current behavior + with no regression in create/update/test/preview flows. +3. WHEN a Gotify token is provided, THE SYSTEM SHALL accept it only on create + and update write paths. +4. WHEN a Gotify token is accepted, THE SYSTEM SHALL store it securely + server-side. +5. WHEN provider data is returned on read/test/preview responses, THE SYSTEM + SHALL NOT return token values or secret derivatives. +6. WHEN validation errors or logs are emitted, THE SYSTEM SHALL NOT echo token, + auth header, or secret material. +7. WHEN wrapper dispatch is used, THE SYSTEM SHALL enforce HTTPS-only targets by + default. +8. WHEN development override is required for HTTP targets, THE SYSTEM SHALL + allow it only via explicit controlled dev flag, disabled by default. +9. WHEN redirects are encountered, THE SYSTEM SHALL deny redirects by default; + if redirects are enabled, THE SYSTEM SHALL re-validate each hop. +10. WHEN resolving destination addresses, THE SYSTEM SHALL block loopback, + link-local, private, multicast, and IPv6 ULA ranges. +11. WHEN DNS resolution changes during request lifecycle, THE SYSTEM SHALL + perform re-resolution checks and reject rebinding to blocked ranges. +12. WHEN wrapper mode dispatches Gotify/Webhook, THE SYSTEM SHALL use `POST` + only. +13. WHEN preview/test/send errors are returned, THE SYSTEM SHALL return only + sanitized categories and include `request_id`. +14. WHEN preview/test/send errors are returned, THE SYSTEM SHALL NOT include raw + payloads, token values, or raw query-string data. +15. WHEN wrapper transport executes, THE SYSTEM SHALL enforce max request and + response body sizes, strict header allowlist, and bounded retry budget with + exponential backoff and jitter. +16. WHEN retries are evaluated, THE SYSTEM SHALL retry only on network errors, + `429`, and `5xx`; it SHALL NOT retry other `4xx` responses. ## Technical Specifications -### Compatibility scope map (code touch inventory) +### Backend contract -#### Build/packaging +- New module: `backend/internal/notifications/http_wrapper.go` +- Core types: `HTTPWrapperRequest`, `RetryPolicy`, `HTTPWrapperResult`, + `HTTPWrapper` +- Core functions: `NewNotifyHTTPWrapper`, `Send`, `isRetryableStatus`, + `sanitizeOutboundHeaders` -- `Dockerfile` - - `ARG CADDY_VERSION` - - `ARG XCADDY_VERSION` - - `caddy-builder` stage (`xcaddy build`, plugin list, `go get` patches) -- `.github/workflows/docker-build.yml` - - binary dependency checks (`go version -m` extraction/gates) -- `.github/renovate.json` - - regex managers tracking `Dockerfile` patch dependencies +### Gotify secret contract -#### Caddy runtime config generation +- Token accepted only in write path: + - `POST /api/v1/notifications/providers` + - `PUT /api/v1/notifications/providers/:id` +- Token stored securely server-side. +- Token never returned in: + - provider reads/lists + - test responses + - preview responses +- Token never shown in: + - validation details + - logs + - debug payload echoes +- Token transport uses header `X-Gotify-Key` only. +- Query token usage is rejected. -- `backend/internal/caddy/manager.go` - - `NewManager(...)` - - `ApplyConfig(ctx)` -- `backend/internal/caddy/config.go` - - `GenerateConfig(...)` -- `backend/internal/caddy/types.go` - - JSON struct model for Caddy config (`Server`, `TrustedProxies`, etc.) +### SSRF hardening requirements -#### Settings and admin surface +- HTTPS-only by default. +- Controlled dev override for HTTP (explicit flag, default-off). +- Redirect policy: + - deny redirects by default, or + - if enabled, re-validate each redirect hop before follow. +- Address range blocking includes: + - loopback + - link-local + - private RFC1918 + - multicast + - IPv6 ULA + - other internal/non-routable ranges used by current SSRF guard. +- DNS rebinding mitigation: + - resolve before request + - re-resolve before connect/use + - reject when resolved destination shifts into blocked space. +- Wrapper dispatch method for Gotify/Webhook remains `POST` only. -- `backend/internal/api/handlers/settings_handler.go` - - `UpdateSetting(...)`, `PatchConfig(...)` -- `backend/internal/api/routes/routes.go` - - Caddy manager wiring + settings routes -- `frontend/src/pages/SystemSettings.tsx` - - current Caddy-related controls +### Error leakage controls -#### Caddyfile import behavior +- Preview/Test/Send errors return: + - `error` + - `code` + - `category` (sanitized) + - `request_id` +- Forbidden in error payloads/logs: + - raw request payloads + - tokens/auth headers + - full query strings containing secrets + - raw upstream response dumps that can leak sensitive fields. -- `backend/internal/api/handlers/import_handler.go` - - `RegisterRoutes(...)`, `Upload(...)`, `GetPreview(...)` -- `backend/internal/caddy/importer.go` - - `NormalizeCaddyfile(...)`, `ParseCaddyfile(...)`, `ExtractHosts(...)` -- `frontend/src/pages/ImportCaddy.tsx` - - import UX and warning handling +### Wrapper transport safety -### Feature impact assessment (2.11.x) +- Request body max: 256 KiB. +- Response body max: 1 MiB. +- Strict outbound header allowlist: + - `Content-Type` + - `User-Agent` + - `X-Request-ID` + - `X-Gotify-Key` + - explicitly allowlisted custom headers only. +- Retry budget: + - max attempts: 3 + - exponential backoff + jitter + - retry on network error, `429`, `5xx` + - no retry on other `4xx`. -#### Candidate features for potential Charon exposure +## API Behavior by Mode -1. Keepalive server options (`keepalive_idle`, `keepalive_count`) - - Candidate mapping: advanced per-host connection tuning - - Likely files: `backend/internal/caddy/types.go`, - `backend/internal/caddy/config.go`, host settings API + UI -2. `trusted_proxies_unix` - - Candidate mapping: trusted local socket proxy chains - - Current `TrustedProxies` struct lacks explicit unix-socket trust fields -3. Certificate lifecycle tunables (`renewal_window_ratio`, maintenance interval) - - Candidate mapping: advanced TLS policy controls - - Potentially belongs under system-level TLS settings, not per-host UI +### `gotify` -#### Features likely backend-only / no new UI by default +- Required: `type`, `url`, valid payload with `message`. +- Token accepted only on create/update writes. +- Outbound auth via `X-Gotify-Key` header. +- Query-token requests are rejected. -1. Reverse-proxy automatic `Host` rewrite for TLS upstreams -2. ECH key auto-rotation -3. `SIGUSR1` reload fallback behavior -4. Logging backend internals (`timberjack`, ordering fixes) +### `webhook` -Plan decision rule: expose only options that produce clear operator value and -can be represented without adding UX complexity. +- Required: `type`, `url`, valid renderable template. +- Outbound dispatch through wrapper (`POST` JSON) with strict header controls. -### Security patch relevance matrix +### `discord` -#### Advisory exploitability rubric and ownership +- Existing behavior remains unchanged for this migration. -Use the following deterministic rubric for each advisory before any promotion: +## Frontend Design -| Field | Required Values | Rule | -| --- | --- | --- | -| Exploitability | `Affected` / `Not affected` / `Mitigated` | `Affected` means a reachable vulnerable path exists in Charon runtime; `Not affected` means required feature/path is not present; `Mitigated` means vulnerable path exists upstream but Charon deployment/runtime controls prevent exploitation. | -| Evidence source | advisory + code/config/runtime proof | Must include at least one authoritative upstream source (GitHub advisory/Caddy release) and one Charon-local proof (config path, test, scan, or runtime verification). | -| Owner | named role | Security owner for final disposition (`QA_Security` lead or delegated maintainer). | -| Recheck cadence | `weekly` / `release-candidate` / `on-upstream-change` | Minimum cadence: weekly until CVE enrichment is complete and disposition is stable for two consecutive checks. | +- `frontend/src/api/notifications.ts` + - supports `discord`, `gotify`, `webhook` + - submits token only on create/update writes + - never expects token in read/test/preview payloads +- `frontend/src/pages/Notifications.tsx` + - conditional provider fields + - masked Gotify token input + - no token re-display in readback views +- `frontend/src/pages/__tests__/Notifications.test.tsx` + - update discord-only assumptions + - add redaction checks -Promotion gate: every advisory must have all four fields populated and signed by -owner in the PR evidence bundle. +## Test Matrix Expansion -#### High-priority for Charon context +### Playwright E2E -1. `GHSA-879p-475x-rqh2` (admin API cross-origin no-cors) - - Charon binds admin API internally but still uses `0.0.0.0:2019` in - generated config. Must verify actual network isolation and container - exposure assumptions. -2. `GHSA-hffm-g8v7-wrv7` (mTLS fail-open) - - Relevant if client-auth CA pools are configured anywhere in generated or - imported config paths. -3. matcher bypass advisories (`GHSA-x76f-jf84-rqj8`, `GHSA-g7pc-pc7g-h8jh`) - - Potentially relevant to host/path-based access control routing in Caddy. +- Update: `tests/settings/notifications.spec.ts` +- Add: `tests/settings/notifications-payload.spec.ts` -#### Contextual/conditional relevance +Required scenarios: -- `GHSA-5r3v-vc8m-m96g` (FastCGI split_path) - - Relevant only if FastCGI transport is in active use. -- `GHSA-4xrr-hq4w-6vf4` (file matcher glob sanitization) - - Relevant when file matchers are used in route logic. +1. Redirect-to-internal SSRF attempt is blocked. +2. DNS rebinding simulation is blocked (unit/integration + E2E observable path). +3. Retry policy verification: + - retry on `429` and `5xx` + - no retry on non-`429` `4xx`. +4. Token redaction checks across API/log/UI surfaces. +5. Query-token rejection. +6. Oversized payload rejection. +7. Discord regression coverage. -### xcaddy patch retirement candidates +### Backend Unit/Integration -#### Candidate to re-evaluate for removal +- Update/add: + - `backend/internal/services/notification_service_json_test.go` + - `backend/internal/services/notification_service_test.go` + - `backend/internal/services/enhanced_security_notification_service_test.go` + - `backend/internal/api/handlers/notification_provider_handler_test.go` + - `backend/internal/api/handlers/notification_provider_handler_validation_test.go` +- Add integration file: + - `backend/integration/notification_http_wrapper_integration_test.go` -- `go get github.com/slackhq/nebula@v1.9.7` - - Upstream Caddy has moved forward to `nebula v1.10.3` and references - security-related maintenance in the 2.11.x line. - - Existing Charon pin comment may be stale after upstream smallstep updates. +Mandatory assertions: -#### Likely retain until proven redundant - -- `go get github.com/expr-lang/expr@v1.17.7` -- `go get github.com/hslatman/ipstore@v0.4.0` - -Retention/removal decision must be made using reproducible build + binary -inspection evidence, not assumption. - -#### Hard retirement gates (mandatory before removing any pin) - -Pin removal is blocked unless all gates pass: - -1. Binary module diff gate - - Produce before/after `go version -m` module diff for Caddy binary. - - No unexpected module major-version jumps outside approved advisory scope. -2. Security regression gate - - No new HIGH/CRITICAL findings in CodeQL/Trivy/Grype compared to baseline. -3. Reproducible build parity gate - - Two clean rebuilds produce equivalent module inventory and matching runtime - smoke results. -4. Rollback proof gate (mandatory, with explicit `nebula` focus) - - Demonstrate one-command rollback to previous pin set, with successful - compile + runtime smoke set after rollback. - -Retirement decision for `nebula` cannot proceed without explicit rollback proof -artifact attached to PR evidence. - -### Feature-to-control mapping (exposure decision matrix) - -| Feature | Control surface | Expose vs backend-only rationale | Persistence path | -| --- | --- | --- | --- | -| `keepalive_idle`, `keepalive_count` | Existing advanced system settings (if approved) | Expose only if operators need deterministic upstream connection control; otherwise keep backend defaults to avoid UX bloat. | `frontend/src/pages/SystemSettings.tsx` → `frontend/src/api/settings.ts` → `backend/internal/api/handlers/settings_handler.go` → DB settings → `backend/internal/caddy/config.go` (`GenerateConfig`) | -| `trusted_proxies_unix` | Backend-only default initially | Backend-only until proven demand for unix-socket trust tuning; avoid misconfiguration risk in general UI. | backend config model (`backend/internal/caddy/types.go`) + generated config path (`backend/internal/caddy/config.go`) | -| `renewal_window_ratio`, cert maintenance interval | Backend-only policy | Keep backend-only unless operations requires explicit lifecycle tuning controls. | settings store (if introduced) → `settings_handler.go` → `GenerateConfig` | -| Reverse-proxy Host rewrite / ECH rotation / reload fallback internals | Backend-only | Operational internals with low direct UI value; exposing would increase complexity without clear user benefit. | backend runtime defaults and generated Caddy config only | +- redirect-hop SSRF blocking +- DNS rebinding mitigation +- retry/non-retry classification +- token redaction in API/log/UI +- query-token rejection +- oversized payload rejection ## Implementation Plan -### Phase 1: Playwright and behavior baselining (mandatory first) +### Phase 1 — Backend safety foundation -Objective: capture stable pre-upgrade behavior and ensure UI/UX parity checks. +- implement wrapper contract +- implement secret contract + SSRF/error/transport controls +- keep frontend unchanged -1. Run targeted E2E suites covering Caddy-critical flows: - - `tests/tasks/import-caddyfile.spec.ts` - - `tests/security-enforcement/zzz-caddy-imports/*.spec.ts` - - system settings-related tests around Caddy admin API and SSL provider -2. Capture baseline artifacts: - - Caddy import warning behavior - - security settings save/reload behavior - - admin API connectivity assumptions from test fixtures -3. Produce a baseline report in `docs/reports/` for diffing in later phases. +Exit criteria: -### Phase 2: Backend and build compatibility research implementation +- backend tests green +- no Discord regression in backend paths -Objective: validate compile/runtime compatibility of Caddy 2.11.1 with current -plugin set and patch set. +### Phase 2 — Frontend enablement -1. Bump candidate in `Dockerfile`: - - `ARG CADDY_VERSION=2.11.1` -2. Execute matrix builds with toggles: - - Scenario A: current patch set unchanged - - Scenario B: remove `nebula` pin only - - Scenario C: remove `nebula` + retain `expr/ipstore` -3. Execute explicit compatibility gate matrix (deterministic): +- enable Gotify/Webhook UI/client paths +- enforce token write-only UX semantics - | Dimension | Values | - | --- | --- | - | Plugin set | `caddy-security`, `coraza-caddy`, `caddy-crowdsec-bouncer`, `caddy-geoip2`, `caddy-ratelimit` | - | Patch scenario | `A` current pins, `B` no `nebula` pin, `C` no `nebula` pin + retained `expr/ipstore` pins | - | Platform/arch | `linux/amd64`, `linux/arm64` | - | Runtime smoke set | boot Caddy, apply generated config, admin API health, import preview, one secured proxy request path | +Exit criteria: - Deterministic pass/fail rule: - - **Pass**: all plugin modules compile/load for the matrix cell AND all smoke - tests pass. - - **Fail**: any compile/load error, missing module, or smoke failure. +- frontend tests green +- accessibility and form behavior validated - Promotion criteria: - - PR-1 promotion requires 100% pass for Scenario A on both architectures. - - Scenario B/C may progress only as candidate evidence; they cannot promote to - default unless all hard retirement gates pass. -4. Validate generated binary dependencies from CI/local: - - verify `expr`, `ipstore`, `nebula`, `smallstep/certificates` versions -5. Validate runtime config application path: - - `backend/internal/caddy/manager.go` → `ApplyConfig(ctx)` - - `backend/internal/caddy/config.go` → `GenerateConfig(...)` -6. Run Caddy package tests and relevant integration tests: - - `backend/internal/caddy/*` - - security middleware integration paths that rely on Caddy behavior +### Phase 3 — E2E and coverage hardening -### Phase 3: Security hardening and vulnerability posture updates +- add expanded matrix scenarios +- enforce DoD sequence and patch-report artifacts -Objective: translate upstream advisories into Charon policy and tests. +Exit criteria: -1. Add/adjust regression tests for advisory-sensitive behavior in - `backend/internal/caddy` and integration test suites, especially: - - host matcher behavior with large host lists - - escaped path matcher handling - - admin API cross-origin assumptions -2. Update security documentation and operational guidance: - - identify which advisories are mitigated by upgrade alone - - identify deployment assumptions (e.g., local admin API exposure) -3. Introduce watchlist process for RESERVED CVEs pending NVD enrichment: - - monitor Caddy advisories and module-level disclosures weekly - -### Phase 4: Frontend and API exposure decisions (only if justified) - -Objective: decide whether 2.11.x features merit UI controls. - -1. Evaluate additions to existing `SystemSettings` UX only (no new page): - - optional advanced toggles for keepalive tuning and trusted proxy unix scope -2. Add backend settings keys and mapping only where persisted behavior is - needed: - - settings handler support in - `backend/internal/api/handlers/settings_handler.go` - - propagation to config generation in `GenerateConfig(...)` -3. If no high-value operator need is proven, keep features backend-default and - document rationale. - -### Phase 5: Validation, docs, and release readiness - -Objective: ensure secure, reversible, and auditable rollout. - -1. Re-run full DoD sequence (E2E, patch report, security scans, coverage). -2. Update architectural docs if behavior/config model changes. -3. Publish release decision memo: - - accepted changes - - rejected/deferred UX features - - retained/removed patches with evidence +- E2E matrix passing +- `test-results/local-patch-report.md` generated +- `test-results/local-patch-report.json` generated ## PR Slicing Strategy -### Decision +Decision: Multiple PRs for security and rollback safety. -Use **multiple PRs (PR-1/PR-2/PR-3)**. +### Schema migration decision -Reasoning: +- Decision: no schema migration in `PR-1`. +- Contingency: if schema changes become necessary, create separate `PR-0` for + migration-only changes before `PR-1`. -1. Work spans infra/build security + backend runtime + potential frontend UX. -2. Caddy is a blast-radius-critical dependency; rollback safety is mandatory. -3. Review quality and CI signal are stronger with isolated, testable slices. - -### PR-1: Compatibility and evidence foundation +### PR-1 — Backend wrapper + safety controls Scope: -- `Dockerfile` Caddy candidate bump (and temporary feature branch matrix toggles) -- CI/workflow compatibility instrumentation if needed -- compatibility report artifacts and plan-linked documentation +- wrapper module + service/handler integration +- secret contract + SSRF + leakage + transport controls +- unit/integration tests -Dependencies: +Mandatory rollout safety: -- None +- feature flags for Gotify/Webhook dispatch are default `OFF` in PR-1. -Acceptance criteria: +Validation gates: -1. Caddy 2.11.1 compiles with existing plugin set under at least one stable - patch scenario. -2. Compatibility gate matrix (plugin × patch scenario × platform/arch × runtime - smoke set) executed with deterministic pass/fail output and attached evidence. -3. Binary module inventory report generated and attached. -4. No production behavior changes merged beyond compatibility scaffolding. +- backend tests pass +- no token leakage in API/log/error flows +- no Discord regression -Release guard (mandatory for PR-1): - -- Candidate tag only (`*-rc`/`*-candidate`) is allowed. -- Release pipeline exclusion is required; PR-1 artifacts must not be eligible - for production release jobs. -- Promotion to releasable tag is blocked until PR-2 security/retirement gates - pass. - -Rollback notes: - -- Revert `Dockerfile` arg changes and instrumentation only. - -### PR-2: Security patch posture + patch retirement decision +### PR-2 — Frontend provider UX Scope: -- finalize retained/removed `go get` patch lines in `Dockerfile` -- update security tests/docs tied to six Caddy advisories -- tighten/confirm admin API exposure assumptions +- API client and Notifications page updates +- frontend tests for mode handling and redaction -Dependencies: +Dependencies: PR-1 merged. -- PR-1 evidence +Validation gates: -Acceptance criteria: +- frontend tests pass +- accessibility checks pass -1. Decision logged for each patch (`expr`, `ipstore`, `nebula`) with rationale. -2. Advisory coverage matrix completed with Charon applicability labels. -3. Security scans clean at required policy thresholds. +### PR-3 — Playwright matrix and coverage hardening -Rollback notes: +Scope: -- Revert patch retirement lines and keep previous pinned patch model. +- notifications E2E matrix expansion +- fixture updates as required -### PR-3: Optional UX/API exposure and cleanup (Focused Execution Update) +Dependencies: PR-1 and PR-2 merged. -Decision summary: +Validation gates: -- PR-3 remains optional and value-gated. -- Expose only controls with clear operator value on existing `SystemSettings`. -- Keep low-value/high-risk knobs backend-default and non-exposed. +- security matrix scenarios pass +- patch-report artifacts generated -Operator-value exposure decision: +## Risks and Mitigations -| Candidate | Operator value | Decision in PR-3 | -| --- | --- | --- | -| `keepalive_idle`, `keepalive_count` | Helps operators tune long-lived upstream behavior (streaming, websocket-heavy, high-connection churn) without editing config by hand. | **Expose minimally** (only if PR-2 confirms stable runtime behavior). | -| `trusted_proxies_unix` | Niche socket-chain use case, easy to misconfigure, low value for default Charon operators. | **Do not expose**; backend-default only. | -| `renewal_window_ratio` / cert maintenance internals | Advanced certificate lifecycle tuning with low day-to-day value and higher support burden. | **Do not expose**; backend-default only. | +1. Risk: secret leakage via error/log paths. + - Mitigation: mandatory redaction and sanitized-category responses. +2. Risk: SSRF bypass via redirects/rebinding. + - Mitigation: default redirect deny + per-hop re-validation + re-resolution. +3. Risk: retry storms or payload abuse. + - Mitigation: capped retries, exponential backoff+jitter, size caps. +4. Risk: Discord regression. + - Mitigation: preserved behavior, regression tests, default-off new flags. -Strict scope constraints: +## Acceptance Criteria (Definition of Done) -- No new routes, pages, tabs, or modals. -- UI changes limited to existing `frontend/src/pages/SystemSettings.tsx` general/system section. -- API surface remains existing settings endpoints only (`POST /settings`, `PATCH /config`). -- Preserve backend defaults when setting is absent, empty, or invalid. +1. `docs/plans/current_spec.md` contains one active Notify migration plan only. +2. Gotify token contract is explicit: write-path only, secure storage, zero + read/test/preview return. +3. SSRF hardening includes HTTPS default, redirect controls, blocked ranges, + rebinding checks, and POST-only wrapper method. +4. Preview/test error details are sanitized with `request_id` and no raw + payload/token/query leakage. +5. Transport safety includes body size limits, strict header allowlist, and + bounded retry/backoff+jitter policy. +6. Test matrix includes redirect-to-internal SSRF, rebinding simulation, + retry split, redaction checks, query-token rejection, oversized-payload + rejection. +7. PR slicing includes PR-1 default-off flags and explicit schema decision. +8. No conflicting language remains. +9. Status remains: Ready for Supervisor Review. -Minimum viable controls (if PR-3 is activated): +## Supervisor Handoff -1. `caddy.keepalive_idle` (optional) - - Surface: `SystemSettings` under existing Caddy/system controls. - - UX: bounded select/input for duration-like value (validated server-side). - - Persistence: existing `updateSetting()` flow. -2. `caddy.keepalive_count` (optional) - - Surface: `SystemSettings` adjacent to keepalive idle. - - UX: bounded numeric control (validated server-side). - - Persistence: existing `updateSetting()` flow. +Ready for Supervisor review. -Exact files/functions/components to change: +--- -Backend (no new endpoints): +## GAS Warning Remediation Plan — Missing Code Scanning Configurations (2026-02-24) -1. `backend/internal/caddy/manager.go` - - Function: `ApplyConfig(ctx context.Context) error` - - Change: read optional settings keys (`caddy.keepalive_idle`, `caddy.keepalive_count`), normalize/validate parsed values, pass sanitized values into config generation. - - Default rule: on missing/invalid values, pass empty/zero equivalents so generated config keeps current backend-default behavior. -2. `backend/internal/caddy/config.go` - - Function: `GenerateConfig(...)` - - Change: extend function parameters with optional keepalive values and apply them only when non-default/valid. - - Change location: HTTP server construction block where server-level settings (including trusted proxies) are assembled. -3. `backend/internal/caddy/types.go` - - Type: `Server` - - Change: add optional fields required to emit keepalive keys in Caddy JSON only when provided. -4. `backend/internal/api/handlers/settings_handler.go` - - Functions: `UpdateSetting(...)`, `PatchConfig(...)` - - Change: add narrow validation for `caddy.keepalive_idle` and `caddy.keepalive_count` to reject malformed/out-of-range values while preserving existing generic settings behavior for unrelated keys. +Status: Planned (ready for implementation PR) +Issue: GitHub Advanced Security warning on PRs: -Frontend (existing surface only): +> Code scanning cannot determine alerts introduced by this PR because 3 configurations present on refs/heads/development were not found: `trivy-nightly (nightly-build.yml)`, `.github/workflows/docker-build.yml:build-and-push`, `.github/workflows/docker-publish.yml:build-and-push`. -1. `frontend/src/pages/SystemSettings.tsx` - - Component: `SystemSettings` - - Change: add local state load/save wiring for optional keepalive controls using existing settings query/mutation flow. - - Change: render controls in existing General/System card only. -2. `frontend/src/api/settings.ts` - - No contract expansion required; reuse `updateSetting(key, value, category, type)`. -3. Localization files (labels/help text only, if controls are exposed): - - `frontend/src/locales/en/translation.json` - - `frontend/src/locales/de/translation.json` - - `frontend/src/locales/es/translation.json` - - `frontend/src/locales/fr/translation.json` - - `frontend/src/locales/zh/translation.json` +### 1) Root Cause Summary -Tests to update/add (targeted): +Research outcome from current workflow state and history: -1. `frontend/src/pages/__tests__/SystemSettings.test.tsx` - - Verify control rendering, default-state behavior, and save calls for optional keepalive keys. -2. `backend/internal/caddy/config_generate_test.go` - - Verify keepalive keys are omitted when unset/invalid and emitted when valid. -3. `backend/internal/api/handlers/settings_handler_test.go` - - Verify validation pass/fail for keepalive keys via both `UpdateSetting` and `PatchConfig` paths. -4. Existing E2E settings coverage (no new suite) - - Extend existing settings-related specs only if UI controls are activated in PR-3. +- `.github/workflows/docker-publish.yml` was deleted in commit `f640524baaf9770aa49f6bd01c5bde04cd50526c` (2025-12-21), but historical code-scanning configuration identity from that workflow (`.github/workflows/docker-publish.yml:build-and-push`) still exists in baseline comparisons. +- Both legacy `docker-publish.yml` and current `docker-build.yml` used job id `build-and-push` and uploaded Trivy SARIF only for non-PR events (`push`/scheduled paths), so PR branches often do not produce configuration parity. +- `.github/workflows/nightly-build.yml` uploads SARIF with explicit category `trivy-nightly`, but this workflow is schedule/manual only, so PR branches do not emit `trivy-nightly`. +- Current PR scanning in `docker-build.yml` uses `scan-pr-image` with category `docker-pr-image`, which does not satisfy parity for legacy/base configuration identities. +- Result: GitHub cannot compute “introduced by this PR” for those 3 baseline configurations because matching configurations are absent in PR analysis runs. -Dependencies: +### 2) Minimal-Risk Remediation Strategy (Future-PR Safe) -- PR-2 must establish stable runtime/security baseline first. -- PR-3 activation requires explicit operator-value confirmation from PR-2 evidence. +Decision: keep existing security scans and add compatibility SARIF uploads in PR context, without changing branch/release behavior. -Acceptance criteria (PR-3 complete): +Why this is minimal risk: -1. No net-new page; all UI changes are within `SystemSettings` only. -2. No new backend routes/endpoints; existing settings APIs are reused. -3. Only approved controls (`caddy.keepalive_idle`, `caddy.keepalive_count`) are exposed, and exposure is allowed only if the PR-3 Value Gate checklist is fully satisfied. -4. `trusted_proxies_unix`, `renewal_window_ratio`, and certificate-maintenance internals remain backend-default and non-exposed. -5. Backend preserves current behavior when optional keepalive settings are absent or invalid (no generated-config drift). -6. Unit tests pass for settings validation + config generation default/override behavior. -7. Settings UI tests pass for load/save/default behavior on exposed controls. -8. Deferred/non-exposed features are explicitly documented in PR notes as intentional non-goals. +- No changes to image build semantics, release tags, or nightly promotion flow. +- Reuses already-generated SARIF files (no new scanner runtime dependency). +- Limited to additive upload steps and explicit categories. +- Provides immediate parity for PRs while allowing controlled cleanup of legacy configuration. -#### PR-3 Value Gate (required evidence and approval) +### 3) Exact Workflow Edits to Apply -Required evidence checklist (all items required): +#### A. `.github/workflows/docker-build.yml` -- [ ] PR-2 evidence bundle contains an explicit operator-value decision record for PR-3 controls, naming `caddy.keepalive_idle` and `caddy.keepalive_count` individually. -- [ ] Decision record includes objective evidence for each exposed control from at least one concrete source: test/baseline artifact, compatibility/security report, or documented operator requirement. -- [ ] PR includes before/after evidence proving scope containment: no new page, no new route, and no additional exposed Caddy keys beyond the two approved controls. -- [ ] Validation artifacts for PR-3 are attached: backend unit tests, frontend settings tests, and generated-config assertions for default/override behavior. +In job `scan-pr-image`, after existing `Upload Trivy scan results` step: -Approval condition (pass/fail): +1. Add compatibility upload step reusing `trivy-pr-results.sarif` with category: + - `.github/workflows/docker-build.yml:build-and-push` +2. Add compatibility alias upload step reusing `trivy-pr-results.sarif` with category: + - `trivy-nightly` +3. Add temporary legacy compatibility upload step reusing `trivy-pr-results.sarif` with category: + - `.github/workflows/docker-publish.yml:build-and-push` -- **Pass**: all checklist items are complete and a maintainer approval explicitly states "PR-3 Value Gate approved". -- **Fail**: any checklist item is missing or approval text is absent; PR-3 control exposure is blocked and controls remain backend-default/non-exposed. +Implementation notes: -Rollback notes: +- Keep existing `docker-pr-image` category upload unchanged. +- Add SARIF file existence guards before each compatibility upload (for example, conditional check that `trivy-pr-results.sarif` exists) to avoid spurious step failures. +- Keep compatibility upload steps non-blocking with `continue-on-error: true`; use `if: always()` plus existence guard so upload attempts are resilient but quiet when SARIF is absent. +- Add TODO/date marker in step name/description indicating temporary status for `docker-publish` alias and planned removal checkpoint. -- Revert only PR-3 UI/settings mapping changes while retaining PR-1/PR-2 runtime and security upgrades. +#### B. Mandatory category hardening (same PR) -## Config File Review and Proposed Updates +In `docker-build.yml` non-PR Trivy upload, explicitly set category to `.github/workflows/docker-build.yml:build-and-push`. -### Dockerfile (required updates) +- Requirement level: mandatory (not optional). +- Purpose: make identity explicit and stable even if future upload defaults change. +- Safe because it aligns with currently reported baseline identity. -1. Update `ARG CADDY_VERSION` target to `2.11.1` after PR-1 gating. -2. Reassess and potentially remove stale `nebula` pin in caddy-builder stage - if matrix build proves compatibility and security posture improves. -3. Keep `expr`/`ipstore` patch enforcement until binary inspection proves - upstream transitive versions are consistently non-vulnerable. +### 4) Migration/Cleanup for Legacy `docker-publish` Configuration -### .gitignore (suggested updates) +Planned two-stage cleanup: -No mandatory update for rollout, but recommended if new evidence artifacts are -generated in temporary paths: +1. **Stabilization window (concrete trigger):** + - Keep compatibility upload for `.github/workflows/docker-publish.yml:build-and-push` enabled. + - Keep temporary alias active through **2026-03-24** and until **at least 8 merged PRs** with successful `scan-pr-image` runs are observed (both conditions required). + - Verify warning is gone across representative PRs. -- ensure transient compatibility artifacts are ignored (for example, - `test-results/caddy-compat/**` if used). +2. **Retirement window:** + - Remove compatibility step for `docker-publish` category from `docker-build.yml`. + - In GitHub UI/API, close/dismiss remaining alerts tied only to legacy configuration if they persist and are no longer actionable. + - Confirm new PRs still show introduced-alert computation without warnings. -### .dockerignore (suggested updates) +### 5) Validation Steps (Expected Workflow Observations) -No mandatory update; current file already excludes heavy test/docs/security -artifacts and keeps build context lean. Revisit only if new compatibility -fixture directories are introduced. +For at least two PRs (one normal feature PR and one workflow-only PR), verify: -### codecov.yml (suggested updates) +1. `docker-build.yml` runs `scan-pr-image` and uploads SARIF under: + - `docker-pr-image` + - `.github/workflows/docker-build.yml:build-and-push` + - `trivy-nightly` + - `.github/workflows/docker-publish.yml:build-and-push` (temporary) +2. PR Security tab no longer shows: + - “Code scanning cannot determine alerts introduced by this PR because ... configurations ... were not found”. +3. No regression: + - Existing Trivy PR blocking behavior remains intact. + - Main/development/nightly push flows continue unchanged. -No mandatory change for version upgrade itself. If new compatibility harness -tests are intentionally non-coverage-bearing, add explicit ignore patterns to -avoid noise in project and patch coverage reports. +### 6) Rollback Notes -## Risk Register and Mitigations +If compatibility uploads create noise, duplicate alert confusion, or unstable checks: -1. Plugin/API incompatibility with Caddy 2.11.1 - - Mitigation: matrix compile + targeted runtime tests before merge. -2. False confidence from scanner-only dependency policies - - Mitigation: combine advisory-context review with binary-level inspection. -3. Behavioral drift in reverse proxy/matcher semantics - - Mitigation: baseline E2E + focused security regression tests. -4. UI sprawl from exposing too many Caddy internals - - Mitigation: only extend existing settings surface when operator value is - clear and validated. +1. Revert only the newly added compatibility upload steps (keep original uploads). +2. Re-run workflows on a test PR and confirm baseline behavior restored. +3. If warning reappears, switch to fallback strategy: + - Keep only `.github/workflows/docker-build.yml:build-and-push` compatibility upload. + - Remove `trivy-nightly` alias and handle nightly parity via separate dedicated PR-safe workflow. -## Acceptance Criteria +### 7) PR Slicing Strategy for This Fix -1. Charon builds and runs with Caddy 2.11.1 and current plugin set under - deterministic CI validation. -2. A patch disposition table exists for `expr`, `ipstore`, and `nebula` - (retain/remove/replace + evidence). -3. Caddy advisory applicability matrix is documented, including exploitability - notes for Charon deployment model. -4. Any added settings are mapped end-to-end: - frontend state → API payload → persisted setting → `GenerateConfig(...)`. -5. E2E, security scans, and coverage gates pass without regression. -6. PR-1/PR-2/PR-3 deliverables are independently reviewable and rollback-safe. - -## Handoff - -After approval of this plan: - -1. Delegate PR-1 execution to implementation workflow. -2. Require evidence artifacts before approving PR-2 scope reductions - (especially patch removals). -3. Treat PR-3 as optional and value-driven, not mandatory for the security - update itself. - -## PR-3 QA Closure Addendum (2026-02-23) - -### Scope - -PR-3 closure only: - -1. Keepalive controls (`caddy.keepalive_idle`, `caddy.keepalive_count`) -2. Safe defaults/fallback behavior when keepalive values are missing or invalid -3. Non-exposure constraints for deferred settings - -### Final QA Outcome - -- Verdict: **READY (PASS)** -- Targeted PR-3 E2E rerun: **30 passed, 0 failed** -- Local patch preflight: **PASS** with required LCOV artifact present -- Coverage/type-check/security gates: **PASS** - -### Scope Guardrails Confirmed - -- UI scope remains constrained to existing System Settings surface. -- No PR-3 expansion beyond approved keepalive controls. -- Non-exposed settings remain non-exposed (`trusted_proxies_unix` and certificate lifecycle internals). -- Safe fallback/default behavior remains intact for invalid or absent keepalive input. - -### Reviewer References - -- QA closure report: `docs/reports/qa_report.md` -- Manual verification plan: `docs/issues/manual_test_pr3_keepalive_controls_closure.md` +- **PR-1 (recommended single PR, low-risk additive):** add compatibility SARIF uploads in `docker-build.yml` (`scan-pr-image`) with SARIF existence guards, `continue-on-error` on compatibility uploads, and mandatory non-PR category hardening, plus brief inline rationale comments. +- **PR-2 (cleanup PR, delayed):** remove `.github/workflows/docker-publish.yml:build-and-push` compatibility upload after stabilization window and verify no warning recurrence. diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 6b0e0eba..1349137c 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,3 +1,52 @@ +## QA/Security Audit — PR-1 Backend Slice (Notify HTTP Wrapper) + +- Date: 2026-02-23 +- Scope: Current PR-1 backend slice implementation (notification provider handler/service, wrapper path, security gating) +- Verdict: **READY (PASS WITH NON-BLOCKING WARNINGS)** + +## Commands Run + +1. `git rev-parse --abbrev-ref HEAD && git rev-parse --abbrev-ref --symbolic-full-name @{u} && git diff --name-only origin/main...HEAD` +2. `./.github/skills/scripts/skill-runner.sh docker-rebuild-e2e` +3. `PLAYWRIGHT_BASE_URL=http://localhost:8080 npx playwright test tests/settings/notifications.spec.ts` +4. `bash scripts/local-patch-report.sh` +5. `bash scripts/go-test-coverage.sh` +6. `pre-commit run --all-files` +7. `./.github/skills/scripts/skill-runner.sh security-scan-trivy` +8. `./.github/skills/scripts/skill-runner.sh security-scan-docker-image` +9. `bash scripts/pre-commit-hooks/codeql-go-scan.sh` +10. `bash scripts/pre-commit-hooks/codeql-js-scan.sh` +11. `bash scripts/pre-commit-hooks/codeql-check-findings.sh` +12. `./scripts/scan-gorm-security.sh --check` + +## Gate Results + +| Gate | Status | Evidence | +| --- | --- | --- | +| 1) Playwright E2E first | PASS | Notifications feature suite passed: **79/79** on local E2E environment. | +| 2) Local patch coverage preflight | PASS (WARN) | Artifacts generated: `test-results/local-patch-report.md` and `test-results/local-patch-report.json`; mode=`warn` due missing `frontend/coverage/lcov.info`. | +| 3) Backend coverage + threshold | PASS | `scripts/go-test-coverage.sh` reported **87.7% line** / **87.4% statement**; threshold 85% met. | +| 4) `pre-commit --all-files` | PASS | All configured hooks passed. | +| 5a) Trivy filesystem scan | PASS | No CRITICAL/HIGH/MEDIUM findings reported by skill at configured scanners/severities. | +| 5b) Docker image security scan | PASS | No CRITICAL/HIGH; Grype summary from `grype-results.json`: **Medium=10, Low=4**. | +| 5c) CodeQL Go + JS CI-aligned + findings check | PASS | Go and JS scans completed; findings check reported no security issues in both languages. | +| 6) GORM scanner (`--check`) | PASS | 0 CRITICAL/HIGH/MEDIUM; 2 INFO suggestions only. | + +## Blockers / Notes + +- **No merge-blocking security or QA failures** were found for this PR-1 backend slice. +- Non-blocking operational notes: + - E2E initially failed until stale conflicting container was removed and E2E environment was rebuilt. + - `scripts/local-patch-report.sh` completed artifact generation in warning mode because frontend coverage input was absent. + - `pre-commit run codeql-check-findings --all-files` hook id was not registered in this local setup; direct script execution (`scripts/pre-commit-hooks/codeql-check-findings.sh`) passed. + +## Recommendation + +- **Proceed to PR-2**. +- Carry forward two non-blocking follow-ups: + 1. Ensure frontend coverage artifact generation before local patch preflight to eliminate warning mode. + 2. Optionally align local pre-commit hook IDs with documented CodeQL findings check command. + ## QA Report — PR-2 Security Patch Posture Audit - Date: 2026-02-23 @@ -55,3 +104,96 @@ All PR-2 QA/security gates required for merge are passing. No PR-3 scope is incl ## PR-3 Closure Statement PR-3 is **ready to merge** with no open QA blockers. + +--- + +## QA/Security Audit — PR-2 Frontend Slice (Notifications) + +- Date: 2026-02-24 +- Scope: PR-2 frontend notifications slice only (UI/API contract alignment, tests, QA/security gates) +- Verdict: **READY (PASS WITH NON-BLOCKING WARNINGS)** + +## Commands Run + +1. `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e` +2. `/projects/Charon/node_modules/.bin/playwright test /projects/Charon/tests/settings/notifications.spec.ts --config=/projects/Charon/playwright.config.js --project=firefox` +3. `bash /projects/Charon/scripts/local-patch-report.sh` +4. `/projects/Charon/.github/skills/scripts/skill-runner.sh test-frontend-coverage` +5. `cd /projects/Charon/frontend && npm run type-check` +6. `cd /projects/Charon && pre-commit run --all-files` +7. VS Code task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` +8. VS Code task: `Security: CodeQL Go Scan (CI-Aligned) [~60s]` +9. `cd /projects/Charon && bash scripts/pre-commit-hooks/codeql-check-findings.sh` +10. `/projects/Charon/.github/skills/scripts/skill-runner.sh security-scan-trivy` + +## Gate Results + +| Gate | Status | Evidence | +| --- | --- | --- | +| 1) Playwright E2E first (notifications-focused) | PASS | `tests/settings/notifications.spec.ts`: **27 passed, 0 failed** after PR-2-aligned expectation update. | +| 2) Local patch coverage preflight artifacts | PASS (WARN) | Artifacts generated: `test-results/local-patch-report.md` and `test-results/local-patch-report.json`; report mode=`warn` with `changed_lines=0` for current baseline range. | +| 3) Frontend coverage + threshold | PASS | `test-frontend-coverage` skill completed successfully; coverage gate **PASS** at **89% lines** vs minimum **87%**. | +| 4) TypeScript check | PASS | `npm run type-check` completed with `tsc --noEmit` and no type errors. | +| 5) `pre-commit run --all-files` | PASS | All configured hooks passed, including frontend lint/type checks and fast Go linters. | +| 6a) CodeQL JS (CI-aligned) | PASS | JS scan completed and SARIF generated (`codeql-results-js.sarif`). | +| 6b) CodeQL Go (CI-aligned) | PASS | Go scan completed and SARIF generated (`codeql-results-go.sarif`). | +| 6c) CodeQL findings gate | PASS | `scripts/pre-commit-hooks/codeql-check-findings.sh` reported no security issues in Go/JS. | +| 6d) Trivy filesystem scan | PASS | `security-scan-trivy` completed with **0 vulnerabilities** and **0 secrets** at configured severities. | +| 6e) GORM scanner | SKIPPED (N/A) | Not required for PR-2 frontend-only slice (no `backend/internal/models/**` or GORM persistence scope changes). | + +## Low-Risk Fixes Applied During Audit + +1. Updated Playwright notifications spec to match PR-2 provider UX (`discord/gotify/webhook` selectable, not disabled): + - `tests/settings/notifications.spec.ts` +2. Updated legacy frontend API unit test expectations from Discord-only to supported provider contract: + - `frontend/src/api/__tests__/notifications.test.ts` + +## Blockers / Notes + +- **No merge-blocking QA/security blockers** for PR-2 frontend slice. +- Non-blocking notes: + - Local patch preflight is in `warn` mode with `changed_lines=0` against `origin/development...HEAD`; artifacts are present and valid. + - Local command execution is cwd-sensitive; absolute paths were used for reliable gate execution. + +## Recommendation + +- **Proceed to PR-3**. +- No blocking items remain for the PR-2 frontend slice. + +--- + +## Final QA/Security Audit — Notify Migration (PR-1/PR-2/PR-3) + +- Date: 2026-02-24 +- Scope: Final consolidated verification for completed notify migration slices (PR-1 backend, PR-2 frontend, PR-3 E2E/coverage hardening) +- Verdict: **ALL-PASS** + +## Mandatory Gate Sequence Results + +| Gate | Status | Evidence | +| --- | --- | --- | +| 1) Playwright E2E first (notifications-focused, including new payload suite) | PASS | `npx playwright test tests/settings/notifications.spec.ts tests/settings/notifications-payload.spec.ts --project=firefox --workers=1 --reporter=line` → **37 passed, 0 failed**. | +| 2) Local patch coverage preflight artifacts generation | PASS (WARN mode allowed) | `bash scripts/local-patch-report.sh` generated `test-results/local-patch-report.md` and `test-results/local-patch-report.json` with artifact verification. | +| 3) Backend coverage threshold check | PASS | `bash scripts/go-test-coverage.sh` → **Line coverage 87.4%**, minimum required **85%**. | +| 4) Frontend coverage threshold check | PASS | `bash scripts/frontend-test-coverage.sh` → **Lines 89%**, minimum required **85%** (coverage gate PASS). | +| 5) Frontend TypeScript check | PASS | `cd frontend && npm run type-check` completed with `tsc --noEmit` and no errors. | +| 6) `pre-commit run --all-files` | PASS | First run auto-fixed EOF in `tests/settings/notifications-payload.spec.ts`; rerun passed all hooks. | +| 7a) Trivy filesystem scan | PASS | `./.github/skills/scripts/skill-runner.sh security-scan-trivy` → no CRITICAL/HIGH/MEDIUM issues and no secrets detected. | +| 7b) Docker image scan | PASS | `./.github/skills/scripts/skill-runner.sh security-scan-docker-image` → **Critical 0 / High 0 / Medium 10 / Low 4**; gate policy passed (no critical/high). | +| 7c) CodeQL Go scan (CI-aligned) | PASS | CI-aligned Go scan completed; results written to `codeql-results-go.sarif`. | +| 7d) CodeQL JS scan (CI-aligned) | PASS | CI-aligned JS scan completed; results written to `codeql-results-js.sarif`. | +| 7e) CodeQL findings gate | PASS | `bash scripts/pre-commit-hooks/codeql-check-findings.sh` → no security issues in Go or JS findings gate. | +| 8) GORM security check mode (applicable) | PASS | `./scripts/scan-gorm-security.sh --check` → **0 CRITICAL / 0 HIGH / 0 MEDIUM**, INFO suggestions only. | + +## Final Verdict + +- all-pass / blockers: **ALL-PASS, no unresolved blockers** +- exact failing gates: **None (final reruns all passed)** +- proceed to handoff: **YES** + +## Notes + +- Transient issues were resolved during audit execution: + - Initial Playwright run saw container availability drop (`ECONNREFUSED`); after E2E environment rebuild and deterministic rerun, gate passed. + - Initial pre-commit run required one automatic EOF fix and passed on rerun. + - Shell working-directory drift caused temporary command-not-found noise for root-level security scripts; rerun from repo root passed. diff --git a/frontend/src/api/__tests__/notifications.test.ts b/frontend/src/api/__tests__/notifications.test.ts index 3a3eb73e..5339161a 100644 --- a/frontend/src/api/__tests__/notifications.test.ts +++ b/frontend/src/api/__tests__/notifications.test.ts @@ -52,9 +52,9 @@ describe('notifications api', () => { await testProvider({ id: '2', name: 'test', type: 'discord' }) expect(client.post).toHaveBeenCalledWith('/notifications/providers/test', { id: '2', name: 'test', type: 'discord' }) - await expect(createProvider({ name: 'x', type: 'slack' })).rejects.toThrow('Only discord notification providers are supported') - await expect(updateProvider('2', { name: 'updated', type: 'generic' })).rejects.toThrow('Only discord notification providers are supported') - await expect(testProvider({ id: '2', name: 'test', type: 'telegram' })).rejects.toThrow('Only discord notification providers are supported') + 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') }) it('templates and previews use merged payloads', async () => { @@ -68,7 +68,10 @@ describe('notifications api', () => { expect(preview).toEqual({ preview: 'ok' }) expect(client.post).toHaveBeenCalledWith('/notifications/providers/preview', { name: 'provider', type: 'discord', data: { user: 'alice' } }) - await expect(previewProvider({ name: 'provider', type: 'webhook' }, { user: 'alice' })).rejects.toThrow('Only discord notification providers are supported') + vi.mocked(client.post).mockResolvedValueOnce({ data: { preview: 'webhook-ok' } }) + const webhookPreview = await previewProvider({ name: 'provider', type: 'webhook' }, { user: 'alice' }) + expect(webhookPreview).toEqual({ preview: 'webhook-ok' }) + expect(client.post).toHaveBeenCalledWith('/notifications/providers/preview', { name: 'provider', type: 'webhook', data: { user: 'alice' } }) }) it('external template endpoints shape payloads', async () => { diff --git a/frontend/src/api/notifications.test.ts b/frontend/src/api/notifications.test.ts index 59d4861c..36a01b60 100644 --- a/frontend/src/api/notifications.test.ts +++ b/frontend/src/api/notifications.test.ts @@ -88,14 +88,38 @@ describe('notifications api', () => { expect(mockedClient.delete).toHaveBeenCalledWith('/notifications/providers/new') }) - it('rejects non-discord type before submit for provider mutations and preview', async () => { - await expect(createProvider({ name: 'Bad', type: 'slack' })).rejects.toThrow('Only discord notification providers are supported') - await expect(updateProvider('bad', { type: 'generic' })).rejects.toThrow('Only discord notification providers are supported') - await expect(testProvider({ id: 'bad', type: 'email' })).rejects.toThrow('Only discord notification providers are supported') - await expect(previewProvider({ id: 'bad', type: 'gotify' })).rejects.toThrow('Only discord notification providers are supported') + it('supports discord, gotify, and webhook while enforcing token payload contract', async () => { + mockedClient.post.mockResolvedValue({ data: { id: 'ok' } }) + mockedClient.put.mockResolvedValue({ data: { id: 'ok' } }) - expect(mockedClient.post).not.toHaveBeenCalled() - expect(mockedClient.put).not.toHaveBeenCalled() + await createProvider({ name: 'Gotify', type: 'gotify', gotify_token: 'secret-token' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers', { + name: 'Gotify', + type: 'gotify', + token: 'secret-token', + }) + + await updateProvider('ok', { type: 'webhook', url: 'https://example.com/webhook', gotify_token: 'should-not-send' }) + expect(mockedClient.put).toHaveBeenCalledWith('/notifications/providers/ok', { + type: 'webhook', + url: 'https://example.com/webhook', + }) + + await testProvider({ id: 'ok', type: 'gotify', gotify_token: 'should-not-send' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers/test', { + id: 'ok', + type: 'gotify', + }) + + await previewProvider({ id: 'ok', type: 'gotify', gotify_token: 'should-not-send' }) + expect(mockedClient.post).toHaveBeenCalledWith('/notifications/providers/preview', { + id: 'ok', + type: 'gotify', + }) + + 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: 'email' })).rejects.toThrow('Unsupported notification provider type: email') }) 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 ab2dcd59..53912dc7 100644 --- a/frontend/src/api/notifications.ts +++ b/frontend/src/api/notifications.ts @@ -1,6 +1,24 @@ import client from './client'; -const DISCORD_PROVIDER_TYPE = 'discord' as const; +export const SUPPORTED_NOTIFICATION_PROVIDER_TYPES = ['discord', 'gotify', 'webhook'] as const; +export type SupportedNotificationProviderType = (typeof SUPPORTED_NOTIFICATION_PROVIDER_TYPES)[number]; +const DEFAULT_PROVIDER_TYPE: SupportedNotificationProviderType = 'discord'; + +const isSupportedNotificationProviderType = (type: string | undefined): type is SupportedNotificationProviderType => + typeof type === 'string' && SUPPORTED_NOTIFICATION_PROVIDER_TYPES.includes(type.toLowerCase() as SupportedNotificationProviderType); + +const resolveProviderTypeOrThrow = (type: string | undefined): SupportedNotificationProviderType => { + if (typeof type === 'undefined') { + return DEFAULT_PROVIDER_TYPE; + } + + const normalizedType = type.toLowerCase(); + if (isSupportedNotificationProviderType(normalizedType)) { + return normalizedType; + } + + throw new Error(`Unsupported notification provider type: ${type}`); +}; /** Notification provider configuration. */ export interface NotificationProvider { @@ -10,6 +28,8 @@ export interface NotificationProvider { url: string; config?: string; template?: string; + gotify_token?: string; + token?: string; enabled: boolean; notify_proxy_hosts: boolean; notify_remote_servers: boolean; @@ -23,19 +43,39 @@ export interface NotificationProvider { created_at: string; } -const withDiscordType = (data: Partial): Partial => { - const normalizedType = typeof data.type === 'string' ? data.type.toLowerCase() : undefined; - if (normalizedType !== DISCORD_PROVIDER_TYPE) { - return { ...data, type: DISCORD_PROVIDER_TYPE }; +const sanitizeProviderForWriteAction = (data: Partial): Partial => { + const type = resolveProviderTypeOrThrow(data.type); + const payload: Partial = { + ...data, + type, + }; + + const normalizedToken = typeof payload.gotify_token === 'string' && payload.gotify_token.trim().length > 0 + ? payload.gotify_token.trim() + : typeof payload.token === 'string' && payload.token.trim().length > 0 + ? payload.token.trim() + : undefined; + + delete payload.gotify_token; + + if (type !== 'gotify') { + delete payload.token; + return payload; } - return { ...data, type: DISCORD_PROVIDER_TYPE }; + if (normalizedToken) { + payload.token = normalizedToken; + } else { + delete payload.token; + } + + return payload; }; -const assertDiscordOnlyInput = (data: Partial): void => { - if (typeof data.type === 'string' && data.type.toLowerCase() !== DISCORD_PROVIDER_TYPE) { - throw new Error('Only discord notification providers are supported'); - } +const sanitizeProviderForReadLikeAction = (data: Partial): Partial => { + const payload = sanitizeProviderForWriteAction(data); + delete payload.token; + return payload; }; /** @@ -55,8 +95,7 @@ export const getProviders = async () => { * @throws {AxiosError} If creation fails */ export const createProvider = async (data: Partial) => { - assertDiscordOnlyInput(data); - const response = await client.post('/notifications/providers', withDiscordType(data)); + const response = await client.post('/notifications/providers', sanitizeProviderForWriteAction(data)); return response.data; }; @@ -68,8 +107,7 @@ export const createProvider = async (data: Partial) => { * @throws {AxiosError} If update fails or provider not found */ export const updateProvider = async (id: string, data: Partial) => { - assertDiscordOnlyInput(data); - const response = await client.put(`/notifications/providers/${id}`, withDiscordType(data)); + const response = await client.put(`/notifications/providers/${id}`, sanitizeProviderForWriteAction(data)); return response.data; }; @@ -88,8 +126,7 @@ export const deleteProvider = async (id: string) => { * @throws {AxiosError} If test fails */ export const testProvider = async (provider: Partial) => { - assertDiscordOnlyInput(provider); - await client.post('/notifications/providers/test', withDiscordType(provider)); + await client.post('/notifications/providers/test', sanitizeProviderForReadLikeAction(provider)); }; /** @@ -116,8 +153,7 @@ export interface NotificationTemplate { * @throws {AxiosError} If preview fails */ export const previewProvider = async (provider: Partial, data?: Record) => { - assertDiscordOnlyInput(provider); - const payload: Record = withDiscordType(provider) as Record; + const payload: Record = sanitizeProviderForReadLikeAction(provider) as Record; if (data) payload.data = data; const response = await client.post('/notifications/providers/preview', payload); return response.data; diff --git a/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx b/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx index 61d09a15..52cb1c68 100644 --- a/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx +++ b/frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx @@ -78,14 +78,15 @@ describe('Security Notification Settings on Notifications page', () => { expect(document.querySelector('.fixed.inset-0')).toBeNull(); }); - it('keeps provider setup focused on the Discord webhook flow', async () => { + it('defaults to Discord webhook flow while exposing supported provider modes', async () => { const user = userEvent.setup(); renderPage(); 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']); + expect(Array.from(typeSelect.options).map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook']); + expect(typeSelect.value).toBe('discord'); const webhookInput = screen.getByTestId('provider-url') as HTMLInputElement; expect(webhookInput.placeholder).toContain('discord.com/api/webhooks'); diff --git a/frontend/src/locales/en/translation.json b/frontend/src/locales/en/translation.json index e89e2d99..e300da76 100644 --- a/frontend/src/locales/en/translation.json +++ b/frontend/src/locales/en/translation.json @@ -542,6 +542,9 @@ "providerName": "Name", "urlWebhook": "URL / Webhook", "urlRequired": "URL is required", + "gotifyToken": "Gotify Token", + "gotifyTokenPlaceholder": "Enter new token", + "gotifyTokenWriteOnlyHint": "Token is write-only and only sent on save.", "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 6877cab5..d3344584 100644 --- a/frontend/src/pages/Notifications.tsx +++ b/frontend/src/pages/Notifications.tsx @@ -1,14 +1,22 @@ import { useEffect, useState, type FC } from 'react'; import { useTranslation } from 'react-i18next'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; -import { getProviders, createProvider, updateProvider, deleteProvider, testProvider, getTemplates, previewProvider, NotificationProvider, getExternalTemplates, previewExternalTemplate, ExternalTemplate, createExternalTemplate, updateExternalTemplate, deleteExternalTemplate, NotificationTemplate } from '../api/notifications'; +import { getProviders, createProvider, updateProvider, deleteProvider, testProvider, getTemplates, previewProvider, NotificationProvider, getExternalTemplates, previewExternalTemplate, ExternalTemplate, createExternalTemplate, updateExternalTemplate, deleteExternalTemplate, NotificationTemplate, SUPPORTED_NOTIFICATION_PROVIDER_TYPES, type SupportedNotificationProviderType } from '../api/notifications'; import { Card } from '../components/ui/Card'; import { Button } from '../components/ui/Button'; import { Bell, Plus, Trash2, Edit2, Send, Check, X, Loader2 } from 'lucide-react'; import { useForm } from 'react-hook-form'; import { toast } from '../utils/toast'; -const DISCORD_PROVIDER_TYPE = 'discord' as const; +const DISCORD_PROVIDER_TYPE: SupportedNotificationProviderType = 'discord'; + +const isSupportedProviderType = (providerType: string | undefined): providerType is SupportedNotificationProviderType => { + if (!providerType) { + return false; + } + + return SUPPORTED_NOTIFICATION_PROVIDER_TYPES.includes(providerType.toLowerCase() as SupportedNotificationProviderType); +}; // supportsJSONTemplates returns true if the provider type can use JSON templates const supportsJSONTemplates = (providerType: string | undefined): boolean => { @@ -16,26 +24,44 @@ const supportsJSONTemplates = (providerType: string | undefined): boolean => { return providerType.toLowerCase() === DISCORD_PROVIDER_TYPE; }; -const isNonDiscordProvider = (providerType: string | undefined): boolean => { - if (!providerType) { - return false; - } +const isUnsupportedProviderType = (providerType: string | undefined): boolean => !isSupportedProviderType(providerType); - return providerType.toLowerCase() !== DISCORD_PROVIDER_TYPE; -}; - -const normalizeProviderType = (providerType: string | undefined): typeof DISCORD_PROVIDER_TYPE => { - if (!providerType || providerType.toLowerCase() !== DISCORD_PROVIDER_TYPE) { +const normalizeProviderType = (providerType: string | undefined): SupportedNotificationProviderType => { + if (!isSupportedProviderType(providerType)) { return DISCORD_PROVIDER_TYPE; } - return DISCORD_PROVIDER_TYPE; + return providerType.toLowerCase() as SupportedNotificationProviderType; +}; + +const normalizeProviderPayloadForSubmit = (data: Partial): Partial => { + const type = normalizeProviderType(data.type); + const payload: Partial = { + ...data, + type, + }; + + if (type === 'gotify') { + const normalizedToken = typeof payload.gotify_token === 'string' ? payload.gotify_token.trim() : ''; + + if (normalizedToken.length > 0) { + payload.token = normalizedToken; + } else { + delete payload.token; + } + } else { + delete payload.token; + } + + delete payload.gotify_token; + return payload; }; const defaultProviderValues: Partial = { type: DISCORD_PROVIDER_TYPE, enabled: true, config: '', + gotify_token: '', template: 'minimal', notify_proxy_hosts: true, notify_remote_servers: true, @@ -64,7 +90,7 @@ const ProviderForm: FC<{ useEffect(() => { // Reset form state per open/edit to avoid event checkbox leakage between runs. const normalizedInitialData = initialData - ? { ...defaultProviderValues, ...initialData, type: normalizeProviderType(initialData.type) } + ? { ...defaultProviderValues, ...initialData, type: normalizeProviderType(initialData.type), gotify_token: '' } : defaultProviderValues; reset(normalizedInitialData); @@ -87,7 +113,7 @@ const ProviderForm: FC<{ const handleTest = () => { const formData = watch(); - testMutation.mutate({ ...formData, type: DISCORD_PROVIDER_TYPE } as Partial); + testMutation.mutate({ ...formData, type: normalizeProviderType(formData.type) } as Partial); }; const handlePreview = async () => { @@ -100,7 +126,7 @@ const ProviderForm: FC<{ const res = await previewExternalTemplate(formData.template, undefined, undefined); if (res.parsed) setPreviewContent(JSON.stringify(res.parsed, null, 2)); else setPreviewContent(res.rendered); } else { - const res = await previewProvider({ ...formData, type: DISCORD_PROVIDER_TYPE } as Partial); + const res = await previewProvider({ ...formData, type: normalizeProviderType(formData.type) } as Partial); if (res.parsed) setPreviewContent(JSON.stringify(res.parsed, null, 2)); else setPreviewContent(res.rendered); } } catch (err: unknown) { @@ -109,10 +135,11 @@ const ProviderForm: FC<{ } }; - const type = watch('type'); + const type = normalizeProviderType(watch('type')); + const isGotify = type === 'gotify'; useEffect(() => { - if (type !== DISCORD_PROVIDER_TYPE) { - setValue('type', DISCORD_PROVIDER_TYPE, { shouldDirty: false, shouldTouch: false }); + if (type !== 'gotify') { + setValue('gotify_token', '', { shouldDirty: false, shouldTouch: false }); } }, [type, setValue]); @@ -141,9 +168,9 @@ const ProviderForm: FC<{ }; return ( -
onSubmit({ ...data, type: DISCORD_PROVIDER_TYPE }))} className="space-y-4"> + onSubmit(normalizeProviderPayloadForSubmit(data as Partial)))} className="space-y-4">
- +
- +
- + + {isGotify && ( +
+ + +

{t('notificationProviders.gotifyTokenWriteOnlyHint')}

+
+ )} + {supportsJSONTemplates(type) && (
@@ -563,7 +609,7 @@ const Notifications: FC = () => {
{providers?.map((provider) => ( - {editingId === provider.id && !isNonDiscordProvider(provider.type) ? ( + {editingId === provider.id && !isUnsupportedProviderType(provider.type) ? ( setEditingId(null)} @@ -582,7 +628,7 @@ const Notifications: FC = () => { {t('common.saved')} )} - {isNonDiscordProvider(provider.type) && ( + {isUnsupportedProviderType(provider.type) && (
{
- {!isNonDiscordProvider(provider.type) && ( + {!isUnsupportedProviderType(provider.type) && ( )} - {!isNonDiscordProvider(provider.type) && ( + {!isUnsupportedProviderType(provider.type) && ( diff --git a/frontend/src/pages/__tests__/Notifications.test.tsx b/frontend/src/pages/__tests__/Notifications.test.tsx index d4f2adb8..0d935169 100644 --- a/frontend/src/pages/__tests__/Notifications.test.tsx +++ b/frontend/src/pages/__tests__/Notifications.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { fireEvent, screen, waitFor, within } from '@testing-library/react' +import { screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import Notifications from '../Notifications' import { renderWithQueryClient } from '../../test-utils/renderWithQueryClient' @@ -14,6 +14,7 @@ vi.mock('react-i18next', () => ({ })) vi.mock('../../api/notifications', () => ({ + SUPPORTED_NOTIFICATION_PROVIDER_TYPES: ['discord', 'gotify', 'webhook'], getProviders: vi.fn(), createProvider: vi.fn(), updateProvider: vi.fn(), @@ -62,10 +63,13 @@ const setupMocks = (providers: NotificationProvider[] = []) => { vi.mocked(notificationsApi.updateProvider).mockResolvedValue(baseProvider) } +let user: ReturnType + describe('Notifications', () => { beforeEach(() => { vi.clearAllMocks() setupMocks() + user = userEvent.setup() }) afterEach(() => { @@ -73,7 +77,6 @@ describe('Notifications', () => { }) it('rejects invalid protocol URLs', async () => { - const user = userEvent.setup() renderWithQueryClient() await user.click(await screen.findByTestId('add-provider-btn')) @@ -134,7 +137,7 @@ describe('Notifications', () => { expect(payload.type).toBe('discord') }) - it('shows Discord as the only provider type option', async () => { + it('shows supported provider type options', async () => { const user = userEvent.setup() renderWithQueryClient() @@ -143,21 +146,32 @@ describe('Notifications', () => { const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement const options = Array.from(typeSelect.options) - expect(options).toHaveLength(1) - expect(options[0].value).toBe('discord') - expect(typeSelect.disabled).toBe(true) + expect(options).toHaveLength(3) + expect(options.map((option) => option.value)).toEqual(['discord', 'gotify', 'webhook']) + expect(typeSelect.disabled).toBe(false) }) - it('normalizes stale non-discord type to discord on submit', async () => { + it('associates provider type label with select control', async () => { const user = userEvent.setup() renderWithQueryClient() await user.click(await screen.findByTestId('add-provider-btn')) + + const typeSelect = screen.getByTestId('provider-type') + expect(typeSelect).toHaveAttribute('id', 'provider-type') + expect(screen.getByLabelText('common.type')).toBe(typeSelect) + }) + + it('submits selected provider type without forcing discord', async () => { + renderWithQueryClient() + + await user.click(await screen.findByTestId('add-provider-btn')) + await user.selectOptions(screen.getByTestId('provider-type'), 'webhook') await user.type(screen.getByTestId('provider-name'), 'Normalized Provider') await user.type(screen.getByTestId('provider-url'), 'https://example.com/webhook') const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement - expect(typeSelect.value).toBe('discord') + expect(typeSelect.value).toBe('webhook') await user.click(screen.getByTestId('provider-save-btn')) @@ -166,7 +180,7 @@ describe('Notifications', () => { }) const payload = vi.mocked(notificationsApi.createProvider).mock.calls[0][0] - expect(payload.type).toBe('discord') + expect(payload.type).toBe('webhook') }) it('shows and hides the update indicator after save', async () => { @@ -324,11 +338,53 @@ describe('Notifications', () => { 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']) + expect(typeSelect.value).toBe('discord') expect(screen.getByTestId('provider-url')).toHaveAttribute('placeholder', 'https://discord.com/api/webhooks/...') expect(screen.queryByRole('link')).toBeNull() }) + it('submits gotify token on create for gotify provider mode', async () => { + const user = userEvent.setup() + renderWithQueryClient() + + await user.click(await screen.findByTestId('add-provider-btn')) + await user.selectOptions(screen.getByTestId('provider-type'), 'gotify') + await user.type(screen.getByTestId('provider-name'), 'Gotify Alerts') + await user.type(screen.getByTestId('provider-url'), 'https://gotify.example.com/message') + await user.type(screen.getByTestId('provider-gotify-token'), 'super-secret-token') + await user.click(screen.getByTestId('provider-save-btn')) + + await waitFor(() => { + expect(notificationsApi.createProvider).toHaveBeenCalled() + }) + + const payload = vi.mocked(notificationsApi.createProvider).mock.calls[0][0] + expect(payload.type).toBe('gotify') + expect(payload.token).toBe('super-secret-token') + }) + + it('uses masked gotify token input and never pre-fills token on edit', async () => { + const gotifyProvider: NotificationProvider = { + ...baseProvider, + id: 'provider-gotify', + type: 'gotify', + url: 'https://gotify.example.com/message', + } + + setupMocks([gotifyProvider]) + + const user = userEvent.setup() + renderWithQueryClient() + + const row = await screen.findByTestId('provider-row-provider-gotify') + const buttons = within(row).getAllByRole('button') + await user.click(buttons[1]) + + const tokenInput = screen.getByTestId('provider-gotify-token') as HTMLInputElement + expect(tokenInput.type).toBe('password') + expect(tokenInput.value).toBe('') + }) + it('renders external template action buttons and skips delete when confirm is cancelled', async () => { const template = { id: 'template-cancel', @@ -425,7 +481,7 @@ describe('Notifications', () => { }) }) - it('treats empty legacy type as editable and enforces discord type in form', async () => { + it('treats empty legacy type as unsupported and keeps row read-only', async () => { const emptyTypeProvider: NotificationProvider = { ...baseProvider, id: 'provider-empty-type', @@ -434,23 +490,12 @@ describe('Notifications', () => { setupMocks([emptyTypeProvider]) - const user = userEvent.setup() renderWithQueryClient() const row = await screen.findByTestId('provider-row-provider-empty-type') const buttons = within(row).getAllByRole('button') - expect(buttons).toHaveLength(3) - - await user.click(buttons[1]) - - const typeSelect = screen.getByTestId('provider-type') as HTMLSelectElement - expect(typeSelect.value).toBe('discord') - - fireEvent.change(typeSelect, { target: { value: 'slack' } }) - - await waitFor(() => { - expect(typeSelect.value).toBe('discord') - }) + expect(buttons).toHaveLength(1) + expect(screen.getByTestId('provider-deprecated-status-provider-empty-type')).toHaveTextContent('notificationProviders.deprecatedReadOnly') }) it('triggers row-level send test action with discord payload', async () => { diff --git a/tests/settings/notifications-payload.spec.ts b/tests/settings/notifications-payload.spec.ts new file mode 100644 index 00000000..aa1741cb --- /dev/null +++ b/tests/settings/notifications-payload.spec.ts @@ -0,0 +1,553 @@ +import { test, expect, loginUser } from '../fixtures/auth-fixtures'; +import { request as playwrightRequest } from '@playwright/test'; +import { waitForLoadingComplete } from '../utils/wait-helpers'; + +const SETTINGS_FLAGS_ENDPOINT = '/api/v1/settings'; +const PROVIDERS_ENDPOINT = '/api/v1/notifications/providers'; + +function buildDiscordProviderPayload(name: string) { + return { + name, + type: 'discord', + url: 'https://discord.com/api/webhooks/123456789/testtoken', + enabled: true, + notify_proxy_hosts: true, + notify_remote_servers: false, + notify_domains: false, + notify_certs: true, + notify_uptime: false, + notify_security_waf_blocks: false, + notify_security_acl_denies: false, + notify_security_rate_limit_hits: false, + }; +} + +async function enableNotifyDispatchFlags(page: import('@playwright/test').Page, token: string) { + const keys = [ + 'feature.notifications.service.gotify.enabled', + 'feature.notifications.service.webhook.enabled', + ]; + + for (const key of keys) { + const response = await page.request.post(SETTINGS_FLAGS_ENDPOINT, { + headers: { Authorization: `Bearer ${token}` }, + data: { + key, + value: 'true', + category: 'feature', + type: 'bool', + }, + }); + + expect(response.ok()).toBeTruthy(); + } +} + +test.describe('Notifications Payload Matrix', () => { + test.beforeEach(async ({ page, adminUser }) => { + await loginUser(page, adminUser); + await waitForLoadingComplete(page); + await page.goto('/settings/notifications'); + await waitForLoadingComplete(page); + }); + + test('valid payload flows for discord, gotify, and webhook', async ({ page }) => { + const createdProviders: Array> = []; + const capturedCreatePayloads: Array> = []; + + await test.step('Mock providers create/list endpoints', 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(createdProviders), + }); + return; + } + + if (request.method() === 'POST') { + const payload = (await request.postDataJSON()) as Record; + capturedCreatePayloads.push(payload); + const created = { + id: `provider-${capturedCreatePayloads.length}`, + ...payload, + }; + createdProviders.push(created); + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify(created), + }); + return; + } + + await route.continue(); + }); + }); + + const scenarios = [ + { + type: 'discord', + name: `discord-matrix-${Date.now()}`, + url: 'https://discord.com/api/webhooks/123/discordtoken', + }, + { + type: 'gotify', + name: `gotify-matrix-${Date.now()}`, + url: 'https://gotify.example.com/message', + }, + { + type: 'webhook', + name: `webhook-matrix-${Date.now()}`, + url: 'https://example.com/notify', + }, + ] as const; + + for (const scenario of scenarios) { + await test.step(`Create ${scenario.type} provider and capture outgoing payload`, async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + + await page.getByTestId('provider-name').fill(scenario.name); + await page.getByTestId('provider-type').selectOption(scenario.type); + await page.getByTestId('provider-url').fill(scenario.url); + + if (scenario.type === 'gotify') { + await page.getByTestId('provider-gotify-token').fill(' gotify-secret-token '); + } + + await page.getByTestId('provider-save-btn').click(); + }); + } + + await test.step('Verify payload contract per provider type', async () => { + expect(capturedCreatePayloads).toHaveLength(3); + + const discordPayload = capturedCreatePayloads.find((payload) => payload.type === 'discord'); + expect(discordPayload).toBeTruthy(); + expect(discordPayload?.token).toBeUndefined(); + expect(discordPayload?.gotify_token).toBeUndefined(); + + const gotifyPayload = capturedCreatePayloads.find((payload) => payload.type === 'gotify'); + expect(gotifyPayload).toBeTruthy(); + expect(gotifyPayload?.token).toBe('gotify-secret-token'); + expect(gotifyPayload?.gotify_token).toBeUndefined(); + + const webhookPayload = capturedCreatePayloads.find((payload) => payload.type === 'webhook'); + expect(webhookPayload).toBeTruthy(); + expect(webhookPayload?.token).toBeUndefined(); + expect(typeof webhookPayload?.config).toBe('string'); + }); + }); + + test('malformed payload scenarios return sanitized validation errors', async ({ page }) => { + await test.step('Malformed JSON to preview endpoint returns INVALID_REQUEST', async () => { + const response = await page.request.post('/api/v1/notifications/providers/preview', { + headers: { 'Content-Type': 'application/json' }, + data: '{"type":', + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('INVALID_REQUEST'); + expect(body.category).toBe('validation'); + }); + + await test.step('Malformed template content returns TEMPLATE_PREVIEW_FAILED', async () => { + const response = await page.request.post('/api/v1/notifications/providers/preview', { + data: { + type: 'webhook', + url: 'https://example.com/notify', + template: 'custom', + config: '{"message": {{.Message}', + }, + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('TEMPLATE_PREVIEW_FAILED'); + expect(body.category).toBe('validation'); + }); + }); + + test('missing required fields block submit and show validation', async ({ page }) => { + let createCalled = false; + + await test.step('Prevent create call from being silently sent', async () => { + await page.route('**/api/v1/notifications/providers', async (route, request) => { + if (request.method() === 'POST') { + createCalled = true; + } + + await route.continue(); + }); + }); + + await test.step('Submit empty provider form', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await page.getByTestId('provider-save-btn').click(); + }); + + await test.step('Validate required field errors and no outbound create', async () => { + await expect(page.getByTestId('provider-url-error')).toBeVisible(); + await expect(page.getByTestId('provider-name')).toHaveAttribute('aria-invalid', 'true'); + expect(createCalled).toBeFalsy(); + }); + }); + + test('auth/header behavior checks for protected settings endpoint', async ({ page, adminUser }) => { + const providerName = `auth-check-${Date.now()}`; + let providerID = ''; + + await test.step('Protected settings write rejects invalid bearer token', async () => { + const unauthenticatedRequest = await playwrightRequest.newContext({ + baseURL: process.env.PLAYWRIGHT_BASE_URL || 'http://127.0.0.1:8080', + }); + + try { + const noAuthResponse = await unauthenticatedRequest.post(SETTINGS_FLAGS_ENDPOINT, { + headers: { Authorization: 'Bearer invalid-token' }, + data: { + key: 'feature.notifications.service.webhook.enabled', + value: 'true', + category: 'feature', + type: 'bool', + }, + }); + + expect([401, 403]).toContain(noAuthResponse.status()); + } finally { + await unauthenticatedRequest.dispose(); + } + }); + + await test.step('Create provider with bearer token succeeds', async () => { + const authResponse = await page.request.post(PROVIDERS_ENDPOINT, { + headers: { Authorization: `Bearer ${adminUser.token}` }, + data: buildDiscordProviderPayload(providerName), + }); + + expect(authResponse.status()).toBe(201); + const created = (await authResponse.json()) as Record; + providerID = String(created.id ?? ''); + expect(providerID.length).toBeGreaterThan(0); + }); + + await test.step('Cleanup created provider', async () => { + const deleteResponse = await page.request.delete(`${PROVIDERS_ENDPOINT}/${providerID}`, { + headers: { Authorization: `Bearer ${adminUser.token}` }, + }); + + expect(deleteResponse.ok()).toBeTruthy(); + }); + }); + + test('provider-specific transformation strips gotify token from test and preview payloads', async ({ page }) => { + let capturedPreviewPayload: Record | null = null; + let capturedTestPayload: Record | null = null; + + await test.step('Mock preview and test endpoints to capture payloads', async () => { + await page.route('**/api/v1/notifications/providers/preview', async (route, request) => { + capturedPreviewPayload = (await request.postDataJSON()) as Record; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ rendered: '{"ok":true}', parsed: { ok: true } }), + }); + }); + + await page.route('**/api/v1/notifications/providers/test', async (route, request) => { + capturedTestPayload = (await request.postDataJSON()) as Record; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ message: 'Test notification sent' }), + }); + }); + }); + + await test.step('Fill gotify form with write-only token', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await page.getByTestId('provider-type').selectOption('gotify'); + await page.getByTestId('provider-name').fill(`gotify-transform-${Date.now()}`); + await page.getByTestId('provider-url').fill('https://gotify.example.com/message'); + await page.getByTestId('provider-gotify-token').fill('super-secret-token'); + }); + + await test.step('Trigger preview and test calls', async () => { + await page.getByTestId('provider-preview-btn').click(); + await page.getByTestId('provider-test-btn').click(); + }); + + await test.step('Assert token is not sent on preview/test payloads', async () => { + expect(capturedPreviewPayload).toBeTruthy(); + expect(capturedPreviewPayload?.type).toBe('gotify'); + expect(capturedPreviewPayload?.token).toBeUndefined(); + expect(capturedPreviewPayload?.gotify_token).toBeUndefined(); + + expect(capturedTestPayload).toBeTruthy(); + expect(capturedTestPayload?.type).toBe('gotify'); + expect(capturedTestPayload?.token).toBeUndefined(); + expect(capturedTestPayload?.gotify_token).toBeUndefined(); + }); + }); + + test('security: SSRF redirect/internal target, query-token, and oversized payload are blocked', async ({ page, adminUser }) => { + await test.step('Enable gotify and webhook dispatch feature flags', async () => { + await enableNotifyDispatchFlags(page, adminUser.token); + }); + + await test.step('Redirect/internal SSRF-style target is blocked', async () => { + const response = await page.request.post('/api/v1/notifications/providers/test', { + data: { + type: 'webhook', + name: 'ssrf-test', + url: 'https://127.0.0.1/internal', + template: 'custom', + config: '{"message":"{{.Message}}"}', + }, + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('PROVIDER_TEST_FAILED'); + expect(body.category).toBe('dispatch'); + expect(String(body.error ?? '')).not.toContain('127.0.0.1'); + }); + + await test.step('Gotify query-token URL is rejected with sanitized error', async () => { + const queryToken = 's3cr3t-query-token'; + const response = await page.request.post('/api/v1/notifications/providers/test', { + data: { + type: 'gotify', + name: 'query-token-test', + url: `https://gotify.example.com/message?token=${queryToken}`, + template: 'custom', + config: '{"message":"{{.Message}}"}', + }, + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('PROVIDER_TEST_FAILED'); + expect(body.category).toBe('dispatch'); + + const responseText = JSON.stringify(body); + expect(responseText).not.toContain(queryToken); + expect(responseText.toLowerCase()).not.toContain('token='); + }); + + await test.step('Oversized payload/template is rejected', async () => { + const oversizedTemplate = `{"message":"${'x'.repeat(12_500)}"}`; + const response = await page.request.post('/api/v1/notifications/providers/test', { + data: { + type: 'webhook', + name: 'oversized-template-test', + url: 'https://example.com/webhook', + template: 'custom', + config: oversizedTemplate, + }, + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('PROVIDER_TEST_FAILED'); + expect(body.category).toBe('dispatch'); + }); + }); + + test('security: DNS-rebinding-observable hostname path is blocked with sanitized response', async ({ page, adminUser }) => { + await test.step('Enable gotify and webhook dispatch feature flags', async () => { + await enableNotifyDispatchFlags(page, adminUser.token); + }); + + await test.step('Hostname resolving to loopback is blocked (E2E-observable rebinding guard path)', async () => { + const blockedHostname = 'rebind-check.127.0.0.1.nip.io'; + const response = await page.request.post('/api/v1/notifications/providers/test', { + data: { + type: 'webhook', + name: 'dns-rebinding-observable', + url: `https://${blockedHostname}/notify`, + template: 'custom', + config: '{"message":"{{.Message}}"}', + }, + }); + + expect(response.status()).toBe(400); + const body = (await response.json()) as Record; + expect(body.code).toBe('PROVIDER_TEST_FAILED'); + expect(body.category).toBe('dispatch'); + + const responseText = JSON.stringify(body); + expect(responseText).not.toContain(blockedHostname); + expect(responseText).not.toContain('127.0.0.1'); + }); + }); + + test('security: retry split distinguishes retryable and non-retryable failures with deterministic response semantics', async ({ page }) => { + const capturedTestPayloads: Array> = []; + let nonRetryableBody: Record | null = null; + let retryableBody: Record | null = null; + + await test.step('Stub provider test endpoint with deterministic retry split contract', async () => { + await page.route('**/api/v1/notifications/providers/test', async (route, request) => { + const payload = (await request.postDataJSON()) as Record; + capturedTestPayloads.push(payload); + + const scenarioName = String(payload.name ?? ''); + const isRetryable = scenarioName.includes('retryable') && !scenarioName.includes('non-retryable'); + const requestID = isRetryable ? 'stub-request-retryable' : 'stub-request-non-retryable'; + + await route.fulfill({ + status: 400, + contentType: 'application/json', + body: JSON.stringify({ + code: 'PROVIDER_TEST_FAILED', + category: 'dispatch', + error: 'Provider test failed', + request_id: requestID, + retryable: isRetryable, + }), + }); + }); + }); + + await test.step('Open provider form and execute deterministic non-retryable test call', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await page.getByTestId('provider-type').selectOption('webhook'); + await page.getByTestId('provider-name').fill('retry-split-non-retryable'); + await page.getByTestId('provider-url').fill('https://non-retryable.example.invalid/notify'); + + const nonRetryableResponsePromise = page.waitForResponse( + (response) => + /\/api\/v1\/notifications\/providers\/test$/.test(response.url()) + && response.request().method() === 'POST' + && (response.request().postData() ?? '').includes('retry-split-non-retryable') + ); + + await page.getByTestId('provider-test-btn').click(); + const nonRetryableResponse = await nonRetryableResponsePromise; + nonRetryableBody = (await nonRetryableResponse.json()) as Record; + + expect(nonRetryableResponse.status()).toBe(400); + expect(nonRetryableBody.code).toBe('PROVIDER_TEST_FAILED'); + expect(nonRetryableBody.category).toBe('dispatch'); + expect(nonRetryableBody.error).toBe('Provider test failed'); + expect(nonRetryableBody.retryable).toBe(false); + expect(nonRetryableBody.request_id).toBe('stub-request-non-retryable'); + }); + + await test.step('Execute deterministic retryable test call on the same contract endpoint', async () => { + await page.getByTestId('provider-name').fill('retry-split-retryable'); + await page.getByTestId('provider-url').fill('https://retryable.example.invalid/notify'); + + const retryableResponsePromise = page.waitForResponse( + (response) => + /\/api\/v1\/notifications\/providers\/test$/.test(response.url()) + && response.request().method() === 'POST' + && (response.request().postData() ?? '').includes('retry-split-retryable') + ); + + await page.getByTestId('provider-test-btn').click(); + const retryableResponse = await retryableResponsePromise; + retryableBody = (await retryableResponse.json()) as Record; + + expect(retryableResponse.status()).toBe(400); + expect(retryableBody.code).toBe('PROVIDER_TEST_FAILED'); + expect(retryableBody.category).toBe('dispatch'); + expect(retryableBody.error).toBe('Provider test failed'); + expect(retryableBody.retryable).toBe(true); + expect(retryableBody.request_id).toBe('stub-request-retryable'); + }); + + await test.step('Assert stable split distinction and sanitized API contract shape', async () => { + expect(capturedTestPayloads).toHaveLength(2); + + expect(capturedTestPayloads[0]?.name).toBe('retry-split-non-retryable'); + expect(capturedTestPayloads[1]?.name).toBe('retry-split-retryable'); + + expect(nonRetryableBody).toMatchObject({ + code: 'PROVIDER_TEST_FAILED', + category: 'dispatch', + error: 'Provider test failed', + retryable: false, + }); + expect(retryableBody).toMatchObject({ + code: 'PROVIDER_TEST_FAILED', + category: 'dispatch', + error: 'Provider test failed', + retryable: true, + }); + + test.info().annotations.push({ + type: 'retry-split-semantics', + description: 'non-retryable and retryable contracts are validated via deterministic route-stubbed /providers/test responses', + }); + }); + }); + + test('security: token does not leak in list and visible edit surfaces', async ({ page, adminUser }) => { + const name = `gotify-redaction-${Date.now()}`; + let providerID = ''; + + await test.step('Create gotify provider with token on write path', async () => { + const createResponse = await page.request.post(PROVIDERS_ENDPOINT, { + headers: { Authorization: `Bearer ${adminUser.token}` }, + data: { + ...buildDiscordProviderPayload(name), + type: 'gotify', + url: 'https://gotify.example.com/message', + token: 'write-only-secret-token', + config: '{"message":"{{.Message}}"}', + }, + }); + + expect(createResponse.status()).toBe(201); + const created = (await createResponse.json()) as Record; + providerID = String(created.id ?? ''); + expect(providerID.length).toBeGreaterThan(0); + }); + + await test.step('List providers does not expose token fields', async () => { + const listResponse = await page.request.get(PROVIDERS_ENDPOINT, { + headers: { Authorization: `Bearer ${adminUser.token}` }, + }); + expect(listResponse.ok()).toBeTruthy(); + + const providers = (await listResponse.json()) as Array>; + const gotify = providers.find((provider) => provider.id === providerID); + expect(gotify).toBeTruthy(); + expect(gotify?.token).toBeUndefined(); + expect(gotify?.gotify_token).toBeUndefined(); + }); + + await test.step('Edit form does not pre-fill token in visible surface', async () => { + await page.reload(); + await waitForLoadingComplete(page); + + const row = page.getByTestId(`provider-row-${providerID}`); + await expect(row).toBeVisible({ timeout: 10000 }); + + const testButton = row.getByRole('button', { name: /send test notification/i }); + await expect(testButton).toBeVisible(); + await testButton.focus(); + await page.keyboard.press('Tab'); + await page.keyboard.press('Enter'); + + const tokenInput = page.getByTestId('provider-gotify-token'); + await expect(tokenInput).toBeVisible(); + await expect(tokenInput).toHaveValue(''); + + const pageText = await page.locator('main').innerText(); + expect(pageText).not.toContain('write-only-secret-token'); + }); + + await test.step('Cleanup created provider', async () => { + const deleteResponse = await page.request.delete(`${PROVIDERS_ENDPOINT}/${providerID}`, { + headers: { Authorization: `Bearer ${adminUser.token}` }, + }); + + expect(deleteResponse.ok()).toBeTruthy(); + }); + }); +}); diff --git a/tests/settings/notifications.spec.ts b/tests/settings/notifications.spec.ts index 50d9f7d8..3ed915b4 100644 --- a/tests/settings/notifications.spec.ts +++ b/tests/settings/notifications.spec.ts @@ -123,10 +123,8 @@ test.describe('Notification Providers', () => { }); await test.step('Verify empty state message', async () => { - const emptyState = page.getByText(/no.*providers|no notification providers/i) - .or(page.locator('.border-dashed')); - - await expect(emptyState.first()).toBeVisible({ timeout: 5000 }); + const emptyState = page.getByText(/no notification providers configured\.?/i); + await expect(emptyState).toBeVisible({ timeout: 5000 }); }); }); @@ -159,7 +157,7 @@ test.describe('Notification Providers', () => { }); await test.step('Verify Discord type badge', async () => { - const discordBadge = page.locator('span').filter({ hasText: /discord/i }).first(); + const discordBadge = page.getByTestId('provider-row-1').getByText(/^discord$/i); await expect(discordBadge).toBeVisible(); }); @@ -243,7 +241,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form', async () => { await page.getByTestId('provider-name').fill(providerName); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/12345/abcdef'); }); @@ -278,10 +275,10 @@ test.describe('Notification Providers', () => { }); /** - * Test: Form only offers Discord provider type + * Test: Form offers supported provider types * Priority: P0 */ - test('should offer only Discord provider type option in form', async ({ page }) => { + test('should offer supported provider type options in form', async ({ page }) => { await test.step('Click Add Provider button', async () => { const addButton = page.getByRole('button', { name: /add.*provider/i }); @@ -295,11 +292,11 @@ test.describe('Notification Providers', () => { await expect(nameInput).toBeVisible({ timeout: 5000 }); }); - await test.step('Verify provider type select contains only Discord option', async () => { + await test.step('Verify provider type select contains supported options', async () => { const providerTypeSelect = page.getByTestId('provider-type'); - await expect(providerTypeSelect.locator('option')).toHaveCount(1); - await expect(providerTypeSelect.locator('option')).toHaveText(/discord/i); - await expect(providerTypeSelect).toBeDisabled(); + await expect(providerTypeSelect.locator('option')).toHaveCount(3); + await expect(providerTypeSelect.locator('option')).toHaveText(['Discord', 'Gotify', 'Generic Webhook']); + await expect(providerTypeSelect).toBeEnabled(); }); }); @@ -407,14 +404,15 @@ test.describe('Notification Providers', () => { }); await test.step('Click edit button on provider', async () => { - // Find the provider card and click its edit button - const providerText = page.getByText('Original Provider').first(); - const providerCard = providerText.locator('..').locator('..').locator('..'); + const providerRow = page.getByTestId('provider-row-test-edit-id'); + const sendTestButton = providerRow.getByRole('button', { name: /send test/i }); - // The edit button is typically the second icon button (after test button) - const editButton = providerCard.getByRole('button').filter({ has: page.locator('svg') }).nth(1); - await expect(editButton).toBeVisible({ timeout: 5000 }); - await editButton.click(); + 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('Modify provider name', async () => { @@ -635,7 +633,6 @@ test.describe('Notification Providers', () => { await test.step('Fill form with invalid URL', async () => { await page.getByTestId('provider-name').fill(providerName); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('not-a-valid-url'); }); @@ -702,7 +699,6 @@ test.describe('Notification Providers', () => { await test.step('Leave name empty and fill other fields', async () => { await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/test/token'); }); @@ -754,7 +750,6 @@ test.describe('Notification Providers', () => { await test.step('Select provider type that supports templates', async () => { await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); }); await test.step('Select minimal template button', async () => { @@ -792,29 +787,9 @@ test.describe('Notification Providers', () => { }); await test.step('Click New Template button in the template management area', async () => { - // Look specifically for buttons in the template management section - // Find ALL buttons that mention "template" and pick the one that has a Plus icon or is a "new" button - const allButtons = page.getByRole('button'); - let found = false; - - // Try to find the "New Template" button by looking at multiple patterns - const newTemplateBtn = allButtons.filter({ hasText: /new.*template|create.*template|add.*template/i }).first(); - - if (await newTemplateBtn.isVisible({ timeout: 3000 }).catch(() => false)) { - await newTemplateBtn.click(); - found = true; - } else { - // Fallback: Try to find it by looking for the button with Plus icon that opens template management - const templateMgmtButtons = page.locator('div').filter({ hasText: /external.*templates/i }).locator('button'); - const createButton = templateMgmtButtons.last(); // Typically the "New Template" button is the last one in the section - - if (await createButton.isVisible({ timeout: 3000 }).catch(() => false)) { - await createButton.click(); - found = true; - } - } - - expect(found).toBeTruthy(); + const newTemplateBtn = page.getByRole('button', { name: /new template/i }); + await expect(newTemplateBtn).toBeVisible({ timeout: 5000 }); + await newTemplateBtn.click(); }); await test.step('Wait for template form to appear in the page', async () => { @@ -854,10 +829,7 @@ test.describe('Notification Providers', () => { }); await test.step('Click New Template button', async () => { - // Find and click the 'New Template' button - const newTemplateBtn = page.getByRole('button').filter({ - hasText: /new.*template|add.*template/i - }).last(); + const newTemplateBtn = page.getByRole('button', { name: /new template/i }); await expect(newTemplateBtn).toBeVisible({ timeout: 5000 }); await newTemplateBtn.click(); }); @@ -1119,7 +1091,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form', async () => { await page.getByTestId('provider-name').fill('Test Provider'); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/test/token'); }); @@ -1177,7 +1148,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form', async () => { await page.getByTestId('provider-name').fill('Success Test Provider'); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/success/test'); }); @@ -1217,7 +1187,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form', async () => { await page.getByTestId('provider-name').fill('Preview Provider'); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/preview/test'); const configTextarea = page.getByTestId('provider-config'); @@ -1263,6 +1232,103 @@ test.describe('Notification Providers', () => { expect(previewText).toContain('alert'); }); }); + + test('should preserve Discord request payload contract for save, preview, and test', async ({ page }) => { + const providerName = generateProviderName('discord-regression'); + const discordURL = 'https://discord.com/api/webhooks/regression/token'; + let capturedCreatePayload: Record | null = null; + let capturedPreviewPayload: Record | null = null; + let capturedTestPayload: Record | null = null; + const providers: Array> = []; + + await test.step('Mock provider list/create and preview/test endpoints', 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(providers), + }); + return; + } + + if (request.method() === 'POST') { + capturedCreatePayload = (await request.postDataJSON()) as Record; + const created = { + id: 'discord-regression-id', + ...capturedCreatePayload, + }; + providers.splice(0, providers.length, created); + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify(created), + }); + return; + } + + await route.continue(); + }); + + await page.route('**/api/v1/notifications/providers/preview', async (route, request) => { + capturedPreviewPayload = (await request.postDataJSON()) as Record; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ rendered: '{"content":"ok"}', parsed: { content: 'ok' } }), + }); + }); + + await page.route('**/api/v1/notifications/providers/test', async (route, request) => { + capturedTestPayload = (await request.postDataJSON()) as Record; + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ message: 'Test notification sent successfully' }), + }); + }); + }); + + await test.step('Open add provider form and verify accessible form structure', async () => { + await page.getByRole('button', { name: /add.*provider/i }).click(); + await expect(page.getByTestId('provider-name')).toBeVisible(); + await expect(page.getByLabel('Name')).toBeVisible(); + await expect(page.getByLabel('Type')).toBeVisible(); + await expect(page.getByLabel(/URL \/ Webhook/i)).toBeVisible(); + await expect(page.getByTestId('provider-preview-btn')).toBeVisible(); + await expect(page.getByTestId('provider-test-btn')).toBeVisible(); + await expect(page.getByTestId('provider-save-btn')).toBeVisible(); + }); + + await test.step('Submit preview and test from Discord form', async () => { + await page.getByTestId('provider-name').fill(providerName); + await expect(page.getByTestId('provider-type')).toHaveValue('discord'); + await page.getByTestId('provider-url').fill(discordURL); + await page.getByTestId('provider-preview-btn').click(); + await page.getByTestId('provider-test-btn').click(); + }); + + await test.step('Save Discord provider', async () => { + await page.getByTestId('provider-save-btn').click(); + }); + + await test.step('Assert Discord payload contract remained unchanged', async () => { + expect(capturedPreviewPayload).toBeTruthy(); + expect(capturedPreviewPayload?.type).toBe('discord'); + expect(capturedPreviewPayload?.url).toBe(discordURL); + expect(capturedPreviewPayload?.token).toBeUndefined(); + + expect(capturedTestPayload).toBeTruthy(); + expect(capturedTestPayload?.type).toBe('discord'); + expect(capturedTestPayload?.url).toBe(discordURL); + expect(capturedTestPayload?.token).toBeUndefined(); + + expect(capturedCreatePayload).toBeTruthy(); + expect(capturedCreatePayload?.type).toBe('discord'); + expect(capturedCreatePayload?.url).toBe(discordURL); + expect(capturedCreatePayload?.token).toBeUndefined(); + }); + }); }); test.describe('Event Selection', () => { @@ -1395,7 +1461,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form with specific events', async () => { await page.getByTestId('provider-name').fill(providerName); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/events/test'); // Configure specific events @@ -1606,7 +1671,6 @@ test.describe('Notification Providers', () => { await test.step('Fill provider form', async () => { await page.getByTestId('provider-name').fill('Error Test Provider'); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/invalid'); }); @@ -1652,7 +1716,6 @@ test.describe('Notification Providers', () => { await test.step('Fill form with invalid JSON config', async () => { await page.getByTestId('provider-name').fill('Invalid Template Provider'); await expect(page.getByTestId('provider-type')).toHaveValue('discord'); - await expect(page.getByTestId('provider-type')).toBeDisabled(); await page.getByTestId('provider-url').fill('https://discord.com/api/webhooks/invalid/template'); const configTextarea = page.getByTestId('provider-config');