Conversation
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…t errors Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
… guide - Add auth-related i18n keys to zh-CN and en-US locale files - Update auth middleware to inject i18n-translated unauthorized message into context - Enhance comments on AuthProvider interface, NoopProvider, and server bootstrap - Add test for i18n message injection in auth middleware - Create docs/plugin-guide.md with plugin architecture suggestion plan (Chinese) Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…n-US) Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
- Rename auth module framing: "Gateway 防护" instead of "认证/登录" - Add GatewayProtectionConfig to GlobalConfig for config persistence - Expose gateway_protection in /api/config/general GET/POST endpoints - Add Gateway Protection toggle switch in GeneralSettings.tsx - Add i18n keys for gateway protection (zh-CN, en-US, frontend) - Update docs/plugin-guide.md with Gateway protection terminology - Update all comments to clarify: protects Gateway, not user login Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…ion keys, fix struct alignment Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…nal skill test classification Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
… accuracy Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…, update docs and tests Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
…n, consistent assertions Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Relax read_file/write_file sandbox, fix vector search CI failures, fix internal skill test classification
There was a problem hiding this comment.
Pull request overview
该 PR 以“Gateway 防护插件化 + 内化工具重写”为主线:引入可插拔的 Gateway 防护接口与中间件,并将部分高风险/跨平台不佳的技能(read_file / write_file / terminal)改为 Go 内置实现,同时补齐前端配置项与测试、i18n 文案及 CI 细节。
Changes:
- 新增
core.AuthProvider插件接口与 Gin 中间件封装,并在 Server 启动时可选注入,实现 Gateway 防护“默认关闭、按需启用”。 - 将
read_file/write_file/terminal增加 Go 内置实现并注册为 internal skills,新增对应单测与安全说明文档。 - Dashboard 侧增加 Gateway 防护开关与 Cron 文案 i18n 化,补充多语言文案与 CI workspace 准备步骤。
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/write_file/SKILL.md | 更新 write_file 技能说明,强调支持绝对/相对路径与 workspace 解析 |
| skills/terminal/terminal_cli.sh | 为 shell 版 terminal 增加基础危险字符/危险命令拦截参数 |
| skills/read_file/read_file_cli.sh | 为 shell 版 read_file 增加路径遍历与目录越界拦截逻辑 |
| pkg/i18n/locales/zh-CN.json | 增加 Gateway 防护相关 i18n key(中文) |
| pkg/i18n/locales/en-US.json | 增加 Gateway 防护相关 i18n key(英文) |
| internal/usecase/skills/vector_search_test.go | Ollama/embedding 不可用时改为 Skip,避免 CI 误报失败 |
| internal/usecase/skills/skill_mgr_real_test.go | 对“internal skill not registered”场景改为跳过统计,减少噪声失败 |
| internal/usecase/skills/executor.go | Windows 下执行 .sh 外部技能时尝试用 bash/sh 包装执行 |
| internal/usecase/skills/builtins/write_file_test.go | 新增 write_file 内置实现测试覆盖(相对/绝对路径等) |
| internal/usecase/skills/builtins/write_file.go | write_file 改为 Go 内置实现,支持绝对/相对路径写入并返回 JSON |
| internal/usecase/skills/builtins/terminal_test.go | 新增 terminal 内置实现安全校验测试(危险字符/危险命令等) |
| internal/usecase/skills/builtins/terminal.go | 新增 terminal Go 内置实现与危险字符/命令校验、超时控制与 JSON 输出 |
| internal/usecase/skills/builtins/registry.go | 将 read_file/terminal 注册为 internal skills |
| internal/usecase/skills/builtins/read_file_test.go | 新增 read_file 内置实现测试覆盖(相对/绝对路径、缺参、不存在等) |
| internal/usecase/skills/builtins/read_file.go | 新增 read_file Go 内置实现,返回 JSON(success/path/content 等) |
| internal/usecase/auth/noop_provider_test.go | 新增 NoopProvider 单测 |
| internal/usecase/auth/noop_provider.go | 新增默认 NoopProvider(默认不启用 Gateway 防护) |
| internal/infrastructure/bootstrap/server.go | Server 支持可选注入 AuthProvider,并在 Gin 上挂载防护中间件 |
| internal/core/auth.go | 新增 Gateway 防护插件接口 AuthProvider 定义 |
| internal/config/paths_test.go | 新增 GetInstallPath 测试覆盖 |
| internal/config/paths.go | GetInstallPath 优化:支持 MINDX_PATH 优先与 bin/ 目录回退到上级 |
| internal/config/global.go | 增加 GatewayProtectionConfig 到全局配置结构体 |
| internal/adapters/http/middleware/auth_test.go | 新增 Gateway 防护中间件单测(nil/noop/启用/禁用/白名单/i18n 注入) |
| internal/adapters/http/middleware/auth.go | 新增 Gateway 防护中间件封装(provider.Enabled/PublicPaths/i18n 注入) |
| internal/adapters/http/handlers/config.go | General config API 增加 gateway_protection 字段读写 |
| internal/adapters/channels/gateway_stability_test.go | 稳定性测试增加 channel.Stop 的 defer,避免资源泄漏/干扰 |
| docs/plugin-guide.md | 新增插件化能力的实现建议与架构说明文档 |
| docs/internalized-tools-report.md | 新增内化工具安全修正报告(read_file/terminal/write_file) |
| docs/gateway-protection-report.md | 新增 Gateway 防护实现报告(架构、流程、测试覆盖等) |
| dashboard/src/i18n.ts | 增加 Cron 与 Gateway 防护相关前端 i18n 文案 |
| dashboard/src/components/Monitor.tsx | 将 Record<string, any> 收紧为 Record<string, unknown> |
| dashboard/src/components/GeneralSettings.tsx | 增加 Gateway 防护开关 UI 与 i18n 文案;保存提示改为国际化 |
| dashboard/src/components/GeneralSettings.css | 新增 Gateway 防护开关样式与描述文案样式 |
| dashboard/src/components/Cron.tsx | Cron 页面文案全面替换为 i18n key |
| dashboard/src/components/CapabilityIcon.tsx | 收紧 iconMap 组件 props 类型,避免 any 泛滥 |
| .github/workflows/ci.yml | 测试 workspace 准备阶段先创建 .test 目录,避免 cp 失败 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cleanFilename := filepath.Clean(filename) | ||
|
|
||
| if path, ok := params["path"].(string); ok && path != "" { | ||
| filePath = filepath.Join(workDir, "documents", path, filename) | ||
| // "path" param provided: treat as directory, append filename | ||
| cleanPath := filepath.Clean(path) | ||
| if filepath.IsAbs(cleanPath) { | ||
| filePath = filepath.Join(cleanPath, cleanFilename) | ||
| } else { | ||
| workDir := os.Getenv("MINDX_WORKSPACE") | ||
| if workDir == "" { | ||
| return "", fmt.Errorf("MINDX_WORKSPACE environment variable is not set") | ||
| } | ||
| filePath = filepath.Join(workDir, cleanPath, cleanFilename) | ||
| } | ||
| } else if filepath.IsAbs(cleanFilename) { | ||
| // filename itself is an absolute path | ||
| filePath = cleanFilename | ||
| } else { | ||
| filePath = filepath.Join(workDir, "documents", filename) | ||
| // Relative filename: resolve against workspace | ||
| workDir := os.Getenv("MINDX_WORKSPACE") | ||
| if workDir == "" { | ||
| return "", fmt.Errorf("MINDX_WORKSPACE environment variable is not set") | ||
| } | ||
| filePath = filepath.Join(workDir, cleanFilename) |
There was a problem hiding this comment.
write_file 目前允许 filename 为绝对路径,且相对路径在 filepath.Join(workDir, cleanPath, cleanFilename) 后如果包含 .. 也可能逃逸出 workspace(例如 filename="../x" 或 path="../")。如果期望“相对路径基于 workspace”且不允许越界写入,建议在 join+clean 后校验结果仍在 workDir 下(或显式拒绝包含 .. 的相对输入),并将“写任意路径”改为需要显式开关(类似 terminal 的 dangerous=true)。
skills/read_file/read_file_cli.sh
Outdated
| # 规范化路径并验证(使用realpath的-m选项,不会检查文件是否存在) | ||
| REAL_PATH=$(realpath -m "$FULL_PATH" 2>/dev/null) | ||
| REAL_BASE=$(realpath -m "$BASE_DIR" 2>/dev/null) | ||
|
|
There was a problem hiding this comment.
read_file_cli.sh 使用 realpath -m 进行规范化,但 macOS 自带的 BSD realpath 通常不支持 -m 选项,会导致脚本在 macOS 上直接失败。建议改用跨平台的路径规范化实现(例如用 Python/Go 实现,或用 cd -P/pwd -P 等兼容方案),并在脚本内检测 realpath 能力后降级处理。
| .toggle-switch { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 10px; | ||
| cursor: pointer; | ||
| padding: 4px; | ||
| border-radius: 20px; | ||
| background-color: #374151; | ||
| width: 52px; | ||
| height: 28px; | ||
| position: relative; | ||
| transition: background-color 0.3s; | ||
| } |
There was a problem hiding this comment.
GeneralSettings.css 新增了全局 .toggle-switch 选择器,但项目里 Capabilities.css 也定义了同名 .toggle-switch(且结构不同)。在全局 CSS 作用域下这会产生样式互相覆盖/串扰的风险,导致其它页面的开关样式异常。建议将选择器作用域限定到 .general-settings .toggle-switch,或改用更特定的类名(例如 .gateway-toggle-switch)。
| <div className="config-item"> | ||
| <label>{t('settings.gatewayProtection')}</label> | ||
| <div | ||
| className={`toggle-switch ${config.gateway_protection.enabled ? 'active' : ''}`} | ||
| onClick={() => setConfig({ | ||
| ...config, | ||
| gateway_protection: { | ||
| ...config.gateway_protection, | ||
| enabled: !config.gateway_protection.enabled, | ||
| }, | ||
| })} | ||
| > |
There was a problem hiding this comment.
Gateway 防护开关目前用可点击的 <div> 实现,没有键盘操作(Enter/Space)、role="switch"/aria-checked 等语义,也没有可聚焦元素;这会造成可访问性问题,并且与项目中其它开关(Capabilities 使用 <input type="checkbox">)不一致。建议改为原生 checkbox/switch(或至少补齐 tabIndex、键盘事件与 ARIA 属性)。
| // NewServer 创建 HTTP API 服务器实例 | ||
| func NewServer(port int, staticDir string) (*Server, error) { | ||
| // authProvider 为可选的 Gateway 防护插件,传入 nil 或 NoopProvider 则不启用防护 | ||
| // 防护模块采用插件化设计,不与核心层耦合,仅在需要保护 Gateway 时启用 | ||
| func NewServer(port int, staticDir string, authProvider ...core.AuthProvider) (*Server, error) { |
There was a problem hiding this comment.
NewServer 的函数签名已改为 NewServer(port, staticDir, authProvider ...core.AuthProvider),但当前仓库仍存在只传 2 个参数的调用点(例如 internal/infrastructure/bootstrap/app.go:401)。这会导致编译失败;建议同时更新所有调用点,或保持向后兼容(例如保留旧签名/提供包装函数)。
| // dangerousChars lists shell metacharacters that indicate injection | ||
| var dangerousChars = []string{";", "&", "|", "`", "$(", "${", ")", ">", ">>", "<", "\n", "\r"} | ||
|
|
There was a problem hiding this comment.
Terminal 仍通过 sh -c/cmd /C 执行整段字符串,这会让 shell 展开/转义规则介入;当前 dangerousChars 没有拦截 $(仅拦截 $( 和 ${),导致诸如 $SHELL -c ... 或 $'\n'(某些 shell 支持)等形式可能绕过基于字符串的注入检测与 dangerousCommands 基础命令判断。建议避免使用 shell(改为解析成可执行文件+参数并直接 exec.CommandContext),或至少补齐对 $/ANSI-C quoting 等展开形式的拦截并增加对应测试用例。
|
|
||
| func getJSONTerminalResult(output, errMsg string, exitCode int, elapsed time.Duration) (string, error) { | ||
| result := map[string]interface{}{ | ||
| "exit_code": exitCode, |
There was a problem hiding this comment.
getJSONTerminalResult 的 elapsed 参数当前未被使用,导致该参数没有实际意义且容易误导调用方。建议要么把耗时写入返回 JSON(如 elapsed_ms),要么删除该参数并在调用处移除传入值。
| "exit_code": exitCode, | |
| "exit_code": exitCode, | |
| "elapsed_ms": elapsed.Milliseconds(), |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Co-authored-by: XXY-CH <70793154+XXY-CH@users.noreply.github.com>
Address PR#7 review gaps: tighten write_file boundaries, normalize read_file_cli fallback, and close remaining review comments
十分抱歉上一个PR质量不高的困扰,这次使用了Claude+人工构建骨架+关键功能,应当有所改善。
在上一个PR的基础上,保证了防护作为Plugin的模式集成,由于上个PR的目的不明确,在新branch上重新构建了防护措施,明确是为防护Gateway而不是登录功能,并且在新的PR上调用了Copilot修复了问题,现在workflow验证除了需要本地LLM接口的功能应当都测试通过。