getsentry / team-mobile

Meta issues for the Mobile team
4 stars 1 forks source link

SQL span integrations shouldn't include values in span descriptions to avoid PII #34

Open bruno-garcia opened 2 years ago

bruno-garcia commented 2 years ago

If we just take the raw SQL (or ORMs such as AndroidX Room, Apple Core Data, .NET EF) and add to Sentry events. In the form of crumbs or spans, it can leak PII

We need to have some guidelines for SDK engineers. Similar to the HTTP one on the develop docs. There we can point out possible pitfalls and what's suggested in such cases.

Some open questions:

Some notes from @untitaker:

sending the parameters should definetly be behind sendDefaultPii=true, because the higher-level guidance that we should communicate is to not send PII by default. I don't think it makes as much sense to exhaustively enumerate every piece of the event payload and whether it is PII. that is always dependent on what you're going to put there. for example, if you know a particular db parameter value is not PII because of some platform peculiarity, advice like "this location in the event is PII" doesn't apply and you should feel free to send it

  • anything that is typically PII by nature has to be behind sendDefaultPii. this implies that db parameters are, but we don't need to document that
  • span descriptions should not contain high-cardinality fields, because we attempt to group db queries together by description
brustolin commented 2 years ago

We need to update documentation to explain that user need to use parameterised queries instead of writing filter values direct in the query, because this may leak PII.

kahest commented 1 year ago

There's been 3 recent RFCs about scrubbing and documenting PII:

This issue should be re-written to take the outcome of the RFCs into account.