From ac68cb6dc1aa6808479f66760bce4be77c611dfa Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 7 Nov 2023 11:12:02 +0100 Subject: [PATCH 1/3] feat(sdk-crashes): Filenames for system frames Keep file names for system library frames as they don't any sensitive data. The event stripper replaces the filename for sdk frames as it could contain sensitive data. --- src/sentry/utils/sdk_crashes/event_stripper.py | 4 ++-- src/sentry/utils/sdk_crashes/sdk_crash_detector.py | 2 +- .../sentry/utils/sdk_crashes/test_event_stripper.py | 3 +++ .../utils/sdk_crashes/test_sdk_crash_detection.py | 12 ++++++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 376319f201db74..0fb514b90897fc 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -38,8 +38,8 @@ def with_explanation(self, explanation: str) -> "Allow": "values": { "stacktrace": { "frames": { - "filename": Allow.NEVER.with_explanation( - "The filename path could contain the app name." + "filename": Allow.SIMPLE_TYPE.with_explanation( + "We overwrite the filename for SDK frames and it's acceptable to keep it for system library frames." ), "function": Allow.SIMPLE_TYPE, "raw_function": Allow.SIMPLE_TYPE, diff --git a/src/sentry/utils/sdk_crashes/sdk_crash_detector.py b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py index 1788c30fad38cc..f2f216dc387a92 100644 --- a/src/sentry/utils/sdk_crashes/sdk_crash_detector.py +++ b/src/sentry/utils/sdk_crashes/sdk_crash_detector.py @@ -59,7 +59,7 @@ def __init__( @property def fields_containing_paths(self) -> Set[str]: - return {"package", "module", "abs_path"} + return {"package", "module", "abs_path", "filename"} def replace_sdk_frame_path(self, field: str) -> str: for matcher, replacement_name in self.config.sdk_frame_path_replacement_names.items(): diff --git a/tests/sentry/utils/sdk_crashes/test_event_stripper.py b/tests/sentry/utils/sdk_crashes/test_event_stripper.py index fdd02916839731..11290a8be4edf2 100644 --- a/tests/sentry/utils/sdk_crashes/test_event_stripper.py +++ b/tests/sentry/utils/sdk_crashes/test_event_stripper.py @@ -212,6 +212,7 @@ def test_strip_event_data_keeps_exception_stacktrace(store_and_strip_event): "raw_function": "raw_function", "module": "module", "abs_path": "abs_path", + "filename": "EventStripperTestFrame.swift", "in_app": False, "instruction_addr": "0x1a4e8f000", "addr_mode": "0x1a4e8f000", @@ -297,6 +298,7 @@ def test_strip_frames_sdk_frames(store_and_strip_event): "package": "Sentry.framework", "abs_path": "Sentry.framework", "module": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, "image_addr": "0x100304000", } @@ -331,6 +333,7 @@ def test_strip_frames_sdk_frames_multiple_replacement_names(store_and_strip_even "package": "SentryPackage", "abs_path": "Sentry.framework", "module": "SentryModule", + "filename": "Sentry.framework", "in_app": True, "image_addr": "0x100304000", } diff --git a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py index f8b2414bb02906..a8a9aa4b2468c0 100644 --- a/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py +++ b/tests/sentry/utils/sdk_crashes/test_sdk_crash_detection.py @@ -311,6 +311,7 @@ def test_metric_kit_crash_is_detected(self, mock_sdk_crash_reporter): "raw_function": "closure #1 (MXDiagnosticPayload) in SentryMXManager.didReceive([MXDiagnosticPayload])", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { @@ -318,42 +319,49 @@ def test_metric_kit_crash_is_detected(self, mock_sdk_crash_reporter): "raw_function": "closure #1 (SentryMXCallStackTree) in closure #3 (MXCPUExceptionDiagnostic) in closure #1 (MXDiagnosticPayload) in SentryMXManager.didReceive([MXDiagnosticPayload])", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "-[SentryMetricKitIntegration captureEventNotPerThread:params:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "+[SentrySDK captureEvent:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "-[SentryFileManager readAppStateFrom:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "+[SentrySerialization appStateWithData:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "-[SentryAppState initWithJSONObject:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "+[NSDate(SentryExtras) sentry_fromIso8601String:]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { @@ -448,24 +456,28 @@ def test_thread_inspector_crash_is_detected(self, mock_sdk_crash_reporter): "function": "-[SentryANRTracker detectANRs]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "-[SentryANRTracker ANRDetected]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "-[SentryANRTrackingIntegration anrDetected]", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, { "function": "getStackEntriesFromThread", "package": "Sentry.framework", "abs_path": "Sentry.framework", + "filename": "Sentry.framework", "in_app": True, }, ] From 1f80f86d627f6f9d08d86892d53f406dcfe4554e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 7 Nov 2023 11:15:37 +0100 Subject: [PATCH 2/3] pass the content of the field --- src/sentry/utils/sdk_crashes/event_stripper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index 0fb514b90897fc..a9035f7aeb2804 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -179,7 +179,7 @@ def strip_frame(frame: MutableMapping[str, Any]) -> MutableMapping[str, Any]: # The path field usually contains the name of the application, which we can't keep. for field in sdk_crash_detector.fields_containing_paths: if frame.get(field): - frame[field] = sdk_crash_detector.replace_sdk_frame_path(field) + frame[field] = sdk_crash_detector.replace_sdk_frame_path(frame.get(field)) else: frame["in_app"] = False From 4485eede72e9053bd6083f05300d05d12a9f2501 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 7 Nov 2023 13:20:59 +0100 Subject: [PATCH 3/3] Code review --- src/sentry/utils/sdk_crashes/event_stripper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/utils/sdk_crashes/event_stripper.py b/src/sentry/utils/sdk_crashes/event_stripper.py index a9035f7aeb2804..0fb514b90897fc 100644 --- a/src/sentry/utils/sdk_crashes/event_stripper.py +++ b/src/sentry/utils/sdk_crashes/event_stripper.py @@ -179,7 +179,7 @@ def strip_frame(frame: MutableMapping[str, Any]) -> MutableMapping[str, Any]: # The path field usually contains the name of the application, which we can't keep. for field in sdk_crash_detector.fields_containing_paths: if frame.get(field): - frame[field] = sdk_crash_detector.replace_sdk_frame_path(frame.get(field)) + frame[field] = sdk_crash_detector.replace_sdk_frame_path(field) else: frame["in_app"] = False