alibaba / alibaba/open-code-review

bug(tool): prevent path traversal bypass and normalize paths in code_search and code_comment

Closed
#1,088 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
24.4k
Forks
1.8k
Avg merge
2d 4m
Merged PRs (30d)
102

Description

### OpenCodeReview Version

`main` at `eda2548` (built from source)

### Operating System

Windows (also impacts cross-platform path handling across Windows/Linux/macOS)

### Installation Method

Built from source

### LLM Provider

Other OpenAI-compatible endpoint / Anthropic

### Bug Description

Two cross-platform path handling issues exist in `internal/tool`:

1. **Path traversal validation bypass in `code_search`**:
In `internal/tool/code_search.go`, `hasTraversalPathComponent` only checks for `/` path separators:
```go
func hasTraversalPathComponent(pathspec string) bool {
for _, part := range strings.Split(pathspec, "/") {
if part == ".." {
return true
}
}
return false
}
```
When `file_patterns` contain Windows-style backslashes (such as `..\pkg` or `pkg\..\internal`), `strings.Split` fails to separate path segments by `\`, allowing path traversal validation to be bypassed. Furthermore, `file_patterns` are forwarded directly to `git grep` without normalizing `\` to `/`, which causes `git grep` pathspec matching to fail.

2. **Silent comment dropping due to unnormalized path in `code_comment`**:
In `internal/tool/code_comment.go`, `parseCommentsInner` directly sets `cm.Path` from the tool arguments without normalizing path separators or redundant prefixes (e.g., `pkg\util.go`, `./pkg/util.go`, or `pkg//util.go`).
Downstream in `internal/agent/agent.go`, comments are looked up via `CommentCollector.CommentsForPath(d.NewPath)` where `d.NewPath` is the canonical forward-slash path (e.g. `pkg/util.go`). Because `CommentsForPath` performs strict equality (`cm.Path == path`), any comment generated with backslashes or `./` is silently dropped and never included in the review results.

### Steps to Reproduce

1. Call `CodeSearchProvider.Execute` with `file_patterns: ["..\\pkg"]`.
- Observe that `hasTraversalPathComponent` returns `false` and does not reject the pattern with `"Error: file_patterns must not contain .."`.
2. Call `CodeCommentProvider.Execute` (or `ParseComments`) with `path: "pkg\\util.go"` or `path: "./pkg/util.go"`.
- Store comments in `CommentCollector`.
- Call `CommentCollector.CommentsForPath("pkg/util.go")`.
- Observe that 0 comments are returned because the path was not normalized.

### Expected Behavior

1. `hasTraversalPathComponent` should normalize `\` to `/` before checking for `..`, and `file_patterns` should be normalized to forward slashes before being passed to `git grep`.
2. `parseCommentsInner` should normalize comment paths (`\` to `/`, stripping leading `./`, cleaning redundant slashes) so comments reliably match `CommentsForPath(d.NewPath)`.

### Additional Context

This aligns `code_search` and `code_comment` with the cross-platform path normalization patterns recently established in `file_find.go` (#1075).

Contributor guide

Open the contributing guide

Research direction

Start with internal/tool/code_search.go and internal/tool/code_comment.go, including CodeSearchProvider.Execute, CodeCommentProvider.Execute or ParseComments, and the path handling established in file_find.go (#1075). Reproduce the Windows-style traversal and comment-path cases, then verify that traversal is rejected, git grep receives normalized paths, and CommentsForPath("pkg/util.go") retains comments from equivalent path forms.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
security, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.