Merge candidate 20260907T175505Z-core-runtime

Integrate the reviewed bounded webhook diagnostic confidentiality repair and its exact candidate ancestry.

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"},