diff --git a/relay-event-schema/src/processor/traits.rs b/relay-event-schema/src/processor/traits.rs index b6bf80a3124..7729b5b5057 100644 --- a/relay-event-schema/src/processor/traits.rs +++ b/relay-event-schema/src/processor/traits.rs @@ -114,7 +114,7 @@ pub trait Processor: Sized { process_method!(process_trace_context, crate::protocol::TraceContext); process_method!(process_native_image_path, crate::protocol::NativeImagePath); process_method!(process_contexts, crate::protocol::Contexts); - process_method!(process_attributes, crate::protocol::Attributes); + process_method!(process_attribute, crate::protocol::Attribute); fn process_other( &mut self, diff --git a/relay-event-schema/src/protocol/attributes.rs b/relay-event-schema/src/protocol/attributes.rs index 6cd3d248926..b6caa3704b0 100644 --- a/relay-event-schema/src/protocol/attributes.rs +++ b/relay-event-schema/src/protocol/attributes.rs @@ -6,7 +6,7 @@ use std::{borrow::Borrow, fmt}; use crate::processor::{ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType}; -#[derive(Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] +#[derive(Clone, PartialEq, Empty, FromValue, IntoValue)] pub struct Attribute { #[metastructure(flatten)] pub value: AttributeValue, @@ -38,6 +38,36 @@ impl fmt::Debug for Attribute { } } +impl ProcessValue for Attribute { + fn value_type(&self) -> EnumSet { + ValueType::for_field(&self.value.value) + } + + fn process_value

( + &mut self, + meta: &mut Meta, + processor: &mut P, + state: &ProcessingState<'_>, + ) -> ProcessingResult + where + P: Processor, + { + processor.process_attribute(self, meta, state) + } + + fn process_child_values

( + &mut self, + processor: &mut P, + state: &ProcessingState<'_>, + ) -> ProcessingResult + where + P: Processor, + { + self.value.process_child_values(processor, state)?; + self.other.process_child_values(processor, state) + } +} + #[derive(Debug, Clone, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] pub struct AttributeValue { #[metastructure(field = "type", required = true, trim = false)] @@ -152,7 +182,7 @@ impl IntoValue for AttributeType { } /// Wrapper struct around a collection of attributes with some convenience methods. -#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue)] +#[derive(Debug, Clone, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)] pub struct Attributes(pub Object); impl Attributes { @@ -242,34 +272,3 @@ impl FromIterator<(String, Annotated)> for Attributes { Self(Object::from_iter(iter)) } } - -impl ProcessValue for Attributes { - #[inline] - fn value_type(&self) -> EnumSet { - EnumSet::only(ValueType::Object) - } - - #[inline] - fn process_value

( - &mut self, - meta: &mut Meta, - processor: &mut P, - state: &ProcessingState<'_>, - ) -> ProcessingResult - where - P: Processor, - { - processor.process_attributes(self, meta, state) - } - - fn process_child_values

( - &mut self, - _processor: &mut P, - _state: &ProcessingState<'_>, - ) -> ProcessingResult - where - P: Processor, - { - Ok(()) - } -} diff --git a/relay-pii/src/processor.rs b/relay-pii/src/processor.rs index a5f84488cb2..a7e360b8828 100644 --- a/relay-pii/src/processor.rs +++ b/relay-pii/src/processor.rs @@ -5,8 +5,8 @@ use std::sync::OnceLock; use regex::Regex; use relay_event_schema::processor::{ - self, Chunk, Pii, ProcessValue, ProcessingAction, ProcessingResult, ProcessingState, Processor, - ValueType, enum_set, process_value, + self, Chunk, FieldAttrs, Pii, ProcessValue, ProcessingAction, ProcessingResult, + ProcessingState, Processor, ValueType, enum_set, process_value, }; use relay_event_schema::protocol::{ AsPair, Event, IpAddr, NativeImagePath, PairList, Replay, ResponseContext, User, @@ -19,8 +19,21 @@ use crate::redactions::Redaction; use crate::regexes::{self, ANYTHING_REGEX, PatternType, ReplaceBehavior}; use crate::utils; +/// Controls how scrubbing rules are applied to attributes. +#[derive(Debug, Clone, Copy)] +pub enum AttributeMode { + /// Treat the attribute as an object and allow referring + /// to individual fields. + Object, + /// Identify the attribute with its value and apply all + /// rules there directly. + ValueOnly, +} + /// A processor that performs PII stripping. pub struct PiiProcessor<'a> { + /// Controls how rules are applied to attributes. + attribute_mode: AttributeMode, compiled_config: &'a CompiledPiiConfig, } @@ -29,7 +42,16 @@ impl<'a> PiiProcessor<'a> { pub fn new(compiled_config: &'a CompiledPiiConfig) -> PiiProcessor<'a> { // this constructor needs to be cheap... a new PiiProcessor is created for each event. Move // any init logic into CompiledPiiConfig::new. - PiiProcessor { compiled_config } + PiiProcessor { + compiled_config, + attribute_mode: AttributeMode::Object, + } + } + + /// Sets an `AttributeMode` on this processor. + pub fn attribute_mode(mut self, attribute_mode: AttributeMode) -> Self { + self.attribute_mode = attribute_mode; + self } fn apply_all_rules( @@ -45,10 +67,8 @@ impl<'a> PiiProcessor<'a> { for (selector, rules) in self.compiled_config.applications.iter() { if selector.matches_path(&state.path()) { - #[allow(clippy::needless_option_as_deref)] for rule in rules { - let reborrowed_value = value.as_deref_mut(); - apply_rule_to_value(meta, rule, state.path().key(), reborrowed_value)?; + apply_rule_to_value(meta, rule, state.path().key(), value.as_deref_mut())?; } } } @@ -199,13 +219,24 @@ impl Processor for PiiProcessor<'_> { utils::process_pairlist(self, value, state) } - fn process_attributes( + fn process_attribute( &mut self, - value: &mut relay_event_schema::protocol::Attributes, + value: &mut relay_event_schema::protocol::Attribute, _meta: &mut Meta, state: &ProcessingState, ) -> ProcessingResult { - utils::process_attributes(value, self, state) + match self.attribute_mode { + AttributeMode::Object => { + // Treat the attribute as an object and recurse into its children. + value.process_child_values(self, state) + } + AttributeMode::ValueOnly => { + // Identify the attribute with its value and apply rules there directly. + let value_state = + state.enter_nothing(Some(Cow::Owned(FieldAttrs::new().pii(Pii::True)))); + processor::process_value(&mut value.value.value, self, &value_state) + } + } } fn process_user( diff --git a/relay-pii/src/utils.rs b/relay-pii/src/utils.rs index e6757ed888b..4e27b8dedb4 100644 --- a/relay-pii/src/utils.rs +++ b/relay-pii/src/utils.rs @@ -1,12 +1,9 @@ -use std::borrow::Cow; - use hmac::{Hmac, Mac}; use relay_event_schema::processor::{ - self, FieldAttrs, Pii, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType, - enum_set, + self, ProcessValue, ProcessingResult, ProcessingState, Processor, ValueType, }; -use relay_event_schema::protocol::{AsPair, Attributes, PairList}; +use relay_event_schema::protocol::{AsPair, PairList}; use sha1::Sha1; pub fn process_pairlist( @@ -41,46 +38,6 @@ pub fn process_pairlist( Ok(()) } -pub fn process_attributes( - value: &mut Attributes, - slf: &mut P, - state: &ProcessingState, -) -> ProcessingResult { - // Check if we're in default rules (no root ValueType) - // This is a workaround to support explicit selectors eg. $log.attributes.KEY.value to work but keep implicit selectors for default rules. - let is_advanced_rules = state - .iter() - .nth(1) - .is_some_and(|s| s.value_type().contains(ValueType::OurLog)); - - for (key, annotated_attribute) in value.iter_mut() { - if let Some(attribute) = annotated_attribute.value_mut() { - if is_advanced_rules { - // Process normally to allow explicit selectors like $log.attributes.KEY.value to work - let field_value_type = ValueType::for_field(annotated_attribute); - let key_state = state.enter_borrowed(key, state.inner_attrs(), field_value_type); - processor::process_value(annotated_attribute, slf, &key_state)?; - } else { - // For the default rules, we want Pii::True since we want them to always be scrubbed. - let attrs = FieldAttrs::new().pii(Pii::True); - let inner_value = &mut attribute.value.value; - let inner_value_type = ValueType::for_field(inner_value); - let entered = - state.enter_borrowed(key, Some(Cow::Borrowed(&attrs)), inner_value_type); - processor::process_value(inner_value, slf, &entered)?; - - let object_value_type = enum_set!(ValueType::Object); - for (key, value) in attribute.other.iter_mut() { - let other_state = - state.enter_borrowed(key, Some(Cow::Borrowed(&attrs)), object_value_type); - processor::process_value(value, slf, &other_state)?; - } - } - } - } - Ok(()) -} - pub fn hash_value(data: &[u8]) -> String { let mut mac = Hmac::::new_from_slice(&[]).unwrap(); mac.update(data); diff --git a/relay-server/src/processing/logs/process.rs b/relay-server/src/processing/logs/process.rs index e4d39b9aa5d..da0d116582b 100644 --- a/relay-server/src/processing/logs/process.rs +++ b/relay-server/src/processing/logs/process.rs @@ -1,8 +1,8 @@ use relay_event_normalization::{SchemaProcessor, eap}; -use relay_event_schema::processor::{ProcessingState, ValueType, process_value}; +use relay_event_schema::processor::{ProcessingState, process_value}; use relay_event_schema::protocol::{OurLog, OurLogHeader}; use relay_ourlogs::OtelLog; -use relay_pii::PiiProcessor; +use relay_pii::{AttributeMode, PiiProcessor}; use relay_protocol::Annotated; use relay_quotas::DataCategory; @@ -139,17 +139,17 @@ fn scrub_log(log: &mut Annotated, ctx: Context<'_>) -> Result<()> { .map_err(|e| Error::PiiConfig(e.clone()))?; if let Some(ref config) = ctx.project_info.config.pii_config { - let mut processor = PiiProcessor::new(config.compiled()); - let root_state = ProcessingState::root().enter_static("", None, Some(ValueType::OurLog)); - process_value(log, &mut processor, &root_state)?; + let mut processor = PiiProcessor::new(config.compiled()) + // For advanced rules we want to treat attributes as objects. + .attribute_mode(AttributeMode::Object); + process_value(log, &mut processor, ProcessingState::root())?; } if let Some(config) = pii_config_from_scrubbing { - let mut processor = PiiProcessor::new(config.compiled()); - // Use empty root (assumed to be Event) for legacy/default scrubbing rules. - // process_attributes will collapse Attribute into it's value for the default rules. - let root_state = ProcessingState::root(); - process_value(log, &mut processor, root_state)?; + let mut processor = PiiProcessor::new(config.compiled()) + // For "legacy" rules we want to identify attributes with their values. + .attribute_mode(AttributeMode::ValueOnly); + process_value(log, &mut processor, ProcessingState::root())?; } Ok(()) @@ -309,10 +309,7 @@ mod tests { scrubbing_config.scrub_data = true; scrubbing_config.scrub_defaults = true; scrubbing_config.scrub_ip_addresses = true; - scrubbing_config.sensitive_fields = vec![ - "value".to_owned(), // Make sure the inner 'value' of the attribute object isn't scrubbed. - "very_sensitive_data".to_owned(), - ]; + scrubbing_config.sensitive_fields = vec!["very_sensitive_data".to_owned()]; scrubbing_config.exclude_fields = vec!["public_data".to_owned()]; let ctx = make_context(scrubbing_config, None); @@ -1098,7 +1095,7 @@ mod tests { }, }, "applications": { - "$log.attributes.remove_this_string_abc123.value": ["remove_abc123"], + "attributes.remove_this_string_abc123.value": ["remove_abc123"], } })) .unwrap();