diff --git a/client/setec/store.go b/client/setec/store.go index c7e5bee..beb28dd 100644 --- a/client/setec/store.go +++ b/client/setec/store.go @@ -14,6 +14,7 @@ import ( "log" "math/rand" "os" + "slices" "sync" "time" @@ -855,10 +856,18 @@ func (c StoreConfig) secretNames() ([]string, []*Fields, error) { return nil, nil, fmt.Errorf("parse struct fields: %w", err) } - // We don't need to deduplicate here, the constructor merges all the - // names into a map. sec = append(sec, fs.Secrets()...) svs = append(svs, fs) } + // Sort and compact (deduplicate) secret names. + // Although the constructor puts them into a map, duplicates can "poison" + // the initialization, so remove them up front. + slices.Sort(sec) + sec = slices.Compact(sec) + for _, name := range sec { + if name == "" { + return nil, nil, errors.New("empty secret name not allowed") + } + } return sec, svs, nil } diff --git a/client/setec/store_test.go b/client/setec/store_test.go index bc76a14..c75fed7 100644 --- a/client/setec/store_test.go +++ b/client/setec/store_test.go @@ -115,6 +115,32 @@ func TestStore(t *testing.T) { t.Errorf("Lookup(bravo): got %q, want error", s.Get()) } }) + + t.Run("NewStore_emptyName", func(t *testing.T) { + st, err := setec.NewStore(ctx, setec.StoreConfig{ + Client: cli, + Secrets: []string{""}, + Logf: logger.Discard, + AllowLookup: true, + }) + if err == nil { + t.Fatalf("NewStore(empty): got %+v, want error", st) + } + }) + + t.Run("NewStore_dupName", func(t *testing.T) { + // Regression: A duplicate name can poison the initialization map, so we + // need to deduplicate as part of collection. + st, err := setec.NewStore(ctx, setec.StoreConfig{ + Client: cli, + Secrets: []string{"alpha", "alpha"}, + Logf: logger.Discard, + }) + if err != nil { + t.Fatalf("NewStore(dup): unexpected error: %v", err) + } + t.Logf("NewStore: got secret %q", st.Secret("alpha").GetString()) + }) } func TestCachedStore(t *testing.T) {