Move an existing secretscan exception into config.toml

ee/tables/secretscan/config.toml

@@ -5,11 +5,26 @@ title = "Kolide secretscan config"
 [extend]
 useDefault = true
 
-# Ignore K8s Sealed Secrets
-# https://github.com/gitleaks/gitleaks/issues/1728
 [[rules]]
 id = "generic-api-key"
 [[rules.allowlists]]
+description = "Ignore K8s Sealed Secrets (https://github.com/gitleaks/gitleaks/issues/1728)"
 condition = "AND"
 regexes = ['''Ag[a-zA-Z0-9+/]{500,}={0,2}''']
 paths = ['''(?i).*\.ya?ml$''']
+[[rules.allowlists]]
+description = "Ignore all-caps variable names with empty values (https://github.com/gitleaks/gitleaks/issues/1828)"
+condition = "AND"
+# We limit length to 35 as a proxy for not allowlisting high-entropy values here.
+# Variable names are probably usually under that length. We restrict to all-caps
+# for the same reason.
+regexes = ['''^\s*[A-Z\d][A-Z\d_-]{0,35}=$''']
+paths = ['''.*\.env(\..+)?$''']
+[[rules.allowlists]]
+description = "Ignore all-lowercase variable names with empty values (https://github.com/gitleaks/gitleaks/issues/1828)"
+condition = "AND"
+# We limit length to 35 as a proxy for not allowlisting high-entropy values here.
+# Variable names are probably usually under that length. We restrict to all-lowercase
+# for the same reason.
+regexes = ['''^\s*[a-z\d][a-z\d_-]{0,35}=$''']
+paths = ['''.*\.env(\..+)?$''']

ee/tables/secretscan/table.go

@@ -240,7 +240,6 @@ func (t *Table) findingsToRows(ctx context.Context, argon2idSalts []string, find
 
 	// Just for logging purposes -- we're curious how frequently we detect false positives
 	encryptedJwtFalsePositiveCount := 0
-	emptyVariableFalsePositiveCount := 0
 	for idx, f := range findings {
 		// We sometimes see false positives under the "generic-api-key" rule.
 		// Check for these.
@@ -249,10 +248,6 @@ func (t *Table) findingsToRows(ctx context.Context, argon2idSalts []string, find
 				encryptedJwtFalsePositiveCount += 1
 				continue
 			}
-			if isEmptyVariable(f) {
-				emptyVariableFalsePositiveCount += 1
-				continue
-			}
 		}
 
 		// Get the hash of this secret. If there's an error, log it, and allow the rest of the data to be returned.
@@ -287,11 +282,10 @@ func (t *Table) findingsToRows(ctx context.Context, argon2idSalts []string, find
 		results = append(results, row)
 	}
 
-	if encryptedJwtFalsePositiveCount > 0 || emptyVariableFalsePositiveCount > 0 {
+	if encryptedJwtFalsePositiveCount > 0 {
 		t.slogger.Log(ctx, slog.LevelInfo,
 			"detected and skipped false positive generic-api-key findings",
 			"jwt_family_count", encryptedJwtFalsePositiveCount,
-			"empty_variable", emptyVariableFalsePositiveCount,
 		)
 	}
 
@@ -340,46 +334,6 @@ func isEncryptedJWTFamilyValue(finding report.Finding) bool {
 	return false
 }
 
-// emptyVariableRegexp matches strings that start with a word char,
-// contain only word chars and underscores or hyphens, and end with a
-// singular equal sign -- for example, `MY_ENV_VAR=`.
-var emptyVariableRegexp = regexp.MustCompile(`^\w[\w-]*=$`)
-
-// isEmptyVariable inspects the given finding to determine if it is actually
-// an empty variable name instead.
-func isEmptyVariable(finding report.Finding) bool {
-	// This type of false positive typically has an entropy score around 3,
-	// so we exclude higher-entropy values right off the bat.
-	if finding.Entropy >= 4 {
-		return false
-	}
-
-	// Next, check for our regex match.
-	if !emptyVariableRegexp.MatchString(finding.Secret) {
-		return false
-	}
-
-	// We expect that this "secret" would be at the start of a line, with either nothing
-	// or whitespace in front of it. However, sometimes our finding.Line will contain
-	// multiple lines -- in this case, it looks like "\nMY_ENV_VAR1=\nMY_ENV_VAR2=".
-	// So first we isolate the actual line we're looking at, then check to see if there's
-	// anything besides whitespace in front of it.
-	lines := strings.Split(strings.ReplaceAll(finding.Line, "\r\n", "\n"), "\n")
-	var lineWithSecret string
-	for _, line := range lines {
-		if strings.Contains(line, finding.Secret) {
-			lineWithSecret = line
-			break
-		}
-	}
-	if lineWithSecret == "" {
-		return false
-	}
-	before, _, _ := strings.Cut(lineWithSecret, finding.Secret)
-	beforeTrimmed := strings.TrimSpace(before)
-	return beforeTrimmed == ""
-}
-
 // findingsToKeyNames attempts to extract the key names (eg: in an .env file) to help understand the context
 // of the discovered secret. Because of the multitude of possible ways people can stash secrets, and the myriad of
 // secret types, this is very hard to get right. So instead, we aim to solve the simple case, and ignore the rest.

ee/tables/secretscan/table_test.go

@@ -414,123 +414,104 @@ func Test_isEncryptedJWTFamilyValue(t *testing.T) {
 	}
 }
 
-func Test_isEmptyVariable(t *testing.T) {
+// Test_kolideConfig confirms that our overrides in config.toml work as expected
+func Test_kolideConfig(t *testing.T) {
 	t.Parallel()
 
 	// Make sure config exists
 	newConfigOnce()
 	require.NoError(t, configErr)
 
 	for _, tt := range []struct {
-		testCaseName   string
-		rawData        string
-		expectedReturn bool
+		testCaseName    string
+		pathName        string
+		rawData         string
+		expectedFinding bool
 	}{
 		{
-			testCaseName: "underscore",
+			testCaseName: "K8s sealed secrets",
+			pathName:     "k8s.yaml",
+			rawData: `
+apiVersion: bitnami.com/v1alpha1
+kind: SealedSecret
+metadata:
+  creationTimestamp: null
+  name: basic-auth
+  namespace: default
+spec:
+  encryptedData:
+    password: AgAYsM4W6e5MMp90oqj7sIolG+//xxJTwRh8ke50CUFiZ8M/r5BoV/vHh3zKnlskt/s4jK058M1i4Y3ETgfsrbiYD/3ADLKRHjr2TO5O6xUrWtfFh6aDR8fOSbVpm1qCSmxFGMaHq5QLJ3Ab0FQTKF/eFehogkzXb8opY/PlI9r+9DcUJ1epAjbNjzvHDDSEZxwi/i5kCNfxqnQNZHQ4uIgEysOk/kjBEtfISxyeD2PGYiyMYk9zMtEUk9LoUTR3/EXiFcJc4iw83DUVICbaCdMPh2ZRsv3A1CWx608rtIyF+qqKnmfX5me7njnYi0vGVDY97T4cV5rKrdZTTOZVggur2l+sPG+BJSYmEVqz3cZ81mVOr4znwU6w3f2e5HxD7ivdJJEz70xgPFX8pFruQulAOohd8qXakqdtA+ew+tr0h3M+cWvOu6VNXQlbijRgC4R89CclHW4/GX3j0OulJUotrV29rTpmVZ7MTGrnZkwJbNUYAe1GEFo7LNws+GJTcNM9R6QA/AYvJ2eE3SjE2G7VaIUh2RvSe2O094Ln7yIJctzEK8afiCiIQQnIgy/M+YheoC+TzogvLZGuNsvrt/oiiilUNa6WODTr1DmJGIyM4cg0pZuVKJ8dx+zf4l7+efkBOa/2mzU9DakvoRQK8/ClR6tuOAVXHZksPehcI3eTX/ZI0MtM2CLJouoi86hPbgwEorBt+nLClkw=
+    user: AgAMrlzJFHS+mqUhv2ZpG57VNoTSRrewuu6FZVPZcV35dCmdZwesz3MSrmweNHXAlJbVMSMRlINIEBQZKgjw0szEh2ZKkXjGv1926p34GJJSB5/rqYBIDUFkIRY3aJsijMN6etjLRQi68sbYIQAZQM4pGdN23++CfNmXoQgDm9ItspcSYAcOeKP8tZ799pQTdM1pMMur5EyrYWxckORCz+OT1+buCL9+5DJkjU7JuQCk5QkwXRE1sm76FvmmP+a3FbNCIgqsBpD42AqqD4/ex0PogKr7gDkG27MWZNIvCWDd2iF4x1cwJgOtNZJEzQ7tDr+Mf4438w03sJQPMtCEUuuzX1I7SPuT6D9eRSV00GA/IS0/fmNbPf2pPHFxj8t1RMsGI2ZLdZBOXapn3P0SLYZ2Xh6QIqdxzb3VR37WS/Ir4c8v86ZzDTbnqVdT/rwb7U4Iy2k3nDj+/ghxD+7HQbmBx4zzFVYe70Sb1QIWthzZHtuvoX7+FeSa6iU6ipUj4g0U9r53vD+AYt7ntJtCI3EdX+Fh9yJe4AAL4ToHjnt3s2EG4K5i0/21KwAy9WX2rkwBb8GD3POT3zZq2b4uB5gUYyF467kw0J7MfEsPJjAfhd72+IMM9BZDU4tlrFBJPRubVsmRJKelM/o1YTkbl+eFNyWBE1t5IQ9DFjHpcppgUm1CUiDdI0RbIc2goHfFWNePxZZQBg==
+  template:
+    metadata:
+      creationTimestamp: null
+      name: basic-auth
+      namespace: default
+`,
+			expectedFinding: false,
+		},
+		{
+			testCaseName: "empty variable, with underscore",
+			pathName:     ".env",
 			rawData: `
 123_S3_CREDS=
 123_S3_IP_REGION=
 `,
-			expectedReturn: true,
+			expectedFinding: false,
 		},
 		{
-			testCaseName: "hyphen",
+			testCaseName: "empty variable, with hyphen",
+			pathName:     ".env",
 			rawData: `
 123-S3-CREDS=
 123-S3-IP-REGION=
 `,
-			expectedReturn: true,
+			expectedFinding: false,
 		},
 		{
-			testCaseName: "alphanumeric",
+			testCaseName: "empty variable, with alphanumeric",
+			pathName:     ".env.local",
 			rawData: `
 123S3CREDS=
 123S3IPREGION=
 `,
-			expectedReturn: true,
+			expectedFinding: false,
 		},
 		{
-			testCaseName: "tab before empty variable",
+			testCaseName: "empty variable, with tab before empty variable",
+			pathName:     "aws.env",
 			rawData: `
 	123_S3_CREDS=
 	123_S3_IP_REGION=
 `,
-			expectedReturn: true,
+			expectedFinding: false,
 		},
 		{
-			testCaseName: "non-empty",
+			testCaseName: "empty variable, all lowercase",
+			pathName:     ".env",
 			rawData: `
-123_S3_CREDS=9b065cc5-cf2e-4b3f-9a20-3422e060807a
-123_S3_IP_REGION=52b22b1e-2178-4a1e-bbba-50d0160ffab3
+123_s3_creds=
+123_s3_ip_region=
 `,
-			expectedReturn: false,
+			expectedFinding: false,
 		},
 		{
-			testCaseName: "high entropy", // 4.19 entropy
+			testCaseName: "empty variable (true positive, variable is not empty)",
+			pathName:     ".env",
 			rawData: `
-375E6860-39D4-11F1-B4AC-0800200C9A66-375E6861-39D4-11F1-B4AC-0800200C9A66_123_S3_CREDS=
-4DE613D1-39D4-11F1-B4AC-0800200C9A66_123_S3_IP_REGION_4DE613D0-39D4-11F1-B4AC-0800200C9A66=
+123_S3_CREDS=9b065cc5-cf2e-4b3f-9a20-3422e060807a
+123_S3_IP_REGION=52b22b1e-2178-4a1e-bbba-50d0160ffab3
 `,
-			expectedReturn: false,
+			expectedFinding: true,
 		},
-	} {
-		t.Run(tt.testCaseName, func(t *testing.T) {
-			t.Parallel()
-
-			detector := detect.NewDetector(*kolideConfig)
-			fileSource := &sources.File{
-				Content: strings.NewReader(tt.rawData),
-				Config:  &detector.Config,
-			}
-
-			findings, err := detector.DetectSource(t.Context(), fileSource)
-			require.NoError(t, err)
-			require.Greater(t, len(findings), 0)
-
-			for _, finding := range findings {
-				// Make sure the test finding we generated is the type we expected
-				require.Equal(t, "generic-api-key", finding.RuleID)
-				// Confirm that isEmptyVariable classifies the finding appropriately
-				require.Equal(t, tt.expectedReturn, isEmptyVariable(finding))
-			}
-		})
-	}
-}
-
-// Test_kolideConfig confirms that our overrides in config.toml work as expected
-func Test_kolideConfig(t *testing.T) {
-	t.Parallel()
-
-	// Make sure config exists
-	newConfigOnce()
-	require.NoError(t, configErr)
-
-	for _, tt := range []struct {
-		testCaseName string
-		pathName     string
-		rawData      string
-	}{
 		{
-			testCaseName: "K8s sealed secrets",
-			pathName:     "k8s.yaml",
+			testCaseName: "empty variable (true positive, long variable with high entropy)",
+			pathName:     ".env",
 			rawData: `
-apiVersion: bitnami.com/v1alpha1
-kind: SealedSecret
-metadata:
-  creationTimestamp: null
-  name: basic-auth
-  namespace: default
-spec:
-  encryptedData:
-    password: AgAYsM4W6e5MMp90oqj7sIolG+//xxJTwRh8ke50CUFiZ8M/r5BoV/vHh3zKnlskt/s4jK058M1i4Y3ETgfsrbiYD/3ADLKRHjr2TO5O6xUrWtfFh6aDR8fOSbVpm1qCSmxFGMaHq5QLJ3Ab0FQTKF/eFehogkzXb8opY/PlI9r+9DcUJ1epAjbNjzvHDDSEZxwi/i5kCNfxqnQNZHQ4uIgEysOk/kjBEtfISxyeD2PGYiyMYk9zMtEUk9LoUTR3/EXiFcJc4iw83DUVICbaCdMPh2ZRsv3A1CWx608rtIyF+qqKnmfX5me7njnYi0vGVDY97T4cV5rKrdZTTOZVggur2l+sPG+BJSYmEVqz3cZ81mVOr4znwU6w3f2e5HxD7ivdJJEz70xgPFX8pFruQulAOohd8qXakqdtA+ew+tr0h3M+cWvOu6VNXQlbijRgC4R89CclHW4/GX3j0OulJUotrV29rTpmVZ7MTGrnZkwJbNUYAe1GEFo7LNws+GJTcNM9R6QA/AYvJ2eE3SjE2G7VaIUh2RvSe2O094Ln7yIJctzEK8afiCiIQQnIgy/M+YheoC+TzogvLZGuNsvrt/oiiilUNa6WODTr1DmJGIyM4cg0pZuVKJ8dx+zf4l7+efkBOa/2mzU9DakvoRQK8/ClR6tuOAVXHZksPehcI3eTX/ZI0MtM2CLJouoi86hPbgwEorBt+nLClkw=
-    user: AgAMrlzJFHS+mqUhv2ZpG57VNoTSRrewuu6FZVPZcV35dCmdZwesz3MSrmweNHXAlJbVMSMRlINIEBQZKgjw0szEh2ZKkXjGv1926p34GJJSB5/rqYBIDUFkIRY3aJsijMN6etjLRQi68sbYIQAZQM4pGdN23++CfNmXoQgDm9ItspcSYAcOeKP8tZ799pQTdM1pMMur5EyrYWxckORCz+OT1+buCL9+5DJkjU7JuQCk5QkwXRE1sm76FvmmP+a3FbNCIgqsBpD42AqqD4/ex0PogKr7gDkG27MWZNIvCWDd2iF4x1cwJgOtNZJEzQ7tDr+Mf4438w03sJQPMtCEUuuzX1I7SPuT6D9eRSV00GA/IS0/fmNbPf2pPHFxj8t1RMsGI2ZLdZBOXapn3P0SLYZ2Xh6QIqdxzb3VR37WS/Ir4c8v86ZzDTbnqVdT/rwb7U4Iy2k3nDj+/ghxD+7HQbmBx4zzFVYe70Sb1QIWthzZHtuvoX7+FeSa6iU6ipUj4g0U9r53vD+AYt7ntJtCI3EdX+Fh9yJe4AAL4ToHjnt3s2EG4K5i0/21KwAy9WX2rkwBb8GD3POT3zZq2b4uB5gUYyF467kw0J7MfEsPJjAfhd72+IMM9BZDU4tlrFBJPRubVsmRJKelM/o1YTkbl+eFNyWBE1t5IQ9DFjHpcppgUm1CUiDdI0RbIc2goHfFWNePxZZQBg==
-  template:
-    metadata:
-      creationTimestamp: null
-      name: basic-auth
-      namespace: default
+375E6860-39D4-11F1-B4AC-0800200C9A66-375E6861-39D4-11F1-B4AC-0800200C9A66_123_S3_CREDS=
+4DE613D1-39D4-11F1-B4AC-0800200C9A66_123_S3_IP_REGION_4DE613D0-39D4-11F1-B4AC-0800200C9A66=
 `,
+			expectedFinding: true,
 		},
 	} {
 		t.Run(tt.testCaseName, func(t *testing.T) {
@@ -545,7 +526,11 @@ spec:
 
 			findings, err := detector.DetectSource(t.Context(), fileSource)
 			require.NoError(t, err)
-			require.Equal(t, 0, len(findings))
+			if tt.expectedFinding {
+				require.Less(t, 0, len(findings))
+			} else {
+				require.Equal(t, 0, len(findings))
+			}
 		})
 	}
 }