mirror of
https://github.com/calesthio/OpenMontage.git
synced 2026-08-13 20:27:11 +08:00
fix(math_animate): gate caller-supplied scene_code execution (#219)
math_animate writes caller-supplied Python to scene.py and runs Manim on it — arbitrary local code execution with no boundary surfaced in the tool contract. In an agent-driven system the scene_code may be LLM-generated or influenced by untrusted prompt content, so import-time code or construct() could read secrets/SSH material, open network connections, or spawn subprocesses. Add a static AST safety scan that rejects dangerous imports (os, subprocess, socket, requests, ctypes, ...), dangerous builtins (eval/exec/compile/open/ __import__), and sandbox-escape dunders (__globals__, __subclasses__, ...) before Manim runs. Genuine math scenes (manim, numpy, math, ...) pass untouched. This is defense-in-depth, not a sandbox: a determined attacker can evade a static denylist, so it is paired with an explicit allow_unsafe_code opt-out and a tool contract (schema + side_effects) that names the boundary. Closes #219
This commit is contained in:
0xDevNinja
122
tests/tools/test_math_animate_safety.py
Normal file
122
tests/tools/test_math_animate_safety.py
Normal file
@@ -0,0 +1,122 @@
|
||||
"""Tests for math_animate scene_code safety scan (issue #219).
|
||||
|
||||
math_animate executes caller-supplied Python via Manim. The static scan blocks
|
||||
the constructs an attack needs (system/network/subprocess/secret access) while
|
||||
leaving genuine math-animation scenes untouched, and can be bypassed only with
|
||||
an explicit allow_unsafe_code opt-out.
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
|
||||
sys.path.insert(0, str(PROJECT_ROOT))
|
||||
|
||||
from tools.graphics.math_animate import MathAnimate # noqa: E402
|
||||
|
||||
SAFE_SCENE = (
|
||||
"from manim import *\n"
|
||||
"import numpy as np\n"
|
||||
"import math\n"
|
||||
"class Demo(Scene):\n"
|
||||
" def construct(self):\n"
|
||||
" self.play(Create(Circle(radius=np.pi / math.tau)))\n"
|
||||
)
|
||||
|
||||
|
||||
def test_safe_scene_passes_scan():
|
||||
assert MathAnimate._scan_scene_code(SAFE_SCENE) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"snippet, needle",
|
||||
[
|
||||
("import os\nos.environ", "import 'os'"),
|
||||
("import subprocess", "import 'subprocess'"),
|
||||
("import socket", "import 'socket'"),
|
||||
("from urllib.request import urlopen", "from 'urllib.request' import ..."),
|
||||
("import requests", "import 'requests'"),
|
||||
],
|
||||
)
|
||||
def test_blocks_dangerous_imports(snippet, needle):
|
||||
code = f"from manim import *\n{snippet}\nclass S(Scene):\n def construct(self):\n pass\n"
|
||||
violations = MathAnimate._scan_scene_code(code)
|
||||
assert needle in violations
|
||||
|
||||
|
||||
@pytest.mark.parametrize("call", ["eval", "exec", "compile", "open", "__import__"])
|
||||
def test_blocks_dangerous_calls(call):
|
||||
code = (
|
||||
"from manim import *\n"
|
||||
"class S(Scene):\n"
|
||||
" def construct(self):\n"
|
||||
f" {call}('x')\n"
|
||||
)
|
||||
assert f"call to '{call}()'" in MathAnimate._scan_scene_code(code)
|
||||
|
||||
|
||||
def test_blocks_sandbox_escape_dunders():
|
||||
code = (
|
||||
"from manim import *\n"
|
||||
"class S(Scene):\n"
|
||||
" def construct(self):\n"
|
||||
" ().__class__.__bases__[0].__subclasses__()\n"
|
||||
)
|
||||
violations = MathAnimate._scan_scene_code(code)
|
||||
assert "attribute access '.__bases__'" in violations
|
||||
assert "attribute access '.__subclasses__'" in violations
|
||||
|
||||
|
||||
def test_syntax_error_defers_to_manim():
|
||||
# A parse failure must not mask as a safety violation; Manim reports it.
|
||||
assert MathAnimate._scan_scene_code("class S(Scene):\n def construct(self)\n") == []
|
||||
|
||||
|
||||
def test_execute_blocks_dangerous_code_before_running_manim(monkeypatch):
|
||||
# Pretend manim is installed so execute() reaches the safety gate rather
|
||||
# than short-circuiting on a missing binary. The scan must reject before any
|
||||
# subprocess runs.
|
||||
monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/manim")
|
||||
|
||||
def boom(*a, **k): # subprocess must never be reached
|
||||
raise AssertionError("subprocess.run should not be called for blocked code")
|
||||
|
||||
monkeypatch.setattr("subprocess.run", boom)
|
||||
|
||||
dangerous = (
|
||||
"from manim import *\n"
|
||||
"import os\n"
|
||||
"class S(Scene):\n"
|
||||
" def construct(self):\n"
|
||||
" print(os.environ)\n"
|
||||
)
|
||||
result = MathAnimate().execute({"scene_code": dangerous})
|
||||
assert result.success is False
|
||||
assert "safety scan" in result.error
|
||||
assert "allow_unsafe_code" in result.error
|
||||
|
||||
|
||||
def test_allow_unsafe_code_bypasses_scan(monkeypatch):
|
||||
# With the opt-out, execution proceeds past the scan to Manim (which we stub
|
||||
# to fail); the failure must NOT be the safety-scan message.
|
||||
monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/manim")
|
||||
|
||||
class FakeProc:
|
||||
returncode = 1
|
||||
stderr = "manim ran"
|
||||
stdout = ""
|
||||
|
||||
monkeypatch.setattr("subprocess.run", lambda *a, **k: FakeProc())
|
||||
|
||||
dangerous = (
|
||||
"from manim import *\n"
|
||||
"import os\n"
|
||||
"class S(Scene):\n"
|
||||
" def construct(self):\n"
|
||||
" print(os.environ)\n"
|
||||
)
|
||||
result = MathAnimate().execute({"scene_code": dangerous, "allow_unsafe_code": True})
|
||||
assert result.success is False
|
||||
assert "safety scan" not in (result.error or "")
|
||||
@@ -6,6 +6,7 @@ using the Manim Community Edition engine. Free, local, no API key required.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
@@ -28,6 +29,31 @@ from tools.base_tool import (
|
||||
)
|
||||
|
||||
|
||||
# --- Safety: caller-supplied scene_code is a local code-execution boundary ---
|
||||
# math_animate runs Manim on Python supplied by the caller (often an LLM or
|
||||
# prompt-influenced reference material). That is arbitrary local code execution
|
||||
# (see issue #219). The static scan below is defense-in-depth: it blocks the
|
||||
# constructs an attack needs — reading secrets/SSH material, opening network
|
||||
# connections, spawning subprocesses — while leaving genuine math/animation
|
||||
# scenes untouched. It is NOT a security sandbox: a determined attacker can
|
||||
# evade a static denylist, so it is paired with an explicit `allow_unsafe_code`
|
||||
# opt-out and a tool contract that names the boundary. A passing scan is not
|
||||
# proof that code is safe to run.
|
||||
_BLOCKED_IMPORTS = frozenset({
|
||||
"os", "sys", "subprocess", "socket", "shutil", "requests", "urllib",
|
||||
"http", "ftplib", "smtplib", "telnetlib", "ctypes", "pickle", "marshal",
|
||||
"importlib", "builtins", "multiprocessing", "threading", "pty", "glob",
|
||||
"resource", "signal", "tempfile", "webbrowser", "pathlib",
|
||||
})
|
||||
_BLOCKED_CALLS = frozenset({
|
||||
"eval", "exec", "compile", "__import__", "open", "input", "breakpoint",
|
||||
})
|
||||
_BLOCKED_ATTRS = frozenset({
|
||||
"__globals__", "__builtins__", "__subclasses__", "__bases__", "__mro__",
|
||||
"__code__",
|
||||
})
|
||||
|
||||
|
||||
# Quality presets mapping to Manim CLI flags
|
||||
QUALITY_PRESETS = {
|
||||
"low": {"flag": "-ql", "resolution": "854x480", "fps": 15},
|
||||
@@ -76,7 +102,20 @@ class MathAnimate(BaseTool):
|
||||
"description": (
|
||||
"Python code defining a Manim scene. Must contain a class "
|
||||
"inheriting from Scene with a construct() method. "
|
||||
"Import 'from manim import *' is auto-added if missing."
|
||||
"Import 'from manim import *' is auto-added if missing. "
|
||||
"SECURITY: this code is EXECUTED on the host by Manim. It is "
|
||||
"scanned for dangerous constructs (system/network/subprocess "
|
||||
"access) and rejected by default; treat scene_code as trusted "
|
||||
"input only."
|
||||
),
|
||||
},
|
||||
"allow_unsafe_code": {
|
||||
"type": "boolean",
|
||||
"default": False,
|
||||
"description": (
|
||||
"Bypass the scene_code safety scan. Only set this for code "
|
||||
"you fully trust — it permits arbitrary local code execution "
|
||||
"(filesystem, network, subprocess). See issue #219."
|
||||
),
|
||||
},
|
||||
"scene_name": {
|
||||
@@ -117,7 +156,13 @@ class MathAnimate(BaseTool):
|
||||
)
|
||||
retry_policy = RetryPolicy(max_retries=1, retryable_errors=["timeout"])
|
||||
idempotency_key_fields = ["scene_code", "scene_name", "quality"]
|
||||
side_effects = ["writes video/image file to output_path", "creates temp files"]
|
||||
side_effects = [
|
||||
"EXECUTES caller-supplied Python (Manim scene_code) on the host — this "
|
||||
"is a local code-execution boundary; scene_code is scanned and rejected "
|
||||
"by default unless allow_unsafe_code=true (see issue #219)",
|
||||
"writes video/image file to output_path",
|
||||
"creates temp files",
|
||||
]
|
||||
user_visible_verification = [
|
||||
"Watch the animation for correctness and visual quality",
|
||||
"Verify math formulas render correctly (requires LaTeX)",
|
||||
@@ -160,6 +205,47 @@ class MathAnimate(BaseTool):
|
||||
result.duration_seconds = round(time.time() - start, 2)
|
||||
return result
|
||||
|
||||
@staticmethod
|
||||
def _scan_scene_code(code: str) -> list[str]:
|
||||
"""Static safety scan of caller-supplied Manim scene code (issue #219).
|
||||
|
||||
Returns a de-duplicated list of disallowed constructs (dangerous
|
||||
imports, builtins, and sandbox-escape dunders). Empty list means the
|
||||
scan found nothing to block — which is NOT a guarantee the code is safe.
|
||||
A syntax error is left for Manim to report, so it returns no violations.
|
||||
"""
|
||||
try:
|
||||
tree = ast.parse(code)
|
||||
except SyntaxError:
|
||||
return []
|
||||
|
||||
violations: list[str] = []
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
root = alias.name.split(".")[0]
|
||||
if root in _BLOCKED_IMPORTS:
|
||||
violations.append(f"import '{alias.name}'")
|
||||
elif isinstance(node, ast.ImportFrom):
|
||||
root = (node.module or "").split(".")[0]
|
||||
if root in _BLOCKED_IMPORTS:
|
||||
violations.append(f"from '{node.module}' import ...")
|
||||
elif isinstance(node, ast.Call):
|
||||
fn = node.func
|
||||
if isinstance(fn, ast.Name) and fn.id in _BLOCKED_CALLS:
|
||||
violations.append(f"call to '{fn.id}()'")
|
||||
elif isinstance(node, ast.Attribute):
|
||||
if node.attr in _BLOCKED_ATTRS:
|
||||
violations.append(f"attribute access '.{node.attr}'")
|
||||
|
||||
seen: set[str] = set()
|
||||
deduped: list[str] = []
|
||||
for v in violations:
|
||||
if v not in seen:
|
||||
seen.add(v)
|
||||
deduped.append(v)
|
||||
return deduped
|
||||
|
||||
def _render(self, inputs: dict[str, Any]) -> ToolResult:
|
||||
scene_code = inputs["scene_code"]
|
||||
scene_name = inputs.get("scene_name")
|
||||
@@ -174,6 +260,23 @@ class MathAnimate(BaseTool):
|
||||
if "from manim import" not in scene_code:
|
||||
scene_code = "from manim import *\n\n" + scene_code
|
||||
|
||||
# Safety gate: scene_code is executed on the host by Manim. Reject
|
||||
# dangerous constructs unless the caller explicitly opts out. (issue #219)
|
||||
if not inputs.get("allow_unsafe_code", False):
|
||||
violations = self._scan_scene_code(scene_code)
|
||||
if violations:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
error=(
|
||||
"scene_code blocked by the math_animate safety scan. This "
|
||||
"tool executes caller-supplied Python on the host; the "
|
||||
"following constructs are disallowed by default:\n - "
|
||||
+ "\n - ".join(violations)
|
||||
+ "\nIf you fully trust this code and require them, pass "
|
||||
"allow_unsafe_code=true. See issue #219."
|
||||
),
|
||||
)
|
||||
|
||||
# Auto-detect scene name if not provided
|
||||
if not scene_name:
|
||||
scene_name = self._detect_scene_name(scene_code)
|
||||
|
||||
Reference in New Issue
Block a user