diff --git a/sieval/community/CLAUDE.md b/sieval/community/CLAUDE.md index dd2e064f..7137ec37 100644 --- a/sieval/community/CLAUDE.md +++ b/sieval/community/CLAUDE.md @@ -2,7 +2,7 @@ ## Purpose -This directory contains local adaptations of third-party evaluation tools (e.g. livecodebench, instruction_following_eval, simple_evals). These are wrappers around upstream implementations, not original code. +This directory contains local adaptations of third-party evaluation tools (e.g. livecodebench, instruction_following_eval, simple_evals). These are wrappers around upstream implementations, not original code — with the narrow exception below. ## Requirements @@ -15,3 +15,18 @@ This directory contains local adaptations of third-party evaluation tools (e.g. * No mandatory test coverage. * No mandatory internal code style enforcement (but keep it readable). * License attribution must be preserved where required by upstream. + +## First-Party Modules + +`_sympy_guards.py` is original code, not a wrapper: the execution guards +`deepseek_math` and `ugmathbench` share. Holding it *outside* both serves +upstream alignment rather than working against it — the vendored files keep only +a small annotated divergence each, instead of carrying a copy of the guards +inline where it would swamp a diff against upstream. Do not add more original +code here without the same argument; a helper with one caller belongs in that +caller's module. + +The package-wide `ruff` / `mypy` / `pre-commit` exclusions exist to keep vendored +code byte-identical and cover this file too, which is the wrong default for a +security boundary. Until they are narrowed, lint it by hand: +`ruff check --config 'exclude=["vendor"]' sieval/community/_sympy_guards.py`. diff --git a/sieval/community/_sympy_guards.py b/sieval/community/_sympy_guards.py new file mode 100644 index 00000000..1fb5a9c2 --- /dev/null +++ b/sieval/community/_sympy_guards.py @@ -0,0 +1,156 @@ +"""Guards for handing model output to sympy. + +Both math graders in this package parse a model's answer with sympy, and sympy +parsing *is* an execution path. The three guards here are the same in both, and +have to stay the same: a new escape route closes in one place or the other +grader is left open. That shared contract is why they live here rather than in +either module. + +The threat has three legs, and closing one without the others buys nothing: + +1. ``parse_expr``'s default global namespace is built by + ``exec("from sympy import *", ...)``, which also injects ``__builtins__`` -- + so model output gets ``__import__`` and ``open``. :func:`sympy_globals` + removes them. +2. Sympy re-sympifies a *string* argument with its own default namespace, which + has the builtins back, so a call carrying a string literal escapes leg 1. + :func:`quotes_free` refuses the quote instead of the callee. +3. ``parse_expr`` evaluates as it parses, so ``9**9**9`` never returns. + :func:`evaluable` screens it out with an unevaluated pre-parse. + +**None of this makes a string safe to hand to ``sympify``, ``simplify``, ``N`` +or ``S``.** Those take the default namespace, not the caller's, so they defeat +legs 1 and 2 on their own -- a payload needs no quote at all once ``__import__`` +resolves (``__import__(chr(111)+chr(115))``). A caller must ensure a *parsed +sympy object* reaches them, never the raw text; refusing at the parse step and +then falling through to ``simplify(text)`` reopens exactly what was closed. + +Callers are responsible for their own time bound. Leg 3 only screens the two +shapes common enough to be worth not spending a worker on; an eagerly-evaluating +sympy callable needs no exponent to be expensive (``primepi(10**12)`` takes 48 s, +``factorial(1000000)`` 3.8 s), and enumerating those is the same losing game as +allowlisting callees in leg 2. The bound that actually holds is +:data:`~sieval.core.utils.offload.GRADE_TIMEOUT` in the worker process. + +AI-Generated Code - Claude Opus 5 (1M context) (Anthropic) +""" + +#: Largest integer exponent :func:`evaluable` will admit. Sympy computes +#: ``a**b`` eagerly, so a boxed ``9^9^9^9`` asks for a 370-million-digit integer +#: and never returns. Nothing either benchmark asks for comes close — the +#: largest exponent in UGMathBench's pinned references is three digits — so the +#: cap costs no reachable comparison. +MAX_EXPONENT = 10_000 + + +def sympy_globals() -> dict: + """Namespace for :func:`parse_expr`, with the builtins removed. + + ``parse_expr`` evaluates its input, and its *default* global namespace is + built by ``exec("from sympy import *", ...)`` — which also injects + ``__builtins__``. Since the string being parsed is model output, that hands + a model ``__import__`` and ``open``: a boxed + ``__import__('os').system(...)`` runs, and the grader still reports the slot + wrong, so nothing in the run looks unusual. + + Clearing ``__builtins__`` closes that without narrowing the dialect. The + sympy names have to stay: ``auto_symbol`` rewrites an unknown callable into + ``Function('sin')``, so a namespace holding *only* the answer aliases fails + every legitimate ``sin(pi*x/5)`` with ``NameError: name 'Function' is not + defined``. + + This is a namespace restriction, not a sandbox — attribute access on sympy + objects still resolves, and it only covers the namespace *this* parse runs + in. It does not survive a nested parse, which is why :func:`quotes_free` + exists. + """ + namespace: dict = {} + exec("from sympy import *", namespace) # noqa: S102 - fixed literal, not input + namespace["__builtins__"] = {} + return namespace + + +def quotes_free(text: str) -> bool: + """Is *text* free of the string literals that reopen the interpreter? + + :func:`sympy_globals` sanitizes the namespace the top-level parse runs in, + and that is not enough on its own. Sympy re-sympifies a *string* argument + with its own default namespace, which has the builtins back, so a call + carrying a string literal escapes the restriction and runs:: + + eval("__import__('os').system(...)") + + ``sympify``, ``S`` and ``N`` do the same thing, and ``auto_symbol`` turns + any unrecognized name into a ``Function``, so the callee cannot be + allowlisted — every function call is a potential carrier. What can be + refused is the payload: without a quote there is no string literal for the + nested parse to read, and the argument comes back as a sympy object + (``eval(chr(112))`` evaluates ``chr`` symbolically and does nothing). + + This holds only for text reaching ``parse_expr`` with a cleared namespace. + It is **not** sufficient for text handed straight to ``sympify`` / ``N`` / + ``simplify``, where ``__import__`` resolves without any quote at all — see + the module docstring. + + Nothing legitimate is lost, on evidence from both dialects: not one of + UGMathBench's 42,064 gold slots on the pinned revision contains a quote + (sympy source, where quotes have no meaning), and for deepseek's LaTeX it is + the 6,319-sample replay in that module's deviations note. A refused + prediction only loses this one reading, with the LaTeX and literal-equality + paths still offered to the comparison. + """ + return "'" not in text and '"' not in text + + +def evaluable(cleaned: str, local: dict | None = None, transformations=None) -> bool: + """Is *cleaned* free of the two exponent shapes that never finish? + + Deliberately narrower than "would this terminate". ``parse_expr`` evaluates + as it parses, so the check cannot run afterwards — by then the process is + already computing. Parsing with ``evaluate=False`` first builds the tree + without doing the arithmetic (microseconds even for the pathological cases), + which is cheap enough to screen on. + + Rejected: a power whose exponent is itself a power (``9**9**9``, the tower + shape), and an integer exponent above :data:`MAX_EXPONENT`. A left-nested + ``(x**2)**3`` is fine and stays — only the right-nested tower explodes. + + What it does **not** screen is covered in the module docstring: an eagerly + evaluating sympy callable needs no exponent to be expensive, and the bound + that holds is the caller's worker timeout. This screen only buys back the + two shapes common enough to be worth not spending a worker on. + + A rejected answer grades wrong rather than hanging the run. That is the + correct trade for a grader: this pass only ever *upgrades* a verdict (it is + reached after every other strategy said "not equal"), so refusing to run it + can lose a point but can never invent one. + """ + import sympy + from sympy.parsing.sympy_parser import parse_expr + from sympy.parsing.sympy_parser import ( + standard_transformations as _standard, + ) + + if "**" not in cleaned: + return True + try: + tree = parse_expr( + cleaned, + local_dict=local, + global_dict=sympy_globals(), + transformations=_standard if transformations is None else transformations, + evaluate=False, + ) + except Exception: + # Unparseable under this reading; the real parse will fail the same way + # and is harmless. Screening is not the place to decide that. + return True + for node in sympy.preorder_traversal(tree): + if not isinstance(node, sympy.Pow): + continue + exponent = node.exp + if exponent.has(sympy.Pow): + return False + if exponent.is_Integer and abs(int(exponent)) > MAX_EXPONENT: + return False + return True diff --git a/sieval/community/deepseek_math.py b/sieval/community/deepseek_math.py index 288859f8..e3218954 100644 --- a/sieval/community/deepseek_math.py +++ b/sieval/community/deepseek_math.py @@ -27,6 +27,30 @@ (`few_shot_prompts/cot_minerva_math_4_shot.py` + the `math-cot-test` path). Deviations from upstream: +- **`symbolic_equal` does not execute model output.** Upstream parses a + prediction with a bare `parse_expr`, whose default namespace carries + `__builtins__`, and — when both parsers fail — returns the *raw string*, which + then reaches `N`, and `N` sympifies it with sympy's own default namespace. + (Only `N`: `simplify(a - b)` raises `TypeError` first, since sympy's + arithmetic dunders sympify strictly.) Either route runs + `__import__('os').system(...)` supplied as an answer, and the grader still + reports the sample wrong, so nothing in the run looks unusual. Two changes + close it: `parse_expr` runs under `_sympy_guards` (cleared namespace, quote + screen, unevaluated exponent pre-parse), and an unparseable answer becomes + `None` and refuses the comparison rather than reaching `N` as text. The second + matters more than the first — once `__import__` resolves through the default + namespace, a payload needs no quote at all, so guarding only the parse would + have moved the hole rather than closed it. + MEASURED DIVERGENCE: ZERO. Replayed over two full stored runs — GSM8K 1319 + samples (deepseek-llm-7b-chat) and MATH 5000 (Qwen2.5-72B) — under both a + working and a deliberately disabled `parse_latex`, the latter forcing every + comparison down the guarded path (1622 of 5000 fall through on MATH). All + four cells agree with upstream on every sample: GSM8K 63.3813 / 63.3055 and + MATH 61.2600 / 60.0200, upstream and guarded alike. + The exponent pre-parse also declines a right-nested `**` tower and an exponent + above `MAX_EXPONENT`, which upstream evaluates; `parse_latex` reads those + spellings first, so the zero covers them too. See `sieval/tasks/CLAUDE.md` on + why this ships under the unqualified task names. - `math_equal` is only ever called with the default `timeout=False` (via `eval_math` / `is_correct` and the GSM8K path), so the `symbolic_equal_process` / `call_with_timeout` multiprocessing path is unused @@ -54,6 +78,8 @@ from sympy.parsing.latex import parse_latex from sympy.parsing.sympy_parser import parse_expr +from ._sympy_guards import evaluable, quotes_free, sympy_globals + def _fix_fracs(string): substrs = string.split("\\frac") @@ -312,16 +338,36 @@ def is_digit(num): # paired with parse_digits return parse_digits(num) is not None +def _guarded_parse_expr(s): + """`parse_expr` with the three guards in `_sympy_guards` applied. + + SIEVAL DIVERGENCE (execution safety). Upstream calls bare `parse_expr(s)` + on model output, whose default namespace carries `__builtins__`, so a + prediction of `__import__('os').system(...)` runs. See `_sympy_guards`. + """ + if not quotes_free(s) or not evaluable(s): + raise ValueError("refused by sieval: unsafe to hand to sympy") + return parse_expr(s, global_dict=sympy_globals()) + + def symbolic_equal(a, b): def _parse(s): - for f in [parse_latex, parse_expr]: + for f in [parse_latex, _guarded_parse_expr]: try: return f(s) except: pass - return s + # SIEVAL DIVERGENCE (execution safety). Upstream returns `s` here, the + # raw model output, which reaches `N` below -- and `N` sympifies it with + # sympy's own default namespace, not the caller's. (Not `simplify(a-b)`: + # the subtraction raises TypeError first.) That alone defeats the guards + # above -- with `__import__` resolvable a payload needs no quote -- so an + # unparseable answer becomes None and the comparison is refused. + return None a = _parse(a) b = _parse(b) + if a is None or b is None: + return False try: if simplify(a-b) == 0: diff --git a/sieval/community/ugmathbench.py b/sieval/community/ugmathbench.py index 104f64b9..24f4d704 100644 --- a/sieval/community/ugmathbench.py +++ b/sieval/community/ugmathbench.py @@ -120,14 +120,14 @@ is model output and ``parse_expr`` evaluates what it parses. Three shapes: - A boxed ``__import__('os').system(...)`` would otherwise run. - :func:`_sympy_globals` removes the builtins from the parse namespace. + :func:`~sieval.community._sympy_guards.sympy_globals` removes the builtins from the parse namespace. - A boxed ``eval("...")`` — or ``sympify``/``S``/``N``, or any name at all, since ``auto_symbol`` makes every unknown one callable — hands a *string* back to sympy, which re-sympifies it with its own default namespace and so - gets the builtins back. :func:`_quotes_free` refuses the quote instead of + gets the builtins back. :func:`~sieval.community._sympy_guards.quotes_free` refuses the quote instead of the callee, which is the only end of it that can be enumerated. - A boxed ``9^9^9^9`` asks for a 370-million-digit integer that never - returns. :func:`_evaluable` screens it out with an unevaluated pre-parse. + returns. :func:`~sieval.community._sympy_guards.evaluable` screens it out with an unevaluated pre-parse. Grading runs in a worker process, so this occupies one worker rather than the shared event loop, but a worker held forever is still a worker lost. @@ -187,6 +187,8 @@ import math import re +from ._sympy_guards import evaluable, quotes_free, sympy_globals + #: The 16 subject configs of the HF dataset, in the order the benchmark lists #: them. Doubles as the default load order, so a sliced run is reproducible. SUBJECTS: tuple[str, ...] = ( @@ -552,123 +554,6 @@ def _parse_math(text: str) -> list: _MIN_CLEAN_PROBES = 3 _MAX_FREE_SYMBOLS = 4 -#: Largest integer exponent :func:`_parse_sympy_source` will evaluate. Sympy -#: computes ``a**b`` eagerly, so a boxed ``9^9^9^9`` asks for a 370-million-digit -#: integer and never returns. Nothing this benchmark asks for comes close — the -#: largest exponent in the pinned references is three digits — so the cap costs -#: no reachable comparison. See :func:`_evaluable`. -_MAX_EXPONENT = 10_000 - - -def _sympy_globals() -> dict: - """Namespace for :func:`parse_expr`, with the builtins removed. - - ``parse_expr`` evaluates its input, and its *default* global namespace is - built by ``exec("from sympy import *", ...)`` — which also injects - ``__builtins__``. Since the string being parsed is model output, that hands - a model ``__import__`` and ``open``: a boxed - ``__import__('os').system(...)`` runs, and the grader still reports the slot - wrong, so nothing in the run looks unusual. - - Clearing ``__builtins__`` closes that without narrowing the dialect. The - sympy names have to stay: ``auto_symbol`` rewrites an unknown callable into - ``Function('sin')``, so a namespace holding *only* the answer aliases fails - every legitimate ``sin(pi*x/5)`` with ``NameError: name 'Function' is not - defined``. - - This is a namespace restriction, not a sandbox — attribute access on sympy - objects still resolves, and it only covers the namespace *this* parse runs - in. It does not survive a nested parse, which is why :func:`_quotes_free` - exists. - """ - namespace: dict = {} - exec("from sympy import *", namespace) # noqa: S102 - fixed literal, not input - namespace["__builtins__"] = {} - return namespace - - -def _quotes_free(text: str) -> bool: - """Is *text* free of the string literals that reopen the interpreter? - - :func:`_sympy_globals` sanitizes the namespace the top-level parse runs in, - and that is not enough on its own. Sympy re-sympifies a *string* argument - with its own default namespace, which has the builtins back, so a call - carrying a string literal escapes the restriction and runs:: - - eval("__import__('os').system(...)") - - ``sympify``, ``S`` and ``N`` do the same thing, and ``auto_symbol`` turns - any unrecognized name into a ``Function``, so the callee cannot be - allowlisted — every function call is a potential carrier. What can be - refused is the payload: without a quote there is no string literal for the - nested parse to read, and the argument comes back as a sympy object - (``eval(chr(112))`` evaluates ``chr`` symbolically and does nothing). - - Nothing legitimate is lost. Not one of the 42,064 gold slots on the pinned - revision contains a quote — the dialect is sympy source, where quotes have - no meaning — and a refused prediction only loses this one reading, with the - LaTeX and literal-equality paths still offered to the comparison. - """ - return "'" not in text and '"' not in text - - -def _evaluable(cleaned: str, local: dict, transformations) -> bool: - """Is *cleaned* free of the two exponent shapes that never finish? - - Deliberately narrower than "would this terminate". ``parse_expr`` evaluates - as it parses, so the check cannot run afterwards — by then the process is - already computing. Parsing with ``evaluate=False`` first builds the tree - without doing the arithmetic (microseconds even for the pathological cases), - which is cheap enough to screen on. - - Rejected: a power whose exponent is itself a power (``9**9**9``, the tower - shape), and an integer exponent above :data:`_MAX_EXPONENT`. A left-nested - ``(x**2)**3`` is fine and stays — only the right-nested tower explodes. - - What it does **not** screen: an eagerly-evaluating sympy callable that needs - no exponent to be expensive. ``from sympy import *`` puts ``factorial``, - ``prime`` and ``primepi`` in the parse namespace, and the early return below - lets anything without ``**`` straight through — measured, ``primepi(10**12)`` - takes 48 s and ``factorial(1000000)`` 3.8 s. Enumerating those callees is - the same losing game as allowlisting them in :func:`_quotes_free`, so the - bound that actually holds is the caller's: grading runs in a worker process - under :data:`~sieval.core.utils.offload.GRADE_TIMEOUT`, which scores the slot - wrong and moves on. This screen only buys back the two shapes common enough - to be worth not spending a worker on. - - A rejected answer grades wrong rather than hanging the run. That is the - correct trade for a grader: this pass only ever *upgrades* a verdict - (:func:`math_equal` reaches it after every other strategy said "not equal"), - so refusing to run it can lose a point but can never invent one. - """ - import sympy - from sympy.parsing.sympy_parser import parse_expr - - if "**" not in cleaned: - return True - try: - tree = parse_expr( - cleaned, - local_dict=local, - global_dict=_sympy_globals(), - transformations=transformations, - evaluate=False, - ) - except Exception: - # Unparseable under this reading; the real parse will fail the same way - # and is harmless. Screening is not the place to decide that. - return True - for node in sympy.preorder_traversal(tree): - if not isinstance(node, sympy.Pow): - continue - exponent = node.exp - if exponent.has(sympy.Pow): - return False - if exponent.is_Integer and abs(int(exponent)) > _MAX_EXPONENT: - return False - return True - - def _parse_sympy_source(text: str) -> list: """Parse UGMathBench's plain-sympy answer syntax as sympy source. @@ -684,9 +569,9 @@ def _parse_sympy_source(text: str) -> list: The text reaching this function is model output, and ``parse_expr`` evaluates what it parses, so all three halves of that are guarded: - :func:`_sympy_globals` takes the interpreter out of the parse namespace, - :func:`_quotes_free` keeps a nested parse from handing it back, and - :func:`_evaluable` refuses arithmetic that would not finish. + :func:`~sieval.community._sympy_guards.sympy_globals` takes the interpreter out of the parse namespace, + :func:`~sieval.community._sympy_guards.quotes_free` keeps a nested parse from handing it back, and + :func:`~sieval.community._sympy_guards.evaluable` refuses arithmetic that would not finish. """ import sympy from sympy.parsing.sympy_parser import parse_expr @@ -701,7 +586,7 @@ def _parse_sympy_source(text: str) -> list: .replace("$", "") .strip() ) - if not cleaned or not _quotes_free(cleaned): + if not cleaned or not quotes_free(cleaned): return [] local = { "e": sympy.E, @@ -725,9 +610,9 @@ def _parse_sympy_source(text: str) -> list: "arctanh": sympy.atanh, } out: list = [] - globals_ = _sympy_globals() + globals_ = sympy_globals() for transformations in _source_transformations(): - if not _evaluable(cleaned, local, transformations): + if not evaluable(cleaned, local, transformations): continue try: out.append( diff --git a/sieval/meta/index.json b/sieval/meta/index.json index 26f4d59b..feea94ff 100644 --- a/sieval/meta/index.json +++ b/sieval/meta/index.json @@ -1287,7 +1287,7 @@ "reference_impl": { "source": "deepseek-ai/DeepSeek-Math", "url": "https://github.com/deepseek-ai/DeepSeek-Math/tree/b8b0f8ce093d80bf8e9a641e44142f06d092c305/evaluation", - "notes": "gsm8k-test zero-shot CoT protocol: user turn = question + \"Please reason step by step, and put your final answer within \\boxed{}.\", chat template applied by the serving backend; extract_answer(exhaust=False) (= extract_last_single_answer) and is_correct/math_equal (= eval_last_single_answer) scoring are vendored byte-for-byte in sieval.community.deepseek_math. Gold derived from openai/gsm8k like process_gsm8k_test (answer.split('####')[-1], commas removed)." + "notes": "gsm8k-test zero-shot CoT protocol: user turn = question + \"Please reason step by step, and put your final answer within \\boxed{}.\", chat template applied by the serving backend; extract_answer(exhaust=False) (= extract_last_single_answer) and is_correct/math_equal (= eval_last_single_answer) scoring are vendored byte-for-byte in sieval.community.deepseek_math, with ONE divergence, taken for execution safety rather than as a repair: upstream's symbolic_equal hands model output to a bare parse_expr and, when parsing fails, to N as raw text -- both sympify with a namespace carrying __builtins__, so an answer of __import__('os').system(...) runs while the sample still grades wrong (not simplify(a-b), which raises TypeError first). Here the parse is guarded and an unparseable answer refuses the comparison instead. MEASURED DIVERGENCE: ZERO -- replaying this benchmark's full 1319-sample stored run (deepseek-llm-7b-chat) through upstream's reading and this one gives identical verdicts on every sample, 63.3813 either way, and 63.3055 either way with parse_latex disabled (the case that forces every comparison down the guarded path). The exponent pre-parse also declines a right-nested ** tower and an exponent above 10000, which upstream evaluates; parse_latex reads those spellings first, so the zero covers them too. Gold derived from openai/gsm8k like process_gsm8k_test (answer.split('####')[-1], commas removed). NOTE ON parse_latex: sympy 1.14's LaTeX grammar needs antlr4-python3-runtime 4.11.0, pinned in the [math] extra. Without it parse_latex raises into upstream's bare except and the symbolic path silently never runs -- worth 0.08 pp here and 1.24 pp on MATH, so an environment missing it scores lower for a reason no log reports." }, "status": "stable" }, @@ -1351,7 +1351,7 @@ "reference_impl": { "source": "DeepSeek-Math", "url": "https://github.com/deepseek-ai/DeepSeek-Math/tree/b8b0f8ce093d80bf8e9a641e44142f06d092c305/evaluation", - "notes": "math-cot-test path: MinervaMathPrompt 4-shot, extract_math_few_shot_cot_answer (list-valued) + eval_math/math_equal." + "notes": "math-cot-test path: MinervaMathPrompt 4-shot, extract_math_few_shot_cot_answer (list-valued) + eval_math/math_equal, vendored in sieval.community.deepseek_math with ONE divergence, taken for execution safety rather than as a repair: upstream's symbolic_equal hands model output to a bare parse_expr and, when parsing fails, to N as raw text -- both sympify with a namespace carrying __builtins__, so an answer of __import__('os').system(...) runs while the sample still grades wrong (not simplify(a-b), which raises TypeError first). Here the parse is guarded and an unparseable answer refuses the comparison instead. MEASURED DIVERGENCE: ZERO -- replaying the full 5000-sample stored run (Qwen2.5-72B) through upstream's reading and this one gives identical verdicts on every sample, 61.2600 either way, and 60.0200 either way with parse_latex disabled -- the adversarial case, since it sends every comparison down the guarded path (1622 of 5000 fall through to the refusal). The exponent pre-parse also declines a right-nested ** tower and an exponent above 10000, which upstream evaluates; parse_latex reads those spellings first, so the zero covers them too. NOTE ON parse_latex: sympy 1.14's LaTeX grammar needs antlr4-python3-runtime 4.11.0, pinned in the [math] extra. Without it parse_latex raises into upstream's bare except and the symbolic path silently never runs, costing 1.24 pp (61.26 -> 60.02) on this benchmark with no signal in any log -- the two figures above are the same run measured with and without it." }, "status": "stable" }, diff --git a/sieval/tasks/gsm8k_0shot_gen.py b/sieval/tasks/gsm8k_0shot_gen.py index ba09c2b3..b39cab59 100644 --- a/sieval/tasks/gsm8k_0shot_gen.py +++ b/sieval/tasks/gsm8k_0shot_gen.py @@ -99,9 +99,30 @@ def _gold_answer(answer: str) -> str: '\\boxed{}.", chat template applied by the serving backend; ' "extract_answer(exhaust=False) (= extract_last_single_answer) and " "is_correct/math_equal (= eval_last_single_answer) scoring are " - "vendored byte-for-byte in sieval.community.deepseek_math. Gold " + "vendored byte-for-byte in sieval.community.deepseek_math, with ONE " + "divergence, taken for execution safety rather than as a repair: " + "upstream's symbolic_equal hands model output to a bare parse_expr " + "and, when parsing fails, to N as raw text -- both sympify with a " + "namespace carrying __builtins__, so an answer of " + "__import__('os').system(...) runs while the sample still grades " + "wrong (not simplify(a-b), which raises TypeError first). Here the " + "parse is guarded and an unparseable answer refuses the comparison " + "instead. MEASURED DIVERGENCE: ZERO -- replaying " + "this benchmark's full 1319-sample stored run (deepseek-llm-7b-chat) " + "through upstream's reading and this one gives identical verdicts on " + "every sample, 63.3813 either way, and 63.3055 either way with " + "parse_latex disabled (the case that forces every comparison down " + "the guarded path). The exponent pre-parse also declines a " + "right-nested ** tower and an exponent above 10000, which upstream " + "evaluates; parse_latex reads those spellings first, so the zero " + "covers them too. Gold " "derived from openai/gsm8k like process_gsm8k_test " - "(answer.split('####')[-1], commas removed)." + "(answer.split('####')[-1], commas removed). NOTE ON parse_latex: " + "sympy 1.14's LaTeX grammar needs antlr4-python3-runtime 4.11.0, " + "pinned in the [math] extra. Without it parse_latex raises into " + "upstream's bare except and the symbolic path silently never runs -- " + "worth 0.08 pp here and 1.24 pp on MATH, so an environment missing " + "it scores lower for a reason no log reports." ), ), ) diff --git a/sieval/tasks/hendrycks_math_kshot_base_gen.py b/sieval/tasks/hendrycks_math_kshot_base_gen.py index 88e5a5f8..38545a1d 100644 --- a/sieval/tasks/hendrycks_math_kshot_base_gen.py +++ b/sieval/tasks/hendrycks_math_kshot_base_gen.py @@ -72,7 +72,30 @@ url="https://github.com/deepseek-ai/DeepSeek-Math/tree/b8b0f8ce093d80bf8e9a641e44142f06d092c305/evaluation", notes=( "math-cot-test path: MinervaMathPrompt 4-shot, " - "extract_math_few_shot_cot_answer (list-valued) + eval_math/math_equal." + "extract_math_few_shot_cot_answer (list-valued) + eval_math/math_equal, " + "vendored in sieval.community.deepseek_math with ONE divergence, " + "taken for execution safety rather than as a repair: upstream's " + "symbolic_equal hands model output to a bare parse_expr and, when " + "parsing fails, to N as raw text -- both sympify with " + "a namespace carrying __builtins__, so an answer of " + "__import__('os').system(...) runs while the sample still grades " + "wrong (not simplify(a-b), which raises TypeError first). Here the " + "parse is guarded and an unparseable answer refuses the comparison " + "instead. MEASURED DIVERGENCE: ZERO -- replaying the " + "full 5000-sample stored run (Qwen2.5-72B) through upstream's " + "reading and this one gives identical verdicts on every sample, " + "61.2600 either way, and 60.0200 either way with parse_latex " + "disabled -- the adversarial case, since it sends every comparison " + "down the guarded path (1622 of 5000 fall through to the refusal). " + "The exponent pre-parse also declines a right-nested ** tower and an " + "exponent above 10000, which upstream evaluates; parse_latex reads " + "those spellings first, so the zero covers them too. " + "NOTE ON parse_latex: sympy 1.14's LaTeX grammar needs " + "antlr4-python3-runtime 4.11.0, pinned in the [math] extra. Without " + "it parse_latex raises into upstream's bare except and the symbolic " + "path silently never runs, costing 1.24 pp (61.26 -> 60.02) on this " + "benchmark with no signal in any log -- the two figures above are " + "the same run measured with and without it." ), ), ) diff --git a/tests/unit/community/test__sympy_guards.py b/tests/unit/community/test__sympy_guards.py new file mode 100644 index 00000000..f65715a6 --- /dev/null +++ b/tests/unit/community/test__sympy_guards.py @@ -0,0 +1,88 @@ +""" +Unit tests for the shared sympy execution guards. + +AI-Generated Code - Claude Opus 5 (1M context) (Anthropic) +""" + +import pytest +from sympy.parsing.sympy_parser import parse_expr + +from sieval.community._sympy_guards import ( + MAX_EXPONENT, + evaluable, + quotes_free, + sympy_globals, +) + + +def test_sympy_globals_has_no_builtins(): + assert sympy_globals()["__builtins__"] == {} + + +def test_sympy_globals_keeps_the_sympy_dialect(): + """Clearing builtins must not narrow what a legitimate answer may say. + + `auto_symbol` rewrites an unknown callable into `Function(...)`, so a + namespace holding only the answer aliases fails every real `sin(pi*x/5)`. + """ + ns = sympy_globals() + for name in ("Function", "sin", "pi", "sqrt", "Symbol"): + assert name in ns + assert parse_expr("sin(pi*x/5)", global_dict=sympy_globals()) is not None + + +def test_cleared_namespace_blocks_the_direct_payload(tmp_path): + target = tmp_path / "direct" + with pytest.raises(AttributeError): + parse_expr( + f"__import__('os').system('touch {target}')", global_dict=sympy_globals() + ) + assert not target.exists() + + +def test_cleared_namespace_alone_is_not_enough(tmp_path): + """Pins why `quotes_free` exists: a nested parse gets the builtins back. + + If this ever stops executing, the quote screen has become unnecessary and + the restriction it imposes should be revisited rather than kept on faith. + """ + target = tmp_path / "nested" + payload = f"eval('__import__(\\'os\\').system(\\'touch {target}\\')')" + parse_expr(payload, global_dict=sympy_globals()) + assert target.exists() + assert not quotes_free(payload) + + +@pytest.mark.parametrize( + "text, expected", + [ + ("x**2 + 1", True), + ("sin(pi/6)", True), + ("eval('x')", False), + ('eval("x")', False), + ("'quoted'", False), + ], +) +def test_quotes_free(text, expected): + assert quotes_free(text) is expected + + +@pytest.mark.parametrize( + "text, expected", + [ + ("2 + 2", True), # no `**` at all — early return + ("x**2", True), + ("(x**2)**3", True), # left-nested is fine + (f"2**{MAX_EXPONENT - 1}", True), + ("9**9**9", False), # right-nested tower + (f"2**{MAX_EXPONENT + 1}", False), + ], +) +def test_evaluable(text, expected): + assert evaluable(text) is expected + + +def test_evaluable_defaults_match_bare_parse_expr(): + """Called with no local/transformations, as deepseek_math does.""" + assert evaluable("x**2 + 1") is True + assert evaluable("9**9**9") is False diff --git a/tests/unit/community/test_deepseek_math.py b/tests/unit/community/test_deepseek_math.py index 22f98759..6cd07f6a 100644 --- a/tests/unit/community/test_deepseek_math.py +++ b/tests/unit/community/test_deepseek_math.py @@ -3,12 +3,17 @@ AI-Generated Code - Claude Opus 4.8 (Anthropic) """ +import pytest + from sieval.community.deepseek_math import ( STOP_WORDS, eval_math, extract_math_answer, extract_math_few_shot_cot_answer, format_prompt, + is_correct, + math_equal, + symbolic_equal, ) _FA = "\nFinal Answer: The final answer is ${}$. I hope it is correct." @@ -53,3 +58,72 @@ def test_eval_math_set_matches_lists(): def test_eval_math_percentage_numeric_layer(): # math_equal numeric layer: 50\% == 0.5 (no parse_latex needed) assert bool(eval_math({"prediction": ["50\\%"], "answer": ["0.5"]})) + + +# --- execution safety: symbolic_equal must not run the answer it grades ------ + + +def _payload(target): + return f"__import__('os').system('touch {target}')" + + +@pytest.mark.parametrize("entry", ["symbolic_equal", "math_equal", "is_correct"]) +def test_grading_a_payload_executes_nothing(entry, tmp_path): + """Upstream runs this. Reachable from a model's extracted answer.""" + target = tmp_path / entry + call = { + "symbolic_equal": lambda p: symbolic_equal(p, "1"), + "math_equal": lambda p: math_equal(p, "1"), + "is_correct": lambda p: is_correct({"prediction": p, "answer": "1"}), + }[entry] + assert call(_payload(target)) is False + assert not target.exists() + + +def test_quoteless_payload_via_the_raw_string_fallback(tmp_path): + """The second path: unparseable text used to reach `simplify`/`N` verbatim. + + Guarding only `parse_expr` would leave this open — `sympify` uses sympy's + default namespace, where `__import__` resolves with no quote required. + """ + target = tmp_path / "fallthrough" + + # chr() builds every string without a quote, so the quote screen alone + # would not catch it; the refusal of the raw-string fallback does. + def chrs(text): + return "+".join(f"chr({ord(c)})" for c in text) + + payload = f"__import__({chrs('os')}).system({chrs(f'touch {target}')})" + assert symbolic_equal(payload, "1") is False + assert not target.exists() + + +def test_unparseable_answer_is_refused_not_sympified(): + """The behavioural shape of the divergence: no verdict is invented.""" + assert symbolic_equal("not math at all", "1") is False + + +@pytest.mark.parametrize( + "prediction, reference", + [ + ("\\frac{1}{2}", "0.5"), + ("x^{1/2}", "\\sqrt{x}"), + ("7(x-3)(x+3)", "7(x+3)(x-3)"), + ("\\frac{3\\sqrt{20}}{5}", "\\frac{6\\sqrt{5}}{5}"), + ], +) +def test_symbolic_equality_still_works(prediction, reference): + """The guards must not cost the symbolic path they protect. + + All four are real disagreements from the stored MATH run that only the + symbolic path resolves — they are exactly what is lost when `parse_latex` + is unavailable, so they also pin that the `[math]` extra's antlr4 pin is + doing its job. + + Spelled in LaTeX on purpose: `parse_latex` is tried first and *succeeds* + on `x**2 - 1` by silently truncating at the `**` (returning `x`), so a + Python-spelled case never reaches `parse_expr` and tests nothing here. + That is upstream behaviour, unchanged by the guards. + """ + assert symbolic_equal(prediction, reference) is True + assert bool(eval_math({"prediction": [prediction], "answer": [reference]}))