From abeb50b525aabbe5d4f7ab4cd09cd9b74c7101d6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 21:32:24 +0000 Subject: [PATCH] Bound JSON-compatible value copying --- backends.go | 3 +- docs/formats.md | 1 + docs/internal/overview.md | 2 +- engine_test.go | 25 ++ internal/jsonvalue/jsonvalue.go | 213 ++++++++--- internal/jsonvalue/jsonvalue_test.go | 378 ++++++++++++++++---- internal/usecase/prepared_execution_test.go | 40 +++ public_contract_test.go | 1 + types.go | 9 +- 9 files changed, 539 insertions(+), 133 deletions(-) diff --git a/backends.go b/backends.go index 4e979d4..4491277 100644 --- a/backends.go +++ b/backends.go @@ -34,7 +34,8 @@ type Backend struct { // JSON-compatible, finite, acyclic, and keyed by non-empty strings. Keys // must not be model, session_id, messages, temperature, max_tokens, top_p, // service_tier, reasoning_effort, or response_format. An empty map supplies - // no defaults. NewEngine deeply copies the map. + // no defaults. NewEngine deeply copies the map and rejects excessively deep + // or large values for safety. ExtraParams map[string]any // ConcurrencyLimit is the maximum number of simultaneous model-generation // calls allowed for this backend within one Engine. Zero leaves the backend diff --git a/docs/formats.md b/docs/formats.md index 8aa1775..61c4bbd 100644 --- a/docs/formats.md +++ b/docs/formats.md @@ -183,6 +183,7 @@ GoDoc. objects with string keys. Keys must be non-empty. With the built-in client, they also cannot collide with the standard fields listed in the [outbound request contract](integrations/openai-compatible-chat.md#request-body). +Excessively deep or large JSON-shaped values are rejected for safety. ### Defaults And Overrides diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 3196e8a..c55d856 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -19,7 +19,7 @@ contributor workflow and validation. | `internal/domain` | Defines internal framework values for requests, artifacts, prompt definitions, profiles, execution targets, rendering, generation, and validation, and owns source-neutral invariants for shared execution settings, session identifiers, and output contracts. Source parsing, required fields, source-specific normalization and defaulting, and boundary-specific error classification remain with their callers. | [Domain declarations](../../internal/domain/domain.go) | | `internal/defaults` | Defines application-neutral framework constants and constructs the default execution target. It contains no CLI, server, or inbound HTTP limits. | [Framework defaults](../../internal/defaults/defaults.go) | | `internal/filecatalog` | Provides deterministic YAML discovery and path helpers for operating-system filesystems and `fs.FS` sources. | [File catalog](../../internal/filecatalog/catalog.go) | -| `internal/jsonvalue` | Validates and deeply copies JSON-compatible extra-parameter and prepared-schema trees while preserving supported concrete value types. | [JSON values](../../internal/jsonvalue/jsonvalue.go) | +| `internal/jsonvalue` | Validates and deeply copies bounded JSON-compatible extra-parameter and prepared-schema trees while preserving supported concrete value types and rejecting cycles or excessive depth and work. | [JSON values](../../internal/jsonvalue/jsonvalue.go) | | `internal/promptdef` | Loads strictly decoded, validated prompt definitions from filesystem and `fs.FS` sources, including version selection and contained file-backed message content. | [Framework formats](../formats.md), [prompt-definition repository](../../internal/promptdef/filesystem_repository.go) | | `internal/profile` | Loads strictly decoded, validated execution profiles, including backend selection, from filesystem and `fs.FS` sources and composes repositories with error-preserving fallback. | [Framework formats](../formats.md), [profile repositories](../../internal/profile/filesystem_repository.go) | | `internal/profile/builtin` | Embeds the built-in profile catalog, whose entries select OpenRouter. | [Built-in catalog](../formats.md#built-in-profile-catalog), [repository](../../internal/profile/builtin/repository.go) | diff --git a/engine_test.go b/engine_test.go index e3f7cb2..d5b43f1 100644 --- a/engine_test.go +++ b/engine_test.go @@ -2202,6 +2202,7 @@ func TestWithProfilesRejectsInvalidExtraParams(t *testing.T) { {name: "nan", extraParams: map[string]any{"bad": math.NaN()}}, {name: "positive infinity", extraParams: map[string]any{"bad": math.Inf(1)}}, {name: "negative infinity", extraParams: map[string]any{"bad": math.Inf(-1)}}, + {name: "excessively large value", extraParams: map[string]any{"bad": excessivelyLargeJSONValue()}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -2466,6 +2467,7 @@ func TestRunRejectsInvalidExtraParams(t *testing.T) { {name: "cyclic slice", extraParams: map[string]any{"cycle": cyclicSlice}}, {name: "malformed JSON number", extraParams: map[string]any{"value": json.Number("+1")}}, {name: "empty nested key", extraParams: map[string]any{"nested": map[string]any{"": true}}}, + {name: "excessively deep value", extraParams: map[string]any{"nested": excessivelyDeepJSONValue()}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -2490,6 +2492,29 @@ func TestRunRejectsInvalidExtraParams(t *testing.T) { } } +func excessivelyDeepJSONValue() any { + const clearlyUnsafeContainerDepth = 1_000 + var value any = true + for level := 0; level < clearlyUnsafeContainerDepth; level++ { + if level%2 == 0 { + value = map[string]any{"child": value} + } else { + value = []any{value} + } + } + return value +} + +func excessivelyLargeJSONValue() any { + const clearlyUnsafeSharedOccurrences = 60_000 + shared := []any{true} + values := make([]any, clearlyUnsafeSharedOccurrences) + for i := range values { + values[i] = shared + } + return values +} + func TestWithLLMClientRejectsNilClient(t *testing.T) { _, err := promptkit.NewEngine(contractConfig(frameworkSchemaDir), promptkit.WithLLMClient(nil)) if !errors.Is(err, promptkit.ErrInvalidConfig) { diff --git a/internal/jsonvalue/jsonvalue.go b/internal/jsonvalue/jsonvalue.go index 33254f2..2561e46 100644 --- a/internal/jsonvalue/jsonvalue.go +++ b/internal/jsonvalue/jsonvalue.go @@ -1,5 +1,5 @@ -// Package jsonvalue validates and defensively copies JSON-compatible value -// trees used by configuration, request, and prepared-state boundaries. +// Package jsonvalue validates and defensively copies bounded JSON-compatible +// value trees used by configuration, request, and prepared-state boundaries. package jsonvalue import ( @@ -8,29 +8,39 @@ import ( "math" "reflect" "sort" - "strconv" ) -const maxSafeJSONInteger = 1<<53 - 1 +const ( + maxContainerDepth = 100 + maxProducedNodes = 100_000 +) type visit struct { typ reflect.Type ptr uintptr } +type traversalState struct { + active map[visit]struct{} + producedNodes int +} + // Copy validates and deeply copies a JSON-compatible value while preserving -// compatible concrete map, slice, array, scalar, and number types. +// compatible concrete map, slice, array, scalar, and number types. It rejects +// cycles and values that exceed the package's traversal limits. func Copy(src any) (any, error) { - return copyValue(reflect.ValueOf(src), "value", make(map[visit]struct{}), true) + return copyValue(reflect.ValueOf(src), "value", newTraversalState(), true, 0) } // CopyMap validates and deeply copies an extra-parameter map while preserving -// compatible concrete map, slice, array, scalar, and number types. +// compatible concrete map, slice, array, scalar, and number types. It rejects +// empty object keys, cycles, and values that exceed the package's traversal +// limits. func CopyMap(src map[string]any) (map[string]any, error) { if src == nil { return nil, nil } - copied, err := copyValue(reflect.ValueOf(src), "extra_params", make(map[visit]struct{}), false) + copied, err := copyValue(reflect.ValueOf(src), "extra_params", newTraversalState(), false, 0) if err != nil { return nil, err } @@ -44,71 +54,90 @@ func CopyMap(src map[string]any) (map[string]any, error) { func copyValue( value reflect.Value, path string, - seen map[visit]struct{}, + state *traversalState, allowEmptyMapKeys bool, + containerDepth int, ) (any, error) { - if !value.IsValid() { + resolved, cleanup, isNull, err := state.resolveIndirection(value, path) + if err != nil { + return nil, err + } + defer cleanup() + if isNull { + if err := state.produceNode(path); err != nil { + return nil, err + } return nil, nil } - if value.Kind() == reflect.Interface { - if value.IsNil() { - return nil, nil - } - return copyValue(value.Elem(), path, seen, allowEmptyMapKeys) - } + value = resolved if !value.CanInterface() { return nil, fmt.Errorf("%s: value cannot be copied", path) } if number, ok := value.Interface().(json.Number); ok { - if _, err := json.Marshal(number); err != nil { + if !validJSONNumber(number) { return nil, fmt.Errorf("%s: invalid JSON number", path) } - f, err := strconv.ParseFloat(number.String(), 64) - if err != nil || math.IsNaN(f) || math.IsInf(f, 0) { - return nil, fmt.Errorf("%s: invalid JSON number", path) + if err := state.produceNode(path); err != nil { + return nil, err } return number, nil } switch value.Kind() { case reflect.Bool, reflect.String: + if err := state.produceNode(path); err != nil { + return nil, err + } return value.Interface(), nil case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - if value.Int() < -maxSafeJSONInteger || value.Int() > maxSafeJSONInteger { - return nil, fmt.Errorf("%s: integer is outside the JSON-safe range", path) + if err := state.produceNode(path); err != nil { + return nil, err } return value.Interface(), nil case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: - if value.Uint() > maxSafeJSONInteger { - return nil, fmt.Errorf("%s: integer is outside the JSON-safe range", path) + if err := state.produceNode(path); err != nil { + return nil, err } return value.Interface(), nil case reflect.Float32, reflect.Float64: - number := value.Convert(reflect.TypeOf(float64(0))).Float() + number := value.Float() if math.IsNaN(number) || math.IsInf(number, 0) { return nil, fmt.Errorf("%s: floating-point value must be finite", path) } + if err := state.produceNode(path); err != nil { + return nil, err + } return value.Interface(), nil - case reflect.Pointer: + case reflect.Map: if value.IsNil() { + if err := state.produceNode(path); err != nil { + return nil, err + } return nil, nil } - current := visit{typ: value.Type(), ptr: value.Pointer()} - if _, ok := seen[current]; ok { - return nil, fmt.Errorf("%s: cyclic value is not supported", path) + nextDepth, err := state.enterContainer(path, containerDepth) + if err != nil { + return nil, err } - seen[current] = struct{}{} - defer delete(seen, current) - return copyValue(value.Elem(), path, seen, allowEmptyMapKeys) - case reflect.Map: - return copyMapValue(value, path, seen, allowEmptyMapKeys) + return copyMapValue(value, path, state, allowEmptyMapKeys, nextDepth) case reflect.Slice: if value.IsNil() { + if err := state.produceNode(path); err != nil { + return nil, err + } return nil, nil } - return copySequenceValue(value, path, seen, allowEmptyMapKeys) + nextDepth, err := state.enterContainer(path, containerDepth) + if err != nil { + return nil, err + } + return copySequenceValue(value, path, state, allowEmptyMapKeys, nextDepth) case reflect.Array: - return copySequenceValue(value, path, seen, allowEmptyMapKeys) + nextDepth, err := state.enterContainer(path, containerDepth) + if err != nil { + return nil, err + } + return copySequenceValue(value, path, state, allowEmptyMapKeys, nextDepth) default: return nil, fmt.Errorf("%s: unsupported JSON value type %s", path, value.Type()) } @@ -117,22 +146,26 @@ func copyValue( func copyMapValue( value reflect.Value, path string, - seen map[visit]struct{}, + state *traversalState, allowEmptyMapKeys bool, + containerDepth int, ) (any, error) { - if value.IsNil() { - return nil, nil - } if value.Type().Key().Kind() != reflect.String { return nil, fmt.Errorf("%s: map key type %s is not supported", path, value.Type().Key()) } + if err := state.produceNode(path); err != nil { + return nil, err + } + if err := state.ensureChildCapacity(path, value.Len()); err != nil { + return nil, err + } current := visit{typ: value.Type(), ptr: value.Pointer()} - if _, ok := seen[current]; ok { + if _, ok := state.active[current]; ok { return nil, fmt.Errorf("%s: cyclic value is not supported", path) } - seen[current] = struct{}{} - defer delete(seen, current) + state.active[current] = struct{}{} + defer delete(state.active, current) keys := value.MapKeys() sort.Slice(keys, func(i, j int) bool { @@ -152,7 +185,13 @@ func copyMapValue( if name == "" && !allowEmptyMapKeys { return nil, fmt.Errorf("%s: map key must not be empty", path) } - copied, err := copyValue(value.MapIndex(key), path+"."+name, seen, allowEmptyMapKeys) + copied, err := copyValue( + value.MapIndex(key), + path+"."+name, + state, + allowEmptyMapKeys, + containerDepth, + ) if err != nil { return nil, err } @@ -190,17 +229,25 @@ func copyMapValue( func copySequenceValue( value reflect.Value, path string, - seen map[visit]struct{}, + state *traversalState, allowEmptyMapKeys bool, + containerDepth int, ) (any, error) { + if err := state.produceNode(path); err != nil { + return nil, err + } + if err := state.ensureChildCapacity(path, value.Len()); err != nil { + return nil, err + } + var current visit if value.Kind() == reflect.Slice { current = visit{typ: value.Type(), ptr: value.Pointer()} - if _, ok := seen[current]; ok { + if _, ok := state.active[current]; ok { return nil, fmt.Errorf("%s: cyclic value is not supported", path) } - seen[current] = struct{}{} - defer delete(seen, current) + state.active[current] = struct{}{} + defer delete(state.active, current) } values := make([]any, value.Len()) @@ -210,8 +257,9 @@ func copySequenceValue( copied, err := copyValue( value.Index(i), fmt.Sprintf("%s[%d]", path, i), - seen, + state, allowEmptyMapKeys, + containerDepth, ) if err != nil { return nil, err @@ -248,6 +296,73 @@ func copySequenceValue( return out, nil } +func validJSONNumber(number json.Number) bool { + var parsed json.Number + if err := json.Unmarshal([]byte(number.String()), &parsed); err != nil { + return false + } + return parsed.String() == number.String() +} + +func newTraversalState() *traversalState { + return &traversalState{active: make(map[visit]struct{})} +} + +func (state *traversalState) produceNode(path string) error { + if state.producedNodes >= maxProducedNodes { + return fmt.Errorf("%s: JSON value work limit exceeded", path) + } + state.producedNodes++ + return nil +} + +func (state *traversalState) enterContainer(path string, depth int) (int, error) { + depth++ + if depth > maxContainerDepth { + return 0, fmt.Errorf("%s: JSON container depth limit exceeded", path) + } + return depth, nil +} + +func (state *traversalState) ensureChildCapacity(path string, count int) error { + if count > maxProducedNodes-state.producedNodes { + return fmt.Errorf("%s: JSON value work limit exceeded", path) + } + return nil +} + +func (state *traversalState) resolveIndirection( + value reflect.Value, + path string, +) (reflect.Value, func(), bool, error) { + var visits []visit + cleanup := func() { + for _, current := range visits { + delete(state.active, current) + } + } + + for value.IsValid() && (value.Kind() == reflect.Interface || value.Kind() == reflect.Pointer) { + if value.IsNil() { + return reflect.Value{}, cleanup, true, nil + } + if value.Kind() == reflect.Pointer { + current := visit{typ: value.Type(), ptr: value.Pointer()} + if _, ok := state.active[current]; ok { + cleanup() + return reflect.Value{}, nil, false, fmt.Errorf("%s: cyclic value is not supported", path) + } + state.active[current] = struct{}{} + visits = append(visits, current) + } + value = value.Elem() + } + if !value.IsValid() { + return reflect.Value{}, cleanup, true, nil + } + return value, cleanup, false, nil +} + func canAssignNil(typ reflect.Type) bool { switch typ.Kind() { case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: diff --git a/internal/jsonvalue/jsonvalue_test.go b/internal/jsonvalue/jsonvalue_test.go index bafd373..f06bd42 100644 --- a/internal/jsonvalue/jsonvalue_test.go +++ b/internal/jsonvalue/jsonvalue_test.go @@ -1,112 +1,332 @@ -package jsonvalue_test +package jsonvalue import ( "encoding/json" "math" "reflect" + "strings" "testing" - - "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" ) -func TestCopyMapPreservesTypesAndIsolatesMutations(t *testing.T) { - nested := map[string]int{"limit": 2} - sequence := []string{"one", "two"} - input := map[string]any{ - "count": int64(7), - "number": json.Number("-1.25e+2"), - "nested": nested, - "sequence": sequence, - } - - copied, err := jsonvalue.CopyMap(input) - if err != nil { - t.Fatalf("copy map: %v", err) - } - nested["limit"] = 99 - sequence[0] = "changed" - input["added"] = true - - if got, ok := copied["count"].(int64); !ok || got != 7 { - t.Fatalf("integer type or value changed: %#v", copied["count"]) - } - if got, ok := copied["number"].(json.Number); !ok || got != "-1.25e+2" { - t.Fatalf("JSON number type or value changed: %#v", copied["number"]) - } - if got := copied["nested"].(map[string]int)["limit"]; got != 2 { - t.Fatalf("nested map was not isolated: %d", got) - } - if got := copied["sequence"].([]string)[0]; got != "one" { - t.Fatalf("sequence was not isolated: %q", got) - } - if _, ok := copied["added"]; ok { - t.Fatalf("top-level map was not isolated: %#v", copied) - } -} - -func TestCopyAllowsEmptyObjectKeysAndIsolatesMutations(t *testing.T) { - nested := map[string]any{"": []any{"original"}} - - copiedValue, err := jsonvalue.Copy(nested) - if err != nil { - t.Fatalf("copy value: %v", err) - } - nested[""].([]any)[0] = "changed" - - copied := copiedValue.(map[string]any) - if got := copied[""].([]any)[0]; got != "original" { - t.Fatalf("copied value was not isolated: %v", got) - } -} - -func TestCopyMapRejectsInvalidValues(t *testing.T) { - cyclicMap := map[string]any{} - cyclicMap["self"] = cyclicMap - cyclicSlice := []any{nil} - cyclicSlice[0] = cyclicSlice +type ( + namedBool bool + namedString string + namedInt64 int64 + namedUint64 uint64 + namedFloat32 float32 + namedFloat64 float64 + namedKey string + namedMap map[namedKey]namedInt64 + namedSlice []namedString + namedArray [1]map[string]int +) +func TestCopyPreservesSupportedScalarAndNumberTypes(t *testing.T) { + maxInt := int(^uint(0) >> 1) + minInt := -maxInt - 1 tests := []struct { name string value any }{ - {name: "empty nested key", value: map[string]int{"": 1}}, - {name: "non-string map key", value: map[int]string{1: "one"}}, - {name: "unsupported value", value: make(chan int)}, - {name: "cyclic map", value: cyclicMap}, - {name: "cyclic slice", value: cyclicSlice}, - {name: "NaN", value: math.NaN()}, - {name: "positive infinity", value: math.Inf(1)}, - {name: "unsafe signed integer", value: int64(1 << 53)}, - {name: "unsafe unsigned integer", value: uint64(1 << 53)}, + {name: "bool", value: true}, + {name: "named bool", value: namedBool(true)}, + {name: "string", value: "value"}, + {name: "named string", value: namedString("value")}, + {name: "int", value: minInt}, + {name: "int8", value: int8(-1 << 7)}, + {name: "int16", value: int16(-1 << 15)}, + {name: "int32", value: int32(-1 << 31)}, + {name: "int64", value: int64(-1 << 63)}, + {name: "named int64", value: namedInt64(1<<63 - 1)}, + {name: "uint", value: ^uint(0)}, + {name: "uint8", value: ^uint8(0)}, + {name: "uint16", value: ^uint16(0)}, + {name: "uint32", value: ^uint32(0)}, + {name: "uint64", value: ^uint64(0)}, + {name: "uintptr", value: ^uintptr(0)}, + {name: "named uint64", value: namedUint64(^uint64(0))}, + {name: "float32", value: float32(1.25)}, + {name: "float64", value: float64(-2.5e100)}, + {name: "named float32", value: namedFloat32(3.5)}, + {name: "named float64", value: namedFloat64(-4.5e200)}, + {name: "JSON number integer", value: json.Number("18446744073709551615")}, + {name: "JSON number fraction", value: json.Number("-1.25e+2")}, + {name: "JSON number beyond float64", value: json.Number("1e9999")}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if _, err := jsonvalue.CopyMap(map[string]any{"value": tc.value}); err == nil { + got, err := Copy(tc.value) + if err != nil { + t.Fatalf("copy value: %v", err) + } + if !reflect.DeepEqual(got, tc.value) { + t.Fatalf("value or concrete type changed: got %#v (%T), want %#v (%T)", got, got, tc.value, tc.value) + } + }) + } +} + +func TestCopyRejectsInvalidNumbers(t *testing.T) { + tests := []struct { + name string + value any + }{ + {name: "float32 NaN", value: float32(math.NaN())}, + {name: "float64 NaN", value: math.NaN()}, + {name: "named float NaN", value: namedFloat64(math.NaN())}, + {name: "positive infinity", value: math.Inf(1)}, + {name: "negative infinity", value: math.Inf(-1)}, + {name: "empty JSON number", value: json.Number("")}, + {name: "leading zero JSON number", value: json.Number("01")}, + {name: "leading plus JSON number", value: json.Number("+1")}, + {name: "trailing decimal JSON number", value: json.Number("1.")}, + {name: "leading decimal JSON number", value: json.Number(".1")}, + {name: "non-number JSON number", value: json.Number("NaN")}, + {name: "spaced JSON number", value: json.Number(" 1")}, + {name: "quoted JSON number", value: json.Number(`"1"`)}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if _, err := Copy(tc.value); err == nil { t.Fatal("expected validation error") } }) } } -func TestCopyMapValidatesJSONNumberSyntaxAndRange(t *testing.T) { - for _, number := range []json.Number{"0", "-1", "1.25", "-1.25e+2"} { - t.Run("valid "+number.String(), func(t *testing.T) { - got, err := jsonvalue.CopyMap(map[string]any{"value": number}) +func TestCopyPreservesCompatibleCollectionsAndNilEmptyDistinctions(t *testing.T) { + collections := []struct { + name string + value any + }{ + {name: "unnamed map", value: map[string]int{"limit": 2}}, + {name: "named map", value: namedMap{"limit": 2}}, + {name: "unnamed slice", value: []string{"one", "two"}}, + {name: "named slice", value: namedSlice{"one", "two"}}, + {name: "unnamed array", value: [2]int{1, 2}}, + {name: "named array", value: namedArray{{"limit": 2}}}, + {name: "empty map", value: map[string]int{}}, + {name: "empty named map", value: namedMap{}}, + {name: "empty slice", value: []string{}}, + {name: "empty named slice", value: namedSlice{}}, + {name: "empty array", value: [0]string{}}, + } + + for _, tc := range collections { + t.Run(tc.name, func(t *testing.T) { + got, err := Copy(tc.value) if err != nil { - t.Fatalf("copy valid JSON number: %v", err) + t.Fatalf("copy collection: %v", err) } - if !reflect.DeepEqual(got["value"], number) { - t.Fatalf("JSON number changed: got %#v want %#v", got["value"], number) + if !reflect.DeepEqual(got, tc.value) || reflect.TypeOf(got) != reflect.TypeOf(tc.value) { + t.Fatalf("collection changed: got %#v (%T), want %#v (%T)", got, got, tc.value, tc.value) + } + kind := reflect.ValueOf(got).Kind() + if (kind == reflect.Map || kind == reflect.Slice) && reflect.ValueOf(got).IsNil() { + t.Fatal("non-nil collection became nil") } }) } - for _, number := range []json.Number{"", "01", "+1", "1.", ".1", "1e9999", "not-a-number"} { - t.Run("invalid "+number.String(), func(t *testing.T) { - if _, err := jsonvalue.CopyMap(map[string]any{"value": number}); err == nil { - t.Fatal("expected invalid JSON number error") + var nilMap map[string]int + var nilSlice []string + var nilPointer *namedInt64 + for _, value := range []any{nil, nilMap, nilSlice, nilPointer} { + got, err := Copy(value) + if err != nil { + t.Fatalf("copy null value: %v", err) + } + if got != nil { + t.Fatalf("null value became %#v (%T)", got, got) + } + } + + gotNil, err := CopyMap(nil) + if err != nil || gotNil != nil { + t.Fatalf("nil CopyMap result = %#v, %v", gotNil, err) + } + gotEmpty, err := CopyMap(map[string]any{}) + if err != nil || gotEmpty == nil || len(gotEmpty) != 0 { + t.Fatalf("empty CopyMap result = %#v, %v", gotEmpty, err) + } +} + +func TestCopyHandlesIndirectionAndIsolatesNestedMutations(t *testing.T) { + integer := namedInt64(7) + nestedMap := namedMap{"limit": 2} + nestedSlice := namedSlice{"original"} + nestedArray := namedArray{{"limit": 3}} + shared := []any{map[string]int{"value": 4}} + input := map[string]any{ + "integer": &integer, + "map": nestedMap, + "slice": nestedSlice, + "array": nestedArray, + "first": shared, + "second": shared, + } + + copiedValue, err := Copy(input) + if err != nil { + t.Fatalf("copy mixed tree: %v", err) + } + copied := copiedValue.(map[string]any) + nestedMap["limit"] = 20 + nestedSlice[0] = "changed" + nestedArray[0]["limit"] = 30 + shared[0].(map[string]int)["value"] = 40 + + if got, ok := copied["integer"].(namedInt64); !ok || got != 7 { + t.Fatalf("pointer target changed: %#v", copied["integer"]) + } + if got := copied["map"].(namedMap)["limit"]; got != 2 { + t.Fatalf("nested map aliased input: %d", got) + } + if got := copied["slice"].(namedSlice)[0]; got != "original" { + t.Fatalf("nested slice aliased input: %q", got) + } + if got := copied["array"].(namedArray)[0]["limit"]; got != 3 { + t.Fatalf("nested array aliased input: %d", got) + } + first := copied["first"].([]any) + second := copied["second"].([]any) + if got := first[0].(map[string]int)["value"]; got != 4 { + t.Fatalf("shared child aliased input: %d", got) + } + first[0].(map[string]int)["value"] = 99 + if got := second[0].(map[string]int)["value"]; got != 4 { + t.Fatalf("repeated acyclic value shared copied output: %d", got) + } +} + +func TestCopyAndCopyMapApplyDistinctEmptyKeyRules(t *testing.T) { + nested := map[string]any{"": []any{"original"}} + copiedValue, err := Copy(nested) + if err != nil { + t.Fatalf("Copy rejected empty schema key: %v", err) + } + nested[""].([]any)[0] = "changed" + if got := copiedValue.(map[string]any)[""].([]any)[0]; got != "original" { + t.Fatalf("copied schema value was not isolated: %v", got) + } + + _, err = CopyMap(map[string]any{"nested": map[string]any{"": true}}) + if err == nil || !strings.Contains(err.Error(), "extra_params.nested") { + t.Fatalf("CopyMap empty-key error = %v", err) + } +} + +func TestCopyRejectsUnsupportedValuesAndActiveCycles(t *testing.T) { + cyclicMap := map[string]any{} + cyclicMap["self"] = cyclicMap + cyclicSlice := []any{nil} + cyclicSlice[0] = cyclicSlice + var cyclicPointer any + cyclicPointer = &cyclicPointer + + tests := []struct { + name string + value any + wantPath string + }{ + {name: "non-string map key", value: map[int]string{1: "one"}, wantPath: "value"}, + {name: "unsupported channel", value: make(chan int), wantPath: "value"}, + {name: "deterministic map path", value: map[string]any{"z": make(chan int), "a": make(chan int)}, wantPath: "value.a"}, + {name: "cyclic map", value: cyclicMap, wantPath: "value.self"}, + {name: "cyclic slice", value: cyclicSlice, wantPath: "value[0]"}, + {name: "cyclic pointer", value: cyclicPointer, wantPath: "value"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := Copy(tc.value) + if err == nil || !strings.Contains(err.Error(), tc.wantPath) { + t.Fatalf("error = %v, want structural path %q", err, tc.wantPath) } }) } } + +func TestCopyEnforcesContainerDepth(t *testing.T) { + tests := []struct { + name string + depth int + wantErr bool + }{ + {name: "just below", depth: maxContainerDepth - 1}, + {name: "at limit", depth: maxContainerDepth}, + {name: "over limit", depth: maxContainerDepth + 1, wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := Copy(alternatingContainers(tc.depth)) + if tc.wantErr { + if err == nil || !strings.HasPrefix(err.Error(), "value") || !strings.Contains(err.Error(), "container depth limit") { + t.Fatalf("depth error = %v", err) + } + return + } + if err != nil { + t.Fatalf("copy depth %d: %v", tc.depth, err) + } + }) + } +} + +func TestCopyEnforcesProducedNodeBudgetForRepeatedAcyclicValues(t *testing.T) { + shared := []any{true} + sharedOccurrences := (maxProducedNodes - 2) / 2 + justBelow := repeatedValues(shared, sharedOccurrences, 0) + atLimit := repeatedValues(shared, sharedOccurrences, 1) + overLimit := repeatedValues(shared, sharedOccurrences, 2) + + for name, value := range map[string]any{ + "just below": justBelow, + "at limit": atLimit, + } { + t.Run(name, func(t *testing.T) { + if _, err := Copy(value); err != nil { + t.Fatalf("copy value within work budget: %v", err) + } + }) + } + + _, err := Copy(overLimit) + if err == nil || !strings.HasPrefix(err.Error(), "value[") || !strings.Contains(err.Error(), "value work limit") { + t.Fatalf("work-budget error = %v", err) + } + + _, err = Copy(make([]any, maxProducedNodes)) + if err == nil || !strings.HasPrefix(err.Error(), "value:") || !strings.Contains(err.Error(), "value work limit") { + t.Fatalf("flat work-budget error = %v", err) + } +} + +func alternatingContainers(depth int) any { + var value any = true + for level := 0; level < depth; level++ { + switch level % 3 { + case 0: + value = map[string]any{"child": value} + case 1: + value = []any{value} + default: + value = [1]any{value} + } + } + return value +} + +func repeatedValues(shared []any, occurrences, leadingScalars int) []any { + values := make([]any, 0, leadingScalars+occurrences) + for i := 0; i < leadingScalars; i++ { + values = append(values, false) + } + for i := 0; i < occurrences; i++ { + values = append(values, shared) + } + return values +} diff --git a/internal/usecase/prepared_execution_test.go b/internal/usecase/prepared_execution_test.go index 0d4973f..61ebcb7 100644 --- a/internal/usecase/prepared_execution_test.go +++ b/internal/usecase/prepared_execution_test.go @@ -164,6 +164,46 @@ func TestRunnerPrepareExecutionCompletesWithoutAdmissionOrGeneration(t *testing. } } +func TestRunnerPrepareExecutionRejectsExcessivelyDeepPreparedSchema(t *testing.T) { + def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0) + def.Validation.SchemaPath = "schema.json" + llmClient := &fakeLLM{forbid: true} + validator := &recordingValidationPreparer{ + plan: &recordingPreparedValidation{schemaDocument: excessivelyDeepPreparedJSONValue()}, + } + runner := NewRunner( + &fakePromptRepo{def: def}, + &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": defaultExecutionProfile()}}, + nil, + defaultArtifactReader(), + defaultRenderer(), + llmClient, + validator, + nil, + ) + + _, err := runner.PrepareExecution(context.Background(), domain.RunRequest{ + PromptID: "p", + ProfileID: "exec", + Inputs: singleInputRef(), + }) + if !errors.Is(err, ErrInvalidRequest) { + t.Fatalf("expected ErrInvalidRequest, got %v", err) + } + if llmClient.calls != 0 { + t.Fatalf("invalid prepared schema reached generation: %d calls", llmClient.calls) + } +} + +func excessivelyDeepPreparedJSONValue() any { + const clearlyUnsafeContainerDepth = 1_000 + var value any = true + for level := 0; level < clearlyUnsafeContainerDepth; level++ { + value = map[string]any{"child": value} + } + return value +} + func TestRunnerRunPreparedRechecksEnvironmentCredentialBeforeAdmission(t *testing.T) { const environmentName = "PROMPTKIT_PREPARED_EXECUTION_TEST_KEY" t.Setenv(environmentName, "available-during-preparation") diff --git a/public_contract_test.go b/public_contract_test.go index 6e38218..63e6b00 100644 --- a/public_contract_test.go +++ b/public_contract_test.go @@ -757,6 +757,7 @@ func TestBackendRegistrationRejectsInvalidAndDuplicateDefinitions(t *testing.T) {name: "reserved extra parameter", backends: []promptkit.Backend{{ID: "custom", Endpoint: "http://example.test/v1", ExtraParams: map[string]any{"model": "override"}}}}, {name: "cyclic extra parameter", backends: []promptkit.Backend{{ID: "custom", Endpoint: "http://example.test/v1", ExtraParams: cycle}}}, {name: "malformed JSON number", backends: []promptkit.Backend{{ID: "custom", Endpoint: "http://example.test/v1", ExtraParams: map[string]any{"value": json.Number("01")}}}}, + {name: "excessively deep extra parameter", backends: []promptkit.Backend{{ID: "custom", Endpoint: "http://example.test/v1", ExtraParams: map[string]any{"value": excessivelyDeepJSONValue()}}}}, {name: "duplicate consumer id", backends: []promptkit.Backend{ {ID: " custom ", Endpoint: "http://one.example/v1"}, {ID: "custom", Endpoint: "http://two.example/v1"}, diff --git a/types.go b/types.go index 256bc40..c9459d7 100644 --- a/types.go +++ b/types.go @@ -82,7 +82,8 @@ const ( // Prepare, PrepareExecution, and Run copy the request's maps, pointers, and // nested JSON-compatible values before using them. The caller may mutate the // request after any method returns. A successful PrepareExecution retains its -// own private execution snapshot for RunPrepared. +// own private execution snapshot for RunPrepared. Excessively deep or large +// JSON-shaped values are rejected for safety. type RunRequest struct { // PromptID is the required non-empty prompt identifier. PromptID string @@ -428,7 +429,8 @@ type ExecutionTargetOverride struct { APIKeyEnv string // ExtraParams, when non-empty, replaces the complete profile or backend map. // Values must be JSON-compatible: nil, booleans, finite numbers, strings, - // arrays or slices, and maps with non-empty string keys. Cycles are invalid. + // arrays or slices, and maps with non-empty string keys. Cycles and + // excessively deep or large values are invalid. ExtraParams map[string]any } @@ -479,7 +481,8 @@ type Profile struct { APIKeyRequired bool // ExtraParams contains provider-specific JSON-compatible values. An empty // map inherits backend request defaults, when any. WithProfiles validates - // and deeply copies it during NewEngine. + // and deeply copies it during NewEngine. Excessively deep or large values + // are rejected for safety. ExtraParams map[string]any }