Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| FileSearchTool(file_store=self.file_store_backend), | ||
| FileListTool(file_store=self.file_store_backend), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Missing sandbox field handling in to_dict serialization
Medium Severity
The new sandbox field in Agent is not added to to_dict_exclude_params and is not explicitly handled in to_dict(). This breaks the established pattern used for similar nested objects like file_store, llm, tools, and memory. Without proper handling, SandboxConfig.to_dict() and Sandbox.to_dict() won't be called during serialization, causing the type field to be missing, for_tracing logic to be bypassed, and potential issues with tracing callbacks and YAML serialization roundtrip. The same issue applies to SandboxShellTool which doesn't exclude its sandbox field from serialization.
Additional Locations (1)
| FileSearchTool(file_store=self.file_store_backend), | ||
| FileListTool(file_store=self.file_store_backend), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Sandbox and file store inconsistency when files uploaded
Medium Severity
When sandbox is enabled, __init__ treats sandbox and file_store as mutually exclusive (using elif at line 305). However, execute() doesn't check for sandbox - it creates an InMemoryFileStore and adds file tools whenever files are uploaded and file_store_backend is None. This creates a state where both sandbox tools and file tools exist but operate on separate storage backends. Uploaded files go to InMemoryFileStore (inaccessible from sandbox shell), while files created via sandbox shell remain in the E2B filesystem (inaccessible from file tools). Users uploading files to a sandbox-enabled agent won't be able to access them via shell commands.
| kwargs.pop("include_secure_params", None) | ||
| config_data = self.model_dump(exclude={"backend"}, **kwargs) | ||
| config_data["backend"] = self.backend.to_dict() | ||
| return config_data |
There was a problem hiding this comment.
API key exposure through sandbox serialization ignoring tracing flags
Medium Severity
The Sandbox.to_dict() method pops for_tracing and include_secure_params from kwargs but never uses them - it just calls model_dump() which includes all fields. For E2BSandbox, this serializes the connection field containing the E2B api_key. Additionally, SandboxConfig.to_dict() extracts for_tracing but calls self.backend.to_dict() without passing it, so the tracing-safe flag is not propagated. The BaseConnection.to_dict() method has special logic to return only id and type when for_tracing=True, but since these flags are discarded, sensitive credentials could be exposed in tracing/logging output.
Additional Locations (1)
| background: bool = Field( | ||
| default=False, | ||
| description="If True, run the command in background without waiting for output.", | ||
| ) |
There was a problem hiding this comment.
I would call this something like run_in_background to be more explicit
| # Check allowed commands | ||
| if self.allowed_commands: | ||
| is_allowed = any(cmd_lower.startswith(allowed.lower()) for allowed in self.allowed_commands) | ||
| if not is_allowed: | ||
| raise ToolExecutionException( | ||
| f"Command '{command}' is not in the allowed commands list.", | ||
| recoverable=True, | ||
| ) |
There was a problem hiding this comment.
What if the input command is ls; rm -rf /? We won't be able to handle this case if only check the command with .startswith() method
There was a problem hiding this comment.
I agree that we should have better validation
| default=None, | ||
| description="Optional list of allowed command prefixes. If set, only these commands are permitted.", | ||
| ) | ||
| blocked_commands: list[str] | None = Field( |
There was a problem hiding this comment.
It makes sense to maintain a default list of prohibited commands (such as rm -rf) and check for any matching patterns against that list.
| List of tool instances (Node objects). | ||
| """ | ||
| # Lazy import to avoid circular dependency | ||
| from dynamiq.sandbox.tools.shell import SandboxShellTool |
There was a problem hiding this comment.
Please move this import to the beginning of the file
| backend=e2b_sandbox, | ||
| ) | ||
|
|
||
| # Create shell tool that uses the sandbox |
| from dynamiq.utils.logger import logger | ||
| from examples.llm_setup import setup_llm | ||
|
|
||
| AGENT_ROLE = """ |
There was a problem hiding this comment.
lets have example with yaml
olbychos
left a comment
There was a problem hiding this comment.
add examples, restrict unsafe commands
| @@ -490,9 +502,14 @@ def execute( | |||
| if files: | |||
| if not self.file_store_backend: | |||
There was a problem hiding this comment.
What if sandbox enabled? Shoudl we still add such tools? Because we partially handle this in init
|
|
||
| return child_context | ||
|
|
||
| def cleanup(self) -> None: |
There was a problem hiding this comment.
What if we wanna reuse sandbox on next request? We are going to connect to it?
Also let's rename to def close(self) -> None: as this is more clear name for this purpose and we used if few more nodes it
|
|
||
| from dynamiq.sandbox.tools.shell import SandboxShellInputSchema, SandboxShellTool | ||
|
|
||
| __all__ = [ |
There was a problem hiding this comment.
This is redunant as we don't use import *
| background: bool = Field( | ||
| default=False, | ||
| description="If True, run the command in background without waiting for output.", | ||
| ) |
| @@ -0,0 +1,8 @@ | |||
| """Sandbox tools for command execution and file operations.""" | |||
There was a problem hiding this comment.
I'm thinking it make sense to rename sandbox folder to sandboxes to be consistent
| # Check allowed commands | ||
| if self.allowed_commands: | ||
| is_allowed = any(cmd_lower.startswith(allowed.lower()) for allowed in self.allowed_commands) | ||
| if not is_allowed: | ||
| raise ToolExecutionException( | ||
| f"Command '{command}' is not in the allowed commands list.", | ||
| recoverable=True, | ||
| ) |
There was a problem hiding this comment.
I agree that we should have better validation
| # Lazy import to avoid circular dependency | ||
| from dynamiq.sandbox.tools.shell import SandboxShellTool | ||
|
|
||
| shell_tool = SandboxShellTool( |
There was a problem hiding this comment.
How we can configure some specific tool params per Agent as here all predefined?
|
|
||
| enabled: bool = False | ||
| backend: Sandbox = Field(..., description="Sandbox backend to use.") | ||
| config: dict[str, Any] = Field(default_factory=dict) |
There was a problem hiding this comment.
I think here also need to add tools with ability to configure each of it. Or maybe in backend. Not sure for now. Idea like:
nodes:
coding-agent:
type: dynamiq.nodes.agents.Agent
sandbox:
enabled: true
backend:
type: dynamiq.sandbox.E2BSandbox
connection: e2b-conn
tools:
shell:
enabled: true
allowed_commands: ["python", "pip", "ls", "cat"]
blocked_commands: ["rm -rf", "sudo"]
| data["type"] = self.type | ||
| return data | ||
|
|
||
| def run_command( |
There was a problem hiding this comment.
let's call it run_command_shell as we can have later run_command_{type_based_on_tool}
|
Try to start from creating YAML with 2 agents that working with different sanboxes and with a different tool set (at lest configuration). This would help to build proper dependencies and components set |


Note
High Risk
Introduces remote shell command execution as an agent tool and new resource-lifecycle cleanup paths, which are security- and stability-sensitive and could enable unintended command execution or leaks if misconfigured.
Overview
Agents can now be configured with an optional
sandboxthat injects sandbox-provided tools (currently ashellcommand tool) instead of the built-in file-store tools when enabled.This introduces a new
dynamiq.sandboxmodule with a baseSandbox/SandboxConfiginterface, anE2BSandboximplementation usinge2b-desktop, andSandboxShellTool(with simple allow/block prefix validation) plus an example workflow showing E2B usage. Agent lifecycle handling is updated to attempt sandbox cleanup viacleanup()/__del__, and dependencies are updated to adde2b-desktop(and bumppillowvia lockfile changes).Written by Cursor Bugbot for commit 532ddb8. This will update automatically on new commits. Configure here.