Skip to content

Conversation

@vbartonicek
Copy link
Contributor

@vbartonicek vbartonicek commented Jan 25, 2024

This updates the email regex to exclude any strings containing a valid email. Check updated tests for more info.

@vbartonicek vbartonicek requested review from a team January 25, 2024 22:29
@deomsj
Copy link
Contributor

deomsj commented Jan 30, 2024

For current scope of impact from various libs that we manage, I found this visual from kibana (see screenshot below) and this integrationMethodMap in our int dash code to be helpful.

Screenshot 2024-01-31 at 4 03 38 PM

Our "Frontend Beacon / UI" delegates network calls to this client-js lib, so I feel great about where we are adding this layer of security / defensive programming.

Copy link
Contributor

@deomsj deomsj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for leading the way on this @vbartonicek. Changes to util and related specs make sense and work great, nice work!

const PII_REGEX = [
{
pattern: /^[\w\-+\\.]+@([\w-]+\.)+[\w-]{2,4}$/,
pattern: /[\w\-+\\.]+@([\w-]+\.)+[\w-]{2,4}/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing the "starts with" (^) and "ends with" ($) criteria from this pattern makes perfect conceptual sense to me.

this not only helps us match on strings with whitespace on either side of an email but also with other characters as well. We do want to prevent sending emails in tracking events regardless of whether the entire string is an email. that is what this change does. i like it

Copy link
Contributor

@esezen esezen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@esezen esezen merged commit 737ac2f into master Jan 31, 2024
@esezen esezen deleted the psl-3160-investigate-and-fix-emails-detected-in-search-submit-terms branch January 31, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants