Make sandboxes pluggable and add a Lua sandbox - #15
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors Legion’s code execution into a pluggable Legion.Sandbox behaviour, extracts shared resource-limiting execution into Legion.Sandbox.Runner, and introduces a new Lua sandbox (Legion.Sandbox.Lua) backed by luerl with a tool-bridging layer.
Changes:
- Introduce
Legion.Sandboxbehaviour + move the existing Elixir implementation toLegion.Sandbox.Elixirand update executor/prompt plumbing to usesandbox.prompt_info/0,check/2,execute/5, andbinding_names/1. - Add
Legion.Sandbox.Luawith Lua constraints/prompt text, tool bridging, persistent Lua-state bindings, and tests for safety/bridge semantics. - Improve persistence footprint by compressing stored conversation snapshots in the Postgres store.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/legion/sandbox/lua_test.exs | Adds coverage for Lua eval, bindings, tool bridge conversions, and sandbox escape blocks. |
| test/legion/sandbox/elixir_test.exs | Updates tests to target Legion.Sandbox.Elixir and adds bindings round-trip assertion. |
| test/legion/sandbox/ast_checker/rce_attack_vectors_test.exs | Updates sandbox aliasing to point at Legion.Sandbox.Elixir. |
| test/legion/sandbox/ast_checker_test.exs | Adds regression tests for struct-forging vectors via Map.* callback APIs. |
| test/legion/parallel_and_pipeline_test.exs | Verifies AgentTool.parallel/2 accepts Lua-bridge list pairs. |
| test/legion/executor_test.exs | Ensures sandbox selection affects schema/prompt and parse rejection handling. |
| test/legion/agent_server_test.exs | Verifies Lua bindings persistence across turns in :conversation scope. |
| README.md | Updates sandbox feature description and documents the new sandbox config key. |
| mix.lock | Locks new dependencies for Lua support (lua, luerl). |
| mix.exs | Adds the :lua dependency and includes the new Sandboxes guide in ExDoc extras. |
| lib/legion/tools/agent_tool.ex | Normalizes [agent, task] pairs to tuples for Lua bridge compatibility. |
| lib/legion/store/postgres.ex | Compresses persisted snapshots via :erlang.term_to_binary/2. |
| lib/legion/sandbox/runner.ex | New shared runner enforcing timeout/heap/reduction limits consistently across sandboxes. |
| lib/legion/sandbox/lua/constraints.eex | Lua-specific constraint text injected into the system prompt. |
| lib/legion/sandbox/lua.ex | Implements the Lua sandbox: parse check, tool bridging, bindings as luerl state, GC sweep. |
| lib/legion/sandbox/elixir/constraints.eex | Elixir-specific constraint text extracted for prompt injection. |
| lib/legion/sandbox/elixir.ex | New Elixir sandbox module wrapping AST checks + Runner execution. |
| lib/legion/sandbox/ast_checker.ex | Updates docs and tightens Map.* allowlist to prevent :__struct__ leakage routes. |
| lib/legion/sandbox.ex | Converts prior module into the Legion.Sandbox behaviour contract + docs. |
| lib/legion/prompts/system_prompt.eex | Generalizes prompt template to be language-agnostic and sandbox-driven. |
| lib/legion/executor.ex | Routes validation/execution/binding display through the selected sandbox module. |
| lib/legion/agent.ex | Documents the new sandbox configuration key for agents. |
| lib/legion/agent_server.ex | Allows sandbox as a known config key. |
| lib/legion/agent_prompt.ex | Pulls language/constraints/tool-usage from sandbox.prompt_info/0. |
| guides/sandboxes.md | New guide explaining sandbox selection, tradeoffs, bridge semantics, and limits. |
| CHANGELOG.md | Documents the new pluggable sandboxes + Lua sandbox addition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5ec0e9d to
1c81c02
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
lib/legion/tools/agent_tool.ex:216
pipeline/1has the same silent-shape problem asparallel/2: thefor {agent, _} <- stepscomprehension skips non-tuples, so invalid step entries can bypass validation and fail later insideLegion.pipeline/1. Validating step shapes up front yields clearer errors and avoids surprising behavior.
def pipeline(steps) when is_list(steps) do
steps = Enum.map(steps, &normalize_pair/1)
for {agent, _} <- steps, do: check_allowed!(agent)
Legion.pipeline(steps)
lib/legion/tools/agent_tool.ex:204
parallel/2normalizes[agent, task]pairs, but the subsequent comprehension only checks items that match{agent, _task}; any malformed element is silently skipped and then forwarded toLegion.parallel/2, which can fail later with a harder-to-debug error. It’s better to validate every entry and raise a clearArgumentErrorwhen an entry isn’t a{module, task}pair.
This issue also appears on line 213 of the same file.
def parallel(tasks, timeout \\ :infinity) when is_list(tasks) do
tasks = Enum.map(tasks, &normalize_pair/1)
for {agent, _task} <- tasks, do: check_allowed!(agent)
Legion.parallel(tasks, timeout)
end
lib/legion/agent.ex:45
- The Agent config docs describe
Legion.Sandbox.Luaas running in a “pure-Erlang VM”, but this PR adds:lua(tv-labs/lua), which is a Lua VM written in pure Elixir. Updating this avoids confusing readers about the dependency and threat model.
- `sandbox` — a `Legion.Sandbox` module that validates and evaluates the
code the agent writes. `Legion.Sandbox.Elixir` (the default) evaluates
Elixir behind an AST allowlist; `Legion.Sandbox.Lua` evaluates Lua in a
pure-Erlang VM where only bridged tool functions can reach the host
(default: `Legion.Sandbox.Elixir`)
7eb0d8b to
0a645a8
Compare
| | | `Legion.Sandbox.Elixir` | `Legion.Sandbox.Lua` | | ||
| |---|---|---| | ||
| | Language | Elixir minus denied forms | Lua 5.3 semantics | | ||
| | Stdlib | Allowlisted `Enum`, `String`, `Map`, `Date`/`DateTime`, `Regex`, `JSON`, `URI`, `:math`, ... | Lua's `string`, `table`, `math`; `os.time`/`os.date` (`io`, `file`, `os.getenv`/`os.execute`, `require`, `load`, `print` are blocked) | |
There was a problem hiding this comment.
Nit: is block the right word here? more like "not allowed"?
There was a problem hiding this comment.
I think not allowed === blocked ;)
| Both sandboxes run under the same `Legion.Sandbox.Runner` (timeout, | ||
| `max_heap` plus off-heap binary polling, `max_reductions`, priority), so |
There was a problem hiding this comment.
Nit: why are some parameters specified as codeblocks, while others are normal text?
There was a problem hiding this comment.
Priority is not configurable by the user - that's the reason.
0a645a8 to
868fab8
Compare
There was a problem hiding this comment.
Shouldn't this be moved under Legion.Sandbox.Elixir? I don't see any uses inside lua sandbox; and the doc references Elixir sandbox only
| def execute(code_string, timeout_ms, allowed_modules \\ [], bindings \\ [], limits \\ []) | ||
| when is_binary(code_string) and is_list(allowed_modules) and is_list(limits) and | ||
| (is_integer(timeout_ms) or timeout_ms == :infinity) do | ||
| with :ok <- ASTChecker.check(code_string, allowed_modules) do |
| conversation, upserted on every save. Step snapshots therefore require no additional migration. | ||
|
|
There was a problem hiding this comment.
Nit: formatting
| conversation, upserted on every save. Step snapshots therefore require no additional migration. | |
| Agent ids must be strings. Snapshots are stored as compressed | |
| `:erlang.term_to_binary/2` blobs - readable only from Elixir, one row | |
| per conversation, upserted on every save. Step snapshots therefore require | |
| no additional migration. | |
|
legion/lib/legion/eval_guard.ex Lines 5 to 8 in 0a645a8 We shouldn't mention ASTChecker, since it's not present in general (only in Elixir sandbox) |
tom-ehh
left a comment
There was a problem hiding this comment.
LGTM! Bar some small nits
Code execution becomes a
Legion.Sandboxbehaviour (check,execute,binding_names,prompt_info) - a sandbox owns static validation, evaluation, how bindings persist between executions, and the language-specific parts of the system prompt. The existing Elixir eval moves toLegion.Sandbox.Elixir, stays the default, and behaves as before.Legion.Sandbox.Luais the first alternative: Lua via lua, a Lua 5.3 VM written in pure Elixir. The Elixir sandbox has to deny-list its way around the whole language surface, and new escape vectors in that surface keep turning up. Lua inverts the model - nothing inside the VM can reach the host BEAM except the tool functions explicitly bridged in, which makes it the safer choice for less trusted code.Tools are bridged as global Lua tables (
EchoTool.add(1, 2)), with arguments and results converted between Lua tables and Elixir maps/lists. Bindings are the Lua state: globals persist across executions, and the state survives the store round-trip. Evaluation runs through the sameLegion.Sandbox.Runner, so timeout, memory, and CPU limits behave identically in both languages. The VM has no state garbage collector yet, so dead tables from an eval stay in the state until upstream ships one; per-eval growth is bounded by the memory limit, and persisted snapshots are compressed.Select per agent:
or globally: