Skip to content

replay: add sensitive content widget to default masking #2989

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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Add [SensitiveContent](https://main-api.flutter.dev/flutter/widgets/SensitiveContent-class.html) widget to default masking ([#2989](https://github.com/getsentry/sentry-dart/pull/2989))
- Tag all spans with thread info ([#3101](https://github.com/getsentry/sentry-dart/pull/3101))

### Enhancements
Expand Down
58 changes: 58 additions & 0 deletions packages/flutter/lib/src/sentry_privacy_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import 'package:meta/meta.dart';
import '../sentry_flutter.dart';
import 'screenshot/masking_config.dart';
import 'screenshot/widget_filter.dart';
// ignore: implementation_imports
import 'package:sentry/src/event_processor/enricher/flutter_runtime.dart'
as flutter_runtime;

/// Configuration of the experimental privacy feature.
class SentryPrivacyOptions {
Expand Down Expand Up @@ -75,6 +78,11 @@ class SentryPrivacyOptions {
));
}

const flutterVersion = flutter_runtime.FlutterVersion.version;
if (flutterVersion != null) {
_maybeAddSensitiveContentRule(rules, flutterVersion);
}

// In Debug mode, check if users explicitly mask (or unmask) widgets that
// look like they should be masked, e.g. Videos, WebViews, etc.
if (runtimeChecker.isDebugMode()) {
Expand Down Expand Up @@ -159,6 +167,56 @@ class SentryPrivacyOptions {
}
}

/// Returns `true` if a SensitiveContent masking rule _should_ be added for a
/// given [flutterVersion] string. The SensitiveContent widget was introduced
/// in Flutter 3.33, therefore we only add the masking rule when the detected
/// version is >= 3.33.
bool _shouldAddSensitiveContentRule(String version) {
final dot = version.indexOf('.');
if (dot == -1) return false;

final major = int.tryParse(version.substring(0, dot));
final nextDot = version.indexOf('.', dot + 1);
final minor = int.tryParse(
version.substring(dot + 1, nextDot == -1 ? version.length : nextDot));

return major != null &&
minor != null &&
(major > 3 || (major == 3 && minor >= 33));
}

/// Adds a masking rule for the [SensitiveContent] widget.
///
/// The rule masks any widget that exposes a `sensitivity` property which is an
/// [Enum]. This is how the [SensitiveContent] widget can be detected
/// without depending on its type directly (which would fail to compile on
/// older Flutter versions).
void _maybeAddSensitiveContentRule(
List<SentryMaskingRule> rules, String flutterVersion) {
if (!_shouldAddSensitiveContentRule(flutterVersion)) {
return;
}

SentryMaskingDecision maskSensitiveContent(Element element, Widget widget) {
try {
final dynamic dynWidget = widget;
final sensitivity = dynWidget.sensitivity;
// If the property exists, we assume this is the SensitiveContent widget.
assert(sensitivity is Enum);
return SentryMaskingDecision.mask;
} catch (_) {
// Property not found – continue processing other rules.
return SentryMaskingDecision.continueProcessing;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Masking Behavior Varies Between Modes

The maskSensitiveContent function exhibits inconsistent widget masking behavior between debug and release modes. In debug mode, widgets are only masked if their sensitivity property exists and is an Enum (due to an assert statement; otherwise, the try/catch prevents masking). In release mode, the assert is disabled, causing any widget with a sensitivity property to be masked, regardless of its type. This leads to different masking behavior between development and production environments, making testing unreliable.

Fix in Cursor Fix in Web


rules.add(SentryMaskingCustomRule<Widget>(
callback: maskSensitiveContent,
name: 'SensitiveContent',
description: 'Mask SensitiveContent widget.',
));
}

SentryMaskingDecision _maskImagesExceptAssets(Element element, Image widget) {
final image = widget.image;
if (image is AssetBundleImageProvider) {
Expand Down
72 changes: 66 additions & 6 deletions packages/flutter/test/screenshot/masking_config_test.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
Expand Down Expand Up @@ -125,7 +126,7 @@ void main() async {
SentryMaskingDecision.unmask);
});

testWidgets('retuns false if no rule matches', (tester) async {
testWidgets('returns false if no rule matches', (tester) async {
final sut = SentryMaskingConfig([
SentryMaskingCustomRule<Image>(
callback: (e, w) => SentryMaskingDecision.continueProcessing,
Expand Down Expand Up @@ -169,6 +170,7 @@ void main() async {
'SentryMaskingConstantRule<Text>(mask)',
'SentryMaskingConstantRule<EditableText>(mask)',
'SentryMaskingConstantRule<RichText>(mask)',
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
]);
});
Expand All @@ -181,6 +183,7 @@ void main() async {
expect(rulesAsStrings(sut), [
...alwaysEnabledRules,
'SentryMaskingConstantRule<Image>(mask)',
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
]);
});
Expand All @@ -193,6 +196,7 @@ void main() async {
expect(rulesAsStrings(sut), [
...alwaysEnabledRules,
'SentryMaskingCustomRule<Image>(Mask all images except asset images.)',
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
]);
});
Expand All @@ -207,6 +211,7 @@ void main() async {
'SentryMaskingConstantRule<Text>(mask)',
'SentryMaskingConstantRule<EditableText>(mask)',
'SentryMaskingConstantRule<RichText>(mask)',
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
]);
});
Expand All @@ -218,31 +223,66 @@ void main() async {
..maskAssetImages = false;
expect(rulesAsStrings(sut), [
...alwaysEnabledRules,
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
]);
});

test(
'SensitiveContent rule is automatically added when current Flutter version is equal or newer than 3.33',
() {
final sut = SentryPrivacyOptions();
final version = FlutterVersion.version!;
final dot = version.indexOf('.');
final major = int.tryParse(version.substring(0, dot));
final nextDot = version.indexOf('.', dot + 1);
final minor = int.tryParse(
version.substring(dot + 1, nextDot == -1 ? version.length : nextDot));

if (major! > 3 || (major == 3 && minor! >= 33)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Version Parsing Flaw in Test Code

The version parsing logic in masking_config_test.dart and its _maybeWithSensitiveContent helper function incorrectly uses non-null assertions (major!, minor!) on int.tryParse() results. This lack of null checking can lead to runtime exceptions for malformed version strings, a flaw not present in the main sentry_privacy_options.dart implementation.

Fix in Cursor Fix in Web

expect(
rulesAsStrings(sut).contains(
'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)'),
isTrue,
reason: 'Test failed with version: ${FlutterVersion.version}');
} else {
expect(
rulesAsStrings(sut).contains(
'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)'),
isFalse,
reason: 'Test failed with version: ${FlutterVersion.version}');
}
}, skip: FlutterVersion.version == null);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Version Parsing Vulnerability in SensitiveContent Rule

The version parsing logic in the _maybeWithSensitiveContent() helper function and the SensitiveContent rule test case is unsafe. It can lead to a RangeError if the Flutter version string lacks a dot, and runtime exceptions if int.tryParse() returns null. This is due to missing boundary checks before substring() and reliance on null assertion operators (!) without prior null checks, contrasting with the main implementation's robust error handling.

Additional Locations (1)
Fix in Cursor Fix in Web


group('user rules', () {
final defaultRules = [
...alwaysEnabledRules,
'SentryMaskingCustomRule<Image>(Mask all images except asset images.)',
'SentryMaskingConstantRule<Text>(mask)',
'SentryMaskingConstantRule<EditableText>(mask)',
'SentryMaskingConstantRule<RichText>(mask)',
..._maybeWithSensitiveContent(),
'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)'
];

test('mask() takes precedence', () {
final sut = SentryPrivacyOptions();
sut.mask<Image>();
expect(rulesAsStrings(sut),
['SentryMaskingConstantRule<Image>(mask)', ...defaultRules]);
expect(rulesAsStrings(sut), [
'SentryMaskingConstantRule<Image>(mask)',
...defaultRules,
]);
});

test('unmask() takes precedence', () {
final sut = SentryPrivacyOptions();
sut.unmask<Image>();
expect(rulesAsStrings(sut),
['SentryMaskingConstantRule<Image>(unmask)', ...defaultRules]);
expect(rulesAsStrings(sut), [
'SentryMaskingConstantRule<Image>(unmask)',
...defaultRules,
]);
});

test('are ordered in the call order', () {
var sut = SentryPrivacyOptions();
sut.mask<Image>();
Expand Down Expand Up @@ -272,13 +312,14 @@ void main() async {
...defaultRules
]);
});

test('maskCallback() takes precedence', () {
final sut = SentryPrivacyOptions();
sut.maskCallback(
(Element element, Image widget) => SentryMaskingDecision.mask);
expect(rulesAsStrings(sut), [
'SentryMaskingCustomRule<Image>(Custom callback-based rule (description unspecified))',
...defaultRules
...defaultRules,
]);
});
test('User cannot add $SentryMask and $SentryUnmask rules', () {
Expand All @@ -300,6 +341,25 @@ void main() async {
});
}

List<String> _maybeWithSensitiveContent() {
final version = FlutterVersion.version;
if (version == null) {
return [];
}
final dot = version.indexOf('.');
final major = int.tryParse(version.substring(0, dot));
final nextDot = version.indexOf('.', dot + 1);
final minor = int.tryParse(
version.substring(dot + 1, nextDot == -1 ? version.length : nextDot));
if (major! > 3 || (major == 3 && minor! >= 33)) {
return [
'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)'
];
} else {
return [];
}
}

extension on Element {
Element findFirstOfType<T>() {
late Element result;
Expand Down
Loading