Skip to content

fix(code-executor): 修复 Windows 宿主到 Container workspace 的路径兼容性 - #251

Open
2021210507 wants to merge 4 commits into
trpc-group:mainfrom
2021210507:fix/container-workspace-windows-paths
Open

fix(code-executor): 修复 Windows 宿主到 Container workspace 的路径兼容性#251
2021210507 wants to merge 4 commits into
trpc-group:mainfrom
2021210507:fix/container-workspace-windows-paths

Conversation

@2021210507

Copy link
Copy Markdown

概述

修复 Windows 宿主机运行 Docker Container workspace 时的路径兼容性和清理稳定性问题。

Container 内部运行的是 Linux,但 SDK 原先在构造 Container workspace 路径时使用宿主机 pathlib.Path。在 Windows 上会生成 \,随后被传入 Linux Container 的 shell 命令和文件 stage 路径,可能导致 workspace、Skill 或输入文件路径不正确。

修改内容

  • 使用 PurePosixPath 统一构造 Container 内部路径,确保始终使用 /
  • 覆盖 workspace 创建、Skill/input stage、tar 写入、metadata 读写等 Container 内部路径。
  • Container 清理完成后将 _container 置为 None,使析构期或重复清理成为无操作。
  • local workspace 清理时恢复只读 staged Skill 文件的写权限,确保 finally 阶段可完成清理。
  • 新增 3 条回归测试:
    • Windows 宿主下 Container workspace 命令不含反斜杠;
    • Container 清理后不再重复 stop/remove;
    • local runtime 可清理只读 staged Skill 文件。

验证

使用仓库 .venv 执行:

.\.venv\Scripts\python.exe -m pytest \
  tests/code_executors/container/test_container_ws_runtime.py \
  tests/code_executors/container/test_container_cli.py::TestContainerClientCleanup \
  tests/code_executors/local/test_local_ws_runtime.py::TestLocalWorkspaceManager \
  -q -p no:cacheprovider --basetemp .pytest-sdk-pr-regression

结果:126 passed

.\.venv\Scripts\python.exe -m flake8 \
  trpc_agent_sdk/code_executors/container/_container_cli.py \
  trpc_agent_sdk/code_executors/container/_container_ws_runtime.py \
  trpc_agent_sdk/code_executors/local/_local_ws_runtime.py

结果:零违规。

范围

本 PR 仅修复共享 CodeExecutor workspace runtime:

  • 3 个 SDK 运行时文件;
  • 3 个对应 SDK 测试文件。

不包含自动代码评审 Agent 示例、Skill、数据库或规则逻辑。

该问题在验证自动代码评审 Agent 的 Windows + Container 路径时发现。

关联 #92

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.74359% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@81c798a). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../code_executors/container/_container_ws_runtime.py 92.53731% 5 Missing ⚠️
...gent_sdk/code_executors/local/_local_ws_runtime.py 70.00000% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main        #251   +/-   ##
==========================================
  Coverage        ?   87.86169%           
==========================================
  Files           ?         482           
  Lines           ?       45204           
  Branches        ?           0           
==========================================
  Hits            ?       39717           
  Misses          ?        5487           
  Partials        ?           0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

基于我对 pr.diff 及相关上下文代码的全面审查,以下是我的审查结论:

发现的问题

🚨 Critical

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:589_stage_workspace_inputdst 未做 POSIX 归一化,Windows 宿主上会注入反斜杠导致容器内命令失败

    • _stage_workspace_input(src, dst, mode) 直接把 dst(即 dst_abs)拼进 bash 命令 ln -sfn '{src}' '{dst}' / cp -a '{src}' '{dst}',但本 PR 的修复目标正是消除容器路径中的反斜杠。src_container_path 已归一化,parent 也经 _container_parent 归一化,唯独 dst 本身仍原样使用。当 ws.pathspec.dst\(Windows 配置或用户传入)时,'{dst}' 会含反斜杠,在 bash 中被解释成转义/拼接,造成链接/拷贝目标错误或失败。应改为 dst = _container_parent(dst) 的同款归一化(如 dst.replace("\\","/") 或复用 _container_path)后再插入命令。
    ...
    parent = PurePosixPath(_container_parent(dst))
    if mode == "link":
        cmd_str = (f"[ -e '{parent}' ] || mkdir -p '{parent}'; "
                   f"ln -sfn '{src}' '{dst}'")  # dst 未归一化
    ...
  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:573577_stage_host_inputdst 同样未归一化,与上一条同根因

    • link/copy 分支里 '{dst}' 直接取自 dst_abs,而 parent 已用 _container_parent(dst) 归一化。若 dst_abs 含反斜杠,mkdir -p "$parent" 可能成功但 ln -sfn '...{dst}' 的目标路径错误。修复方式同上,统一对 dst 做 POSIX 归一化。

⚠️ Warning

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:784run_programcwd = f"{ws.path}/{spec.cwd}" 仍用字符串拼接,未走 _container_path

    • 本 PR 把容器路径全面替换为 _container_path 以避免反斜杠,但 spec.cwd(来自 WorkspaceRunProgramSpec.cwd,外部可传入)直接用 / 拼接,若 spec.cwd\cd '{cwd}' 失败。属同类遗漏,建议 cwd = _container_path(ws.path, spec.cwd) if spec.cwd else ws.path。该行不在 diff 新增范围但属同一修复目标的一致性问题,且可从 WorkspaceRunProgramSpec.cwd 字段验证。
  • trpc_agent_sdk/code_executors/local/_local_ws_runtime.py:153_remove_read_only_path 仅设 S_IWRITE(owner write),对 group/other 无写权限的只读目录删除可能失败

    • _make_tree_read_only& ~0o222 清除 owner/group/other 全部写位(_files.py:make_tree_read_only 同样)。onerror 回调在删除目录(非文件)时被触发,os.chmod(path, stat.S_IWRITE) 只恢复 owner 写位;当目录本身因 group/other 写位被清而触发删除失败时通常足够,但对其中文件onerror 仅恢复 owner 写、若文件原 mode 为 0o444(本 PR 测试用 S_IREAD0o400)能恢复;但若实际只读是 0o0444(group/other 只读无影响,删除看 owner)通常可过。真正的边界风险在于:当 function(path) 再次失败(如权限仍不足或只读文件系统)时异常会逃逸导致 rmtree 抛错、workspace 残留。建议在回调内 try/except 容错或用 stat.S_IWRITE | current_mode 保底,与 make_tree_read_only 的清位语义对称。
  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:98-107_container_path/_container_parentNone 分段会抛 AttributeError

    • 两个辅助函数对每个 part 调 str(part).replace(...),若上游传入 None(如 spec.dstNone 而非空串、或 rel_pathPath 但某处传 None)会崩。当前调用点多用 or 兜底为字符串,但 dst 直接来自 spec.dst.strip(),若 WorkspaceInputSpec.dst 允许 None(pydantic 默认可空)则 dst_rel 计算会触发。建议在函数内 str(part) if part is not None else ""

💡 Suggestion

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:541,554_put_bytes_tardest.replace("\\","/")_container_path 逻辑重复
    • 已有 _container_parent/_container_path 统一封装 POSIX 归一化,此处又手写 PurePosixPath(dest.replace(...))。可抽一个 _container_name 或复用 PurePosixPath(_container_parent(dest).replace(...)) 风格,避免三处 replace("\\","/") 散落,降低后续维护时漏改风险。

总结

本 PR 的 Windows 路径归一化方向正确,但 _stage_workspace_input / _stage_host_inputdst 本身未被归一化(仅 parentsrc 被归一化),构成与修复目标自相矛盾的 Critical 缺陷;run_programcwd 拼接为同类遗漏。存在必须修复的问题。

测试建议

  • 新增针对 stage_inputsworkspace://host:// link/copy 分支)在 ws.path / spec.dst 含反斜杠时断言生成的 bash 命令中 dst 不含 \ 的测试,覆盖上述 Critical。
  • run_programspec.cwd\ 时补一条命令生成测试,验证 cwd 已归一化。

cmd_str = (f"parent='{_container_parent(dst)}'; "
f"[ -e \"$parent\" ] || mkdir -p \"$parent\"; "
f"cp -a '{container_src}' '{dst}'")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_stage_workspace_input 中 dst 未做 POSIX 归一化

dst(即 dst_abs)直接拼进 ln -sfn '{src}' '{dst}' / cp -a '{src}' '{dst}',src 与 parent 已归一化但 dst 本身未归一化。Windows 宿主上 ws.path 或 spec.dst 含反斜杠时,命令在 bash 中会被错误解释为转义/拼接,导致链接/拷贝目标错误或失败,与本 PR 消除反斜杠的目标自相矛盾。建议对 dst 同样复用 _container_path/_container_parent 做归一化后再插入命令。

@@ -552,14 +565,14 @@ async def _stage_host_input(self, ws: WorkspaceInfo, host: str, dst: str, mode:
if self.config.inputs_host_base:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_stage_host_input 中 dst 同样未归一化

link/copy 分支里 '{dst}' 直接取自 dst_abs,而 parent 已用 _container_parent(dst) 归一化,同根因遗漏。若 dst_abs 含反斜杠,mkdir -p 可能成功但 ln -sfn 目标路径错误。建议统一对 dst 做 POSIX 归一化。

@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

发现的问题

⚠️ Warning

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:98-101_container_path 仅做 \\/ 替换后用 PurePosixPath 拼接,但 PurePosixPath 不会对 .. 做归一化,传入包含 .. 的相对片段(如来自用户 spec.dst/spec.cwd)时会原样保留,旧代码用 os.path.join+os.path.normpath 的本地路径(path_join)则会归一化。容器侧由此可能生成形如 /tmp/run/ws_x/work/../../../etc 的路径,被 cd/mkdir -p 解释后逃逸出 workspace。建议在归一化后补充 .resolve() 或显式 reject 包含 .. 的相对片段;至少应保证与本地 runtime 的 path_join 行为一致。

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:572-598292306:容器路径被以裸单引号方式拼入 bash -lc 字符串('{container_dst}''{container_src}/.'),而 container_dst/container_src 最终可源自用户可控的 spec.dstspec.cwd(见 stage_inputs_stage_host_input/_stage_workspace_inputrun_program)。若其中含单引号即构成命令注入。这是旧代码既有问题、本 PR 未引入回归,但本 PR 在多条新命令上沿用了同一不安全拼接方式且未用已有的 _shell_quote。建议统一对这些路径走 _shell_quote,避免后续扩展放大风险。

  • trpc_agent_sdk/code_executors/local/_local_ws_runtime.py:150-154_remove_read_only_path 对任何 onerror 回调都先 os.chmod(path, stat.S_IWRITE) 再调用 function(path)。当 functionos.rmdir 且目录非空、或为 os.listdir 遇到非权限类错误(如路径不存在)时,os.chmod 要么无意义要么抛 FileNotFoundError 掩盖原始错误,使 rmtree 在异常路径上行为不稳定。建议仅对 PermissionError 处理(参考官方 shutil.rmtree 文档示例),其他异常重新抛出。

💡 Suggestion

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:104-107_container_parent 的 docstring 提到“兼容 Python 3.10 的 f-string 语法限制”,但函数体并未在 f-string 中使用、也无版本相关分支,注释具有误导性;_put_bytes_tarPurePosixPath(dest.replace("\\", "/"))_container_parent/_container_path 逻辑重复。建议更正注释并统一复用 _container_path/_container_parent,减少重复的 replace("\\","/") 散点。

总结

本 PR 主要是 Windows 宿主下容器路径 POSIX 归一化、容器引用释放(取消注释 self._container = None)及本地只读文件清理三类修复,方向正确且测试覆盖了对应路径。未发现必须立即修复的阻塞性问题;但路径归一化未做 .. 规范化、容器命令对用户可控路径仍裸拼接,存在既有但被本 PR 沿用的注入/逃逸隐患,建议一并收敛。

测试建议

  • 补充 spec.dst / spec.cwd.. 或绝对路径时 _stage_host_input_stage_workspace_inputrun_program 的断言,确认生成命令不会逃逸出 workspace。
  • 补充 spec.dst 含单引号等特殊字符时 _stage_* 命令不发生注入(或显式拒绝)。

@2021210507

Copy link
Copy Markdown
Author

我已阅读CLA文件,特此签署CLA

@2021210507

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@2021210507

Copy link
Copy Markdown
Author

recheck

Rook1ex added a commit to trpc-group/cla-database that referenced this pull request Jul 29, 2026
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

基于我对 diff 及相关上下文的全面分析,以下是我的审查结果:

发现的问题

🚨 严重

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:120-128_container_relative_path 的路径逃逸检测存在缺口,对 workspace://skill:// 来源的相对逃逸无法拦截
    • dst 参数(stage_directory/stage_inputs)检测时走 _container_relative_path(dst, allow_current=True),而 posixpath.normpath("work/../..") 结果为 ../..,能被 normalized.startswith("../") 拦截;但 workspace://skill:// 走的是 removeprefix 后传入的相对路径同样能被拦截——实际真正问题在 _stage_workspace_inputsrc/dstsrc 来自已校验的 workspace://,但 dst_abs_container_path(ws.path, dst_rel) 拼出的绝对路径再传给 _stage_workspace_input(dst),这里 dst 是绝对路径,_container_path 不会再校验逃逸,导致 ln -sfn <src> <dst>dst 若包含 .. 仍可越界创建软链。建议在 _stage_workspace_inputdst 再次做 _container_relative_path 校验或强制以 ws.path 为根做 relative_to 断言。
    ...
    container_dst = _container_path(dst)   # dst 已是绝对路径,不再校验
    ...

⚠️ 警告

  • trpc_agent_sdk/code_executors/local/_local_ws_runtime.py:146-158shutil.rmtree 仍使用已弃用的 onerror 回调,且 _remove_read_only_path 重新 raise exception 时若 function(path) 再次失败会抛新异常掩盖原始错误

    • 项目 requires-python >= 3.10onerror 自 3.12 起弃用并将在未来版本移除;若 CI 跑在 3.12+ 会产生 DeprecationWarning。此外回调内 os.chmodfunction(path) 若失败会以 chmod 后的异常冒泡,而非原始 PermissionError,偏离"保留原始失败语义"的注释意图。建议改用 onexc=(3.12+)或在 try/except 内对 chmod 后的二次失败回退为 raise exception
  • trpc_agent_sdk/code_executors/local/_local_ws_runtime.py:156os.stat(path, follow_symlinks=False) 在清理回调中调用,若 path 在 rmtree 过程中已被删除会抛 FileNotFoundError

    • rmtree 的 onerror 回调触发时 path 是当前失败节点,但并发或父目录被删后该 os.stat 会抛非 PermissionError,被 if not isinstance(exception, PermissionError) 分支重新 raise,导致清理整体失败而原始 PermissionError 信息丢失。建议对 os.stat 单独 try/except,stat 失败时直接 raise exception
  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:103_container_path 使用 PurePosixPath 拼接但先对每段做 replace("\\", "/"),对含合法反斜杠的路径名会产生意外语义变更

    • 若容器内文件名本身含反斜杠(合法 POSIX 文件名字符),该转换会把单文件名拆成多级目录,导致路径错配。低概率但属正确性隐患;建议仅在确认是 Windows 宿主输入路径时做转换,容器内部已归一化路径不再替换。

💡 建议

  • trpc_agent_sdk/code_executors/container/_container_ws_runtime.py:112-129_container_relative_path 对 Windows 绝对路径检测依赖 PureWindowsPath(raw_path).is_absolute(),在 Linux 宿主上 r"C:\Windows" 形态会被判为绝对路径(符合预期),但 C:foo(带驱动器盘符的相对路径)既不被 posixpath.isabs 也不被 PureWindowsPath.is_absolute 拦截,存在残余越界面。可选:额外拒绝含 : 的盘符前缀。

总结

整体风险集中在容器路径校验的边界缺口与本地 rmtree 回调的异常语义/弃用 API 问题;存在一个可能导致软链越界的 Critical 问题需修复,两个 Warning 建议在合并前处理。

测试建议

  • 补充用例:spec.dst 传入 work/../../etc 时验证 stage_inputsworkspace:///skill:// 分支会抛 ValueError 且不发起 exec_run(当前测试覆盖了顶层 dst 拒绝,但未覆盖经 _stage_workspace_input 的绝对 dst 路径)。
  • 补充用例:_remove_read_only_pathos.stat(path)FileNotFoundError 时应保留原始 PermissionError 而非抛 FileNotFoundError

if not raw_path:
if allow_current:
return ""
raise ValueError("container relative path must not be empty")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_container_relative_path 路径逃逸检测存在缺口

_stage_workspace_input 接收的 dst_container_path(ws.path, dst_rel) 拼出的绝对路径,_container_path 不再校验逃逸,若 dst.. 仍可越界创建软链。建议在 _stage_workspace_inputdst 再次做 _container_relative_path 校验或强制以 ws.path 为根做 relative_to 断言。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants