fix(config): validate rules when [extend] is used (#1592)

config/config.go

@@ -219,7 +219,8 @@ func (vc *ViperConfig) Translate() (Config, error) {
 		}
 	}
 
-	if maxExtendDepth != extendDepth {
+	currentExtendDepth := extendDepth
+	if maxExtendDepth != currentExtendDepth {
 		// disallow both usedefault and path from being set
 		if c.Extend.Path != "" && c.Extend.UseDefault {
 			return Config{}, errors.New("unable to load config due to extend.path and extend.useDefault being set")
@@ -236,7 +237,7 @@ func (vc *ViperConfig) Translate() (Config, error) {
 	}
 
 	// Validate the rules after everything has been assembled (including extended configs).
-	if extendDepth == 0 {
+	if currentExtendDepth == 0 {
 		for _, rule := range c.Rules {
 			if err := rule.Validate(); err != nil {
 				return Config{}, err

config/config_test.go

@@ -93,7 +93,7 @@ func TestTranslate(t *testing.T) {
 		{
 			cfgName:   "invalid/rule_missing_id",
 			cfg:       Config{},
-			wantError: errors.New("rule |id| is missing or empty, regex: (?i)(discord[a-z0-9_ .\\-,]{0,25})(=|>|:=|\\|\\|:|<=|=>|:).{0,5}['\\\"]([a-h0-9]{64})['\\\"]"),
+			wantError: errors.New("rule |id| is missing or empty, description: Discord API key, regex: (?i)(discord[a-z0-9_ .\\-,]{0,25})(=|>|:=|\\|\\|:|<=|=>|:).{0,5}['\\\"]([a-h0-9]{64})['\\\"]"),
 		},
 		{
 			cfgName:   "invalid/rule_no_regex_or_path",
@@ -449,7 +449,7 @@ func TestTranslateExtend(t *testing.T) {
 				Rules: map[string]Rule{"aws-access-key": {
 					RuleID:      "aws-access-key",
 					Description: "AWS Access Key",
-					Regex:       regexp.MustCompile("(?:a)(?:a)"),
+					Regex:       regexp.MustCompile("(a)(a)"),
 					SecretGroup: 2,
 					Keywords:    []string{},
 					Tags:        []string{"key", "AWS"},
@@ -563,6 +563,10 @@ func TestTranslateExtend(t *testing.T) {
 		},
 
 		// Invalid
+		{
+			cfgName:   "invalid/extend_invalid_ruleid",
+			wantError: errors.New("rule |id| is missing or empty"),
+		},
 	}
 
 	for _, tt := range tests {
@@ -589,8 +593,17 @@ func testTranslate(t *testing.T, test translateCase) {
 	err = viper.Unmarshal(&vc)
 	require.NoError(t, err)
 	cfg, err := vc.Translate()
-	if err != nil && !assert.EqualError(t, err, test.wantError.Error()) {
-		return
+	if err != nil {
+		if test.wantError != nil {
+			assert.EqualError(t, err, test.wantError.Error())
+		} else {
+			require.NoError(t, err)
+		}
+	} else {
+		if test.wantError != nil {
+			t.Fatalf("expected error but got none: %v", test.wantError)
+			return
+		}
 	}
 
 	if len(test.rules) > 0 {

config/rule.go

@@ -69,15 +69,17 @@ 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
+		var sb strings.Builder
+		if r.Description != "" {
+			sb.WriteString(", description: " + r.Description)
+		}
 		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
+			sb.WriteString(", regex: " + r.Regex.String())
+		}
+		if r.Path != nil {
+			sb.WriteString(", path: " + r.Path.String())
 		}
-		return errors.New("rule |id| is missing or empty" + context)
+		return errors.New("rule |id| is missing or empty" + sb.String())
 	}
 
 	// Ensure the rule actually matches something.

testdata/config/invalid/extend_invalid_ruleid.toml

@@ -0,0 +1,7 @@
+[extend]
+useDefault = true
+
+[[rules]]
+
+[[rules.allowlists]]
+paths = ['^.*\.(xml|log|json)$']

testdata/config/valid/extend_rule_override_secret_group.toml

@@ -5,5 +5,5 @@ path = "../testdata/config/simple.toml"
 
 [[rules]]
 id = "aws-access-key"
-regex = '''(?:a)(?:a)'''
+regex = '''(a)(a)'''
 secretGroup = 2