Polite Verification

[ahrav]

Description:

This PR changes the default behavior of verifying secrets to be more polite. If a single secret is detected by two or more different detectors in a single chunk, then verification of that secret is disabled.

You can override this behavior by using the --allow-verification-overlap option.

Checklist:

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

[dustin-decker]

Should we track and check these both separately? Could we potentially have a raw v2 in a different order, or a detector with a raw but not a rawv2 and it doesn't seem like a likely duplicate?

[rosecodym]

I have a few questions about the overlap detection logic itself

[rosecodym]

How does this work? If the producers are outpacing the consumer, then won't the consumer limit overall throughput irrespective of the buffer size?

[ahrav]

You raise a good point. If the producers are generating data faster than the consumers can process it, there is a risk of the consumers becoming a bottleneck, which could limit overall throughput. This would be true regardless of buffer size.

Simply increasing the buffer capacity provides only a temporary solution, as the buffer will eventually fill up if the consumers can't keep pace. A more robust approach may be to scale the number of consumer goroutines to match the production rate. This helps ensure the consumers have enough total capacity to handle all of the data being produced in parallel. This would likely require a bit more testing and fine-tuning so I took the faster approach in the interim.

[rgmz]

If a single secret is detected by two or more different detectors in a single chunk, then verification of that secret is disabled.

What are the potential implications of this? Would versioned detectors that match the same secret, such as GitLab , result in verification being disabled?

[dustin-decker]

If a single secret is detected by two or more different detectors in a single chunk, then verification of that secret is disabled.

What are the potential implications of this? Would versioned detectors that match the same secret, such as GitLab , result in verification being disabled?

Richard brings up a good point here. I think the answer is yes with the current code. Using the proto name of the detector instead within detector info might resolve that.

[ahrav]

If a single secret is detected by two or more different detectors in a single chunk, then verification of that secret is disabled.

What are the potential implications of this? Would versioned detectors that match the same secret, such as GitLab , result in verification being disabled?

Richard brings up a good point here. I think the answer is yes with the current code. Using the proto name of the detector instead within detector info might resolve that.

Well for the Gitlab detector this works correctly because the regexes are different enough that the likelyDuplicate will return false. If the regexes were similar, then yes we'd be marking them as unverified. I don't believe we currently have any versioned detectors that fall into this category, but this could be an issue with custom detectors with regexes that are very similar.

I think this test is an example.

[ahrav]

If a single secret is detected by two or more different detectors in a single chunk, then verification of that secret is disabled.

What are the potential implications of this? Would versioned detectors that match the same secret, such as GitLab , result in verification being disabled?

Richard brings up a good point here. I think the answer is yes with the current code. Using the proto name of the detector instead within detector info might resolve that.

Well for the Gitlab detector this works correctly because the regexes are different enough that the likelyDuplicate will return false. If the regexes were similar, then yes we'd be marking them as unverified. I don't believe we currently have any versioned detectors that fall into this category, but this could be an issue with custom detectors with regexes that are very similar.

I think this test is an example.

Added some logic to handle the edge case, and included a test

[rgmz]

BTW this breaks make test-community since the secrets aren't accessible.

--- FAIL: TestEngine_VersionedDetectorsVerifiedSecrets (0.01s)
    engine_test.go:231:
                Error Trace:    /tmp/trufflehog/pkg/engine/engine_test.go:231
                Error:          Received unexpected error:
                                failed to create secretmanager client: google: could not find default credentials. See https://cloud.google.com/docs/authentication/external/set-up-adc for more information
                                github.com/trufflesecurity/trufflehog/v3/pkg/common.GetSecret
                                        /tmp/trufflehog/pkg/common/secrets.go:55