Skip to content

fix(cosh-ng): [core] confine file writes - #2211

Open
kongche-jbw wants to merge 1 commit into
alibaba:mainfrom
kongche-jbw:fix/cosh-ng/confine-file-writes
Open

fix(cosh-ng): [core] confine file writes#2211
kongche-jbw wants to merge 1 commit into
alibaba:mainfrom
kongche-jbw:fix/cosh-ng/confine-file-writes

Conversation

@kongche-jbw

Copy link
Copy Markdown
Collaborator

Why

write_file and edit resolved paths without the pinned workspace filesystem,
so auto-approved tool calls could traverse or follow symlinks outside the
workspace. The write path also remained vulnerable to path replacement races
after validation.

What changed

  • Resolve write targets beneath pinned workspace directory descriptors.
  • Perform snapshot reads and atomic replacement relative to pinned parent FDs.
  • Reject traversal, external symlinks, and resolved-leaf symlink races.
  • Create missing parent directories safely beneath the workspace.
  • Move blocking path preparation off Tokio worker threads.

Related issue

closes #2183

User / Agent impact

write_file and edit now reject paths outside the session workspace without
modifying external files. Internal symlinks, nested directory creation, atomic
replacement, permission preservation, and edit conflict detection remain
supported.

Risk and compatibility

  • Public CLI, API, configuration, or documented behavior changed
  • Privileged or security-sensitive behavior changed

Writes that previously targeted paths outside the workspace now fail closed.
The descriptor-relative atomic replacement is covered by traversal, symlink,
root-replacement, durability, permission, and concurrency regression tests.

Validation

  • cargo test --package cosh-core tool:: — 205 passed
  • cargo fmt --all -- --check
  • cargo clippy --package cosh-core --all-targets -- -D warnings
  • git diff --check
  • Workspace-wide gates were not run; broader coverage is delegated to CI.

Documentation and rollback

No documentation changes are required. Reverting commit 2566aeba restores
the previous behavior, but also restores the workspace escape vulnerability.

@github-actions github-actions Bot added the component:cosh-ng src/cosh-ng label Aug 4, 2026
@kongche-jbw
kongche-jbw marked this pull request as ready for review August 4, 2026 10:17

@qoderai qoderai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 新增目录权限默认 0777 依赖 umask,建议收紧或继承 workspace root 权限,避免在敏感 workspace 中意外放宽访问控制。
[P2] 通过 /proc/self/fd/<fd> 重新打开目录获取锁在 Linux 受限环境下可能失败,建议在文档或错误信息中明确该前提依赖,便于调用方理解。
[P2] WorkspaceFs 错误统一透传为工具错误字符串,LLM 难以区分安全拒绝与普通路径错误,建议为安全拒绝类错误提供稳定前缀或代码。


🤖 Generated by QoderView workflow run

Comment thread src/cosh-ng/crates/cosh-core/src/tool/workspace_fs.rs
Comment thread src/cosh-ng/crates/cosh-core/src/tool/atomic_file.rs
Comment thread src/cosh-ng/crates/cosh-core/src/tool/edit.rs
@kongche-jbw
kongche-jbw force-pushed the fix/cosh-ng/confine-file-writes branch from 2566aeb to 4ab6981 Compare August 5, 2026 09:52
@kongche-jbw
kongche-jbw requested a review from SunnyQjm August 5, 2026 09:58
@SunnyQjm

SunnyQjm commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

PR number: #2211
head_sha: 4ab6981
reviewed_at: 2026-08-05T10:16:13Z

Findings

  • [P2] macOS 目录锁无互斥,并发冲突检测降级:atomic_file.rs macOS 分支 open_parent_for_lock 直接 parent.try_clone()(约 atomic_file.rs:392-397)。flock 关联 open file description,dup 出的 fd 共享同一把锁,第二次 lock_exclusive 对同一 description 是 no-op 成功。并发 replaceclone_target 从同一 WorkspaceWriteTarget 复制 fd,在 macOS 上完全失去串行化:两个并发 edit 可双双通过 snapshot 校验、后写覆盖先写且都报成功,atomic_file.rs 中"恰好一个成功"的并发测试(约 atomic_file.rs:635-656)在 macOS 上应失败或不稳定。Linux 分支特意经 /proc/self/fd 重开获得独立锁句柄,说明作者清楚该语义;macOS 无等价手段时至少应注释该平台冲突检测降级。本 head 与上轮相同,该项未收敛。

  • [P2] macOS 写路径不支持任何符号链接组件,与 PR 声明矛盾:workspace_fs.rs macOS prepare_write_beneath(约 workspace_fs.rs:361-418)逐组件 openat(..., NOFOLLOW | DIRECTORY),遇符号链接得到 ELOOP 直接映射为 Escape(约 workspace_fs.rs:408-410);叶子 symlink 的 snapshot 读走 open_targetNOFOLLOWatomic_file.rs)同样失败。PR 描述称 "Internal symlinks… remain supported",但新增 write_preserves_internal_file_and_directory_symlinkswrite_file.rscfg(unix))在 macOS 上会被这条路径拒绝。若 macOS 属支持平台则为功能回退;若有意 fail-closed,应在 PR 描述写明平台差异。本 head 未收敛。

  • [P2] mkdirat(..., 0o777) 依赖进程 umask:macOS 分支(约 workspace_fs.rs:397)与 Linux resolve_beneath 写分支(约 workspace_fs.rs:705)均以 Mode::from_raw_mode(0o777) 创建父目录,最终权限完全由 umask 决定(umask 0 时得到 0777 可写目录)。作为安全收敛 PR,新建目录建议显式收紧(如 0o755 或继承 workspace root 权限)。前序已报,本 head 未改。

  • [P2] workspace_fs.rs 规模继续增长:按 diff hunk 增量估算(末 hunk @@ -1548,14 +1755,12 @@ 之后仍有内容),改后约 1760+ 行,本 PR 净增约 200 行。standard.md §6 的 >1000/>1500 行约束与 review.md 阻断条款本轮仅由 cosh-shell gate 强制,cosh-core 无实测 gate,故按强提醒处理(上轮已指出,本 head 未收敛)。建议为写路径解析登记拆分计划(如 workspace_fs/write.rs)。行数为 diff 推算,无 CI gate 实测。

除上述外,未发现 blocking package/module/public API 组织问题:变更全部位于 cosh-core/src/tool/ owner 内;无新 crate、无新依赖、无新 root implementation 文件;新增 WorkspaceWriteTarget/prepare_write 均为 pub(super),未扩大 crate public surface;mod.rs 删除 resolve_path 属 dead code 收敛;单 crate 安全修复,无跨 crate 混杂提交。

前序已报仍未解决(供跟踪)

  • Linux 锁句柄依赖 /proc/self/fd 的环境前提未在文档或错误信息中记录。
  • 安全拒绝与普通路径错误同为裸字符串;本 PR 测试已把 "escapes workspace root" 固定为事实契约,后续改文案需同步改测试。

Open Questions

  • CI Test cosh-ng / Test cosh-ng fast checks 现已 SUCCESS;macOS 是否在测试矩阵内仍无法从输入确认——若在矩阵内,前两条 macOS 发现应已被 CI 暴露,需作者解释为何通过。
  • edit.rs 将写失败从 invoke 级 Err 改为 Ok(ToolResult::error)edit.rs:131-141),且移除了 "File not found" 前置检查(现报 "Failed to read … no such file"),属工具协议错误通道与 LLM 可见文案的行为变化,PR 描述未提及,建议确认是否有意。
  • resolve_user_path 未出现在 diff 中,~/... 展开语义是否保持原 resolve_path 行为无法从输入确认。

Validation

  • Not run:纯静态评审,未执行 cargo check/cargo test/clippy;行数为 diff 推算。
  • 输入状态:Test cosh-ngTest cosh-ng fast checks 已 COMPLETED/SUCCESS;其余 cosh-ng 相关检查(Build cosh-ng release、PR Checks、Commit Message Lint)均通过。
  • PR 自报:cargo test --package cosh-core tool::(205 passed)、cargo fmt --checkcargo clippy -D warningsgit diff --check;workspace 级 gate 未跑。

- Resolve writes beneath pinned workspace directory descriptors.
- Preserve atomic replacement while rejecting traversal and symlink races.
- Move blocking path preparation off Tokio worker threads.

Fixes: 81af2d9 ("feat(tui): add tool execution framework")
Signed-off-by: kongche-jbw <kongche.jbw@alibaba-inc.com>
@kongche-jbw
kongche-jbw force-pushed the fix/cosh-ng/confine-file-writes branch from 4ab6981 to 0617211 Compare August 6, 2026 02:57
@kongche-jbw

Copy link
Copy Markdown
Collaborator Author

已基于最新 up/mainfd32848b)rebase,并在 0617211f 中处理本轮反馈:

  • 已修复 macOS 目录锁:通过 openat(parent, ".") 重新打开父目录,获得独立 open file description,避免 try_clone 共享 flock 导致并发 edit 双写成功。
  • 已修复 macOS 内部 symlink:写路径现在基于 pinned directory descriptor 使用 statat/readlinkat 解析 file/directory symlink,校验 symlink 与已打开目录的 inode/device 身份,保留 40 层上限,并继续拒绝 workspace 外目标与跨设备路径。
  • 保留 mkdirat(..., 0o777):这与原先 create_dir_all 及 Unix mkdir 的默认语义一致,最终权限仍由进程 umask 收紧;改为固定 0755 会破坏协作 workspace 的组写权限。
  • workspace_fs.rs 拆分作为后续维护项:该文件在基线中已超过 2000 行,当前 cosh-core 没有对应阻断 gate;本安全修复不额外引入结构性重构。
  • /proc/self/fd 是 Linux WorkspaceFs 已有的启动与读取前提,并非本 PR 新增;稳定错误码需要跨工具错误模型调整,留待独立改造。

验证:

  • cargo test --package cosh-core tool:::207 passed
  • cargo clippy --package cosh-core --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • cargo check --package cosh-core --all-targets --target aarch64-apple-darwin
  • cargo clippy --package cosh-core --all-targets --target aarch64-apple-darwin -- -D warnings
  • git diff --check

本机为 Linux,因此 macOS 完成了 cross-check/cross-clippy;现有并发与内部 symlink 回归测试均为 Unix 测试,可由 macOS runner 直接执行。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cosh-ng] bug: write_file/edit 未经 WorkspaceFs 校验,auto 模式下可无感知写入 workspace 外任意路径

2 participants