-
Notifications
You must be signed in to change notification settings - Fork 104
feat(processor): Add Span Attachment scrubbing logic #5449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(processor): Add Span Attachment scrubbing logic #5449
Conversation
jjbayer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
|
||
| let processor = PiiAttachmentsProcessor::new(config.compiled()); | ||
| let mut payload = body.to_vec(); | ||
| if processor.scrub_attachment(filename, &mut payload) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should emit the timer(RelayTimers::AttachmentScrubbing) metric here, either directly or by calling processing::utils::attachments::scrub_attachment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will update. Was not a fan of calling fn scrub_attachment(item: &mut crate::envelope::Item, config: &relay_pii::PiiConfig) because than we would need to construct a mock item since we don't have an item here anymore (but maybe doing so is worth it just to have the code all in one place 🤔).
…om:getsentry/relay into tobias-wilfert/feat/attachmentv2-scrubbing
| let filename = meta | ||
| .value() | ||
| .and_then(|m| m.filename.as_str()) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Filename extraction after meta scrubbing breaks body rule matching
The filename is extracted from meta after the meta has already been scrubbed (lines 291-296). Since AttachmentV2Meta.filename is marked with pii = "true", filenames containing PII patterns (like emails or IPs) would be modified during meta scrubbing. When the filename is then used for body scrubbing rule matching (e.g., $attachments.'[email protected]'), the rule won't match because the filename has been scrubbed to something like [email].log. This could cause body scrubbing rules to silently fail, leaving PII unscrubbed in the attachment body.
Adds the logic to scrub the attachment body as well as the attachment meta.
Closes INGEST-648