Implement broad-spectrum filtering for possible path and filenames in exception strings.

Change-Id: Ibe3d08739e2f1f199f11d40c01699ff194fc5028
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303442
Commit-Queue: Janice Collins <jcollins@google.com>
Reviewed-by: Jacob Richman <jacobr@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/telemetry/lib/crash_reporting.dart

@@ -9,6 +9,7 @@ import 'dart:math' as math;
 import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
 import 'package:stack_trace/stack_trace.dart';
+import 'package:telemetry/src/pii_regexp.dart';
 
 import 'src/utils.dart';
 
@@ -129,7 +130,7 @@ class CrashReportSender {
       fields['osVersion'] = Platform.operatingSystemVersion;
       fields['type'] = _dartTypeId;
       fields['error_runtime_type'] = '${error.runtimeType}';
-      fields['error_message'] = '$error';
+      fields['error_message'] = filterPiiFromErrorMessage('$error');
 
       // Optional comments.
       if (comment != null) {

pkg/telemetry/lib/src/pii_regexp.dart

@@ -0,0 +1,77 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// This library gives some backstops to make sure that arbitrary string
+/// exceptions generated by something that might be sent to the crash reporting
+/// framework do not contain PII.   The regular expressions here are
+/// intentionally broad to make sure they get everything, possibly at the
+/// expense of readability for the error message.  Use care when reducing
+/// their scope.
+library pii_regexp;
+
+/// Contain an ordered list of regular expressions to be applied in sequence
+/// with [applyTo] to a string, with any matches being substituted with
+/// [substitution].  While a single regular expression could be
+/// used in place of this class, this is intending to make the regular
+/// expressions being applied more comprehensible to non-expert regular
+/// expression authors and to make it very simple to add or remove individual
+/// cases.
+class _RegExpList {
+  /// This list should be usually be ordered from more specific to more general
+  /// cases in the event the regular expressions overlap.
+  final List<RegExp> _regExps;
+  final String substitution;
+
+  _RegExpList(List<String> uncompiledRegexps, this.substitution)
+      : _regExps = uncompiledRegexps.map((s) => RegExp(s)).toList();
+
+  String applyTo(String input) => _regExps.fold(
+      input, (previousInput, r) => previousInput.replaceAll(r, substitution));
+}
+
+/// An ordered list of regular expressions to be substituted out and replaced
+/// with a generic '<path>' string. To be applied in sequence.
+/// The regular expressions here intentionally eat all characters after a
+/// path is identified, with the hope of catching all potential complex
+/// pathnames and assuming that most error messages will have any paths last.
+/// For example: `throw Exception('error in: $fileName)`
+final _piiPathRegexps = _RegExpList([
+  // C:\Some\Things\Including Spaces\Too
+  r'\w:\\.*',
+  // \\Windows\Shares\Look\Like This
+  r'\\\\\S+\\.*',
+  // ..\StartsWithDots\Windows Path
+  r'[.]+\\+\S*\\.*',
+  // A\Relative\Windows Path
+  r'\b\S+\\.*',
+  // ../StartsWithDots/Unix path
+  r'[.]+/\S*/.*',
+  // /AUnix/path
+  r'/(\S|[.])+/.*',
+  // a/unix\ relative path/possibly-containing-strange-characters
+  r'\b(\S|[.])+/.*',
+], '<path>');
+
+/// An ordered list of regular expressions to be substituted out and replaced
+/// with a generic '<filename>' string.  To be applied after
+/// all paths from [_piiPathRegexps] are applied.   This is somewhat less
+/// all-encompassing than the path finder, above, just to try and grab things
+/// that could be file names but without being quite so greedy or depending
+/// on the presence of path separators.
+final _piiFileRegexps = _RegExpList([
+  // foo.dart
+  // something.exe
+  // code.js
+  r'\b\S+[.]\w{1,4}\b',
+], '<filename>');
+
+/// Attempt to remove things that look like they could be PII from the
+/// given [message].
+/// Returns the filtered string.
+/// Not suitable for pre-scrubbed strings that intentionally include things
+/// that look like filenames.
+String filterPiiFromErrorMessage(String message) => [
+      _piiPathRegexps,
+      _piiFileRegexps,
+    ].fold(message, (previousMessage, r) => r.applyTo(previousMessage));

pkg/telemetry/test/crash_reporting_test.dart

@@ -43,6 +43,18 @@ void main() {
       expect(body, contains('test-error'));
     });
 
+    test('hits pii filters', () async {
+      CrashReportSender sender = CrashReportSender.prod(
+          analytics.trackingId, shouldSend,
+          httpClient: mockClient);
+
+      await sender.sendReport('filter this: filename.exe', StackTrace.current);
+
+      String body = utf8.decode(request.bodyBytes);
+      expect(body, contains('String')); // error.runtimeType
+      expect(body, contains('filter this: <filename>'));
+    });
+
     test('reportsSent', () async {
       CrashReportSender sender = CrashReportSender.prod(
           analytics.trackingId, shouldSend,

pkg/telemetry/test/pii_regexp_test.dart

@@ -0,0 +1,69 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:telemetry/src/pii_regexp.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('filterPiiFromErrorMessage', () {
+    test('filters paths greedily', () {
+      expect(filterPiiFromErrorMessage(r'a message: C:\My\Special Path'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(
+              r'a message: C:\My\Overly\..\Complicated\Special Path'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(r'a message: ..\A\Relative\Windows Path'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(
+              r'a message: A\Relatively\..\Complicated\Windows Path'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(r'a message: \\My\Windows\Network Share'),
+          equals('a message: <path>'));
+      expect(filterPiiFromErrorMessage(r'a message: Q:\I Have A Lot of Drives'),
+          equals('a message: <path>'));
+      expect(filterPiiFromErrorMessage(r'a message: R:\'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(r'a message: /Users/macuser/Desktop/dart/'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(
+              r'a message: ../relative_path/to_some/dart'),
+          equals('a message: <path>'));
+      expect(
+          filterPiiFromErrorMessage(
+              r'a message: embedded_dots/./../to_some/dart'),
+          equals('a message: <path>'));
+    });
+    test('filters some complex examples derived from real exceptions', () {
+      // Real world examples may not actually be perfectly readable due to the
+      // greediness of the matchers, but that's the tradeoff for making sure we
+      // get everything.
+      expect(
+          filterPiiFromErrorMessage(
+              r'Could not resolve "package:an_important_package/an_important_package.dart" in C:\Users\AUser\dart\files\foo.dart'),
+          equals('Could not resolve "<path>'));
+    });
+    test('filters filenames without path separators without being greedy', () {
+      expect(filterPiiFromErrorMessage(r'mine.dart: a special file'),
+          equals('<filename>: a special file'));
+      expect(filterPiiFromErrorMessage(r'foo.js: a special file'),
+          equals('<filename>: a special file'));
+      expect(filterPiiFromErrorMessage(r'something.exe: a special file'),
+          equals('<filename>: a special file'));
+    });
+    test('does not filter things that should not match', () {
+      expect(filterPiiFromErrorMessage('User ran a Commodore 64 executable'),
+          equals('User ran a Commodore 64 executable'));
+      expect(
+          filterPiiFromErrorMessage(
+              'franchiseError: somehow Palpatine returned'),
+          equals('franchiseError: somehow Palpatine returned'));
+    });
+  });
+}