fix(extension-agent): fallback when bwrap is unusable#801
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough在本次变更中,非 Windows 平台的 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a functional check for Bubblewrap availability before it is used for sandboxing, including a caching mechanism for the probe results. The review feedback identified a risk of process crashes due to missing error handling in the probe function and noted that temporary directories created during the check were not being cleaned up.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/extension-agent/src/computer/backends/local/sandbox.ts (1)
115-118: 建议在 bwrap 探测失败时添加日志记录。当
canUseBubblewrap返回 false 时,命令将以无沙箱模式运行(fail-open 设计)。这是一个安全相关的行为变化,用户应该知道沙箱被禁用了。缺少日志会导致用户无法感知系统正在以较低安全级别运行。♻️ 建议添加警告日志
需要在文件顶部添加 logger 导入(根据项目现有模式),然后:
if (!canUseBubblewrap(bwrap, tmp)) { + // Consider logging: logger.warn('bwrap detected but unusable, running command without sandbox') return command }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extension-agent/src/computer/backends/local/sandbox.ts` around lines 115 - 118, When canUseBubblewrap(bwrap, tmp) returns false in sandbox.ts the code currently silently falls back to unsandboxed execution; import the project's logger used elsewhere in this module (add the same logger import pattern at the top of the file) and add a warning log immediately before returning the unsandboxed command (include context like bwrap path/flags and tmp directory and mention that bubblewrap detection failed and execution will be unsandboxed). Keep the return(command) behavior (fail-open) but ensure the warning is logged using the module's logger so operators can see the reduced security mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/extension-agent/src/computer/backends/local/sandbox.ts`:
- Around line 164-198: The canUseBubblewrap probe can throw from fs.mkdirSync
which would bubble up and break the fail-open design; update the
canUseBubblewrap function to catch exceptions around fs.mkdirSync (and any
filesystem prep) and on error log/debug if desired and return false so the probe
fails closed to sandbox usage without throwing, preserving existing caching via
BWRAP_PROBE_CACHE and keeping the rest of the spawnSync logic unchanged.
---
Nitpick comments:
In `@packages/extension-agent/src/computer/backends/local/sandbox.ts`:
- Around line 115-118: When canUseBubblewrap(bwrap, tmp) returns false in
sandbox.ts the code currently silently falls back to unsandboxed execution;
import the project's logger used elsewhere in this module (add the same logger
import pattern at the top of the file) and add a warning log immediately before
returning the unsandboxed command (include context like bwrap path/flags and tmp
directory and mention that bubblewrap detection failed and execution will be
unsandboxed). Keep the return(command) behavior (fail-open) but ensure the
warning is logged using the module's logger so operators can see the reduced
security mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e3a0320d-8b4b-46ed-9140-aa65b13ec4c1
📒 Files selected for processing (1)
packages/extension-agent/src/computer/backends/local/sandbox.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd27b5272a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
hyw,直接关掉 bwrap 吗?那沙箱怎么办 |
Fixes #799
问题
在 Linux 环境下,
chatluna-agent的 local backend 只要检测到bwrap存在,就会无条件为命令套上 bubblewrap sandbox。这在 Ubuntu 24.04 的 AppArmor / unprivileged user namespace 限制环境里会导致 local 命令整体失败,典型报错包括:
bwrap: Can't mount proc on /newroot/proc: Operation not permittedbwrap: setting up uid map: Permission denied根因
问题不在于
bwrap是否安装,而在于bwrap在当前宿主环境中是否真的可用。Ubuntu 24.04 默认启用了对 unprivileged user namespace 的限制。此时即使系统里已经安装了
bwrap,它也可能因为 AppArmor / userns 限制而无法初始化 sandbox,最终把 local backend 的命令执行一并打死。修改方式
本 PR 只修改了一个文件:
packages/extension-agent/src/computer/backends/local/sandbox.ts修改内容:
bwrapbwrap不存在时直接返回原命令bwrap存在时,先执行一次最小bwrap探测也就是说,这个改动把原来的:
bwrap不存在 => 裸跑扩展为:
bwrap不存在 => 裸跑bwrap存在但当前环境不可用 => 裸跑bwrap存在且可用 => 继续使用 sandbox设计取舍
这是一个 runtime mitigation,不是系统级根治。
它不试图让 ChatLuna 管理宿主机的 AppArmor profile 或 sysctl,也不改变
bwrap正常可用环境下的既有行为。目标只是避免 local backend 因为“检测到bwrap已安装,但实际上无法初始化”而直接失效。验证方式
仓库当前没有自动化测试框架,因此本次验证为定向手工验证。
1. 本地函数级验证
bwrap:wrapCommandWithSandbox()继续返回带bwrap的包装命令bwrap:wrapCommandWithSandbox()返回原始命令2.
LocalComputerSession.execute()验证bwrap环境:execute('printf ok')成功,stdout=okbwrap环境:execute('printf ok')仍成功,stdout=ok3. 真实 Ubuntu 24.04 + AppArmor VM 对照验证
在本地完整 Ubuntu 24.04 VM 中进行了原始版本与补丁版本对照验证:
kernel.apparmor_restrict_unprivileged_userns=1时bwrap包装命令并失败kernel.apparmor_restrict_unprivileged_userns=0时bwrap正常执行bwrap,不破坏正常路径对照结果说明:
bwrap不可用时回退bwrap正常可用路径风险边界
bwrap”时直接返回原命令