diff --git a/.github/workflows/public-docs.yml b/.github/workflows/public-docs.yml index 4feef109e1..700293558e 100644 --- a/.github/workflows/public-docs.yml +++ b/.github/workflows/public-docs.yml @@ -17,6 +17,8 @@ on: - "scripts/tests/test_agent_profile_docs.py" - "scripts/tests/test_pbs_docs.py" - "scripts/tests/test_audit_recovery_docs.py" + - "scripts/tests/test_notification_troubleshooting_docs.py" + - "scripts/tests/test_webhook_verification_docs.py" - "scripts/tests/test_pve_backup_troubleshooting_docs.py" - "scripts/tests/test_vm_disk_diagnostics.py" - "scripts/tests/test_memory_troubleshooting_docs.py" @@ -42,6 +44,8 @@ on: - "scripts/tests/test_agent_profile_docs.py" - "scripts/tests/test_pbs_docs.py" - "scripts/tests/test_audit_recovery_docs.py" + - "scripts/tests/test_notification_troubleshooting_docs.py" + - "scripts/tests/test_webhook_verification_docs.py" - "scripts/tests/test_pve_backup_troubleshooting_docs.py" - "scripts/tests/test_vm_disk_diagnostics.py" - "scripts/tests/test_memory_troubleshooting_docs.py" @@ -90,6 +94,8 @@ jobs: test_agent_profile_docs.py \ test_pbs_docs.py \ test_audit_recovery_docs.py \ + test_notification_troubleshooting_docs.py \ + test_webhook_verification_docs.py \ test_pve_backup_troubleshooting_docs.py \ test_vm_disk_diagnostics.py \ test_memory_troubleshooting_docs.py \ diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 4c777f9b3a..99c1d7002e 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -923,7 +923,8 @@ when they were queued.** Editing a URL, recipient, credential, header or templat does not replace that saved configuration. A test uses the edited settings; retrying an old delivery can still use the old endpoint or credential and fail again. If the old destination must no longer receive data, disable it rather -than relying on a URL edit to redirect queued work. +than relying on a URL edit to redirect queued work. This is not a recall of +requests already sent; see the [pause and cancellation limits](#pause-and-cancellation-limits). **Retry retained deliveries** gives all retained terminal failures a fresh retry budget, not just the destination you tested. Use it only when sending those @@ -940,6 +941,33 @@ Dismissal clears the warning without retrying them; delivery history remains. Neither action deletes the audit trail. Do not delete `notification_queue.db` or audit data to clear the warning. +#### Pause and cancellation limits + +In **Alerts → Notifications**, global **Notifications paused** is not a +store-and-forward hold. Monitoring continues, but pausing clears buffered alert +groups and cancels pending queue deliveries for email, webhooks and Apprise, +including recovery deliveries. **Turning delivery back on does not replay those +cancelled deliveries.** Retained terminal failures are separate: pausing does +not dismiss them or repair their saved destination settings. + +Disabling or removing an individual destination prevents a queued delivery that +sees that policy from sending. A policy-skipped item is cancelled, not a +successful provider delivery, and **Retry retained deliveries** does not replay +cancelled items. Re-enabling a destination is not proof of catch-up or receipt. +For a scheduled quiet period, use [Quiet hours](CONFIGURATION.md#quiet-hours-and-notification-holds) +instead, reviewing its critical-alert exceptions and later re-evaluation; it +still does not guarantee that every held message will be sent. + +**Pause, disable and removal do not guarantee cancellation of a request already +in flight.** The receiver may accept it after the settings change. Neither a +paused banner nor a cancelled queue row proves that the destination received no +data, and Pulse cannot recall a message or delete the receiver's copy. Reconcile +the original time with **Recent delivery activity** and the receiver's existing +record, keeping both private; do not send another Test or replay a queue to +check containment. If a destination credential was exposed, revoke it with +the provider too: editing its URL or turning delivery off is not credential +revocation and cannot undo an earlier disclosure. + #### Emails not sending Follow [retained-failure recovery](#recover-retained-delivery-failures) first. diff --git a/docs/WEBHOOKS.md b/docs/WEBHOOKS.md index 27184ed561..1a2e26830a 100644 --- a/docs/WEBHOOKS.md +++ b/docs/WEBHOOKS.md @@ -150,6 +150,15 @@ batch retries to diagnose rate limiting. A receiver can see a duplicate if an earlier request was accepted but Pulse did not receive its response; preserve [receiver deduplication](#receiver-correlation-and-deduplication). +Global **Notifications paused** cancels pending alert deliveries and clears +buffered groups; turning delivery back on does not replay those cancelled +items. This differs from scheduled quiet-hours holds. Disabling or removing a +destination can block queued work, but **does not recall a request already in +flight**, revoke a provider credential or delete the receiver's copy. Follow +the [pause and cancellation limits](TROUBLESHOOTING.md#pause-and-cancellation-limits) +before treating a paused banner or cancelled row as containment. Alert delivery +controls do not stop the separate [audit forwarding route](#-audit-webhooks-prolegacy-procloud). + **Correlation header.** Alert webhooks carry `X-Pulse-Event-ID` in the form `:` (e.g. `a1b2c3:alert`, `a1b2c3:resolved`). Retries retain it, but later occurrences, severity changes and reminders can share it too. A group diff --git a/frontend-modern/public/docs/TROUBLESHOOTING.md b/frontend-modern/public/docs/TROUBLESHOOTING.md index 4c777f9b3a..99c1d7002e 100644 --- a/frontend-modern/public/docs/TROUBLESHOOTING.md +++ b/frontend-modern/public/docs/TROUBLESHOOTING.md @@ -923,7 +923,8 @@ when they were queued.** Editing a URL, recipient, credential, header or templat does not replace that saved configuration. A test uses the edited settings; retrying an old delivery can still use the old endpoint or credential and fail again. If the old destination must no longer receive data, disable it rather -than relying on a URL edit to redirect queued work. +than relying on a URL edit to redirect queued work. This is not a recall of +requests already sent; see the [pause and cancellation limits](#pause-and-cancellation-limits). **Retry retained deliveries** gives all retained terminal failures a fresh retry budget, not just the destination you tested. Use it only when sending those @@ -940,6 +941,33 @@ Dismissal clears the warning without retrying them; delivery history remains. Neither action deletes the audit trail. Do not delete `notification_queue.db` or audit data to clear the warning. +#### Pause and cancellation limits + +In **Alerts → Notifications**, global **Notifications paused** is not a +store-and-forward hold. Monitoring continues, but pausing clears buffered alert +groups and cancels pending queue deliveries for email, webhooks and Apprise, +including recovery deliveries. **Turning delivery back on does not replay those +cancelled deliveries.** Retained terminal failures are separate: pausing does +not dismiss them or repair their saved destination settings. + +Disabling or removing an individual destination prevents a queued delivery that +sees that policy from sending. A policy-skipped item is cancelled, not a +successful provider delivery, and **Retry retained deliveries** does not replay +cancelled items. Re-enabling a destination is not proof of catch-up or receipt. +For a scheduled quiet period, use [Quiet hours](CONFIGURATION.md#quiet-hours-and-notification-holds) +instead, reviewing its critical-alert exceptions and later re-evaluation; it +still does not guarantee that every held message will be sent. + +**Pause, disable and removal do not guarantee cancellation of a request already +in flight.** The receiver may accept it after the settings change. Neither a +paused banner nor a cancelled queue row proves that the destination received no +data, and Pulse cannot recall a message or delete the receiver's copy. Reconcile +the original time with **Recent delivery activity** and the receiver's existing +record, keeping both private; do not send another Test or replay a queue to +check containment. If a destination credential was exposed, revoke it with +the provider too: editing its URL or turning delivery off is not credential +revocation and cannot undo an earlier disclosure. + #### Emails not sending Follow [retained-failure recovery](#recover-retained-delivery-failures) first. diff --git a/frontend-modern/public/docs/WEBHOOKS.md b/frontend-modern/public/docs/WEBHOOKS.md index 27184ed561..1a2e26830a 100644 --- a/frontend-modern/public/docs/WEBHOOKS.md +++ b/frontend-modern/public/docs/WEBHOOKS.md @@ -150,6 +150,15 @@ batch retries to diagnose rate limiting. A receiver can see a duplicate if an earlier request was accepted but Pulse did not receive its response; preserve [receiver deduplication](#receiver-correlation-and-deduplication). +Global **Notifications paused** cancels pending alert deliveries and clears +buffered groups; turning delivery back on does not replay those cancelled +items. This differs from scheduled quiet-hours holds. Disabling or removing a +destination can block queued work, but **does not recall a request already in +flight**, revoke a provider credential or delete the receiver's copy. Follow +the [pause and cancellation limits](TROUBLESHOOTING.md#pause-and-cancellation-limits) +before treating a paused banner or cancelled row as containment. Alert delivery +controls do not stop the separate [audit forwarding route](#-audit-webhooks-prolegacy-procloud). + **Correlation header.** Alert webhooks carry `X-Pulse-Event-ID` in the form `:` (e.g. `a1b2c3:alert`, `a1b2c3:resolved`). Retries retain it, but later occurrences, severity changes and reminders can share it too. A group diff --git a/internal/notifications/notification_pause_guide_test.go b/internal/notifications/notification_pause_guide_test.go new file mode 100644 index 0000000000..80741a1ed2 --- /dev/null +++ b/internal/notifications/notification_pause_guide_test.go @@ -0,0 +1,155 @@ +package notifications + +import ( + "encoding/json" + "errors" + "net/http" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" +) + +// The guide must not equate a settings change with remote containment. Wait +// until the real receiver has the request before changing policy; a subsequent +// queued attempt must be skipped, but the original request can still succeed. +func TestQueuedWebhookPolicyChangeCannotRecallInFlight(t *testing.T) { + for _, policy := range []string{"global-pause", "destination-disable", "destination-remove"} { + t.Run(policy, func(t *testing.T) { + started, release := make(chan struct{}, 1), make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + var requests atomic.Int32 + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + started <- struct{}{} + <-release + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + defer unblock() // Also drain the receiver on an assertion failure. + m := NewNotificationManagerWithDeferredQueue("", t.TempDir()) + defer m.Stop() + if err := m.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + hook := WebhookConfig{ID: "guide-ops", Enabled: true, Service: "generic", URL: server.URL} + m.AddWebhook(hook) + config, err := json.Marshal(hook) + if err != nil { + t.Fatal(err) + } + job := &QueuedNotification{ID: "in-flight", Type: "webhook", Config: config, + Alerts: []*alerts.Alert{{ID: "guide-incident", ResourceName: "synthetic-node", Type: "cpu", + Level: alerts.AlertLevelWarning, StartTime: time.Now().Add(-time.Minute)}}} + done := make(chan error, 1) + go func() { done <- m.ProcessQueuedNotification(job) }() + select { + case <-started: + case err := <-done: + t.Fatalf("request ended before receiver admission: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("receiver did not observe request") + } + switch policy { + case "global-pause": + m.SetEnabled(false) + case "destination-disable": + hook.Enabled = false + if err := m.UpdateWebhook(hook.ID, hook); err != nil { + t.Fatal(err) + } + case "destination-remove": + if err := m.DeleteWebhook(hook.ID); err != nil { + t.Fatal(err) + } + } + if err := m.ProcessQueuedNotification(job); !errors.Is(err, ErrNotificationDeliverySkipped) { + t.Fatalf("later attempt = %v; want policy skip", err) + } + unblock() + select { + case err := <-done: + if err != nil { + t.Fatalf("original in-flight request did not complete: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("original request did not drain") + } + if got := requests.Load(); got != 1 { + t.Fatalf("receiver requests = %d; want exactly the original request", got) + } + t.Log("policy blocks a later attempt, but receiver accepts the already in-flight request") + }) + } +} + +// Global pause is cancellation, unlike a quiet-hours hold. Exercise real +// enqueue, buffered grouping, pause, resume and persistent reconstruction; +// neither resume nor the retained-failure retry button restores cancelled work. +func TestGlobalNotificationPauseDoesNotCatchUpCancelledWork(t *testing.T) { + dir := t.TempDir() + open := func() *NotificationManager { + m := NewNotificationManagerWithDeferredQueue("", dir) + m.AddWebhook(WebhookConfig{ID: "guide-ops", Enabled: true, Service: "generic", URL: "https://example.test/hook"}) + t.Cleanup(m.Stop) + return m + } + m := open() + m.SetGroupingConfig(false, 0, false, false) + alert := &alerts.Alert{ID: "pending-before-pause", ResourceName: "synthetic-node", Type: "cpu", + Level: alerts.AlertLevelWarning, StartTime: time.Now().Add(-time.Minute)} + m.SendAlert(alert) + rows, err := m.queue.GetPending(10) + if err != nil || len(rows) != 1 { + t.Fatalf("pending before pause = %d, %v; want one delivery", len(rows), err) + } + for _, kind := range []string{"webhook_resolved", "email", "email_resolved", "apprise", "apprise_resolved"} { + if err := m.queue.Enqueue(&QueuedNotification{ID: kind, Type: kind, Config: []byte(`{}`), + Alerts: []*alerts.Alert{alert}, CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + } + if err := m.queue.Enqueue(&QueuedNotification{ID: "retained-failure", Type: "webhook", Status: QueueStatusDLQ, + Config: []byte(`{}`), Alerts: []*alerts.Alert{alert}, CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + m.SetGroupingConfig(true, 3600, false, false) + buffered := alert.Clone() + buffered.ID = "buffered-before-pause" + m.SendAlert(buffered) + if len(m.pendingAlerts) != 1 { + t.Fatal("fixture did not buffer one alert group") + } + m.SetEnabled(false) + if len(m.pendingAlerts) != 0 || m.groupTimer != nil { + t.Fatal("pause did not clear buffered groups and their timer") + } + paused := alert.Clone() + paused.ID = "during-pause" + m.SendAlert(paused) + m.SetEnabled(true) + if len(m.pendingAlerts) != 0 { + t.Fatal("resume recreated a buffered group") + } + m.Stop() + m = open() // Saved queue, fully enabled policy, no automatic workers. + rows, err = m.queue.GetPending(10) + if err != nil || len(rows) != 0 { + t.Fatalf("pending after resume/reconstruction = %d, %v; want none", len(rows), err) + } + stats, err := m.queue.GetQueueStats() + if err != nil || stats["cancelled"] != 6 || stats["dlq"] != 1 || len(stats) != 2 { + t.Fatalf("retained queue = %v, %v; want six cancelled deliveries and one separate retained failure", stats, err) + } + if count, err := m.queue.RetryTerminalFailures(); err != nil || count != 1 { + t.Fatalf("retained-failure retry = %d, %v; want only the separate failure", count, err) + } + rows, err = m.queue.GetPending(10) + if err != nil || len(rows) != 1 || rows[0].ID != "retained-failure" { + t.Fatalf("retry rows = %+v, %v; want only the original retained failure, no cancelled replay", rows, err) + } + t.Log("pause cancels pending work and drops buffered/paused alerts; resume/restart/retry creates no catch-up") +} diff --git a/scripts/tests/test_notification_troubleshooting_docs.py b/scripts/tests/test_notification_troubleshooting_docs.py index 8a20a1b3d2..c4da43d685 100644 --- a/scripts/tests/test_notification_troubleshooting_docs.py +++ b/scripts/tests/test_notification_troubleshooting_docs.py @@ -188,6 +188,28 @@ class NotificationTroubleshootingDocsTest(unittest.TestCase): guide = (ROOT / "docs/WEBHOOKS.md").read_text() self.assertIn("TROUBLESHOOTING.md#telegram-test-works-but-real-alerts-say-message-text-is-empty", guide) + def test_pause_is_cancellation_not_catchup_or_remote_containment(self): + limits = " ".join(section("Pause and cancellation limits", 4).split()) + for boundary in ( + "not a store-and-forward hold", "Monitoring continues", "clears buffered alert groups", + "cancels pending queue deliveries", "including recovery deliveries", + "does not replay those cancelled deliveries", "pausing does not dismiss them", + "policy-skipped item is cancelled, not a successful provider delivery", + "does not replay cancelled items", "critical-alert exceptions", + "do not guarantee cancellation of a request already in flight", + "receiver may accept it after the settings change", "cancelled queue row", + "cannot recall a message", "receiver's existing record", "keeping both private", + "do not send another Test or replay", "revoke it with the provider", + "not credential revocation", + ): + self.assertIn(boundary, limits) + self.assertIn("#pause-and-cancellation-limits", section("Recover retained delivery failures", 4)) + guide = " ".join((ROOT / "docs/WEBHOOKS.md").read_text().split()) + for boundary in ("TROUBLESHOOTING.md#pause-and-cancellation-limits", + "does not recall a request already in flight", + "Alert delivery controls do not stop the separate"): + self.assertIn(boundary, guide) + if __name__ == "__main__": unittest.main() diff --git a/scripts/tests/test_public_docs_workflow.py b/scripts/tests/test_public_docs_workflow.py index 3510acd18c..76e6e8441b 100644 --- a/scripts/tests/test_public_docs_workflow.py +++ b/scripts/tests/test_public_docs_workflow.py @@ -28,6 +28,8 @@ TESTS = ( "test_agent_profile_docs.py", "test_pbs_docs.py", "test_audit_recovery_docs.py", + "test_notification_troubleshooting_docs.py", + "test_webhook_verification_docs.py", "test_pve_backup_troubleshooting_docs.py", "test_vm_disk_diagnostics.py", "test_memory_troubleshooting_docs.py",