diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 0290b36c6..10ff4193b 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -2134,6 +2134,13 @@ payload shape change when the portal presents compact client rows. 66. `internal/api/ai_intelligence_handlers.go` shared with `ai-runtime`: AI intelligence handlers are both an AI runtime control surface and a canonical API payload contract boundary. 67. `internal/api/alerting/notification_queue.go` shared with `notifications`: the notification queue and DLQ handler is both a notification delivery consequence surface and a canonical API payload boundary for operational transition links. 68. `internal/api/alerting/notifications.go` shared with `notifications`: notification handlers are both a notification delivery control surface and a canonical API payload contract boundary. + Apprise configuration-update logs contain normalised mode, counts and + presence flags, not endpoint paths, config keys, targets or arbitrary CLI + and header strings. Test-error responses retain safe structured failure + summaries and HTTP status, never raw Apprise output or provider response + bodies. Request/response schemas and authorised configuration round-trips + remain unchanged; `internal/api/alerting/notifications_test.go` exercises + both the update log and the real sender through the test handler. 69. `internal/api/configapi/config_setup_handlers.go` shared with `agent-lifecycle`: auto-register and setup handlers are both an agent lifecycle control surface and a canonical API payload contract boundary. 70. `internal/api/configapi/setup_script_render.go` shared with `agent-lifecycle`, `storage-recovery`: the generated Proxmox setup-script is a shared boundary across agent lifecycle (forced-command keys, install/uninstall edits), API contracts (rendered token shape and encoded rerun URL), and storage/recovery (backup visibility grants, Pulse-managed temperature SSH keys, and SMART disk-temperature collection). That same shared boundary also owns reachable-host selection truth for canonical Proxmox registration: runtime callers may propose ordered `candidateHosts`, but the API contract must persist and echo the first candidate Pulse can actually reach instead of freezing the caller's rejected first preference into the stored node endpoint. diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 55c36f3cf..83ced5ecf 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -74,6 +74,7 @@ or displayed a notification. 11. `internal/notifications/deadman_config.go` 12. `internal/notifications/failure_class.go` 13. `internal/notifications/quiet_hours_queue.go` +14. `internal/notifications/apprise_diagnostics.go` ## Shared Boundaries @@ -142,6 +143,35 @@ stable opaque routing identities and must not expose credentials. ## Current State +### Apprise diagnostic confidentiality + +Apprise CLI targets may carry credentials in arbitrary provider schemes. CLI +output and HTTP response/error text may echo target credentials, API keys, +configuration keys or private alert content. Firing, recovery, test and queued +delivery must not copy those bytes into logs, returned errors, attempt audits, +dead letters or delivery-log projections. Configuration-update logs use only +normalised mode, counts, presence flags, timeout and TLS-policy flags. Authorised +configuration reads/edits and private admitted queue configuration are unchanged. + +`apprise_diagnostics.go` retains structured failure classes and typed causes for +`errors.Is`/`errors.As`, but formats only fixed safe reasons or a numeric CLI exit. +HTTP status remains the authoritative retry verdict; response bodies are drained +only to the existing size limit and discarded. CLI diagnostics retain byte and +target counts, not output or target values. Shared URL validation and redirect +checks use an opaque Apprise diagnostic label, including allowlist debug logs; +ordinary webhook diagnostics retain their existing redaction. Endpoint, mounted +path, escaped configuration key, target arguments, authentication headers, +payloads, TLS verification, SSRF checks, retry budget and delivery state do not +change. Existing historical logs/audits are not rewritten or claimed cleared. + +Verification: `apprise_confidentiality_test.go` captures actual CLI/loopback HTTP +logs and errors across firing, recovery and test sends; verifies unchanged +arguments/requests; exercises malformed URLs, permitted and refused redirects, +typed transport failures and the real queue's retry/DLQ/audit/log projection. +`internal/api/alerting/notifications_test.go` covers configuration logging and +the real sender's test-error HTTP response. All secrets are synthetic; these +controls do not establish installed Apprise acceptance or prior user exposure. + ### Current-policy quiet-hours replay The persistent queue checks every due provider attempt against an immutable diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 2b60224a0..222a9fd47 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -3280,6 +3280,7 @@ "internal/api/ai_handlers_more_test.go", "internal/api/ai_handlers_patrol_actions_additional_test.go", "internal/api/alerting/external_probe_notifications_test.go", + "internal/api/alerting/notifications_test.go", "internal/api/audit_handlers_test.go", "internal/api/availability_handlers_test.go", "internal/api/contract_test.go", diff --git a/internal/api/alerting/notifications.go b/internal/api/alerting/notifications.go index dcf32bd1e..3cde888f0 100644 --- a/internal/api/alerting/notifications.go +++ b/internal/api/alerting/notifications.go @@ -321,17 +321,16 @@ func (h *NotificationHandlers) UpdateAppriseConfig(w http.ResponseWriter, r *htt config.MinimumSeverity = existingConfig.MinimumSeverity } + diagnosticConfig := notifications.NormalizeAppriseConfig(config) log.Info(). - Bool("enabled", config.Enabled). - Str("mode", string(config.Mode)). - Int("targetCount", len(config.Targets)). - Str("cliPath", config.CLIPath). - Str("serverUrl", config.ServerURL). - Str("configKey", config.ConfigKey). - Bool("hasApiKey", config.APIKey != ""). - Str("apiKeyHeader", config.APIKeyHeader). - Bool("skipTlsVerify", config.SkipTLSVerify). - Int("timeoutSeconds", config.TimeoutSeconds). + Bool("enabled", diagnosticConfig.Enabled). + Str("mode", string(diagnosticConfig.Mode)). + Int("targetCount", len(diagnosticConfig.Targets)). + Bool("hasServerURL", diagnosticConfig.ServerURL != ""). + Bool("hasConfigKey", diagnosticConfig.ConfigKey != ""). + Bool("hasApiKey", diagnosticConfig.APIKey != ""). + Bool("skipTlsVerify", diagnosticConfig.SkipTLSVerify). + Int("timeoutSeconds", diagnosticConfig.TimeoutSeconds). Msg("Parsed Apprise configuration update") if err := monitor.GetConfigPersistence().SaveAppriseConfig(config); err != nil { diff --git a/internal/api/alerting/notifications_test.go b/internal/api/alerting/notifications_test.go index 62a0429cb..270b5d42d 100644 --- a/internal/api/alerting/notifications_test.go +++ b/internal/api/alerting/notifications_test.go @@ -14,10 +14,84 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/notifications" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) +// Configuration responses intentionally let an authorised editor round-trip +// targets; routine logs must not duplicate their credentials or config keys. +func TestAppriseConfigurationLogsWithholdSecrets(t *testing.T) { + const secret = "apprise-api-synthetic-secret" + var captured bytes.Buffer + original := log.Logger + log.Logger = zerolog.New(zerolog.SyncWriter(&captured)) + defer func() { log.Logger = original }() + cfg := notifications.AppriseConfig{Enabled: true, Mode: notifications.AppriseModeHTTP, ServerURL: "https://fixture/" + secret, + ConfigKey: secret, APIKey: secret, CLIPath: secret, APIKeyHeader: secret, Targets: []string{"future://" + secret}} + manager := new(MockNotificationManager) + persistence := new(MockNotificationConfigPersistence) + monitor := new(MockNotificationMonitor) + manager.On("GetAppriseConfig").Return(cfg).Twice() + manager.On("SetAppriseConfig", mock.MatchedBy(func(got notifications.AppriseConfig) bool { + return got.ServerURL == cfg.ServerURL && got.ConfigKey == secret && got.APIKey == secret && got.Targets[0] == cfg.Targets[0] + })).Return().Once() + persistence.On("SaveAppriseConfig", mock.MatchedBy(func(got notifications.AppriseConfig) bool { + return got.ServerURL == cfg.ServerURL && got.ConfigKey == secret && got.APIKey == secret && got.Targets[0] == cfg.Targets[0] + })).Return(nil).Once() + monitor.On("GetNotificationManager").Return(manager) + monitor.On("GetConfigPersistence").Return(persistence) + body, _ := json.Marshal(cfg) + w := httptest.NewRecorder() + NewNotificationHandlers(nil, monitor).UpdateAppriseConfig(w, httptest.NewRequest(http.MethodPost, "/api/notifications/apprise", bytes.NewReader(body))) + if w.Code != http.StatusOK || strings.Contains(captured.String(), secret) { + t.Fatal("configuration update failed or a synthetic credential reached its log") + } + if !strings.Contains(captured.String(), "hasConfigKey") || !strings.Contains(captured.String(), "targetCount") { + t.Fatal("structured configuration diagnostics were lost") + } + var response map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil || response["apiKey"] != "" || response["hasApiKey"] != true { + t.Fatal("saved API-key response contract changed") + } + manager.AssertExpectations(t) + persistence.AssertExpectations(t) +} + +// Exercise the real sender through the handler, not an assumed safe mock error. +func TestAppriseTestResponseWithholdsProviderSecrets(t *testing.T) { + const secret = "apprise-api-synthetic-secret" + var captured bytes.Buffer + original := log.Logger + log.Logger = zerolog.New(zerolog.SyncWriter(&captured)) + defer func() { log.Logger = original }() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(w, "provider-private-content ", secret, " ", r.URL.String()) + })) + defer server.Close() + manager := notifications.NewNotificationManagerWithDataDir("", t.TempDir()) + defer manager.Stop() + if err := manager.UpdateAllowedPrivateCIDRs("127.0.0.1/32,::1/128"); err != nil { + t.Fatal(err) + } + monitor := new(MockNotificationMonitor) + monitor.On("GetNotificationManager").Return(manager) + body, _ := json.Marshal(map[string]any{"method": "apprise", "config": notifications.AppriseConfig{Enabled: true, Mode: notifications.AppriseModeHTTP, + ServerURL: server.URL + "/" + secret, ConfigKey: secret, APIKey: secret}}) + w := httptest.NewRecorder() + NewNotificationHandlers(nil, monitor).TestNotification(w, httptest.NewRequest(http.MethodPost, "/api/notifications/test", bytes.NewReader(body))) + if w.Code != http.StatusBadRequest || !strings.Contains(w.Body.String(), "HTTP 401") { + t.Fatal("structured HTTP failure was lost from the test result") + } + for _, forbidden := range []string{secret, "provider-private-content"} { + if strings.Contains(w.Body.String(), forbidden) || strings.Contains(captured.String(), forbidden) { + t.Error("test response or log exposed synthetic provider credentials/content") + } + } +} + func TestRedactSecretsFromURL(t *testing.T) { tests := []struct { name string diff --git a/internal/notifications/apprise_confidentiality_test.go b/internal/notifications/apprise_confidentiality_test.go new file mode 100644 index 000000000..72f67fcb6 --- /dev/null +++ b/internal/notifications/apprise_confidentiality_test.go @@ -0,0 +1,357 @@ +package notifications + +import ( + "bytes" + "context" + "crypto/x509" + "encoding/json" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os/exec" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +// All credentials and private messages in these tests are synthetic. +const appriseSecret = "apprise-synthetic-secret" + +type appriseLogBuffer struct { + sync.Mutex + bytes.Buffer +} + +func (b *appriseLogBuffer) Write(p []byte) (int, error) { + b.Lock() + defer b.Unlock() + return b.Buffer.Write(p) +} + +func (b *appriseLogBuffer) String() string { + b.Lock() + defer b.Unlock() + return b.Buffer.String() +} + +func captureAppriseLogs(t *testing.T) *appriseLogBuffer { + t.Helper() + b := &appriseLogBuffer{} + original := log.Logger + log.Logger = zerolog.New(b).Level(zerolog.DebugLevel) + t.Cleanup(func() { log.Logger = original }) + return b +} + +func assertAppriseConfidential(t *testing.T, message string) { + t.Helper() + for _, secret := range []string{appriseSecret, url.PathEscape(appriseSecret + "/config"), "provider-private-content", "private-alert-body"} { + if strings.Contains(message, secret) { + t.Error("Apprise diagnostic exposed a synthetic secret or private content") + } + } +} + +func sendAppriseForConfidentiality(n *NotificationManager, cfg AppriseConfig, kind string) error { + alert := &alerts.Alert{ID: "confidentiality", ResourceName: "fixture-node", Message: "private-alert-body", Level: alerts.AlertLevelWarning} + switch kind { + case "resolved": + return n.sendResolvedApprise(cfg, []*alerts.Alert{alert}, time.Now()) + case "test": + return n.SendTestAppriseWithConfig(cfg) + default: + return n.sendGroupedApprise(cfg, []*alerts.Alert{alert}) + } +} + +func TestAppriseCLIConfidentiality(t *testing.T) { + for _, kind := range []string{"firing", "resolved", "test"} { + for _, failing := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/failure=%t", kind, failing), func(t *testing.T) { + captured := captureAppriseLogs(t) + targets := []string{"tgram://" + appriseSecret + "/-1001234567890:42", "future-provider://user:" + appriseSecret + "@fixture/path"} + cause := errors.New("provider-private-content " + appriseSecret) + calls := 0 + n := &NotificationManager{appriseExec: func(_ context.Context, args []string) ([]byte, error) { + calls++ + if len(args) != 4+len(targets) || args[0] != "-t" || args[2] != "-b" || !slices.Equal(args[4:], targets) { + t.Error("CLI payload or exact target arguments changed") + } + if kind != "test" && !strings.Contains(args[3], "private-alert-body") { + t.Error("alert content was removed from delivery, not just diagnostics") + } + if failing { + return []byte("provider-private-content " + strings.Join(args, " ")), cause + } + return []byte("provider-private-content " + strings.Join(args, " ")), nil + }} + err := sendAppriseForConfidentiality(n, AppriseConfig{Enabled: true, Targets: targets}, kind) + if calls != 1 || (err != nil) != failing { + t.Fatalf("calls=%d error=%t, want one call/failure=%t", calls, err != nil, failing) + } + if err != nil { + assertAppriseConfidential(t, err.Error()) + if !errors.Is(err, cause) || ClassifyNotificationFailureError(err) != NotificationFailureUnknown { + t.Error("CLI cause or failure class changed") + } + } + assertAppriseConfidential(t, captured.String()) + if !strings.Contains(captured.String(), "outputBytes") || !strings.Contains(captured.String(), "targetCount") { + t.Error("structured CLI diagnostics were lost") + } + }) + } + } +} + +func TestAppriseHTTPConfidentiality(t *testing.T) { + for _, kind := range []string{"firing", "resolved", "test"} { + for _, status := range []int{http.StatusOK, http.StatusUnauthorized, http.StatusTooManyRequests, http.StatusInternalServerError} { + t.Run(fmt.Sprintf("%s/%d", kind, status), func(t *testing.T) { + captured := captureAppriseLogs(t) + var calls atomic.Int32 + configKey := appriseSecret + "/config" + target := "tgram://" + appriseSecret + "/-1001234567890:42" + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + var payload struct { + Body, Title, Type string + URLs []string + } + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Error("invalid HTTP payload") + } + wantType := "warning" + if kind == "resolved" { + wantType = "info" + } + if r.Method != http.MethodPost || r.URL.EscapedPath() != "/mounted/"+appriseSecret+"/notify/"+url.PathEscape(configKey) || r.Header.Get("X-API-KEY") != appriseSecret || !slices.Equal(payload.URLs, []string{target}) || payload.Type != wantType { + t.Error("HTTP endpoint, credentials, targets or event type changed") + } + if kind != "test" && !strings.Contains(payload.Body, "private-alert-body") { + t.Error("HTTP alert content was removed") + } + w.WriteHeader(status) + fmt.Fprint(w, "provider-private-content ", appriseSecret, " ", r.URL.String()) + })) + defer server.Close() + n := &NotificationManager{} + if err := n.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + err := sendAppriseForConfidentiality(n, AppriseConfig{Enabled: true, Mode: AppriseModeHTTP, ServerURL: server.URL + "/mounted/" + appriseSecret, ConfigKey: configKey, APIKey: appriseSecret, Targets: []string{target}}, kind) + if calls.Load() != 1 || (err != nil) != (status != http.StatusOK) { + t.Fatalf("calls=%d error=%t, want one call/status=%d", calls.Load(), err != nil, status) + } + if err != nil { + assertAppriseConfidential(t, err.Error()) + if !strings.Contains(err.Error(), fmt.Sprintf("HTTP %d", status)) || ClassifyNotificationFailureError(err) != ClassFromHTTPStatus(status) { + t.Error("structured HTTP status/class was lost") + } + } + assertAppriseConfidential(t, captured.String()) + if status == http.StatusOK && !strings.Contains(captured.String(), "responseBytes") { + t.Error("HTTP response observation was lost") + } + }) + } + } +} + +func TestAppriseValidationAndRedirectConfidentiality(t *testing.T) { + for _, raw := range []string{ + "ftp://fixture/" + appriseSecret, + "http://fixture/%zz" + appriseSecret, + "http://" + appriseSecret + ":password@fixture", + "http://fixture/path?key=" + appriseSecret, + "http://fixture/#" + appriseSecret, + } { + t.Run(fmt.Sprintf("invalid-%d", len(raw)), func(t *testing.T) { + captured := captureAppriseLogs(t) + err := (&NotificationManager{}).SendTestAppriseWithConfig(AppriseConfig{Enabled: true, Mode: AppriseModeHTTP, ServerURL: raw}) + if err == nil { + t.Fatal("unsafe base URL was accepted") + } + assertAppriseConfidential(t, err.Error()) + assertAppriseConfidential(t, captured.String()) + if ClassifyNotificationFailureError(err) != NotificationFailureConfiguration { + t.Error("validation retry class changed") + } + }) + } + for _, allowed := range []bool{false, true} { + t.Run(fmt.Sprintf("redirect-allowed=%t", allowed), func(t *testing.T) { + captured := captureAppriseLogs(t) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/notify") { + target := "http://169.254.169.254/" + appriseSecret + if allowed { + target = "/accepted/" + appriseSecret + } + http.Redirect(w, r, target, http.StatusTemporaryRedirect) + return + } + fmt.Fprint(w, "provider-private-content ", appriseSecret) + })) + defer server.Close() + n := &NotificationManager{} + if err := n.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + err := n.SendTestAppriseWithConfig(AppriseConfig{Enabled: true, Mode: AppriseModeHTTP, ServerURL: server.URL, ConfigKey: appriseSecret}) + if (err == nil) != allowed { + t.Fatal("redirect security behaviour changed") + } + if err != nil { + assertAppriseConfidential(t, err.Error()) + if !strings.Contains(err.Error(), "link-local addresses are not allowed") { + t.Error("actionable redirect refusal was lost") + } + } + assertAppriseConfidential(t, captured.String()) + }) + } +} + +func TestAppriseTransportConfidentiality(t *testing.T) { + captured := captureAppriseLogs(t) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Error(err) + return + } + conn.Close() + })) + defer server.Close() + n := &NotificationManager{} + if err := n.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + err := n.SendTestAppriseWithConfig(AppriseConfig{Enabled: true, Mode: AppriseModeHTTP, ServerURL: server.URL + "/" + appriseSecret, ConfigKey: appriseSecret}) + if err == nil { + t.Fatal("broken HTTP connection was treated as delivered") + } + assertAppriseConfidential(t, err.Error()) + assertAppriseConfidential(t, captured.String()) + var transportErr *url.Error + if !errors.As(err, &transportErr) { + t.Error("typed transport cause was lost") + } +} + +func TestAppriseSafeErrorPreservesClassification(t *testing.T) { + for _, cause := range []error{ + context.DeadlineExceeded, + &net.DNSError{Name: appriseSecret, Err: "private DNS error"}, + x509.HostnameError{Host: appriseSecret, Certificate: &x509.Certificate{}}, + FailfWithClass(NotificationFailureAuthentication, "%s", appriseSecret), + errors.New("rate limit " + appriseSecret), + } { + err := safeAppriseError("operation", cause) + assertAppriseConfidential(t, err.Error()) + if !errors.Is(err, cause) || ClassifyNotificationFailureError(err) != ClassifyNotificationFailureError(cause) { + t.Error("original cause or retry classification changed") + } + } + cmd := exec.Command("sh", "-c", "exit 7") + cause := cmd.Run() + err := safeAppriseError("execute apprise CLI", cause) + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || !strings.Contains(err.Error(), "exit status 7") { + t.Error("structured CLI exit was lost") + } + if safeAppriseError("operation", nil) != nil { + t.Error("nil cause became a failure") + } +} + +// The real worker persists only safe summaries to retry/DLQ, attempt audits and +// the delivery-log projection. Admitted config bytes remain private queue data. +func TestAppriseQueueConfidentiality(t *testing.T) { + for _, kind := range []string{"apprise", "apprise_resolved"} { + for _, status := range []int{http.StatusUnauthorized, http.StatusTooManyRequests, http.StatusInternalServerError} { + t.Run(fmt.Sprintf("%s/%d", kind, status), func(t *testing.T) { + captured := captureAppriseLogs(t) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(status) + fmt.Fprint(w, "provider-private-content ", appriseSecret) + })) + defer server.Close() + q, err := NewNotificationQueue(t.TempDir()) + if err != nil { + t.Fatal(err) + } + defer q.Stop() + cfg := AppriseConfig{Enabled: true, Mode: AppriseModeHTTP, ServerURL: server.URL + "/" + appriseSecret, ConfigKey: appriseSecret, APIKey: appriseSecret, TimeoutSeconds: 5} + n := &NotificationManager{enabled: true, queue: q, appriseConfig: cfg} + if err := n.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + config, _ := json.Marshal(cfg) + notif := &QueuedNotification{ID: "confidentiality", Type: kind, Status: QueueStatusPending, Config: config, MaxAttempts: 3, + Alerts: []*alerts.Alert{{ID: "fixture", StartTime: time.Now(), Message: "private-alert-body"}}} + if err := q.Enqueue(notif); err != nil { + t.Fatal(err) + } + q.SetProcessor(n.ProcessQueuedNotification) + q.processBatch() + // The autonomous worker can win the atomic claim. Its committed + // attempt audit, not the return from our stale snapshot, is proof + // the send completed. Stop processing before the retry wake. + deadline := time.Now().Add(3 * time.Second) + for { + var count int + if err := q.db.QueryRow("SELECT count(*) FROM notification_audit WHERE notification_id = ?", notif.ID).Scan(&count); err != nil { + t.Fatal(err) + } + if count > 0 { + break + } + if time.Now().After(deadline) { + t.Fatal("queue attempt did not complete") + } + time.Sleep(time.Millisecond) + } + q.SetProcessor(nil) + var gotStatus, lastError, rawConfig string + var attempts int + if err := q.db.QueryRow("SELECT status, attempts, last_error, config FROM notification_queue WHERE id = ?", notif.ID).Scan(&gotStatus, &attempts, &lastError, &rawConfig); err != nil { + t.Fatal(err) + } + wantStatus := QueueStatusPending + if status == http.StatusUnauthorized { + wantStatus = QueueStatusDLQ + } + if gotStatus != string(wantStatus) || attempts != 1 || rawConfig != string(config) { + t.Error("queue lifecycle, attempt budget or admitted credentials changed") + } + assertAppriseConfidential(t, lastError) + var auditError, class string + if err := q.db.QueryRow("SELECT error_message, failure_class FROM notification_audit WHERE notification_id = ?", notif.ID).Scan(&auditError, &class); err != nil { + t.Fatal(err) + } + if class != string(ClassFromHTTPStatus(status)) || !strings.Contains(auditError, fmt.Sprintf("HTTP %d", status)) { + t.Error("audit HTTP status/class was lost") + } + assertAppriseConfidential(t, auditError) + entries, err := q.GetDeliveryLog(time.Time{}, 10) + if err != nil || len(entries) != 1 { + t.Fatalf("delivery log entries=%d error=%v", len(entries), err) + } + projection, _ := json.Marshal(entries) + assertAppriseConfidential(t, string(projection)) + assertAppriseConfidential(t, captured.String()) + }) + } + } +} diff --git a/internal/notifications/apprise_diagnostics.go b/internal/notifications/apprise_diagnostics.go new file mode 100644 index 000000000..784ec7018 --- /dev/null +++ b/internal/notifications/apprise_diagnostics.go @@ -0,0 +1,90 @@ +package notifications + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "os/exec" + "strings" + "syscall" +) + +// Apprise targets use many credential-bearing URL schemes, and both CLI output +// and HTTP responses can echo the targets, API key, config key or alert body. +// Do not try to recognise every provider's secret syntax. Diagnostics retain +// structured outcomes, never endpoint paths or arbitrary provider/error text. +func appriseDiagnosticURL(string) string { return "[Apprise endpoint]" } + +type appriseDiagnosticError struct { + message string + cause error +} + +func (e *appriseDiagnosticError) Error() string { return e.message } +func (e *appriseDiagnosticError) Unwrap() error { return e.cause } + +func safeAppriseError(operation string, err error) error { + if err == nil { + return nil + } + // Classify before hiding prose so existing retry decisions remain intact. + // Retain the cause for errors.Is/As, but never format it in diagnostics. + class := ClassifyNotificationFailureError(fmt.Errorf("%s: %w", operation, err)) + reason := "delivery failed (details withheld)" + switch class { + case NotificationFailureAuthentication: + reason = "authentication failed" + case NotificationFailureRateLimited: + reason = "rate limited" + case NotificationFailureConnectivity: + reason = "server connection failed" + case NotificationFailureTLS: + reason = "TLS certificate or handshake failed" + case NotificationFailureConfiguration: + reason = "invalid configuration" + case NotificationFailureRejected: + reason = "request rejected" + case NotificationFailureServerError: + reason = "server unavailable" + } + var dnsErr *net.DNSError + var netErr net.Error + switch { + case errors.Is(err, exec.ErrNotFound): + reason = "executable file not found in PATH" + case errors.Is(err, syscall.EACCES): + reason = "permission denied" + case errors.As(err, &dnsErr): + reason = "no such host or DNS lookup failed" + case errors.Is(err, syscall.ECONNREFUSED): + reason = "connection refused" + case errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &netErr) && netErr.Timeout()): + reason = "connection timed out (deadline exceeded)" + case errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF): + reason = "unexpected EOF from server" + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + reason = fmt.Sprintf("exit status %d (output withheld)", exitErr.ExitCode()) + } + // These literal validation reasons are actionable and contain no input. + // Inspect wrapped redirect refusals too; unknown reasons stay withheld. + for cause := err; cause != nil; cause = errors.Unwrap(cause) { + switch cause.Error() { + case "URL userinfo is not allowed", "base URL must not include query or fragment", + "URL host is required", "URL hostname is required", "base URL path must be host-local", + "webhook URL userinfo is not allowed", "webhook URL missing hostname", + "webhook URLs pointing to unspecified addresses are not allowed", + "webhook URLs pointing to localhost are not allowed for security reasons", + "webhook URLs pointing to link-local addresses are not allowed", + "webhook URLs pointing to cloud metadata services are not allowed": + reason = cause.Error() + } + if strings.Contains(cause.Error(), "private networks are not allowed for security (configure allowlist in System Settings)") { + reason = "private networks are not allowed for security (configure allowlist in System Settings)" + } + } + return FailWithClass(class, &appriseDiagnosticError{message: operation + ": " + reason, cause: err}) +} diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index 744ebbd5f..17189a504 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -107,6 +107,10 @@ func (n *NotificationManager) createSecureWebhookClient(timeout time.Duration) * // createSecureWebhookClientWithTLS creates a secure HTTP client with optional TLS verification override. func (n *NotificationManager) createSecureWebhookClientWithTLS(timeout time.Duration, skipTLSVerify bool) *http.Client { + return n.createSecureWebhookClientWithURLDiagnostics(timeout, skipTLSVerify, RedactWebhookURLSecrets) +} + +func (n *NotificationManager) createSecureWebhookClientWithURLDiagnostics(timeout time.Duration, skipTLSVerify bool, diagnosticURL func(string) string) *http.Client { // dedicated transport that pins DNS resolution to prevent rebinding transport := &http.Transport{ // Proxy intentionally nil — outbound proxies would bypass DialContext @@ -195,7 +199,7 @@ func (n *NotificationManager) createSecureWebhookClientWithTLS(timeout time.Dura return fmt.Errorf("stopped after %d redirects", WebhookMaxRedirects) } // Re-validate strictly on redirect - return n.ValidateWebhookURL(req.URL.String()) + return n.validateWebhookURL(req.URL.String(), diagnosticURL) }, } } @@ -2071,7 +2075,6 @@ func (n *NotificationManager) sendGroupedApprise(config AppriseConfig, alertList log.Warn(). Err(err). Str("mode", string(cfg.Mode)). - Str("serverUrl", cfg.ServerURL). Msg("failed to send Apprise notification via API") return fmt.Errorf("apprise HTTP send failed: %w", err) } @@ -2080,8 +2083,7 @@ func (n *NotificationManager) sendGroupedApprise(config AppriseConfig, alertList log.Warn(). Err(err). Str("mode", string(cfg.Mode)). - Str("cliPath", cfg.CLIPath). - Strs("targets", cfg.Targets). + Int("targetCount", len(cfg.Targets)). Msg("failed to send Apprise notification") return fmt.Errorf("apprise CLI send failed: %w", err) } @@ -2246,18 +2248,18 @@ func (n *NotificationManager) sendAppriseViaCLI(cfg AppriseConfig, title, body s if len(output) > 0 { log.Debug(). Str("cliPath", "apprise"). - Strs("targets", cfg.Targets). - Str("output", string(output)). + Int("targetCount", len(cfg.Targets)). + Int("outputBytes", len(output)). Msg("apprise CLI output (error)") } - return fmt.Errorf("execute apprise CLI %q: %w", "apprise", err) + return safeAppriseError("execute apprise CLI", err) } if len(output) > 0 { log.Debug(). Str("cliPath", "apprise"). - Strs("targets", cfg.Targets). - Str("output", string(output)). + Int("targetCount", len(cfg.Targets)). + Int("outputBytes", len(output)). Msg("apprise CLI output") } return nil @@ -2282,16 +2284,19 @@ func (n *NotificationManager) sendAppriseViaHTTP(cfg AppriseConfig, title, body, serverURL := cfg.ServerURL lowerURL := strings.ToLower(serverURL) if !strings.HasPrefix(lowerURL, "http://") && !strings.HasPrefix(lowerURL, "https://") { - return fmt.Errorf("apprise server URL must start with http or https: %s", serverURL) + return fmt.Errorf("apprise server URL must start with http or https") } - validatedBaseURL, err := n.validatedWebhookBaseURL(serverURL) + validatedBaseURL, err := securityutil.NormalizeHTTPBaseURL(serverURL, "") + if err == nil { + err = n.validateWebhookURL(validatedBaseURL.String(), appriseDiagnosticURL) + } if err != nil { + err = safeAppriseError("apprise server URL validation failed", err) log.Error(). Err(err). - Str("serverURL", serverURL). Msg("apprise server URL validation failed - possible SSRF attempt") - return fmt.Errorf("apprise server URL validation failed: %w", err) + return err } ctx, cancel := context.WithTimeout(context.Background(), time.Duration(cfg.TimeoutSeconds)*time.Second) @@ -2304,7 +2309,7 @@ func (n *NotificationManager) sendAppriseViaHTTP(cfg AppriseConfig, title, body, targetURL, err := securityutil.ResolveRelativeURL(validatedBaseURL, notifyEndpoint) if err != nil { - return fmt.Errorf("apprise server URL validation failed: %w", err) + return safeAppriseError("apprise server URL validation failed", err) } payload := map[string]any{ @@ -2325,7 +2330,7 @@ func (n *NotificationManager) sendAppriseViaHTTP(cfg AppriseConfig, title, body, req, err := securityutil.NewValidatedRequestWithContext(ctx, http.MethodPost, targetURL, bytes.NewReader(payloadBytes)) if err != nil { - return fmt.Errorf("failed to create Apprise request: %w", err) + return safeAppriseError("failed to create Apprise request", err) } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") @@ -2338,32 +2343,30 @@ func (n *NotificationManager) sendAppriseViaHTTP(cfg AppriseConfig, title, body, } } - client := n.createSecureWebhookClientWithTLS( + client := n.createSecureWebhookClientWithURLDiagnostics( time.Duration(cfg.TimeoutSeconds)*time.Second, validatedBaseURL.Scheme == "https" && cfg.SkipTLSVerify, + appriseDiagnosticURL, ) resp, err := client.Do(req) if err != nil { - return fmt.Errorf("failed to reach Apprise server: %w", err) + return safeAppriseError("failed to reach Apprise server", err) } defer resp.Body.Close() limited := io.LimitReader(resp.Body, WebhookMaxResponseSize) - respBody, _ := io.ReadAll(limited) + responseBytes, _ := io.Copy(io.Discard, limited) if resp.StatusCode < 200 || resp.StatusCode >= 300 { - if len(respBody) > 0 { - return FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "apprise server returned HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(respBody))) - } return FailfWithClass(ClassFromHTTPStatus(resp.StatusCode), "apprise server returned HTTP %d", resp.StatusCode) } - if len(respBody) > 0 { + if responseBytes > 0 { log.Debug(). Str("mode", string(cfg.Mode)). - Str("serverUrl", cfg.ServerURL). - Str("response", string(respBody)). + Int("statusCode", resp.StatusCode). + Int64("responseBytes", responseBytes). Msg("apprise API response") } @@ -2391,7 +2394,6 @@ func (n *NotificationManager) sendResolvedApprise(config AppriseConfig, alertLis log.Warn(). Err(err). Str("mode", string(cfg.Mode)). - Str("serverUrl", cfg.ServerURL). Msg("failed to send resolved Apprise notification via API") return fmt.Errorf("apprise HTTP send failed: %w", err) } @@ -2400,8 +2402,7 @@ func (n *NotificationManager) sendResolvedApprise(config AppriseConfig, alertLis log.Warn(). Err(err). Str("mode", string(cfg.Mode)). - Str("cliPath", cfg.CLIPath). - Strs("targets", cfg.Targets). + Int("targetCount", len(cfg.Targets)). Msg("failed to send resolved Apprise notification") return fmt.Errorf("apprise CLI send failed: %w", err) } @@ -3493,6 +3494,12 @@ func isNumeric(s string) bool { // ValidateWebhookURL validates that a webhook URL is safe and properly formed func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error { + return n.validateWebhookURL(webhookURL, RedactWebhookURLSecrets) +} + +// Validation and redirect security are shared; only diagnostic URL rendering +// differs for Apprise, whose endpoint path may itself be a configuration key. +func (n *NotificationManager) validateWebhookURL(webhookURL string, diagnosticURL func(string) string) error { if webhookURL == "" { return fmt.Errorf("webhook URL cannot be empty") } @@ -3528,7 +3535,7 @@ func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error { } log.Debug(). Str("host", host). - Str("url", RedactWebhookURLSecrets(webhookURL)). + Str("url", diagnosticURL(webhookURL)). Msg("localhost webhook URL allowed via allowlist") } @@ -3551,7 +3558,7 @@ func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error { if n.isIPInAllowlist(ip) { log.Debug(). Str("ip", ip.String()). - Str("url", RedactWebhookURLSecrets(webhookURL)). + Str("url", diagnosticURL(webhookURL)). Msg("webhook URL resolves to private IP in allowlist") } else { return fmt.Errorf("webhook URL resolves to private IP %s - private networks are not allowed for security (configure allowlist in System Settings)", ip.String()) @@ -3575,7 +3582,7 @@ func (n *NotificationManager) ValidateWebhookURL(webhookURL string) error { // This helps prevent SSRF attacks using numeric IPs to bypass filters if u.Scheme == "https" && isNumericIP(host) { log.Warn(). - Str("url", RedactWebhookURLSecrets(webhookURL)). + Str("url", diagnosticURL(webhookURL)). Msg("webhook URL uses numeric IP with HTTPS - certificate validation may fail") } @@ -3876,7 +3883,7 @@ func (n *NotificationManager) SendTestAppriseWithConfig(config AppriseConfig) er Bool("enabled", cfg.Enabled). Str("mode", string(cfg.Mode)). Int("targetCount", len(cfg.Targets)). - Str("serverURL", cfg.ServerURL). + Bool("hasServerURL", cfg.ServerURL != ""). Msg("testing Apprise notification with provided config") if !cfg.Enabled {