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
This commit is contained in:
pulse-triage[bot]
2026-09-07 16:36:37 +01:00
parent aab925d675
commit db0c72c367
3 changed files with 56 additions and 4 deletions
@@ -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.
@@ -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 {
@@ -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") {