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

Support multiple detectors per match #2065

Merged
merged 17 commits into from
Nov 3, 2023

Conversation

rosecodym
Copy link
Collaborator

@rosecodym rosecodym commented Oct 30, 2023

Description:

#1711 inadvertently removed the ability to match multiple custom detectors, or multiple detectors of the same type but different version, to a given keyword. (#2060 re-added support for multiple versions of detectors globally, and #2064 re-added support for multiple custom detectors globally, but neither fixed trufflehog's inability to support multiple such detectors for a given keyword match.) This PR re-adds the removed functionality (and narrows the AhoCorasickCore interface in the process.)

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@rosecodym rosecodym force-pushed the support-multiple-detectors-per-match branch from 326f910 to b7939ff Compare November 1, 2023 18:23
@rosecodym rosecodym marked this pull request as ready for review November 1, 2023 18:23
@rosecodym rosecodym requested a review from a team as a code owner November 1, 2023 18:23
pkg/engine/ahocorasickcore.go Outdated Show resolved Hide resolved
pkg/engine/ahocorasickcore.go Outdated Show resolved Hide resolved
@rosecodym
Copy link
Collaborator Author

@ankushgoel27 has confirmed that this branch resolves at least one case of improper detector versioning they've seen!

@rosecodym rosecodym requested review from mcastorina and ahrav November 2, 2023 17:11
@rosecodym
Copy link
Collaborator Author

looping @ahrav in to see if i missed anything his original implementation covered

Copy link
Collaborator

@ahrav ahrav left a comment

Choose a reason for hiding this comment

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

This looks like a good solution to me. Thanks for cleaning this up.

@rosecodym rosecodym merged commit 7a15633 into main Nov 3, 2023
@rosecodym rosecodym deleted the support-multiple-detectors-per-match branch November 3, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants