This repository was archived by the owner on Aug 14, 2024. It is now read-only.
Explain how to sanitize url parameters
#760
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
cleptric
reviewed
Nov 15, 2022
antonpirker
changed the title
url parametersurl parameters
marandaneto
reviewed
Nov 15, 2022
src/docs/sdk/features.mdx
Outdated
| @@ -268,6 +268,12 @@ If Performance Monitoring is both supported by the SDK and enabled in the client | |||
| - span status must match HTTP response status code ([see Span status to HTTP status code mapping](/sdk/event-payloads/span/)) | |||
| - when network error occurs, span status must be set to `internal_error` | |||
|
|
|||
| The `url` in breadcumbs and `$url` in span descriptions must be stripped of sensitive data like query parameters and/or username/password. | |||
| This example URL `https://username:password@example.com/bla/blub?token=abc&sessionid=123&save=true#fragment` has to be modified to look like this `https://%s:%s@example.com/bla/blub?token=%s&sessionid=%s&save=%s#fragment`. | |||
There was a problem hiding this comment.
username:password is called userInfo IIRC, or part of the authority.
There was a problem hiding this comment.
Thanks for updating the docs, @antonpirker. The recent addition leaves plenty of room for interpretation. Please be more specific on what sensitive data is precisely.
src/docs/sdk/features.mdx
Outdated
| @@ -268,6 +268,12 @@ If Performance Monitoring is both supported by the SDK and enabled in the client | |||
| - span status must match HTTP response status code ([see Span status to HTTP status code mapping](/sdk/event-payloads/span/)) | |||
| - when network error occurs, span status must be set to `internal_error` | |||
|
|
|||
| The `url` in breadcumbs and `$url` in span descriptions must be stripped of sensitive data like query parameters and/or username/password. | |||
There was a problem hiding this comment.
Right now, query parameters are the only sensitive data we can reliably detect. Which other sensitive data can you think of removing? If any come to mind, please mention that here. This sentence leaves a lot of room for interpretation.
Please try to be as specific as possible.
marandaneto
reviewed
Nov 16, 2022
src/docs/sdk/features.mdx
Outdated
| @@ -268,6 +268,12 @@ If Performance Monitoring is both supported by the SDK and enabled in the client | |||
| - span status must match HTTP response status code ([see Span status to HTTP status code mapping](/sdk/event-payloads/span/)) | |||
| - when network error occurs, span status must be set to `internal_error` | |||
|
|
|||
| The `url` in breadcumbs and `$url` in span descriptions must be stripped of sensitive data like query parameters and/or username/password. | |||
There was a problem hiding this comment.
I suspect that URL info could be in different places as well, See this.
|
|
||
| The SDK should maintain a list of query params that can include sensitive data. The default of the list should be the same list that relay uses to scrub sensitive data: https://github.com/getsentry/relay/blob/master/relay-general/src/pii/regexes.rs#L272 | ||
|
|
||
| The values of all query parameters whose name is in the list of params with sensitive data must be scrubbed. |
There was a problem hiding this comment.
Once we have the list of params, we should add them here.
|
|
||
| SDKs should also give the user the possibility to define a custom list of query param names that should be scrubbed. Given the user full control over what data gets sent do Sentry. | ||
|
|
||
| There should be a config option to the SDKs `init()` call that can set a list of query params that should be scrubbed. By setting this config option the default list of query params with sensitive data is overridden. |
There was a problem hiding this comment.
We still need to specify the name of the option, once we decided on one.
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
| @@ -277,7 +277,7 @@ HTTP Client Integrations record the URLs of HTTP requests. URLs can contain two | |||
|
|
|||
| #### Privacy Related Data | |||
|
|
|||
| Privacy related data from the `url` in breadcumbs and `$url` in span descriptions must always be scrubbed. | |||
| The SDKs must scrub privacy-related data from the `url` in breadcumbs and `$url` in span descriptions. | |||
There was a problem hiding this comment.
🥇 for specifying the subject and using the active voice.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is for getting rid of PII in URLs used in bread crumbs or span descriptions.