Repository navigation
fix(sandbox): oncall 文件沙盒下 auto-worktree 失败改为 fail-closed,杜绝回退真实目录 - #921
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13a8c62ecc
ℹ️ 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".
| c.cliId, undefined, c.backendType, config.daemon.backendType, | ||
| ) === 'riff'; | ||
| if (riff) return false; | ||
| return c.sandbox === true || c.readIsolation === true || process.env.BOTMUX_SANDBOX === '1'; |
There was a problem hiding this comment.
Include API-only isolation in fail-closed detection
For an apiOnly: true bot with no explicit sandbox flags, this condition leaves failClosed false, but forkWorker later forces readIsolation for every no-transport session in src/core/worker-pool.ts:6859-6860. If auto-worktree creation fails during a Dashboard-created session, this helper therefore falls back to the real default directory and launches the forced sandbox there, preserving the exact real-project write exposure this change is intended to prevent; include the no-transport/API-only isolation condition here as well.
AGENTS.md reference: AGENTS.md:L68-L68
Useful? React with 👍 / 👎.
| try { | ||
| await notify(t('worktree.auto_fail_closed', { dir: baseDir, error: e.reason }, localeForBot(larkAppId))); | ||
| } catch { /* notices are best-effort — never mask the refusal */ } |
There was a problem hiding this comment.
Make refusal cleanup atomic with the takeover check
When pendingRepo is still true at the check above but a notifier adoption consumes it while this awaited network notification is in flight, execution resumes and removes the now-live session from activeSessions and marks its durable record closed. This reopens the takeover race the preceding guard is meant to prevent and leaves the adopted worker orphaned; perform the cleanup before awaiting the notice or recheck pendingRepo immediately afterward under the relevant admission/ownership guard.
Useful? React with 👍 / 👎.
|
感谢这个修复 🙏 方向完全正确——文件沙盒 DIRECT 模式把 workingDir 读写直绑真实宿主目录,auto-worktree 是唯一的隔离屏障,失败时静默回退真实目录确实是逃逸,改成 fail-closed 是对的。构建通过、测试覆盖也很扎实。 自动评审时发现一处会在合入当前 master 时立即触发的问题,供参考(这是初步的自动评审意见,最终以维护者审阅为准):
PR 的 base 早于 mojo 后端(#803)合入,在那个 base 上 worker 的豁免就等价于「非 riff」,所以写死 把本 PR 合到当前 master 后实测:一个 建议修法(避免手抄 worker 的 union 再次漂移):把 worker 那段 sandbox 判定抽成一个依赖轻的共享纯谓词, 两点要点:① 远端豁免要包住整个 union(含 env 强制项);② mojo 的远端证明不止 另外 @ 的自动评审已在 PR 里留了两条 inline(apiOnly / 无 Lark transport 会话会被 forkWorker 强制 readIsolation,同样要纳入谓词;以及 refusal cleanup 在 |
13a8c62 to
2b5547f
Compare
|
自动评审初步意见(以维护者最终审阅为准):R2 的共享谓词修复了静态判定漂移,但目前仍有一个可达的 fail-open 时间窗口。
最小运行时复现(当前 master 合并 PR head):
因此 gate 可按旧值允许回退,而 worker 按新值在真实项目目录开启本地文件沙盒,原本要堵住的写入逃逸仍可能发生。建议在 pending session 建立时冻结 worker 将采用的安全决策输入,并让 auto-worktree gate 与 fork 都消费该 session 快照;仅在 fallback 后重读 live config 仍会留下 recheck→fork 的 await 窗口。建议补一条 worktree/notice in-flight 时 sandbox off→on 的回归测试。 |
2b5547f to
21f415e
Compare
76fb650 to
2f5da62
Compare
oncall 文件沙盒(bwrap/Seatbelt DIRECT 模式)把 workingDir 读写直绑宿主真实目录, auto-worktree 是阻止 Agent 写入真实项目的唯一隔离层。此前 worktree 创建失败(非 git / git 失败)会静默回退真实默认目录,沙盒会话的写入直接落进真实项目。 - default-worktree:新增 AutoWorktreeFailClosedError 与 autoWorktreeMustFailClosed; 本次 spawn 实际会跑在本地 oncall 沙盒下时,失败直接拒绝而非回退。scratch(全根 COW, 宿主零改动)与远端后端(riff / remote-runner / 可证明全远端的 mojo)不受影响、照常回退; 显式 /tw(force)保持原有 fail-closed 语义(会话留在 pendingRepo)。 - sandbox:抽出 resolveLocalSandboxMode 共享解析器(localSandboxApplies 远端豁免包住 整个 resolveSandboxMode),worker 的 sandboxMode 与 gate 复用同一函数,避免再次漂移; 非法 sandbox 值 / BOTMUX_SANDBOX 在 gate 侧按 fail-closed 处理。 - worker-pool:把 forkWorker 内联的沙盒决策冻结抽成 freezeSessionSandboxDecision; runAutoWorktreeCommit 在任何 await 之前先冻结,gate 与后续 fork 消费同一快照,关闭 「构建途中 PUT /api/bot-sandbox off→on,gate 按旧值回退、fork 按新值在真实目录开沙盒」 的 fail-open 窗口。gate 同时使用会话自身的 cliId / backend stamp 解析有效后端。 - card-handler:fail-closed 时关闭未启动的挂起会话并提示修复方式;await 通知前后两次 检查 pendingRepo,避免误关被 notifier 接管的会话。 基于原 PR deepcoldy#921 的实现 rebase 到当前 master 并适配三态沙盒 / remote-runner 等新形态。 Co-authored-by: huangyuhang.edu <huangyuhang.edu@bytedance.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
已 rebase 到最新 master。因为冲突较多,重写成了一个提交,原作者保留 Co-authored-by 署名。主要调整:gate 改用与 worker 共享的三态解析器 🤖 Generated with Claude Code |
|
自动评审初步意见(以维护者最终审阅为准)。已基于最新 master(4be4e6f6a)本地 rebase 复审新提交 2f5da62。 历史评审意见核对:均已解决。 三态共享解析器 两点新意见 + 一个 rebase 注意项:
|
|
补充上一条的 P2:该 fail-open 残口已由第二位评审用独立运行时探针复现,不是纯静态推断——直接调用生产函数
即合入前需:① 修 P2(pending 建立时连 mojo 身份与证明输入一并冻结,或冻结 gate 结论、fork 侧只许加强不许减弱);② 补 P3「进入 gate 时 snapshot 与 live 已分歧」两个方向的用例。 |
oncall 文件沙盒(bwrap/Seatbelt DIRECT 模式)把 workingDir 读写直绑宿主真实目录, auto-worktree 是阻止 Agent 写入真实项目的唯一隔离层。此前 worktree 创建失败(非 git / git 失败)会静默回退真实默认目录,沙盒会话的写入直接落进真实项目。 - default-worktree:新增 AutoWorktreeFailClosedError 与 autoWorktreeMustFailClosed; 本次 spawn 实际会跑在本地 oncall 沙盒下时,失败直接拒绝而非回退。scratch(全根 COW, 宿主零改动)与远端后端(riff / remote-runner / 可证明全远端的 mojo)不受影响、照常回退; 显式 /tw(force)保持原有 fail-closed 语义(会话留在 pendingRepo)。 - sandbox:抽出 resolveLocalSandboxMode 共享解析器(localSandboxApplies 远端豁免包住 整个 resolveSandboxMode),worker 的 sandboxMode 与 gate 复用同一函数,避免再次漂移; 非法 sandbox 值 / BOTMUX_SANDBOX 在 gate 侧按 fail-closed 处理。 - worker-pool:把 forkWorker 内联的沙盒决策冻结抽成 freezeSessionSandboxDecision; runAutoWorktreeCommit 在任何 await 之前先冻结,gate 与后续 fork 消费同一快照,关闭 「构建途中 PUT /api/bot-sandbox off→on,gate 按旧值回退、fork 按新值在真实目录开沙盒」 的 fail-open 窗口。gate 同时使用会话自身的 cliId / backend stamp 解析有效后端。 - card-handler:fail-closed 时关闭未启动的挂起会话并提示修复方式;await 通知前后两次 检查 pendingRepo,避免误关被 notifier 接管的会话。 基于原 PR deepcoldy#921 的实现 rebase 到当前 master 并适配三态沙盒 / remote-runner 等新形态。 Co-authored-by: huangyuhang.edu <huangyuhang.edu@bytedance.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2f5da62 to
ad59da0
Compare
|
已再次 rebase 到最新 master,并解决了与 #1849(死代码清理 / noUnusedLocals)的冲突:补回 card-handler 中 fail-closed 收尾所需的 🤖 Generated with Claude Code |
gate 在构建前判定「不施加本地沙盒」并回退真实默认目录后,mojo 远端证明等输入 仍可能在构建途中被改(如 cloud 翻为 false),fork 时 worker 解析为本地 oncall, 从而在真实目录上读写直绑。现在回退时把结论持久化到会话(autoWorktreeFallbackDir), worker 在解析出 oncall 且 workingDir 即该目录时拒绝启动;off / scratch 不受影响。 同时补齐「进入 gate 时快照与 live 已不一致」两个方向的用例,并用复现场景写回归测试。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
已追加提交 a37996c,针对上面两条意见: P2(mojo 远端证明的 TOCTOU):gate 判定可以回退到真实默认目录时,会把这个结论持久化到会话上( P3:补了「进入 gate 时快照和 live 已经不一致」两个方向的用例:快照 ON / live OFF 必须拒绝,快照 OFF / live ON 必须回退。变异验证:把 gate 改回读 live 后,这两条加上 scratch 快照用例共 3 条变红;去掉 worker 侧的 throw、去掉 card-handler 的标记写入,也会分别让对应的测试变红。
🤖 Generated with Claude Code |
|
自动评审跟进(初步意见,以维护者最终审阅为准):新提交 a37996c 已基于最新 master(3f5838f87,两提交,rebase 干净、build 通过),上一轮两点均已解决,本地复审未发现新问题。 P2(mojo 证明输入 TOCTOU 逃逸残口)— 已修。 采用了「fork 侧只许加强」方案:gate 在 T0 判定回退真实默认目录时,把该结论持久化到会话( P3(测试缺口)— 已补。 新增「进入 gate 时快照与 live 已分歧」两个方向用例,并用复现场景写了 T0 回退 → 构建中 cloud 翻 false → T1 oncall 被拒的端到端回归;另含标记持久化/真 worktree 不落标/守卫纯函数(off/scratch/异目录/无标记放行)与接线测试。 变异验证(均在本地新基线上做、做完即还原):
测试: PR 四测试文件 141 全绿;本地沙盒/mojo blast-radius 除 2 个在 pristine master 同样红的 root EACCES 环境用例外全绿;CI 十腿(build / bun-test / test×3 分片 / network-boundary / bun-binary×3)全绿。 |
|
🚀 Released in v3.44.0 |
优先级:P0
根因
文件沙盒会把工作目录以读写方式绑定到宿主机,auto-worktree 是避免 Agent 写入真实项目的隔离层。此前 auto-worktree 创建失败时会静默回退真实默认目录,导致沙盒会话的新文件直接落入真实项目。
改动
AutoWorktreeFailClosedError,关闭挂起会话并提示修复默认目录或通过/repo选择目录。config.sandbox、legacyreadIsolation和BOTMUX_SANDBOX=1。影响面
涉及 auto-worktree 启动路径及本地 sandbox/readIsolation 会话;普通非沙盒会话继续允许降级,riff 远端后端行为不变。PTY/Tmux 的后续 worker 运行路径未改动。
验证
BOTMUX_SANDBOX=1以及接管竞态回归。pnpm build通过。维护者跟进
为什么重写成一个提交
PR 分出去之后,master 已经前进了 900 多个提交:会话存储、文件沙盒(新增 off / oncall / scratch 三态)、remote-runner 后端都有大改。原来 7 个提交逐个 rebase 冲突太多,所以把它们合成一个提交,再按 master 现状重新适配。原作者保留
Co-authored-by署名。三处设计调整
resolveLocalSandboxMode(src/adapters/backend/sandbox.ts):在resolveSandboxMode外面包一层localSandboxApplies远端豁免。worker 的sandboxMode和 auto-worktree gate 调用同一个函数,以后不会再出现两边判定不一致。只有解析结果是oncall时才 fail-closed。远端后端(riff / remote-runner / 可证明全远端的 mojo)被豁免,覆盖 config、legacy readIsolation 和BOTMUX_SANDBOX全部来源。sandbox 值或BOTMUX_SANDBOX非法、bot 不存在时,gate 按 fail-closed 处理。Session.readIsolation新字段:freezeSessionSandboxDecision直接沿用 master 中 forkWorker 原有的冻结逻辑:三态、legacy readIsolation 并入 oncall、scratch 选项、networkPolicy。SpawnOpts 的readIsolation保持 master 原样。原因是PUT /api/bot-read-isolation在该 bot 有任何活跃会话时都会拒绝,而 pendingRepo 占位会话就在 activeSessions 里,所以构建途中这个开关切不动。没有这道保护的PUT /api/bot-sandbox由冻结来兜住。sandboxSnapshot):只有 CLI 本身是远端 CLI 时,reconcileRiffBackendType才保留远端后端。会话自选本地 CLI 时,如果按 bot 级的 cliId 判断,会被误判成 riff 豁免。有意保留的两处设计
/tw的沙盒拒绝直接关闭挂起会话:auto-worktree 的入口包括新话题、dashboard 创建和 webhook/trigger,后两者没有人会回来执行/repo自救,挂着不关会越积越多。显式/tw是用户主动发起的,保持 master 原有语义:会话停在 pendingRepo,并提示用/repo选择仓库。已处理的 review 意见
buildEffectiveMojoConfig+botInjectedEnv)。await notify前后各检查一次pendingRepo,测试已覆盖。runAutoWorktreeCommit在第一个 await 之前就冻结,gate 和 fork 用同一份快照。新增回归测试:构建途中翻转 live 配置后,断言 fork 时session.sandbox仍为 false。影响面
core/worker-pool.ts(冻结逻辑抽成函数,行为不变)、worker.ts(sandboxMode改走共享解析器,结果与原逻辑等价)、adapters/backend/sandbox.ts。/tw的行为都不变。与 CLI 种类无关,PTY 和持久后端(tmux 等)走同一条路径。测试
bun run build:通过(包括 tsc、typecheck:scripts、typecheck:test-mocks、dist 审计)。bun run vitest run --project unit default-worktree session-sandbox-freeze riff-sandbox-bypass card-handler-repo-select api-only-mode-wiring sandbox-mode backend-gate read-isolation trigger-session dashboard-create-session codex-notifier pending-repo scratch:36 个文件、828 项全部通过。plugin-registry-sandbox-read和plugin-mcp-sandbox有失败:前者在未改动的 master 上同样失败,后者在 canonical checkout 里能通过,与本改动无关。🤖 Generated with Claude Code