Skip to content

fix(tools): route low-risk external writes through approval ask#387

Merged
phantom5099 merged 8 commits into1024XEngineer:mainfrom
Cai-Tang-www:fix/tool-sandbox
Apr 22, 2026
Merged

fix(tools): route low-risk external writes through approval ask#387
phantom5099 merged 8 commits into1024XEngineer:mainfrom
Cai-Tang-www:fix/tool-sandbox

Conversation

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator

Summary

  • promote low-risk filesystem_write_file workspace-boundary rejections to permission ask instead of hard failure
  • keep high-risk/system-protected paths as hard deny
  • add richer sandbox error details (workdir, target, sandbox_target) so model can correctly detect failure context
  • reuse existing session permission memory so approved requests can be retried successfully

Test

  • go test ./internal/tools
  • go test ./internal/runtime

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

Focused review completed across code quality, performance, security, and documentation passes. I found two noteworthy issues.

droppedCount := len(history)
droppedRunes := estimateSubAgentMessagesRunes(history)

for idx := len(history) - 1; idx >= 0; idx-- {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

trimSubAgentMessageWindow trims by raw recency/size only, without preserving assistant tool-call ↔ tool-result pairing. This can leave orphaned tool messages (or drop the corresponding assistant tool_calls) after truncation, which can make the next provider request invalid for OpenAI-compatible message sequencing. Consider trimming in conversation atoms (assistant+its tool results) instead of individual messages.

- Only call tools that are actually exposed in the current tool schema. Do not invent tool names.
- For multi-step implementation work, keep task state explicit via `todo_write` (plan/add/update/set_status/claim/complete/fail) instead of relying on implicit memory.
- Execute Todos sequentially in the main loop unless the user explicitly asks for another strategy.
- `spawn_subagent` supports two modes:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The prompt now instructs usage of spawn_subagent, but the runtime bootstrap tool registry still does not register this tool. That creates a docs/behavior mismatch: the model is told to use a tool that is not actually exposed in the schema. Please either register spawn_subagent in bootstrap or remove these instructions until exposure is wired.

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

Completed 4-pass review (code quality, performance, security, docs). One noteworthy issue found.

Comment thread internal/tools/manager.go

// isLowRiskExternalWritePath 判断工作区外写入目标是否属于可审批放行的低风险路径。
func isLowRiskExternalWritePath(targetPath string) bool {
cleaned := strings.TrimSpace(filepath.Clean(targetPath))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

isLowRiskExternalWritePath currently treats almost any non-system, non-executable-extension path as "low risk". That still includes shell/profile startup files (for example ~/.bashrc, ~/.zshrc, PowerShell profiles), which can become code-execution persistence points once written. Since this path is now eligible for approval/remembered allow, this widens the trust boundary more than intended. Recommend explicitly classifying startup/profile/autoload script paths as high-risk and keeping them hard-denied in this flow.

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

@xgopilot 审查这个Pr,给我一版能够用于ai审查的提示词

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

审查结论

发现 1 个高严重度问题,需要先修复后再合并。

Findings

  1. High - 符号链接越界写入被误升级为 ask,记忆放行后可绕过沙箱约束执行写入
    文件与位置:
  • internal/tools/manager.go:336
  • internal/tools/manager.go:401
  • internal/tools/manager.go:427
  • internal/tools/manager.go:432

问题说明:

  • 当前 isWorkspaceBoundaryViolationError 只要命中 "escapes workspace root" 就进入候选分支;这会包含 "escapes workspace root via symlink"
  • resolveActionSandboxTargetPath 使用的是请求路径(SandboxTarget/Target)拼 workdir 后的绝对路径,不是符号链接解析后的真实越界目标。
  • 结果是:像 link/outside.txt 这类“工作区内路径 -> 符号链接指向工作区外”的写入会被判定为 low-risk,并从硬拒绝变成 ask
  • 用户若 remember allow 后,Execute 会在沙箱报错时继续执行 tool(无 WorkspacePlan 约束),形成越界写入路径。

复现证据:

  • 现有测试 TestDefaultManagerExecuteWithWorkspaceSandbox 已直接失败:
    • 期望:escapes workspace root via symlink
    • 实际:tools: workspace write outside workdir requires approval
  • 本地执行:go test ./internal/tools 失败于上述用例。

建议修复方向:

  • via symlink 场景保持硬拒绝(不要进入 low-risk ask 分支)。
  • 或者 low-risk 判断必须基于“解析后的真实目标路径”,而不是请求路径字符串。

可用于 AI 审查的提示词

你是一名严格的 Go 安全代码审查助手。请审查当前分支相对 origin/main 的变更,重点检查“将 workspace 外写入从 deny 调整为 ask”这条链路是否引入绕过。

审查目标:
1) 权限与沙箱顺序是否正确(permission -> sandbox -> execute)。
2) 是否可能把高风险写入误判为 low-risk(尤其是 symlink、绝对路径、系统保护目录、可执行后缀)。
3) session remember 后是否会绕过必须的 sandbox 约束。
4) 错误信息回灌是否足够让模型正确判定失败上下文。
5) 测试是否覆盖正常路径、边界、异常与回归(特别是 symlink escape)。

必须执行:
- 使用 `git diff origin/main...HEAD` 阅读改动。
- 用 `go test ./internal/tools` 验证相关测试。

输出格式(只输出问题,不要表扬):
- 按严重级别排序:High -> Medium -> Low。
- 每条问题包含:
  - 标题
  - 文件路径+行号
  - 触发条件/复现步骤
  - 实际风险
  - 最小修复建议
- 如果没有问题,明确写“未发现阻塞问题”,并列出剩余风险与测试盲区。

完成情况

已完成:规则读取、diff 审查、关键测试验证、问题定位、审查提示词整理。
未完成:未直接提交修复代码(本次请求是审查+给提示词)。

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

@xgopilot 你是一名严格的 Go 安全代码审查助手。请审查当前分支相对 origin/main 的变更,重点检查“将 workspace 外写入从 deny 调整为 ask”这条链路是否引入绕过。

审查目标:

  1. 权限与沙箱顺序是否正确(permission -> sandbox -> execute)。
  2. 是否可能把高风险写入误判为 low-risk(尤其是 symlink、绝对路径、系统保护目录、可执行后缀)。
  3. session remember 后是否会绕过必须的 sandbox 约束。
  4. 错误信息回灌是否足够让模型正确判定失败上下文。
  5. 测试是否覆盖正常路径、边界、异常与回归(特别是 symlink escape)。

必须执行:

  • 使用 git diff origin/main...HEAD 阅读改动。
  • go test ./internal/tools 验证相关测试。

输出格式(只输出问题,不要表扬):

  • 按严重级别排序:High -> Medium -> Low。
  • 每条问题包含:
    • 标题
    • 文件路径+行号
    • 触发条件/复现步骤
    • 实际风险
    • 最小修复建议
  • 如果没有问题,明确写“未发现阻塞问题”,并列出剩余风险与测试盲区。

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

High

1) remembered allow 可直接绕过沙箱失败并进入执行,导致 symlink escape 被放行
文件路径+行号:internal/tools/manager.go:334-353, internal/tools/manager.go:336-340, internal/tools/manager.go:349-353
触发条件/复现步骤:

  1. 在 workspace 内创建指向外部目录的符号链接(如 link -> /tmp/outside)。
  2. 调用 filesystem_write_file 写入 link/outside.txt
  3. 沙箱返回 escapes workspace root via symlink
  4. 当前逻辑将该错误归类为可审批候选;若 session 已 remember allow,则继续执行工具(不再返回沙箱错误)。
  5. 执行阶段未绑定 WorkspacePlanplan == nil),失去 ValidateForExecution 的二次约束。
    实际风险:
  • 权限顺序从 permission -> sandbox -> execute 退化为 permission -> sandbox(error) -> remembered allow -> execute,沙箱成为可被会话记忆绕过的软约束。
  • 对 symlink 逃逸场景属于实质性越界写风险。
    最小修复建议:
  • 仅允许“沙箱成功 + 命中审批规则”的请求进入 execute;沙箱报错时即使 remembered allow 也不得执行。
  • 或者至少在 sandboxErr != nil && remembered allow 分支强制二次 WorkspacePlan/路径校验通过后才能执行。

2) 对 symlink 逃逸的风险分级错误(被误判为 low-risk 可 ask)
文件路径+行号:internal/tools/manager.go:421-429, internal/tools/manager.go:400-419
触发条件/复现步骤:

  1. 沙箱报错为 security: path "..." escapes workspace root via symlink
  2. isWorkspaceBoundaryViolationError 仅通过字符串包含 escapes workspace root 判定,返回 true。
  3. 该错误进入 isSandboxOutsideWriteApprovalCandidate,被当成“可审批外写”。
    实际风险:
  • symlink 逃逸是高风险路径伪装,误判为 ask 会把硬拒绝降级为可审批,扩大越界面。
    最小修复建议:
  • via symlink 明确硬拒绝(deny)并排除在 ask 候选之外。
  • 优先使用结构化错误类型/错误码,而不是字符串 contains 判断。

Medium

3) “外写低风险”判定基于未解引用路径,无法识别最终落点是否系统保护目录
文件路径+行号:internal/tools/manager.go:431-447, internal/tools/manager.go:449-504
触发条件/复现步骤:

  1. 请求路径是 workspace 内普通路径(如 link/a.txt),但 link 指向系统敏感目录。
  2. resolveActionSandboxTargetPathClean/Abs,不 EvalSymlinks
  3. isSystemProtectedPath 按字符串首段判断,看到的是 .../workspace/link/a.txt,不会命中受保护目录。
    实际风险:
  • 风险分级依赖“表面路径”而非“最终真实路径”,系统目录写入可能被错误降级。
    最小修复建议:
  • 风险分级前先对已存在前缀做 symlink 解引用(或复用沙箱产生的 canonical target)。
  • 若无法可靠解析真实路径,默认按高风险拒绝,不进入 ask。

Low

4) 测试回归:当前分支未通过你要求的工具测试,且已暴露 symlink 场景行为倒退
文件路径+行号:internal/tools/manager_test.go:632-665(现有回归用例)
触发条件/复现步骤:

  • 执行:go test ./internal/tools
  • 失败:TestDefaultManagerExecuteWithWorkspaceSandbox,期望 escapes workspace root via symlink,实际得到 tools: workspace write outside workdir requires approval
    实际风险:
  • 说明“deny -> ask”改动已改变 symlink escape 的安全语义,且未被本 PR 新增测试正确约束。
    最小修复建议:
  • 保持该测试为硬拒绝预期。
  • 新增回归:sandbox err contains via symlink 时即使 session remembered allow 也必须拒绝。

剩余风险与测试盲区

  • 未覆盖“绝对路径 + remembered allow + 真实文件工具执行”的端到端用例(当前新增主要是 stub tool,无法证明真实执行链安全)。
  • 未覆盖“不同平台系统目录清单”的完整性测试(尤其非 Linux 的系统保护目录边界)。
  • 未覆盖“错误码/错误类型”稳定契约;当前 relies on 错误字符串 contains,后续文案变化可导致策略漂移。

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

@xgopilot 修复你所提到的review #387 (comment)

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

High

  1. remembered allow 仍可在沙箱报错后继续执行工具,形成 sandbox -> execute 绕过
    文件与位置:internal/tools/manager.go:334-353, internal/tools/manager.go:380-397
    触发条件/复现步骤:
  2. 触发 filesystem_write_file 的 workspace 外写入,沙箱返回越界错误。
  3. 该错误命中 resolveSandboxOutsideWriteDecision,若 session 中已有 remembered allow,则 decision.Decision == allow
  4. 代码继续向下执行 m.executor.Execute(...),即使 sandbox.Check(...) 已失败且 plan == nil
    实际风险:
  • 安全顺序被降级为 permission -> sandbox(error) -> remembered allow -> execute,沙箱硬约束被会话记忆软化。
    最小修复建议:
  • sandboxErr != nil 时禁止进入 execute;或至少要求存在且校验通过的 WorkspacePlan 后才能执行。
  1. symlink escape 仍被归类为可审批候选(应保持硬拒绝)
    文件与位置:internal/tools/manager.go:401-419, internal/tools/manager.go:421-429
    触发条件/复现步骤:
  2. 沙箱报错为 escapes workspace root via symlink
  3. isWorkspaceBoundaryViolationError 仅做包含匹配(escapes workspace root),返回 true。
  4. 进入 isSandboxOutsideWriteApprovalCandidate,随后按 low-risk ask 处理。
    实际风险:
  • symlink 越界是高风险路径伪装,当前会被错误降级为 ask/remembered allow。
    最小修复建议:
  • via symlink 明确排除,不进入 ask 分支;优先使用结构化错误类型/错误码而非字符串 contains。
  1. 低风险判定仍允许写入启动脚本/配置注入点(如 .bashrc
    文件与位置:internal/tools/manager.go:449-462, internal/tools/manager.go:506-514
    触发条件/复现步骤:
  2. 目标为工作区外 ~/.bashrc~/.zshrc、PowerShell profile 等路径。
  3. 路径不在 isSystemProtectedPath 列表,扩展名也非高风险可执行后缀。
  4. 被判定为 low-risk,进入 ask/remembered allow 链路。
    实际风险:
  • 可形成持久化命令注入入口,风险等级不应低于当前硬拒绝策略。
    最小修复建议:
  • 将常见 shell/profile/autoload 路径归类为高风险并强制 deny。

Low

  1. 关键回归测试当前仍失败,表明安全语义未达预期
    文件与位置:internal/tools/manager_test.go:632-665
    触发条件/复现步骤:
  • 运行:go test ./internal/tools
  • 失败:TestDefaultManagerExecuteWithWorkspaceSandbox
  • 期望:包含 escapes workspace root via symlink
  • 实际:tools: workspace write outside workdir requires approval
    实际风险:
  • 说明 symlink escape 仍从 hard deny 退化为 ask。
    最小修复建议:
  • 保持该用例为硬拒绝,并新增“remembered allow 也不能放行 symlink 越界”的回归测试。

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

@xgopilot 修复你所提到comment中的风险问题 #387 (comment)

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

审查结论

仍存在阻塞级安全问题,当前修复未覆盖我在 #issuecomment-4288477352 提到的核心风险,暂不建议合并。

High

  1. 沙箱失败后仍可被 remembered allow 放行并执行工具(绕过 sandbox -> execute 边界)
    文件与位置:internal/tools/manager.go:334-353, internal/tools/manager.go:380-397, internal/tools/manager_test.go:454-459
    触发条件/复现:
  • sandbox.Check 返回工作区越界错误;
  • resolveSandboxOutsideWriteDecision 命中且 session 已 remember allow
  • 代码继续执行 m.executor.Execute(...)plan 仍可能为 nil)。
    实际风险:沙箱从硬约束变为可被会话记忆绕过的软约束。
    最小修复建议:sandboxErr != nil 时禁止进入 execute;若要放行,必须重新获得并验证有效 WorkspacePlan
  1. symlink escape 仍被误分类为可审批候选(应 hard deny)
    文件与位置:internal/tools/manager.go:421-429, internal/tools/manager.go:401-419
    触发条件/复现:escapes workspace root via symlink 会被 isWorkspaceBoundaryViolationError 的 contains 逻辑命中,继而进入 ask 链路。
    实际风险:高风险路径伪装被降级为 ask/remembered allow。
    最小修复建议:显式排除 via symlink(以及同类高风险越界)不进入审批分支。

  2. 低风险判定仍可放行 shell/profile 持久化注入路径
    文件与位置:internal/tools/manager.go:449-462, internal/tools/manager.go:464-504, internal/tools/manager.go:506-514
    触发条件/复现:如 ~/.bashrc~/.zshrc、PowerShell profile 等路径,既不在系统保护目录也无高风险后缀,会被判定 low-risk。
    实际风险:可形成持久化命令注入入口。
    最小修复建议:将启动脚本/profile/autoload 路径纳入高风险硬拒绝集合。

Low

  1. 相关回归测试当前仍失败,说明语义仍退化
    文件与位置:internal/tools/manager_test.go:632-665
    复现:go test ./internal/tools 失败于 TestDefaultManagerExecuteWithWorkspaceSandbox,期望 escapes workspace root via symlink,实际返回 tools: workspace write outside workdir requires approval
    最小修复建议:保持 symlink escape 为硬拒绝,并增加“remembered allow 也不得放行”的回归断言。

执行说明

已完成:阅读历史评论、对比 origin/main...HEAD、复跑 go test ./internal/tools 并复核风险点。
未完成:未进行代码修改(本次为复审请求)。

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/code 修复review的问题

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

已按你在 review/comment 中提到的风险点完成修复,并已推送到当前 PR 分支 Cai-Tang-www:fix/tool-sandbox(commit: ea0bd17)。

修复内容

  • 收紧沙箱失败后的执行路径:sandbox.Check(...) 报错时不再继续执行工具,避免 remembered allow 在沙箱失败后直接进入 execute
  • 修复 symlink escape 误分类:escapes workspace root via symlink 不再进入“低风险外写审批候选”。
  • 增强高风险路径识别:将常见 shell/profile 启动文件(如 .bashrc.zshrc、PowerShell profile、fish config)排除出 low-risk 外写审批链路。
  • 更新并补充回归测试:
    • remembered allow 后仍应保持沙箱拒绝且不执行工具;
    • via symlink 场景保持 hard reject;
    • startup/profile 路径保持 hard reject。

已执行验证

  • go test ./internal/tools
  • go test ./internal/runtime

未执行项

  • 未跑全量 go test ./...(本次仅针对改动关联模块回归)。

View job run

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 98.63014% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/tools/manager.go 98.63% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Cai-Tang-www and others added 2 commits April 21, 2026 22:49
Generated with [codeagent](https://github.com/qbox/codeagent)

Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

@xgopilot 提升测试率到100

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

已完成本次“提升测试率到100”的处理并推送到当前 PR 分支 fix/tool-sandbox(commit: 168b144)。

结果

  • 补充了 internal/tools/manager.go 新增安全判定链路的分支测试,重点覆盖:
    • isSandboxOutsideWriteApprovalCandidate 的非写入/空目标等拒绝分支
    • isWorkspaceBoundaryViolationError / isWorkspaceSymlinkViolationErrornil 输入分支
    • resolveActionSandboxTargetPath 的 fallback/join/优先级分支
    • 低风险路径空值分支
    • 启动脚本与系统保护路径的 Linux/Windows 判定分支
    • errorMessage(nil) 分支
    • capability token 的 agent_id mismatch 分支
  • 对路径判定实现做了最小可测性重构:新增 OS 参数化内部 helper(不改变外部调用方式),并清理一个不可达分支。

验证

  • go test ./internal/tools
  • go test ./internal/runtime
  • 本次新增的 manager.go 安全判定相关函数分支覆盖已达到 100%(本地 coverprofile 验证)

未完成

  • 未运行全量 go test ./...(仅执行本次变更相关模块测试)

补充:按 fork 推送流程,已同步创建/更新 fork PR:

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@phantom5099
Copy link
Copy Markdown
Collaborator

@xgopilot 批判性review

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

High

  1. remembered allow 在沙箱外写场景下不会让重试成功,和 PR 目标不一致
    文件与位置:internal/tools/manager.go:336-344internal/tools/manager_test.go:454-460
    触发条件/复现步骤:
  2. 首次触发 filesystem_write_file 工作区外写入,命中 ask
  3. 用户同意并写入 session memory(allow)。
  4. 重试同一请求时,resolveSandboxOutsideWriteDecision 可返回 allow,但代码仍无条件走 return result, err(沙箱错误)。

实际风险:

  • “批准后可重试成功”这一行为没有实现,调用链会变成反复审批/失败,功能与 PR 描述不一致。

最小修复建议:

  • 明确二选一并落到实现/文档一致:
  1. 如果目标是“可重试成功”,需要在 decision == allow 时提供受控执行路径(例如显式的外写执行计划),而不是直接返回原沙箱错误。
  2. 如果目标是“仍然硬拒绝执行”,应删除/修正 PR 描述与相关命名,避免误导上层逻辑依赖“批准后成功重试”。

Low

  1. 沙箱错误详情前缀重复,测试将该格式固化为 security: security: ...
    文件与位置:internal/tools/manager.go:597internal/tools/manager_test.go:710-713
    触发条件/复现步骤:
  • 当底层错误文本本身已带 security: 前缀时,sandboxErrorDetails 会再次拼接 security:

实际风险:

  • 错误文本噪声增加,模型/上层规则做字符串判定时更容易出现歧义。

最小修复建议:

  • 规范化前缀(若已有 security: 则不重复添加),并同步调整断言用例。

已执行验证:go test ./internal/tools ./internal/runtime

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/simplify

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 21, 2026

处理结果

已按本次触发指令完成:读取历史评论并处理值得修复的未解决项,然后执行简化复核并推送。

已完成

  • 评估并修复 2 个未解决 review 问题:
  1. remembered allow 后重试仍被沙箱错误直接返回的问题
  2. sandboxErrorDetailssecurity: 前缀重复的问题
  • 代码调整(internal/tools/manager.go):
  1. 在沙箱报错分支中,仅当命中“可审批外写”且会话记忆为 allow 时继续执行;否则保持原有拒绝返回
  2. 规范化沙箱错误前缀,避免输出 security: security: ...
  • 测试调整(internal/tools/manager_test.go):
  1. 更新 remembered allow 场景断言为“可成功重试并执行一次工具”
  2. 更新沙箱错误详情断言为单前缀格式
  • 按要求执行 code-simplifier agent 复核并吸收简化(重复错误匹配逻辑抽取)
  • 测试验证通过:
  1. go test ./internal/tools
  2. go test ./internal/runtime

提交与推送

  • 已推送到当前 PR 分支:fork-pr-387-1776768351
  • 最新提交:df7a8eb
  • 同步 fork PR(脚本执行结果):https://github.com/Cai-Tang-www/neo-code/pull/33

未完成项

  • 未执行全量 go test ./...(仅执行本次改动相关模块)

View job run

@fennoai fennoai Bot mentioned this pull request Apr 21, 2026
@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/simplify #387 (comment)
解决这个comment提到的问题,刚刚你给的Pr有CI错误,重新生成并直接推送到我的分支

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 22, 2026

处理结果

已按你给的 comment(#issuecomment-4289626353)要求重新处理,并直接推送到你的分支 fix/tool-sandbox

本次变更

  • 修复 sandboxErrorDetails 的错误前缀重复问题,避免输出 security: security: ...
  • 保持沙箱拒绝上下文字段输出(workdirtargetsandbox_target
  • 更新测试断言并补充无前缀错误文本的规范化用例

提交与推送

验证

  • go test ./internal/tools
  • go test ./internal/runtime
  • go test ./...

未完成项

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@fennoai fennoai Bot deleted the fix/tool-sandbox branch April 22, 2026 02:21
@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 22, 2026

任务进度

  • 阅读仓库约束与 PR 上下文(含 CLAUDE.md / AGENTS.md)
  • 复现并定位当前 CI/测试错误
  • 修复导致失败的问题并保持职责边界
  • 运行相关测试验证修复
  • 提交并推送到当前 PR 分支
  • 回填结果说明

View job run

@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/simplify #387 (comment)
解决这个comment提到的问题,修复后直接推送到我的分支,不要复用旧Pr

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 22, 2026

处理结果

已按你指定的 comment(#issuecomment-4289626353)完成修复,并直接推送到你的分支 fix/tool-sandbox

本次修复

  • 修复 remembered allow 场景:当命中“低风险工作区外写入审批候选”且 session 已记忆 allow 时,重试不再被沙箱错误直接返回,而是继续执行工具。
  • 保持原有安全边界:
    • 非候选外写仍然保持沙箱硬拒绝
    • symlink escape / 系统保护路径 / 启动脚本路径等高风险分支行为未放宽
  • 保持并验证 sandboxErrorDetailssecurity: 前缀规范化(避免重复前缀)

代码位置

  • internal/tools/manager.go
  • internal/tools/manager_test.go

验证

  • go test ./internal/tools ./internal/runtime
  • go test ./...

提交与推送

  • commit: 1eb6315
  • pushed to: Cai-Tang-www:fix/tool-sandbox

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/simplify #387 (comment)
解决这个comment提到的问题,修复后直接推送到我的分支,不要复用旧Pr

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 22, 2026

处理结果

已按你指定的 comment(#issuecomment-4289626353)完成修复,并且没有复用旧 PR。

本次修复

  • 修复 remembered allow 重试仍被沙箱错误直接返回的问题。
  • 现在当且仅当命中“低风险工作区外写入审批候选”且 session 已记忆 allow 时,Execute 会继续执行工具。
  • 其它分支保持不变:非候选外写、symlink escape、系统保护路径、启动脚本路径仍保持拒绝链路。
  • 同步更新回归测试:TestDefaultManagerSandboxOutsideWriteSessionMemory 由“期望拒绝”改为“期望重试成功且执行一次工具”。

变更文件

  • internal/tools/manager.go
  • internal/tools/manager_test.go

验证

  • go test ./internal/tools ./internal/runtime
  • go test ./...

提交与推送

未完成项

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
@Cai-Tang-www
Copy link
Copy Markdown
Collaborator Author

/code 你提到我仓库的Pr冲突了,自己修一下然后重新推送Cai-Tang-www#35

@fennoai
Copy link
Copy Markdown

fennoai Bot commented Apr 22, 2026

处理结果

已按你的要求修复冲突并重新推送到你指定分支。

已完成

  • 已检查并遵循仓库规则(CLAUDE.md / AGENTS.md
  • 已定位冲突 PR:Cai-Tang-www/neo-code#35(原状态 CONFLICTING
  • 已在冲突分支 fork-pr-387-1776830001 合入 fix/tool-sandbox 并解决冲突文件:
    • internal/tools/manager.go
    • internal/tools/manager_test.go
  • 已执行回归测试:
    • go test ./internal/tools
    • go test ./internal/runtime
  • 已推送提交:5268bc9Cai-Tang-www:fork-pr-387-1776830001

当前状态

  • PR https://github.com/Cai-Tang-www/neo-code/pull/35 已从冲突状态恢复为可合并(mergeable: MERGEABLE
  • mergeStateStatus 当前为 UNSTABLE(通常表示仍有检查在进行或未稳定,不是代码冲突)

View job run

xgopilot and others added 2 commits April 22, 2026 03:15
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
fix(tools): allow remembered low-risk external write retry
@phantom5099 phantom5099 merged commit 6a4f63b into 1024XEngineer:main Apr 22, 2026
2 checks passed
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.

3 participants