Tribler / tribler

Privacy enhanced BitTorrent client with P2P content discovery
https://www.tribler.org
GNU General Public License v3.0
4.8k stars 444 forks source link

Reports to Sentry are silently ignored on some Tribler installations #7966

Closed kozlovsky closed 5 months ago

kozlovsky commented 5 months ago

Recently, I discovered that when an error occurs on my machine and the Feedback dialog suggests sending the report to the Tribler developer, clicking the "SEND REPORT" button does nothing. The confirmation box with the message "Successfully sent the report! Thanks for your contribution" shows, but no errors appear on the Sentry server.

Specifying the debug=True option when calling sentry_sdk.init(...) allows me to see Sentry logs configured weirdly:

With enabled Sentry logs, the following log record appeared in stderr:

 [sentry] ERROR: Unexpected status code: 400 (body: b'{"detail":"invalid JSON data","causes":["invalid value: string \\"<>d27beecffcdb4caa9f914986dcc56828<>\\", expected an event identifier at line 1 column 8048"]}')

It allowed me to investigate the reason for the error. As it turns out, SentryScrubber produced a corrupted error report that was then rejected by the Sentry server.

SentryScrubber tries to remove sensitive information from the error report. It generates a set of sensitive strings with corresponding pseudo-random substitutes. For example, a username "John" may be replaced with the string "<foo>", and a machine name "JOHN-PC" may be replaced with the string "<bar>". Then, using regular expressions, SentryScrubber replaces corresponding substrings in all places of the error report.

As it turns out, the reason for the problem was that sometimes SentryReported decides to replace an empty string "" with the substitution "<>" and applies this replacement to all possible substrings. As a result, even elements added to the report by Sentry itself were transformed; for example, the identifier value

"d27beecffcdb4caa9f914986dcc56828"

was transformed into

"<>d27beecffcdb4caa9f914986dcc56828<>"

and all other string elements of the report were changed in the same way.

As a simple fix, SentryScrubber should stop considering empty strings as a potential target for replacement. After such a fix, with the current logic, SentryScrubber still has a chance to corrupt the report if some sensitive value accidentally matches the Sentry technical field value, but the chance of corruption becomes much lower.

As another change, it is worthwhile to configure Sentry logs in the following way: