diff --git a/CHANGELOG.md b/CHANGELOG.md index 21213821a3..ee7ab5d37f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,14 @@ ## Unreleased +### Fixes + +- Redact Authorization headers before sending events to Sentry ([#4164](https://github.com/getsentry/sentry-dotnet/pull/4164)) + ### Features - New source generator allows Sentry to see true build variables like PublishAot and PublishTrimmed to properly adapt checks in the Sentry SDK ([#4101](https://github.com/getsentry/sentry-dotnet/pull/4101)) + ### Dependencies - Bump CLI from v2.43.1 to v2.44.0 ([#4169](https://github.com/getsentry/sentry-dotnet/pull/4169)) diff --git a/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs b/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs index 349832a72e..d6e652b5c7 100644 --- a/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs +++ b/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Reflection; namespace Sentry.AspNet.Internal; @@ -54,12 +55,12 @@ public SystemWebRequestEventProcessor(IRequestPayloadExtractor payloadExtractor, foreach (var key in context.Request.Headers.AllKeys) { - if (!_options.SendDefaultPii - // Don't add headers which might contain PII - && key is "Cookie" or "Authorization") + // Don't add cookies that might contain PII + if (!_options.SendDefaultPii && key is "Cookie") { continue; } + @event.Request.Headers[key] = context.Request.Headers[key]; } diff --git a/src/Sentry.AspNetCore/ScopeExtensions.cs b/src/Sentry.AspNetCore/ScopeExtensions.cs index dcddf56ded..10ed791335 100644 --- a/src/Sentry.AspNetCore/ScopeExtensions.cs +++ b/src/Sentry.AspNetCore/ScopeExtensions.cs @@ -4,6 +4,7 @@ using Microsoft.Net.Http.Headers; using Sentry.AspNetCore.Extensions; using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry.AspNetCore; @@ -135,10 +136,8 @@ private static void SetEnv(Scope scope, HttpContext context, SentryAspNetCoreOpt scope.Request.QueryString = context.Request.QueryString.ToString(); foreach (var requestHeader in context.Request.Headers) { - if (!options.SendDefaultPii - // Don't add headers which might contain PII - && (requestHeader.Key == HeaderNames.Cookie - || requestHeader.Key == HeaderNames.Authorization)) + // Don't add cookies that might contain PII + if (!options.SendDefaultPii && requestHeader.Key == HeaderNames.Cookie) { continue; } diff --git a/src/Sentry/HttpHeadersExtensions.cs b/src/Sentry/HttpHeadersExtensions.cs index cb55f4e7ac..0731c66cfb 100644 --- a/src/Sentry/HttpHeadersExtensions.cs +++ b/src/Sentry/HttpHeadersExtensions.cs @@ -1,3 +1,6 @@ +using Sentry.Internal; +using Sentry.Internal.Extensions; + namespace Sentry; internal static class HttpHeadersExtensions @@ -6,4 +9,14 @@ internal static string GetCookies(this HttpHeaders headers) => headers.TryGetValues("Cookie", out var values) ? string.Join("; ", values) : string.Empty; + + internal static RedactedHeaders? ToRedactedHeaders(this Dictionary? headers) + { + var items = headers?.WhereNotNullValue(); + if (items is null || !items.Any()) + { + return null; + } + return headers!; + } } diff --git a/src/Sentry/Internal/RedactedHeaders.cs b/src/Sentry/Internal/RedactedHeaders.cs new file mode 100644 index 0000000000..531dc3a6ff --- /dev/null +++ b/src/Sentry/Internal/RedactedHeaders.cs @@ -0,0 +1,53 @@ +using Sentry.Internal.Extensions; + +namespace Sentry.Internal; + +internal class RedactedHeaders : IDictionary +{ + private static readonly string[] SensitiveKeys = ["Authorization", "Proxy-Authorization"]; + + private readonly Dictionary _inner = new(StringComparer.OrdinalIgnoreCase); + + private static string Redact(string key, string value) => + SensitiveKeys.Contains(key, StringComparer.OrdinalIgnoreCase) ? "[Filtered]" : value; + + public string this[string key] + { + get => _inner[key]; + set => _inner[key] = Redact(key, value); + } + + public void Add(string key, string value) => _inner.Add(key, Redact(key, value)); + + // Delegate rest to _inner + public bool ContainsKey(string key) => _inner.ContainsKey(key); + public bool Remove(string key) => _inner.Remove(key); +#if NET8_0_OR_GREATER + public bool TryGetValue(string key, [MaybeNullWhen(false)] out string value) => _inner.TryGetValue(key, out value); +#else + public bool TryGetValue(string key, out string value) => _inner.TryGetValue(key, out value); +#endif + public ICollection Keys => _inner.Keys; + public ICollection Values => _inner.Values; + public int Count => _inner.Count; + public bool IsReadOnly => false; + + public void Add(KeyValuePair item) => Add(item.Key, item.Value); + public void Clear() => _inner.Clear(); + public bool Contains(KeyValuePair item) => _inner.Contains(item); + public void CopyTo(KeyValuePair[] array, int arrayIndex) => + ((IDictionary)_inner).CopyTo(array, arrayIndex); + public bool Remove(KeyValuePair item) => _inner.Remove(item.Key); + public IEnumerator> GetEnumerator() => _inner.GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); + + public static implicit operator RedactedHeaders(Dictionary source) + { + var result = new RedactedHeaders(); + foreach (var kvp in source) + { + result[kvp.Key] = kvp.Value; // This will sanitize + } + return result; + } +} diff --git a/src/Sentry/Protocol/Response.cs b/src/Sentry/Protocol/Response.cs index 2ed1032285..7b32b7ff0a 100644 --- a/src/Sentry/Protocol/Response.cs +++ b/src/Sentry/Protocol/Response.cs @@ -30,7 +30,7 @@ public sealed class Response : ISentryJsonSerializable, ICloneable, IU /// public const string Type = "response"; - internal Dictionary? InternalHeaders { get; private set; } + internal RedactedHeaders? InternalHeaders { get; private set; } /// /// Gets or sets the HTTP response body size. @@ -57,7 +57,7 @@ public sealed class Response : ISentryJsonSerializable, ICloneable, IU /// /// If a header appears multiple times it needs to be merged according to the HTTP standard for header merging. /// - public IDictionary Headers => InternalHeaders ??= new Dictionary(); + public IDictionary Headers => InternalHeaders ??= new(); /// /// Gets or sets the HTTP Status response code @@ -69,10 +69,13 @@ internal void AddHeaders(IEnumerable>> { foreach (var header in headers) { - Headers.Add( - header.Key, - string.Join("; ", header.Value) - ); + // Always redact the Authorization header + if (header.Key.Equals("Authorization", StringComparison.OrdinalIgnoreCase)) + { + Headers.Add(header.Key, PiiExtensions.RedactedText); + continue; + } + Headers.Add(header.Key, string.Join("; ", header.Value)); } } @@ -144,7 +147,7 @@ public static Response FromJson(JsonElement json) BodySize = bodySize, Cookies = cookies, Data = data, - InternalHeaders = headers?.WhereNotNullValue().ToDict(), + InternalHeaders = headers.ToRedactedHeaders(), StatusCode = statusCode }; } diff --git a/src/Sentry/SentryRequest.cs b/src/Sentry/SentryRequest.cs index e2a3ca432e..9f00a6b0f1 100644 --- a/src/Sentry/SentryRequest.cs +++ b/src/Sentry/SentryRequest.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry; @@ -31,7 +32,7 @@ public sealed class SentryRequest : ISentryJsonSerializable internal Dictionary? InternalOther { get; private set; } - internal Dictionary? InternalHeaders { get; private set; } + internal RedactedHeaders? InternalHeaders { get; private set; } /// /// Gets or sets the full request URL, if available. @@ -81,7 +82,7 @@ public sealed class SentryRequest : ISentryJsonSerializable /// If a header appears multiple times it needs to be merged according to the HTTP standard for header merging. /// /// The headers. - public IDictionary Headers => InternalHeaders ??= new Dictionary(); + public IDictionary Headers => InternalHeaders ??= new(); /// /// Gets or sets the optional environment data. @@ -102,6 +103,12 @@ internal void AddHeaders(IEnumerable>> { foreach (var header in headers) { + // Always redact the Authorization header + if (header.Key.Equals("Authorization", StringComparison.OrdinalIgnoreCase)) + { + Headers.Add(header.Key, PiiExtensions.RedactedText); + continue; + } Headers.Add(header.Key, string.Join("; ", header.Value)); } } @@ -176,7 +183,7 @@ public static SentryRequest FromJson(JsonElement json) { InternalEnv = env?.WhereNotNullValue().ToDict(), InternalOther = other?.WhereNotNullValue().ToDict(), - InternalHeaders = headers?.WhereNotNullValue().ToDict(), + InternalHeaders = headers.ToRedactedHeaders(), Url = url, Method = method, Data = data, diff --git a/test/Sentry.Tests/Internals/RedactedHeadersTests.cs b/test/Sentry.Tests/Internals/RedactedHeadersTests.cs new file mode 100644 index 0000000000..1185e9c529 --- /dev/null +++ b/test/Sentry.Tests/Internals/RedactedHeadersTests.cs @@ -0,0 +1,118 @@ +namespace Sentry.Tests.Internals; + +public class RedactedHeadersTests +{ + [Fact] + public void Add_WithAuthorizationKey_ShouldStoreFilteredValue() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + headers.Add("Authorization", "Bearer 123"); + + // Assert + headers["Authorization"].Should().Be("[Filtered]"); + } + + [Fact] + public void IndexerSet_WithAuthorizationKey_ShouldStoreFilteredValue() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + headers["Authorization"] = "Bearer 456"; + + // Assert + headers["Authorization"].Should().Be("[Filtered]"); + } + + [Fact] + public void Add_WithOtherKey_ShouldStoreOriginalValue() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + headers.Add("User-Agent", "TestAgent"); + + // Assert + headers["User-Agent"].Should().Be("TestAgent"); + } + + [Fact] + public void IndexerGet_WithMissingKey_ShouldThrowKeyNotFoundException() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + var act = () => _ = headers["Missing"]; + + // Assert + act.Should().Throw(); + } + + [Fact] + public void TryGetValue_WithExistingKey_ShouldReturnTrueAndValue() + { + // Arrange + var headers = new RedactedHeaders + { + ["Authorization"] = "secret" + }; + + // Act + var success = headers.TryGetValue("Authorization", out var value); + + // Assert + success.Should().BeTrue(); + value.Should().Be("[Filtered]"); + } + + [Fact] + public void TryGetValue_WithMissingKey_ShouldReturnFalse() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + var success = headers.TryGetValue("Nonexistent", out var value); + + // Assert + success.Should().BeFalse(); + value.Should().BeNull(); // nullable context may allow this + } + + [Fact] + public void ImplicitConversion_FromDictionary_ShouldRedactAuthorization() + { + // Arrange + var dict = new Dictionary + { + { "Authorization", "Token xyz" }, + { "Custom", "Value" } + }; + + // Act + RedactedHeaders headers = dict; + + // Assert + headers["Authorization"].Should().Be("[Filtered]"); + headers["Custom"].Should().Be("Value"); + } + + [Fact] + public void CaseInsensitiveKeyMatching_ShouldRedactAuthorization() + { + // Arrange + var headers = new RedactedHeaders(); + + // Act + headers.Add("authorization", "should be filtered"); + + // Assert + headers["AUTHORIZATION"].Should().Be("[Filtered]"); + } +}