From db0c72c367bb2bd16fdc97a82e78a209d8958582 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:36:37 +0100 Subject: [PATCH] fix(notifications): redact webhook URL userinfo from diagnostics Mask username and password credentials before existing path and query redaction, and fail closed when the URL cannot be parsed. Reproduce the leak through rate-limit logs and transport errors, preserving error unwrapping and destination configuration. Change-source: pulse-maintainer --- .../v6/internal/subsystems/notifications.md | 17 ++++++++++ .../notifications/webhook_url_redaction.go | 12 +++++++ .../webhook_url_redaction_test.go | 31 ++++++++++++++++--- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index fc986a404..48e59e7ca 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -682,3 +682,20 @@ acceptance stops after the second attempt. This checks the sender's wrapped errors and retry termination, not only the failure classifier. The in-memory plain-SMTP fixture sends no external mail and does not qualify TLS, installed recipient receipt or queue persistence. + +### Webhook diagnostic userinfo confidentiality + +`RedactWebhookURLSecrets` masks the entire URL userinfo (including username-only +credentials), before its existing Telegram-path and query-secret redaction. +Unparseable URLs produce `[invalid webhook URL]`, not a raw credential-bearing +fallback. Valid destination host/path and non-secret query fields remain useful +for diagnosis. Transport-error redaction copies the URL error and retains its +underlying cause without changing the original error or the configured URL. +This follows the credential-exclusion principle in the +[OWASP Logging Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html#data-to-exclude). + +Regression tests cover plain/encoded/user-only credentials, malformed URLs, +non-authority at signs, combined path/query redaction, error unwrapping and +actual rate-limit log output. These queue-free tests establish local diagnostic +redaction, not destination receipt, installed recovery or release qualification. +No claim is made that arbitrary custom path/query secrets are recognised. diff --git a/internal/notifications/webhook_url_redaction.go b/internal/notifications/webhook_url_redaction.go index 7f5f80d51..9786f4886 100644 --- a/internal/notifications/webhook_url_redaction.go +++ b/internal/notifications/webhook_url_redaction.go @@ -9,6 +9,18 @@ import ( // RedactWebhookURLSecrets masks credentials commonly embedded in webhook URLs // while preserving the URL shape needed for operator diagnostics. func RedactWebhookURLSecrets(urlString string) string { + // Userinfo can contain a password or a credential used as the username. + // Do not use URL.Redacted: it preserves usernames. Fail closed on invalid + // URLs rather than returning unparsed credentials to diagnostic callers. + parsed, err := url.Parse(urlString) + if err != nil { + return "[invalid webhook URL]" + } + if parsed.User != nil { + parsed.User = url.User("REDACTED") + urlString = parsed.String() + } + // Telegram bot credentials are path components rather than query values. if idx := strings.Index(urlString, "/bot"); idx != -1 { if endIdx := strings.Index(urlString[idx+4:], "/"); endIdx != -1 { diff --git a/internal/notifications/webhook_url_redaction_test.go b/internal/notifications/webhook_url_redaction_test.go index 73b690d06..23f5263ed 100644 --- a/internal/notifications/webhook_url_redaction_test.go +++ b/internal/notifications/webhook_url_redaction_test.go @@ -16,6 +16,26 @@ func TestRedactWebhookURLSecrets(t *testing.T) { input string want string }{ + "basic auth": { + input: "https://hook-user:hook-password@example.com/hook", + want: "https://REDACTED@example.com/hook", + }, + "username only": { + input: "https://hook-user@example.com/hook", + want: "https://REDACTED@example.com/hook", + }, + "encoded credentials with other secrets": { + input: "https://hook%40user:p%40ss@example.com/bot123:secret/send?token=query-secret&channel=ops", + want: "https://REDACTED@example.com/botREDACTED/send?token=REDACTED&channel=ops", + }, + "malformed credential URL": { + input: "https://hook-user:hook-password@example.com/%zz", + want: "[invalid webhook URL]", + }, + "at sign outside authority": { + input: "https://example.com/hooks/@ops?channel=team@ops", + want: "https://example.com/hooks/@ops?channel=team@ops", + }, "gotify token": { input: "https://gotify.example/message?token=gotify-secret", want: "https://gotify.example/message?token=REDACTED", @@ -43,17 +63,20 @@ func TestRedactWebhookTransportErrorPreservesBehaviorWithoutToken(t *testing.T) cause := errors.New("connection refused") original := &url.Error{ Op: "Post", - URL: "https://gotify.example/message?token=gotify-secret", + URL: "https://hook-user:hook-password@gotify.example/message?token=gotify-secret", Err: cause, } redacted := redactWebhookTransportError(original) - if strings.Contains(redacted.Error(), "gotify-secret") { + if strings.Contains(redacted.Error(), "gotify-secret") || strings.Contains(redacted.Error(), "hook-password") || strings.Contains(redacted.Error(), "hook-user") { t.Fatalf("redacted transport error exposed token: %v", redacted) } if !strings.Contains(redacted.Error(), "token=REDACTED") { t.Fatalf("redacted transport error omitted diagnostic URL shape: %v", redacted) } + if original.URL != "https://hook-user:hook-password@gotify.example/message?token=gotify-secret" { + t.Fatal("redaction mutated the original transport error") + } if !errors.Is(redacted, cause) { t.Fatal("redacted transport error no longer unwraps to its original cause") } @@ -67,7 +90,7 @@ func TestRedactWebhookTransportErrorPreservesBehaviorWithoutToken(t *testing.T) // likely to fire repeatedly. func TestWebhookRateLimitLogsRedactURLSecrets(t *testing.T) { const secret = "gotify-secret" - webhookURL := "https://gotify.example/message?token=" + secret + webhookURL := "https://hook-user:hook-password@gotify.example/message?token=" + secret var captured bytes.Buffer original := log.Logger @@ -87,7 +110,7 @@ func TestWebhookRateLimitLogsRedactURLSecrets(t *testing.T) { if !strings.Contains(out, "rate limit exceeded") { t.Fatalf("expected the rate-limit drop to be logged, got %q", out) } - if strings.Contains(out, secret) { + if strings.Contains(out, secret) || strings.Contains(out, "hook-password") || strings.Contains(out, "hook-user") { t.Fatalf("webhook token leaked into logs: %q", out) } if !strings.Contains(out, "token=REDACTED") {