Security: replace eval() with ast.literal_eval() and add path traversal checks - #1542
Security: replace eval() with ast.literal_eval() and add path traversal checks#1542johnpippett wants to merge 2 commits into
Conversation
… code execution Replaces all eval() calls on LLM output and database-stored config values with ast.literal_eval(), which only permits Python literals and cannot execute arbitrary code. Affected files: - superagi/agent/output_handler.py - superagi/agent/queue_step_handler.py - superagi/models/agent_execution_config.py - superagi/models/agent_template.py - superagi/models/agent.py - superagi/controllers/knowledges.py Fixes critical finding: Arbitrary code execution via eval() on LLM output.
Adds _validate_path_containment() to ResourceHelper and applies it to get_resource_path(), get_agent_write_resource_path(), and get_agent_read_resource_path(). This prevents file tools from reading or writing outside the intended resource directories via ../ sequences or absolute path injection. Affected files: - superagi/helper/resource_helper.py Fixes critical finding: Path traversal in file tools.
Ujjawal0204
left a comment
There was a problem hiding this comment.
Nice security pass. One edge case in the new containment check: in get_agent_read_resource_path(), validation happens before {agent_id} / {agent_execution_id} placeholders are expanded, while the write path validates after formatting.
If formatted names can contain path separators or traversal-like content, the final path may differ from the path that was validated. Could we either expand placeholders before _validate_path_containment(...) or revalidate after expansion for consistency with get_agent_write_resource_path()?
|
@Ujjawal0204 I took a local look and I think your concern is valid. I reproduced the path-shape issue against One extra finding: this is not limited to So the safer fix is to either sanitize the formatted agent/run path components so they cannot contain |
Fixes two critical security findings:
Syntax validated; standalone verification confirms malicious payloads are rejected.