fix(security): sandbox sympy expression parsing against arbitrary code execution (#42)

[franciszver]

Closes #42

Vulnerability

sympy.parse_expr/sympify compile+eval their string input; with unrestricted globals a string like __import__("os").system(...) runs arbitrary code. Reachable via _canonical_key (model-generated choice text) and the eval graders (fixture/model text); MathGenerator.validate_answer is designed for student input (not handler-wired today, fixed preemptively). Confirmed with a live os.system PoC.

Fix — src/services/practice/safe_expr.py (new), one helper used everywhere

• Primary control: allowlist namespace. _SAFE_GLOBALS contains only safe math constructors/functions/constants (Symbol, Integer, Rational, Float, sqrt, trig, log, exp, factorial, gcd, …) with __builtins__ stripped. Crucially it does NOT include sympify/S/parse_expr/lambdify — anything that re-parses/compiles a string — so those degrade to inert auto-symbols instead of executing.
• Defense-in-depth: a minimal deny-list (__, backtick).
• global_dict/local_dict force-set so a caller can't reintroduce the bypass.
• All string-parse sites (_canonical_key, _parse_answer, 3 grader sites) rewired; simplify() on live sympy objects left untouched; each site's existing fallback preserved.

Review journey (this is why it took the rounds it did)

• Round 1 (allowlist-less, import * + deny-list): a fresh review executed a real RCE via sympify("_"+"_"+"imp"+"ort"+…) — the namespace itself contained a re-parsing callable, and a concatenated payload evaded the static deny-list. Green tests missed it.
• Round 2: rebuilt as the allowlist above. A second adversarial review tried all vectors (esp. the string-accepting Float/Rational/Integer constructors) and could not break it.
• Round 3: pinned the string-constructor threat model as regression tests.

Verification

• Full suite: 494 passed, 1 skipped. run_eval: PASS. black/isort/flake8 clean.
• RCE independently re-verified closed (os.system + subprocess mocked, assert_not_called across all call sites for every payload class); legit math parses identically.

🤖 Generated with Claude Code

https://claude.ai/code/session_016v4C1daTwNakL6QDbmy3MJ