Skip to content
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

Avoid False Positives Plugin: ArtifactoryDetector #499

Merged
merged 3 commits into from
Oct 4, 2022

Conversation

gabrielsoltz
Copy link
Contributor

@gabrielsoltz gabrielsoltz commented Dec 7, 2021

ArtifactoryDetector plugins it's matching what it's expected but also generate false positives when that regexp it's a substring of another string.
By simply adding $ we avoid matching substring and still enforce the starting of the regexp.

Before:
re.compile(r'(?:\s|=|:|"|^)AKC[a-zA-Z0-9]{10,}') Matchs: AKChOBZmLqdWHyGcBssssd7bssskixTgm2E5P7KN/123456
This is a false positive as / is not part of the regexp.

Now:
re.compile(r'(?:\s|=|:|"|^)AKC[a-zA-Z0-9]{10,}'$) No Match: AKChOBZmLqdWHyGcBssssd7bssskixTgm2E5P7KN/123456
re.compile(r'(?:\s|=|:|"|^)AKC[a-zA-Z0-9]{10,}'$) Match: AKChOBZmLqdWHyGcBssssd7bssskixTgm2E5P7KN

@gabrielsoltz gabrielsoltz changed the title avoid_false_positives Avoid False Positives Plugin: ArtifactoryDetector Dec 7, 2021
Copy link
Contributor

@domanchi domanchi left a comment

Choose a reason for hiding this comment

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

If it passes test cases, it looks good enough for me.

FYI @killuazhu if you wanted to chime in.

@jpdakran
Copy link
Member

@gabrielsoltz Hello. Can you please see my latest commit and verify that it satisfies your main objectives in this PR. Thanks

@gabrielsoltz
Copy link
Contributor Author

LGTM

@jpdakran jpdakran merged commit 16cb918 into Yelp:master Oct 4, 2022
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.

None yet

3 participants