Skip to content

Commit 832278d

Browse files
authored
Fix unbounded arena growth in CEL Evaluator (#831)
The Evaluator holds a single `google::protobuf::Arena` that was shared across every `CompileAndEvaluate` call. Due to Arena being a monotonic bump allocator that never frees individual allocations, this caused us to leak compilation temporaries and materialized variable values (e.g. args, envs etc. ) on each evaluation. * `CompileAndEvaluate()` now creates a stack-local Arena that is destroyed at end of scope, freeing all temporaries. * Updated helpers to take an arena as an argument and pass in the stack-local arena. Added tests to ensure this actually fixes the problem and does not regress. Before these changes my tests showed 600MB+ of leak after 5000 iterations after these changes it's constant.
1 parent c13da6b commit 832278d

7 files changed

Lines changed: 203 additions & 27 deletions

File tree

Source/common/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,6 +1111,7 @@ test_suite(
11111111
":ScopedFileTest",
11121112
":ScopedIOObjectRefTest",
11131113
":TelemetryEventMapTest",
1114+
"//Source/common/cel:ArenaGrowthTest",
11141115
"//Source/common/cel:CELTest",
11151116
"//Source/common/faa:unit_tests",
11161117
"//Source/common/ne:unit_tests",
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/// Copyright 2026 North Pole Security, Inc.
2+
///
3+
/// Licensed under the Apache License, Version 2.0 (the "License");
4+
/// you may not use this file except in compliance with the License.
5+
/// You may obtain a copy of the License at
6+
///
7+
/// https://www.apache.org/licenses/LICENSE-2.0
8+
///
9+
/// Unless required by applicable law or agreed to in writing, software
10+
/// distributed under the License is distributed on an "AS IS" BASIS,
11+
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
/// See the License for the specific language governing permissions and
13+
/// limitations under the License.
14+
15+
#include "Source/common/cel/Activation.h"
16+
#include "Source/common/cel/CELProtoTraits.h"
17+
#include "Source/common/cel/Evaluator.h"
18+
19+
#import <Foundation/Foundation.h>
20+
#import <XCTest/XCTest.h>
21+
22+
#include <mach/mach.h>
23+
#include <cstddef>
24+
#include <string>
25+
#include <vector>
26+
27+
#include "absl/status/statusor.h"
28+
29+
static size_t GetResidentMemoryBytes() {
30+
mach_task_basic_info_data_t info;
31+
mach_msg_type_number_t count = MACH_TASK_BASIC_INFO_COUNT;
32+
if (task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&info, &count) !=
33+
KERN_SUCCESS) {
34+
return 0;
35+
}
36+
return info.resident_size;
37+
}
38+
39+
@interface ArenaGrowthTest : XCTestCase
40+
@end
41+
42+
@implementation ArenaGrowthTest
43+
44+
/// Regression test for unbounded arena growth in CompileAndEvaluate.
45+
///
46+
/// Previously, the Evaluator used a single protobuf Arena for every
47+
/// compile+evaluate cycle. Since Arena is a monotonic bump allocator that never
48+
/// frees individual allocations, every evaluation leaked materialized variable
49+
/// values (args strings, env maps, etc.) and compilation temporaries.
50+
///
51+
/// The fix uses a stack-local Arena in CompileAndEvaluate so temporaries are
52+
/// freed at end of scope. This test verifies memory stays bounded.
53+
- (void)testCompileAndEvaluateArenaGrowth {
54+
using ExecutableFileT = santa::cel::CELProtoTraits<true>::ExecutableFileT;
55+
using AncestorT = santa::cel::CELProtoTraits<true>::AncestorT;
56+
57+
auto sut = santa::cel::Evaluator<true>::Create();
58+
if (!sut.ok()) {
59+
XCTFail(@"Failed to create evaluator: %.*s", (int)sut.status().message().size(),
60+
sut.status().message().data());
61+
return;
62+
}
63+
64+
// Build a large args list to amplify per-evaluation arena allocation.
65+
// This simulates a process like `node` or `yarn` with many arguments.
66+
std::vector<std::string> largeArgs;
67+
largeArgs.reserve(200);
68+
for (int i = 0; i < 200; i++) {
69+
largeArgs.push_back(std::string(256, 'a' + (i % 26)));
70+
}
71+
const auto *argsPtr = &largeArgs;
72+
73+
// Expression that forces materialization of the full args list onto the arena.
74+
absl::string_view expr = "args.exists(x, x == 'nonexistent_value')";
75+
76+
// Warm up: run a few iterations to stabilize RSS and JIT/lazy allocations.
77+
for (int i = 0; i < 10; i++) {
78+
@autoreleasepool {
79+
auto f = std::make_unique<ExecutableFileT>();
80+
f->mutable_signing_time()->set_seconds(1748436989);
81+
santa::cel::Activation<true> activation(
82+
std::move(f),
83+
^std::vector<std::string>() {
84+
return *argsPtr;
85+
},
86+
^std::map<std::string, std::string>() {
87+
return {};
88+
},
89+
^uid_t() {
90+
return 501;
91+
},
92+
^std::string() {
93+
return "/usr/local/bin";
94+
},
95+
^std::vector<AncestorT>() {
96+
return {};
97+
});
98+
auto result = sut.value()->CompileAndEvaluate(expr, activation);
99+
if (!result.ok()) {
100+
XCTFail(@"Warmup failed: %.*s", (int)result.status().message().size(),
101+
result.status().message().data());
102+
return;
103+
}
104+
}
105+
}
106+
107+
size_t rssBaseline = GetResidentMemoryBytes();
108+
XCTAssertGreaterThan(rssBaseline, (size_t)0, @"Failed to read RSS");
109+
110+
// Run many iterations of CompileAndEvaluate. Each iteration materializes
111+
// ~200 * 256 = ~50KB of arg strings onto the arena, plus compilation
112+
// temporaries. Before the fix this grew by ~600MB+ over 5000 iterations.
113+
const int iterations = 5000;
114+
for (int i = 0; i < iterations; i++) {
115+
@autoreleasepool {
116+
auto f = std::make_unique<ExecutableFileT>();
117+
f->mutable_signing_time()->set_seconds(1748436989);
118+
santa::cel::Activation<true> activation(
119+
std::move(f),
120+
^std::vector<std::string>() {
121+
return *argsPtr;
122+
},
123+
^std::map<std::string, std::string>() {
124+
return {{"PATH", "/usr/bin"}, {"HOME", "/Users/test"}};
125+
},
126+
^uid_t() {
127+
return 501;
128+
},
129+
^std::string() {
130+
return "/usr/local/bin";
131+
},
132+
^std::vector<AncestorT>() {
133+
return {};
134+
});
135+
auto result = sut.value()->CompileAndEvaluate(expr, activation);
136+
if (!result.ok()) {
137+
XCTFail(@"Iteration %d failed: %.*s", i, (int)result.status().message().size(),
138+
result.status().message().data());
139+
return;
140+
}
141+
}
142+
}
143+
144+
size_t rssAfter = GetResidentMemoryBytes();
145+
size_t growth = rssAfter > rssBaseline ? rssAfter - rssBaseline : 0;
146+
147+
double growthMB = (double)growth / (1024.0 * 1024.0);
148+
NSLog(@"Arena growth test: baseline=%.1fMB, after=%.1fMB, growth=%.1fMB over %d iterations",
149+
(double)rssBaseline / (1024.0 * 1024.0), (double)rssAfter / (1024.0 * 1024.0), growthMB,
150+
iterations);
151+
152+
// Threshold: 50MB is generous enough to avoid flakiness from normal heap
153+
// activity, but will clearly catch unbounded arena growth (~600MB before fix).
154+
double thresholdMB = 50.0;
155+
XCTAssertLessThan(growthMB, thresholdMB,
156+
@"CompileAndEvaluate leaked %.1fMB over %d iterations "
157+
@"(threshold: %.0fMB). This indicates unbounded arena growth.",
158+
growthMB, iterations, thresholdMB);
159+
}
160+
161+
@end

Source/common/cel/BUILD

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,13 @@ santa_unit_test(
8484
"@northpolesec_protos//celv2:v2_cc_proto",
8585
],
8686
)
87+
88+
santa_unit_test(
89+
name = "ArenaGrowthTest",
90+
srcs = ["ArenaGrowthTest.mm"],
91+
deps = [
92+
":CEL",
93+
"@abseil-cpp//absl/status:statusor",
94+
"@northpolesec_protos//celv2:v2_cc_proto",
95+
],
96+
)

Source/common/cel/Evaluator.h

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class Evaluator {
6565

6666
Evaluator(std::unique_ptr<::cel::Compiler> compiler,
6767
std::unique_ptr<google::protobuf::Arena> arena)
68-
: arena_(std::move(arena)), compiler_(std::move(compiler)) {};
68+
: compiler_(std::move(compiler)), compiler_arena_(std::move(arena)) {};
6969
~Evaluator() = default;
7070

7171
Evaluator(Evaluator &&other) = default;
@@ -76,25 +76,26 @@ class Evaluator {
7676
Evaluator &operator=(const Evaluator &other) = delete;
7777

7878
// Compile a CEL expression from a string into an expression plan
79-
// ready for evaluation. These expression plans could be cached but it's
80-
// important that the compiled expression is not used after the Evaluator
81-
// is destroyed.
79+
// ready for evaluation. The caller-provided arena is used for constant
80+
// folding and must outlive the returned expression plan.
8281
absl::StatusOr<std::unique_ptr<::google::api::expr::runtime::CelExpression>>
83-
Compile(absl::string_view cel_expr);
82+
Compile(absl::string_view cel_expr, google::protobuf::Arena *arena);
8483

85-
// Evaluate an expression plan with a SantaActivation object.
84+
// Evaluate an expression plan with a SantaActivation object. The
85+
// caller-provided arena is used for evaluation temporaries.
8686
absl::StatusOr<EvaluationResultT> Evaluate(
8787
::google::api::expr::runtime::CelExpression const *expression_plan,
88-
const ActivationT &activation);
88+
const ActivationT &activation, google::protobuf::Arena *arena);
8989

90-
// Convenience method that combines Compile() and Evaluate() into a single
91-
// call.
90+
// Compile and evaluate a CEL expression in a single call. Uses a
91+
// stack-local arena internally so no allocations persist after return.
9292
absl::StatusOr<EvaluationResultT> CompileAndEvaluate(
9393
absl::string_view cel_expr, const ActivationT &activation);
9494

9595
private:
96-
std::unique_ptr<google::protobuf::Arena> arena_;
9796
std::unique_ptr<::cel::Compiler> compiler_;
97+
std::unique_ptr<google::protobuf::Arena>
98+
compiler_arena_; // Kept alive for compiler type refs
9899
};
99100

100101
} // namespace cel

Source/common/cel/Evaluator.mm

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@
9393

9494
template <bool IsV2>
9595
absl::StatusOr<std::unique_ptr<Evaluator<IsV2>>> Evaluator<IsV2>::Create() {
96-
std::unique_ptr<google::protobuf::Arena> arena = std::make_unique<google::protobuf::Arena>();
96+
auto arena = std::make_unique<google::protobuf::Arena>();
9797

9898
auto compiler = CreateCompiler<IsV2>(arena.get());
9999
if (!compiler.ok()) {
@@ -104,7 +104,7 @@
104104

105105
template <bool IsV2>
106106
absl::StatusOr<std::unique_ptr<::cel_runtime::CelExpression>> Evaluator<IsV2>::Compile(
107-
absl::string_view expr) {
107+
absl::string_view expr, google::protobuf::Arena *arena) {
108108
if (!compiler_) {
109109
return absl::InvalidArgumentError("Evaluator not properly initialized");
110110
}
@@ -127,7 +127,7 @@
127127
// Setup a default environment for building expressions.
128128
cel_runtime::InterpreterOptions options;
129129
options.constant_folding = true;
130-
options.constant_arena = arena_.get();
130+
options.constant_arena = arena;
131131
options.enable_regex_precompilation = true;
132132

133133
std::unique_ptr<cel_runtime::CelExpressionBuilder> builder =
@@ -158,14 +158,13 @@
158158
builder->CreateExpression(&cel_expr);
159159

160160
return expression_plan;
161-
};
161+
}
162162

163163
template <bool IsV2>
164164
absl::StatusOr<typename Evaluator<IsV2>::EvaluationResultT> Evaluator<IsV2>::Evaluate(
165-
const ::cel_runtime::CelExpression *expression_plan, const ActivationT &activation) {
166-
// Evaluate the parsed expression.
167-
absl::StatusOr<cel_runtime::CelValue> result =
168-
expression_plan->Evaluate(activation, arena_.get());
165+
const ::cel_runtime::CelExpression *expression_plan, const ActivationT &activation,
166+
google::protobuf::Arena *arena) {
167+
absl::StatusOr<cel_runtime::CelValue> result = expression_plan->Evaluate(activation, arena);
169168

170169
if (!result.ok()) {
171170
return result.status();
@@ -219,11 +218,12 @@
219218
template <bool IsV2>
220219
absl::StatusOr<typename Evaluator<IsV2>::EvaluationResultT> Evaluator<IsV2>::CompileAndEvaluate(
221220
absl::string_view cel_expr, const ActivationT &activation) {
222-
absl::StatusOr<std::unique_ptr<::cel_runtime::CelExpression>> expr = Compile(cel_expr);
221+
google::protobuf::Arena arena;
222+
auto expr = Compile(cel_expr, &arena);
223223
if (!expr.ok()) {
224224
return expr.status();
225225
}
226-
return Evaluate(expr->get(), activation);
226+
return Evaluate(expr->get(), activation, &arena);
227227
}
228228

229229
// Explicit template instantiations

Source/common/cel/Test.mm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,14 @@ - (void)testBasic {
9090
}
9191
{
9292
// Re-use of a compiled expression.
93-
auto expr = sut.value()->Compile("target.signing_time >= timestamp('2025-05-28T12:00:00Z')");
93+
google::protobuf::Arena arena;
94+
auto expr =
95+
sut.value()->Compile("target.signing_time >= timestamp('2025-05-28T12:00:00Z')", &arena);
9496
if (!expr.ok()) {
9597
XCTFail("Failed to compile: %s", expr.status().message().data());
9698
}
9799

98-
auto result = sut.value()->Evaluate(expr.value().get(), activation);
100+
auto result = sut.value()->Evaluate(expr.value().get(), activation, &arena);
99101
if (!result.ok()) {
100102
XCTFail("Failed to evaluate: %s", result.status().message().data());
101103
} else {
@@ -123,7 +125,7 @@ - (void)testBasic {
123125
return {};
124126
});
125127

126-
auto result2 = sut.value()->Evaluate(expr.value().get(), activation2);
128+
auto result2 = sut.value()->Evaluate(expr.value().get(), activation2, &arena);
127129
if (!result2.ok()) {
128130
XCTFail("Failed to evaluate: %s", result2.status().message().data());
129131
} else {

Source/santad/DataLayer/SNTRuleTable.mm

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,12 +576,12 @@ - (BOOL)addExecutionRules:(NSArray<SNTRule *> *)executionRules
576576
}
577577

578578
if (rule.state == SNTRuleStateCEL || rule.state == SNTRuleStateCELv2) {
579+
google::protobuf::Arena arena;
579580
absl::StatusOr<std::unique_ptr<::google::api::expr::runtime::CelExpression>> celExpr;
580-
581581
if (rule.state == SNTRuleStateCEL && _celEvaluator != nullptr) {
582-
celExpr = _celEvaluator->Compile(santa::NSStringToUTF8StringView(rule.celExpr));
582+
celExpr = _celEvaluator->Compile(santa::NSStringToUTF8StringView(rule.celExpr), &arena);
583583
} else if (rule.state == SNTRuleStateCELv2 && _celV2Evaluator != nullptr) {
584-
celExpr = _celV2Evaluator->Compile(santa::NSStringToUTF8StringView(rule.celExpr));
584+
celExpr = _celV2Evaluator->Compile(santa::NSStringToUTF8StringView(rule.celExpr), &arena);
585585
}
586586
if (!celExpr.ok()) {
587587
[errors addObject:
@@ -854,7 +854,8 @@ - (void)updateStaticRules:(NSArray<NSDictionary *> *)staticRules {
854854
if (!r) continue;
855855

856856
if (r.state == SNTRuleStateCEL && _celEvaluator) {
857-
auto celExpr = _celEvaluator->Compile(santa::NSStringToUTF8StringView(r.celExpr));
857+
google::protobuf::Arena arena;
858+
auto celExpr = _celEvaluator->Compile(santa::NSStringToUTF8StringView(r.celExpr), &arena);
858859
if (!celExpr.ok()) {
859860
LOGE(@"Failed to compile CEL expression for static rule %@: %s", r.identifier,
860861
std::string(celExpr.status().message()).c_str());

0 commit comments

Comments
 (0)