From 03c0f27d7e66f9c0e012176f8c07cb4c667c95de Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 12 May 2023 11:19:20 +1200 Subject: [PATCH 01/18] Added a UrlPiiSanitizer that can be used to strip auth details from URLs --- src/Sentry/Internal/UrlPiiSanitizer.cs | 65 +++++++++++++++++++ .../Internals/UrlPiiSanitizerTests.cs | 65 +++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 src/Sentry/Internal/UrlPiiSanitizer.cs create mode 100644 test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs diff --git a/src/Sentry/Internal/UrlPiiSanitizer.cs b/src/Sentry/Internal/UrlPiiSanitizer.cs new file mode 100644 index 0000000000..6d76e11e9e --- /dev/null +++ b/src/Sentry/Internal/UrlPiiSanitizer.cs @@ -0,0 +1,65 @@ +namespace Sentry.Internal; + +/// +/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. +/// +internal class UrlPiiSanitizer +{ + private readonly SentryOptions _options; + + /// + /// Creates a sanitizer that will only send default PII if is set to true. + /// + /// + public UrlPiiSanitizer(SentryOptions options) + { + _options = options; + } + + /// + /// Searches for URLs in text data and redacts any PII + /// data from these, as required. + /// + /// The data to be searched + /// + /// The data if SendDefaultPii is enabled or if the data does not contain any PII. + /// A redacted copy of the data otherwise. + /// + public string? Sanitize(string? data) + { + // If we are sending default PII or the data is empty, we don't need to sanitize anything + if (_options.SendDefaultPii || string.IsNullOrWhiteSpace(data)) + { + return data; + } + + // The pattern @"(?i)\b(https?://.*@.*)\b" uses the \b word boundary anchors to ensure that the match occurs at + // a word boundary. This allows the URL to be matched even if it is part of a larger text. The (?i) flag ensures + // case-insensitive matching for "https" or "http". + var authRegex = new Regex(@"(?i)\b(https?://.*@.*)\b"); + var result = authRegex.Replace(data, match => + { + var matchedUrl = match.Groups[1].Value; + return SanitizeUrl(matchedUrl); + }); + + return result; + } + + private static string SanitizeUrl(string data) + { + // ^ matches the start of the string. (?i)(https?://) gives a case-insensitive matching of the protocol. + // (.*@) matches the username and password (authentication information). (.*)$ matches the rest of the URL. + var userInfoMatcher = new Regex(@"^(?i)(https?://)(.*@)(.*)$").Match(data); + if (userInfoMatcher.Success && userInfoMatcher.Groups.Count == 4) + { + var userInfoString = userInfoMatcher.Groups[2].Value; + var replacementString = userInfoString.Contains(":") ? "[Filtered]:[Filtered]@" : "[Filtered]@"; + return userInfoMatcher.Groups[1].Value + replacementString + userInfoMatcher.Groups[3].Value; + } + else + { + return data; + } + } +} diff --git a/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs b/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs new file mode 100644 index 0000000000..ac8e4bc973 --- /dev/null +++ b/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs @@ -0,0 +1,65 @@ +namespace Sentry.Tests.Internals; + +public class UrlPiiSanitizerTests +{ + private class Fixture + { + internal SentryOptions Options { get; } = new(); + + public UrlPiiSanitizer GetSut() => new(Options); + } + + private readonly Fixture _fixture = new Fixture(); + + [Fact] + public void Sanitize_Null() + { + var sut = _fixture.GetSut(); + + var actual = sut.Sanitize(null); + + Assert.Null(actual); + } + + [Theory] + [InlineData("I'm a harmless string.", "doesn't affect ordinary strings")] + [InlineData("htps://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed https urls")] + [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] + public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) + { + var sut = _fixture.GetSut(); + + var actual = sut.Sanitize(original); + + actual.Should().Be(original, reason); + } + + [Theory] + [InlineData("https://user:password@sentry.io?q=1&s=2&token=secret#top", "https://[Filtered]:[Filtered]@sentry.io?q=1&s=2&token=secret#top", "strips user info with user and password from https")] + [InlineData("https://user:password@sentry.io", "https://[Filtered]:[Filtered]@sentry.io", "strips user info with user and password from https without query")] + [InlineData("https://user@sentry.io", "https://[Filtered]@sentry.io", "strips user info with user only from https without query")] + [InlineData("http://user:password@sentry.io?q=1&s=2&token=secret#top", "http://[Filtered]:[Filtered]@sentry.io?q=1&s=2&token=secret#top", "strips user info with user and password from http")] + [InlineData("http://user:password@sentry.io", "http://[Filtered]:[Filtered]@sentry.io", "strips user info with user and password from http without query")] + [InlineData("http://user@sentry.io", "http://[Filtered]@sentry.io", "strips user info with user only from http without query")] + [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] + public void Sanitize_Data_IsNotNull_WithPii(string original, string expected, string reason) + { + var sut = _fixture.GetSut(); + + var actual = sut.Sanitize(original); + + actual.Should().Be(expected, reason); + } + + [Fact] + public void Sanitize_Data_SendDefaultPii() + { + _fixture.Options.SendDefaultPii = true; + var sut = _fixture.GetSut(); + var original = "https://user:password@sentry.io?q=1&s=2&token=secret#top"; + + var actual = sut.Sanitize(original); + + actual.Should().Be(original, "should not sanitize urls when SendDefaultPii is true"); + } +} From 36c04ff08f37f76e779bea47982718d758ca7535 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 12 May 2023 13:34:05 +1200 Subject: [PATCH 02/18] Urls in the Message or Data of Breadcrumbs are now sanitized --- src/Sentry/Internal/BreadcrumbPiiSanitizer.cs | 29 ++++++++++++ src/Sentry/Internal/UrlPiiSanitizer.cs | 19 ++------ src/Sentry/Scope.cs | 5 ++ .../Internals/BreadcrumbPiiSanitizerTests.cs | 43 +++++++++++++++++ .../Internals/UrlPiiSanitizerTests.cs | 33 ++----------- test/Sentry.Tests/ScopeTests.cs | 47 +++++++++++++++++++ 6 files changed, 131 insertions(+), 45 deletions(-) create mode 100644 src/Sentry/Internal/BreadcrumbPiiSanitizer.cs create mode 100644 test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs diff --git a/src/Sentry/Internal/BreadcrumbPiiSanitizer.cs b/src/Sentry/Internal/BreadcrumbPiiSanitizer.cs new file mode 100644 index 0000000000..043769d64f --- /dev/null +++ b/src/Sentry/Internal/BreadcrumbPiiSanitizer.cs @@ -0,0 +1,29 @@ +namespace Sentry.Internal; + +/// +/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. +/// +internal static class BreadcrumbPiiSanitizer +{ + /// + /// Redacts PII from the breadcrumb message or data + /// + /// The breadcrumb to be sanitized + /// A new Breadcrumb with redacted copies of the message and data in the original + public static Breadcrumb Sanitize(Breadcrumb breadcrumb) + { + var sanitizedData = breadcrumb.Data?.ToDictionary( + x => x.Key, + x => UrlPiiSanitizer.Sanitize(x.Value) ?? string.Empty + ); + + return new Breadcrumb( + timestamp : breadcrumb.Timestamp, + message : UrlPiiSanitizer.Sanitize(breadcrumb.Message), + type : breadcrumb.Type, + data : sanitizedData, + category : breadcrumb.Category, + level : breadcrumb.Level + ); + } +} diff --git a/src/Sentry/Internal/UrlPiiSanitizer.cs b/src/Sentry/Internal/UrlPiiSanitizer.cs index 6d76e11e9e..00696b7131 100644 --- a/src/Sentry/Internal/UrlPiiSanitizer.cs +++ b/src/Sentry/Internal/UrlPiiSanitizer.cs @@ -3,19 +3,8 @@ namespace Sentry.Internal; /// /// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. /// -internal class UrlPiiSanitizer +internal static class UrlPiiSanitizer { - private readonly SentryOptions _options; - - /// - /// Creates a sanitizer that will only send default PII if is set to true. - /// - /// - public UrlPiiSanitizer(SentryOptions options) - { - _options = options; - } - /// /// Searches for URLs in text data and redacts any PII /// data from these, as required. @@ -25,10 +14,10 @@ public UrlPiiSanitizer(SentryOptions options) /// The data if SendDefaultPii is enabled or if the data does not contain any PII. /// A redacted copy of the data otherwise. /// - public string? Sanitize(string? data) + public static string? Sanitize(string? data) { - // If we are sending default PII or the data is empty, we don't need to sanitize anything - if (_options.SendDefaultPii || string.IsNullOrWhiteSpace(data)) + // If the data is empty then we don't need to sanitize anything + if (string.IsNullOrWhiteSpace(data)) { return data; } diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index bd4db02382..30e0e5e6e9 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -270,6 +270,11 @@ public void AddBreadcrumb(Breadcrumb breadcrumb) _breadcrumbs.TryDequeue(out _); } + if (!Options.SendDefaultPii) + { + breadcrumb = BreadcrumbPiiSanitizer.Sanitize(breadcrumb); + } + _breadcrumbs.Enqueue(breadcrumb); if (Options.EnableScopeSync) { diff --git a/test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs b/test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs new file mode 100644 index 0000000000..fbe6f710f4 --- /dev/null +++ b/test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs @@ -0,0 +1,43 @@ +using FluentAssertions.Execution; + +namespace Sentry.Tests.Internals; + +public class BreadcrumbPiiSanitizerTests +{ + [Fact] + public void Sanitize_Urls() + { + // Arrange + var breadcrumbData = new Dictionary + { + {"url", "https://user@sentry.io"}, + {"method", "GET"}, + {"status_code", "403"} + }; + var breadcrumb = new Breadcrumb( + timestamp : DateTimeOffset.UtcNow, + message : "message https://user@sentry.io", + type : "fake_type", + data : breadcrumbData, + category : "fake_category", + level : BreadcrumbLevel.Error + ); + + // Act + var actual = BreadcrumbPiiSanitizer.Sanitize(breadcrumb); + + // Assert + using (new AssertionScope()) + { + actual.Should().NotBeNull(); + actual.Timestamp.Should().Be(breadcrumb.Timestamp); + actual.Message.Should().Be(UrlPiiSanitizer.Sanitize(breadcrumb.Message)); // should be sanitized + actual.Type.Should().Be(breadcrumb.Type); + actual.Data?["url"].Should().Be(UrlPiiSanitizer.Sanitize(breadcrumb.Data?["url"])); // should be sanitized + actual.Data?["method"].Should().Be(breadcrumb.Data?["method"]); + actual.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); + actual.Category.Should().Be(breadcrumb.Category); + actual.Level.Should().Be(breadcrumb.Level); + } + } +} diff --git a/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs b/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs index ac8e4bc973..e9adb3e51d 100644 --- a/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs +++ b/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs @@ -2,21 +2,10 @@ namespace Sentry.Tests.Internals; public class UrlPiiSanitizerTests { - private class Fixture - { - internal SentryOptions Options { get; } = new(); - - public UrlPiiSanitizer GetSut() => new(Options); - } - - private readonly Fixture _fixture = new Fixture(); - [Fact] public void Sanitize_Null() { - var sut = _fixture.GetSut(); - - var actual = sut.Sanitize(null); + var actual = UrlPiiSanitizer.Sanitize(null); Assert.Null(actual); } @@ -27,9 +16,7 @@ public void Sanitize_Null() [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) { - var sut = _fixture.GetSut(); - - var actual = sut.Sanitize(original); + var actual = UrlPiiSanitizer.Sanitize(original); actual.Should().Be(original, reason); } @@ -44,22 +31,8 @@ public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] public void Sanitize_Data_IsNotNull_WithPii(string original, string expected, string reason) { - var sut = _fixture.GetSut(); - - var actual = sut.Sanitize(original); + var actual = UrlPiiSanitizer.Sanitize(original); actual.Should().Be(expected, reason); } - - [Fact] - public void Sanitize_Data_SendDefaultPii() - { - _fixture.Options.SendDefaultPii = true; - var sut = _fixture.GetSut(); - var original = "https://user:password@sentry.io?q=1&s=2&token=secret#top"; - - var actual = sut.Sanitize(original); - - actual.Should().Be(original, "should not sanitize urls when SendDefaultPii is true"); - } } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index d7de09a391..f5e5ab999e 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -462,6 +462,53 @@ public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumbIfEnabled(bool obs // Assert observer.Received(expectedCount).AddBreadcrumb(Arg.Is(breadcrumb)); } + + // AddBreadcrumb sanitizes any URL data in the breadcrumb message + [Fact] + public void AddBreadcrumb_Sanitize_Message() + { + // Arrange + var scope = new Scope(); + var original = "Goto https://user@sentry.io"; + var expected = UrlPiiSanitizer.Sanitize(original); + + // Act + scope.AddBreadcrumb(original); + + // Assert + scope.Breadcrumbs.First().Message.Should().Be(expected); + } + + [Fact] + public void AddBreadcrumb_Sanitize_Data() + { + // Arrange + var scope = new Scope(); + var originalUrl = "https://user@sentry.io"; + var expectedUrl = UrlPiiSanitizer.Sanitize(originalUrl); + + var breadcrumbData = new Dictionary + { + {"url", originalUrl}, + {"method", "GET"}, + {"status_code", "403"} + }; + var breadcrumb = new Breadcrumb(message: "message", data: breadcrumbData); + + // Act + scope.AddBreadcrumb(breadcrumb); + + // Assert + var actual = scope.Breadcrumbs.First(); + using (new AssertionScope()) + { + actual.Should().NotBeNull(); + actual.Message.Should().Be(breadcrumb.Message); + actual.Data?["url"].Should().Be(expectedUrl); + actual.Data?["method"].Should().Be(breadcrumb.Data?["method"]); + actual.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); + } + } } public static class ScopeTestExtensions From 3ec26ffd4287bc15c0ea049ab150e5a8deee8f7b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 12 May 2023 17:00:50 +1200 Subject: [PATCH 03/18] Transaction Description sanitized when the transaction is captured by the SentryClient --- CHANGELOG.md | 1 + ...Sanitizer.cs => PiiBreadcrumbSanitizer.cs} | 8 ++--- .../Internal/PiiTransactionSanitizer.cs | 18 ++++++++++ ...{UrlPiiSanitizer.cs => PiiUrlSanitizer.cs} | 4 +-- src/Sentry/Scope.cs | 2 +- src/Sentry/Sentry.csproj | 34 +++++-------------- src/Sentry/SentryClient.cs | 5 +++ ...ests.cs => PiiBreadcrumbSanitizerTests.cs} | 8 ++--- ...itizerTests.cs => PiiUrlSanitizerTests.cs} | 8 ++--- test/Sentry.Tests/ScopeTests.cs | 4 +-- test/Sentry.Tests/SentryClientTests.cs | 30 ++++++++++++++++ 11 files changed, 79 insertions(+), 43 deletions(-) rename src/Sentry/Internal/{BreadcrumbPiiSanitizer.cs => PiiBreadcrumbSanitizer.cs} (76%) create mode 100644 src/Sentry/Internal/PiiTransactionSanitizer.cs rename src/Sentry/Internal/{UrlPiiSanitizer.cs => PiiUrlSanitizer.cs} (95%) rename test/Sentry.Tests/Internals/{BreadcrumbPiiSanitizerTests.cs => PiiBreadcrumbSanitizerTests.cs} (84%) rename test/Sentry.Tests/Internals/{UrlPiiSanitizerTests.cs => PiiUrlSanitizerTests.cs} (90%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0d92ba6ab..1a6494b1ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Add tag filters to SentryLoggingOptions ([#2360](https://github.com/getsentry/sentry-dotnet/pull/2360)) +- Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) ## 3.31.0 diff --git a/src/Sentry/Internal/BreadcrumbPiiSanitizer.cs b/src/Sentry/Internal/PiiBreadcrumbSanitizer.cs similarity index 76% rename from src/Sentry/Internal/BreadcrumbPiiSanitizer.cs rename to src/Sentry/Internal/PiiBreadcrumbSanitizer.cs index 043769d64f..f960e7c0ca 100644 --- a/src/Sentry/Internal/BreadcrumbPiiSanitizer.cs +++ b/src/Sentry/Internal/PiiBreadcrumbSanitizer.cs @@ -3,23 +3,23 @@ namespace Sentry.Internal; /// /// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. /// -internal static class BreadcrumbPiiSanitizer +internal static class PiiBreadcrumbSanitizer { /// /// Redacts PII from the breadcrumb message or data /// /// The breadcrumb to be sanitized /// A new Breadcrumb with redacted copies of the message and data in the original - public static Breadcrumb Sanitize(Breadcrumb breadcrumb) + public static Breadcrumb Sanitize(this Breadcrumb breadcrumb) { var sanitizedData = breadcrumb.Data?.ToDictionary( x => x.Key, - x => UrlPiiSanitizer.Sanitize(x.Value) ?? string.Empty + x => x.Value.Sanitize() ?? string.Empty ); return new Breadcrumb( timestamp : breadcrumb.Timestamp, - message : UrlPiiSanitizer.Sanitize(breadcrumb.Message), + message : breadcrumb.Message.Sanitize(), type : breadcrumb.Type, data : sanitizedData, category : breadcrumb.Category, diff --git a/src/Sentry/Internal/PiiTransactionSanitizer.cs b/src/Sentry/Internal/PiiTransactionSanitizer.cs new file mode 100644 index 0000000000..83aac7939b --- /dev/null +++ b/src/Sentry/Internal/PiiTransactionSanitizer.cs @@ -0,0 +1,18 @@ +namespace Sentry.Internal; + +/// +/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. +/// +internal static class PiiTransactionSanitizer +{ + /// + /// Redacts PII from the transaction description + /// + /// The transaction to be sanitized + /// The Transaction with redacted description + public static Transaction Sanitize(this Transaction transaction) + { + transaction.Description = transaction.Description.Sanitize(); + return transaction; + } +} diff --git a/src/Sentry/Internal/UrlPiiSanitizer.cs b/src/Sentry/Internal/PiiUrlSanitizer.cs similarity index 95% rename from src/Sentry/Internal/UrlPiiSanitizer.cs rename to src/Sentry/Internal/PiiUrlSanitizer.cs index 00696b7131..4fefae5a0a 100644 --- a/src/Sentry/Internal/UrlPiiSanitizer.cs +++ b/src/Sentry/Internal/PiiUrlSanitizer.cs @@ -3,7 +3,7 @@ namespace Sentry.Internal; /// /// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. /// -internal static class UrlPiiSanitizer +internal static class PiiUrlSanitizer { /// /// Searches for URLs in text data and redacts any PII @@ -14,7 +14,7 @@ internal static class UrlPiiSanitizer /// The data if SendDefaultPii is enabled or if the data does not contain any PII. /// A redacted copy of the data otherwise. /// - public static string? Sanitize(string? data) + public static string? Sanitize(this string? data) { // If the data is empty then we don't need to sanitize anything if (string.IsNullOrWhiteSpace(data)) diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 30e0e5e6e9..56f04e3f0c 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -272,7 +272,7 @@ public void AddBreadcrumb(Breadcrumb breadcrumb) if (!Options.SendDefaultPii) { - breadcrumb = BreadcrumbPiiSanitizer.Sanitize(breadcrumb); + breadcrumb = breadcrumb.Sanitize(); } _breadcrumbs.Enqueue(breadcrumb); diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index 83a535c652..cd05434cac 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -92,35 +92,17 @@ <_OSArchitecture>$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture) - - - - - - - + + + + + + + - + diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 320802a400..71a949a887 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -140,6 +140,11 @@ public void CaptureTransaction(Transaction transaction) return; } + if (!_options.SendDefaultPii) + { + processedTransaction = processedTransaction.Sanitize(); + } + CaptureEnvelope(Envelope.FromTransaction(processedTransaction)); } diff --git a/test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs b/test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs similarity index 84% rename from test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs rename to test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs index fbe6f710f4..c89f631c8d 100644 --- a/test/Sentry.Tests/Internals/BreadcrumbPiiSanitizerTests.cs +++ b/test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs @@ -2,7 +2,7 @@ namespace Sentry.Tests.Internals; -public class BreadcrumbPiiSanitizerTests +public class PiiBreadcrumbSanitizerTests { [Fact] public void Sanitize_Urls() @@ -24,16 +24,16 @@ public void Sanitize_Urls() ); // Act - var actual = BreadcrumbPiiSanitizer.Sanitize(breadcrumb); + var actual = breadcrumb.Sanitize(); // Assert using (new AssertionScope()) { actual.Should().NotBeNull(); actual.Timestamp.Should().Be(breadcrumb.Timestamp); - actual.Message.Should().Be(UrlPiiSanitizer.Sanitize(breadcrumb.Message)); // should be sanitized + actual.Message.Should().Be(PiiUrlSanitizer.Sanitize(breadcrumb.Message)); // should be sanitized actual.Type.Should().Be(breadcrumb.Type); - actual.Data?["url"].Should().Be(UrlPiiSanitizer.Sanitize(breadcrumb.Data?["url"])); // should be sanitized + actual.Data?["url"].Should().Be(PiiUrlSanitizer.Sanitize(breadcrumb.Data?["url"])); // should be sanitized actual.Data?["method"].Should().Be(breadcrumb.Data?["method"]); actual.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); actual.Category.Should().Be(breadcrumb.Category); diff --git a/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs b/test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs similarity index 90% rename from test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs rename to test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs index e9adb3e51d..26c4ada489 100644 --- a/test/Sentry.Tests/Internals/UrlPiiSanitizerTests.cs +++ b/test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs @@ -1,11 +1,11 @@ namespace Sentry.Tests.Internals; -public class UrlPiiSanitizerTests +public class PiiUrlSanitizerTests { [Fact] public void Sanitize_Null() { - var actual = UrlPiiSanitizer.Sanitize(null); + var actual = PiiUrlSanitizer.Sanitize(null); Assert.Null(actual); } @@ -16,7 +16,7 @@ public void Sanitize_Null() [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) { - var actual = UrlPiiSanitizer.Sanitize(original); + var actual = PiiUrlSanitizer.Sanitize(original); actual.Should().Be(original, reason); } @@ -31,7 +31,7 @@ public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] public void Sanitize_Data_IsNotNull_WithPii(string original, string expected, string reason) { - var actual = UrlPiiSanitizer.Sanitize(original); + var actual = PiiUrlSanitizer.Sanitize(original); actual.Should().Be(expected, reason); } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index f5e5ab999e..7aef4ad085 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -470,7 +470,7 @@ public void AddBreadcrumb_Sanitize_Message() // Arrange var scope = new Scope(); var original = "Goto https://user@sentry.io"; - var expected = UrlPiiSanitizer.Sanitize(original); + var expected = original.Sanitize(); // Act scope.AddBreadcrumb(original); @@ -485,7 +485,7 @@ public void AddBreadcrumb_Sanitize_Data() // Arrange var scope = new Scope(); var originalUrl = "https://user@sentry.io"; - var expectedUrl = UrlPiiSanitizer.Sanitize(originalUrl); + var expectedUrl = originalUrl.Sanitize(); var breadcrumbData = new Dictionary { diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index dd71edb322..b8b6f2161b 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -667,6 +667,36 @@ public void CaptureTransaction_DisposedClient_DoesNotThrow() }); } + [Fact] + public void CaptureTransaction_Sanitize_Description() + { + // Arrange + _fixture.SentryOptions.SendDefaultPii = false; + var client = _fixture.GetSut(); + var original = new Transaction( + "test name", + "test operation" + ) + { + IsSampled = true, + Description = "The URL: https://user@sentry.io has PII data in it", + EndTimestamp = DateTimeOffset.Now // finished + }; + + // Act + Envelope envelope = null; + client.Worker.EnqueueEnvelope(Arg.Do(e => envelope = e)); + client.CaptureTransaction(original); + + // Assert + envelope.Should().NotBeNull(); + envelope.Items.Count.Should().Be(1); + var actual = (envelope.Items[0].Payload as JsonSerializable)?.Source as Transaction; + actual?.Name.Should().Be(original.Name); + actual?.Operation.Should().Be(original.Operation); + actual?.Description.Should().Be(original.Description.Sanitize()); // Should be sanitized + } + [Fact] public void CaptureTransaction_BeforeSendTransaction_RejectEvent() { From 98e826c4b122bb1ba81dcc73ef415a61a796069b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 15 May 2023 21:26:18 +1200 Subject: [PATCH 04/18] Moved URL redaction logic to SentryClient, just before envelope creation --- CHANGELOG.md | 2 +- src/Sentry/Breadcrumb.cs | 25 +++++- src/Sentry/Internal/PiiBreadcrumbSanitizer.cs | 29 ------- .../{PiiUrlSanitizer.cs => PiiExtensions.cs} | 19 ++-- .../Internal/PiiTransactionSanitizer.cs | 18 ---- src/Sentry/Request.cs | 6 ++ src/Sentry/Scope.cs | 5 -- src/Sentry/SentryClient.cs | 7 +- src/Sentry/SentryEvent.cs | 9 ++ src/Sentry/Transaction.cs | 12 +++ .../Internals/PiiBreadcrumbSanitizerTests.cs | 43 ---------- ...anitizerTests.cs => PiiExtensionsTests.cs} | 8 +- test/Sentry.Tests/Protocol/BreadcrumbTests.cs | 48 ++++++++++- .../Sentry.Tests/Protocol/SentryEventTests.cs | 86 +++++++++++++++++++ ...tionTests.cs => TransactionTracerTests.cs} | 4 +- test/Sentry.Tests/ScopeTests.cs | 47 ---------- test/Sentry.Tests/SentryClientTests.cs | 2 +- test/Sentry.Tests/TransactionTests.cs | 72 ++++++++++++++++ 18 files changed, 279 insertions(+), 163 deletions(-) delete mode 100644 src/Sentry/Internal/PiiBreadcrumbSanitizer.cs rename src/Sentry/Internal/{PiiUrlSanitizer.cs => PiiExtensions.cs} (74%) delete mode 100644 src/Sentry/Internal/PiiTransactionSanitizer.cs delete mode 100644 test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs rename test/Sentry.Tests/Internals/{PiiUrlSanitizerTests.cs => PiiExtensionsTests.cs} (90%) rename test/Sentry.Tests/Protocol/{TransactionTests.cs => TransactionTracerTests.cs} (99%) create mode 100644 test/Sentry.Tests/TransactionTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a6494b1ce..fe67ddc6f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Features - Add tag filters to SentryLoggingOptions ([#2360](https://github.com/getsentry/sentry-dotnet/pull/2360)) -- Remove authority from URLs sent to Sentry ([#2366](https://github.com/getsentry/sentry-java/pull/2366)) +- Remove authority from URLs sent to Sentry ([#2365](https://github.com/getsentry/sentry-dotnet/pull/2365)) ## 3.31.0 diff --git a/src/Sentry/Breadcrumb.cs b/src/Sentry/Breadcrumb.cs index 4e3794bc98..6640671cdb 100644 --- a/src/Sentry/Breadcrumb.cs +++ b/src/Sentry/Breadcrumb.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -9,6 +10,12 @@ namespace Sentry; [DebuggerDisplay("Message: {" + nameof(Message) + "}, Type: {" + nameof(Type) + "}")] public sealed class Breadcrumb : IJsonSerializable { + private readonly IReadOnlyDictionary? _data; + private readonly string? _message; + + private bool _sendDefaultPii = true; + internal void Redact() => _sendDefaultPii = false; + /// /// A timestamp representing when the breadcrumb occurred. /// @@ -21,7 +28,11 @@ public sealed class Breadcrumb : IJsonSerializable /// If a message is provided, it’s rendered as text and the whitespace is preserved. /// Very long text might be abbreviated in the UI. /// - public string? Message { get; } + public string? Message + { + get => _sendDefaultPii ? _message : _message.RedactUrl(); + private init => _message = value; + } /// /// The type of breadcrumb. @@ -39,7 +50,17 @@ public sealed class Breadcrumb : IJsonSerializable /// Contains a sub-object whose contents depend on the breadcrumb type. /// Additional parameters that are unsupported by the type are rendered as a key/value table. /// - public IReadOnlyDictionary? Data { get; } + public IReadOnlyDictionary? Data + { + get => _sendDefaultPii + ? _data + : _data?.ToDictionary( + x => x.Key, + x => x.Value?.RedactUrl() + )! + ; + private init => _data = value; + } /// /// Dotted strings that indicate what the crumb is or where it comes from. diff --git a/src/Sentry/Internal/PiiBreadcrumbSanitizer.cs b/src/Sentry/Internal/PiiBreadcrumbSanitizer.cs deleted file mode 100644 index f960e7c0ca..0000000000 --- a/src/Sentry/Internal/PiiBreadcrumbSanitizer.cs +++ /dev/null @@ -1,29 +0,0 @@ -namespace Sentry.Internal; - -/// -/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. -/// -internal static class PiiBreadcrumbSanitizer -{ - /// - /// Redacts PII from the breadcrumb message or data - /// - /// The breadcrumb to be sanitized - /// A new Breadcrumb with redacted copies of the message and data in the original - public static Breadcrumb Sanitize(this Breadcrumb breadcrumb) - { - var sanitizedData = breadcrumb.Data?.ToDictionary( - x => x.Key, - x => x.Value.Sanitize() ?? string.Empty - ); - - return new Breadcrumb( - timestamp : breadcrumb.Timestamp, - message : breadcrumb.Message.Sanitize(), - type : breadcrumb.Type, - data : sanitizedData, - category : breadcrumb.Category, - level : breadcrumb.Level - ); - } -} diff --git a/src/Sentry/Internal/PiiUrlSanitizer.cs b/src/Sentry/Internal/PiiExtensions.cs similarity index 74% rename from src/Sentry/Internal/PiiUrlSanitizer.cs rename to src/Sentry/Internal/PiiExtensions.cs index 4fefae5a0a..80747cc4a3 100644 --- a/src/Sentry/Internal/PiiUrlSanitizer.cs +++ b/src/Sentry/Internal/PiiExtensions.cs @@ -1,20 +1,21 @@ namespace Sentry.Internal; /// -/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. +/// Extensions to help redact data that might contain Personally Identifiable Information (PII) before sending it to +/// Sentry. /// -internal static class PiiUrlSanitizer +internal static class PiiExtensions { + internal const string RedactedText = "[Filtered]"; + /// - /// Searches for URLs in text data and redacts any PII - /// data from these, as required. + /// Searches for URLs in text data and redacts any PII data from these, as required. /// /// The data to be searched /// - /// The data if SendDefaultPii is enabled or if the data does not contain any PII. - /// A redacted copy of the data otherwise. + /// The data, if no PII data is present or a copy of the data with PII data redacted otherwise /// - public static string? Sanitize(this string? data) + public static string? RedactUrl(this string? data) { // If the data is empty then we don't need to sanitize anything if (string.IsNullOrWhiteSpace(data)) @@ -29,13 +30,13 @@ internal static class PiiUrlSanitizer var result = authRegex.Replace(data, match => { var matchedUrl = match.Groups[1].Value; - return SanitizeUrl(matchedUrl); + return RedactAuth(matchedUrl); }); return result; } - private static string SanitizeUrl(string data) + private static string RedactAuth(string data) { // ^ matches the start of the string. (?i)(https?://) gives a case-insensitive matching of the protocol. // (.*@) matches the username and password (authentication information). (.*)$ matches the rest of the URL. diff --git a/src/Sentry/Internal/PiiTransactionSanitizer.cs b/src/Sentry/Internal/PiiTransactionSanitizer.cs deleted file mode 100644 index 83aac7939b..0000000000 --- a/src/Sentry/Internal/PiiTransactionSanitizer.cs +++ /dev/null @@ -1,18 +0,0 @@ -namespace Sentry.Internal; - -/// -/// Sanitizes data that potentially contains Personally Identifiable Information (PII) before sending it to Sentry. -/// -internal static class PiiTransactionSanitizer -{ - /// - /// Redacts PII from the transaction description - /// - /// The transaction to be sanitized - /// The Transaction with redacted description - public static Transaction Sanitize(this Transaction transaction) - { - transaction.Description = transaction.Description.Sanitize(); - return transaction; - } -} diff --git a/src/Sentry/Request.cs b/src/Sentry/Request.cs index 9cd8ded3f3..9331ad5b47 100644 --- a/src/Sentry/Request.cs +++ b/src/Sentry/Request.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -176,4 +177,9 @@ public static Request FromJson(JsonElement json) Cookies = cookies }; } + + internal void Redact() + { + Url = Url.RedactUrl(); + } } diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 56f04e3f0c..bd4db02382 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -270,11 +270,6 @@ public void AddBreadcrumb(Breadcrumb breadcrumb) _breadcrumbs.TryDequeue(out _); } - if (!Options.SendDefaultPii) - { - breadcrumb = breadcrumb.Sanitize(); - } - _breadcrumbs.Enqueue(breadcrumb); if (Options.EnableScopeSync) { diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 71a949a887..25aaf45b88 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -142,7 +142,7 @@ public void CaptureTransaction(Transaction transaction) if (!_options.SendDefaultPii) { - processedTransaction = processedTransaction.Sanitize(); + processedTransaction.Redact(); } CaptureEnvelope(Envelope.FromTransaction(processedTransaction)); @@ -271,6 +271,11 @@ private SentryId DoSendEvent(SentryEvent @event, Scope? scope) return SentryId.Empty; } + if (!Options.SendDefaultPii) + { + processedEvent.Redact(); + } + return CaptureEnvelope(Envelope.FromEvent(processedEvent, _options.DiagnosticLogger, scope.Attachments, scope.SessionUpdate)) ? processedEvent.EventId : SentryId.Empty; diff --git a/src/Sentry/SentryEvent.cs b/src/Sentry/SentryEvent.cs index 423a45ce60..77e3900628 100644 --- a/src/Sentry/SentryEvent.cs +++ b/src/Sentry/SentryEvent.cs @@ -242,6 +242,15 @@ public void SetTag(string key, string value) => public void UnsetTag(string key) => (_tags ??= new Dictionary()).Remove(key); + internal void Redact() + { + _request?.Redact(); + foreach (var breadcrumb in Breadcrumbs) + { + breadcrumb.Redact(); + } + } + /// public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) { diff --git a/src/Sentry/Transaction.cs b/src/Sentry/Transaction.cs index ba755ef58a..b148f64c4b 100644 --- a/src/Sentry/Transaction.cs +++ b/src/Sentry/Transaction.cs @@ -298,6 +298,18 @@ public void SetMeasurement(string name, Measurement measurement) => SpanId, IsSampled); + /// + /// Redacts PII from the transaction + /// + internal void Redact() + { + Description = Description.RedactUrl(); + foreach (var breadcrumb in Breadcrumbs) + { + breadcrumb.Redact(); + } + } + /// public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) { diff --git a/test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs b/test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs deleted file mode 100644 index c89f631c8d..0000000000 --- a/test/Sentry.Tests/Internals/PiiBreadcrumbSanitizerTests.cs +++ /dev/null @@ -1,43 +0,0 @@ -using FluentAssertions.Execution; - -namespace Sentry.Tests.Internals; - -public class PiiBreadcrumbSanitizerTests -{ - [Fact] - public void Sanitize_Urls() - { - // Arrange - var breadcrumbData = new Dictionary - { - {"url", "https://user@sentry.io"}, - {"method", "GET"}, - {"status_code", "403"} - }; - var breadcrumb = new Breadcrumb( - timestamp : DateTimeOffset.UtcNow, - message : "message https://user@sentry.io", - type : "fake_type", - data : breadcrumbData, - category : "fake_category", - level : BreadcrumbLevel.Error - ); - - // Act - var actual = breadcrumb.Sanitize(); - - // Assert - using (new AssertionScope()) - { - actual.Should().NotBeNull(); - actual.Timestamp.Should().Be(breadcrumb.Timestamp); - actual.Message.Should().Be(PiiUrlSanitizer.Sanitize(breadcrumb.Message)); // should be sanitized - actual.Type.Should().Be(breadcrumb.Type); - actual.Data?["url"].Should().Be(PiiUrlSanitizer.Sanitize(breadcrumb.Data?["url"])); // should be sanitized - actual.Data?["method"].Should().Be(breadcrumb.Data?["method"]); - actual.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); - actual.Category.Should().Be(breadcrumb.Category); - actual.Level.Should().Be(breadcrumb.Level); - } - } -} diff --git a/test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs similarity index 90% rename from test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs rename to test/Sentry.Tests/Internals/PiiExtensionsTests.cs index 26c4ada489..4e6e865453 100644 --- a/test/Sentry.Tests/Internals/PiiUrlSanitizerTests.cs +++ b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs @@ -1,11 +1,11 @@ namespace Sentry.Tests.Internals; -public class PiiUrlSanitizerTests +public class PiiExtensionsTests { [Fact] public void Sanitize_Null() { - var actual = PiiUrlSanitizer.Sanitize(null); + var actual = PiiExtensions.RedactUrl(null); Assert.Null(actual); } @@ -16,7 +16,7 @@ public void Sanitize_Null() [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) { - var actual = PiiUrlSanitizer.Sanitize(original); + var actual = original.RedactUrl(); actual.Should().Be(original, reason); } @@ -31,7 +31,7 @@ public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] public void Sanitize_Data_IsNotNull_WithPii(string original, string expected, string reason) { - var actual = PiiUrlSanitizer.Sanitize(original); + var actual = original.RedactUrl(); actual.Should().Be(expected, reason); } diff --git a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs index d9f154b8c0..a41008f6cc 100644 --- a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs +++ b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs @@ -1,6 +1,8 @@ +using FluentAssertions.Execution; + namespace Sentry.Tests.Protocol; -public class BreadcrumbTests : ImmutableTests +public class BreadcrumbTests { private readonly IDiagnosticLogger _testOutputLogger; @@ -9,6 +11,50 @@ public BreadcrumbTests(ITestOutputHelper output) _testOutputLogger = new TestOutputDiagnosticLogger(output); } + [Fact] + public void Redact_Redacts_Urls() + { + // Arrange + var breadcrumbData = new Dictionary + { + {"url", "https://user@sentry.io"}, + {"method", "GET"}, + {"status_code", "403"} + }; + var timestamp = DateTimeOffset.UtcNow; + var message = "message https://user@sentry.io"; + var type = "fake_type"; + var data = breadcrumbData; + var category = "fake_category"; + var level = BreadcrumbLevel.Error; + + var breadcrumb = new Breadcrumb( + timestamp : timestamp, + message : message, + type : type, + data : breadcrumbData, + category : category, + level : level + ); + + // Act + breadcrumb.Redact(); + + // Assert + using (new AssertionScope()) + { + breadcrumb.Should().NotBeNull(); + breadcrumb.Timestamp.Should().Be(timestamp); + breadcrumb.Message.Should().Be("message https://[Filtered]@sentry.io"); // should be sanitized + breadcrumb.Type.Should().Be(type); + breadcrumb.Data?["url"].Should().Be("https://[Filtered]@sentry.io"); // should be sanitized + breadcrumb.Data?["method"].Should().Be(breadcrumb.Data?["method"]); + breadcrumb.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); + breadcrumb.Category.Should().Be(category); + breadcrumb.Level.Should().Be(level); + } + } + [Fact] public void SerializeObject_ParameterlessConstructor_IncludesTimestamp() { diff --git a/test/Sentry.Tests/Protocol/SentryEventTests.cs b/test/Sentry.Tests/Protocol/SentryEventTests.cs index 81032eddd7..c234ebf7de 100644 --- a/test/Sentry.Tests/Protocol/SentryEventTests.cs +++ b/test/Sentry.Tests/Protocol/SentryEventTests.cs @@ -1,3 +1,5 @@ +using FluentAssertions.Execution; + namespace Sentry.Tests.Protocol; public partial class SentryEventTests @@ -80,4 +82,88 @@ public void Modules_Getter_NotNull() var evt = new SentryEvent(); Assert.NotNull(evt.Modules); } + + [Fact] + public void Redact_Redacts_Urls() + { + // Arrange + var message = "message123 https://user@not.redacted"; + var logger = "logger123 https://user@not.redacted"; + var platform = "platform123 https://user@not.redacted"; + var serverName = "serverName123 https://user@not.redacted"; + var release = "release123 https://user@not.redacted"; + var distribution = "distribution123 https://user@not.redacted"; + var moduleValue = "module123 https://user@not.redacted"; + var transactionName = "transactionName123 https://user@sentry.io"; + var requestUrl = "https://user@sentry.io"; + var username = "bob"; + var email = "bob@127.0.0.1"; + var ipAddress = "127.0.0.1"; + var environment = "environment123 https://user@not.redacted"; + + var breadcrumbMessage = "message https://user@sentry.io"; // should be redacted + var breadcrumbDataValue = "data-value https://user@sentry.io"; // should be redacted + var tagValue = "tag_value https://user@not.redacted"; + + var timestamp = DateTimeOffset.MaxValue; + + var evt = new SentryEvent() + { + Message = message, + Logger = logger, + Platform = platform, + ServerName = serverName, + Release = release, + Distribution = distribution, + TransactionName = transactionName, + Request = new Request() + { + Method = "GET", + Url = requestUrl + }, + User = new User() + { + Username = username, + Email = email, + IpAddress = ipAddress + }, + Environment = environment, + }; + evt.Modules.Add("module", moduleValue); + evt.AddBreadcrumb(new Breadcrumb(timestamp, breadcrumbMessage)); + evt.AddBreadcrumb(new Breadcrumb( + timestamp, + "message", + "type", + new Dictionary { { "data-key", breadcrumbDataValue } }, + "category", + BreadcrumbLevel.Warning)); + evt.SetTag("tag_key", tagValue); + + // Act + evt.Redact(); + + // Assert + using (new AssertionScope()) + { + evt.Message.Message.Should().Be(message); + evt.Logger.Should().Be(logger); + evt.Platform.Should().Be(platform); + evt.ServerName.Should().Be(serverName); + evt.Release.Should().Be(release); + evt.Distribution.Should().Be(distribution); + evt.Modules["module"].Should().Be(moduleValue); + evt.TransactionName.Should().Be(transactionName); + evt.Request.Url.Should().Be($"https://{PiiExtensions.RedactedText}@sentry.io"); + evt.User.Username.Should().Be(username); + evt.User.Email.Should().Be(email); + evt.User.IpAddress.Should().Be(ipAddress); + evt.Environment.Should().Be(environment); + var breadcrumbs = evt.Breadcrumbs.ToArray(); + breadcrumbs.Length.Should().Be(2); + breadcrumbs[0].Message.Should().Be($"message https://{PiiExtensions.RedactedText}@sentry.io"); + breadcrumbs[1].Data?["data-key"].Should().Be($"data-value https://{PiiExtensions.RedactedText}@sentry.io"); + evt.Tags["tag_key"].Should().Be(tagValue); + } + } } diff --git a/test/Sentry.Tests/Protocol/TransactionTests.cs b/test/Sentry.Tests/Protocol/TransactionTracerTests.cs similarity index 99% rename from test/Sentry.Tests/Protocol/TransactionTests.cs rename to test/Sentry.Tests/Protocol/TransactionTracerTests.cs index da447c669a..4dbeea0849 100644 --- a/test/Sentry.Tests/Protocol/TransactionTests.cs +++ b/test/Sentry.Tests/Protocol/TransactionTracerTests.cs @@ -1,10 +1,10 @@ namespace Sentry.Tests.Protocol; -public class TransactionTests +public class TransactionTracerTests { private readonly IDiagnosticLogger _testOutputLogger; - public TransactionTests(ITestOutputHelper output) + public TransactionTracerTests(ITestOutputHelper output) { _testOutputLogger = new TestOutputDiagnosticLogger(output); } diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index 7aef4ad085..d7de09a391 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -462,53 +462,6 @@ public void AddBreadcrumb_ObserverExist_ObserverAddsBreadcrumbIfEnabled(bool obs // Assert observer.Received(expectedCount).AddBreadcrumb(Arg.Is(breadcrumb)); } - - // AddBreadcrumb sanitizes any URL data in the breadcrumb message - [Fact] - public void AddBreadcrumb_Sanitize_Message() - { - // Arrange - var scope = new Scope(); - var original = "Goto https://user@sentry.io"; - var expected = original.Sanitize(); - - // Act - scope.AddBreadcrumb(original); - - // Assert - scope.Breadcrumbs.First().Message.Should().Be(expected); - } - - [Fact] - public void AddBreadcrumb_Sanitize_Data() - { - // Arrange - var scope = new Scope(); - var originalUrl = "https://user@sentry.io"; - var expectedUrl = originalUrl.Sanitize(); - - var breadcrumbData = new Dictionary - { - {"url", originalUrl}, - {"method", "GET"}, - {"status_code", "403"} - }; - var breadcrumb = new Breadcrumb(message: "message", data: breadcrumbData); - - // Act - scope.AddBreadcrumb(breadcrumb); - - // Assert - var actual = scope.Breadcrumbs.First(); - using (new AssertionScope()) - { - actual.Should().NotBeNull(); - actual.Message.Should().Be(breadcrumb.Message); - actual.Data?["url"].Should().Be(expectedUrl); - actual.Data?["method"].Should().Be(breadcrumb.Data?["method"]); - actual.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); - } - } } public static class ScopeTestExtensions diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index b8b6f2161b..d157746654 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -694,7 +694,7 @@ public void CaptureTransaction_Sanitize_Description() var actual = (envelope.Items[0].Payload as JsonSerializable)?.Source as Transaction; actual?.Name.Should().Be(original.Name); actual?.Operation.Should().Be(original.Operation); - actual?.Description.Should().Be(original.Description.Sanitize()); // Should be sanitized + actual?.Description.Should().Be(original.Description.RedactUrl()); // Should be sanitized } [Fact] diff --git a/test/Sentry.Tests/TransactionTests.cs b/test/Sentry.Tests/TransactionTests.cs new file mode 100644 index 0000000000..5f7e0f37e8 --- /dev/null +++ b/test/Sentry.Tests/TransactionTests.cs @@ -0,0 +1,72 @@ +using FluentAssertions.Execution; + +namespace Sentry.Tests; + +public class TransactionTests +{ + private readonly IDiagnosticLogger _testOutputLogger; + + public TransactionTests(ITestOutputHelper output) + { + _testOutputLogger = new TestOutputDiagnosticLogger(output); + } + + [Fact] + public void Redact_Redacts_Urls() + { + // Arrange + var name = "name123 https://user@not.redacted"; + var operation = "op123 https://user@not.redacted"; + var description = "desc123 https://user@sentry.io"; // should be redacted + var platform = "platform123 https://user@not.redacted"; + var release = "release123 https://user@not.redacted"; + var distribution = "distribution123 https://user@not.redacted"; + var environment = "environment123 https://user@not.redacted"; + var breadcrumbMessage = "message https://user@sentry.io"; // should be redacted + var breadcrumbDataValue = "data-value https://user@sentry.io"; // should be redacted + var tagValue = "tag_value https://user@not.redacted"; + + var timestamp = DateTimeOffset.MaxValue; + + var transaction = new Transaction(name, operation, TransactionNameSource.Url) + { + Description = description, + Platform = platform, + Release = release, + Distribution = distribution, + // Request = Request, + // User = User, + Environment = environment + }; + + transaction.AddBreadcrumb(new Breadcrumb(timestamp, breadcrumbMessage)); + transaction.AddBreadcrumb(new Breadcrumb( + timestamp, + "message", + "type", + new Dictionary { { "data-key", breadcrumbDataValue } }, + "category", + BreadcrumbLevel.Warning)); + transaction.SetTag("tag_key", tagValue); + + // Act + transaction.Redact(); + + // Assert + using (new AssertionScope()) + { + transaction.Name.Should().Be(name); + transaction.Operation.Should().Be(operation); + transaction.Description.Should().Be($"desc123 https://{PiiExtensions.RedactedText}@sentry.io"); + transaction.Platform.Should().Be(platform); + transaction.Release.Should().Be(release); + transaction.Distribution.Should().Be(distribution); + transaction.Environment.Should().Be(environment); + var breadcrumbs = transaction.Breadcrumbs.ToArray(); + breadcrumbs.Length.Should().Be(2); + breadcrumbs[0].Message.Should().Be($"message https://{PiiExtensions.RedactedText}@sentry.io"); + breadcrumbs[1].Data?["data-key"].Should().Be($"data-value https://{PiiExtensions.RedactedText}@sentry.io"); + transaction.Tags["tag_key"].Should().Be(tagValue); + } + } +} From 1bf388d588ec557f2d37cb421a420ed4bc0cd86c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 15 May 2023 22:13:37 +1200 Subject: [PATCH 05/18] Renamed Sanitize to Redact in unit tests and comments for consistency --- src/Sentry/Internal/PiiExtensions.cs | 2 +- test/Sentry.Tests/Internals/PiiExtensionsTests.cs | 6 +++--- test/Sentry.Tests/Protocol/BreadcrumbTests.cs | 4 ++-- test/Sentry.Tests/SentryClientTests.cs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Sentry/Internal/PiiExtensions.cs b/src/Sentry/Internal/PiiExtensions.cs index 80747cc4a3..2208de5f04 100644 --- a/src/Sentry/Internal/PiiExtensions.cs +++ b/src/Sentry/Internal/PiiExtensions.cs @@ -17,7 +17,7 @@ internal static class PiiExtensions /// public static string? RedactUrl(this string? data) { - // If the data is empty then we don't need to sanitize anything + // If the data is empty then we don't need to redact anything if (string.IsNullOrWhiteSpace(data)) { return data; diff --git a/test/Sentry.Tests/Internals/PiiExtensionsTests.cs b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs index 4e6e865453..759ce0faf2 100644 --- a/test/Sentry.Tests/Internals/PiiExtensionsTests.cs +++ b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs @@ -3,7 +3,7 @@ namespace Sentry.Tests.Internals; public class PiiExtensionsTests { [Fact] - public void Sanitize_Null() + public void RedactUrl_Null() { var actual = PiiExtensions.RedactUrl(null); @@ -14,7 +14,7 @@ public void Sanitize_Null() [InlineData("I'm a harmless string.", "doesn't affect ordinary strings")] [InlineData("htps://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed https urls")] [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] - public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) + public void RedactUrl_Data_IsNotNull_WithoutPii(string original, string reason) { var actual = original.RedactUrl(); @@ -29,7 +29,7 @@ public void Sanitize_Data_IsNotNull_WithoutPii(string original, string reason) [InlineData("http://user:password@sentry.io", "http://[Filtered]:[Filtered]@sentry.io", "strips user info with user and password from http without query")] [InlineData("http://user@sentry.io", "http://[Filtered]@sentry.io", "strips user info with user only from http without query")] [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] - public void Sanitize_Data_IsNotNull_WithPii(string original, string expected, string reason) + public void RedactUrl_Data_IsNotNull_WithPii(string original, string expected, string reason) { var actual = original.RedactUrl(); diff --git a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs index a41008f6cc..f946e7eacd 100644 --- a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs +++ b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs @@ -45,9 +45,9 @@ public void Redact_Redacts_Urls() { breadcrumb.Should().NotBeNull(); breadcrumb.Timestamp.Should().Be(timestamp); - breadcrumb.Message.Should().Be("message https://[Filtered]@sentry.io"); // should be sanitized + breadcrumb.Message.Should().Be("message https://[Filtered]@sentry.io"); // should be redacted breadcrumb.Type.Should().Be(type); - breadcrumb.Data?["url"].Should().Be("https://[Filtered]@sentry.io"); // should be sanitized + breadcrumb.Data?["url"].Should().Be("https://[Filtered]@sentry.io"); // should be redacted breadcrumb.Data?["method"].Should().Be(breadcrumb.Data?["method"]); breadcrumb.Data?["status_code"].Should().Be(breadcrumb.Data?["status_code"]); breadcrumb.Category.Should().Be(category); diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index d157746654..c3465b5606 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -668,7 +668,7 @@ public void CaptureTransaction_DisposedClient_DoesNotThrow() } [Fact] - public void CaptureTransaction_Sanitize_Description() + public void CaptureTransaction_Redact_Description() { // Arrange _fixture.SentryOptions.SendDefaultPii = false; @@ -694,7 +694,7 @@ public void CaptureTransaction_Sanitize_Description() var actual = (envelope.Items[0].Payload as JsonSerializable)?.Source as Transaction; actual?.Name.Should().Be(original.Name); actual?.Operation.Should().Be(original.Operation); - actual?.Description.Should().Be(original.Description.RedactUrl()); // Should be sanitized + actual?.Description.Should().Be(original.Description.RedactUrl()); // Should be redacted } [Fact] From 3ad89e7e51205a845d81a27b369b75a3f0941d10 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 11:56:38 +1200 Subject: [PATCH 06/18] Transaction.Spans are now redacted before they are sent to Sentry - Included some comments on why we don't redact User and Request on the Transaction and SentryEvent. - Moved FluentAssertions.Execution to global usings in Directory.Build.props - Added end to end test to ensure events captured on the SentryClient get redacted, if necessary --- src/Sentry/Request.cs | 5 - src/Sentry/SentryEvent.cs | 1 - src/Sentry/Span.cs | 6 + src/Sentry/Transaction.cs | 5 + test/Directory.Build.props | 1 + .../Internals/PiiExtensionsTests.cs | 4 +- .../Internals/SentryScopeManagerTests.cs | 1 - test/Sentry.Tests/Protocol/BreadcrumbTests.cs | 2 - .../Sentry.Tests/Protocol/SentryEventTests.cs | 13 +- ...tionTracerTests.cs => TransactionTests.cs} | 111 +++++++++++++++++- test/Sentry.Tests/ScopeTests.cs | 1 - test/Sentry.Tests/SentryClientTests.cs | 23 ++++ .../SentryFailedRequestHandlerTests.cs | 2 - test/Sentry.Tests/TransactionTests.cs | 72 ------------ 14 files changed, 153 insertions(+), 94 deletions(-) rename test/Sentry.Tests/Protocol/{TransactionTracerTests.cs => TransactionTests.cs} (70%) delete mode 100644 test/Sentry.Tests/TransactionTests.cs diff --git a/src/Sentry/Request.cs b/src/Sentry/Request.cs index 9331ad5b47..205b5e23c6 100644 --- a/src/Sentry/Request.cs +++ b/src/Sentry/Request.cs @@ -177,9 +177,4 @@ public static Request FromJson(JsonElement json) Cookies = cookies }; } - - internal void Redact() - { - Url = Url.RedactUrl(); - } } diff --git a/src/Sentry/SentryEvent.cs b/src/Sentry/SentryEvent.cs index 77e3900628..31c8de2ea1 100644 --- a/src/Sentry/SentryEvent.cs +++ b/src/Sentry/SentryEvent.cs @@ -244,7 +244,6 @@ public void UnsetTag(string key) => internal void Redact() { - _request?.Redact(); foreach (var breadcrumb in Breadcrumbs) { breadcrumb.Redact(); diff --git a/src/Sentry/Span.cs b/src/Sentry/Span.cs index 3a76cbd0ad..cee6ef175c 100644 --- a/src/Sentry/Span.cs +++ b/src/Sentry/Span.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -145,4 +146,9 @@ public static Span FromJson(JsonElement json) _extra = data! }; } + + internal void Redact() + { + Description = Description.RedactUrl(); + } } diff --git a/src/Sentry/Transaction.cs b/src/Sentry/Transaction.cs index b148f64c4b..25ffb70763 100644 --- a/src/Sentry/Transaction.cs +++ b/src/Sentry/Transaction.cs @@ -308,6 +308,11 @@ internal void Redact() { breadcrumb.Redact(); } + + foreach (var span in Spans) + { + span.Redact(); + } } /// diff --git a/test/Directory.Build.props b/test/Directory.Build.props index 12a704d825..83ea9c567f 100644 --- a/test/Directory.Build.props +++ b/test/Directory.Build.props @@ -39,6 +39,7 @@ + diff --git a/test/Sentry.Tests/Internals/PiiExtensionsTests.cs b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs index 759ce0faf2..cf1a1ac414 100644 --- a/test/Sentry.Tests/Internals/PiiExtensionsTests.cs +++ b/test/Sentry.Tests/Internals/PiiExtensionsTests.cs @@ -14,7 +14,7 @@ public void RedactUrl_Null() [InlineData("I'm a harmless string.", "doesn't affect ordinary strings")] [InlineData("htps://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed https urls")] [InlineData("htp://user:password@sentry.io?q=1&s=2&token=secret#top", "doesn't affect malformed http urls")] - public void RedactUrl_Data_IsNotNull_WithoutPii(string original, string reason) + public void RedactUrl_NotNull_WithoutPii(string original, string reason) { var actual = original.RedactUrl(); @@ -29,7 +29,7 @@ public void RedactUrl_Data_IsNotNull_WithoutPii(string original, string reason) [InlineData("http://user:password@sentry.io", "http://[Filtered]:[Filtered]@sentry.io", "strips user info with user and password from http without query")] [InlineData("http://user@sentry.io", "http://[Filtered]@sentry.io", "strips user info with user only from http without query")] [InlineData("GET https://user@sentry.io for goodness", "GET https://[Filtered]@sentry.io for goodness", "strips user info from URL embedded in text")] - public void RedactUrl_Data_IsNotNull_WithPii(string original, string expected, string reason) + public void RedactUrl_NotNull_WithPii(string original, string expected, string reason) { var actual = original.RedactUrl(); diff --git a/test/Sentry.Tests/Internals/SentryScopeManagerTests.cs b/test/Sentry.Tests/Internals/SentryScopeManagerTests.cs index e25da4f30e..2c00dae4ef 100644 --- a/test/Sentry.Tests/Internals/SentryScopeManagerTests.cs +++ b/test/Sentry.Tests/Internals/SentryScopeManagerTests.cs @@ -1,4 +1,3 @@ -using FluentAssertions.Execution; using Sentry.Internal.ScopeStack; namespace Sentry.Tests.Internals; diff --git a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs index f946e7eacd..66d824222e 100644 --- a/test/Sentry.Tests/Protocol/BreadcrumbTests.cs +++ b/test/Sentry.Tests/Protocol/BreadcrumbTests.cs @@ -1,5 +1,3 @@ -using FluentAssertions.Execution; - namespace Sentry.Tests.Protocol; public class BreadcrumbTests diff --git a/test/Sentry.Tests/Protocol/SentryEventTests.cs b/test/Sentry.Tests/Protocol/SentryEventTests.cs index c234ebf7de..69ab456e66 100644 --- a/test/Sentry.Tests/Protocol/SentryEventTests.cs +++ b/test/Sentry.Tests/Protocol/SentryEventTests.cs @@ -1,5 +1,3 @@ -using FluentAssertions.Execution; - namespace Sentry.Tests.Protocol; public partial class SentryEventTests @@ -95,9 +93,9 @@ public void Redact_Redacts_Urls() var distribution = "distribution123 https://user@not.redacted"; var moduleValue = "module123 https://user@not.redacted"; var transactionName = "transactionName123 https://user@sentry.io"; - var requestUrl = "https://user@sentry.io"; - var username = "bob"; - var email = "bob@127.0.0.1"; + var requestUrl = "https://user@not.redacted"; + var username = "username"; + var email = "bob@foo.com"; var ipAddress = "127.0.0.1"; var environment = "environment123 https://user@not.redacted"; @@ -154,7 +152,10 @@ public void Redact_Redacts_Urls() evt.Distribution.Should().Be(distribution); evt.Modules["module"].Should().Be(moduleValue); evt.TransactionName.Should().Be(transactionName); - evt.Request.Url.Should().Be($"https://{PiiExtensions.RedactedText}@sentry.io"); + // We don't redact the User or the Request since, if SendDefaultPii is false, we don't add these to the + // transaction in the SDK anyway (by default they don't get sent... but the user can always override this + // behavior if they need) + evt.Request.Url.Should().Be(requestUrl); evt.User.Username.Should().Be(username); evt.User.Email.Should().Be(email); evt.User.IpAddress.Should().Be(ipAddress); diff --git a/test/Sentry.Tests/Protocol/TransactionTracerTests.cs b/test/Sentry.Tests/Protocol/TransactionTests.cs similarity index 70% rename from test/Sentry.Tests/Protocol/TransactionTracerTests.cs rename to test/Sentry.Tests/Protocol/TransactionTests.cs index 4dbeea0849..8a26bc9c8b 100644 --- a/test/Sentry.Tests/Protocol/TransactionTracerTests.cs +++ b/test/Sentry.Tests/Protocol/TransactionTests.cs @@ -1,14 +1,121 @@ namespace Sentry.Tests.Protocol; -public class TransactionTracerTests +public class TransactionTests { private readonly IDiagnosticLogger _testOutputLogger; - public TransactionTracerTests(ITestOutputHelper output) + public TransactionTests(ITestOutputHelper output) { _testOutputLogger = new TestOutputDiagnosticLogger(output); } + [Fact] + public void Redact_Redacts_Urls() + { + // Arrange + var timestamp = DateTimeOffset.MaxValue; + var name = "name123 https://user@not.redacted"; + var operation = "op123 https://user@not.redacted"; + var description = "desc123 https://user@sentry.io"; // should be redacted + var platform = "platform123 https://user@not.redacted"; + var release = "release123 https://user@not.redacted"; + var distribution = "distribution123 https://user@not.redacted"; + var environment = "environment123 https://user@not.redacted"; + var breadcrumbMessage = "message https://user@sentry.io"; // should be redacted + var breadcrumbDataValue = "data-value https://user@sentry.io"; // should be redacted + var tagValue = "tag_value https://user@not.redacted"; + var context = new TransactionContext( + SpanId.Create(), + SpanId.Create(), + SentryId.Create(), + name, + operation, + description, + SpanStatus.AlreadyExists, + null, + true, + TransactionNameSource.Component + ); + + var txTracer = new TransactionTracer(DisabledHub.Instance, context) + { + Name = name, + Operation = operation, + Description = description, + Platform = platform, + Release = release, + Distribution = distribution, + Status = SpanStatus.Aborted, + // We don't redact the User or the Request since, if SendDefaultPii is false, we don't add these to the + // transaction in the SDK anyway (by default they don't get sent... but the user can always override this + // behavior if they need) + User = new User { Id = "user-id", Username = "username", Email = "bob@foo.com", IpAddress = "127.0.0.1" }, + Request = new Request { Method = "POST", Url = "https://user@not.redacted"}, + Sdk = new SdkVersion { Name = "SDK-test", Version = "1.1.1" }, + Environment = environment, + Level = SentryLevel.Fatal, + Contexts = + { + ["context_key"] = "context_value", + [".NET Framework"] = new Dictionary + { + [".NET Framework"] = "\"v2.0.50727\", \"v3.0\", \"v3.5\"", + [".NET Framework Client"] = "\"v4.8\", \"v4.0.0.0\"", + [".NET Framework Full"] = "\"v4.8\"" + } + } + }; + + txTracer.Sdk.AddPackage(new Package("name", "version")); + txTracer.AddBreadcrumb(new Breadcrumb(timestamp, breadcrumbMessage)); + txTracer.AddBreadcrumb(new Breadcrumb( + timestamp, + "message", + "type", + new Dictionary { { "data-key", breadcrumbDataValue } }, + "category", + BreadcrumbLevel.Warning)); + txTracer.SetTag("tag_key", tagValue); + + var child1 = txTracer.StartChild("child_op123", "child_desc123 https://user@sentry.io"); + child1.Status = SpanStatus.Unimplemented; + child1.SetTag("q", "v"); + child1.SetExtra("f", "p"); + child1.Finish(SpanStatus.Unimplemented); + + var child2 = txTracer.StartChild("child_op999", "child_desc999 https://user:password@sentry.io"); + child2.Status = SpanStatus.OutOfRange; + child2.SetTag("xxx", "zzz"); + child2.SetExtra("f222", "p111"); + child2.Finish(SpanStatus.OutOfRange); + + txTracer.Finish(SpanStatus.Aborted); + + // Act + var transaction = new Transaction(txTracer); + transaction.Redact(); + + // Assert + using (new AssertionScope()) + { + transaction.Name.Should().Be(name); + transaction.Operation.Should().Be(operation); + transaction.Description.Should().Be($"desc123 https://{PiiExtensions.RedactedText}@sentry.io"); + transaction.Platform.Should().Be(platform); + transaction.Release.Should().Be(release); + transaction.Distribution.Should().Be(distribution); + transaction.Environment.Should().Be(environment); + var breadcrumbs = transaction.Breadcrumbs.ToArray(); + breadcrumbs.Length.Should().Be(2); + breadcrumbs.Should().Contain(b => b.Message == $"message https://{PiiExtensions.RedactedText}@sentry.io"); + breadcrumbs.Should().Contain(b => b.Data != null && b.Data["data-key"] == $"data-value https://{PiiExtensions.RedactedText}@sentry.io"); + var spans = transaction.Spans.ToArray(); + spans.Should().Contain(s => s.Operation == "child_op123" && s.Description == $"child_desc123 https://{PiiExtensions.RedactedText}@sentry.io"); + spans.Should().Contain(s => s.Operation == "child_op999" && s.Description == $"child_desc999 https://{PiiExtensions.RedactedText}:{PiiExtensions.RedactedText}@sentry.io"); + transaction.Tags["tag_key"].Should().Be(tagValue); + } + } + [Fact] public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject() { diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index d7de09a391..82d0947a4e 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -1,4 +1,3 @@ -using FluentAssertions.Execution; namespace Sentry.Tests; public class ScopeTests diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index c3465b5606..4d50e9b6a0 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -246,6 +246,29 @@ public void CaptureEvent_EventAndScope_CopyScopeIntoEvent() Assert.Equal(scope.Breadcrumbs, @event.Breadcrumbs); } + [Fact] + public void CaptureEvent_Redact_Breadcrumbs() + { + // Act + var scope = new Scope(_fixture.SentryOptions); + scope.AddBreadcrumb("Visited https://user@sentry.io in session"); + var @event = new SentryEvent(); + + // Act + Envelope envelope = null; + var sut = _fixture.GetSut(); + sut.Worker.EnqueueEnvelope(Arg.Do(e => envelope = e)); + _ = sut.CaptureEvent(@event, scope); + + // Assert + envelope.Should().NotBeNull(); + envelope.Items.Count.Should().Be(1); + var actual = (SentryEvent)(envelope.Items[0].Payload as JsonSerializable)?.Source; + actual.Should().NotBeNull(); + actual?.Breadcrumbs.Count.Should().Be(1); + actual?.Breadcrumbs.ToArray()[0].Message.Should().Be($"Visited https://{PiiExtensions.RedactedText}@sentry.io in session"); + } + [Fact] public void CaptureEvent_BeforeEvent_RejectEvent() { diff --git a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs index 6582f82e23..6cd67f1a69 100644 --- a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs +++ b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs @@ -1,5 +1,3 @@ -using FluentAssertions.Execution; - namespace Sentry.Tests; public class SentryFailedRequestHandlerTests diff --git a/test/Sentry.Tests/TransactionTests.cs b/test/Sentry.Tests/TransactionTests.cs deleted file mode 100644 index 5f7e0f37e8..0000000000 --- a/test/Sentry.Tests/TransactionTests.cs +++ /dev/null @@ -1,72 +0,0 @@ -using FluentAssertions.Execution; - -namespace Sentry.Tests; - -public class TransactionTests -{ - private readonly IDiagnosticLogger _testOutputLogger; - - public TransactionTests(ITestOutputHelper output) - { - _testOutputLogger = new TestOutputDiagnosticLogger(output); - } - - [Fact] - public void Redact_Redacts_Urls() - { - // Arrange - var name = "name123 https://user@not.redacted"; - var operation = "op123 https://user@not.redacted"; - var description = "desc123 https://user@sentry.io"; // should be redacted - var platform = "platform123 https://user@not.redacted"; - var release = "release123 https://user@not.redacted"; - var distribution = "distribution123 https://user@not.redacted"; - var environment = "environment123 https://user@not.redacted"; - var breadcrumbMessage = "message https://user@sentry.io"; // should be redacted - var breadcrumbDataValue = "data-value https://user@sentry.io"; // should be redacted - var tagValue = "tag_value https://user@not.redacted"; - - var timestamp = DateTimeOffset.MaxValue; - - var transaction = new Transaction(name, operation, TransactionNameSource.Url) - { - Description = description, - Platform = platform, - Release = release, - Distribution = distribution, - // Request = Request, - // User = User, - Environment = environment - }; - - transaction.AddBreadcrumb(new Breadcrumb(timestamp, breadcrumbMessage)); - transaction.AddBreadcrumb(new Breadcrumb( - timestamp, - "message", - "type", - new Dictionary { { "data-key", breadcrumbDataValue } }, - "category", - BreadcrumbLevel.Warning)); - transaction.SetTag("tag_key", tagValue); - - // Act - transaction.Redact(); - - // Assert - using (new AssertionScope()) - { - transaction.Name.Should().Be(name); - transaction.Operation.Should().Be(operation); - transaction.Description.Should().Be($"desc123 https://{PiiExtensions.RedactedText}@sentry.io"); - transaction.Platform.Should().Be(platform); - transaction.Release.Should().Be(release); - transaction.Distribution.Should().Be(distribution); - transaction.Environment.Should().Be(environment); - var breadcrumbs = transaction.Breadcrumbs.ToArray(); - breadcrumbs.Length.Should().Be(2); - breadcrumbs[0].Message.Should().Be($"message https://{PiiExtensions.RedactedText}@sentry.io"); - breadcrumbs[1].Data?["data-key"].Should().Be($"data-value https://{PiiExtensions.RedactedText}@sentry.io"); - transaction.Tags["tag_key"].Should().Be(tagValue); - } - } -} From 6fd500485bdcf9118c409e6ed6ba3e302b1d511d Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 11:59:44 +1200 Subject: [PATCH 07/18] Update sentry-cocoa --- modules/sentry-cocoa | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/sentry-cocoa b/modules/sentry-cocoa index 8da8166d3e..e6dcfba32f 160000 --- a/modules/sentry-cocoa +++ b/modules/sentry-cocoa @@ -1 +1 @@ -Subproject commit 8da8166d3ee068228849f6e4ed93a99d1c4eadc0 +Subproject commit e6dcfba32f2861438b82c7ad34e058b23c83daf6 From e3457f38c901c0d24ea58e2256664db77dd3b65e Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 22:37:08 +1200 Subject: [PATCH 08/18] Update src/Sentry/SentryClient.cs Co-authored-by: Matt Johnson-Pint --- src/Sentry/SentryClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 25aaf45b88..a8b8258be9 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -271,7 +271,7 @@ private SentryId DoSendEvent(SentryEvent @event, Scope? scope) return SentryId.Empty; } - if (!Options.SendDefaultPii) + if (!_options.SendDefaultPii) { processedEvent.Redact(); } From 7b15cafa585716b0010879e976c2ffb13713fe19 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 22:38:07 +1200 Subject: [PATCH 09/18] Update CHANGELOG.md Co-authored-by: Matt Johnson-Pint --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e41c57e44..9fa26c2713 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ ### Features -- Add tag filters to SentryLoggingOptions ([#2360](https://github.com/getsentry/sentry-dotnet/pull/2360)) - Remove authority from URLs sent to Sentry ([#2365](https://github.com/getsentry/sentry-dotnet/pull/2365)) - Allow setting the active span on the scope ([#2364](https://github.com/getsentry/sentry-dotnet/pull/2364)) - Note: Obsoletes the `Scope.GetSpan` method in favor of a `Scope.Span` property (which now has a setter as well). From caadabbe9ff74af1dfc288db5e15fa2a2af2447b Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 22:42:04 +1200 Subject: [PATCH 10/18] Update src/Sentry/Internal/PiiExtensions.cs Co-authored-by: Matt Johnson-Pint --- src/Sentry/Internal/PiiExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Internal/PiiExtensions.cs b/src/Sentry/Internal/PiiExtensions.cs index 2208de5f04..c5de5f1e8f 100644 --- a/src/Sentry/Internal/PiiExtensions.cs +++ b/src/Sentry/Internal/PiiExtensions.cs @@ -15,7 +15,7 @@ internal static class PiiExtensions /// /// The data, if no PII data is present or a copy of the data with PII data redacted otherwise /// - public static string? RedactUrl(this string? data) + public static string RedactUrl(this string data) { // If the data is empty then we don't need to redact anything if (string.IsNullOrWhiteSpace(data)) From ee9c4b2800372c91caf6f148fdf776b5625b445a Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 22:46:03 +1200 Subject: [PATCH 11/18] Update src/Sentry/Request.cs Co-authored-by: Matt Johnson-Pint --- src/Sentry/Request.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Sentry/Request.cs b/src/Sentry/Request.cs index 205b5e23c6..9cd8ded3f3 100644 --- a/src/Sentry/Request.cs +++ b/src/Sentry/Request.cs @@ -1,5 +1,4 @@ using Sentry.Extensibility; -using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; From b3a0416bf1f97a19badfc6092e1b2f46ad5ef3f3 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 23:10:28 +1200 Subject: [PATCH 12/18] Fixed nullability errors after changing signature on RedactUrl extension --- src/Sentry/Breadcrumb.cs | 2 +- src/Sentry/Sentry.csproj | 34 ++++++++++++++----- src/Sentry/Span.cs | 2 +- src/Sentry/Transaction.cs | 2 +- .../Sentry.Tests/Protocol/SentryEventTests.cs | 6 ++-- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/Sentry/Breadcrumb.cs b/src/Sentry/Breadcrumb.cs index 6640671cdb..360181ed79 100644 --- a/src/Sentry/Breadcrumb.cs +++ b/src/Sentry/Breadcrumb.cs @@ -30,7 +30,7 @@ public sealed class Breadcrumb : IJsonSerializable /// public string? Message { - get => _sendDefaultPii ? _message : _message.RedactUrl(); + get => _sendDefaultPii ? _message : _message?.RedactUrl(); private init => _message = value; } diff --git a/src/Sentry/Sentry.csproj b/src/Sentry/Sentry.csproj index cd05434cac..83a535c652 100644 --- a/src/Sentry/Sentry.csproj +++ b/src/Sentry/Sentry.csproj @@ -92,17 +92,35 @@ <_OSArchitecture>$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture) - - - - - - - + + + + + + + - + diff --git a/src/Sentry/Span.cs b/src/Sentry/Span.cs index cee6ef175c..9673bde909 100644 --- a/src/Sentry/Span.cs +++ b/src/Sentry/Span.cs @@ -149,6 +149,6 @@ public static Span FromJson(JsonElement json) internal void Redact() { - Description = Description.RedactUrl(); + Description = Description?.RedactUrl(); } } diff --git a/src/Sentry/Transaction.cs b/src/Sentry/Transaction.cs index 25ffb70763..e4e823fe1c 100644 --- a/src/Sentry/Transaction.cs +++ b/src/Sentry/Transaction.cs @@ -303,7 +303,7 @@ public void SetMeasurement(string name, Measurement measurement) => /// internal void Redact() { - Description = Description.RedactUrl(); + Description = Description?.RedactUrl(); foreach (var breadcrumb in Breadcrumbs) { breadcrumb.Redact(); diff --git a/test/Sentry.Tests/Protocol/SentryEventTests.cs b/test/Sentry.Tests/Protocol/SentryEventTests.cs index 69ab456e66..0beea4069c 100644 --- a/test/Sentry.Tests/Protocol/SentryEventTests.cs +++ b/test/Sentry.Tests/Protocol/SentryEventTests.cs @@ -114,12 +114,12 @@ public void Redact_Redacts_Urls() Release = release, Distribution = distribution, TransactionName = transactionName, - Request = new Request() + Request = new Request { - Method = "GET", + Method = "GET", Url = requestUrl }, - User = new User() + User = new User { Username = username, Email = email, From 52fe34a8d950eaccc3b3bad5e17ef3ddc70e82f9 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 16 May 2023 23:34:14 +1200 Subject: [PATCH 13/18] Made Regex instances private static readonly --- src/Sentry/Internal/PiiExtensions.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/Sentry/Internal/PiiExtensions.cs b/src/Sentry/Internal/PiiExtensions.cs index c5de5f1e8f..7fece7e4e0 100644 --- a/src/Sentry/Internal/PiiExtensions.cs +++ b/src/Sentry/Internal/PiiExtensions.cs @@ -7,6 +7,8 @@ namespace Sentry.Internal; internal static class PiiExtensions { internal const string RedactedText = "[Filtered]"; + private static readonly Regex AuthRegex = new (@"(?i)\b(https?://.*@.*)\b", RegexOptions.Compiled); + private static readonly Regex UserInfoMatcher = new (@"^(?i)(https?://)(.*@)(.*)$", RegexOptions.Compiled); /// /// Searches for URLs in text data and redacts any PII data from these, as required. @@ -26,8 +28,7 @@ public static string RedactUrl(this string data) // The pattern @"(?i)\b(https?://.*@.*)\b" uses the \b word boundary anchors to ensure that the match occurs at // a word boundary. This allows the URL to be matched even if it is part of a larger text. The (?i) flag ensures // case-insensitive matching for "https" or "http". - var authRegex = new Regex(@"(?i)\b(https?://.*@.*)\b"); - var result = authRegex.Replace(data, match => + var result = AuthRegex.Replace(data, match => { var matchedUrl = match.Groups[1].Value; return RedactAuth(matchedUrl); @@ -40,16 +41,13 @@ private static string RedactAuth(string data) { // ^ matches the start of the string. (?i)(https?://) gives a case-insensitive matching of the protocol. // (.*@) matches the username and password (authentication information). (.*)$ matches the rest of the URL. - var userInfoMatcher = new Regex(@"^(?i)(https?://)(.*@)(.*)$").Match(data); - if (userInfoMatcher.Success && userInfoMatcher.Groups.Count == 4) - { - var userInfoString = userInfoMatcher.Groups[2].Value; - var replacementString = userInfoString.Contains(":") ? "[Filtered]:[Filtered]@" : "[Filtered]@"; - return userInfoMatcher.Groups[1].Value + replacementString + userInfoMatcher.Groups[3].Value; - } - else + var match = UserInfoMatcher.Match(data); + if (match is not { Success: true, Groups.Count: 4 }) { return data; } + var userInfoString = match.Groups[2].Value; + var replacementString = userInfoString.Contains(":") ? "[Filtered]:[Filtered]@" : "[Filtered]@"; + return match.Groups[1].Value + replacementString + match.Groups[3].Value; } } From 80dc7b24035398b56c9523a3e78b18e9cbc6cab4 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 17 May 2023 08:50:25 +1200 Subject: [PATCH 14/18] Removed unecessary null forgiving operator from Breadcrumb.Data --- src/Sentry/Breadcrumb.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sentry/Breadcrumb.cs b/src/Sentry/Breadcrumb.cs index 360181ed79..4985c51160 100644 --- a/src/Sentry/Breadcrumb.cs +++ b/src/Sentry/Breadcrumb.cs @@ -56,8 +56,8 @@ public IReadOnlyDictionary? Data ? _data : _data?.ToDictionary( x => x.Key, - x => x.Value?.RedactUrl() - )! + x => x.Value.RedactUrl() + ) ; private init => _data = value; } From d05f5bd7ed76a0d037c7fd389450bacef7aa7806 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 17 May 2023 22:20:19 +1200 Subject: [PATCH 15/18] Strip UserInfo from the Request.Url before capturing Failed Requests --- src/Sentry/SentryFailedRequestHandler.cs | 16 ++++++------- .../SentryFailedRequestHandlerTests.cs | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/Sentry/SentryFailedRequestHandler.cs b/src/Sentry/SentryFailedRequestHandler.cs index 283bab5387..21d3a79b55 100644 --- a/src/Sentry/SentryFailedRequestHandler.cs +++ b/src/Sentry/SentryFailedRequestHandler.cs @@ -78,24 +78,24 @@ public void HandleResponse(HttpResponseMessage response) var sentryRequest = new Request { - Url = uri?.AbsoluteUri, QueryString = uri?.Query, Method = response.RequestMessage.Method.Method, }; - if (_options.SendDefaultPii) - { - sentryRequest.Cookies = response.RequestMessage.Headers.GetCookies(); - sentryRequest.AddHeaders(response.RequestMessage.Headers); - } - var responseContext = new Response { StatusCode = (short)response.StatusCode, BodySize = bodySize }; - if (_options.SendDefaultPii) + if (!_options.SendDefaultPii) { + sentryRequest.Url = $"{uri?.Scheme}://{uri?.Authority}{uri?.AbsolutePath}"; + } + else + { + sentryRequest.Url = uri?.AbsoluteUri; + sentryRequest.Cookies = response.RequestMessage.Headers.GetCookies(); + sentryRequest.AddHeaders(response.RequestMessage.Headers); responseContext.Cookies = response.Headers.GetCookies(); responseContext.AddHeaders(response.Headers); } diff --git a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs index 6cd67f1a69..dd9314e1c4 100644 --- a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs +++ b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs @@ -120,6 +120,29 @@ public void HandleResponse_Capture_FailedRequest() _hub.Received(1).CaptureEvent(Arg.Any(), Arg.Any()); } + [Fact] + public void HandleResponse_Capture_FailedRequest_No_Pii() + { + // Arrange + var options = new SentryOptions + { + CaptureFailedRequests = true + }; + var sut = GetSut(options); + + var response = InternalServerErrorResponse(); + var requestUri = new Uri("http://admin:1234@localhost"); + response.RequestMessage = new HttpRequestMessage(HttpMethod.Get, requestUri); + + // Act + SentryEvent @event = null; + _hub.CaptureEvent(Arg.Do(e => @event = e)); + sut.HandleResponse(response); + + // Assert + @event.Request.Url.Should().StartWith("http://localhost"); // No admin:1234 + } + [Fact] public void HandleResponse_Capture_RequestAndResponse() { From ab6d2241739ec0b8de985722ae31abcbbaab3d21 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 17 May 2023 11:30:18 -0700 Subject: [PATCH 16/18] Undo submodule update --- modules/sentry-cocoa | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/sentry-cocoa b/modules/sentry-cocoa index e6dcfba32f..8dab665edf 160000 --- a/modules/sentry-cocoa +++ b/modules/sentry-cocoa @@ -1 +1 @@ -Subproject commit e6dcfba32f2861438b82c7ad34e058b23c83daf6 +Subproject commit 8dab665edf492580977d353f8f8bf2da57c04420 From 1e2cf7e183af22ee1d19f82e3d3e44b4d45487a1 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 18 May 2023 11:34:38 +1200 Subject: [PATCH 17/18] Replaced bespoke code with Uri.GetComponents --- src/Sentry/SentryFailedRequestHandler.cs | 2 +- test/Sentry.Tests/SentryFailedRequestHandlerTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Sentry/SentryFailedRequestHandler.cs b/src/Sentry/SentryFailedRequestHandler.cs index 21d3a79b55..e06f63bfe3 100644 --- a/src/Sentry/SentryFailedRequestHandler.cs +++ b/src/Sentry/SentryFailedRequestHandler.cs @@ -89,7 +89,7 @@ public void HandleResponse(HttpResponseMessage response) if (!_options.SendDefaultPii) { - sentryRequest.Url = $"{uri?.Scheme}://{uri?.Authority}{uri?.AbsolutePath}"; + sentryRequest.Url = uri?.GetComponents(UriComponents.HttpRequestUrl, UriFormat.Unescaped); } else { diff --git a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs index dd9314e1c4..2b5c1e779a 100644 --- a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs +++ b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs @@ -131,7 +131,7 @@ public void HandleResponse_Capture_FailedRequest_No_Pii() var sut = GetSut(options); var response = InternalServerErrorResponse(); - var requestUri = new Uri("http://admin:1234@localhost"); + var requestUri = new Uri("http://admin:1234@localhost/test/path?query=string#fragment"); response.RequestMessage = new HttpRequestMessage(HttpMethod.Get, requestUri); // Act @@ -140,7 +140,7 @@ public void HandleResponse_Capture_FailedRequest_No_Pii() sut.HandleResponse(response); // Assert - @event.Request.Url.Should().StartWith("http://localhost"); // No admin:1234 + @event.Request.Url.Should().Be("http://localhost/test/path?query=string"); // No admin:1234 } [Fact] From 97f104a48d79482ecd97ca58331fdad848934a65 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 18 May 2023 11:50:14 +1200 Subject: [PATCH 18/18] Fixed missing arg in test --- test/Sentry.Tests/SentryFailedRequestHandlerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs index af6005ba6b..9fa415dcd4 100644 --- a/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs +++ b/test/Sentry.Tests/SentryFailedRequestHandlerTests.cs @@ -140,7 +140,7 @@ public void HandleResponse_Capture_FailedRequest_No_Pii() // Act SentryEvent @event = null; - _hub.CaptureEvent(Arg.Do(e => @event = e)); + _hub.CaptureEvent(Arg.Do(e => @event = e), Arg.Any()); sut.HandleResponse(response); // Assert