[WIP] feat: support bwrap/seatbelt-based local sandbox runtime for Computer Use#5487
[WIP] feat: support bwrap/seatbelt-based local sandbox runtime for Computer Use#5487
Conversation
dosubot
bot
added
the
size:L
Soulter
changed the title
Summary of ChangesHello @Soulter, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security of the agent's local execution capabilities by introducing a Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
dosubot
bot
added
the
area:core
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 4 个安全问题，并给出了一些总体反馈：
安全问题：
- 检测到子进程函数
Popen使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用shlex.escape()。(link) - 检测到子进程函数
run使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用shlex.escape()。(link) - 检测到子进程函数
run使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用shlex.escape()。(link) - 检测到子进程函数
run使用了用户可控数据。恶意行为者可以利用这一点进行命令注入。你可以考虑使用shlex.quote()。(link)
总体评论：
LocalSandboxPolicy.ensure_writable_path()依赖Path.is_relative_to()，该方法只在 Python 3.9+ 中可用；如果这个项目仍然需要兼容 3.8 或更早版本，就需要为这个检查提供一个回退实现。- seatbelt 后端将 sandbox profile 作为单个字符串参数传递给
sandbox-exec -p；请再次确认这种调用形式在你的目标运行环境中是被支持的，如果sandbox-exec期望的是文件路径，考虑将 profile 写入临时文件后再传递。 local_booters是一个以(session_id, sandboxed)为 key 的全局字典，而且从不清理；在存在大量会话的长生命周期进程中，这个结构可能会无限增长，因此建议增加清理策略，或者在合适的场景下在会话之间复用 booter。
面向 AI 智能体的提示
Please address the comments from this code review:
## Overall Comments
- LocalSandboxPolicy.ensure_writable_path() relies on Path.is_relative_to(), which is only available in Python 3.9+; if this project still targets 3.8 or earlier you’ll need a fallback implementation for that check.
- The seatbelt backend passes the sandbox profile as a single string argument to `sandbox-exec -p`; please double-check that this invocation form is supported in your target environments, and consider writing the profile to a temp file if `sandbox-exec` expects a file path instead.
- local_booters is a global dict keyed by (session_id, sandboxed) and never pruned; in long-lived processes with many sessions this can grow unbounded, so consider adding a cleanup strategy or reusing booters across sessions where appropriate.
## Individual Comments
### Comment 1
<location path="astrbot/core/computer/booters/local.py" line_range="194-203" />
<code_context>
proc = subprocess.Popen(
wrapped_command,
shell=False,
cwd=working_dir,
env=run_env,
stdin=subprocess.DEVNULL,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'Popen' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 2
<location path="astrbot/core/computer/booters/local.py" line_range="206-215" />
<code_context>
result = subprocess.run(
wrapped_command,
shell=False,
cwd=working_dir,
env=run_env,
timeout=timeout,
stdin=subprocess.DEVNULL,
capture_output=True,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 3
<location path="astrbot/core/computer/booters/local.py" line_range="248-253" />
<code_context>
result = subprocess.run(
wrapped_command,
cwd=working_dir,
timeout=timeout,
capture_output=True,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location path="astrbot/core/computer/booters/local.py" line_range="249" />
<code_context>
wrapped_command,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args):** Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
*Source: opengrep*
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈持续改进代码审查质量。
Original comment in English
Hey - I've found 4 security issues, and left some high level feedback:
Security issues:
- Detected subprocess function 'Popen' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'. (link)
General comments:
- LocalSandboxPolicy.ensure_writable_path() relies on Path.is_relative_to(), which is only available in Python 3.9+; if this project still targets 3.8 or earlier you’ll need a fallback implementation for that check.
- The seatbelt backend passes the sandbox profile as a single string argument to
sandbox-exec -p; please double-check that this invocation form is supported in your target environments, and consider writing the profile to a temp file ifsandbox-execexpects a file path instead. - local_booters is a global dict keyed by (session_id, sandboxed) and never pruned; in long-lived processes with many sessions this can grow unbounded, so consider adding a cleanup strategy or reusing booters across sessions where appropriate.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- LocalSandboxPolicy.ensure_writable_path() relies on Path.is_relative_to(), which is only available in Python 3.9+; if this project still targets 3.8 or earlier you’ll need a fallback implementation for that check.
- The seatbelt backend passes the sandbox profile as a single string argument to `sandbox-exec -p`; please double-check that this invocation form is supported in your target environments, and consider writing the profile to a temp file if `sandbox-exec` expects a file path instead.
- local_booters is a global dict keyed by (session_id, sandboxed) and never pruned; in long-lived processes with many sessions this can grow unbounded, so consider adding a cleanup strategy or reusing booters across sessions where appropriate.
## Individual Comments
### Comment 1
<location path="astrbot/core/computer/booters/local.py" line_range="194-203" />
<code_context>
proc = subprocess.Popen(
wrapped_command,
shell=False,
cwd=working_dir,
env=run_env,
stdin=subprocess.DEVNULL,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'Popen' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 2
<location path="astrbot/core/computer/booters/local.py" line_range="206-215" />
<code_context>
result = subprocess.run(
wrapped_command,
shell=False,
cwd=working_dir,
env=run_env,
timeout=timeout,
stdin=subprocess.DEVNULL,
capture_output=True,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 3
<location path="astrbot/core/computer/booters/local.py" line_range="248-253" />
<code_context>
result = subprocess.run(
wrapped_command,
cwd=working_dir,
timeout=timeout,
capture_output=True,
text=True,
)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location path="astrbot/core/computer/booters/local.py" line_range="249" />
<code_context>
wrapped_command,
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args):** Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| proc = subprocess.Popen( | ||
| command, | ||
| shell=shell, | ||
| wrapped_command, | ||
| shell=False, | ||
| cwd=working_dir, | ||
| env=run_env, | ||
| stdin=subprocess.DEVNULL, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): 检测到子进程函数 Popen 使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用 shlex.escape()。
Source: opengrep
Original comment in English
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'Popen' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| result = subprocess.run( | ||
| wrapped_command, | ||
| shell=False, | ||
| cwd=working_dir, | ||
| env=run_env, | ||
| timeout=timeout, | ||
| stdin=subprocess.DEVNULL, | ||
| capture_output=True, | ||
| text=True, | ||
| ) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): 检测到子进程函数 run 使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用 shlex.escape()。
Source: opengrep
Original comment in English
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| result = subprocess.run( | ||
| [os.environ.get("PYTHON", sys.executable), "-c", code], | ||
| wrapped_command, | ||
| cwd=working_dir, | ||
| timeout=timeout, | ||
| capture_output=True, | ||
| text=True, |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): 检测到子进程函数 run 使用了非静态字符串。如果这些数据可以被恶意行为者控制，可能会导致命令注入。请审计此调用的使用，以确保其不受外部资源控制。你可以考虑使用 shlex.escape()。
Source: opengrep
Original comment in English
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| try: | ||
| result = subprocess.run( | ||
| [os.environ.get("PYTHON", sys.executable), "-c", code], | ||
| wrapped_command, |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args): 检测到子进程函数 run 使用了用户可控数据。恶意行为者可以利用这一点进行命令注入。你可以考虑使用 shlex.quote()。
Source: opengrep
Original comment in English
security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args): Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
Source: opengrep
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -68,11 +68,35 @@
- `FILE_UPLOAD_TOOL`：上传文件到沙盒环境
- `FILE_DOWNLOAD_TOOL`：从沙盒环境下载文件
-- **`runtime="local"`** 提供以下工具：
+- **`runtime="local"`** 或 **`runtime="local_sandboxed"`** 提供以下工具：
- `LOCAL_EXECUTE_SHELL_TOOL`：在本地环境中执行 Shell 命令
+ - 在 `local_sandboxed` 模式下，文件写入受限于工作区目录（`~/.astrbot/workspace/<session>`）
+ - 支持可选的 `cwd`（工作目录）参数
- `LOCAL_PYTHON_TOOL`：在本地环境中执行 Python 代码
+ - 在 `local_sandboxed` 模式下，文件写入受限于工作区目录（`~/.astrbot/workspace/<session>`）
这些工具在 SubAgent handoff 场景下可正常使用，与主 Agent 运行时动态挂载的工具保持一致。
+
+**本地沙箱增强模式（local_sandboxed）**
+
+`local_sandboxed` 运行时选项使用操作系统级沙箱机制增强本地执行的安全性：
+
+- **Linux 系统**：使用 `bwrap`（Bubblewrap）提供命名空间隔离
+- **macOS 系统**：使用 `sandbox-exec` 和 seatbelt 配置文件提供沙箱隔离
+- **工作区隔离**：文件写入操作严格限制在会话专属工作区（`~/.astrbot/workspace/<session>`）
+- **降级处理**：如果 `bwrap` 或 `sandbox-exec` 不可用，系统会记录警告并降级为仅限制文件系统访问至工作区
+
+**工作区配置**
+
+- 工作区根目录可通过环境变量自定义：
+ - `ASTRBOT_LOCAL_WORKSPACE_ROOT`（优先）
+ - `ASTRBOT_LOCAL_WORKSPACE`（备选）
+ - 默认路径：`~/.astrbot/workspace`
+- 每个会话获得独立的工作区子目录，确保会话间隔离
+
+**安全性说明**
+
+相比普通的 `local` 模式，`local_sandboxed` 通过操作系统级沙箱机制防止未授权访问工作区外的文件系统，提供更好的隔离性。
**工具集构建逻辑（_build_handoff_toolset）**
Note: You must be authenticated to accept/decline updates. How did I do? Any feedback? |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'local_sandboxed' runtime option for the agent's computer use, enhancing security by restricting file system access to a designated workspace. The changes modify astr_agent_tool_exec.py, astr_main_agent.py, local.py, computer_client.py, python.py, shell.py, default.py, manager.py, and several i18n files to accommodate this new runtime. The code reviewer raised concerns about the sandbox configurations allowing global read access, potential incompatibility with Python versions older than 3.9 due to the use of pathlib.Path.is_relative_to(), and the lack of an eviction policy for the local_booters cache, which could lead to memory leaks.
| if self.backend == "bwrap": | ||
| return [ | ||
| "bwrap", | ||
| "--die-with-parent", | ||
| "--new-session", | ||
| "--ro-bind", | ||
| "/", | ||
| "/", | ||
| "--bind", | ||
| str(self.workspace), | ||
| str(self.workspace), | ||
| "--proc", | ||
| "/proc", | ||
| "--dev", | ||
| "/dev", | ||
| "--chdir", | ||
| str(working_dir), | ||
| "--", | ||
| *command, | ||
| ] | ||
|
|
||
| if self.backend == "seatbelt": | ||
| workspace_escaped = _escape_seatbelt_string(str(self.workspace)) | ||
| profile = "\n".join( | ||
| [ | ||
| "(version 1)", | ||
| "(deny default)", | ||
| '(import "system.sb")', | ||
| "(allow process*)", | ||
| "(allow file-read*)", | ||
| f'(allow file-write* (subpath "{workspace_escaped}"))', | ||
| "(allow network*)", | ||
| ] | ||
| ) | ||
| return ["sandbox-exec", "-p", profile, *command] |
There was a problem hiding this comment.
The sandbox configurations for both Linux and macOS allow global read access to the host filesystem. On Linux, the entire root / is mounted as read-only (--ro-bind / /). On macOS, the profile explicitly allows all file reads ((allow file-read*)). This allows a potentially compromised LLM to exfiltrate sensitive information from the host system, such as SSH keys, configuration files, and user data, even when running in "sandboxed" mode. A sandbox should ideally restrict both read and write access to the designated workspace.
| if self.sandboxed and not abs_path.is_relative_to(self.workspace): | ||
| raise PermissionError( | ||
| f"Write path is outside workspace: {self.workspace.as_posix()}" | ||
| ) |
There was a problem hiding this comment.
The method pathlib.Path.is_relative_to() was introduced in Python 3.9. If this project supports Python versions older than 3.9, this call will raise an AttributeError, breaking the sandboxing feature for users on those versions. To maintain compatibility, you can use pathlib.Path.relative_to() within a try...except ValueError block to achieve the same check.
| if self.sandboxed and not abs_path.is_relative_to(self.workspace): | |
| raise PermissionError( | |
| f"Write path is outside workspace: {self.workspace.as_posix()}" | |
| ) | |
| if self.sandboxed: | |
| try: | |
| abs_path.relative_to(self.workspace) | |
| except ValueError: | |
| raise PermissionError( | |
| f"Write path is outside workspace: {self.workspace.as_posix()}" | |
| ) |
| def get_local_booter(session_id: str, sandboxed: bool = False) -> ComputerBooter: | ||
| key = (session_id, sandboxed) | ||
| if key not in local_booters: | ||
| local_booters[key] = LocalBooter(session_id=session_id, sandboxed=sandboxed) | ||
| return local_booters[key] |
There was a problem hiding this comment.
The local_booters dictionary is used as a cache but has no eviction policy. Since session_id is part of the key, this dictionary can grow indefinitely as new sessions are created, leading to a memory leak. Over time, this can degrade application performance and potentially cause it to run out of memory. Consider using a cache with an eviction policy, like an LRU (Least Recently Used) cache, to limit the number of LocalBooter instances stored in memory. Alternatively, if there is a session lifecycle management, LocalBooter instances should be removed when their corresponding session ends.
1 participant
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
添加一个沙盒化的本地 computer-use 运行时，用于隔离每个会话的工作空间，并与现有的 shell 和 Python 工具集成。
新功能：
local_sandboxed），在 Linux 上使用bwrap，在 macOS 上使用sandbox-exec（seatbelt），提供按会话划分的工作空间以及受限的文件系统写入。computer_use_runtime选项（none、local、local_sandboxed、sandbox）。增强：
Original summary in English
Summary by Sourcery
Add a sandboxed local computer-use runtime that isolates per-session workspaces and integrates with existing shell and Python tools.
New Features:
Enhancements: