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") {