Fix broken & inaccurate detector unit tests

[rgmz]

The detector unit tests were created by "reverse engineering" the patterns, thus they do not provide any real value or confirm how accurate the detectors are.

Additionally, the structure of the tests are (in my opinion) not maintainable as they are difficult to understand and do not reflect any realistic scenarios.

trufflehog/pkg/detectors/netsuite/netsuite_test.go

Lines 14 to 44 in def734a

	var (
	validConsumerKey = "3WaMEd0KQtHSU7b24HEd79RZzSpMOfMdMUpIaXjq83DbNHVosCVrEVDxKiEQzT15"
	invalidConsumerKey = "3Wa?Ed0KQtHSU7b24HEd79RZzSpMOfMdMUpIaXjq83DbNHVosCVrEVDxKiEQzT15"
	validConsumerSecret = "5BZ70LfNshsJkDya1XaD8bMqtPWlOa2o1yKCk0H2DxnjtoaJKIcAw75GdI6zRaRD"
	invalidConsumerSecret = "5BZ70LfNshsJkDya?XaD8bMqtPWlOa2o1yKCk0H2DxnjtoaJKIcAw75GdI6zRaRD"
	validTokenKey = "KeYcG56ViFDleXPFJuEQ5CAGSJn7o2WDa5iGvLIvVBqZj5rMkaWFmzkp4bveJa74"
	invalidTokenKey = "KeYcG56ViFDleXPFJuEQ5CAGSJn7o2WD?5iGvLIvVBqZj5rMkaWFmzkp4bveJa74"
	validTokenSecret = "GGQUdyYOGDfDImJWCz4Kufk2GevaIDuVv83kIa9zCRuXIDLB4oh2eVDVPmsaSai2"
	invalidTokenSecret = "GGQUdyYOGDfDImJWCz4Kufk2Ge?aIDuVv83kIa9zCRuXIDLB4oh2eVDVPmsaSai2"
	validAccountID = "x1L2_BXo"
	invalidAccountID = "x1L2?BXo"
	keyword = "netsuite"
	inputFormat = `%s id - '%s'
	consumer - '%s' consumer - '%s'
	token - '%s' token - '%s'`
	outputPair1 = validConsumerKey + validConsumerSecret
	outputPair2 = validConsumerSecret + validConsumerKey
	)
	func TestNetsuite_Pattern(t *testing.T) {
	d := Scanner{}
	ahoCorasickCore := ahocorasick.NewAhoCorasickCore([]detectors.Detector{d})
	tests := []struct {
	name string
	input string
	want []string
	}{
	{
	name: "valid pattern - with keyword netsuite",
	input: fmt.Sprintf(inputFormat, keyword, validAccountID, validConsumerKey, validConsumerSecret, validTokenKey, validTokenSecret),
	want: []string{outputPair1, outputPair2, outputPair1, outputPair2},

Examples

There are dozens, if not hundreds, of problematic test files. These are illustrative.

BombBomb

The "valid" tests for BombBomb do not match the detector's pattern.

trufflehog/pkg/detectors/bombbomb/bombbomb.go

Line 24 in def734a

keyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"bombbomb"}) + common.BuildRegexJWT("0,140", "0,419", "0,171"))

trufflehog/pkg/detectors/bombbomb/bombbomb_test.go

Line 15 in def734a

validPattern = "HUmGL.17uQMEShYp2RVMR8vypd1iqj6FZcKkQ4SazuMkbEKhzRFKuvOiwYmNWPSvkE4wiLOv-zWTkK1WkVTScRb9_io0_kvhYX31tpwR3lAJUh27RJzf1BehaJTQDXhJB6aT2gQ2LMT7dda-b3vhmEuZHzPV9AMLV6cOrcqOTkK60vMcB0PTLRQ3c_kY.a.9.hRvgogdlI8mQJrzD0myPBY7lMpjpkcskQDpOgz2I37kNDYhf7IxT6sG-a7rI1LdpJ6HhJacktlNJSswST9jbt4A0ropfJJTHGny2aId4WyPpAnQubM98F1BUnyhfkDzenaUuuQ_ZoPn9mAOsdLQUlAyp4I9oLJ_v8yQ0Q4M.Yujscho9G4ZbVTInC2mP8taCPZdRK5qt-UfAF0CX9B4E0F9NItMUbRdbm3xIkl8C6iPUcgY5OTQDBSJRLKBJgIaEyyXe10pPw.qOUhLKNPcg5qPs1xhgBsZKfW2hNTff2dCL5h6E.940ojPuT0Iw90Q8kpQ2UzeUJrhXH9_GUANKA.pjD0-YcGpnlVEDouyXaXowUoh8pLqD-BtBQfteqyFqz7THGDvQKikMy7wiBuJAo0HttMG3jw1zKtA3gM6_VIXo_K4WN6yz8Ow4n5f6Unn5zn4j2haKA4WWI5-1c8-mm7SF5VqYJVz42wBmRqB6MWXegJ7yLt_EoG1tJHftnHZ"

Kraken

The "valid" pattern is nonsensical and not correct base64 encoding. The detector should not match this, that is a defect.

trufflehog/pkg/detectors/kraken/kraken_test.go

Line 16 in def734a

validKeyPattern = "m=MN/0yYJ/5xqpE15JYDJtCFdDF7RDLuiXtTiSF1FU1H9waiub1kgwI= "

https://support.kraken.com/hc/en-us/articles/360000919966-How-to-create-an-API-key

viewneo

A few hundred detectors contain tests tightly coupled to the current implementation of PrefixRegex. Any changes to the prefix pattern will break the detector tests, which seems inadvisable.

trufflehog/pkg/detectors/viewneo/viewneo_test.go

Lines 38 to 42 in def734a

	{
	name: "valid pattern - key out of prefix range",
	input: fmt.Sprintf("%s keyword is not close to the real key in the data\n = '%s'", keyword, validPattern),
	want: []string{},
	},

[kashifkhan0771]

The bombbomb pattern test case was functioning correctly when I wrote it. However, the failure was introduced by this pull request, which modified the pattern without updating the corresponding test cases. Moving forward, we should ensure that any changes to a pattern are accompanied by updates to the relevant test cases.

Additionally, I’d appreciate your input on some test cases I wrote to simulate real-world scenarios for various detectors. For example, take a look at the bombbomb detector complex pattern test. If this approach looks good, we can work on updating the remaining test cases to follow a similar strategy.

[nabeelalam]

If this approach looks good, we can work on updating the remaining test cases to follow a similar strategy.

Agreed, and this may also involve updating detectors where the existing regex could potentially match incorrect patterns.

[rgmz]

The bombbomb pattern test case was functioning correctly when I wrote it. However, the failure was introduced by this pull request, which modified the pattern without updating the corresponding test cases.

Small mistakes like that should be detected once #3773 is merged.

Additionally, I’d appreciate your input on some test cases I wrote to simulate real-world scenarios for various detectors. For example, take a look at the bombbomb detector complex pattern test.

That's a better approach, although you only really need 1/2 lines of surrounding context. I'd also suggest moving the inputs inline with the test cases (input: `...`) instead of sprintf'ing them.

	complexPattern = `
		
		bombbombToken := "HUmGL.17uQMEShYp2RVMR8vypd1iqj6FZcKkQ4SazuMkbEKhzRFKuvOiwYmNWPSvkE4wiLOv-zWTkK1WkVTScRb9_io0_kvhYX31tpwR3lAJUh27RJzf1BehaJTQDXhJB6aT2gQ2LMT7dda-b3vhmEuZHzPV9AMLV6cOrcqOTkK60vMcB0PTLRQ3c_kY.a.9.hRvgogdlI8mQJrzD0myPBY7lMpjpkcskQDpOgz2I37kNDYhf7IxT6sG-a7rI1LdpJ6HhJacktlNJSswST9jbt4A0ropfJJTHGny2aId4WyPpAnQubM98F1BUnyhfkDzenaUuuQ_ZoPn9mAOsdLQUlAyp4I9oLJ_v8yQ0Q4M.Yujscho9G4ZbVTInC2mP8taCPZdRK5qt-UfAF0CX9B4E0F9NItMUbRdbm3xIkl8C6iPUcgY5OTQDBSJRLKBJgIaEyyXe10pPw.qOUhLKNPcg5qPs1xhgBsZKfW2hNTff2dCL5h6E.940ojPuT0Iw90Q8kpQ2UzeUJrhXH9_GUANKA.pjD0-YcGpnlVEDouyXaXowUoh8pLqD-BtBQfteqyFqz7THGDvQKikMy7wiBuJAo0HttMG3jw1zKtA3gM6_VIXo_K4WN6yz8Ow4n5f6Unn5zn4j2haKA4WWI5-1c8-mm7SF5VqYJVz42wBmRqB6MWXegJ7yLt_EoG1tJHftnHZ"
		req.Header.Set("Authorization", bombbombToken)
	`

I tend to base test cases on results found from GitHub or SourceGraph search.
e.g., https://github.com/trufflesecurity/trufflehog/pull/3784/files#diff-8d1772a0a428fddbb34136f1096bc2461293e55d4faffcfe9a100070e11411e1

[kashifkhan0771]

I can fix the bombbomb pattern test case as of now

[kashifkhan0771]

This Pull request improve pattern test cases for A&B letter detectors.

[patidartanay]

@kashifkhan0771 Is the issue still open? Do we have the list of test cases which needs to be fixed?

[kashifkhan0771]

Hi @patidartanay yes, this is still open. While we don’t have an exact list, you can identify test cases that don’t align with the guidelines mentioned in the issue by reviewing the existing ones. For reference, take a look at this PR of mine that demonstrates how to improve and fix test cases.

You're welcome to submit updates for any number of detectors, but please keep the number of changes in a single PR small to make the review process easier.