Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filtering IP address in Settings - Which option covers what? #5916

Closed
larsqa opened this issue Dec 7, 2022 · 10 comments
Closed

Filtering IP address in Settings - Which option covers what? #5916

larsqa opened this issue Dec 7, 2022 · 10 comments
Labels
Team: Ingest owners-ingest

Comments

@larsqa
Copy link

larsqa commented Dec 7, 2022

Core or SDK?

Core Sentry product

Which part? Which one?

Core Sentry

Description

In Sentry, you have several options to remove IP addresses from being stored.

  1. Inside Security & Privacy Settings, dedicated "Prevent Storing of IP Addresses "toggle button
    image
  2. Custom rule in Security & Privacy Settings
    image

Suggested Solution

The documentation fails to highlight the difference between the two. Which cases do they cover?

@larsqa larsqa changed the title Handling IP address - Too many choices with no clear suggestion Filtering IP address in Settings - Which option covers what? Dec 7, 2022
@imatwawana imatwawana added the Team: Ingest owners-ingest label Dec 7, 2022
@getsentry-release
Copy link
Contributor

Routing to @getsentry/ingest for triage. ⏲️

@olksdr
Copy link
Contributor

olksdr commented Dec 9, 2022

Hi @larsqa,

basically if you use Prevent Storing of IP Addresses from the default settings Sentry will try to scrub all the IPs which can be found in your payload.

But there is also possibility to turn the data scrubbing of the IPs off, as you can see. And you can then, by yourself, add an advance data scrubbing rule, to scrub the IPs only in some specific places. And leave out the rest no scrubbed.

Does this answer your question?

@larsqa
Copy link
Author

larsqa commented Dec 9, 2022

basically if you use Prevent Storing of IP Addresses from the default settings Sentry will try to scrub all the IPs which can be found in your payload.

Is this both for exception and transaction payloads?

But there is also possibility to turn the data scrubbing of the IPs off, as you can see. And you can then, by yourself, add an advance data scrubbing rule, to scrub the IPs only in some specific places. And leave out the rest no scrubbed.

Basically, the Prevent Storing of IP Addresses is the same as a wildcard advanced data scrubbing rule? Do both use the same regex?

@larsqa
Copy link
Author

larsqa commented Dec 9, 2022

@olksdr I have just been in touch with @rhcarvalho via a support ticket. Here is stated:

This toggle in the security settings remove the IP address from the user information (user.ip_address), it will not check every other field. For example, if an IP address is added to another tag, the option "Prevent storing IP address" will not remove it.

Advanced Data Scrubbing rules allow you to set customized scrubbing rules to remove data from any field in the Event. As mentioned in question 4, PII can be present in other fields, and the advanced scrubbing rules are used to remove this information on the server side.

Note that both screenshots for extra/context have IP addresses, these are saved with the Prevent Storing of IP Addresses enabled, and you would have to use advanced rules to scrub this on the server side.

The description under the toggle states:

Preventing IP addresses from being stored for new events

Which one is it now? Does Prevent Storing of IP Addresses only:

  • apply to the event.user.ip_address property
  • to the whole exception payload
  • to the whole transaction payload
  • to all payloads
    ?

@jjbayer
Copy link
Member

jjbayer commented Dec 9, 2022

Prevent Storing of IP Addresses only scrubs these specific fields:

https://github.com/getsentry/relay/blob/e91e1bf6275550fc135cf05852b121311c2de22f/relay-general/src/pii/convert.rs#L15

The one in Advanced Data Scrubbing is more general, scrubbing by regex.

@larsqa
Copy link
Author

larsqa commented Dec 9, 2022

@jjbayer - the link you provided is from the relay repo. My app doesn't use relay. Does this still apply?

@jjbayer
Copy link
Member

jjbayer commented Dec 9, 2022

@jjbayer - the link you provided is from the relay repo. My app doesn't use relay. Does this still apply?

@larsqa yes, sentry also runs a Relay service internally, and that is where the data scrubbing happens.

@larsqa
Copy link
Author

larsqa commented Dec 9, 2022

Alright - good to know this!

Wouldn't it make sense to update the text below the setting in the dashboard? It may confuse users to fully trust that setting to filter out from the full payload.

@jjbayer
Copy link
Member

jjbayer commented Dec 13, 2022

Wouldn't it make sense to update the text below the setting in the dashboard? It may confuse users to fully trust that setting to filter out from the full payload.

Will check with the team if we actually want to expand the behavior of Prevent Storing of IP Addresses (see linked issue). If not, updating the help text definitely makes sense.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Team: Ingest owners-ingest
Projects
None yet
Development

No branches or pull requests

5 participants