From 192a72e05c06b2a2ff3a04bb2ef53ded78950e47 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:02:25 +0100 Subject: [PATCH] fix(notifications): redact Telegram credentials from decoded paths Raw URL matching missed encoded bot prefixes and mistook bot hostnames and query URLs for credential paths. Use the decoded path and clear RawPath so diagnostic errors and rate-limit logs cannot retain these synthetic bot tokens, while preserving local API server support and original destinations. Change-source: pulse-maintainer --- .../v6/internal/subsystems/notifications.md | 16 ++++++++++ .../notifications/webhook_url_redaction.go | 19 ++++++----- .../webhook_url_redaction_test.go | 32 +++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 57e75b308..06252700e 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -729,3 +729,19 @@ identifies the secure webhook token and token-authorised operations. Focused synthetic regressions cover helper output, transport diagnostics and actual rate-limit logs. This is not evidence of customer exposure, recipient receipt, release qualification, or protection of arbitrary custom-host credentials. + +### Telegram diagnostic path parsing + +Telegram bot-path masking operates on the parsed, decoded URL path and clears +RawPath after replacement. This covers percent-encoded bot prefixes without +mistaking a hostname or a URL inside a query for a bot path. Method suffixes, +query diagnostics and fragments remain intact; configured destinations and +transport-error causes are unchanged. Host-independent masking is retained for +local API servers, which are supported by the +[Telegram API documentation](https://core.telegram.org/bots/api#making-requests). + +Focused regression tests cover escaped prefixes/tokens, local servers, missing +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. diff --git a/internal/notifications/webhook_url_redaction.go b/internal/notifications/webhook_url_redaction.go index 26ba2f75e..fe5964ba4 100644 --- a/internal/notifications/webhook_url_redaction.go +++ b/internal/notifications/webhook_url_redaction.go @@ -44,15 +44,18 @@ func RedactWebhookURLSecrets(urlString string) 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 { - urlString = urlString[:idx+4] + "REDACTED" + urlString[idx+4+endIdx:] - } else if queryIdx := strings.Index(urlString[idx+4:], "?"); queryIdx != -1 { - urlString = urlString[:idx+4] + "REDACTED" + urlString[idx+4+queryIdx:] - } else { - urlString = urlString[:idx+4] + "REDACTED" + // Telegram also supports local API servers, so retain host-independent + // masking, but inspect only the decoded path. Searching the whole URL + // misses escaped prefixes and can mistake hostnames or query URLs for + // bot credentials. Clear RawPath to prevent escaped secrets resurfacing. + if idx := strings.Index(parsed.Path, "/bot"); idx != -1 { + end := len(parsed.Path) + if suffix := strings.Index(parsed.Path[idx+4:], "/"); suffix != -1 { + end = idx + 4 + suffix } + parsed.Path = parsed.Path[:idx+4] + "REDACTED" + parsed.Path[end:] + parsed.RawPath = "" + urlString = parsed.String() } queryIndex := strings.Index(urlString, "?") diff --git a/internal/notifications/webhook_url_redaction_test.go b/internal/notifications/webhook_url_redaction_test.go index ec2ff8290..93e27754d 100644 --- a/internal/notifications/webhook_url_redaction_test.go +++ b/internal/notifications/webhook_url_redaction_test.go @@ -53,6 +53,13 @@ func TestRedactWebhookURLSecrets(t *testing.T) { input: "https://gotify.example/message?token=gotify-secret", want: "https://gotify.example/message?token=REDACTED", }, + "telegram escaped prefix": {input: "https://api.telegram.org/%62ot123:telegram-secret/sendMessage", want: "https://api.telegram.org/botREDACTED/sendMessage"}, + "telegram escaped token": {input: "https://api.telegram.org/bot123:telegram%2Dsecret/sendMessage", want: "https://api.telegram.org/botREDACTED/sendMessage"}, + "telegram local escaped prefix": {input: "http://localhost:8081/%62ot123:telegram-secret/sendMessage", want: "http://localhost:8081/botREDACTED/sendMessage"}, + "telegram no method with URL query": {input: "https://api.telegram.org/bot123:telegram-secret?next=https://example.org/status", want: "https://api.telegram.org/botREDACTED?next=https://example.org/status"}, + "telegram fragment": {input: "https://api.telegram.org/bot123:telegram-secret#diagnostic", want: "https://api.telegram.org/botREDACTED#diagnostic"}, + "bot hostname is not a path": {input: "https://bot.example.org/hook?channel=ops", want: "https://bot.example.org/hook?channel=ops"}, + "bot query is not a path": {input: "https://example.org/hook?next=https://bot.example.org/status", want: "https://example.org/hook?next=https://bot.example.org/status"}, "telegram path and query": { input: "https://api.telegram.org/bot123:secret/send?token=query-secret", want: "https://api.telegram.org/botREDACTED/send?token=REDACTED", @@ -179,3 +186,28 @@ func TestDiscordWebhookDiagnosticsRedactPath(t *testing.T) { t.Fatalf("unsafe rate-limit diagnostic: %s", out) } } + +func TestTelegramWebhookDiagnosticsRedactPath(t *testing.T) { + const webhookURL = "https://api.telegram.org/%62ot123:telegram-secret/sendMessage" + cause := errors.New("connection refused") + original := &url.Error{Op: "Post", URL: webhookURL, Err: cause} + redacted := redactWebhookTransportError(original) + if strings.Contains(redacted.Error(), "telegram-secret") || !strings.Contains(redacted.Error(), "/botREDACTED/sendMessage") { + t.Fatalf("unsafe transport diagnostic: %v", redacted) + } + if original.URL != webhookURL || !errors.Is(redacted, cause) { + t.Fatal("transport error identity or cause changed") + } + var captured bytes.Buffer + logger := log.Logger + log.Logger = zerolog.New(&captured) + t.Cleanup(func() { log.Logger = logger }) + nm := &NotificationManager{webhookRateLimits: make(map[string]*webhookRateLimit)} + for range WebhookRateLimitMax + 2 { + nm.checkWebhookRateLimit(webhookURL) + } + out := captured.String() + if !strings.Contains(out, "rate limit exceeded") || !strings.Contains(out, "/botREDACTED/sendMessage") || strings.Contains(out, "telegram-secret") { + t.Fatalf("unsafe rate-limit diagnostic: %s", out) + } +}