fix(strategy/tot): use ast.literal_eval for model-generated thoughts#2069
Open
Jiangrong-W wants to merge 1 commit into
Open
Conversation
6f3507d to
edeb873
Compare
ThoughtSolverBase.generate_thoughts parsed the model's generated thought
block with eval() after only a cosmetic markdown-fence strip. Because the
Tree-of-Thoughts solver feeds raw model output (rsp -> CodeParser.parse_code)
into eval(), a model that emits non-literal Python (e.g. a list whose element
calls __import__('os').system(...)) executes arbitrary code on the host. No
sandbox, try/except, or literal check guarded the call.
The model is instructed (OUTPUT_FORMAT) to return a plain JSON/Python list of
thought nodes, so the value is always a literal data structure. Replace eval()
with ast.literal_eval(), which only evaluates literals and raises on code. Parse
failures are logged and degrade to an empty thought list (matching update_node's
default), preserving behaviour for well-formed output.
Adds a regression test that fails on the previous eval() path (model-supplied
code executes) and passes with literal-only parsing, plus a benign-output test
confirming valid thought lists still parse.
The regression test injects a mocked LLM, but the session-wide autouse llm_mock
fixture (tests/conftest.py) still builds a real OpenAILLM for every test, which
validates config.llm.proxy via httpx. An earlier-collected module assigns a
non-URL object onto the shared config.llm instance and never restores it, so
that leaked proxy made llm_mock raise "Proxy protocol must be ..." at setup of
these tests. A module-scoped autouse fixture restores config.llm.proxy to a sane
value before llm_mock runs and puts it back afterwards, isolating these tests
without affecting any other module.
Signed-off-by: christop <825583681@qq.com>
edeb873 to
d3da0dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Features
Harden the Tree-of-Thoughts solver so model-generated output is parsed as data instead of being executed as code.
ThoughtSolverBase.generate_thoughts(metagpt/strategy/tot.py) took the model's generated thought block and parsed it witheval()after only stripping the markdown code fence:rsp = await self.llm.aask(...)— raw model output;thoughts = CodeParser.parse_code(text=rsp)— regex-extracts the fenced block and returns it verbatim, with no literal/AST validation;thoughts = eval(thoughts)(sink,metagpt/strategy/tot.py:66onmain).Because
eval()evaluates arbitrary Python, any non-literal expression in the model output (rather than the expected list of nodes) would run in the host process. No sandbox, literal check, ortry/exceptguarded the call.BFSSolver.generate_and_evaluate_nodesandDFSSolver._dfs, so every Tree-of-Thoughts run routes raw LLM output into the sink.OUTPUT_FORMAT,metagpt/strategy/tot.py:20) instructs the model to return "strictly a list of nodes, in json format" — i.e. a literal data structure. Theeval()was being used purely as a parser, so evaluating code was never the intended behaviour.eval()withast.literal_eval(), which only evaluates literals (lists/dicts/strings/numbers) and raises on anything else. The call is wrapped intry/except (ValueError, SyntaxError): on a parse failure we log a warning and fall back to an empty thought list (thoughts = []), whichThoughtTree.update_nodealready accepts as its default argument (update_node(thought: List[dict] = [])). Well-formed output is unchanged; malformed or unexpected output degrades safely instead of executing or raising an unhandled error. Standard library only (ast); no new dependency.Feature Docs
Not applicable — internal hardening of an existing code path. No public API or signature changes, no configuration changes.
Influence
ast.literal_evalaccepts the literal shape the prompt requests).BFSSolver,DFSSolver).ast). Scope is limited to the single parse call ingenerate_thoughts.Result
Added
tests/metagpt/strategy/test_tot_generate_thoughts_eval.py(stub LLM, no network):test_generate_thoughts_parses_benign_list— a fenced JSON list of thought nodes is parsed intoThoughtNodes (behaviour preserved).test_generate_thoughts_does_not_execute_model_code— when the model returns a non-literal payload, no side effect occurs (a sentinel file is asserted absent). This fails on the pre-fixeval()path and passes after the fix.test_literal_eval_rejects_code— guards the underlying primitive.Reverting only
metagpt/strategy/tot.pyreproduces the prior behaviour:Lint on the changed files is clean (
ruff,black --line-length 120,isort --profile black).Other
Signed-off-by).