starbind: fix use of ptr variables in starlark (#3386)

_fixtures/testvariables_pointers_not_loaded.go

@@ -0,0 +1,17 @@
+package main
+
+import "runtime"
+
+func main() {
+	type StructWithInterface struct {
+		Val any
+	}
+	var i int
+	var ba = StructWithInterface{}
+	ba.Val = &i
+
+	var ptrPtr **int
+	_ = ptrPtr
+
+	runtime.Breakpoint()
+}

pkg/terminal/starbind/conv.go

@@ -453,10 +453,15 @@ func (v ptrVariableAsStarlarkValue) Attr(name string) (starlark.Value, error) {
 }
 
 func (v ptrVariableAsStarlarkValue) AttrNames() []string {
+	if len(v.v.Children) == 0 {
+		// The pointer variable was not loaded; we don't know the field names.
+		return nil
+	}
 	if v.v.Children[0].Kind != reflect.Struct {
 		return nil
 	}
-	// autodereference
+	// autodereference: present the field names of the pointed-to struct as the
+	// fields of this pointer variable.
 	x := structVariableAsStarlarkValue{&v.v.Children[0], v.env}
 	return x.AttrNames()
 }

pkg/terminal/starbind/starlark.go

@@ -141,12 +141,13 @@ func (env *Env) printFunc() func(_ *starlark.Thread, msg string) {
 // Source can be either a []byte, a string or a io.Reader. If source is nil
 // Execute will execute the file specified by 'path'.
 // After the file is executed if a function named mainFnName exists it will be called, passing args to it.
-func (env *Env) Execute(path string, source interface{}, mainFnName string, args []interface{}) (starlark.Value, error) {
+func (env *Env) Execute(path string, source interface{}, mainFnName string, args []interface{}) (_ starlark.Value, _err error) {
 	defer func() {
 		err := recover()
 		if err == nil {
 			return
 		}
+		_err = fmt.Errorf("panic executing starlark script: %v", err)
 		fmt.Fprintf(env.out, "panic executing starlark script: %v\n", err)
 		for i := 0; ; i++ {
 			pc, file, line, ok := runtime.Caller(i)

pkg/terminal/starlark_test.go

@@ -158,3 +158,70 @@ func TestStarlarkVariable(t *testing.T) {
 		}
 	})
 }
+
+// Test that pointer variables that were not loaded don't lead to crashes when
+// used in Starlark scripts.
+func TestStarlarkVariablePointerNotLoaded(t *testing.T) {
+	withTestTerminal("testvariables_pointers_not_loaded", t, func(term *FakeTerminal) {
+		term.MustExec("continue")
+
+		// We're going to partially load some variables through the eval()
+		// builtin. Then we're going to attempt to evaluate expressions which
+		// try to access a field from a pointer variable that wasn't loaded
+		// (i.e. a ptrVariableAsStarlarkValue with no children). The tests will
+		// verify that we get an error. This is a regression test; we used to
+		// panic.
+		for _, tc := range []struct{ name, script, expErr string }{
+			{
+				// In this test, we'll load v. v will have a child (i.e.
+				// v.Children[0]), but because v is nil, v.Children[0] will not
+				// be loaded. We'll turn v.Children[0] into a
+				// ptrVariableAsStarlarkValue, and attempt to access a field on
+				// it.
+				//
+				// v                   -> structAsStarlarkValue<api.Variable<**int>>
+				// v.Children[0]       -> structAsStarlarkValue<api.Variable<*int>>
+				// v.Children[0].Value -> ptrVariableAsStarlarkValue<*int> ; this pointer variable has not been loaded
+				name: "partial load because of nil ptr",
+				script: `
+v = eval(
+		None, "ptrPtr", None
+	).Variable
+v.Children[0].Value.XXX
+`,
+				expErr: "*int has no .XXX field or method",
+			},
+			{
+				// In this test, MaxStructFields = 10 and MaxVariableRecurse = 0
+				// will cause us to load v, and v.Children[0] (in the sense that
+				// v.Children[0] has a child), but _not_ load
+				// v.Children[0].Children[0] (because the recursion limit is
+				// exhausted by the descent into the top-level struct, so we
+				// don't load what hides under the interface).
+				//
+				// v                               -> structAsStarlarkValue<api.Variable<StructWithInterface>>
+				// v.Children[0]                   -> structAsStarlarkValue<api.Variable<interface{}>>
+				// v.Children[0].Children[0]       -> structAsStarlarkValue<api.Variable<*int>>
+				// v.Children[0].Children[0].Value -> ptrVariableAsStarlarkValue<*int> ; this pointer variable has not been loaded
+				name: "partial load because of recursion limit",
+				script: `
+v = eval(
+		None, "ba", {"MaxVariableRecurse": 0, "MaxStructFields": 10}
+	).Variable;
+v.Children[0].Children[0].Value.XXX
+`,
+				expErr: "*int has no .XXX field or method",
+			},
+		} {
+			t.Run(tc.name, func(t *testing.T) {
+				_, err := term.ExecStarlark(tc.script)
+				if err == nil {
+					t.Fatalf("expected error %q, got success", tc.expErr)
+				}
+				if !strings.Contains(err.Error(), tc.expErr) {
+					t.Fatalf("expected error %q, got \"%s\"", tc.expErr, err)
+				}
+			})
+		}
+	})
+}