Log Warning when secret is detected in DSN (#1749)

getsentry · Jun 28, 2022 · c5d84bf · c5d84bf
1 parent 1e7a298
commit c5d84bf
Show file tree

Hide file tree

Showing 31 changed files with 213 additions and 210 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - Android Scope Sync ([#1737](https://github.com/getsentry/sentry-dotnet/pull/1737))
 - Enable logging in MAUI ([#1738](https://github.com/getsentry/sentry-dotnet/pull/1738))
 - Support IntPtr and UIntPtr serialization ([#1746](https://github.com/getsentry/sentry-dotnet/pull/1746))
+- Log Warning when secret is detected in DSN ([#1749](https://github.com/getsentry/sentry-dotnet/pull/1749))
 - Catch permission exceptions on Android ([#1750](https://github.com/getsentry/sentry-dotnet/pull/1750))
 - Enable offline caching in MAUI ([#1753](https://github.com/getsentry/sentry-dotnet/pull/1753))
 

diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs
@@ -35,18 +35,22 @@ internal static IHub InitHub(SentryOptions options)
 
             // If DSN is null (i.e. not explicitly disabled, just unset), then
             // try to resolve the value from environment.
-            var dsn = options.Dsn ??= DsnLocator.FindDsnStringOrDisable();
+            var dsnString = options.Dsn ??= DsnLocator.FindDsnStringOrDisable();
 
             // If it's either explicitly disabled or we couldn't resolve the DSN
             // from anywhere else, return a disabled hub.
-            if (Dsn.IsDisabled(dsn))
+            if (Dsn.IsDisabled(dsnString))
             {
                 options.LogWarning("Init was called but no DSN was provided nor located. Sentry SDK will be disabled.");
                 return DisabledHub.Instance;
             }
 
             // Validate DSN for an early exception in case it's malformed
-            _ = Dsn.Parse(dsn);
+            var dsn = Dsn.Parse(dsnString);
+            if (dsn.SecretKey != null)
+            {
+                options.LogWarning("The provided DSN that contains a secret key. This is not required and will be ignored.");
+            }
 
             return new Hub(options);
         }

diff --git a/test/Directory.Build.props b/test/Directory.Build.props
@@ -35,6 +35,7 @@
     <Using Include="NSubstitute"/>
     <Using Include="NSubstitute.Core"/>
     <Using Include="NSubstitute.ReturnsExtensions"/>
+    <Using Include="Sentry.DsnSamples" Static="True" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.2.0" />
     <PackageReference Include="NSubstitute" Version="4.3.0" />
     <PackageReference Include="FluentAssertions" Version="6.7.0" />

diff --git a/test/Sentry.AspNetCore.Grpc.Tests/SentryGrpcSdkTestFixture.cs b/test/Sentry.AspNetCore.Grpc.Tests/SentryGrpcSdkTestFixture.cs
@@ -35,7 +35,7 @@ protected override void ConfigureBuilder(WebHostBuilder builder)
             sentryBuilder.AddGrpc();
             sentryBuilder.AddSentryOptions(options =>
             {
-                options.Dsn = DsnSamples.ValidDsnWithSecret;
+                options.Dsn = ValidDsn;
                 options.SentryHttpClientFactory = new DelegateHttpClientFactory(_ => sentryHttpClient);
 
                 Configure?.Invoke(options);

diff --git a/test/Sentry.AspNetCore.Tests/AspNetCoreSentryWebHostBuilder.IntegrationTests.cs b/test/Sentry.AspNetCore.Tests/AspNetCoreSentryWebHostBuilder.IntegrationTests.cs
@@ -12,7 +12,7 @@ public class SentryWebHostBuilderExtensionsIntegrationTests : AspNetSentrySdkTes
     [Fact]
     public void UseSentry_ValidDsnString_EnablesSdk()
     {
-        _ = _webHostBuilder.UseSentry(DsnSamples.ValidDsnWithoutSecret)
+        _ = _webHostBuilder.UseSentry(ValidDsn)
             .Build();
 
         try

diff --git a/test/Sentry.AspNetCore.Tests/AspNetSentrySdkTestFixture.cs b/test/Sentry.AspNetCore.Tests/AspNetSentrySdkTestFixture.cs
@@ -17,7 +17,7 @@ protected override void ConfigureBuilder(WebHostBuilder builder)
         var sentryHttpClient = sentry.CreateClient();
         _ = builder.UseSentry(options =>
         {
-            options.Dsn = DsnSamples.ValidDsnWithSecret;
+            options.Dsn = ValidDsn;
             options.SentryHttpClientFactory = new DelegateHttpClientFactory(_ => sentryHttpClient);
 
             Configure?.Invoke(options);

diff --git a/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs b/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs
@@ -26,7 +26,7 @@ public IntegrationMockedBackgroundWorker()
         {
             _ = builder.UseSentry(options =>
             {
-                options.Dsn = DsnSamples.ValidDsnWithSecret;
+                options.Dsn = ValidDsn;
                 options.BackgroundWorker = Worker;
 
                 Configure?.Invoke(options);

diff --git a/test/Sentry.AspNetCore.Tests/MiddlewareLoggerIntegration.cs b/test/Sentry.AspNetCore.Tests/MiddlewareLoggerIntegration.cs
@@ -41,7 +41,7 @@ public Fixture()
             Client.When(client => client.CaptureEvent(Arg.Any<SentryEvent>(), Arg.Any<Scope>()))
                 .Do(callback => callback.Arg<Scope>().Evaluate());
 
-            var hub = new Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithSecret });
+            var hub = new Hub(new SentryOptions { Dsn = ValidDsn });
             hub.BindClient(Client);
             Hub = hub;
             var provider = new SentryLoggerProvider(hub, Clock, loggingOptions);

diff --git a/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs b/test/Sentry.AspNetCore.Tests/SentryHttpMessageHandlerBuilderFilterTests.cs
@@ -38,7 +38,7 @@ public async Task Generated_client_sends_Sentry_trace_header_automatically()
 
             var hub = new Internal.Hub(new SentryOptions
             {
-                Dsn = DsnSamples.ValidDsnWithoutSecret
+                Dsn = ValidDsn
             });
 
             var server = new TestServer(new WebHostBuilder()

diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs
@@ -19,7 +19,7 @@ public async Task Transactions_are_grouped_by_route()
         // Arrange
         var sentryClient = Substitute.For<ISentryClient>();
 
-        var hub = new Internal.Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret, TracesSampleRate = 1 }, sentryClient);
+        var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient);
 
         var server = new TestServer(new WebHostBuilder()
             .UseDefaultServiceProvider(di => di.EnableValidation())
@@ -65,7 +65,7 @@ public async Task Transaction_is_bound_on_the_scope_automatically()
 
         var sentryClient = Substitute.For<ISentryClient>();
 
-        var hub = new Internal.Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret }, sentryClient);
+        var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn }, sentryClient);
 
         var server = new TestServer(new WebHostBuilder()
             .UseDefaultServiceProvider(di => di.EnableValidation())
@@ -108,7 +108,7 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade
         // Arrange
         var sentryClient = Substitute.For<ISentryClient>();
 
-        var hub = new Internal.Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret, TracesSampleRate = 1 }, sentryClient);
+        var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient);
 
         var server = new TestServer(new WebHostBuilder()
             .UseDefaultServiceProvider(di => di.EnableValidation())
@@ -152,7 +152,7 @@ public async Task Transaction_is_automatically_populated_with_request_data()
 
         var sentryClient = Substitute.For<ISentryClient>();
 
-        var hub = new Internal.Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret, TracesSampleRate = 1 }, sentryClient);
+        var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient);
 
         var server = new TestServer(new WebHostBuilder()
             .UseDefaultServiceProvider(di => di.EnableValidation())
@@ -204,7 +204,7 @@ public async Task Transaction_sampling_context_contains_HTTP_context_data()
 
         var hub = new Internal.Hub(new SentryOptions
         {
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             TracesSampler = ctx =>
             {
                 samplingContext = ctx;
@@ -257,7 +257,7 @@ public async Task Transaction_binds_exception_thrown()
 
         var hub = new Internal.Hub(new SentryOptions
         {
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             TracesSampler = ctx =>
             {
                 samplingContext = ctx;
@@ -318,7 +318,7 @@ public async Task Transaction_TransactionNameProviderSetSet_TransactionNameSet()
             .Do(callback => transaction = callback.Arg<Transaction>());
         var options = new SentryAspNetCoreOptions
         {
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             TracesSampleRate = 1
         };
 
@@ -359,7 +359,7 @@ public async Task Transaction_TransactionNameProviderSetUnset_UnknownTransaction
             .Do(callback => transaction = callback.Arg<Transaction>());
         var options = new SentryAspNetCoreOptions
         {
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             TracesSampleRate = 1
         };
 

diff --git a/test/Sentry.AspNetCore.Tests/SentryWebHostBuilderExtensionsTests.cs b/test/Sentry.AspNetCore.Tests/SentryWebHostBuilderExtensionsTests.cs
@@ -36,7 +36,7 @@ public SentryWebHostBuilderExtensionsTests()
     [Theory, MemberData(nameof(ExpectedServices))]
     public void UseSentry_ValidDsnString_ServicesRegistered(Action<IServiceCollection> assert)
     {
-        _ = WebHostBuilder.UseSentry(DsnSamples.ValidDsnWithoutSecret);
+        _ = WebHostBuilder.UseSentry(ValidDsn);
         assert(Services);
     }
 

diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.cs b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.cs
@@ -21,7 +21,7 @@ public async Task RecordsSql()
         {
             TracesSampleRate = 1,
             Transport = transport,
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             DiagnosticLevel = SentryLevel.Debug
         };
 
@@ -62,7 +62,7 @@ public async Task RecordsEf()
         {
             TracesSampleRate = 1,
             Transport = transport,
-            Dsn = DsnSamples.ValidDsnWithoutSecret,
+            Dsn = ValidDsn,
             DiagnosticLevel = SentryLevel.Debug
         };
 

diff --git a/test/Sentry.EntityFramework.Tests/ErrorProcessorTests.cs b/test/Sentry.EntityFramework.Tests/ErrorProcessorTests.cs
@@ -31,7 +31,7 @@ public Fixture()
             SentryClient = new SentryClient(new SentryOptions
             {
                 BeforeSend = _beforeSend,
-                Dsn = DsnSamples.ValidDsnWithoutSecret,
+                Dsn = ValidDsn,
             }.AddEntityFramework());
         }
     }

diff --git a/test/Sentry.Maui.Tests/SentryMauiAppBuilderExtensionsTests.cs b/test/Sentry.Maui.Tests/SentryMauiAppBuilderExtensionsTests.cs
@@ -26,7 +26,7 @@ public void CanUseSentry_WithConfigurationOnly()
         var builder = _fixture.Builder;
         builder.Services.Configure<SentryMauiOptions>(options =>
         {
-            options.Dsn = DsnSamples.ValidDsnWithoutSecret;
+            options.Dsn = ValidDsn;
         });
 
         // Act
@@ -38,7 +38,7 @@ public void CanUseSentry_WithConfigurationOnly()
         // Assert
         Assert.Same(builder, chainedBuilder);
         Assert.True(SentrySdk.IsEnabled);
-        Assert.Equal(DsnSamples.ValidDsnWithoutSecret, options.Dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
         Assert.False(options.Debug);
     }
 
@@ -49,15 +49,15 @@ public void CanUseSentry_WithDsnStringOnly()
         var builder = _fixture.Builder;
 
         // Act
-        var chainedBuilder = builder.UseSentry(DsnSamples.ValidDsnWithoutSecret);
+        var chainedBuilder = builder.UseSentry(ValidDsn);
 
         using var app = builder.Build();
         var options = app.Services.GetRequiredService<IOptions<SentryMauiOptions>>().Value;
 
         // Assert
         Assert.Same(builder, chainedBuilder);
         Assert.True(SentrySdk.IsEnabled);
-        Assert.Equal(DsnSamples.ValidDsnWithoutSecret, options.Dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
         Assert.False(options.Debug);
     }
 
@@ -70,7 +70,7 @@ public void CanUseSentry_WithOptionsOnly()
         // Act
         var chainedBuilder = builder.UseSentry(options =>
         {
-            options.Dsn = DsnSamples.ValidDsnWithoutSecret;
+            options.Dsn = ValidDsn;
         });
 
         using var app = builder.Build();
@@ -79,7 +79,7 @@ public void CanUseSentry_WithOptionsOnly()
         // Assert
         Assert.Same(builder, chainedBuilder);
         Assert.True(SentrySdk.IsEnabled);
-        Assert.Equal(DsnSamples.ValidDsnWithoutSecret, options.Dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
         Assert.False(options.Debug);
     }
 
@@ -90,7 +90,7 @@ public void CanUseSentry_WithConfigurationAndOptions()
         var builder = _fixture.Builder;
         builder.Services.Configure<SentryMauiOptions>(options =>
         {
-            options.Dsn = DsnSamples.ValidDsnWithoutSecret;
+            options.Dsn = ValidDsn;
         });
 
         // Act
@@ -105,7 +105,7 @@ public void CanUseSentry_WithConfigurationAndOptions()
         // Assert
         Assert.Same(builder, chainedBuilder);
         Assert.True(SentrySdk.IsEnabled);
-        Assert.Equal(DsnSamples.ValidDsnWithoutSecret, options.Dsn);
+        Assert.Equal(ValidDsn, options.Dsn);
         Assert.True(options.Debug);
     }
 
@@ -116,7 +116,7 @@ public void UseSentry_EnablesGlobalMode()
         var builder = _fixture.Builder;
 
         // Act
-        _ = builder.UseSentry(DsnSamples.ValidDsnWithoutSecret);
+        _ = builder.UseSentry(ValidDsn);
 
         using var app = builder.Build();
         var options = app.Services.GetRequiredService<IOptions<SentryMauiOptions>>().Value;
@@ -133,7 +133,7 @@ public void UseSentry_SetsMauiSdkNameAndVersion()
         var builder = _fixture.Builder
             .UseSentry(options =>
             {
-                options.Dsn = DsnSamples.ValidDsnWithoutSecret;
+                options.Dsn = ValidDsn;
                 options.BeforeSend = e =>
                 {
                     // capture the event
@@ -160,7 +160,7 @@ public void UseSentry_EnablesHub()
     {
         // Arrange
         var builder = _fixture.Builder
-            .UseSentry(DsnSamples.ValidDsnWithoutSecret);
+            .UseSentry(ValidDsn);
 
         // Act
         using var app = builder.Build();
@@ -179,7 +179,7 @@ public void UseSentry_AppDispose_DisposesHub()
 
         // Arrange
         var builder = _fixture.Builder
-            .UseSentry(DsnSamples.ValidDsnWithoutSecret);
+            .UseSentry(ValidDsn);
 
         // Act
         IHub hub;

diff --git a/test/Sentry.NLog.Tests/SentryTargetTests.cs b/test/Sentry.NLog.Tests/SentryTargetTests.cs
@@ -8,15 +8,13 @@
 
 namespace Sentry.NLog.Tests;
 
-using static DsnSamples;
-
 public class SentryTargetTests
 {
     private const string DefaultMessage = "This is a logged message";
 
     private class Fixture
     {
-        public SentryNLogOptions Options { get; set; } = new() { Dsn = ValidDsnWithSecret };
+        public SentryNLogOptions Options { get; set; } = new() { Dsn = ValidDsn };
 
         public IHub Hub { get; set; } = Substitute.For<IHub>();
 
@@ -86,7 +84,7 @@ public void Can_configure_from_xml_file()
                         <add type='{typeof(SentryTarget).AssemblyQualifiedName}' />
                     </extensions>
                     <targets>
-                        <target type='Sentry' name='sentry' dsn='{ValidDsnWithoutSecret}' release='1.2.3' environment='test'>
+                        <target type='Sentry' name='sentry' dsn='{ValidDsn}' release='1.2.3' environment='test'>
                             <options>
                                 <attachStacktrace>True</attachStacktrace>
                             </options>
@@ -103,7 +101,7 @@ public void Can_configure_from_xml_file()
         Assert.NotNull(t);
         if (t.Options.Dsn != null)
         {
-            Assert.Equal(ValidDsnWithoutSecret, t.Options.Dsn);
+            Assert.Equal(ValidDsn, t.Options.Dsn);
         }
 
         Assert.Equal("test", t.Options.Environment);
@@ -120,7 +118,7 @@ public void Can_configure_user_from_xml_file()
                         <add type='{typeof(SentryTarget).AssemblyQualifiedName}' />
                     </extensions>
                     <targets>
-                        <target type='Sentry' name='sentry' dsn='{ValidDsnWithoutSecret}'>
+                        <target type='Sentry' name='sentry' dsn='{ValidDsn}'>
                             <user username=""myUser"">
                                 <other name='mood' layout='joyous'/>
                             </user>
@@ -135,7 +133,7 @@ public void Can_configure_user_from_xml_file()
 
         var t = logFactory.Configuration.FindTargetByName("sentry") as SentryTarget;
         Assert.NotNull(t);
-        Assert.Equal(ValidDsnWithoutSecret, t.Options.Dsn);
+        Assert.Equal(ValidDsn, t.Options.Dsn);
         Assert.Equal("'myUser'", t.User.Username.ToString());
         Assert.NotEmpty(t.User.Other);
         Assert.Equal("mood", t.User.Other[0].Name);

diff --git a/test/Sentry.Serilog.Tests/AspNetSentrySdkTestFixture.cs b/test/Sentry.Serilog.Tests/AspNetSentrySdkTestFixture.cs
@@ -47,7 +47,7 @@ protected override void ConfigureBuilder(WebHostBuilder builder)
         var sentryHttpClient = sentry.CreateClient();
         _ = builder.UseSentry(options =>
         {
-            options.Dsn = DsnSamples.ValidDsnWithSecret;
+            options.Dsn = ValidDsn;
             options.SentryHttpClientFactory = new DelegateHttpClientFactory(_ => sentryHttpClient);
 
             Configure?.Invoke(options);

diff --git a/test/Sentry.Serilog.Tests/SentrySerilogSinkExtensionsTests.cs b/test/Sentry.Serilog.Tests/SentrySerilogSinkExtensionsTests.cs
@@ -21,7 +21,7 @@ private class Fixture
         public float SampleRate { get; } = 0.4f;
         public string Release { get; } = nameof(ConfigureSentrySerilogOptions_WithAllParameters_MakesAppropriateChangesToObject);
         public string Environment { get; } = nameof(ConfigureSentrySerilogOptions_WithAllParameters_MakesAppropriateChangesToObject);
-        public string Dsn { get; } = DsnSamples.ValidDsnWithSecret;
+        public string Dsn { get; } = ValidDsn;
         public int MaxQueueItems { get; } = 17;
         public TimeSpan ShutdownTimeout { get; } = TimeSpan.FromDays(1.3);
         public DecompressionMethods DecompressionMethods { get; } = DecompressionMethods.Deflate & DecompressionMethods.GZip;

diff --git a/test/Sentry.Testing/DsnSamples.cs b/test/Sentry.Testing/DsnSamples.cs
@@ -6,11 +6,7 @@ public static class DsnSamples
     /// <summary>
     /// Sentry has dropped the use of secrets
     /// </summary>
-    public const string ValidDsnWithoutSecret = "https://d4d82fc1c2c4032a83f3a29aa3a3aff@fake-sentry.io:65535/2147483647";
-    /// <summary>
-    /// Legacy includes secret
-    /// </summary>
-    public const string ValidDsnWithSecret = "https://d4d82fc1c2c4032a83f3a29aa3a3aff:ed0a8589a0bb4d4793ac4c70375f3d65@fake-sentry.io:65535/2147483647";
+    public const string ValidDsn = "https://d4d82fc1c2c4032a83f3a29aa3a3aff@fake-sentry.io:65535/2147483647";
     /// <summary>
     /// Missing ProjectId
     /// </summary>