Sentry-dart: Dart sdk missing full (absolute) file path to define ownership rules

Created on 10 Mar 2021  ·  25Comments  ·  Source: getsentry/sentry-dart

_Platform:_

  • [ ] Dart
  • [x] Flutter Android or iOS
  • [ ] Flutter Web

_IDE:_

  • [ ] VSCode
  • [x] IntelliJ/AS
  • [ ] XCode
  • [ ] Other, which one?

_split-debug-info and obfuscate (Flutter Android or iOS) or CanvasKit (Flutter Web):_

  • [ ] Enabled
  • [ ] Disabled

_Platform installed with:_

  • [x] pub.dev
  • [ ] GitHub

Output of the command flutter doctor -v below:

[✓] Flutter (Channel unknown, 1.22.5, on Mac OS X 10.15.7 19H524 darwin-x64, locale en)
• Flutter version 1.22.5 at /Users/lukelee/fvm/versions/1.22.5
• Framework revision 7891006299 (3 months ago), 2020-12-10 11:54:40 -0800
• Engine revision ae90085a84
• Dart version 2.10.4

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.0-rc1)
• Android SDK at /Users/lukelee/Library/Android/sdk
• Platform android-30, build-tools 30.0.0-rc1
• Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 1.8.0_212-release-1586-b4-5784211)
• All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 12.4)
• Xcode at /Applications/Xcode.app/Contents/Developer
• Xcode 12.4, Build version 12D4e
• CocoaPods version 1.10.0

[✓] Android Studio (version 3.6)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin version 45.1.1
• Dart plugin version 192.8052
• Java version OpenJDK Runtime Environment (build 1.8.0_212-release-1586-b4-5784211)

[✓] Connected device (2 available)
• Android SDK built for x86 64 (mobile) • emulator-5554 • android-x64 • Android 10 (API 29)
(emulator)
• macOS (desktop) • macos • darwin-x64 • Mac OS X 10.15.7 19H524
darwin-x64

• No issues found!
The version of the SDK (See pubspec.lock):
4.0.4


I have the following issue:

Dart sdk is missing a full (absolute) file path to define ownership rules.
The data is included in JSON files as abs_filepath but the path is not popping up in the ownership rules section.

Steps to reproduce:

  • Step 1: trigger an exception in a flutter app.
  • Step 2: go to sentry and find the exception you triggered.
  • Step 3: click Create Ownership Rule on the right panel.
  • Step 4: the full absolute file path is not popping up so you can't utilize the ownership rule feature to define the paths to assign to a specific team.

Screen Shot 2021-03-10 at 1 24 23 PM

Actual result:
When we use flutter sdk, we don't get the full path.

  • Actual
    Screen Shot 2021-03-03 at 3 51 43 PM

Expected result:
When we log to Android SDK via method channel, we get this full path.

  • Result
    Screen Shot 2021-03-10 at 1 11 08 PM
enhancement

Most helpful comment

I would guess splitting at the lib folder would be a good starting point as practically every Dart programm uses it.
There would be of course some problems if there are more than one lib folder.

All 25 comments

@lukesterlee thanks for reporting this, the reason is:

https://github.com/getsentry/sentry-dart/blob/main/dart/lib/src/sentry_stack_trace_factory.dart#L127-L143

does it mean that using the Flutter SDK works but the pure Dart SDK does not?

eg:
file:///Users/myuser/Github/sentry-dart/dart/example/main.dart so we have PII here but there's also no easy way to know when the project actually starts which would be sentry-dart.
Ideas on how to do it?

I was not aware this would affect ownership rules, good catch

I can confirm that, Flutter SDK would return package:sentry_flutter_example/main.dart instead of the file absolute path.

the problem is, for a Dart App, the frame.uri.scheme is actually a file scheme and we need to filter that :( not sure how we can go around this.

@bruno-garcia ideas?

I only see a way if filtering frame.uri.pathSegments but we'd need to know where to start discarding PII info

I would guess splitting at the lib folder would be a good starting point as practically every Dart programm uses it.
There would be of course some problems if there are more than one lib folder.

yep @ueman that's one of my thoughts, for scripting it'd not work though, but I guess heuristics would be the way to go, or, Dart offering a proper relative path property.
The other way would be getting the user's folder from the OS and substring from there, it'd not avoid PII entirely though.

There is also https://api.dart.dev/stable/2.12.1/dart-io/Platform-class.html which should help in some cases. I'm pretty sure it doesn't work with Flutter, I would have to double check that however.

There is also https://api.dart.dev/stable/2.12.1/dart-io/Platform-class.html which should help in some cases. I'm pretty sure it doesn't work with Flutter, I would have to double check that however.

still returns /Users/myuser/Github/sentry-dart/dart/example/main.dart but yeah I've not googled it yet, might have a reliable workaround somewhere hidden in the Webs :D

@marandaneto I'm sorry for the confusion, I meant we do get the right data in both android sdk and flutter sdk but flutter sdk does not know how to parse the data in the ownership rules section since they are parsing filename instead of abs_path.

This is from our JSON raw payload:

{
"function":"CheckingCardService.lock",
"package":"betterment_flutter",
"filename":"checking_card_service.dart",
"abs_path":"package:betterment_flutter/features/checking/manage_account/services/checking_card_service.dart",
"lineno":39,
"colno":7,
"in_app":true
}

As you can see in the previous screenshots, it seems like android sdk's ownership rules parse abs_path while flutter sdk's ownership rules parse filename. I think there is no problem with sdk, we just have a problem with pulling the right attribute in ownership rules section on the web?

by the way, thanks for the quick response! I really appreciate it. 🙌

@lukesterlee thanks for your reply.

I am a bit lost here.

The data is included in JSON files as abs_filepath but the path is not popping up in the ownership rules section.

this is indeed an issue (Sentry Dart SDK), but unless we use some heuristics, there's no native way to get proper absolute paths without risking PII.

the Sentry Flutter SDK though gets proper absolute file paths, as the tooling doesn't include PII so we send it as it is, so it's all good afaik.

When we log to Android SDK via method channel, we get this full path.

what do you mean by Android SDK? are you talking about the Sentry Flutter SDK or Native SDKs (Java/Kotlin and Swift/ObjC events)? Please review your description so we can get a better understanding :)

(i work with @lukesterlee)

this is indeed an issue (Sentry Dart SDK), but unless we use some heuristics, there's no native way to get proper absolute paths without risking PII.

@marandaneto would it be possible to introduce something like inAppIncludes to identify the root of the app within the file system? https://github.com/getsentry/sentry-dart/blob/ae21d7e44203fb544f97bd7b21b9bd2b4b779454/dart/lib/src/sentry_options.dart#L123

with something like that, it would be possible to trim all path segments _before_ that one and call that the absolute path, right?

edit: and i think that logic would only want to be applied to files whose scheme is file: right?

@lukesterlee thanks for reporting this, the reason is:

https://github.com/getsentry/sentry-dart/blob/main/dart/lib/src/sentry_stack_trace_factory.dart#L127-L143

does it mean that using the Flutter SDK works but the pure Dart SDK does not?

@marandaneto another question, just for clarification, i'm not quite understanding how this code is converting package:betterment_flutter/something.dart into something.dart. i'd expect that since the scheme is package it would not be stripping the path down to just the filename. can you explain why that's happening?

(i work with @lukesterlee)

this is indeed an issue (Sentry Dart SDK), but unless we use some heuristics, there's no native way to get proper absolute paths without risking PII.

@marandaneto would it be possible to introduce something like inAppIncludes to identify the root of the app within the file system?

https://github.com/getsentry/sentry-dart/blob/ae21d7e44203fb544f97bd7b21b9bd2b4b779454/dart/lib/src/sentry_options.dart#L123

with something like that, it would be possible to trim all path segments _before_ that one and call that the absolute path, right?

not sure how that would work as the dir. path would be dynamic depending on the user's folder etc.

edit: and i think that logic would only want to be applied to files whose scheme is file: right?

yes, only to file, when its package or something else, the absolute path is already fine

@lukesterlee thanks for reporting this, the reason is:
https://github.com/getsentry/sentry-dart/blob/main/dart/lib/src/sentry_stack_trace_factory.dart#L127-L143
does it mean that using the Flutter SDK works but the pure Dart SDK does not?

@marandaneto another question, just for clarification, i'm not quite understanding how this code is converting package:betterment_flutter/something.dart into something.dart. i'd expect that since the scheme is package it would not be stripping the path down to just the filename. can you explain why that's happening?

it's not in such case, only for file:...

One proposal we're considering is to disable this PII stripping in the SDK if sendDefaultPii is set to true.

Related: #360

Docs (JS for now until we release for Dart):
https://docs.sentry.io/platforms/javascript/configuration/options/#send-default-pii

I think mentioning android/dart sdk might have confused you. I will try my best to state it with more clarity!
I'm using Flutter SDK 4.0.4.

Step1: make the flutter part crash, then go to the dashboard.

Screen Shot 2021-03-16 at 11 17 19 PM

As you can see ^ above, I was able to see the full path when I hovered checking_card_service.dart exception. (package:betterment_flutter/features/checking/manage_account/services/checking_card_service.dart)

Step2: click Create Ownership Rule on the right panel in the dashboard of the issue.

Screen Shot 2021-03-17 at 1 12 30 PM

Then, you don't see the full path.

Screen Shot 2021-03-16 at 11 17 41 PM

Therefore, I can't use the ownership rule feature since I can't get the full path.

Let me know if there is any step I missed!

@lukesterlee oh yeah indeed that makes sense, please open an issue on https://github.com/getsentry/sentry for that, I'll keep this open here because the pure Dart SDK strip the absolute path off anyway, thanks for reporting.

the file path is used for grouping, so if we change that, we'd need to consider bumping a major bump, we could do it now btw before v5

This line could be related: https://github.com/getsentry/sentry-dart/blob/a3f28f9a5da4404e1650c547603330020ce73ca3/dart/lib/src/sentry_stack_trace_factory.dart#L139

A frame can be relative even if it does not has a scheme.

This line https://github.com/getsentry/sentry-dart/blob/a3f28f9a5da4404e1650c547603330020ce73ca3/dart/lib/src/sentry_stack_trace_factory.dart#L74 just always uses the last segment of the path, even for relativ paths. That would explain the bug which the user described.

I'm guessing that the web path might also be wrong, though I haven't checked that yet.

the reasoning for not using the full segment was because sometimes it contains PII and events coming from different machines would not be grouped together, its not a bug but something we should improve.

I figured out what's the problem. We're sending just the file name as the file_name property.
The docs and examples however state that we should include the file path relative to the project root.

So given the following folder structure, where app is the project root and app is also the name given in the pubspec.yaml

.
└── app/
    └── lib/
        ├── main.dart
        └── foo_bar/
            └── foo_bar.dart

Current behaviour:

  • app/lib/main.dart is reported as main.dart
  • app/lib/foo_bar/foo_bar.dart is reported as foo_bar.dart

This is what we would get by just parsing the stacktrace

  • app/lib/main.dart is reported as app/main.dart
  • app/lib/foo_bar/foo_bar.dart is reported as app/foo_bar/foo_bar.dart

I'm not quite sure what the expected behaviour should be.
I think it should be what the stacktrace is saying but I'm not sure how the webservice handles it

I'm not quite sure what the expected behaviour should be.
I think it should be what the stacktrace is saying but I'm not sure how the webservice handles it

Nice sleuthing.

I'm not sure what the web service expect either, but it appears to accept 2 params: filename and absPath. Typically, i expect that filename is just the final segment of the filepath, so in your examples: main.dart is the filename. Although, given what I described here, it seems like the web service prioritizes filename over absPath in ownership rules which basically means that if the submitted filename only contains the final segment of the file path then the ownership rules glob matching is pretty limited / nearly useless (context: https://github.com/getsentry/sentry/issues/24525#issuecomment-802407388).

So, without a change to the server, one thing this library could do is to start submitting filename as the modified file path strings you showed in the "just by parsing the stacktrace" section. Not sure if y'all wanna do that or not though 🤷🏻

It does make sense to send the full path. Otherwise files with the same name in different folders can't be distinguished.

Though because of the way imports work in Dart, there could be PII in the import path.
For example if you're using relative paths and reference a file in your user folder.

It's quite hard to make it easily usable in every use case :(

I believe package imports should be fine and included by default with its path.

Though because of the way imports work in Dart, there could be PII in the import path.

PII in paths are handled by Sentry I believe. Where it can remove C:\Users\[filtered]\ We should send the paths if we can find a way. This problem exists on Android too where Java only gives us the file name without a path. One option there is to have a best effort an convert the package to a path hoping it's the default class loader (io.sentry.test becomes io/sentry/test/File.java)

I need to check how ownership rules work, its probably based on file path instead of packages or something else, let me get back to this in a bit.

https://github.com/getsentry/sentry/blob/51fffaad802a1f8c4331bf6030fd5f23d8673bff/src/sentry/ownership/grammar.py#L95 is the responsible field (filename) for ownership rules, ideally, the server could work with the "packages" concept too, but that would take longer.
a quick workaround would be what @bruno-garcia said.

io.sentry.test.File becomes io/sentry/test/File.java on Android

https://develop.sentry.dev/sdk/event-payloads/stacktrace/#frame-attributes

filename

The path to the source file relative to the project root directory.

abs_path

The absolute path to the source file.

I'll check if Sentry handles the file path PII automatically or if we should guard against sendDefaultPii

Was this page helpful?
0 / 5 - 0 ratings