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

Expose detector-specific false positive logic #2743

Merged
merged 17 commits into from
Apr 30, 2024

Conversation

rosecodym
Copy link
Contributor

@rosecodym rosecodym commented Apr 24, 2024

Description:

This PR:

  • Creates an optional interface that detectors can use to customize their false positive detection
  • Implements this interface on detectors that have custom logic
  • In most cases this "custom logic" is simply a no-op because the detector does not participate in false positive detection
  • Eliminates inline (old-style) false positive exclusion in a few detectors that Move detectors.IsKnownFalsePositive from the detectors and into the engine #2643 missed

Checklist:

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

@rosecodym rosecodym force-pushed the th-605-detector-fp-customization branch from 2d2527d to 8fcbddb Compare April 24, 2024 22:06
// This function will check false positives for common test words, but also it will make sure the key appears 'random' enough to be a real key.
if !s1.Verified {
if detectors.IsKnownFalsePositive(idMatch, detectors.DefaultFalsePositives, true) ||
detectors.IsKnownFalsePositive(secretMatch, detectors.DefaultFalsePositives, true) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This custom logic wasn't moved into a CustomFalsePositiveChecker implementation because secretMatch is no longer available by the time that runs. This type of change (which "narrows" false positive detection) was made for multiple other detectors in #2643 so I'm not worried about it.

@rosecodym rosecodym marked this pull request as ready for review April 25, 2024 15:27
@rosecodym rosecodym requested review from a team as code owners April 25, 2024 15:27
@@ -43,6 +45,17 @@ func init() {
filter = builder.Build()
}

func GetFalsePositiveCheck(detector Detector) func(Result) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Neat!

@rosecodym rosecodym merged commit 2f7029b into main Apr 30, 2024
12 checks passed
@rosecodym rosecodym deleted the th-605-detector-fp-customization branch April 30, 2024 20:10
@@ -109,6 +110,10 @@ func (c *CustomRegexWebhook) FromData(ctx context.Context, verify bool, data []b
return results, nil
}

func (c *CustomRegexWebhook) IsFalsePositive(_ detectors.Result) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: If all parameters are ignored, you can omit _. You can even omit c in the pointer receiver.

func (*CustomRegexWebhook) IsFalsePositive(detectors.Result) bool

Comment on lines +98 to +100
func (s Scanner) IsFalsePositive(_ detectors.Result) bool {
return false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you consider creating a struct that implements this so it can be embedded into the Scanner struct?

// in detectors package
type SkipFalsePositiveCheck struct{}

func (SkipFalsePositiveCheck) IsFalsePositive(Result) bool {
    return false
}

// elsewhere

type Scanner struct {
     detectors.SkipFalsePositiveCheck
}

Copy link
Contributor Author

@rosecodym rosecodym May 1, 2024

Choose a reason for hiding this comment

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

no, i didn't! does that save any implementation effort?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Marginally? imo it's a "nice-to-have" for a common scenario, especially for areas of the code that get a lot of outside contribution.

haraldh added a commit to matter-labs/vault-auth-tee that referenced this pull request May 6, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[trufflesecurity/trufflehog](https://togithub.com/trufflesecurity/trufflehog)
| action | minor | `v3.74.0` -> `v3.75.0` |

---

### Release Notes

<details>
<summary>trufflesecurity/trufflehog
(trufflesecurity/trufflehog)</summary>

###
[`v3.75.0`](https://togithub.com/trufflesecurity/trufflehog/releases/tag/v3.75.0)

[Compare
Source](https://togithub.com/trufflesecurity/trufflehog/compare/v3.74.0...v3.75.0)

#### What's Changed

- \[chore] - update buffer metrics by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2737
- fix(deps): update module github.com/aws/aws-sdk-go to v1.51.28 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2741
- chore(deps): update golangci/golangci-lint-action action to v5 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2744
- Scan commit metadata by [@&#8203;rgmz](https://togithub.com/rgmz) in
[trufflesecurity/trufflehog#2713
- Fix SQL Server detector tests by
[@&#8203;rosecodym](https://togithub.com/rosecodym) in
[trufflesecurity/trufflehog#2716
- Revert "Scan commit metadata" by
[@&#8203;rosecodym](https://togithub.com/rosecodym) in
[trufflesecurity/trufflehog#2747
- \[bug] - Refactor newDiff constructor to avoid double initialization
of contentWriter by [@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2742
- \[chore] - update buffered file writer metric by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2740
- \[refactor] - lazy buffer retrieval by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2745
- \[chore] Remove broken test by
[@&#8203;mcastorina](https://togithub.com/mcastorina) in
[trufflesecurity/trufflehog#2748
- \[bug] - fix buffer size metric by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2749
- \[bug] - Fix the metric for buffered file writer writes by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2750
- fix(deps): update module github.com/aws/aws-sdk-go to v1.51.29 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2751
- update integration logos by
[@&#8203;dustin-decker](https://togithub.com/dustin-decker) in
[trufflesecurity/trufflehog#2752
- fix(deps): update module github.com/aws/aws-sdk-go to v1.51.30 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2756
- \[chore] - add additional binary extension by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2760
- pkg: fix function names in comment by
[@&#8203;mountcount](https://togithub.com/mountcount) in
[trufflesecurity/trufflehog#2761
- \[chore] - ignore pbix and vsdx files by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2762
- fix(deps): update module github.com/aws/aws-sdk-go to v1.51.31 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2763
- Scan commit metadata by [@&#8203;rgmz](https://togithub.com/rgmz) in
[trufflesecurity/trufflehog#2754
- \[bug] - Correctly set metrics for enumerated orgs by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2757
- \[chore ] -Update ignore extensions by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2764
- \[chore] Add some happy path logs to GitLab by
[@&#8203;mcastorina](https://togithub.com/mcastorina) in
[trufflesecurity/trufflehog#2765
- Fix Git source test by [@&#8203;rgmz](https://togithub.com/rgmz) in
[trufflesecurity/trufflehog#2767
- \[feat] - buffered file reader by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2731
- \[feat] - Add ReadFrom method to BufferedFileWriter by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2759
- fix(deps): update module google.golang.org/protobuf to v1.34.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2766
- \[bug] - Improve BufferedFileReader Close Behavior by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2768
- fixes calendly api key regex by
[@&#8203;ankushgoel27](https://togithub.com/ankushgoel27) in
[trufflesecurity/trufflehog#2368
- Expose detector-specific false positive logic by
[@&#8203;rosecodym](https://togithub.com/rosecodym) in
[trufflesecurity/trufflehog#2743
- Detector-Fix: Reintroduce Cloudflareglobalapikey by
[@&#8203;ankushgoel27](https://togithub.com/ankushgoel27) in
[trufflesecurity/trufflehog#2101
- Detector-Competition-Fix - fixed the alchemy detector regex by
[@&#8203;ankushgoel27](https://togithub.com/ankushgoel27) in
[trufflesecurity/trufflehog#1821
- fix(deps): update module github.com/aws/aws-sdk-go to v1.51.32 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2769
- fix(deps): update module google.golang.org/api to v0.177.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[trufflesecurity/trufflehog#2770
- \[chore] - update imports by
[@&#8203;ahrav](https://togithub.com/ahrav) in
[trufflesecurity/trufflehog#2772
- adds build version to finished scanning log by
[@&#8203;zricethezav](https://togithub.com/zricethezav) in
[trufflesecurity/trufflehog#2773
- Update rabbitmq.go regex detect amqps protocol by
[@&#8203;NikhilPanwar](https://togithub.com/NikhilPanwar) in
[trufflesecurity/trufflehog#2609
- fix for infinite recursion in Postman var sub by
[@&#8203;zricethezav](https://togithub.com/zricethezav) in
[trufflesecurity/trufflehog#2780

#### New Contributors

- [@&#8203;mountcount](https://togithub.com/mountcount) made their first
contribution in
[trufflesecurity/trufflehog#2761

**Full Changelog**:
trufflesecurity/trufflehog@v3.74.0...v3.75.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/matter-labs/vault-auth-tee).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMzEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjMzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
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.

None yet

3 participants