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 } 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); + } } }