From 1a70c2d9f26face61e5bbdc2f922d95bc38df13a Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Thu, 4 Jun 2026 14:24:57 +0900 Subject: [PATCH 1/2] fix handling of trailing commas in SecretStringSliceCSV.Set() Signed-off-by: SungJin1212 --- CHANGELOG.md | 1 + pkg/util/flagext/secretstringslicecsv.go | 18 ++++++++++-- pkg/util/flagext/secretstringslicecsv_test.go | 28 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0756d48d25..f71e7fe568 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ * [BUGFIX] Ring: Fix ring token conflict resolution only applied to updated instance and make constantly token conflict check during instance observe period. * [BUGFIX] Distributor: Fix a panic (`slice bounds out of range`) in the stream push path when the context deadline expires while the worker goroutine is still marshalling a `WriteRequest`. #7541 * [BUGFIX] Query Frontend: Fix native histogram responses not being handled correctly in `minTime()` sort ordering for split_by_interval merge. #7555 +* [BUGFIX] Security: Fix `SecretStringSliceCSV.Set()` accepting empty entries from stray or trailing commas (e.g. `newkey,`). #7587 * [BUGFIX] Distributor: Release the push worker pool goroutines on shutdown by stopping the async executor during the stopping phase when `-distributor.num-push-workers` is set. #7602 * [BUGFIX] Querier: Fix unbounded resource leak in the bucket-scan blocks finder (used when the bucket index is disabled). Per-tenant metadata fetchers, their Prometheus registries, and on-disk meta caches are now evicted once a tenant is no longer active, instead of being retained for the lifetime of the process. #7573 diff --git a/pkg/util/flagext/secretstringslicecsv.go b/pkg/util/flagext/secretstringslicecsv.go index 0a1dea7834..75a5d0ff39 100644 --- a/pkg/util/flagext/secretstringslicecsv.go +++ b/pkg/util/flagext/secretstringslicecsv.go @@ -1,6 +1,9 @@ package flagext -import "strings" +import ( + "fmt" + "strings" +) // SecretStringSliceCSV is a slice of strings that is parsed from a comma-separated string. // It implements flag.Value and yaml Marshalers, but masks the value when marshaled to YAML @@ -15,12 +18,23 @@ func (v SecretStringSliceCSV) String() string { } // Set implements flag.Value +// Each comma-separated entry is trimmed of surrounding whitespace. +// Empty entries (after trimming) are rejected with an error. func (v *SecretStringSliceCSV) Set(s string) error { if s == "" { v.values = nil return nil } - v.values = strings.Split(s, ",") + parts := strings.Split(s, ",") + values := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p == "" { + return fmt.Errorf("invalid key list %q: empty entry after trimming", s) + } + values = append(values, p) + } + v.values = values return nil } diff --git a/pkg/util/flagext/secretstringslicecsv_test.go b/pkg/util/flagext/secretstringslicecsv_test.go index 58772e28c1..fad17c3f05 100644 --- a/pkg/util/flagext/secretstringslicecsv_test.go +++ b/pkg/util/flagext/secretstringslicecsv_test.go @@ -44,4 +44,32 @@ func TestSecretStringSliceCSV(t *testing.T) { require.NoError(t, s.Keys.Set("")) assert.Equal(t, []string(nil), s.Keys.Value()) }) + + t.Run("trailing comma is rejected", func(t *testing.T) { + var s TestStruct + err := s.Keys.Set("newkey,") + require.Error(t, err, "trailing comma must produce an error") + assert.Nil(t, s.Keys.Value(), "values must not be updated on error") + }) + + t.Run("leading comma is rejected", func(t *testing.T) { + var s TestStruct + require.Error(t, s.Keys.Set(",newkey")) + }) + + t.Run("double comma is rejected", func(t *testing.T) { + var s TestStruct + require.Error(t, s.Keys.Set("newkey,,oldkey")) + }) + + t.Run("whitespace-only entry is rejected", func(t *testing.T) { + var s TestStruct + require.Error(t, s.Keys.Set("newkey, ,oldkey")) + }) + + t.Run("surrounding whitespace is trimmed from valid entries", func(t *testing.T) { + var s TestStruct + require.NoError(t, s.Keys.Set(" key1 , key2 ")) + assert.Equal(t, []string{"key1", "key2"}, s.Keys.Value()) + }) } From 2fa8e9349b4ea1cd782255a1b26a605bc43c30d3 Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Fri, 5 Jun 2026 09:40:13 +0900 Subject: [PATCH 2/2] improve error handling Signed-off-by: SungJin1212 --- pkg/util/flagext/secretstringslicecsv.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/util/flagext/secretstringslicecsv.go b/pkg/util/flagext/secretstringslicecsv.go index 75a5d0ff39..e48fc070c4 100644 --- a/pkg/util/flagext/secretstringslicecsv.go +++ b/pkg/util/flagext/secretstringslicecsv.go @@ -1,7 +1,7 @@ package flagext import ( - "fmt" + "errors" "strings" ) @@ -30,7 +30,9 @@ func (v *SecretStringSliceCSV) Set(s string) error { for _, p := range parts { p = strings.TrimSpace(p) if p == "" { - return fmt.Errorf("invalid key list %q: empty entry after trimming", s) + // Do not include the original input value in the error message + // to avoid accidentally exposing secret values. + return errors.New("invalid value: empty entry after trimming") } values = append(values, p) }