fix(config): extend allowlist & handle extend when validating (#1524)

config/config.go

@@ -131,9 +131,6 @@ func (vc *ViperConfig) Translate() (Config, error) {
 				StopWords:   r.Allowlist.StopWords,
 			},
 		}
-		if err := r.Validate(); err != nil {
-			return Config{}, err
-		}
 
 		orderedRules = append(orderedRules, r.RuleID)
 		rulesMap[r.RuleID] = r
@@ -171,7 +168,15 @@ func (vc *ViperConfig) Translate() (Config, error) {
 		} else if c.Extend.Path != "" {
 			c.extendPath()
 		}
+	}
 
+	// Validate the rules after everything has been assembled (including extended configs).
+	if extendDepth == 0 {
+		for _, rule := range rulesMap {
+			if err := rule.Validate(); err != nil {
+				return Config{}, err
+			}
+		}
 	}
 
 	return c, nil
@@ -235,12 +240,22 @@ func (c *Config) extendURL() {
 }
 
 func (c *Config) extend(extensionConfig Config) {
-	for ruleID, rule := range extensionConfig.Rules {
-		if _, ok := c.Rules[ruleID]; !ok {
-			log.Trace().Msgf("adding %s to base config", ruleID)
-			c.Rules[ruleID] = rule
-			c.Keywords = append(c.Keywords, rule.Keywords...)
+	for ruleID, baseRule := range extensionConfig.Rules {
+		currentRule, ok := c.Rules[ruleID]
+		if !ok {
+			// Rule doesn't exist, add it to the config.
+			c.Rules[ruleID] = baseRule
+			c.Keywords = append(c.Keywords, baseRule.Keywords...)
 			c.OrderedRules = append(c.OrderedRules, ruleID)
+		} else {
+			// Rule exists, merge our changes into the base.
+			baseRule.Allowlist.Commits = append(baseRule.Allowlist.Commits, currentRule.Allowlist.Commits...)
+			baseRule.Allowlist.Paths = append(baseRule.Allowlist.Paths, currentRule.Allowlist.Paths...)
+			baseRule.Allowlist.Regexes = append(baseRule.Allowlist.Regexes, currentRule.Allowlist.Regexes...)
+			baseRule.Allowlist.StopWords = append(baseRule.Allowlist.StopWords, currentRule.Allowlist.StopWords...)
+
+			delete(c.Rules, ruleID)
+			c.Rules[ruleID] = baseRule
 		}
 	}
 

config/config_test.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"fmt"
+	"github.com/google/go-cmp/cmp"
 	"regexp"
 	"testing"
 
@@ -91,11 +92,11 @@ func TestTranslate(t *testing.T) {
 			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})['\\\"]"),
 		},
-		//{
-		//	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:   "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",
 			cfg:       Config{},
@@ -129,11 +130,58 @@ func TestTranslate(t *testing.T) {
 				},
 			},
 		},
+		{
+			cfgName: "extend_rule_allowlist",
+			cfg: Config{
+				Rules: map[string]Rule{
+					"aws-secret-key-again-again": {
+						RuleID:      "aws-secret-key-again-again",
+						Description: "AWS Secret Key",
+						Regex:       regexp.MustCompile(`(?i)aws_(.{0,20})?=?.[\'\"0-9a-zA-Z\/+]{40}`),
+						Tags:        []string{"key", "AWS"},
+						Keywords:    []string{},
+						Allowlist: Allowlist{
+							Commits: []string{"abcdefg1"},
+							Regexes: []*regexp.Regexp{
+								regexp.MustCompile(`foo.+bar`),
+							},
+							Paths: []*regexp.Regexp{
+								regexp.MustCompile(`ignore\.xaml`),
+							},
+							StopWords: []string{"example"},
+						},
+					},
+				},
+			},
+		},
+		{
+			cfgName: "extend_empty_regexpath",
+			cfg: Config{
+				Rules: map[string]Rule{
+					"aws-secret-key-again-again": {
+						RuleID:      "aws-secret-key-again-again",
+						Description: "AWS Secret Key",
+						Regex:       regexp.MustCompile(`(?i)aws_(.{0,20})?=?.[\'\"0-9a-zA-Z\/+]{40}`),
+						Tags:        []string{"key", "AWS"},
+						Keywords:    []string{},
+						Allowlist: Allowlist{
+							Paths: []*regexp.Regexp{
+								regexp.MustCompile(`something.py`),
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.cfgName, func(t *testing.T) {
-			viper.Reset()
+			t.Cleanup(func() {
+				extendDepth = 0
+				viper.Reset()
+			})
+
 			viper.AddConfigPath(configPath)
 			viper.SetConfigName(tt.cfgName)
 			viper.SetConfigType("toml")
@@ -144,8 +192,21 @@ func TestTranslate(t *testing.T) {
 			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)
+			if !assert.Equal(t, tt.wantError, err) {
+				return
+			}
+
+			var regexComparer = func(x, y *regexp.Regexp) bool {
+				// Compare the string representation of the regex patterns.
+				if x == nil || y == nil {
+					return x == y
+				}
+				return x.String() == y.String()
+			}
+			opts := cmp.Options{cmp.Comparer(regexComparer)}
+			if diff := cmp.Diff(tt.cfg.Rules, cfg.Rules, opts); diff != "" {
+				t.Errorf("%s diff: (-want +got)\n%s", tt.cfgName, diff)
+			}
 		})
 	}
 }

config/rule.go

@@ -60,12 +60,10 @@ func (r Rule) Validate() error {
 		return fmt.Errorf("rule |id| is missing or empty" + context)
 	}
 
-	// TODO: uncomment this once it works with |extend|.
-	// See: https://github.com/gitleaks/gitleaks/issues/1507#issuecomment-2352559213
 	// 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)
-	//}
+	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() {

go.mod

@@ -16,6 +16,7 @@ require (
 
 require (
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
+	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
 	github.com/mattn/go-isatty v0.0.17 // indirect
 	github.com/mattn/go-runewidth v0.0.14 // indirect

go.sum

@@ -127,6 +127,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=

testdata/config/extend_3.toml

@@ -3,7 +3,7 @@ title = "gitleaks extended 3"
 ## This should not be loaded since we can only extend configs to a depth of 3
 
 [[rules]]
-    description = "AWS Secret Key"
     id = "aws-secret-key-again-again"
+    description = "AWS Secret Key"
     regex = '''(?i)aws_(.{0,20})?=?.[\'\"0-9a-zA-Z\/+]{40}'''
     tags = ["key", "AWS"]

testdata/config/extend_empty_regexpath.toml

@@ -0,0 +1,8 @@
+[extend]
+path="../testdata/config/extend_3.toml"
+
+[[rules]]
+id = "aws-secret-key-again-again"
+[rules.allowlist]
+description = "False positive. Keys used for colors match the rule, and should be excluded."
+paths = ['''something.py''']

testdata/config/extend_rule_allowlist.toml

@@ -0,0 +1,12 @@
+title = "gitleaks extended 3"
+
+[extend]
+path="../testdata/config/extend_3.toml"
+
+[[rules]]
+    id = "aws-secret-key-again-again"
+[rules.allowlist]
+    commits = ['''abcdefg1''']
+    regexes = ['''foo.+bar''']
+    paths = ['''ignore\.xaml''']
+    stopwords = ['''example''']