From 8294cc82f6f4340340ec5b9373488cb1c799c80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20R=C3=BCger?= Date: Mon, 10 Aug 2026 17:49:07 +0200 Subject: [PATCH] Fix nil pointer dereference for interface-typed flag values reflect.TypeOf returns nil when passed a nil interface value. FlagBase is generic over the value type T, and for GenericFlag that type is the Value interface, so reflect.TypeOf(f.Value) and reflect.TypeOf(zero) both yield nil there. Two call sites dereferenced the result without checking, panicking on an otherwise valid flag: * SchemaItemsType() panics for any GenericFlag, since it always inspects reflect.TypeOf(zero).Kind(). * PostParse() panics for a GenericFlag whose value source resolves to an empty string. The `val != ""` short circuit hid this whenever the source produced a non-empty value, so it only reproduces with an empty environment variable or file. TypeName() and IsMultiValueFlag() already guard against a nil type; do the same in both of these. PostParse() now derives the kind once and reuses it, which also drops a duplicate reflect.TypeOf call. Co-Authored-By: Claude Opus 5 (1M context) --- flag_impl.go | 16 +++++++++++++--- flag_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/flag_impl.go b/flag_impl.go index a67208c91c..be702d33f4 100644 --- a/flag_impl.go +++ b/flag_impl.go @@ -131,14 +131,22 @@ func (f *FlagBase[T, C, V]) PostParse() error { if !f.hasBeenSet { if val, source, found := f.Sources.LookupWithSource(); found { - if val != "" || reflect.TypeOf(f.Value).Kind() == reflect.String { + // reflect.TypeOf yields nil when T is an interface type (e.g. + // GenericFlag) and the value is nil, so the kind has to be + // derived defensively. + kind := reflect.Invalid + if ty := reflect.TypeOf(f.Value); ty != nil { + kind = ty.Kind() + } + + if val != "" || kind == reflect.String { if err := f.Set(f.Name, val); err != nil { return fmt.Errorf( "could not parse %[1]q as %[2]T value from %[3]s for flag %[4]s: %[5]s", val, f.Value, source, f.Name, err, ) } - } else if val == "" && reflect.TypeOf(f.Value).Kind() == reflect.Bool { + } else if val == "" && kind == reflect.Bool { _ = f.Set(f.Name, "false") } @@ -316,8 +324,10 @@ func (f *FlagBase[T, C, V]) SchemaType() string { // SchemaItemsType returns the JSON Schema element type for slice flags. func (f *FlagBase[T, C, V]) SchemaItemsType() string { var zero T + // reflect.TypeOf yields nil when T is an interface type (e.g. GenericFlag), + // in which case there are no slice elements to describe. t := reflect.TypeOf(zero) - if t.Kind() == reflect.Slice { + if t != nil && t.Kind() == reflect.Slice { switch t.Elem().Kind() { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: diff --git a/flag_test.go b/flag_test.go index e13d1b3147..44897bd208 100644 --- a/flag_test.go +++ b/flag_test.go @@ -3518,3 +3518,30 @@ func TestEndValue(t *testing.T) { assert.Error(t, cmd.Run(buildTestContext(t), []string{"foo", "-cd="})) assert.Error(t, cmd.Run(buildTestContext(t), []string{"foo", "-cd=s"})) } + +// TestFlagBaseInterfaceValueType covers flags whose value type is an interface +// (GenericFlag), for which reflect.TypeOf on the zero value yields nil. +func TestFlagBaseInterfaceValueType(t *testing.T) { + t.Run("SchemaItemsType", func(t *testing.T) { + fl := &GenericFlag{Name: "gen"} + assert.Empty(t, fl.SchemaItemsType()) + }) + + t.Run("PostParse with empty source value", func(t *testing.T) { + t.Setenv("TEST_GENERIC_EMPTY", "") + + fl := &GenericFlag{Name: "gen", Sources: EnvVars("TEST_GENERIC_EMPTY")} + require.NoError(t, fl.PreParse()) + require.NoError(t, fl.PostParse()) + }) + + t.Run("PostParse with non-empty source value", func(t *testing.T) { + t.Setenv("TEST_GENERIC_SET", "abc,def") + + val := &Parser{} + fl := &GenericFlag{Name: "gen", Value: val, Sources: EnvVars("TEST_GENERIC_SET")} + require.NoError(t, fl.PreParse()) + require.NoError(t, fl.PostParse()) + assert.True(t, fl.IsSet()) + }) +}