Skip to content

Commit 80c8b87

Browse files
Copilotpelikhan
andauthored
Fix allocation-size overflow risk in model pricing merge (#43468)
* Initial plan * Fix model pricing overflow regression Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent 4cecb7a commit 80c8b87

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

pkg/workflow/compiler_model_pricing.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@ func mergeModelPricingIntoModelCosts(modelCosts map[string]any, provider, model
162162
}
163163
modelEntry := map[string]any{"cost": cost}
164164

165-
// Shallow-clone the top-level map. Do not add +1 here to avoid potential
166-
// integer overflow; Go maps grow automatically as entries are added.
167-
result := make(map[string]any, len(modelCosts))
165+
// Shallow-clone the top-level map without pre-sizing to avoid reintroducing
166+
// allocation-size arithmetic in this security-sensitive path.
167+
result := make(map[string]any)
168168
maps.Copy(result, modelCosts)
169169

170170
// Shallow-clone the providers map.

pkg/workflow/compiler_model_pricing_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ package workflow
44

55
import (
66
"context"
7+
"go/ast"
8+
"go/parser"
9+
gotoken "go/token"
10+
"path/filepath"
11+
"runtime"
712
"testing"
813

914
"github.com/stretchr/testify/assert"
@@ -85,6 +90,49 @@ func TestMergeModelPricingIntoModelCosts_DoesNotMutateInput(t *testing.T) {
8590
assert.Empty(t, openai["models"].(map[string]any))
8691
}
8792

93+
func TestMergeModelPricingIntoModelCosts_DoesNotPreSizeTopLevelClone(t *testing.T) {
94+
_, thisFile, _, ok := runtime.Caller(0)
95+
require.True(t, ok, "runtime.Caller failed")
96+
97+
sourcePath := filepath.Join(filepath.Dir(thisFile), "compiler_model_pricing.go")
98+
file, err := parser.ParseFile(gotoken.NewFileSet(), sourcePath, nil, parser.SkipObjectResolution)
99+
require.NoError(t, err)
100+
101+
var foundResultMake bool
102+
ast.Inspect(file, func(n ast.Node) bool {
103+
fn, ok := n.(*ast.FuncDecl)
104+
if !ok || fn.Name.Name != "mergeModelPricingIntoModelCosts" {
105+
return true
106+
}
107+
108+
for _, stmt := range fn.Body.List {
109+
assign, ok := stmt.(*ast.AssignStmt)
110+
if !ok || len(assign.Lhs) != 1 || len(assign.Rhs) != 1 {
111+
continue
112+
}
113+
lhs, ok := assign.Lhs[0].(*ast.Ident)
114+
if !ok || lhs.Name != "result" {
115+
continue
116+
}
117+
call, ok := assign.Rhs[0].(*ast.CallExpr)
118+
if !ok {
119+
continue
120+
}
121+
fun, ok := call.Fun.(*ast.Ident)
122+
if !ok || fun.Name != "make" {
123+
continue
124+
}
125+
126+
foundResultMake = true
127+
assert.Len(t, call.Args, 1, "result map allocation should not pre-size capacity")
128+
return false
129+
}
130+
return false
131+
})
132+
133+
assert.True(t, foundResultMake, "expected to find result map allocation in mergeModelPricingIntoModelCosts")
134+
}
135+
88136
// ── resolveEngineProviderForPricing ─────────────────────────────────────────
89137

90138
func TestResolveEngineProviderForPricing(t *testing.T) {

0 commit comments

Comments
 (0)