From acc9d284a0cd636e571124648e3e12be7c7b9820 Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Thu, 4 Jun 2026 12:51:02 +0900 Subject: [PATCH] enhance sensitive field checks to include SecretStringSliceCSV Signed-off-by: SungJin1212 --- pkg/cortex/modules_test.go | 48 ++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/pkg/cortex/modules_test.go b/pkg/cortex/modules_test.go index 865658e8b2..69ba06479b 100644 --- a/pkg/cortex/modules_test.go +++ b/pkg/cortex/modules_test.go @@ -287,24 +287,28 @@ func TestConfigEndpoint_SecretsMasked(t *testing.T) { } // TestConfig_SensitiveFieldTypes verifies that every struct field in Config -// whose YAML tag name suggests a credential uses flagext.Secret, not string. -// This catches new password/secret fields added as plain strings even if they -// have no default value. +// whose YAML tag name suggests a credential uses a masking type (flagext.Secret +// or flagext.SecretStringSliceCSV), not a plain string or []string. +// This catches new password/secret/key fields added as plain strings even if +// they have no default value. func TestConfig_SensitiveFieldTypes(t *testing.T) { - sensitivePattern := regexp.MustCompile(`(?i)^(password|secret|secret_key|application_credential_secret|basic_auth_password)$`) + // Match exact known sensitive names, plus any tag ending with _key or _keys. + sensitivePattern := regexp.MustCompile(`(?i)^(?:password|secret|secret_key|application_credential_secret|basic_auth_password)$|_keys?$`) secretType := reflect.TypeFor[flagext.Secret]() + secretSliceType := reflect.TypeFor[flagext.SecretStringSliceCSV]() var violations []string - checkSensitiveFields(reflect.TypeFor[Config](), "", sensitivePattern, secretType, &violations) + checkSensitiveFields(reflect.TypeFor[Config](), "", sensitivePattern, secretType, secretSliceType, &violations) for _, v := range violations { - t.Errorf("field should use flagext.Secret, not string: %s", v) + t.Errorf("field should use flagext.Secret or flagext.SecretStringSliceCSV, not a plain string type: %s", v) } } -// checkSensitiveFields recursively walks a type and reports any string field -// whose YAML tag matches the sensitive pattern. -func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp, secretType reflect.Type, violations *[]string) { +// checkSensitiveFields recursively walks a type and reports any string or +// string-slice field whose YAML tag matches the sensitive pattern but does not +// use an approved masking type (secretType or secretSliceType). +func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp, secretType reflect.Type, secretSliceType reflect.Type, violations *[]string) { if t.Kind() == reflect.Pointer { t = t.Elem() } @@ -317,22 +321,27 @@ func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp, yamlTag := f.Tag.Get("yaml") yamlName, _, _ := strings.Cut(yamlTag, ",") - if pattern.MatchString(yamlName) && f.Type.Kind() == reflect.String { - *violations = append(*violations, path+" (yaml:\""+yamlName+"\")") + if pattern.MatchString(yamlName) && f.Type != secretType && f.Type != secretSliceType { + isPlainString := f.Type.Kind() == reflect.String + isStringSlice := f.Type.Kind() == reflect.Slice && f.Type.Elem().Kind() == reflect.String + if isPlainString || isStringSlice { + *violations = append(*violations, path+" (yaml:\""+yamlName+"\")") + } } ft := f.Type if ft.Kind() == reflect.Pointer { ft = ft.Elem() } - if ft.Kind() == reflect.Struct && ft != secretType { - checkSensitiveFields(ft, path+".", pattern, secretType, violations) + if ft.Kind() == reflect.Struct && ft != secretType && ft != secretSliceType { + checkSensitiveFields(ft, path+".", pattern, secretType, secretSliceType, violations) } } } -// setAllSecrets recursively walks a reflect.Value and sets every flagext.Secret -// field's Value to the given sentinel string. +// setAllSecrets recursively walks a reflect.Value and sets every +// flagext.Secret and flagext.SecretStringSliceCSV field to the given sentinel +// string so that TestConfigEndpoint_SecretsMasked can detect regressions. func setAllSecrets(v reflect.Value, sentinel string) { switch v.Kind() { case reflect.Pointer: @@ -341,13 +350,18 @@ func setAllSecrets(v reflect.Value, sentinel string) { } case reflect.Struct: secretType := reflect.TypeFor[flagext.Secret]() + secretSliceType := reflect.TypeFor[flagext.SecretStringSliceCSV]() for _, f := range v.Fields() { if !f.CanSet() { continue } - if f.Type() == secretType { + switch f.Type() { + case secretType: f.Set(reflect.ValueOf(flagext.Secret{Value: sentinel})) - } else { + case secretSliceType: + s := f.Addr().Interface().(*flagext.SecretStringSliceCSV) + _ = s.Set(sentinel) + default: setAllSecrets(f, sentinel) } }