feat(protocol): Implement response context schema [INGEST-1661] #1529
Conversation
There was a problem hiding this comment.
Nice, thanks for picking this up! Can we add a test case to verify that the following headers are successfully stripped by pii scrubbing?
Authorization
Proxy-Authorization
Set-Cookie
For more context, see #1501. That issue is closed but the headers mentioned are still relevant, Set-Cookie in particular.
| /// HTTP response body size. | ||
| pub body_size: Annotated<u64>, | ||
|
|
||
| /// Additional arbitrary fields for forwards compatibility. |
There was a problem hiding this comment.
Just adding some context here:
| /// Additional arbitrary fields for forwards compatibility. | |
| /// Additional arbitrary fields for forwards compatibility. | |
| /// | |
| /// These fields are retained (`retain = "true"`) to keep supporting the format that the Dio integration sends: | |
| /// https://github.com/getsentry/sentry-dart/blob/7011abe27ac69bd160bdc6ecf3314974b8340b97/dart/lib/src/protocol/sentry_response.dart#L4-L8 |
| pub struct ResponseContext { | ||
| /// Type `response`. | ||
| #[metastructure(field = "type")] | ||
| pub ty: Annotated<String>, |
There was a problem hiding this comment.
The type is injected by the Contexts wrapper, so this should not be added as a field here.
|
|
||
| #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] | ||
| #[cfg_attr(feature = "jsonschema", derive(JsonSchema))] | ||
| pub struct ResponseContext { |
relay-general/src/pii/regexes.rs
Outdated
| @@ -269,6 +269,6 @@ static US_SSN_REGEX: Lazy<Regex> = Lazy::new(|| { | |||
|
|
|||
| static PASSWORD_KEY_REGEX: Lazy<Regex> = Lazy::new(|| { | |||
| Regex::new( | |||
| r"(?i)(password|secret|passwd|api_key|apikey|auth|credentials|mysql_pwd|privatekey|private_key|.*token.*)" | |||
| r"(?i)(password|secret|passwd|api_key|apikey|auth|credentials|mysql_pwd|privatekey|private_key|set-cookie|.*token.*)" | |||
There was a problem hiding this comment.
Something I missed on the previous PR: If auth matches Proxy-Authorization, then token matches whatever_token_123 ... so I think we can omit the .*
| r"(?i)(password|secret|passwd|api_key|apikey|auth|credentials|mysql_pwd|privatekey|private_key|set-cookie|.*token.*)" | |
| r"(?i)(password|secret|passwd|api_key|apikey|auth|credentials|mysql_pwd|privatekey|private_key|set-cookie|token)" |
There was a problem hiding this comment.
good point! done in 8e37424
| }; | ||
|
|
||
| if response.cookies.value().is_some() { | ||
| headers.remove("Cookie"); |
There was a problem hiding this comment.
Since Cookie is not a response header, I think we should use Set-Cookie here.
@marandaneto was the intention of the cookies field to contain the parsed contents of the Set-Cookie header?
There was a problem hiding this comment.
Not really, I just noticed that they were not scrubbed and opened the issue, but they were response headers and I was looking into the request headers, so it made sense.
Your call if it should be removed or not due to PII concerns.
There was a problem hiding this comment.
@jjbayer pointed that Cookie is the request header and is not expected in the response. We can just follow pattern and do what was proposed above, remove set-cookie header and convert those values to cookies when needed.
|
@jjbayer if you wanna take another look just to double check if everything is good to go here. |
|
@olksdr is this going to be automatically reflected in https://develop.sentry.dev/sdk/event-payloads/types/#event ? |
yes |
|
corresponding commit in getsentry/develop@6a7e102 |
* master: release: 0.8.15 fix(py): Respect the renormalize flag (#1548) (fix)e2e: Use report self hosted issues env variable (#1539) meta(vscode): Enable all features in Rust-Analyzer (#1542) release: 0.8.14 build(craft): Fix manylinux artifact name (#1547) feat(quotas): New data category for indexed transactions (#1535) test(auth): Unflake re_auth_failure (#1531) replays: add warning log for parse errors (#1534) fix(server): Retain valid cached project states on error (#1426) feat(protocol): Implement response context schema (#1529) feat(replays): emit org_id on recording kafka messages (#1528) feat: Add .NET/Portable-PDB specific protocol fields (#1518) feat(quotas): Enforce rate limits on metrics buckets (#1515) ref(pii): Consider all token as sensitive [INGEST-1550] (#1527) release: 22.10.0
Add
Responsecontext schema and addbody_sizefield to the request.closes: #1530