From 9a55d8ddd4a1976b7944631029b5a2e6308be791 Mon Sep 17 00:00:00 2001 From: m4bard <304653687+m4bard@users.noreply.github.com> Date: Sun, 13 Sep 2026 19:41:06 -0500 Subject: [PATCH 1/2] fix(notifications): sanitize webhook URLs before logging them NotificationService.Webhooks.cs logged every outbound provider request at Information level with LogRedaction.RedactText(webhookUrl, GetSensitiveValuesFromEnvironment()). RedactText only replaces values sourced from the process environment, so a webhook URL typed into Settings was never touched and went out whole. The credential lives in the query string for Pushover and Pushbullet and in a path segment for Telegram, Discord and Slack. Add LogRedaction.SanitizeWebhookUrl: keeps scheme/host/port, drops userinfo and the query string (same logic as the existing SanitizeUrl), and for the four providers whose credential lives in the path, masks just the segment(s) carrying it instead of dropping the whole path (api.telegram.org/bot/sendMessage, discord.com/api/webhooks//, hooks.slack.com/services//...). Every other host keeps its path and loses its query, matching SanitizeUrl's existing behavior. Wire it into all 25 {WebhookUrl} logging call sites in NotificationService.Webhooks.cs (NTFY, Pushover, Telegram, Pushbullet, Slack, Discord, generic) and into NotificationDiagnostics.LogFailedResponseAsync, replacing both the RedactText call and the three call sites that already used SanitizeUrl. Also drop the Pushover form body from its Information log line entirely instead of trying to redact it: the body carries token= and user= as plain form fields, which AggressiveRedact cannot catch for the same environment-only reason. Checked the other providers' request bodies (NTFY, Telegram, Pushbullet, Slack, generic) - none of them put a credential in the body, so no change was needed there. Fixes #982. Co-Authored-By: Claude Sonnet 5 --- .../Diagnostics/NotificationDiagnostics.cs | 2 +- .../Security/Redaction/LogRedaction.cs | 68 +++++++++++++++++++ .../Delivery/NotificationService.Webhooks.cs | 58 ++++++++-------- 3 files changed, 99 insertions(+), 29 deletions(-) diff --git a/listenarr.application/Notifications/Diagnostics/NotificationDiagnostics.cs b/listenarr.application/Notifications/Diagnostics/NotificationDiagnostics.cs index da3c219fa..d5a87f369 100644 --- a/listenarr.application/Notifications/Diagnostics/NotificationDiagnostics.cs +++ b/listenarr.application/Notifications/Diagnostics/NotificationDiagnostics.cs @@ -69,7 +69,7 @@ public static async Task LogFailedResponseAsync(HttpResponseMessage response, st logger.LogDebug(ex, "Failed to read notification response body for diagnostic logging"); } - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); var redactedBody = LogRedaction.RedactText(body, LogRedaction.GetSensitiveValuesFromEnvironment()); redactedBody = AggressiveRedact(redactedBody); if (string.IsNullOrEmpty(redactedBody)) redactedBody = ""; diff --git a/listenarr.application/Security/Redaction/LogRedaction.cs b/listenarr.application/Security/Redaction/LogRedaction.cs index c2bbbd571..e877bec9a 100644 --- a/listenarr.application/Security/Redaction/LogRedaction.cs +++ b/listenarr.application/Security/Redaction/LogRedaction.cs @@ -126,6 +126,74 @@ public static string SanitizeUrl(string? url) } } + // Sanitize a notification webhook URL for logging. Keeps scheme, host and port; drops + // userinfo and the whole query string (same as SanitizeUrl); and for providers whose + // credential lives in the path rather than the query, masks the segments that carry it + // instead of dropping the whole path. + public static string SanitizeWebhookUrl(string? url) + { + if (string.IsNullOrWhiteSpace(url)) + return "[empty-url]"; + + try + { + var uri = new Uri(url); + var portSuffix = uri.IsDefaultPort ? string.Empty : $":{uri.Port}"; + var authority = $"{uri.Scheme}://{uri.Host}{portSuffix}"; + var segments = uri.AbsolutePath.Split('/', StringSplitOptions.RemoveEmptyEntries); + + // Telegram: /bot/ - mask the token segment, keep the method. + if (uri.Host.Equals("api.telegram.org", StringComparison.OrdinalIgnoreCase)) + { + if (segments.Length > 0 && segments[0].StartsWith("bot", StringComparison.OrdinalIgnoreCase)) + { + segments[0] = "bot"; + } + + return $"{authority}/{string.Join('/', segments)}"; + } + + // Discord: /api/webhooks// - keep the "webhooks" segment, mask what follows. + if (uri.Host.Equals("discord.com", StringComparison.OrdinalIgnoreCase) + || uri.Host.EndsWith(".discord.com", StringComparison.OrdinalIgnoreCase)) + { + var webhooksIndex = Array.FindIndex(segments, s => s.Equals("webhooks", StringComparison.OrdinalIgnoreCase)); + if (webhooksIndex >= 0) + { + for (var i = webhooksIndex + 1; i < segments.Length; i++) + { + segments[i] = ""; + } + } + + return $"{authority}/{string.Join('/', segments)}"; + } + + // Slack: /services/// - mask everything after "services". + if (uri.Host.Equals("hooks.slack.com", StringComparison.OrdinalIgnoreCase)) + { + var servicesIndex = Array.FindIndex(segments, s => s.Equals("services", StringComparison.OrdinalIgnoreCase)); + if (servicesIndex >= 0) + { + for (var i = servicesIndex + 1; i < segments.Length; i++) + { + segments[i] = ""; + } + } + + return $"{authority}/{string.Join('/', segments)}"; + } + + // Everything else (e.g. Pushover, Pushbullet, NTFY): the credential lives in the + // query string, already dropped above, so the path can be kept as-is. + return $"{authority}{uri.AbsolutePath}"; + } + catch (Exception caughtEx_5) when (caughtEx_5 is not OperationCanceledException && caughtEx_5 is not OutOfMemoryException && caughtEx_5 is not StackOverflowException) + { + return "[invalid-url]"; + } + } + // Sanitize user-provided text for logging (prevent log injection) public static string SanitizeText(string? text, int maxLength = 200) { diff --git a/listenarr.infrastructure/Notifications/Delivery/NotificationService.Webhooks.cs b/listenarr.infrastructure/Notifications/Delivery/NotificationService.Webhooks.cs index dc9e40a2b..bd6d72889 100644 --- a/listenarr.infrastructure/Notifications/Delivery/NotificationService.Webhooks.cs +++ b/listenarr.infrastructure/Notifications/Delivery/NotificationService.Webhooks.cs @@ -52,7 +52,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh imageContent.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue(attachment.ContentType); multipartContent.Add(imageContent, "files[0]", attachment.Filename); - _logger.LogDebug("Posting multipart to {WebhookUrl} (attachment filename={Filename}, size={Size})", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()), attachment.Filename, attachment.ImageData?.Length ?? 0); + _logger.LogDebug("Posting multipart to {WebhookUrl} (attachment filename={Filename}, size={Size})", LogRedaction.SanitizeWebhookUrl(webhookUrl), attachment.Filename, attachment.ImageData?.Length ?? 0); var response = await PostValidatedAsync(webhookUrl, multipartContent); if (!response.IsSuccessStatusCode) await NotificationDiagnostics.LogFailedResponseAsync(response, webhookUrl, _logger); } @@ -66,7 +66,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Discord notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Discord notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } catch (OperationCanceledException) { @@ -77,7 +77,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh #pragma warning disable CA1031 catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) { - _logger.LogError(ex, "Error sending Discord notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending Discord notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } #pragma warning restore CA1031 @@ -104,7 +104,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh request.Headers.TryAddWithoutValidation("Priority", "3"); request.Headers.TryAddWithoutValidation("Tags", trigger); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); var headers = string.Join(", ", request.Headers.Select(h => $"{h.Key}={string.Join(';', h.Value)}")); var requestBody = request.Content != null ? await request.Content.ReadAsStringAsync() : string.Empty; var redactedRequestBody = NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(requestBody, LogRedaction.GetSensitiveValuesFromEnvironment())); @@ -123,7 +123,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending NTFY notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending NTFY notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } catch (OperationCanceledException) { @@ -132,11 +132,11 @@ public async Task SendNotificationAsync(string trigger, object data, string webh // OperationCanceledException is handled above (re-thrown). No TaskCanceledException handler here. catch (JsonException ex) { - _logger.LogError(ex, "JSON error while building NTFY notification payload for {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "JSON error while building NTFY notification payload for {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } catch (InvalidOperationException ex) { - _logger.LogError(ex, "Invalid operation while sending NTFY notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Invalid operation while sending NTFY notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } } @@ -154,7 +154,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh if (string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(user)) { - _logger.LogWarning("Pushover webhook URL missing 'token' or 'user' query parameter: {WebhookUrl}", LogRedaction.SanitizeUrl(webhookUrl)); + _logger.LogWarning("Pushover webhook URL missing 'token' or 'user' query parameter: {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); // Fall through to generic webhook behaviour below } else @@ -172,10 +172,12 @@ public async Task SendNotificationAsync(string trigger, object data, string webh }; using var content = new FormUrlEncodedContent(values); - var requestBody = await NotificationDiagnostics.TryReadContentAsync(content, _logger); - var redactedRequestBody = NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(requestBody, LogRedaction.GetSensitiveValuesFromEnvironment())); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); - _logger.LogInformation("Sending Pushover POST to {WebhookUrl} with body: {Body}", redactedUrl, redactedRequestBody); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); + // The Pushover form body carries the token/user credentials as plain fields + // (token=, user=), which AggressiveRedact cannot catch since they are not + // sourced from the environment. Drop the body from the log entirely rather + // than half-redact it. + _logger.LogInformation("Sending Pushover POST to {WebhookUrl}", redactedUrl); // Post to the base path (without query) to comply with Pushover API expectations var postUrl = uri.GetLeftPart(UriPartial.Path); @@ -192,7 +194,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Pushover notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Pushover notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } catch (OperationCanceledException) @@ -203,7 +205,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh && ex is not StackOverflowException && ex is not ThreadAbortException) { - _logger.LogError(ex, "Error sending Pushover notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending Pushover notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } } @@ -220,7 +222,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh if (string.IsNullOrWhiteSpace(chatId)) { - _logger.LogWarning("Telegram webhook URL missing 'chat_id' query parameter: {WebhookUrl}", LogRedaction.SanitizeUrl(webhookUrl)); + _logger.LogWarning("Telegram webhook URL missing 'chat_id' query parameter: {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); // Fall through to generic webhook behaviour below } else @@ -233,7 +235,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh var json = JsonSerializer.Serialize(telegramBody); using var content = new StringContent(json, Encoding.UTF8, "application/json"); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); _logger.LogInformation("Sending Telegram POST to {WebhookUrl} with body: {Body}", redactedUrl, NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(json, LogRedaction.GetSensitiveValuesFromEnvironment()))); var response = await PostValidatedAsync(webhookUrl, content); @@ -249,7 +251,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Telegram notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Telegram notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } catch (OperationCanceledException) @@ -261,7 +263,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh #pragma warning disable CA1031 catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) { - _logger.LogError(ex, "Error sending Telegram notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending Telegram notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } #pragma warning restore CA1031 @@ -293,7 +295,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh if (string.IsNullOrWhiteSpace(token)) { - _logger.LogWarning("Pushbullet webhook URL missing access token: {WebhookUrl}", LogRedaction.SanitizeUrl(webhookUrl)); + _logger.LogWarning("Pushbullet webhook URL missing access token: {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); // Fall through to generic webhook behaviour below } else @@ -320,7 +322,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh }; request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); var redactedBody = NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(json, LogRedaction.GetSensitiveValuesFromEnvironment())); _logger.LogInformation("Sending Pushbullet POST to {WebhookUrl} with body: {Body}", redactedUrl, redactedBody); @@ -337,7 +339,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Pushbullet notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Pushbullet notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } catch (OperationCanceledException) @@ -349,7 +351,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh #pragma warning disable CA1031 catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) { - _logger.LogError(ex, "Error sending Pushbullet notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending Pushbullet notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } #pragma warning restore CA1031 @@ -373,7 +375,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh var json = slackObj.ToJsonString(); using var content = new StringContent(json, Encoding.UTF8, "application/json"); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); var redactedBody = NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(json, LogRedaction.GetSensitiveValuesFromEnvironment())); _logger.LogInformation("Sending Slack POST to {WebhookUrl} with body: {Body}", redactedUrl, redactedBody); @@ -389,7 +391,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Slack notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Slack notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } catch (OperationCanceledException) @@ -401,7 +403,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh #pragma warning disable CA1031 catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) { - _logger.LogError(ex, "Error sending Slack notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending Slack notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); return; } #pragma warning restore CA1031 @@ -417,7 +419,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh using var defaultContent = new StringContent(defaultJson, Encoding.UTF8, "application/json"); - var redactedUrl = LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment()); + var redactedUrl = LogRedaction.SanitizeWebhookUrl(webhookUrl); var redactedBody = NotificationDiagnostics.AggressiveRedact(LogRedaction.RedactText(defaultJson, LogRedaction.GetSensitiveValuesFromEnvironment())); _logger.LogInformation("Sending Generic POST to {WebhookUrl} with body: {Body}", redactedUrl, redactedBody); @@ -429,7 +431,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh } catch (HttpRequestException ex) { - _logger.LogError(ex, "HTTP error sending Generic notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "HTTP error sending Generic notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } catch (OperationCanceledException) { @@ -440,7 +442,7 @@ public async Task SendNotificationAsync(string trigger, object data, string webh #pragma warning disable CA1031 catch (Exception ex) when (ex is not OperationCanceledException && ex is not OutOfMemoryException && ex is not StackOverflowException) { - _logger.LogError(ex, "Error sending notification to {WebhookUrl}", LogRedaction.RedactText(webhookUrl, LogRedaction.GetSensitiveValuesFromEnvironment())); + _logger.LogError(ex, "Error sending notification to {WebhookUrl}", LogRedaction.SanitizeWebhookUrl(webhookUrl)); } #pragma warning restore CA1031 } From 86ff24dbaa613a892874c426e1ce81985fea0493 Mon Sep 17 00:00:00 2001 From: m4bard <304653687+m4bard@users.noreply.github.com> Date: Sun, 13 Sep 2026 19:46:35 -0500 Subject: [PATCH 2/2] test(notifications): cover SanitizeWebhookUrl and pin the webhook log call sites Per-provider unit tests for LogRedaction.SanitizeWebhookUrl (Telegram, Discord, Slack, Pushover, plus a control case with no credential-shaped content, so the tests can't pass from a version that just blanks the whole URL). Also three NotificationService integration tests that exercise the real Pushover/Telegram/Discord code paths in NotificationService.Webhooks.cs with a credential that is deliberately not an environment variable: if any of those call sites regressed to LogRedaction.RedactText(webhookUrl, GetSensitiveValuesFromEnvironment()), the credential would sail straight through into the captured log line and these would fail. Co-Authored-By: Claude Sonnet 5 --- .../Security/Redaction/LogRedactionTests.cs | 71 ++++++++++ .../Redaction/SecurityRedactionTests.cs | 134 ++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/tests/Features/Application/Security/Redaction/LogRedactionTests.cs b/tests/Features/Application/Security/Redaction/LogRedactionTests.cs index d99c01a26..3012148e3 100644 --- a/tests/Features/Application/Security/Redaction/LogRedactionTests.cs +++ b/tests/Features/Application/Security/Redaction/LogRedactionTests.cs @@ -63,5 +63,76 @@ public void GetSensitiveValuesFromEnvironment_ReturnsSetVariables() Environment.SetEnvironmentVariable(key, null); } } + + [Fact] + public void SanitizeWebhookUrl_Telegram_MasksTokenSegment_KeepsMethodAndQueryDropped() + { + var token = "123456789:AAExampleBotTokenValueNotReal"; + var url = $"https://api.telegram.org/bot{token}/sendMessage?chat_id=987654321"; + + var sanitized = LogRedaction.SanitizeWebhookUrl(url); + + Assert.DoesNotContain(token, sanitized); + Assert.DoesNotContain("987654321", sanitized); + Assert.Equal("https://api.telegram.org/bot/sendMessage", sanitized); + } + + [Fact] + public void SanitizeWebhookUrl_Discord_MasksIdAndTokenSegments_KeepsApiWebhooksPath() + { + var id = "111222333444555666"; + var token = "DiscordWebhookTokenValueNotReal"; + var url = $"https://discord.com/api/webhooks/{id}/{token}"; + + var sanitized = LogRedaction.SanitizeWebhookUrl(url); + + Assert.DoesNotContain(id, sanitized); + Assert.DoesNotContain(token, sanitized); + Assert.Contains("api/webhooks", sanitized); + Assert.Equal("https://discord.com/api/webhooks//", sanitized); + } + + [Fact] + public void SanitizeWebhookUrl_Slack_MasksEverythingAfterServices() + { + var team = "T00000000"; + var bot = "B00000000"; + var secret = "XXXXXXXXXXXXXXXXXXXXXXXX"; + var url = $"https://hooks.slack.com/services/{team}/{bot}/{secret}"; + + var sanitized = LogRedaction.SanitizeWebhookUrl(url); + + Assert.DoesNotContain(team, sanitized); + Assert.DoesNotContain(bot, sanitized); + Assert.DoesNotContain(secret, sanitized); + Assert.StartsWith("https://hooks.slack.com/services/", sanitized); + } + + [Fact] + public void SanitizeWebhookUrl_Pushover_DropsQueryString_KeepsHostAndPath() + { + var token = "app-token-abc123"; + var user = "user-key-xyz789"; + var url = $"https://api.pushover.net/1/messages.json?token={token}&user={user}"; + + var sanitized = LogRedaction.SanitizeWebhookUrl(url); + + Assert.DoesNotContain(token, sanitized); + Assert.DoesNotContain(user, sanitized); + Assert.DoesNotContain("?", sanitized); + Assert.Equal("https://api.pushover.net/1/messages.json", sanitized); + } + + [Fact] + public void SanitizeWebhookUrl_ControlCase_NonCredentialUrl_PathSurvivesIntact() + { + // No query string, no known-provider host, no credential-shaped path segments. + // A version that just blanks everything would fail this assertion, not just the ones above. + var url = "https://example.com/some/harmless/path"; + + var sanitized = LogRedaction.SanitizeWebhookUrl(url); + + Assert.Equal("https://example.com/some/harmless/path", sanitized); + } } } diff --git a/tests/Features/Application/Security/Redaction/SecurityRedactionTests.cs b/tests/Features/Application/Security/Redaction/SecurityRedactionTests.cs index 6ff720938..0722937d7 100644 --- a/tests/Features/Application/Security/Redaction/SecurityRedactionTests.cs +++ b/tests/Features/Application/Security/Redaction/SecurityRedactionTests.cs @@ -16,6 +16,7 @@ * along with this program. If not, see . */ using System.Net; +using System.Text.Json.Nodes; namespace Listenarr.Tests.Features.Application.Security.Redaction { @@ -177,5 +178,138 @@ public async Task DiscordController_LogsAreRedacted_WhenTokenValidationFails() Assert.NotNull(capturedLog); Assert.Contains("", capturedLog); } + + private static Mock> CreateCapturingLogger(List capturedLogs) + { + var mockLogger = new Mock>(); + mockLogger + .Setup(l => l.Log(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), (Func)It.IsAny())) + .Callback(new InvocationAction(invocation => + { + var formatter = invocation.Arguments[4] as Func; + var state = invocation.Arguments[2]; + if (formatter != null) + { + capturedLogs.Add(formatter.Invoke(state!, null)); + } + else if (state != null) + { + capturedLogs.Add(state.ToString() ?? string.Empty); + } + })); + return mockLogger; + } + + // These three tests pin down the actual call sites in NotificationService.Webhooks.cs: + // if any of them regressed to LogRedaction.RedactText(webhookUrl, GetSensitiveValuesFromEnvironment()) + // (the environment-only helper the bug report is about), the credential below is not an + // environment value, so it would sail straight through into the captured log line and these + // assertions would fail. + [Fact] + public async Task NotificationService_PushoverSuccessLog_DoesNotLeakTokenOrUser() + { + var token = "pushover-app-token-not-an-env-secret"; + var user = "pushover-user-key-not-an-env-secret"; + var webhookUrl = $"https://api.pushover.net/1/messages.json?token={token}&user={user}"; + + var handler = new Mock(); + handler + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("1") }); + + using var httpClient = new HttpClient(handler.Object); + + var mockConfigService = new Mock(); + mockConfigService.Setup(x => x.GetStartupConfigAsync()).ReturnsAsync(new StartupConfig { UrlBase = "https://listenarr.example.com" }); + + var services = new ServiceCollection(); + services.AddSingleton(); + var payloadBuilder = services.BuildServiceProvider().GetRequiredService(); + + var capturedLogs = new List(); + var mockLogger = CreateCapturingLogger(capturedLogs); + + var service = new NotificationService(httpClient, mockLogger.Object, mockConfigService.Object, payloadBuilder, Mock.Of()); + + await service.SendNotificationAsync("book-added", new { id = 1, title = "Pushover Test" }, webhookUrl, new List { "book-added" }); + + var sendLog = Assert.Single(capturedLogs, l => l.Contains("Sending Pushover POST", StringComparison.Ordinal)); + Assert.DoesNotContain(token, sendLog); + Assert.DoesNotContain(user, sendLog); + // The form body (token=/user=) must be dropped entirely, not merely redacted. + Assert.DoesNotContain("Body", sendLog, StringComparison.Ordinal); + } + + [Fact] + public async Task NotificationService_TelegramSuccessLog_DoesNotLeakBotToken() + { + var token = "123456789:telegram-bot-token-not-an-env-secret"; + var webhookUrl = $"https://api.telegram.org/bot{token}/sendMessage?chat_id=987654321"; + + var handler = new Mock(); + handler + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .ReturnsAsync(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("{\"ok\":true}") }); + + using var httpClient = new HttpClient(handler.Object); + + var mockConfigService = new Mock(); + mockConfigService.Setup(x => x.GetStartupConfigAsync()).ReturnsAsync(new StartupConfig { UrlBase = "https://listenarr.example.com" }); + + var services = new ServiceCollection(); + services.AddSingleton(); + var payloadBuilder = services.BuildServiceProvider().GetRequiredService(); + + var capturedLogs = new List(); + var mockLogger = CreateCapturingLogger(capturedLogs); + + var service = new NotificationService(httpClient, mockLogger.Object, mockConfigService.Object, payloadBuilder, Mock.Of()); + + await service.SendNotificationAsync("book-added", new { id = 1, title = "Telegram Test" }, webhookUrl, new List { "book-added" }); + + var sendLog = Assert.Single(capturedLogs, l => l.Contains("Sending Telegram POST", StringComparison.Ordinal)); + Assert.DoesNotContain(token, sendLog); + Assert.Contains("api.telegram.org/bot/sendMessage", sendLog); + } + + [Fact] + public async Task NotificationService_DiscordHttpErrorLog_DoesNotLeakWebhookIdOrToken() + { + var id = "222333444555666777"; + var token = "discord-webhook-token-not-an-env-secret"; + var webhookUrl = $"https://discord.com/api/webhooks/{id}/{token}"; + + var handler = new Mock(); + handler + .Protected() + .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) + .ThrowsAsync(new HttpRequestException("simulated network failure")); + + using var httpClient = new HttpClient(handler.Object); + + var mockConfigService = new Mock(); + mockConfigService.Setup(x => x.GetStartupConfigAsync()).ReturnsAsync(new StartupConfig { UrlBase = "https://listenarr.example.com" }); + + var mockPayloadBuilder = new Mock(); + mockPayloadBuilder + .Setup(p => p.CreateDiscordPayloadWithAttachmentAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny?>(), It.IsAny?>(), It.IsAny())) + .ReturnsAsync((new JsonObject { ["content"] = "test" }, (NotificationAttachmentInfo?)null)); + + var capturedLogs = new List(); + var mockLogger = CreateCapturingLogger(capturedLogs); + + var service = new NotificationService(httpClient, mockLogger.Object, mockConfigService.Object, mockPayloadBuilder.Object, Mock.Of()); + + await service.SendNotificationAsync("book-added", new { id = 1, title = "Discord Test" }, webhookUrl, new List { "book-added" }); + + var errorLog = Assert.Single(capturedLogs, l => l.Contains("HTTP error sending Discord notification", StringComparison.Ordinal)); + Assert.DoesNotContain(id, errorLog); + Assert.DoesNotContain(token, errorLog); + Assert.Contains("api/webhooks", errorLog); + } } }