From e161f5a8b27822c2013c016d1c1b6b6f99e637e7 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 24 Aug 2026 13:19:00 -0700 Subject: [PATCH 1/3] fix(binding): reject oversized final-value containers Compare source length to remainingNodes before destination allocation so oversized tuples, lists, and dicts fail with ErrValueLimit instead of preallocating from untrusted length. --- internal/binding/output.go | 42 ++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/internal/binding/output.go b/internal/binding/output.go index 6f95b42..1c0a83f 100644 --- a/internal/binding/output.go +++ b/internal/binding/output.go @@ -160,9 +160,21 @@ func convertFloat(value starlark.Float) (any, error) { return float, nil } +// containerLength returns length when that many child nodes fit the remaining budget. +func (converter *finalConverter) containerLength(length int) (int, error) { + if length > converter.remainingNodes { + return 0, fmt.Errorf("%w: value exceeds byte-derived node budget", ErrValueLimit) + } + return length, nil +} + // convertTuple recursively converts an immutable sequence. func (converter *finalConverter) convertTuple(value starlark.Tuple, depth int) ([]any, error) { - items := make([]any, len(value)) + length, err := converter.containerLength(len(value)) + if err != nil { + return nil, err + } + items := make([]any, length) for index, item := range value { converted, err := converter.convert(item, depth+1) if err != nil { @@ -181,7 +193,11 @@ func (converter *finalConverter) convertList(value *starlark.List, depth int) ([ } defer converter.leave(key) - items := make([]any, 0, value.Len()) + length, err := converter.containerLength(value.Len()) + if err != nil { + return nil, err + } + items := make([]any, 0, length) iterator := value.Iterate() defer iterator.Done() var item starlark.Value @@ -203,13 +219,27 @@ func (converter *finalConverter) convertDict(value *starlark.Dict, depth int) (m } defer converter.leave(key) - object := make(map[string]any, value.Len()) - for _, item := range value.Items() { - name, ok := starlark.AsString(item[0]) + length, err := converter.containerLength(value.Len()) + if err != nil { + return nil, err + } + object := make(map[string]any, length) + iterator := value.Iterate() + defer iterator.Done() + var dictKey starlark.Value + for iterator.Next(&dictKey) { + name, ok := starlark.AsString(dictKey) if !ok { return nil, fmt.Errorf("%w: dictionary key must be a string", ErrUnsupportedValue) } - converted, err := converter.convert(item[1], depth+1) + item, found, err := value.Get(dictKey) + if err != nil { + return nil, fmt.Errorf("%w: dictionary lookup: %w", ErrUnsupportedValue, err) + } + if !found { + return nil, fmt.Errorf("%w: missing dictionary value", ErrUnsupportedValue) + } + converted, err := converter.convert(item, depth+1) if err != nil { return nil, err } From 005652edb79096164554bdb20ae4c812db8620ee Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 24 Aug 2026 13:19:33 -0700 Subject: [PATCH 2/3] test(binding): reject oversized final containers before allocation Prove ConvertFinal returns ErrValueLimit for oversized tuples, lists, and dictionaries without allocating destination memory proportional to source length. --- internal/binding/output_test.go | 80 +++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/internal/binding/output_test.go b/internal/binding/output_test.go index 2040ca6..0410eb0 100644 --- a/internal/binding/output_test.go +++ b/internal/binding/output_test.go @@ -3,6 +3,9 @@ package binding import ( "math" "math/big" + "runtime" + "runtime/debug" + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -180,3 +183,80 @@ func TestConvertFinalBoundsSharedSubstructure(t *testing.T) { require.ErrorIs(t, err, ErrValueLimit) assert.Contains(t, err.Error(), "node budget") } + +// TestConvertFinalRejectsOversizedContainersBeforeAllocation proves oversized +// tuple, list, and dictionary sources are rejected before destination materialization. +func TestConvertFinalRejectsOversizedContainersBeforeAllocation(t *testing.T) { + const ( + sourceLen = 128_000 + maxDepth = 4 + maxBytes = 1024 + maxAllocBytes = 1 << 20 + ) + + item := starlark.String("x") + tuple := make(starlark.Tuple, sourceLen) + listItems := make([]starlark.Value, sourceLen) + for index := range sourceLen { + tuple[index] = item + listItems[index] = item + } + list := starlark.NewList(listItems) + + dictionary := starlark.NewDict(sourceLen) + for index := range sourceLen { + require.NoError(t, dictionary.SetKey(starlark.String(strconv.Itoa(index)), item)) + } + + tests := []struct { + // name identifies the oversized container kind. + name string + + // value is the prebuilt source presented for final conversion. + value starlark.Value + }{ + {name: "tuple", value: tuple}, + {name: "list", value: list}, + {name: "dictionary", value: dictionary}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + allocated, err := measureConvertFinalGrowth(t, tt.value, maxDepth, maxBytes) + + require.Error(t, err) + require.ErrorIs(t, err, ErrValueLimit) + assert.Contains(t, err.Error(), "value exceeds byte-derived node budget") + assert.LessOrEqual( + t, + allocated, + uint64(maxAllocBytes), + "ConvertFinal allocated %d bytes converting an oversized %s; destination materialization must not scale with source length %d", + allocated, + tt.name, + sourceLen, + ) + runtime.KeepAlive(tt.value) + }) + } + + runtime.KeepAlive(tests) +} + +// measureConvertFinalGrowth reports heap bytes allocated by ConvertFinal. +// +// Automatic GC is disabled only for the measured call so TotalAlloc reflects +// conversion work rather than concurrent collection, then the previous GC +// percent is restored. +func measureConvertFinalGrowth(t *testing.T, value starlark.Value, maxDepth int, maxBytes int) (uint64, error) { + t.Helper() + + defer debug.SetGCPercent(debug.SetGCPercent(-1)) + + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + _, err := ConvertFinal(value, maxDepth, maxBytes) + runtime.ReadMemStats(&after) + runtime.KeepAlive(value) + return after.TotalAlloc - before.TotalAlloc, err +} From dd42c5d240e0b88cd09472ab9cbd63face91ce3e Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 24 Aug 2026 13:22:09 -0700 Subject: [PATCH 3/3] test(binding): clarify allocation measurement setup --- internal/binding/output_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/binding/output_test.go b/internal/binding/output_test.go index 0410eb0..ed6e04b 100644 --- a/internal/binding/output_test.go +++ b/internal/binding/output_test.go @@ -251,7 +251,8 @@ func TestConvertFinalRejectsOversizedContainersBeforeAllocation(t *testing.T) { func measureConvertFinalGrowth(t *testing.T, value starlark.Value, maxDepth int, maxBytes int) (uint64, error) { t.Helper() - defer debug.SetGCPercent(debug.SetGCPercent(-1)) + previousGCPercent := debug.SetGCPercent(-1) + defer debug.SetGCPercent(previousGCPercent) var before, after runtime.MemStats runtime.ReadMemStats(&before)