-
Notifications
You must be signed in to change notification settings - Fork 2k
Implement remote Python WebAssemblyExecutor #1261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement remote Python WebAssemblyExecutor #1261
Conversation
Looking forward to this! 👀 |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @albertvillanova ! Regarding the use of pyodide with deno I don't have too many comments, it looks reasonable. As you probably saw in pyodide/pyodide#3420 Deno is not exactly an officially supported platform so you might run into some edge cases. Still if you tested on your side and it works well enough then it should be fine :)
Syntax highlighting would be better if the JS code is a standalone file but then you have to read it from python so maybe this is simpler indeed.
I'll also cc @ryanking13 @hoodmane who are more up to date on pyodide development, in case they have comments.
src/smolagents/remote_executors.py
Outdated
# --allow-net=cdn.jsdelivr.net,0.0.0.0:8000; | ||
# --allow-net=pypi.org,files.pythonhosted.org; # allow fetch pyodide packages from PyPI | ||
"--allow-net", # allow fetch pyodide packages & server | ||
"--allow-read", # grant read access for pyodide packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be safer to restrict to a particular folder (node_modules?) where pydide files are. Both for reading and writing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Definitely, the access to the filesystem needs to be restricted here, as it will be the case for the net access as well. I take note so I don't forget:
# TODO: Set minimal permissions
src/smolagents/remote_executors.py
Outdated
for (const pkg of packages) { | ||
try { | ||
// await pyodide.loadPackage(pkg); | ||
await micropip.install(pkg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to use micropip here
Thanks for the ping @rth!
Yep, Deno is not officially supported in Pyodide, but there are already some projects like PydanticAI and Langchain Sandbox that are utilizing Pyodide + Deno combination. So I think it is a reasonable choice. If you encounter any bugs, then feel free to open an issue in the Pyodide repository (or in the Deno repository). |
What about renaming it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new WebAssemblyExecutor to enable secure remote Python code execution in a sandboxed WebAssembly environment using Pyodide and Deno. It includes new unit and integration tests, updates to the CodeAgent to support the new executor type, and documentation and security updates across examples, docs, README, and SECURITY files.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/test_remote_executors.py | Added tests for WebAssemblyExecutor unit and integration. |
src/smolagents/remote_executors.py | Implemented the WebAssemblyExecutor and its startup logic. |
src/smolagents/agents.py | Updated CodeAgent to include the "wasm" executor type. |
examples/sandboxed_execution.py | Provided an example usage of the WebAssembly executor. |
docs/source/en/tutorials/secure_code_execution.mdx, docs/source/en/reference/agents.mdx, SECURITY.md, README.md | Updated documentation to cover the new WebAssembly sandbox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! So If you're happy with the current state @albertvillanova, let's merge this!
We'd like to support Deno so bug reports are welcome. |
src/smolagents/remote_executors.py
Outdated
} | ||
|
||
return { | ||
result: result, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JS note: you can just do
result: result, | |
result, |
self.cleanup() | ||
|
||
JS_CODE = dedent("""\ | ||
// pyodide_runner.js - Runs Python code in Pyodide within Deno |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this would be good to put in a separate file...
src/smolagents/remote_executors.py
Outdated
for (const pkg of packages) { | ||
try { | ||
// await pyodide.loadPackage(pkg); | ||
await micropip.install(pkg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really inefficient since you are waterfalling the package installation. Micropip accepts a list as argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, definitely. I missed that in the review somehow :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your useful review! 🤗 Done.
We can consider this as an initial implementation of WasmExecutor that supports simple straightforward use cases, and merge it in its current form. We can afterwards progressively add support for other scenarios in subsequent PRs. |
Implement remote Python WebAssemblyExecutor.