feat(config): update rule validation (#1466)

config/config.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	_ "embed"
-	"fmt"
 	"regexp"
 	"sort"
 	"strings"
@@ -82,10 +81,6 @@ func (vc *ViperConfig) Translate() (Config, error) {
 	rulesMap := make(map[string]Rule)
 
 	for _, r := range vc.Rules {
-		if strings.TrimSpace(r.ID) == "" {
-			return Config{}, fmt.Errorf("rule ID is missing or empty, regex: %s", r.Regex)
-		}
-
 		var allowlistRegexes []*regexp.Regexp
 		for _, a := range r.Allowlist.Regexes {
 			allowlistRegexes = append(allowlistRegexes, regexp.MustCompile(a))
@@ -120,8 +115,8 @@ func (vc *ViperConfig) Translate() (Config, error) {
 			configPathRegex = regexp.MustCompile(r.Path)
 		}
 		r := Rule{
-			Description: r.Description,
 			RuleID:      r.ID,
+			Description: r.Description,
 			Regex:       configRegex,
 			Path:        configPathRegex,
 			SecretGroup: r.SecretGroup,
@@ -136,11 +131,11 @@ func (vc *ViperConfig) Translate() (Config, error) {
 				StopWords:   r.Allowlist.StopWords,
 			},
 		}
-		orderedRules = append(orderedRules, r.RuleID)
-
-		if r.Regex != nil && r.SecretGroup > r.Regex.NumSubexp() {
-			return Config{}, fmt.Errorf("%s: invalid regex secret group %d, max regex secret group %d", r.RuleID, r.SecretGroup, r.Regex.NumSubexp())
+		if err := r.Validate(); err != nil {
+			return Config{}, err
 		}
+
+		orderedRules = append(orderedRules, r.RuleID)
 		rulesMap[r.RuleID] = r
 	}
 	var allowlistRegexes []*regexp.Regexp

config/config_test.go

@@ -89,7 +89,12 @@ func TestTranslate(t *testing.T) {
 		{
 			cfgName:   "missing_id",
 			cfg:       Config{},
-			wantError: fmt.Errorf("rule ID is missing or empty, regex: (?i)(discord[a-z0-9_ .\\-,]{0,25})(=|>|:=|\\|\\|:|<=|=>|:).{0,5}['\\\"]([a-h0-9]{64})['\\\"]"),
+			wantError: fmt.Errorf("rule |id| is missing or empty, regex: (?i)(discord[a-z0-9_ .\\-,]{0,25})(=|>|:=|\\|\\|:|<=|=>|:).{0,5}['\\\"]([a-h0-9]{64})['\\\"]"),
+		},
+		{
+			cfgName:   "no_regex_or_path",
+			cfg:       Config{},
+			wantError: fmt.Errorf("discord-api-key: both |regex| and |path| are empty, this rule will have no effect"),
 		},
 		{
 			cfgName:   "bad_entropy_group",
@@ -127,18 +132,20 @@ func TestTranslate(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		viper.Reset()
-		viper.AddConfigPath(configPath)
-		viper.SetConfigName(tt.cfgName)
-		viper.SetConfigType("toml")
-		err := viper.ReadInConfig()
-		require.NoError(t, err)
+		t.Run(tt.cfgName, func(t *testing.T) {
+			viper.Reset()
+			viper.AddConfigPath(configPath)
+			viper.SetConfigName(tt.cfgName)
+			viper.SetConfigType("toml")
+			err := viper.ReadInConfig()
+			require.NoError(t, err)
 
-		var vc ViperConfig
-		err = viper.Unmarshal(&vc)
-		require.NoError(t, err)
-		cfg, err := vc.Translate()
-		assert.Equal(t, tt.wantError, err)
-		assert.Equal(t, cfg.Rules, tt.cfg.Rules)
+			var vc ViperConfig
+			err = viper.Unmarshal(&vc)
+			require.NoError(t, err)
+			cfg, err := vc.Translate()
+			assert.Equal(t, tt.wantError, err)
+			assert.Equal(t, cfg.Rules, tt.cfg.Rules)
+		})
 	}
 }

config/rule.go

@@ -1,17 +1,19 @@
 package config
 
 import (
+	"fmt"
 	"regexp"
+	"strings"
 )
 
 // Rules contain information that define details on how to detect secrets
 type Rule struct {
-	// Description is the description of the rule.
-	Description string
-
 	// RuleID is a unique identifier for this rule
 	RuleID string
 
+	// Description is the description of the rule.
+	Description string
+
 	// Entropy is a float representing the minimum shannon
 	// entropy a regex group must have to be considered a secret.
 	Entropy float64
@@ -41,3 +43,32 @@ type Rule struct {
 	// regexes, paths, and/or commits
 	Allowlist Allowlist
 }
+
+// Validate guards against common misconfigurations.
+func (r Rule) Validate() error {
+	// Ensure |id| is present.
+	if strings.TrimSpace(r.RuleID) == "" {
+		// Try to provide helpful context, since |id| is empty.
+		var context string
+		if r.Regex != nil {
+			context = ", regex: " + r.Regex.String()
+		} else if r.Path != nil {
+			context = ", path: " + r.Path.String()
+		} else if r.Description != "" {
+			context = ", description: " + r.Description
+		}
+		return fmt.Errorf("rule |id| is missing or empty" + context)
+	}
+
+	// Ensure the rule actually matches something.
+	if r.Regex == nil && r.Path == nil {
+		return fmt.Errorf("%s: both |regex| and |path| are empty, this rule will have no effect", r.RuleID)
+	}
+
+	// Ensure |secretGroup| works.
+	if r.Regex != nil && r.SecretGroup > r.Regex.NumSubexp() {
+		return fmt.Errorf("%s: invalid regex secret group %d, max regex secret group %d", r.RuleID, r.SecretGroup, r.Regex.NumSubexp())
+	}
+
+	return nil
+}

testdata/config/no_regex_or_path.toml

@@ -0,0 +1,5 @@
+title = "gitleaks config"
+
+[[rules]]
+id = 'discord-api-key'
+description = "Discord API key"