From 218dc227aafb9076e83938bfffa2bbc89a09a42a Mon Sep 17 00:00:00 2001 From: TristonianJones Date: Fri, 28 Aug 2026 15:42:33 -0700 Subject: [PATCH] Saturate runtime cost accumulation to prevent overflow CostTracker.Observe accumulated call costs with an unchecked +=, so an overload reporting an unbounded actual cost wrapped the running total back to a small value whenever any cost had already accrued. json.encode, which reports math.MaxUint64, evaluated to a cost of 0 for json.encode(v) and 1 for json.encode(v) == json.encode(v). Only the all-constant case, where nothing has accrued yet, produced the intended cost. A wrapped total silently defeats CostTrackerLimit: json.encode(v) evaluated without error under a limit of 1000. Accumulate with cost.SafeAdd instead, matching the saturating convention documented throughout the cost package. The qualifier, ident and constructor increments are saturated as well, since those otherwise wrap an already saturated total back down to zero. TestEncodersCosts/json_encode_dyn asserted an unbounded estimated cost alongside an actual cost of 1, which was the wrapped value; it now expects math.MaxUint64. --- ext/encoders_test.go | 58 +++++++++++++++++++++++++++++++-- interpreter/runtimecost.go | 14 ++++---- interpreter/runtimecost_test.go | 49 ++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/ext/encoders_test.go b/ext/encoders_test.go index 1d526a15d..22e02c1bf 100644 --- a/ext/encoders_test.go +++ b/ext/encoders_test.go @@ -249,8 +249,10 @@ func TestEncodersCosts(t *testing.T) { "x": 100, }, estimatedCost: checker.CostEstimate{Min: 2, Max: math.MaxUint64}, - actualCost: 1, - version: 1, + // json.encode reports an unbounded actual cost, which saturates the + // running total rather than overflowing it back to a small value. + actualCost: math.MaxUint64, + version: 1, }, } for _, tc := range tests { @@ -337,3 +339,55 @@ func TestJSONEncodeCostUnbounded(t *testing.T) { } } +// TestJSONEncodeCostUnboundedWithAccruedCost checks that the unbounded cost of json.encode +// survives being combined with cost accrued elsewhere in the expression, and that it still +// trips a cost limit. A total which overflowed back to a small value would not. +func TestJSONEncodeCostUnboundedWithAccruedCost(t *testing.T) { + env, err := cel.NewEnv(Encoders(EncodersVersion(1)), cel.Variable("v", cel.StringType)) + if err != nil { + t.Fatalf("cel.NewEnv() failed: %v", err) + } + exprs := []string{ + "json.encode(v)", + "json.encode(v) == json.encode(v)", + "size(v) > 0 && json.encode(v) != ''", + } + for _, expr := range exprs { + t.Run(expr, func(t *testing.T) { + ast, iss := env.Compile(expr) + if iss.Err() != nil { + t.Fatalf("env.Compile(%q) failed: %v", expr, iss.Err()) + } + est, err := env.EstimateCost(ast, testCostHintEstimator{}) + if err != nil { + t.Fatalf("env.EstimateCost() failed: %v", err) + } + if est.Max != math.MaxUint64 { + t.Errorf("env.EstimateCost() got max %d, wanted %d", est.Max, uint64(math.MaxUint64)) + } + + prg, err := env.Program(ast, cel.CostTracking(nil)) + if err != nil { + t.Fatalf("env.Program() failed: %v", err) + } + _, det, err := prg.Eval(map[string]any{"v": "hello"}) + if err != nil { + t.Fatalf("prg.Eval() failed: %v", err) + } + if det.ActualCost() == nil { + t.Fatal("det.ActualCost() got nil, wanted a value") + } + if *det.ActualCost() != math.MaxUint64 { + t.Errorf("det.ActualCost() got %d, wanted %d", *det.ActualCost(), uint64(math.MaxUint64)) + } + + limited, err := env.Program(ast, cel.CostLimit(1000)) + if err != nil { + t.Fatalf("env.Program() failed: %v", err) + } + if _, _, err := limited.Eval(map[string]any{"v": "hello"}); err == nil { + t.Error("prg.Eval() got nil error, wanted cost limit exceeded") + } + }) + } +} diff --git a/interpreter/runtimecost.go b/interpreter/runtimecost.go index 5558cf6fd..960f52a00 100644 --- a/interpreter/runtimecost.go +++ b/interpreter/runtimecost.go @@ -108,7 +108,7 @@ func (ct *costTrackerFactory) Observe(vars Activation, id int64, programStep any case ConstantQualifier: // TODO: Push identifiers on to the stack before observing constant qualifiers that apply to them // and enable the below pop. Once enabled this can case can be collapsed into the Qualifier case. - tracker.cost++ + tracker.cost = cost.SafeAdd(tracker.cost, 1) case InterpretableConst: // zero cost case InterpretableAttribute: @@ -118,7 +118,7 @@ func (ct *costTrackerFactory) Observe(vars Activation, id int64, programStep any tracker.stack.drop(a.falsy.ID(), a.truthy.ID(), a.expr.ID()) default: tracker.stack.drop(t.Attr().ID()) - tracker.cost += common.SelectAndIdentCost + tracker.cost = cost.SafeAdd(tracker.cost, common.SelectAndIdentCost) } if !tracker.presenceTestHasCost { if _, isTestOnly := programStep.(*evalTestOnly); isTestOnly { @@ -150,20 +150,20 @@ func (ct *costTrackerFactory) Observe(vars Activation, id int64, programStep any case *evalFold: tracker.stack.drop(t.iterRange.ID()) case Qualifier: - tracker.cost++ + tracker.cost = cost.SafeAdd(tracker.cost, 1) case InterpretableCall: if argVals, ok := tracker.stack.dropArgs(t.Args()); ok { - tracker.cost += tracker.costCall(t, argVals, val) + tracker.cost = cost.SafeAdd(tracker.cost, tracker.costCall(t, argVals, val)) } case InterpretableConstructor: tracker.stack.dropArgs(t.InitVals()) switch t.Type() { case types.ListType: - tracker.cost += common.ListCreateBaseCost + tracker.cost = cost.SafeAdd(tracker.cost, common.ListCreateBaseCost) case types.MapType: - tracker.cost += common.MapCreateBaseCost + tracker.cost = cost.SafeAdd(tracker.cost, common.MapCreateBaseCost) default: - tracker.cost += common.StructCreateBaseCost + tracker.cost = cost.SafeAdd(tracker.cost, common.StructCreateBaseCost) } } tracker.stack.push(val, id) diff --git a/interpreter/runtimecost_test.go b/interpreter/runtimecost_test.go index b160318b7..e9a4c11e7 100644 --- a/interpreter/runtimecost_test.go +++ b/interpreter/runtimecost_test.go @@ -904,3 +904,52 @@ func TestRuntimeCost(t *testing.T) { }) } } + +// TestRuntimeCostUnboundedOverloadSaturates verifies that an overload which reports an unbounded +// actual cost saturates the running total rather than overflowing it back to a small value. A +// wrapped total would silently defeat CostTrackerLimit. +func TestRuntimeCostUnboundedOverloadSaturates(t *testing.T) { + unbounded := func(args []ref.Val, result ref.Val) *uint64 { + maxCost := uint64(math.MaxUint64) + return &maxCost + } + vars := []*decls.VariableDecl{ + decls.NewVariable("str1", types.StringType), + decls.NewVariable("str2", types.StringType), + } + in := map[string]any{"str1": "val1", "str2": "val2222222"} + + tests := []struct { + name string + expr string + }{ + // Each expression accrues cost from the variable references before the unbounded + // call is observed, and continues to accrue cost after it in the last two cases. + {name: "unbounded call", expr: `"abcdefg".contains(str1 + str2)`}, + {name: "unbounded call in conjunction", expr: `str1 != "" && "abcdefg".contains(str1 + str2)`}, + {name: "unbounded call then comparison", expr: `"abcdefg".contains(str1 + str2) == true`}, + {name: "unbounded call in list", expr: `["abcdefg".contains(str1 + str2), true]`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + opts := []CostTrackerOption{OverloadCostTracker(overloads.ContainsString, unbounded)} + actualCost, _, err := computeCost(t, tc.expr, vars, constructActivation(t, in), opts) + if err != nil { + t.Fatalf("computeCost() failed: %v", err) + } + if actualCost != math.MaxUint64 { + t.Errorf("computeCost() got cost %d, wanted %d", actualCost, uint64(math.MaxUint64)) + } + + // The saturated cost must also trip a cost limit. + limitOpts := []CostTrackerOption{ + OverloadCostTracker(overloads.ContainsString, unbounded), + CostTrackerLimit(1000), + } + _, _, err = computeCost(t, tc.expr, vars, constructActivation(t, in), limitOpts) + if err == nil { + t.Error("computeCost() with a cost limit got nil error, wanted cost limit exceeded") + } + }) + } +}