fix(notifications): contain encoded query and resolved ntfy diagnostics

Literal-only query matching and the separate resolved ntfy transport caller leaked recognised URL credentials. Decode each query name once and project ntfy transport errors before logging or returning them, without changing destinations or error causes. Add synthetic sink-matrix and HTTP projection regressions plus the bounded notification contract in this commit.

Change-source: pulse-maintainer

docs/release-control/v6/internal/subsystems/notifications.md

@@ -749,3 +749,27 @@ method suffixes, query URLs, fragments, transport errors and rate-limit logs.
 This is diagnostic containment, not evidence of customer exposure or recipient
 delivery. Arbitrary path secrets and unrecognised query credentials remain
 outside this bounded change.
+
+### Bounded diagnostic confidentiality: query representations and bypass callers
+
+Recognised query names are exactly token, apikey, api_key, key, secret and
+password after one URL query decode. Every repeated occurrence is masked,
+including mixed literal/escaped names. Unrelated names, ordering and values
+remain intact; invalid name escapes fail closed. This is diagnostic projection,
+not mutation of configured destinations or a claim to recognise arbitrary secrets.
+
+Resolved ntfy must apply the same transport-error projection before both its
+error log and returned error. Common HTTP execution preserves payload bytes,
+event identity and error causes; URLs containing userinfo remain rejected by
+outbound validation even though historical diagnostic userinfo is masked.
+
+The caller matrix and retained Delivery regression tests exercise URL/message
+helpers, actual rate-limit logs, common transport and resolved-ntfy transport
+errors/logs with synthetic secrets. HTTP delivery-log regression verifies encoded
+and repeated query credentials while retaining diagnostic context and entry
+identity. Existing exact-output tables bound Slack/GovSlack/legacy, Discord,
+Telegram/local paths, malformed URLs and non-secret lookalikes. Earlier proof
+missed decoded query representations and a separate ntfy transport caller:
+provider-only helper examples were not sufficient sink coverage. This contract
+does not assert arbitrary response-body/third-party error secrecy, installed
+recipient delivery, candidate qualification or historical customer exposure.

internal/api/alerting/notifications_health_test.go

@@ -333,6 +333,7 @@ func containsAny(value string, needles ...string) bool {
 func TestGetDeliveryLogDiagnosticContext(t *testing.T) {
 	for _, tc := range []struct{ name, input, want string }{
 		{"plain", "connection refused", "connection refused"},
+		{"encoded repeated query", "Post https://example.test/hook?%74oken=secret&token=secret&channel=ops failed", "Post https://example.test/hook?%74oken=REDACTED&token=REDACTED&channel=ops failed"},
 		{"userinfo", "Post https://user:password@example.test/hook: timeout", "Post https://REDACTED@example.test/hook: timeout"},
 		{"malformed", "Post https://user:password@example.test/%zz: timeout", "[invalid webhook URL]"},
 	} {

internal/notifications/notifications.go

@@ -2776,6 +2776,7 @@ func (n *NotificationManager) sendResolvedWebhookNtfy(webhook WebhookConfig, ale
 
 	resp, err := n.webhookClient.Do(req)
 	if err != nil {
+		err = redactWebhookTransportError(err)
 		log.Error().
 			Err(err).
 			Str("webhook", webhook.Name).

internal/notifications/webhook_confidentiality_contract_test.go

@@ -0,0 +1,158 @@
+package notifications
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
+	"io"
+	"net/http"
+	"net/url"
+	"strings"
+	"testing"
+	"time"
+)
+
+// Bounded Delivery assessment. All credentials are synthetic; no network sends.
+func TestDeliveryEncodedQueryConfidentiality(t *testing.T) {
+	for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
+		t.Run(key, func(t *testing.T) {
+			encoded := "%" + "74" + key[1:]
+			// Encode the first byte of each already-supported query key.
+			switch key[0] {
+			case 'a':
+				encoded = "%61" + key[1:]
+			case 'k':
+				encoded = "%6b" + key[1:]
+			case 's':
+				encoded = "%73" + key[1:]
+			case 'p':
+				encoded = "%70" + key[1:]
+			}
+			target := "https://example.test/hook?" + encoded + "=delivery-fixture-secret&channel=ops"
+			parsed, err := url.Parse(target)
+			if err != nil || parsed.Query().Get(key) != "delivery-fixture-secret" {
+				t.Fatal("invalid fixture")
+			}
+			if strings.Contains(RedactWebhookURLSecrets(target), "delivery-fixture-secret") {
+				t.Error("URL helper exposes encoded-key credential")
+			}
+			if strings.Contains(RedactWebhookDiagnosticSecrets("Post "+target+" failed"), "delivery-fixture-secret") {
+				t.Error("diagnostic helper exposes encoded-key credential")
+			}
+			var captured bytes.Buffer
+			original := log.Logger
+			log.Logger = zerolog.New(&captured)
+			defer func() { log.Logger = original }()
+			manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
+			for range WebhookRateLimitMax + 2 {
+				manager.checkWebhookRateLimit(target)
+			}
+			if !strings.Contains(captured.String(), "rate limit exceeded") {
+				t.Fatal("log path not reached")
+			}
+			if strings.Contains(captured.String(), "delivery-fixture-secret") {
+				t.Error("rate-limit log exposes encoded-key credential")
+			}
+		})
+	}
+}
+
+type deliveryFailTransport struct{ cause error }
+
+func (d deliveryFailTransport) RoundTrip(*http.Request) (*http.Response, error) { return nil, d.cause }
+func TestDeliveryResolvedNtfyConfidentiality(t *testing.T) {
+	for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
+		t.Run(key, func(t *testing.T) { testResolvedNtfyConfidentiality(t, key) })
+	}
+}
+func testResolvedNtfyConfidentiality(t *testing.T, key string) {
+	manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
+	if err := manager.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil {
+		t.Fatal(err)
+	}
+	cause := errors.New("synthetic transport failure")
+	manager.webhookClient = &http.Client{Transport: deliveryFailTransport{cause}}
+	var captured bytes.Buffer
+	original := log.Logger
+	log.Logger = zerolog.New(&captured)
+	defer func() { log.Logger = original }()
+	err := manager.sendResolvedWebhookNtfy(WebhookConfig{Name: "fixture", Service: "ntfy", URL: fmt.Sprintf("http://127.0.0.1/topic?%s=delivery-fixture-secret&%%%02x%s=delivery-fixture-secret", key, key[0], key[1:])}, nil, time.Now())
+	if !errors.Is(err, cause) {
+		t.Fatalf("transport not reached: %v", err)
+	}
+	if strings.Contains(err.Error(), "delivery-fixture-secret") {
+		t.Error("resolved ntfy transport error exposes literal token credential")
+	}
+	if !strings.Contains(captured.String(), "failed to send resolved ntfy webhook") {
+		t.Fatal("error log not reached")
+	}
+	if strings.Contains(captured.String(), "delivery-fixture-secret") {
+		t.Error("resolved ntfy error log exposes literal token credential")
+	}
+}
+
+// A finite cross-sink matrix: no network sends or persistent queue.
+func TestWebhookConfidentialityCallerMatrix(t *testing.T) {
+	targets := []string{
+		"https://fixture-user:fixture-secret@example.test/hook",
+		"https://hooks.slack.com/services/team/id/fixture-secret",
+		"https://hooks.slack-gov.com/legacy/fixture-secret",
+		"https://discord.com/api/v10/webhooks/id/fixture-secret",
+		"https://discordapp.com/api/webhooks/id/fixture-secret",
+		"https://api.telegram.org/%62otfixture-secret/sendMessage",
+		"http://127.0.0.1:8081/botfixture-secret/sendMessage",
+	}
+	for _, key := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
+		encoded := fmt.Sprintf("%%%02x%s", key[0], key[1:])
+		targets = append(targets, "https://example.test/hook?"+key+"=fixture-secret&"+encoded+"=fixture-secret&channel=ops")
+	}
+	for _, target := range targets {
+		t.Run(target, func(t *testing.T) {
+			want := RedactWebhookURLSecrets(target)
+			if strings.Contains(want, "fixture-secret") || strings.Contains(want, "fixture-user") {
+				t.Fatal("unsafe URL")
+			}
+			if got := RedactWebhookDiagnosticSecrets("Post " + target + " failed"); got != "Post "+want+" failed" {
+				t.Fatalf("context lost: %s", got)
+			}
+			var captured bytes.Buffer
+			original := log.Logger
+			log.Logger = zerolog.New(&captured)
+			defer func() { log.Logger = original }()
+			manager := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)}
+			for range WebhookRateLimitMax + 2 {
+				manager.checkWebhookRateLimit(target)
+			}
+			if strings.Contains(captured.String(), "fixture-secret") || !strings.Contains(captured.String(), "rate limit exceeded") {
+				t.Fatal("unsafe/missing rate-limit log")
+			}
+			cause := errors.New("synthetic transport failure")
+			payload := []byte(`{"event":"unchanged"}`)
+			sent := false
+			manager.webhookClient = &http.Client{Transport: confidentialityTransport(func(req *http.Request) (*http.Response, error) {
+				sent = true
+				body, err := io.ReadAll(req.Body)
+				if err != nil || !bytes.Equal(body, payload) || req.URL.String() != target || req.Header.Get("X-Pulse-Event-ID") != "event-1" {
+					t.Error("request identity changed")
+				}
+				return nil, cause
+			})}
+			_, err := manager.executeWebhookRequest(WebhookConfig{URL: target}, payload, webhookRequestOptions{eventID: "event-1"})
+			if strings.Contains(target, "fixture-user") {
+				if sent || err == nil || !strings.Contains(err.Error(), "URL userinfo is not allowed") || strings.Contains(err.Error(), "fixture-secret") {
+					t.Fatalf("userinfo must be safely rejected: %v", err)
+				}
+				return
+			}
+			if !sent || !errors.Is(err, cause) || strings.Contains(err.Error(), "fixture-secret") || !strings.Contains(err.Error(), "synthetic transport failure") {
+				t.Fatalf("unsafe/missing transport failure: %v", err)
+			}
+		})
+	}
+}
+
+type confidentialityTransport func(*http.Request) (*http.Response, error)
+
+func (f confidentialityTransport) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) }

internal/notifications/webhook_url_redaction.go

@@ -60,39 +60,29 @@ func RedactWebhookURLSecrets(urlString string) string {
 		urlString = parsed.String()
 	}
 
-	queryIndex := strings.Index(urlString, "?")
-	if queryIndex == -1 {
-		return urlString
-	}
-
-	for _, parameter := range []string{"token", "apikey", "api_key", "key", "secret", "password"} {
-		pattern := parameter + "="
-		searchStart := queryIndex
-		for {
-			parameterIndex := strings.Index(urlString[searchStart:], pattern)
-			if parameterIndex == -1 {
-				break
+	// Decode names exactly once, as net/url does, but retain the original
+	// spelling, order and unrelated values in diagnostic URLs. Inspect every
+	// occurrence rather than Query().Get(), which would miss repeated keys.
+	parts := strings.Split(parsed.RawQuery, "&")
+	changed := false
+	for i, part := range parts {
+		name, _, hasValue := strings.Cut(part, "=")
+		decoded, err := url.QueryUnescape(name)
+		if err != nil {
+			return invalidWebhookURLDiagnostic
+		}
+		switch decoded {
+		case "token", "apikey", "api_key", "key", "secret", "password":
+			if hasValue {
+				parts[i] = name + "=REDACTED"
+				changed = true
 			}
-			parameterIndex += searchStart
-
-			if parameterIndex > 0 {
-				previous := urlString[parameterIndex-1]
-				if previous != '?' && previous != '&' {
-					searchStart = parameterIndex + len(pattern)
-					continue
-				}
-			}
-
-			valueStart := parameterIndex + len(pattern)
-			valueEnd := valueStart
-			for valueEnd < len(urlString) && urlString[valueEnd] != '&' && urlString[valueEnd] != '#' {
-				valueEnd++
-			}
-			urlString = urlString[:valueStart] + "REDACTED" + urlString[valueEnd:]
-			searchStart = valueStart + len("REDACTED")
 		}
 	}
-
+	if changed {
+		parsed.RawQuery = strings.Join(parts, "&")
+		return parsed.String()
+	}
 	return urlString
 }
 

internal/notifications/webhook_url_redaction_test.go

@@ -16,6 +16,8 @@ func TestRedactWebhookURLSecrets(t *testing.T) {
 		input string
 		want  string
 	}{
+		"malformed query name":             {input: "https://example.test/hook?%zz=secret", want: invalidWebhookURLDiagnostic},
+		"encoded lookalike":                {input: "https://example.test/hook?extra_%74oken=visible&channel=ops#fragment", want: "https://example.test/hook?extra_%74oken=visible&channel=ops#fragment"},
 		"discord":                          {input: "https://discord.com/api/webhooks/123/discord-secret", want: "https://discord.com/api/webhooks/REDACTED"},
 		"discord versioned":                {input: "https://discord.com/api/v10/webhooks/123/discord-secret", want: "https://discord.com/api/v10/webhooks/REDACTED"},
 		"discord legacy":                   {input: "https://discordapp.com/api/webhooks/123/discord-secret", want: "https://discordapp.com/api/webhooks/REDACTED"},