[brainstorming] Data security/PII improvements

[lobsterkatie]

Starting this issue in the wake of the Slope crypto hack (where secrets were compromised by being logged to the console and therefore picked up in breadcrumbs) to think about what we might do to help prevent such a thing in the future. To be clear, in this instance Sentry was only the messenger, so to speak, transmitting and storing data that shouldn't have been out there in the first place. Nonetheless, there might be things we could do on the SDK side to make another such hack less likely.

Already done:

• Add to our default list of keys whose values get scrubbed when an event is ingested: fix(pii): Add private keys as secret key name relay#1376

Other possible ideas:

• Put a warning in the docs somewhere (maybe the breadcrumbs docs) basically saying, "Beware: If you log secret stuff to the console, it will get sent to Sentry unless you either turn off console breadcrumbs or scrub the data from the event before it's sent," with appropriate links to how to do those things.

• While we're not going to duplicate in the SDK anything close to the full server-side scrubbing system, we could think about creating an integration which does basic keyname matching and redacts any associated values during event processing.

• Other things I'm not thinking of? Feel free to add to this list.

[untitaker]

While we're not going to duplicate in the SDK anything close to the full server-side scrubbing system, we could think about creating an integration which does basic keyname matching and redacts any associated values during event processing.

IMO we shouldn't do that if we can avoid it. Anything going into the direction of trying to find patterns (even just substrings) recursively in extra or the like creates serious performance overhead. PII scrubbing is by far the slowest operation in Relay already, and customers care much more if it happens on their machines and causes overhead in their app.

[lobsterkatie]

That's a fair concern. I wonder if there aren't people who might choose to pay the performance cost for the sake of the scrubbing, though, especially in browser-based SDKs (and maybe RN)., where it's only one user's worth of events ever being processed. I'm not saying it should be enabled by default, but unless we're sure it's going to tank their apps, any reason not to think about giving people something easy to turn on if they want to?

Two other thoughts on that: 1) We only serialize to a certain depth, so if that came first, it could limit how much ground the scrubber needs to cover. 2) Another option is to have this happen while we're normalizing, since we're traversing the event data at that point anyway.

[AbhiPrasad]

We've started to document some of this here getsentry/develop#663