diff --git a/CHANGELOG.md b/CHANGELOG.md index 358bf15eb4..fc3c2ec75d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Sanitize sensitive data from URLs (span desc, span data, crumbs, client errors) ([#1327](https://github.com/getsentry/sentry-dart/pull/1327)) + ### Dependencies - Bump Cocoa SDK from v8.3.1 to v8.3.3 ([#1350](https://github.com/getsentry/sentry-dart/pull/1350), [#1355](https://github.com/getsentry/sentry-dart/pull/1355)) diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart index 005941073f..3fee10e6ea 100644 --- a/dart/lib/sentry.dart +++ b/dart/lib/sentry.dart @@ -39,3 +39,8 @@ export 'src/exception_stacktrace_extractor.dart'; // Isolates export 'src/sentry_isolate_extension.dart'; export 'src/sentry_isolate.dart'; +// URL +// ignore: invalid_export_of_internal_element +export 'src/utils/http_sanitizer.dart'; +// ignore: invalid_export_of_internal_element +export 'src/utils/url_details.dart'; diff --git a/dart/lib/sentry_io.dart b/dart/lib/sentry_io.dart index db73e953ba..378baa2e2a 100644 --- a/dart/lib/sentry_io.dart +++ b/dart/lib/sentry_io.dart @@ -1,2 +1,3 @@ +// ignore: invalid_export_of_internal_element export 'sentry.dart'; export 'src/sentry_attachment/io_sentry_attachment.dart'; diff --git a/dart/lib/src/event_processor/enricher/web_enricher_event_processor.dart b/dart/lib/src/event_processor/enricher/web_enricher_event_processor.dart index be91d1e362..7aaa29b788 100644 --- a/dart/lib/src/event_processor/enricher/web_enricher_event_processor.dart +++ b/dart/lib/src/event_processor/enricher/web_enricher_event_processor.dart @@ -49,10 +49,12 @@ class WebEnricherEventProcessor implements EnricherEventProcessor { header.putIfAbsent('User-Agent', () => _window.navigator.userAgent); - return (request ?? SentryRequest()).copyWith( - url: request?.url ?? _window.location.toString(), - headers: header, - ); + final url = request?.url ?? _window.location.toString(); + return (request ?? SentryRequest(url: url)) + .copyWith( + headers: header, + ) + .sanitized(); } SentryDevice _getDevice(SentryDevice? device) { diff --git a/dart/lib/src/http_client/breadcrumb_client.dart b/dart/lib/src/http_client/breadcrumb_client.dart index 4a6dd12920..803a3e18a0 100644 --- a/dart/lib/src/http_client/breadcrumb_client.dart +++ b/dart/lib/src/http_client/breadcrumb_client.dart @@ -2,6 +2,8 @@ import 'package:http/http.dart'; import '../protocol.dart'; import '../hub.dart'; import '../hub_adapter.dart'; +import '../utils/url_details.dart'; +import '../utils/http_sanitizer.dart'; /// A [http](https://pub.dev/packages/http)-package compatible HTTP client /// which records requests as breadcrumbs. @@ -75,15 +77,20 @@ class BreadcrumbClient extends BaseClient { } finally { stopwatch.stop(); + final urlDetails = + HttpSanitizer.sanitizeUrl(request.url.toString()) ?? UrlDetails(); + var breadcrumb = Breadcrumb.http( level: requestHadException ? SentryLevel.error : SentryLevel.info, - url: request.url, + url: Uri.parse(urlDetails.urlOrFallback), method: request.method, statusCode: statusCode, reason: reason, requestDuration: stopwatch.elapsed, requestBodySize: request.contentLength, responseBodySize: responseBodySize, + httpQuery: urlDetails.query, + httpFragment: urlDetails.fragment, ); await _hub.addBreadcrumb(breadcrumb); diff --git a/dart/lib/src/http_client/tracing_client.dart b/dart/lib/src/http_client/tracing_client.dart index cd55198a75..1b117f9eda 100644 --- a/dart/lib/src/http_client/tracing_client.dart +++ b/dart/lib/src/http_client/tracing_client.dart @@ -4,6 +4,7 @@ import '../hub_adapter.dart'; import '../protocol.dart'; import '../tracing.dart'; import '../utils/tracing_utils.dart'; +import '../utils/http_sanitizer.dart'; /// A [http](https://pub.dev/packages/http)-package compatible HTTP client /// which adds support to Sentry Performance feature. @@ -19,17 +20,28 @@ class TracingClient extends BaseClient { @override Future send(BaseRequest request) async { // see https://develop.sentry.dev/sdk/performance/#header-sentry-trace + + final urlDetails = HttpSanitizer.sanitizeUrl(request.url.toString()); + + var description = request.method; + if (urlDetails != null) { + description += ' ${urlDetails.urlOrFallback}'; + } + final currentSpan = _hub.getSpan(); var span = currentSpan?.startChild( 'http.client', - description: '${request.method} ${request.url}', + description: description, ); - // if the span is NoOp, we dont want to attach headers + // if the span is NoOp, we don't want to attach headers if (span is NoOpSentrySpan) { span = null; } + span?.setData('method', request.method); + urlDetails?.applyToSpan(span); + StreamedResponse? response; try { if (span != null) { diff --git a/dart/lib/src/protocol/breadcrumb.dart b/dart/lib/src/protocol/breadcrumb.dart index b25bc16176..f7eea55358 100644 --- a/dart/lib/src/protocol/breadcrumb.dart +++ b/dart/lib/src/protocol/breadcrumb.dart @@ -3,7 +3,7 @@ import 'package:meta/meta.dart'; import '../utils.dart'; import '../protocol.dart'; -/// Structed data to describe more information pior to the event captured. +/// Structured data to describe more information prior to the event captured. /// See `Sentry.captureEvent()`. /// /// The outgoing JSON representation is: @@ -47,6 +47,8 @@ class Breadcrumb { // Size of the response body in bytes int? responseBodySize, + String? httpQuery, + String? httpFragment, }) { return Breadcrumb( type: 'http', @@ -61,6 +63,8 @@ class Breadcrumb { if (requestDuration != null) 'duration': requestDuration.toString(), if (requestBodySize != null) 'request_body_size': requestBodySize, if (responseBodySize != null) 'response_body_size': responseBodySize, + if (httpQuery != null) 'http.query': httpQuery, + if (httpFragment != null) 'http.fragment': httpFragment, }, ); } diff --git a/dart/lib/src/protocol/sentry_request.dart b/dart/lib/src/protocol/sentry_request.dart index d4d44766fc..f6c371bfa1 100644 --- a/dart/lib/src/protocol/sentry_request.dart +++ b/dart/lib/src/protocol/sentry_request.dart @@ -1,6 +1,7 @@ import 'package:meta/meta.dart'; import '../utils/iterable_extension.dart'; +import '../utils/http_sanitizer.dart'; /// The Request interface contains information on a HTTP request related to the event. /// In client SDKs, this can be an outgoing request, or the request that rendered the current web page. @@ -92,31 +93,18 @@ class SentryRequest { @Deprecated('Will be removed in v8. Use [data] instead') Map? other, }) { - // As far as I can tell there's no way to get the uri without the query part - // so we replace it with an empty string. - final urlWithoutQuery = uri - .replace(query: '', fragment: '') - .toString() - .replaceAll('?', '') - .replaceAll('#', ''); - - // Future proof, Dio does not support it yet and even if passing in the path, - // the parsing of the uri returns empty. - final query = uri.query.isEmpty ? null : uri.query; - final fragment = uri.fragment.isEmpty ? null : uri.fragment; - return SentryRequest( - url: urlWithoutQuery, - fragment: fragment, - queryString: query, + url: uri.toString(), method: method, cookies: cookies, data: data, headers: headers, env: env, + queryString: uri.query, + fragment: uri.fragment, // ignore: deprecated_member_use_from_same_package other: other, - ); + ).sanitized(); } /// Deserializes a [SentryRequest] from JSON [Map]. @@ -162,12 +150,13 @@ class SentryRequest { Map? env, @Deprecated('Will be removed in v8. Use [data] instead') Map? other, + bool removeCookies = false, }) => SentryRequest( url: url ?? this.url, method: method ?? this.method, queryString: queryString ?? this.queryString, - cookies: cookies ?? this.cookies, + cookies: removeCookies ? null : cookies ?? this.cookies, data: data ?? _data, headers: headers ?? _headers, env: env ?? _env, diff --git a/dart/lib/src/utils/http_sanitizer.dart b/dart/lib/src/utils/http_sanitizer.dart new file mode 100644 index 0000000000..7a2d391c20 --- /dev/null +++ b/dart/lib/src/utils/http_sanitizer.dart @@ -0,0 +1,105 @@ +import 'package:meta/meta.dart'; + +import '../protocol.dart'; +import 'url_details.dart'; + +@internal +class HttpSanitizer { + static final RegExp _authRegExp = RegExp("(.+://)(.*@)(.*)"); + static final List _securityHeaders = [ + "X-FORWARDED-FOR", + "AUTHORIZATION", + "COOKIE", + "SET-COOKIE", + "X-API-KEY", + "X-REAL-IP", + "REMOTE-ADDR", + "FORWARDED", + "PROXY-AUTHORIZATION", + "X-CSRF-TOKEN", + "X-CSRFTOKEN", + "X-XSRF-TOKEN" + ]; + + /// Parse and sanitize url data for sentry.io + static UrlDetails? sanitizeUrl(String? url) { + if (url == null) { + return null; + } + + final queryIndex = url.indexOf('?'); + final fragmentIndex = url.indexOf('#'); + + if (queryIndex > -1 && fragmentIndex > -1 && fragmentIndex < queryIndex) { + // url considered malformed because of fragment position + return UrlDetails(); + } else { + try { + final uri = Uri.parse(url); + final urlWithAuthRemoved = _urlWithAuthRemoved(uri._url()); + return UrlDetails( + url: urlWithAuthRemoved.isEmpty ? null : urlWithAuthRemoved, + query: uri.query.isEmpty ? null : uri.query, + fragment: uri.fragment.isEmpty ? null : uri.fragment); + } catch (_) { + return null; + } + } + } + + static Map? sanitizedHeaders(Map? headers) { + if (headers == null) { + return null; + } + final sanitizedHeaders = {}; + headers.forEach((key, value) { + if (!_securityHeaders.contains(key.toUpperCase())) { + sanitizedHeaders[key] = value; + } + }); + return sanitizedHeaders; + } + + static String _urlWithAuthRemoved(String url) { + final userInfoMatch = _authRegExp.firstMatch(url); + if (userInfoMatch != null && userInfoMatch.groupCount == 3) { + final userInfoString = userInfoMatch.group(2) ?? ''; + final replacementString = userInfoString.contains(":") + ? "[Filtered]:[Filtered]@" + : "[Filtered]@"; + return '${userInfoMatch.group(1) ?? ''}$replacementString${userInfoMatch.group(3) ?? ''}'; + } else { + return url; + } + } +} + +extension UriPath on Uri { + String _url() { + var buffer = ''; + if (scheme.isNotEmpty) { + buffer += '$scheme://'; + } + if (userInfo.isNotEmpty) { + buffer += '$userInfo@'; + } + buffer += host; + if (path.isNotEmpty) { + buffer += path; + } + return buffer; + } +} + +extension SanitizedSentryRequest on SentryRequest { + SentryRequest sanitized() { + final urlDetails = HttpSanitizer.sanitizeUrl(url) ?? UrlDetails(); + return copyWith( + url: urlDetails.urlOrFallback, + queryString: urlDetails.query, + fragment: urlDetails.fragment, + headers: HttpSanitizer.sanitizedHeaders(headers), + removeCookies: true, + ); + } +} diff --git a/dart/lib/src/utils/url_details.dart b/dart/lib/src/utils/url_details.dart new file mode 100644 index 0000000000..89022a7892 --- /dev/null +++ b/dart/lib/src/utils/url_details.dart @@ -0,0 +1,29 @@ +import 'package:meta/meta.dart'; +import '../../sentry.dart'; + +/// Sanitized url data for sentry.io +@internal +class UrlDetails { + UrlDetails({this.url, this.query, this.fragment}); + + final String? url; + final String? query; + final String? fragment; + + late final urlOrFallback = url ?? 'unknown'; + + void applyToSpan(ISentrySpan? span) { + if (span == null) { + return; + } + if (url != null) { + span.setData('url', url); + } + if (query != null) { + span.setData("http.query", query); + } + if (fragment != null) { + span.setData("http.fragment", fragment); + } + } +} diff --git a/dart/test/event_processor/enricher/web_enricher_test.dart b/dart/test/event_processor/enricher/web_enricher_test.dart index be44614345..d4aa14b61f 100644 --- a/dart/test/event_processor/enricher/web_enricher_test.dart +++ b/dart/test/event_processor/enricher/web_enricher_test.dart @@ -57,6 +57,23 @@ void main() { expect(event.request?.url, 'foo.bar'); }); + test('does not add auth headers to request', () async { + var event = SentryEvent( + request: SentryRequest( + url: 'foo.bar', + headers: { + 'Authorization': 'foo', + 'authorization': 'bar', + }, + ), + ); + var enricher = fixture.getSut(); + event = await enricher.apply(event); + + expect(event.request?.headers['Authorization'], isNull); + expect(event.request?.headers['authorization'], isNull); + }); + test('user-agent is not overridden if already present', () async { var event = SentryEvent( request: SentryRequest( diff --git a/dart/test/http_client/breadcrumb_client_test.dart b/dart/test/http_client/breadcrumb_client_test.dart index da7e1b4b7b..1b7ae91bfa 100644 --- a/dart/test/http_client/breadcrumb_client_test.dart +++ b/dart/test/http_client/breadcrumb_client_test.dart @@ -9,7 +9,7 @@ import 'package:test/test.dart'; import '../mocks/mock_hub.dart'; -final requestUri = Uri.parse('https://example.com'); +final requestUri = Uri.parse('https://example.com/path?foo=bar#baz'); void main() { group(BreadcrumbClient, () { @@ -30,8 +30,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'GET'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['reason'], 'OK'); expect(breadcrumb.data?['duration'], isNotNull); @@ -51,8 +53,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'GET'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 404); expect(breadcrumb.data?['reason'], 'NOT FOUND'); expect(breadcrumb.data?['duration'], isNotNull); @@ -68,8 +72,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'POST'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -84,8 +90,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'PUT'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -100,8 +108,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'DELETE'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -160,8 +170,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com'); + expect(breadcrumb.data?['url'], 'https://example.com/path'); expect(breadcrumb.data?['method'], 'GET'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.level, SentryLevel.error); expect(breadcrumb.data?['duration'], isNotNull); }); diff --git a/dart/test/http_client/failed_request_client_test.dart b/dart/test/http_client/failed_request_client_test.dart index 2e28a0ff61..d7b3b4dba8 100644 --- a/dart/test/http_client/failed_request_client_test.dart +++ b/dart/test/http_client/failed_request_client_test.dart @@ -59,8 +59,8 @@ void main() { expect(request?.url, 'https://example.com'); expect(request?.queryString, 'foo=bar'); expect(request?.fragment, 'myFragment'); - expect(request?.cookies, 'foo=bar'); - expect(request?.headers, {'Cookie': 'foo=bar'}); + expect(request?.cookies, isNull); + expect(request?.headers, {}); // ignore: deprecated_member_use_from_same_package expect(request?.other.keys.contains('duration'), true); // ignore: deprecated_member_use_from_same_package @@ -134,8 +134,8 @@ void main() { expect(request?.url, 'https://example.com'); expect(request?.queryString, 'foo=bar'); expect(request?.fragment, 'myFragment'); - expect(request?.cookies, 'foo=bar'); - expect(request?.headers, {'Cookie': 'foo=bar'}); + expect(request?.cookies, isNull); + expect(request?.headers, {}); // ignore: deprecated_member_use_from_same_package expect(request?.other.keys.contains('duration'), true); // ignore: deprecated_member_use_from_same_package @@ -195,6 +195,24 @@ void main() { expect(event.contexts.response, isNull); }); + test('removes authorization headers', () async { + fixture._hub.options.captureFailedRequests = true; + final sut = fixture.getSut( + client: createThrowingClient(), + ); + + await expectLater( + () async => await sut.get(requestUri, + headers: {'authorization': 'foo', 'Authorization': 'foo'}), + throwsException, + ); + + final event = fixture.transport.events.first; + expect(fixture.transport.calls, 1); + expect(event.request, isNotNull); + expect(event.request?.headers.isEmpty, true); + }); + test('pii is not send on invalid status code', () async { final sut = fixture.getSut( client: fixture.getClient(statusCode: 404), diff --git a/dart/test/http_client/tracing_client_test.dart b/dart/test/http_client/tracing_client_test.dart index dceca47961..0b175174cf 100644 --- a/dart/test/http_client/tracing_client_test.dart +++ b/dart/test/http_client/tracing_client_test.dart @@ -8,7 +8,7 @@ import 'package:test/test.dart'; import '../mocks.dart'; import '../mocks/mock_transport.dart'; -final requestUri = Uri.parse('https://example.com?foo=bar'); +final requestUri = Uri.parse('https://example.com?foo=bar#baz'); void main() { group(TracingClient, () { @@ -37,7 +37,11 @@ void main() { expect(span.status, SpanStatus.ok()); expect(span.context.operation, 'http.client'); - expect(span.context.description, 'GET https://example.com?foo=bar'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); }); test('finish span if errored request', () async { diff --git a/dart/test/protocol/breadcrumb_test.dart b/dart/test/protocol/breadcrumb_test.dart index 35ab621de4..4062e1b895 100644 --- a/dart/test/protocol/breadcrumb_test.dart +++ b/dart/test/protocol/breadcrumb_test.dart @@ -82,16 +82,17 @@ void main() { group('ctor', () { test('Breadcrumb http', () { final breadcrumb = Breadcrumb.http( - url: Uri.parse('https://example.org'), - method: 'GET', - level: SentryLevel.fatal, - reason: 'OK', - statusCode: 200, - requestDuration: Duration.zero, - timestamp: DateTime.now(), - requestBodySize: 2, - responseBodySize: 3, - ); + url: Uri.parse('https://example.org'), + method: 'GET', + level: SentryLevel.fatal, + reason: 'OK', + statusCode: 200, + requestDuration: Duration.zero, + timestamp: DateTime.now(), + requestBodySize: 2, + responseBodySize: 3, + httpQuery: 'foo=bar', + httpFragment: 'baz'); final json = breadcrumb.toJson(); expect(json, { @@ -106,6 +107,8 @@ void main() { 'duration': '0:00:00.000000', 'request_body_size': 2, 'response_body_size': 3, + 'http.query': 'foo=bar', + 'http.fragment': 'baz' }, 'level': 'fatal', 'type': 'http', diff --git a/dart/test/utils/http_sanitizer_test.dart b/dart/test/utils/http_sanitizer_test.dart new file mode 100644 index 0000000000..176d64e51f --- /dev/null +++ b/dart/test/utils/http_sanitizer_test.dart @@ -0,0 +1,179 @@ +import 'package:sentry/src/utils/http_sanitizer.dart'; +import 'package:test/test.dart'; + +void main() { + test('returns null for null', () { + expect(HttpSanitizer.sanitizeUrl(null), isNull); + }); + + test('strips user info with user and password from http', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "http://user:password@sentry.io?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "http://[Filtered]:[Filtered]@sentry.io"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('strips user info with user and password from https', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "https://user:password@sentry.io?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "https://[Filtered]:[Filtered]@sentry.io"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits url', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("https://sentry.io?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "https://sentry.io"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits relative url', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("/users/1?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "/users/1"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits relative root url', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("/?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "/"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits url with just query and fragment', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("/?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "/"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits relative url with query only', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("/users/1?q=1&s=2&token=secret"); + expect(sanitizedUri?.url, "/users/1"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, isNull); + }); + + test('splits relative url with fragment only', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl("/users/1#top"); + expect(sanitizedUri?.url, "/users/1"); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, "top"); + }); + + test('strips user info with user and password without query', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("https://user:password@sentry.io#top"); + expect(sanitizedUri?.url, "https://[Filtered]:[Filtered]@sentry.io"); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, "top"); + }); + + test('splits without query', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl("https://sentry.io#top"); + expect(sanitizedUri?.url, "https://sentry.io"); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, "top"); + }); + + test('strips user info with user and password without fragment', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "https://user:password@sentry.io?q=1&s=2&token=secret"); + expect(sanitizedUri?.url, "https://[Filtered]:[Filtered]@sentry.io"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, isNull); + }); + + test('strips user info with user and password without query or fragment', () { + final sanitizedUri = + HttpSanitizer.sanitizeUrl("https://user:password@sentry.io"); + expect(sanitizedUri?.url, "https://[Filtered]:[Filtered]@sentry.io"); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, isNull); + }); + + test('splits url without query or fragment and no authority', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl("https://sentry.io"); + expect(sanitizedUri?.url, "https://sentry.io"); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, isNull); + }); + + test('strips user info with user only', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "https://user@sentry.io?q=1&s=2&token=secret#top"); + expect(sanitizedUri?.url, "https://[Filtered]@sentry.io"); + expect(sanitizedUri?.query, "q=1&s=2&token=secret"); + expect(sanitizedUri?.fragment, "top"); + }); + + test('no details extracted with query after fragment', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "https://user:password@sentry.io#fragment?q=1&s=2&token=secret"); + expect(sanitizedUri?.url, isNull); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, isNull); + }); + + test('no details extracted with query after fragment without authority', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "https://sentry.io#fragment?q=1&s=2&token=secret"); + expect(sanitizedUri?.url, isNull); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, isNull); + }); + + test('no details extracted from malformed url', () { + final sanitizedUri = HttpSanitizer.sanitizeUrl( + "htps://user@sentry.io#fragment?q=1&s=2&token=secret"); + expect(sanitizedUri?.url, isNull); + expect(sanitizedUri?.query, isNull); + expect(sanitizedUri?.fragment, isNull); + }); + + test('removes security headers', () { + final securityHeaders = [ + "X-FORWARDED-FOR", + "AUTHORIZATION", + "COOKIE", + "SET-COOKIE", + "X-API-KEY", + "X-REAL-IP", + "REMOTE-ADDR", + "FORWARDED", + "PROXY-AUTHORIZATION", + "X-CSRF-TOKEN", + "X-CSRFTOKEN", + "X-XSRF-TOKEN" + ]; + + final headers = {}; + for (final securityHeader in securityHeaders) { + headers[securityHeader] = 'foo'; + headers[securityHeader.toLowerCase()] = 'bar'; + headers[securityHeader._capitalize()] = 'baz'; + } + final sanitizedHeaders = HttpSanitizer.sanitizedHeaders(headers); + expect(sanitizedHeaders, isNotNull); + expect(sanitizedHeaders?.isEmpty, true); + }); + + test('handle throwing uri', () { + final details = HttpSanitizer.sanitizeUrl('::Not valid URI::'); + expect(details, isNull); + }); +} + +extension StringExtension on String { + String _capitalize() { + return "${this[0].toUpperCase()}${substring(1).toLowerCase()}"; + } +} diff --git a/dart/test/utils/url_details_test.dart b/dart/test/utils/url_details_test.dart new file mode 100644 index 0000000000..b667bde972 --- /dev/null +++ b/dart/test/utils/url_details_test.dart @@ -0,0 +1,80 @@ +import 'package:mockito/mockito.dart'; +import 'package:sentry/sentry.dart'; +import 'package:test/test.dart'; + +void main() { + test('does not crash on null span', () { + final urlDetails = + UrlDetails(url: "https://sentry.io/api", query: "q=1", fragment: "top"); + urlDetails.applyToSpan(null); + }); + + test('applies all to span', () { + final urlDetails = + UrlDetails(url: "https://sentry.io/api", query: "q=1", fragment: "top"); + final span = MockSpan(); + urlDetails.applyToSpan(span); + + verify(span.setData("url", "https://sentry.io/api")); + verify(span.setData("http.query", "q=1")); + verify(span.setData("http.fragment", "top")); + }); + + test('applies only url to span', () { + final urlDetails = UrlDetails(url: "https://sentry.io/api"); + final span = MockSpan(); + urlDetails.applyToSpan(span); + + verify(span.setData("url", "https://sentry.io/api")); + verifyNoMoreInteractions(span); + }); + + test('applies only query to span', () { + final urlDetails = UrlDetails(query: "q=1"); + final span = MockSpan(); + urlDetails.applyToSpan(span); + + verify(span.setData("http.query", "q=1")); + verifyNoMoreInteractions(span); + }); + + test('applies only fragment to span', () { + final urlDetails = UrlDetails(fragment: "top"); + final span = MockSpan(); + urlDetails.applyToSpan(span); + + verify(span.setData("http.fragment", "top")); + verifyNoMoreInteractions(span); + }); + + test('applies details to request', () { + final url = "https://sentry.io/api?q=1#top"; + final request = SentryRequest(url: url).sanitized(); + + expect(request.url, "https://sentry.io/api"); + expect(request.queryString, "q=1"); + expect(request.fragment, "top"); + }); + + test('applies details without fragment and url to request', () { + final request = SentryRequest(url: 'https://sentry.io/api').sanitized(); + + expect(request.url, "https://sentry.io/api"); + expect(request.queryString, isNull); + expect(request.fragment, isNull); + }); + + test('removes cookies from request', () { + final request = + SentryRequest(url: 'https://sentry.io/api', cookies: 'foo=bar') + .sanitized(); + expect(request.cookies, isNull); + }); + + test('returns fallback for null URL', () { + final urlDetails = UrlDetails(url: null); + expect(urlDetails.urlOrFallback, "unknown"); + }); +} + +class MockSpan extends Mock implements SentrySpan {} diff --git a/dio/lib/src/breadcrumb_client_adapter.dart b/dio/lib/src/breadcrumb_client_adapter.dart index 17233d984e..a2686031d4 100644 --- a/dio/lib/src/breadcrumb_client_adapter.dart +++ b/dio/lib/src/breadcrumb_client_adapter.dart @@ -55,14 +55,20 @@ class BreadcrumbClientAdapter implements HttpClientAdapter { } finally { stopwatch.stop(); + final urlDetails = + // ignore: invalid_use_of_internal_member + HttpSanitizer.sanitizeUrl(options.uri.toString()) ?? UrlDetails(); + final breadcrumb = Breadcrumb.http( level: requestHadException ? SentryLevel.error : SentryLevel.info, - url: options.uri, + url: Uri.parse(urlDetails.urlOrFallback), method: options.method, statusCode: statusCode, reason: reason, requestDuration: stopwatch.elapsed, responseBodySize: responseBodySize, + httpQuery: urlDetails.query, + httpFragment: urlDetails.fragment, ); await _hub.addBreadcrumb(breadcrumb); diff --git a/dio/lib/src/sentry_transformer.dart b/dio/lib/src/sentry_transformer.dart index ae77084405..eaf6c49d00 100644 --- a/dio/lib/src/sentry_transformer.dart +++ b/dio/lib/src/sentry_transformer.dart @@ -15,10 +15,21 @@ class SentryTransformer implements Transformer { @override Future transformRequest(RequestOptions options) async { + // ignore: invalid_use_of_internal_member + final urlDetails = HttpSanitizer.sanitizeUrl(options.uri.toString()); + var description = options.method; + if (urlDetails != null) { + description += ' ${urlDetails.urlOrFallback}'; + } + final span = _hub.getSpan()?.startChild( _serializeOp, - description: '${options.method} ${options.uri}', + description: description, ); + + span?.setData('method', options.method); + urlDetails?.applyToSpan(span); + String? request; try { request = await _transformer.transformRequest(options); @@ -39,10 +50,21 @@ class SentryTransformer implements Transformer { RequestOptions options, ResponseBody response, ) async { + // ignore: invalid_use_of_internal_member + final urlDetails = HttpSanitizer.sanitizeUrl(options.uri.toString()); + var description = options.method; + if (urlDetails != null) { + description += ' ${urlDetails.urlOrFallback}'; + } + final span = _hub.getSpan()?.startChild( _serializeOp, - description: '${options.method} ${options.uri}', + description: description, ); + + span?.setData('method', options.method); + urlDetails?.applyToSpan(span); + dynamic transformedResponse; try { transformedResponse = diff --git a/dio/lib/src/tracing_client_adapter.dart b/dio/lib/src/tracing_client_adapter.dart index 8a5bf3966f..38f743f0f7 100644 --- a/dio/lib/src/tracing_client_adapter.dart +++ b/dio/lib/src/tracing_client_adapter.dart @@ -23,18 +23,29 @@ class TracingClientAdapter implements HttpClientAdapter { Stream? requestStream, Future? cancelFuture, ) async { + // ignore: invalid_use_of_internal_member + final urlDetails = HttpSanitizer.sanitizeUrl(options.uri.toString()); + + var description = options.method; + if (urlDetails != null) { + description += ' ${urlDetails.urlOrFallback}'; + } + // see https://develop.sentry.dev/sdk/performance/#header-sentry-trace final currentSpan = _hub.getSpan(); var span = currentSpan?.startChild( 'http.client', - description: '${options.method} ${options.uri}', + description: description, ); - // if the span is NoOp, we dont want to attach headers + // if the span is NoOp, we don't want to attach headers if (span is NoOpSentrySpan) { span = null; } + span?.setData('method', options.method); + urlDetails?.applyToSpan(span); + ResponseBody? response; try { if (span != null) { diff --git a/dio/test/breadcrumb_client_adapter_test.dart b/dio/test/breadcrumb_client_adapter_test.dart index 28fa4769df..24250f4824 100644 --- a/dio/test/breadcrumb_client_adapter_test.dart +++ b/dio/test/breadcrumb_client_adapter_test.dart @@ -19,15 +19,17 @@ void main() { final sut = fixture.getSut(fixture.getClient(statusCode: 200, reason: 'OK')); - final response = await sut.get('/'); + final response = await sut.get(''); expect(response.statusCode, 200); expect(fixture.hub.addBreadcrumbCalls.length, 1); final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com/'); + expect(breadcrumb.data?['url'], 'https://example.com'); expect(breadcrumb.data?['method'], 'GET'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['reason'], null); expect(breadcrumb.data?['duration'], isNotNull); @@ -38,15 +40,17 @@ void main() { test('POST: happy path', () async { final sut = fixture.getSut(fixture.getClient(statusCode: 200)); - final response = await sut.post('/'); + final response = await sut.post(''); expect(response.statusCode, 200); expect(fixture.hub.addBreadcrumbCalls.length, 1); final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com/'); + expect(breadcrumb.data?['url'], 'https://example.com'); expect(breadcrumb.data?['method'], 'POST'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -54,15 +58,17 @@ void main() { test('PUT: happy path', () async { final sut = fixture.getSut(fixture.getClient(statusCode: 200)); - final response = await sut.put('/'); + final response = await sut.put(''); expect(response.statusCode, 200); expect(fixture.hub.addBreadcrumbCalls.length, 1); final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com/'); + expect(breadcrumb.data?['url'], 'https://example.com'); expect(breadcrumb.data?['method'], 'PUT'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -70,15 +76,17 @@ void main() { test('DELETE: happy path', () async { final sut = fixture.getSut(fixture.getClient(statusCode: 200)); - final response = await sut.delete('/'); + final response = await sut.delete(''); expect(response.statusCode, 200); expect(fixture.hub.addBreadcrumbCalls.length, 1); final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com/'); + expect(breadcrumb.data?['url'], 'https://example.com'); expect(breadcrumb.data?['method'], 'DELETE'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.data?['status_code'], 200); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -89,17 +97,20 @@ void main() { test('no captureException for Exception', () async { final sut = fixture.getSut( MockHttpClientAdapter((options, requestStream, cancelFuture) async { - expect(options.uri, Uri.parse('https://example.com/')); + expect(options.uri, Uri.parse('https://example.com?foo=bar#baz')); throw Exception('test'); }), ); try { - await sut.get('/'); + await sut.get(''); fail('Method did not throw'); } on DioError catch (e) { expect(e.error.toString(), 'Exception: test'); - expect(e.requestOptions.uri, Uri.parse('https://example.com/')); + expect( + e.requestOptions.uri, + Uri.parse('https://example.com?foo=bar#baz'), + ); } expect(fixture.hub.captureExceptionCalls.length, 0); @@ -108,13 +119,13 @@ void main() { test('breadcrumb gets added when an exception gets thrown', () async { final sut = fixture.getSut( MockHttpClientAdapter((options, requestStream, cancelFuture) async { - expect(options.uri, Uri.parse('https://example.com/')); + expect(options.uri, Uri.parse('https://example.com')); throw Exception('foo bar'); }), ); try { - await sut.get('/'); + await sut.get(''); fail('Method did not throw'); } on DioError catch (_) {} @@ -123,8 +134,10 @@ void main() { final breadcrumb = fixture.hub.addBreadcrumbCalls.first.crumb; expect(breadcrumb.type, 'http'); - expect(breadcrumb.data?['url'], 'https://example.com/'); + expect(breadcrumb.data?['url'], 'https://example.com'); expect(breadcrumb.data?['method'], 'GET'); + expect(breadcrumb.data?['http.query'], 'foo=bar'); + expect(breadcrumb.data?['http.fragment'], 'baz'); expect(breadcrumb.level, SentryLevel.error); expect(breadcrumb.data?['duration'], isNotNull); }); @@ -145,13 +158,13 @@ void main() { test('Breadcrumb has correct duration', () async { final sut = fixture.getSut( MockHttpClientAdapter((options, _, __) async { - expect(options.uri, Uri.parse('https://example.com/')); + expect(options.uri, Uri.parse('https://example.com?foo=bar#baz')); await Future.delayed(Duration(seconds: 1)); return ResponseBody.fromString('', 200); }), ); - final response = await sut.get('/'); + final response = await sut.get(''); expect(response.statusCode, 200); expect(fixture.hub.addBreadcrumbCalls.length, 1); @@ -170,7 +183,7 @@ class Fixture { Dio getSut([MockHttpClientAdapter? client]) { final mc = client ?? getClient(); final dio = Dio( - BaseOptions(baseUrl: 'https://example.com/'), + BaseOptions(baseUrl: 'https://example.com?foo=bar#baz'), ); dio.httpClientAdapter = BreadcrumbClientAdapter(client: mc, hub: hub); return dio; @@ -180,7 +193,7 @@ class Fixture { MockHttpClientAdapter getClient({int statusCode = 200, String? reason}) { return MockHttpClientAdapter((request, requestStream, cancelFuture) async { - expect(request.uri, Uri.parse('https://example.com/')); + expect(request.uri, Uri.parse('https://example.com?foo=bar#baz')); return ResponseBody.fromString( '', diff --git a/dio/test/dio_event_processor_test.dart b/dio/test/dio_event_processor_test.dart index 904d13b3cf..4c8c2c1945 100644 --- a/dio/test/dio_event_processor_test.dart +++ b/dio/test/dio_event_processor_test.dart @@ -105,6 +105,31 @@ void main() { expect(processedEvent.request?.data, null); expect(processedEvent.request?.headers, {}); }); + + test('$DioEventProcessor removes auth headers', () { + final sut = fixture.getSut(sendDefaultPii: false); + + final requestOptionsWithAuthHeaders = requestOptions.copyWith( + headers: {'authorization': 'foo', 'Authorization': 'bar'}, + ); + final throwable = Exception(); + final dioError = DioError( + requestOptions: requestOptionsWithAuthHeaders, + response: Response( + requestOptions: requestOptionsWithAuthHeaders, + ), + ); + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + + expect(processedEvent.request?.headers, {}); + }); }); group('response', () { diff --git a/dio/test/sentry_transformer_test.dart b/dio/test/sentry_transformer_test.dart index 3e6a8c36e4..821b9a6beb 100644 --- a/dio/test/sentry_transformer_test.dart +++ b/dio/test/sentry_transformer_test.dart @@ -11,6 +11,8 @@ import 'mocks.dart'; import 'mocks/mock_transport.dart'; import 'package:sentry/src/sentry_tracer.dart'; +final requestUri = Uri.parse('https://example.com?foo=bar#baz'); + void main() { group(SentryTransformer, () { late Fixture fixture; @@ -18,6 +20,7 @@ void main() { setUp(() { fixture = Fixture(); }); + test('transformRequest creates span', () async { final sut = fixture.getSut(); final tr = fixture._hub.startTransaction( @@ -26,7 +29,7 @@ void main() { bindToScope: true, ); - await sut.transformRequest(RequestOptions(path: 'foo')); + await sut.transformRequest(RequestOptions(path: requestUri.toString())); await tr.finish(); @@ -35,7 +38,11 @@ void main() { expect(span.status, SpanStatus.ok()); expect(span.context.operation, 'serialize.http.client'); - expect(span.context.description, 'GET foo'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); }); test('transformRequest finish span if errored request', () async { @@ -47,7 +54,7 @@ void main() { ); try { - await sut.transformRequest(RequestOptions(path: 'foo')); + await sut.transformRequest(RequestOptions(path: requestUri.toString())); } catch (_) {} await tr.finish(); @@ -57,7 +64,11 @@ void main() { expect(span.status, SpanStatus.internalError()); expect(span.context.operation, 'serialize.http.client'); - expect(span.context.description, 'GET foo'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); expect(span.finished, true); }); @@ -70,7 +81,7 @@ void main() { ); await sut.transformResponse( - RequestOptions(path: 'foo'), + RequestOptions(path: requestUri.toString()), ResponseBody.fromString('', 200), ); @@ -81,8 +92,13 @@ void main() { expect(span.status, SpanStatus.ok()); expect(span.context.operation, 'serialize.http.client'); - expect(span.context.description, 'GET foo'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); }); + test('transformResponse finish span if errored request', () async { final sut = fixture.getSut(throwException: true); final tr = fixture._hub.startTransaction( @@ -93,7 +109,7 @@ void main() { try { await sut.transformResponse( - RequestOptions(path: 'foo'), + RequestOptions(path: requestUri.toString()), ResponseBody.fromString('', 200), ); } catch (_) {} @@ -105,7 +121,11 @@ void main() { expect(span.status, SpanStatus.internalError()); expect(span.context.operation, 'serialize.http.client'); - expect(span.context.description, 'GET foo'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); expect(span.finished, true); }); }); @@ -121,7 +141,7 @@ class Fixture { _hub = Hub(_options); } - Transformer getSut({bool throwException = false}) { + SentryTransformer getSut({bool throwException = false}) { return SentryTransformer( transformer: MockTransformer(throwException), hub: _hub, diff --git a/dio/test/tracing_client_adapter_test.dart b/dio/test/tracing_client_adapter_test.dart index 8db5efb4cd..0e28e6c892 100644 --- a/dio/test/tracing_client_adapter_test.dart +++ b/dio/test/tracing_client_adapter_test.dart @@ -11,8 +11,8 @@ import 'mocks.dart'; import 'mocks/mock_http_client_adapter.dart'; import 'mocks/mock_transport.dart'; -final requestUri = Uri.parse('https://example.com?foo=bar'); -final requestOptions = '?foo=bar'; +final requestUri = Uri.parse('https://example.com?foo=bar#baz'); +final requestOptions = '?foo=bar#baz'; void main() { group(TracingClientAdapter, () { @@ -41,7 +41,11 @@ void main() { expect(span.status, SpanStatus.ok()); expect(span.context.operation, 'http.client'); - expect(span.context.description, 'GET https://example.com?foo=bar'); + expect(span.context.description, 'GET https://example.com'); + expect(span.data['method'], 'GET'); + expect(span.data['url'], 'https://example.com'); + expect(span.data['http.query'], 'foo=bar'); + expect(span.data['http.fragment'], 'baz'); }); test('finish span if errored request', () async { diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index 508091c9ea..c30ca1f5ad 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -1,3 +1,4 @@ +// ignore: invalid_export_of_internal_element /// A Flutter client for Sentry.io crash reporting. export 'package:sentry/sentry.dart';