feat(pii): Allow pii scrubbing expressions for safe fields (#2670)

This doesn't simply make matching on arrays work, it enables full expression support for safe fields in the datascrubbing config. Fixes: #2605
getsentry · Oct 31, 2023 · 344e258 · 344e258
1 parent 20f72be
commit 344e258
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,11 @@
 - Convert transactions to spans for all organizations. ([#2659](https://github.com/getsentry/relay/pull/2659))
 - 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**:
 
@@ -25,10 +30,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);
@@ -1504,4 +1518,51 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
         process_value(&mut data, &mut pii_processor, ProcessingState::root()).unwrap();
         assert_annotated_snapshot!(data);
     }
+
+    #[test]
+    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 {
+            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_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"
     ],