Skip to content

[brainstorming] Data security/PII improvements #5526

Description

@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:

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.

Activity

  1. untitaker commented on Aug 5, 2022

    @untitaker
    Member

    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.

  2. lobsterkatie commented on Aug 5, 2022

    @lobsterkatie
    MemberAuthor

    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.

  3. AbhiPrasad commented on Aug 11, 2022

    @AbhiPrasad
    Contributor

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

  4. locked and limited conversation to collaborators on Oct 14, 2022
  5. converted this issue into a discussion #5961 on Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions