fix(pii): Disable scrubbing for the User-Agent header (#2641)

Some User-Agents are being scrubbed by the IP rule, this disables scrubbing for the entire Header Fixes: #1826
getsentry · Oct 30, 2023 · a705740 · a705740
1 parent 8b069ce
commit a705740
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,10 @@
 - 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
@@ -14,7 +14,7 @@ use crate::{
 /// We define this list independently of `metastructure(pii = true/false)` because the new PII
 /// scrubber should be able to strip more.
 static DATASCRUBBER_IGNORE: Lazy<SelectorSpec> = Lazy::new(|| {
-    "(debug_meta.** | $frame.filename | $frame.abs_path | $logentry.formatted | $error.value)"
+    "(debug_meta.** | $frame.filename | $frame.abs_path | $logentry.formatted | $error.value | $request.headers.user-agent)"
         .parse()
         .unwrap()
 });
@@ -274,10 +274,10 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
 
     #[test]
     fn test_convert_default_pii_config() {
-        insta::assert_json_snapshot!(simple_enabled_pii_config(), @r#"
+        insta::assert_json_snapshot!(simple_enabled_pii_config(), @r###"
         {
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value)": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent)": [
               "@common:filter",
               "@ip:replace"
             ],
@@ -289,7 +289,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ]
           }
         }
-        "#);
+        "###);
     }
 
     #[test]
@@ -299,10 +299,10 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ..simple_enabled_config()
         });
 
-        insta::assert_json_snapshot!(pii_config, @r#"
+        insta::assert_json_snapshot!(pii_config, @r###"
         {
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value)": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent)": [
               "@common:filter",
               "@ip:replace"
             ],
@@ -314,7 +314,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ]
           }
         }
-        "#);
+        "###);
     }
 
     #[test]
@@ -324,7 +324,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ..simple_enabled_config()
         });
 
-        insta::assert_json_snapshot!(pii_config, @r#"
+        insta::assert_json_snapshot!(pii_config, @r###"
         {
           "rules": {
             "strip-fields": {
@@ -337,7 +337,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             }
           },
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value)": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent)": [
               "@common:filter",
               "@ip:replace",
               "strip-fields"
@@ -350,7 +350,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ]
           }
         }
-        "#);
+        "###);
     }
 
     #[test]
@@ -360,10 +360,10 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ..simple_enabled_config()
         });
 
-        insta::assert_json_snapshot!(pii_config, @r#"
+        insta::assert_json_snapshot!(pii_config, @r###"
         {
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value) && !foobar": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent) && !foobar": [
               "@common:filter",
               "@ip:replace"
             ],
@@ -375,7 +375,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ]
           }
         }
-        "#);
+        "###);
     }
 
     #[test]
@@ -387,18 +387,18 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ..Default::default()
         });
 
-        insta::assert_json_snapshot!(pii_config, @r#"
+        insta::assert_json_snapshot!(pii_config, @r###"
         {
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value)": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent)": [
               "@ip:replace"
             ],
             "$http.env.REMOTE_ADDR || $user.ip_address || $sdk.client_ip": [
               "@anything:remove"
             ]
           }
         }
-        "#);
+        "###);
     }
 
     #[test]
@@ -1256,7 +1256,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ..simple_enabled_config()
         });
 
-        insta::assert_json_snapshot!(pii_config, @r#"
+        insta::assert_json_snapshot!(pii_config, @r###"
         {
           "rules": {
             "strip-fields": {
@@ -1269,7 +1269,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             }
           },
           "applications": {
-            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value)": [
+            "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value || $http.headers.user-agent)": [
               "@common:filter",
               "@ip:replace",
               "strip-fields"
@@ -1282,7 +1282,7 @@ THd+9FBxiHLGXNKhG/FRSyREXEt+NyYIf/0cyByc9tNksat794ddUqnLOg0vwSkv
             ]
           }
         }
-        "#);
+        "###);
 
         let pii_config = pii_config.unwrap();
         let mut pii_processor = PiiProcessor::new(pii_config.compiled());

diff --git a/relay-pii/src/processor.rs b/relay-pii/src/processor.rs
@@ -753,6 +753,35 @@ mod tests {
         assert_annotated_snapshot!(event);
     }
 
+    #[test]
+    fn test_ignore_user_agent_ip_scrubbing() {
+        let mut data = Event::from_value(
+            serde_json::json!({
+                "request": {
+                    "headers": [
+                        ["User-Agent", "127.0.0.1"],
+                        ["X-Client-Ip", "10.0.0.1"]
+                    ]
+                },
+            })
+            .into(),
+        );
+
+        let scrubbing_config = DataScrubbingConfig {
+            scrub_data: true,
+            scrub_ip_addresses: true,
+            scrub_defaults: true,
+            ..Default::default()
+        };
+
+        let pii_config = to_pii_config(&scrubbing_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);
+    }
+
     #[test]
     fn test_remove_debugmeta_path() {
         let config = serde_json::from_str::<PiiConfig>(

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
@@ -1,10 +1,10 @@
 ---
-source: relay-general/src/pii/convert.rs
+source: relay-pii/src/convert.rs
 expression: pii_config
 ---
 {
   "applications": {
-    "($string || $number || $array || $object) && !(debug_meta.** || $frame.filename || $frame.abs_path || $logentry.formatted || $error.value) && !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"
     ],

diff --git a/relay-pii/src/snapshots/relay_pii__processor__tests__ignore_user_agent_ip_scrubbing.snap b/relay-pii/src/snapshots/relay_pii__processor__tests__ignore_user_agent_ip_scrubbing.snap
@@ -0,0 +1,39 @@
+---
+source: relay-pii/src/processor.rs
+expression: "&data"
+---
+{
+  "request": {
+    "headers": [
+      [
+        "User-Agent",
+        "127.0.0.1"
+      ],
+      [
+        "X-Client-Ip",
+        "[ip]"
+      ]
+    ]
+  },
+  "_meta": {
+    "request": {
+      "headers": {
+        "1": {
+          "1": {
+            "": {
+              "rem": [
+                [
+                  "@ip:replace",
+                  "s",
+                  0,
+                  4
+                ]
+              ],
+              "len": 8
+            }
+          }
+        }
+      }
+    }
+  }
+}