Background
Condition.expr(expr_string) in burr/core/action.py is the compact-DSL way to express a transition guard against state — e.g. expr("step_count > 10"). Today it parses the string with ast.parse(), compiles it, and eval()s it against state.get_all() as locals.
This is a deliberately full-power evaluator: it accepts arbitrary Python in the expression. The docstring already notes:
"Do not accept expressions generated from user-inputted text."
That contract is appropriate for the intended use case — the framework author writes the expression as part of the graph definition, the same way they could write a lambda. There's no plan to change the default behavior.
The gap
There's a real second use case where users want to accept expressions from a less-trusted source (e.g. a dashboard config, a SaaS user-facing rule editor, a YAML-driven graph definition). For that case, Burr currently offers no safe surface — they'd have to wrap expr() in their own AST-validating layer.
Proposal
Provide an opt-in safe AST evaluator as a separate entry point. Sketch:
Condition.safe_expr(expr_string) — new public API, restricted-AST evaluator.
- Implementation walks the parsed
ast.AST with a NodeVisitor and rejects anything not on an allowlist.
- Allowed node types (rough cut, to refine in design):
- Constants (
int, float, str, bool, None)
- Names (resolved against state)
- Attribute access on Names (but reject any
__dunder__ attribute access)
- Subscript access (
state["key"], items[0])
- Comparisons (
==, !=, <, >, <=, >=, in, not in, is, is not)
- Boolean ops (
and, or, not)
- Arithmetic on numbers (
+, -, *, /, //, %)
- Tuple/list/set/dict literals
- Calls to a small allowlist of safe builtins:
len, abs, min, max, sum, all, any, str, int, float, bool (return-typed, no side effects)
- Explicitly rejected:
- Any other
Call node
Lambda, IfExp, comprehensions (ListComp, DictComp, SetComp, GeneratorExp)
Import/ImportFrom
Subscript slicing with Slice if we want to be conservative (TBD)
- Walrus (
NamedExpr) — TBD
- Dunder attribute access on any node
Evaluation strategy: walk the validated tree and interpret directly, rather than calling eval() on the validated source. This removes the dependency on CPython's eval() semantics entirely.
Out of scope for this issue
- Changing the default behavior of
Condition.expr(). It stays as today (full-power, author-trusted).
- Adding a new dependency (e.g.
simpleeval, asteval). The implementation should be in-tree.
Acceptance criteria
- New
safe_expr (or equivalently named) public API on Condition.
- Existing test patterns for
expr() ported to safe_expr to confirm the common usage works.
- Negative tests proving each rejected node category raises a clear
SyntaxError / ValueError at parse time, not at eval time.
- Docstring on
expr() updated to clearly contrast the two surfaces.
- Worth a CHANGELOG entry as a feature.
Rough sizing
Focused module of new code, defined contract, no live design choices once node-allowlist is settled — ~300–600 LOC including tests. Reviewable in ~20 minutes by someone who knows the AST module.
Background
Condition.expr(expr_string)inburr/core/action.pyis the compact-DSL way to express a transition guard against state — e.g.expr("step_count > 10"). Today it parses the string withast.parse(), compiles it, andeval()s it againststate.get_all()as locals.This is a deliberately full-power evaluator: it accepts arbitrary Python in the expression. The docstring already notes:
That contract is appropriate for the intended use case — the framework author writes the expression as part of the graph definition, the same way they could write a lambda. There's no plan to change the default behavior.
The gap
There's a real second use case where users want to accept expressions from a less-trusted source (e.g. a dashboard config, a SaaS user-facing rule editor, a YAML-driven graph definition). For that case, Burr currently offers no safe surface — they'd have to wrap
expr()in their own AST-validating layer.Proposal
Provide an opt-in safe AST evaluator as a separate entry point. Sketch:
Condition.safe_expr(expr_string)— new public API, restricted-AST evaluator.ast.ASTwith aNodeVisitorand rejects anything not on an allowlist.int,float,str,bool,None)__dunder__attribute access)state["key"],items[0])==,!=,<,>,<=,>=,in,not in,is,is not)and,or,not)+,-,*,/,//,%)len,abs,min,max,sum,all,any,str,int,float,bool(return-typed, no side effects)CallnodeLambda,IfExp, comprehensions (ListComp,DictComp,SetComp,GeneratorExp)Import/ImportFromSubscriptslicing withSliceif we want to be conservative (TBD)NamedExpr) — TBDEvaluation strategy: walk the validated tree and interpret directly, rather than calling
eval()on the validated source. This removes the dependency on CPython'seval()semantics entirely.Out of scope for this issue
Condition.expr(). It stays as today (full-power, author-trusted).simpleeval,asteval). The implementation should be in-tree.Acceptance criteria
safe_expr(or equivalently named) public API onCondition.expr()ported tosafe_exprto confirm the common usage works.SyntaxError/ValueErrorat parse time, not at eval time.expr()updated to clearly contrast the two surfaces.Rough sizing
Focused module of new code, defined contract, no live design choices once node-allowlist is settled — ~300–600 LOC including tests. Reviewable in ~20 minutes by someone who knows the AST module.