Skip to content

Commit

Permalink
fix(pii): Removes legacy special behaviour from safe fields parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
Dav1dde committed Nov 8, 2023
1 parent 4dbecbc commit ef4ab5f
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
**Bug Fixes**:

- Disable scrubbing for the User-Agent header. ([#2641](https://github.com/getsentry/relay/pull/2641))
- Fixes certain safe fields disabling data scrubbing for all string fields. ([#2701](https://github.com/getsentry/relay/pull/2701))

**Internal**:

Expand Down
62 changes: 61 additions & 1 deletion relay-pii/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn to_pii_config(
continue;
}

let spec = match field.parse() {
let spec = match SelectorSpec::parse_non_legacy(field) {
Ok(spec) => spec,
Err(_) => {
// Ideally safe fields should be caught by sentry-side validation,
Expand Down Expand Up @@ -1559,4 +1559,64 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
}
"###);
}

#[test]
fn test_safe_field_legacy_disables_all_string_parsing() {
let mut data = Event::from_value(
serde_json::json!({
"request": {
"data": {
"email": "[email protected]",
"password": "kkee",
"recaptcha": null,
}
},
})
.into(),
);

let pii_config = to_pii_config(&DataScrubbingConfig {
// this triggered legacy behaviour and disabled scrubbing for all fields
exclude_fields: vec!["email".to_owned()],
scrub_data: true,
scrub_ip_addresses: true,
sensitive_fields: vec!["email".to_owned(), "password".to_owned()],
scrub_defaults: true,
..Default::default()
})
.unwrap();

let mut pii_processor = PiiProcessor::new(pii_config.compiled());
process_value(&mut data, &mut pii_processor, ProcessingState::root()).unwrap();
assert_annotated_snapshot!(data, @r###"
{
"request": {
"data": {
"email": "[email protected]",
"password": "[Filtered]",
"recaptcha": null
}
},
"_meta": {
"request": {
"data": {
"password": {
"": {
"rem": [
[
"@password:filter",
"s",
0,
10
]
],
"len": 4
}
}
}
}
}
}
"###);
}
}
23 changes: 14 additions & 9 deletions relay-pii/src/selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,19 @@ pub enum SelectorSpec {
}

impl SelectorSpec {
/// Parses a selector from a string without legacy special handling.
pub fn parse_non_legacy(s: &str) -> Result<SelectorSpec, InvalidSelectorError> {
handle_selector(
SelectorParser::parse(Rule::RootSelector, s)
.map_err(|e| InvalidSelectorError::ParseError(Box::new(e)))?
.next()
.unwrap()
.into_inner()
.next()
.unwrap(),
)
}

/// Checks if a path matches given selector.
///
/// This walks both the selector and the path starting at the end and towards the root
Expand Down Expand Up @@ -315,15 +328,7 @@ impl FromStr for SelectorSpec {
_ => {}
}

handle_selector(
SelectorParser::parse(Rule::RootSelector, s)
.map_err(|e| InvalidSelectorError::ParseError(Box::new(e)))?
.next()
.unwrap()
.into_inner()
.next()
.unwrap(),
)
Self::parse_non_legacy(s)
}
}

Expand Down

0 comments on commit ef4ab5f

Please sign in to comment.