From 9647313570bcae2123ffee13d4f62dfc25aea8b4 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sat, 29 Nov 2025 17:36:23 +0000 Subject: [PATCH] ADA: Fix redactSecretsFromURL bug and add unit tests The Telegram bot token redaction had an off-by-one bug: it searched for the next "/" starting from the "/bot" position, which found the "/" in "/bot" itself (offset 0) instead of the next "/" after the token. Result: tokens were not properly redacted and the URL got corrupted with duplicated path segments, potentially leaking secrets to logs/API responses. Fix: search from idx+4 (after "/bot") and handle edge cases where there's no trailing slash (token at end of URL or before query string). Added 20 comprehensive test cases covering: - No secrets (passthrough) - Telegram bot tokens (various patterns) - Query parameter secrets (token, apikey, api_key, key, secret, password) - Multiple parameters and edge cases --- internal/api/notifications.go | 12 ++- internal/api/notifications_test.go | 125 +++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 internal/api/notifications_test.go diff --git a/internal/api/notifications.go b/internal/api/notifications.go index 50a08c0..cbd888d 100644 --- a/internal/api/notifications.go +++ b/internal/api/notifications.go @@ -468,8 +468,16 @@ func redactSecretsFromURL(urlStr string) string { // Redact Telegram bot tokens if idx := strings.Index(urlStr, "/bot"); idx != -1 { - if endIdx := strings.Index(urlStr[idx:], "/"); endIdx != -1 { - urlStr = urlStr[:idx+4] + "REDACTED" + urlStr[idx+endIdx:] + // Search for next "/" after "/bot" (starting at idx+4) + if endIdx := strings.Index(urlStr[idx+4:], "/"); endIdx != -1 { + urlStr = urlStr[:idx+4] + "REDACTED" + urlStr[idx+4+endIdx:] + } else { + // No trailing slash - token extends to end of URL or query string + if qIdx := strings.Index(urlStr[idx+4:], "?"); qIdx != -1 { + urlStr = urlStr[:idx+4] + "REDACTED" + urlStr[idx+4+qIdx:] + } else { + urlStr = urlStr[:idx+4] + "REDACTED" + } } } diff --git a/internal/api/notifications_test.go b/internal/api/notifications_test.go new file mode 100644 index 0000000..576a455 --- /dev/null +++ b/internal/api/notifications_test.go @@ -0,0 +1,125 @@ +package api + +import "testing" + +func TestRedactSecretsFromURL(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + // No secrets - should pass through unchanged + { + name: "no secrets in URL", + input: "https://example.com/webhook", + expected: "https://example.com/webhook", + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "URL with unrelated query params", + input: "https://example.com/api?foo=bar&baz=qux", + expected: "https://example.com/api?foo=bar&baz=qux", + }, + + // Telegram bot token patterns + { + name: "telegram bot token with sendMessage", + input: "https://api.telegram.org/bot123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11/sendMessage", + expected: "https://api.telegram.org/botREDACTED/sendMessage", + }, + { + name: "telegram bot token no trailing path", + input: "https://api.telegram.org/bot123456:ABC-token", + expected: "https://api.telegram.org/botREDACTED", + }, + { + name: "telegram bot token with query string", + input: "https://api.telegram.org/bot123456:ABC-token?chat_id=123", + expected: "https://api.telegram.org/botREDACTED?chat_id=123", + }, + { + name: "telegram bot token with path and query", + input: "https://api.telegram.org/bot123456:token/sendMessage?chat_id=123", + expected: "https://api.telegram.org/botREDACTED/sendMessage?chat_id=123", + }, + + // Query parameter secrets + { + name: "token query param", + input: "https://example.com/webhook?token=secret123", + expected: "https://example.com/webhook?token=REDACTED", + }, + { + name: "apikey query param", + input: "https://example.com/api?apikey=xyz123", + expected: "https://example.com/api?apikey=REDACTED", + }, + { + name: "api_key query param with underscore", + input: "https://example.com/api?api_key=xyz123", + expected: "https://example.com/api?api_key=REDACTED", + }, + { + name: "key query param", + input: "https://example.com/api?key=mykey123", + expected: "https://example.com/api?key=REDACTED", + }, + { + name: "secret query param", + input: "https://example.com/api?secret=mysecret", + expected: "https://example.com/api?secret=REDACTED", + }, + { + name: "password query param", + input: "https://example.com/api?password=pass123", + expected: "https://example.com/api?password=REDACTED", + }, + + // Multiple parameters + { + name: "secret param with other params before", + input: "https://example.com/api?foo=bar&token=secret", + expected: "https://example.com/api?foo=bar&token=REDACTED", + }, + { + name: "secret param with other params after", + input: "https://example.com/api?token=secret&foo=bar", + expected: "https://example.com/api?token=REDACTED&foo=bar", + }, + { + name: "multiple different secret params", + input: "https://example.com/api?token=tok&apikey=key", + expected: "https://example.com/api?token=REDACTED&apikey=REDACTED", + }, + + // Edge cases + { + name: "secret param with fragment", + input: "https://example.com/api?token=secret#section", + expected: "https://example.com/api?token=REDACTED#section", + }, + { + name: "bot in path but not telegram pattern", + input: "https://example.com/robots.txt", + expected: "https://example.com/robots.txt", + }, + { + name: "combined telegram and query param secrets", + input: "https://api.telegram.org/bot123:token/send?extra_token=abc", + expected: "https://api.telegram.org/botREDACTED/send?extra_token=REDACTED", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := redactSecretsFromURL(tt.input) + if result != tt.expected { + t.Errorf("redactSecretsFromURL(%q)\ngot: %q\nwant: %q", tt.input, result, tt.expected) + } + }) + } +}