Skip to content

Commit acc9d28

Browse files
committed
enhance sensitive field checks to include SecretStringSliceCSV
Signed-off-by: SungJin1212 <tjdwls1201@gmail.com>
1 parent e5eadb1 commit acc9d28

1 file changed

Lines changed: 31 additions & 17 deletions

File tree

pkg/cortex/modules_test.go

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -287,24 +287,28 @@ func TestConfigEndpoint_SecretsMasked(t *testing.T) {
287287
}
288288

289289
// TestConfig_SensitiveFieldTypes verifies that every struct field in Config
290-
// whose YAML tag name suggests a credential uses flagext.Secret, not string.
291-
// This catches new password/secret fields added as plain strings even if they
292-
// have no default value.
290+
// whose YAML tag name suggests a credential uses a masking type (flagext.Secret
291+
// or flagext.SecretStringSliceCSV), not a plain string or []string.
292+
// This catches new password/secret/key fields added as plain strings even if
293+
// they have no default value.
293294
func TestConfig_SensitiveFieldTypes(t *testing.T) {
294-
sensitivePattern := regexp.MustCompile(`(?i)^(password|secret|secret_key|application_credential_secret|basic_auth_password)$`)
295+
// Match exact known sensitive names, plus any tag ending with _key or _keys.
296+
sensitivePattern := regexp.MustCompile(`(?i)^(?:password|secret|secret_key|application_credential_secret|basic_auth_password)$|_keys?$`)
295297
secretType := reflect.TypeFor[flagext.Secret]()
298+
secretSliceType := reflect.TypeFor[flagext.SecretStringSliceCSV]()
296299

297300
var violations []string
298-
checkSensitiveFields(reflect.TypeFor[Config](), "", sensitivePattern, secretType, &violations)
301+
checkSensitiveFields(reflect.TypeFor[Config](), "", sensitivePattern, secretType, secretSliceType, &violations)
299302

300303
for _, v := range violations {
301-
t.Errorf("field should use flagext.Secret, not string: %s", v)
304+
t.Errorf("field should use flagext.Secret or flagext.SecretStringSliceCSV, not a plain string type: %s", v)
302305
}
303306
}
304307

305-
// checkSensitiveFields recursively walks a type and reports any string field
306-
// whose YAML tag matches the sensitive pattern.
307-
func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp, secretType reflect.Type, violations *[]string) {
308+
// checkSensitiveFields recursively walks a type and reports any string or
309+
// string-slice field whose YAML tag matches the sensitive pattern but does not
310+
// use an approved masking type (secretType or secretSliceType).
311+
func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp, secretType reflect.Type, secretSliceType reflect.Type, violations *[]string) {
308312
if t.Kind() == reflect.Pointer {
309313
t = t.Elem()
310314
}
@@ -317,22 +321,27 @@ func checkSensitiveFields(t reflect.Type, prefix string, pattern *regexp.Regexp,
317321
yamlTag := f.Tag.Get("yaml")
318322
yamlName, _, _ := strings.Cut(yamlTag, ",")
319323

320-
if pattern.MatchString(yamlName) && f.Type.Kind() == reflect.String {
321-
*violations = append(*violations, path+" (yaml:\""+yamlName+"\")")
324+
if pattern.MatchString(yamlName) && f.Type != secretType && f.Type != secretSliceType {
325+
isPlainString := f.Type.Kind() == reflect.String
326+
isStringSlice := f.Type.Kind() == reflect.Slice && f.Type.Elem().Kind() == reflect.String
327+
if isPlainString || isStringSlice {
328+
*violations = append(*violations, path+" (yaml:\""+yamlName+"\")")
329+
}
322330
}
323331

324332
ft := f.Type
325333
if ft.Kind() == reflect.Pointer {
326334
ft = ft.Elem()
327335
}
328-
if ft.Kind() == reflect.Struct && ft != secretType {
329-
checkSensitiveFields(ft, path+".", pattern, secretType, violations)
336+
if ft.Kind() == reflect.Struct && ft != secretType && ft != secretSliceType {
337+
checkSensitiveFields(ft, path+".", pattern, secretType, secretSliceType, violations)
330338
}
331339
}
332340
}
333341

334-
// setAllSecrets recursively walks a reflect.Value and sets every flagext.Secret
335-
// field's Value to the given sentinel string.
342+
// setAllSecrets recursively walks a reflect.Value and sets every
343+
// flagext.Secret and flagext.SecretStringSliceCSV field to the given sentinel
344+
// string so that TestConfigEndpoint_SecretsMasked can detect regressions.
336345
func setAllSecrets(v reflect.Value, sentinel string) {
337346
switch v.Kind() {
338347
case reflect.Pointer:
@@ -341,13 +350,18 @@ func setAllSecrets(v reflect.Value, sentinel string) {
341350
}
342351
case reflect.Struct:
343352
secretType := reflect.TypeFor[flagext.Secret]()
353+
secretSliceType := reflect.TypeFor[flagext.SecretStringSliceCSV]()
344354
for _, f := range v.Fields() {
345355
if !f.CanSet() {
346356
continue
347357
}
348-
if f.Type() == secretType {
358+
switch f.Type() {
359+
case secretType:
349360
f.Set(reflect.ValueOf(flagext.Secret{Value: sentinel}))
350-
} else {
361+
case secretSliceType:
362+
s := f.Addr().Interface().(*flagext.SecretStringSliceCSV)
363+
_ = s.Set(sentinel)
364+
default:
351365
setAllSecrets(f, sentinel)
352366
}
353367
}

0 commit comments

Comments
 (0)