feat(pii): Scrub arrays of 2 elements as key-value pairs

CHANGELOG.md

@@ -9,6 +9,7 @@
 **Features**:
 
 - Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552))
+- Allow treating arrays of two elements as key-value pairs during PII scrubbing. ([#3639](https://github.com/getsentry/relay/pull/3639))
 
 **Internal**:
 

relay-pii/src/processor.rs

@@ -5,13 +5,13 @@ use std::sync::OnceLock;
 
 use regex::Regex;
 use relay_event_schema::processor::{
-    self, enum_set, Chunk, Pii, ProcessValue, ProcessingAction, ProcessingResult, ProcessingState,
-    Processor, ValueType,
+    self, enum_set, process_value, Chunk, Pii, ProcessValue, ProcessingAction, ProcessingResult,
+    ProcessingState, Processor, ValueType,
 };
 use relay_event_schema::protocol::{
     AsPair, Event, IpAddr, NativeImagePath, PairList, Replay, ResponseContext, User,
 };
-use relay_protocol::{Annotated, Meta, Remark, RemarkType, Value};
+use relay_protocol::{Annotated, Array, Meta, Remark, RemarkType, Value};
 
 use crate::compiledconfig::{CompiledPiiConfig, RuleRef};
 use crate::config::RuleType;
@@ -101,6 +101,41 @@ impl<'a> Processor for PiiProcessor<'a> {
         self.apply_all_rules(meta, state, None)
     }
 
+    fn process_array<T>(
+        &mut self,
+        value: &mut Array<T>,
+        _meta: &mut Meta,
+        state: &ProcessingState<'_>,
+    ) -> ProcessingResult
+    where
+        T: ProcessValue,
+    {
+        // If the array has length 2, we treat it as key-value pair and try to scrub it. If the
+        // scrubbing doesn't do anything, we try to scrub values individually.
+        if value.len() == 2 {
+            if let Some(key) = value[0].clone().into_value() {
+                if let Value::String(key_name) = key.into_value() {
+                    if let Some(inner_value) = value[1].value() {
+                        // We compute a new state which has the first element of the array as the
+                        // key.
+                        let entered = state.enter_borrowed(
+                            key_name.as_str(),
+                            state.inner_attrs(),
+                            inner_value.value_type(),
+                        );
+                        process_value(&mut value[1], self, &entered)?;
+                    }
+                }
+            }
+        }
+
+        // Recurse into each element of the array since we also want to individually scrub each
+        // value in the array.
+        value.process_child_values(self, state)?;
+
+        Ok(())
+    }
+
     fn process_string(
         &mut self,
         value: &mut String,
@@ -466,10 +501,10 @@ fn insert_replacement_chunks(rule: &RuleRef, text: &str, output: &mut Vec<Chunk<
 
 #[cfg(test)]
 mod tests {
-    use insta::assert_debug_snapshot;
+    use insta::{allow_duplicates, assert_debug_snapshot};
     use relay_event_schema::processor::process_value;
     use relay_event_schema::protocol::{
-        Addr, Breadcrumb, DebugImage, DebugMeta, ExtraValue, Headers, LogEntry, Message,
+        Addr, Breadcrumb, DebugImage, DebugMeta, ExtraValue, FrameVars, Headers, LogEntry, Message,
         NativeDebugImage, Request, Span, TagEntry, Tags, TraceContext,
     };
     use relay_protocol::{assert_annotated_snapshot, get_value, FromValue, Object};
@@ -489,6 +524,30 @@ mod tests {
         rv
     }
 
+    fn extract_vars(event: Option<&Event>) -> &FrameVars {
+        event
+            .unwrap()
+            .exceptions
+            .value()
+            .unwrap()
+            .values
+            .value()
+            .unwrap()[0]
+            .value()
+            .unwrap()
+            .stacktrace
+            .value()
+            .unwrap()
+            .frames
+            .value()
+            .unwrap()[0]
+            .value()
+            .unwrap()
+            .vars
+            .value()
+            .unwrap()
+    }
+
     #[test]
     fn test_scrub_original_value() {
         let mut data = Event::from_value(
@@ -1598,4 +1657,202 @@ mod tests {
     ],
 )"#);
     }
+
+    #[test]
+    fn test_tuple_array_scrubbed_with_path_selector() {
+        // We expect that both of these configs express the same semantics.
+        let configs = vec![
+            // This configuration matches on the authorization element (the 1st element of the array
+            // represents the key).
+            r##"
+                {
+                    "applications": {
+                        "exception.values.0.stacktrace.frames.0.vars.headers.0.authorization": ["@anything:replace"]
+                    }
+                }
+                "##,
+            // This configuration matches on the 2nd element of the array.
+            r##"
+                {
+                    "applications": {
+                        "exception.values.0.stacktrace.frames.0.vars.headers.0.1": ["@anything:replace"]
+                    }
+                }
+                "##,
+        ];
+
+        let mut event = Event::from_value(
+            serde_json::json!(
+            {
+              "message": "hi",
+              "exception": {
+                "values": [
+                  {
+                    "type": "BrokenException",
+                    "value": "Something failed",
+                    "stacktrace": {
+                      "frames": [
+                        {
+                            "vars": {
+                                "headers": [
+                                    ["authorization", "Bearer abc123"]
+                                ]
+                            }
+                        }
+                      ]
+                    }
+                  }
+                ]
+              }
+            })
+            .into(),
+        );
+
+        for config in configs {
+            let config = serde_json::from_str::<PiiConfig>(config).unwrap();
+            let mut processor = PiiProcessor::new(config.compiled());
+            process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
+
+            let vars = extract_vars(event.value());
+
+            allow_duplicates!(assert_debug_snapshot!(vars, @r#"
+        FrameVars(
+            {
+                "headers": Array(
+                    [
+                        Array(
+                            [
+                                String(
+                                    "authorization",
+                                ),
+                                Annotated(
+                                    String(
+                                        "[Filtered]",
+                                    ),
+                                    Meta {
+                                        remarks: [
+                                            Remark {
+                                                ty: Substituted,
+                                                rule_id: "@anything:replace",
+                                                range: Some(
+                                                    (
+                                                        0,
+                                                        10,
+                                                    ),
+                                                ),
+                                            },
+                                        ],
+                                        errors: [],
+                                        original_length: Some(
+                                            13,
+                                        ),
+                                        original_value: None,
+                                    },
+                                ),
+                            ],
+                        ),
+                    ],
+                ),
+            },
+        )
+        "#));
+        }
+    }
+
+    #[test]
+    fn test_tuple_array_scrubbed_with_string_selector_and_password_matcher() {
+        let config = serde_json::from_str::<PiiConfig>(
+            r##"
+                {
+                    "applications": {
+                        "$string": ["@password:remove"]
+                    }
+                }
+                "##,
+        )
+        .unwrap();
+
+        let mut event = Event::from_value(
+            serde_json::json!(
+            {
+              "message": "hi",
+              "exception": {
+                "values": [
+                  {
+                    "type": "BrokenException",
+                    "value": "Something failed",
+                    "stacktrace": {
+                      "frames": [
+                        {
+                            "vars": {
+                                "headers": [
+                                    ["authorization", "abc123"]
+                                ]
+                            }
+                        }
+                      ]
+                    }
+                  }
+                ]
+              }
+            })
+            .into(),
+        );
+
+        let mut processor = PiiProcessor::new(config.compiled());
+        process_value(&mut event, &mut processor, ProcessingState::root()).unwrap();
+
+        let vars = extract_vars(event.value());
+
+        assert_debug_snapshot!(vars, @r###"
+        FrameVars(
+            {
+                "headers": Array(
+                    [
+                        Array(
+                            [
+                                Annotated(
+                                    String(
+                                        "",
+                                    ),
+                                    Meta {
+                                        remarks: [
+                                            Remark {
+                                                ty: Removed,
+                                                rule_id: "@password:remove",
+                                                range: Some(
+                                                    (
+                                                        0,
+                                                        0,
+                                                    ),
+                                                ),
+                                            },
+                                        ],
+                                        errors: [],
+                                        original_length: Some(
+                                            13,
+                                        ),
+                                        original_value: None,
+                                    },
+                                ),
+                                Meta {
+                                    remarks: [
+                                        Remark {
+                                            ty: Removed,
+                                            rule_id: "@password:remove",
+                                            range: None,
+                                        },
+                                    ],
+                                    errors: [],
+                                    original_length: None,
+                                    original_value: None,
+                                },
+                            ],
+                        ),
+                    ],
+                ),
+            },
+        )
+        "###);
+    }
 }

relay-pii/src/selector.rs

@@ -154,7 +154,7 @@ impl SelectorPathItem {
 
 /// A selector that can match paths of processing states.
 ///
-/// To use a a selector, you most likely want to check whether it matches the path of a
+/// To use a selector, you most likely want to check whether it matches the path of a
 /// [`ProcessingState`].  For this you turn the state into a [`Path`] using
 /// [`ProcessingState::path`] and call [`SelectorSpec::matches_path`], which will iterate through
 /// the path items in the processing state and check whether the selector matches.