-
-
Notifications
You must be signed in to change notification settings - Fork 226
fix: Redact Authorization header before sending events to Sentry #4164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
24037a1
5825a24
bab48d3
a47c989
fe4d9cb
3d62f93
5aef44f
f42a3a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| using Sentry.Internal.Extensions; | ||
|
|
||
| namespace Sentry.Internal; | ||
|
|
||
| internal class RedactedHeaders : IDictionary<string, string> | ||
| { | ||
| private static readonly string[] SensitiveKeys = ["Authorization", "Proxy-Authorization"]; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could be a bit more aggressive there. The Python SDK redacts a bunch of headers. See Potential suspects. |
||
|
|
||
| private readonly Dictionary<string, string> _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<string> Keys => _inner.Keys; | ||
| public ICollection<string> Values => _inner.Values; | ||
| public int Count => _inner.Count; | ||
| public bool IsReadOnly => false; | ||
|
|
||
| public void Add(KeyValuePair<string, string> item) => Add(item.Key, item.Value); | ||
| public void Clear() => _inner.Clear(); | ||
| public bool Contains(KeyValuePair<string, string> item) => _inner.Contains(item); | ||
| public void CopyTo(KeyValuePair<string, string>[] array, int arrayIndex) => | ||
| ((IDictionary<string, string>)_inner).CopyTo(array, arrayIndex); | ||
| public bool Remove(KeyValuePair<string, string> item) => _inner.Remove(item.Key); | ||
| public IEnumerator<KeyValuePair<string, string>> GetEnumerator() => _inner.GetEnumerator(); | ||
| IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); | ||
|
|
||
| public static implicit operator RedactedHeaders(Dictionary<string, string> source) | ||
| { | ||
| var result = new RedactedHeaders(); | ||
| foreach (var kvp in source) | ||
| { | ||
| result[kvp.Key] = kvp.Value; // This will sanitize | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ public sealed class Response : ISentryJsonSerializable, ICloneable<Response>, IU | |
| /// </summary> | ||
| public const string Type = "response"; | ||
|
|
||
| internal Dictionary<string, string>? InternalHeaders { get; private set; } | ||
| internal RedactedHeaders? InternalHeaders { get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the HTTP response body size. | ||
|
|
@@ -57,7 +57,7 @@ public sealed class Response : ISentryJsonSerializable, ICloneable<Response>, IU | |
| /// <remarks> | ||
| /// If a header appears multiple times it needs to be merged according to the HTTP standard for header merging. | ||
| /// </remarks> | ||
| public IDictionary<string, string> Headers => InternalHeaders ??= new Dictionary<string, string>(); | ||
| public IDictionary<string, string> Headers => InternalHeaders ??= new(); | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the HTTP Status response code | ||
|
|
@@ -69,10 +69,13 @@ internal void AddHeaders(IEnumerable<KeyValuePair<string, IEnumerable<string>>> | |
| { | ||
| 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; | ||
jamescrosswell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| 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.Redact(), | ||
|
||
| StatusCode = statusCode | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<KeyNotFoundException>(); | ||
| } | ||
|
|
||
| [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<string, string> | ||
| { | ||
| { "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]"); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.