Skip detectors for known bad chunks

[rgmz]

Description:

This is a POC to fix #1517. Skipping detectors on chunks that are known to be problematic (e.g., #1460) should improve performance by reducing the number of false-positives and extraneous network requests.

Any feedback and suggestions are welcome. (Also, I have yet to test whether this specific code works; emphasis on "concept".)

Checklist:

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

[rgmz]

Just tested this. Sadly, it does not work because GetData returns {"github": ...} — life is not so easy.

[rosecodym]

Do you mean that you need to explicitly attempt to unmarshal the metadata to determine whether it's from Git?

[rgmz]

I thought that GetData() returned a top-level object, however, it actually returns an object like {"bitbucket":{...}} | {"github":{...}} | {"gitlab":{...}}. Only calling GetX() returns the inner object.

As far as I can tell, the only solution is to construct an ugly switch statement to handle each SourceType, similar to:

trufflehog/pkg/engine/engine.go

Lines 605 to 609 in b03cc30

	func SupportsLineNumbers(sourceType sourcespb.SourceType) bool {
	switch sourceType {
	case sourcespb.SourceType_SOURCE_TYPE_GIT,
	sourcespb.SourceType_SOURCE_TYPE_GITHUB,
	sourcespb.SourceType_SOURCE_TYPE_GITLAB,

[rosecodym]

I like smuggling the interface to avoid changing the entire Detector signature, and I like the idea of using chunk metadata to help make the decision about whether a given detector should scan a given chunk - assuming we can find a more performant way to do the metadata type check than repeated attempts at unmarshaling.

As for the implied question of whether this is even worth doing: You've convinced me, but I'll leave space for others to weigh in if they have other opinions.

[rosecodym]

Do you mean that you need to explicitly attempt to unmarshal the metadata to determine whether it's from Git?

[rosecodym]

I really like this idea, assuming we can find a performant way to actually use it.

[rgmz]

What are the performance implications of this right now? (Assuming that it works, which it doesn't seem to...)

[rgmz]

Ah, you have to cast the struct to interface{} first.

md := data.GetGit()
if d, ok := interface{}(md).(GitSource); ok {
   d.GetFile() // do something
}

[CLAassistant]

All committers have signed the CLA.

[rgmz]

Using a struct is much less satisfying than an interface. >:(

Also, should these be pointers?

[rgmz]

@rosecodym I've updated the implementation based on feedback and testing. Let me know your thoughts on this.

[rosecodym]

@rgmz we just had an internal conversation about this and a question came up: How much of this problem is associated with lockfiles specifically? If the answer is "a lot," does it make sense to either use file exclusions or to consider skipping them at the engine level rather than adding complexity to detectors?

[rgmz]

I wouldn't say the problem is specific to lock files. There are lots of files that create a high volume of false detections for specific detectors, that doesn't mean the files should be skipped altogether — even lock files have a one-in-a-million chance to contain secrets.