From b69ce5f9a2a8904b2cd9786df6f95a652fa8296c Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Wed, 1 Jul 2026 14:23:29 +0530 Subject: [PATCH] fix(math_animate): gate caller-supplied scene_code execution (#219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- tests/tools/test_math_animate_safety.py | 122 ++++++++++++++++++++++++ tools/graphics/math_animate.py | 107 ++++++++++++++++++++- 2 files changed, 227 insertions(+), 2 deletions(-) create mode 100644 tests/tools/test_math_animate_safety.py diff --git a/tests/tools/test_math_animate_safety.py b/tests/tools/test_math_animate_safety.py new file mode 100644 index 00000000..dafc487d --- /dev/null +++ b/tests/tools/test_math_animate_safety.py @@ -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 "") diff --git a/tools/graphics/math_animate.py b/tools/graphics/math_animate.py index 61c635b6..1caf808e 100644 --- a/tools/graphics/math_animate.py +++ b/tools/graphics/math_animate.py @@ -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)