security: replace eval() in CodeAct constructor-string coercion with AST decoder - #35
Merged
rdasilveiracabral merged 2 commits intoJul 28, 2026
Conversation
…AST decoder
`_maybe_eval_constructor_string()` repaired malformed model output that
supplied a constructor call as a string (e.g. `MyData(data=df)`) by running
it through `eval()` with the session REPL locals as the namespace. This turned
a data-normalization path into a parent-process code-execution path, bypassing
the code validator, `execute_python` middleware, timeout, and cell sandbox.
Setting `__builtins__` to `{}` is insufficient: an attacker can invoke an
object planted by an earlier cell, or walk attributes of the trusted return
type to recover builtins.
Replace `eval()` with a small AST-based data-expression decoder:
- The outer callable must be the exact trusted return type (or a session-local
alias identical to it).
- Bare argument names resolve directly from `session_locals` by dictionary
lookup, preserving pass-by-reference / object identity (so
`return_result("MyData(data=df)")` still hands over the exact `df` object
without copying or serialization).
- Literal lists/tuples/sets/dicts are rebuilt element-by-element and may hold
those references; `*args`/`**kwargs` expansion is limited to exact
list/tuple/dict.
- A fixed allowlist of deterministic helpers (`min`, `max`, `sum`, ...) is
retained but operates only on detached exact-built-in data.
- Attribute access, arbitrary calls, and executable session-local objects are
rejected; a rejected expression falls through to existing validation/retry.
No `eval()` or namespace loading remains in the coercion path. Adds tests for
the closed attack routes and for reference-preserving compatibility.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add unit tests for intended functionality not previously exercised: - `**kwargs` dict expansion: supported, preserves value references, and rejects non-string keys / non-dict expansions. - `*args` expansion is limited to exact list/tuple; a set subclass is not iterated (custom `__iter__` never runs). - Session-local aliases identical to the return type are accepted; a name that shadows the return type still constructs the real return type, not the shadow. - Non-class return types skip coercion. - Allowlisted helpers work without a session-locals entry, and helper arguments are reduced to plain data first so custom iteration/conversion hooks never run. - Set literals decode to plain data; literal dict `**` expansion merges an exact session dict; general arithmetic is rejected while signed/complex literals are accepted. - `_copy_constructor_data` returns detached copies and rejects custom objects and container subclasses. Full suite: 6469 passed, 4 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a parent-process code-execution path in the CodeAct strategy. When a model returns its answer as a constructor-call string (e.g.
return_result("MyData(data=df)")),_maybe_eval_constructor_string()previously repaired it witheval()against the session REPL locals. This turned a data-normalization path into arbitrary code execution in the parent process — bypassing the code validator,execute_pythonmiddleware, timeout, and cell sandbox.__builtins__ = {}is not sufficient protection: an attacker can invoke an object planted by an earlier cell, or walk attributes of the trusted return type to recover builtins.What changed
eval()is replaced with a small AST-based data-expression decoder:session_locals, preserving pass-by-reference / object identity — soreturn_result("MyData(data=df)")still hands over the exactdfobject (no copy, no serialization). A bare-name lookup does not access, convert, call, or iterate the object.*args/**kwargsexpansion is restricted to exactlist/tuple/dict.min,max,sum,len,sorted, ...) is retained, but they operate only on detached exact-built-in data.No
eval()or namespace loading remains in the coercion path.Compatibility
Recovery behavior that affects agent quality is preserved: literal Pydantic constructor strings still validate on the same turn (integration test asserts one LLM call), opaque constructors work, arbitrary objects pass by reference, and common
min(...)expressions remain accepted. Existing direct-JSON results never enter this path.Tests
Adds coverage for the closed attack routes (local factory not called, callable arg not executed, return-type reflection ladder blocked) and for reference-preserving compatibility (direct/nested/mapping refs, Pydantic arbitrary-type field,
*argsexpansion).🤖 Generated with Claude Code