convert exclude fields into selector

getsentry · Oct 30, 2023 · f51327f · f51327f
1 parent f641185
commit f51327f
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,11 @@
 - Group resource spans by scrubbed domain and filename. ([#2654](https://github.com/getsentry/relay/pull/2654))
 - Filter outliers (>180s) for mobile measurements. ([#2649](https://github.com/getsentry/relay/pull/2649))
 - Allow access to more context fields in dynamic sampling and metric extraction. ([#2607](https://github.com/getsentry/relay/pull/2607), [#2640](https://github.com/getsentry/relay/pull/2640))
+- Allow advanced scrubbing expressions for datascrubbing safe fields. ([#2670](https://github.com/getsentry/relay/pull/2670))
+
+**Bug Fixes**:
+
+- Disable scrubbing for the User-Agent header. ([#2641](https://github.com/getsentry/relay/pull/2641))
 
 **Internal**:
 
@@ -23,10 +28,6 @@
 - Remove event spans starting or ending before January 1, 1970 UTC. ([#2627](https://github.com/getsentry/relay/pull/2627))
 - Remove event breadcrumbs dating before January 1, 1970 UTC. ([#2635](https://github.com/getsentry/relay/pull/2635))
 
-**Bug Fixes**:
-
-- Disable scrubbing for the User-Agent header. ([#2641](https://github.com/getsentry/relay/pull/2641))
-
 **Internal**:
 
 - Report global config fetch errors after interval of constant failures elapsed. ([#2628](https://github.com/getsentry/relay/pull/2628))

diff --git a/relay-pii/src/convert.rs b/relay-pii/src/convert.rs
@@ -143,9 +143,23 @@ pub fn to_pii_config(
             continue;
         }
 
-        conjunctions.push(SelectorSpec::Not(Box::new(SelectorSpec::Path(vec![
-            SelectorPathItem::Key(field.to_owned()),
-        ]))));
+        let spec = match field.parse() {
+            Ok(spec) => spec,
+            Err(error) => {
+                // Invalid safe fields should be caught by sentry-side validation.
+                // Log an error if they are not.
+                relay_log::error!(
+                    error = &error as &dyn std::error::Error,
+                    field = field,
+                    "Error parsing safe field into selector",
+                );
+
+                // Fallback to stay compatible with already existing keys.
+                SelectorSpec::Path(vec![SelectorPathItem::Key(field.to_owned())])
+            }
+        };
+
+        conjunctions.push(SelectorSpec::Not(Box::new(spec)));
     }
 
     let applied_selector = SelectorSpec::And(conjunctions);
@@ -164,10 +178,9 @@ pub fn to_pii_config(
 
 #[cfg(test)]
 mod tests {
-    use insta::assert_debug_snapshot;
     use relay_event_schema::processor::{process_value, ProcessingState};
     use relay_event_schema::protocol::Event;
-    use relay_protocol::{assert_annotated_snapshot, get_value, FromValue};
+    use relay_protocol::{assert_annotated_snapshot, FromValue};
     use similar_asserts::assert_eq;
 
     use crate::PiiProcessor;
@@ -1507,29 +1520,49 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
     }
 
     #[test]
-    fn test_exclude_list() {
+    fn test_exclude_expression() {
         let mut data = Event::from_value(
             serde_json::json!({
                 "extra": {
                     "do_not_scrub_1": "password",
                     "do_not_scrub_2": ["password"],
+                    "do_not_scrub.dot": ["password"],
+                },
+                "user": {
+                    "id": "5355849125500546",
                 }
             })
             .into(),
         );
 
         let pii_config = to_pii_config(&DataScrubbingConfig {
-            //sensitive_fields: vec![],
-            exclude_fields: vec!["do_not_scrub_1".into(), "do_not_scrub_2".into()],
+            exclude_fields: vec![
+                "do_not_scrub_1".to_owned(),
+                "do_not_scrub_2.**".to_owned(),
+                "extra.'do_not_scrub.dot'.**".to_owned(),
+                "$user.id".to_owned(),
+            ],
             ..simple_enabled_config()
         })
         .unwrap();
 
         let mut pii_processor = PiiProcessor::new(pii_config.compiled());
         process_value(&mut data, &mut pii_processor, ProcessingState::root()).unwrap();
-        assert_debug_snapshot!(
-            get_value!(data.extra!),
-            @""
-        );
+        assert_annotated_snapshot!(data, @r###"
+        {
+          "user": {
+            "id": "5355849125500546"
+          },
+          "extra": {
+            "do_not_scrub.dot": [
+              "password"
+            ],
+            "do_not_scrub_1": "password",
+            "do_not_scrub_2": [
+              "password"
+            ]
+          }
+        }
+        "###);
     }
 }
diff --git a/relay-pii/src/snapshots/relay_pii__convert__tests__regression_more_odd_keys.snap b/relay-pii/src/snapshots/relay_pii__convert__tests__regression_more_odd_keys.snap
@@ -4,7 +4,7 @@ expression: pii_config
 ---
 {
   "applications": {
-    "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent) && !url && !message && !'http.request.url' && !'*url*' && !'*message*' && !'*http.request.url*'": [
+    "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent) && !url && !message && !http.request.url && !'*url*' && !'*message*' && !'*http.request.url*'": [
       "@common:filter",
       "@ip:replace"
     ],