fix(code_search): replace the fantasy REST endpoint with a real client-side grep
code_search was calling GET /api/v1/repos/{owner}/{repo}/search?type=code — an
endpoint that does not exist. Confirmed against a live Gitea 1.25.5 instance's
swagger spec: only /repos/search, /repos/issues/search, /topics/search etc are
real. Gitea's own web UI code search falls back to server-side `git grep`
because no Repository Indexer is enabled here, and that fallback is an
HTML-only route, not JSON API — so no REST endpoint exists to call, working or
not. Every call to code_search 404'd on the real server.
This went undetected because the existing tests mocked the fantasy endpoint
directly (asserting the request path was .../search?type=code and handing back
a canned JSON envelope) — a textbook case of tests validating a fake instead of
the real system, giving false confidence the tool worked.
SearchCode now does the search itself: resolves the default branch, walks the
tree (GetTree), and substring-matches q (case-insensitive, literal — not a
regex, to keep behavior predictable and avoid a ReDoS surface from
user-supplied input) against fetched file contents (GetFileContents) — the same
approach Gitea's own indexer-less fallback uses, just client-side. Bounded by:
- codeSearchMaxFiles (2000) — files scanned per call
- codeSearchMaxFileSize (512KB, checked via the tree listing's own Size field,
before any fetch) — skip large blobs
- a binary-extension denylist checked before fetch, plus a null-byte content
check after fetch, for extensions the denylist misses
Pagination is over the full sorted result set, recomputed each call (no
server-side index to page through incrementally) — acceptable for the repo
sizes this targets; documented as a known limitation, not a hidden footgun.
The tool layer (internal/tools/code_search.go) is UNCHANGED — SearchCode's
signature and the []CodeSearchHit contract are identical, so this is fully
isolated to the client layer + its tests.
Tests: match found in tree, case-insensitive matching, binary extension
skipped WITHOUT fetching (asserted via the fake's fetch log, not just absent
from results), oversized file skipped without fetching, null-byte content
skipped after fetching, pagination across a full sorted set, empty-query
validation — plus the tool-layer single-repo and fan-out tests rewritten
against the same real endpoints (GetRepo/GetTree/GetFileContents +
ListRepos), replacing their fantasy-endpoint fakes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+135
-15
@@ -1,10 +1,13 @@
|
||||
package gitea
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"path"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
type CodeSearchHit struct {
|
||||
@@ -14,30 +17,147 @@ type CodeSearchHit struct {
|
||||
Score float64 `json:"score,omitempty"`
|
||||
}
|
||||
|
||||
type codeSearchEnvelope struct {
|
||||
Data []CodeSearchHit `json:"data"`
|
||||
OK bool `json:"ok"`
|
||||
// codeSearchMaxFiles bounds how many files a single SearchCode call will fetch
|
||||
// and scan, so one call against a huge repo can't run indefinitely.
|
||||
const codeSearchMaxFiles = 2000
|
||||
|
||||
// codeSearchMaxFileSize skips blobs larger than this (per the tree listing,
|
||||
// before any fetch) — almost certainly binary/vendor/generated content, not
|
||||
// worth the cost of fetching just to reject.
|
||||
const codeSearchMaxFileSize = 512 * 1024
|
||||
|
||||
// codeSearchBinaryExts are skipped WITHOUT fetching — a cheap pre-filter for
|
||||
// obviously-binary content by extension, checked against the tree listing.
|
||||
var codeSearchBinaryExts = map[string]bool{
|
||||
".png": true, ".jpg": true, ".jpeg": true, ".gif": true, ".ico": true, ".webp": true, ".bmp": true,
|
||||
".pdf": true, ".zip": true, ".tar": true, ".gz": true, ".bz2": true, ".xz": true, ".7z": true,
|
||||
".exe": true, ".dll": true, ".so": true, ".dylib": true, ".bin": true, ".class": true, ".jar": true,
|
||||
".woff": true, ".woff2": true, ".ttf": true, ".eot": true, ".otf": true,
|
||||
".mp3": true, ".mp4": true, ".mov": true, ".avi": true, ".webm": true,
|
||||
".pyc": true, ".o": true, ".a": true,
|
||||
}
|
||||
|
||||
// SearchCode does a client-side "git grep"-equivalent search. Gitea's REST API
|
||||
// has no code-content-search endpoint — confirmed against a live 1.25.5
|
||||
// instance's swagger spec (only /repos/search, /repos/issues/search,
|
||||
// /topics/search etc exist). The web UI's OWN code search falls back to
|
||||
// server-side `git grep` because no Repository Indexer is enabled on that
|
||||
// instance, and that fallback is an HTML-only route, not JSON API. So this
|
||||
// walks the tree, fetches text-like blobs (bounded by codeSearchMaxFiles /
|
||||
// codeSearchMaxFileSize), and substring-matches q — case-insensitive, literal
|
||||
// (not a regex, to keep behavior predictable and avoid a ReDoS surface from
|
||||
// user-supplied input) — against file contents.
|
||||
//
|
||||
// Pagination is over the FULL sorted result set, recomputed on every call —
|
||||
// there is no server-side index to page through incrementally, so requesting
|
||||
// page 2 re-scans the tree. Acceptable for the repo sizes this targets; a real
|
||||
// indexer (bleve/elasticsearch) enabled server-side would be the long-term
|
||||
// fix, and is an infra decision, not something gitea-mcp controls.
|
||||
func (c *Client) SearchCode(ctx context.Context, owner, repo, q string, page, limit int) ([]CodeSearchHit, error) {
|
||||
if q == "" {
|
||||
return nil, fmt.Errorf("q is required: %w", ErrValidation)
|
||||
}
|
||||
if page < 1 {
|
||||
page = 1
|
||||
}
|
||||
if limit < 1 {
|
||||
limit = 30
|
||||
}
|
||||
path := fmt.Sprintf("/api/v1/repos/%s/%s/search?q=%s&type=code&page=%d&limit=%d",
|
||||
owner, repo, url.QueryEscape(q), page, limit)
|
||||
body, status, err := c.GetJSON(ctx, path)
|
||||
|
||||
r, err := c.GetRepo(ctx, owner, repo)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, fmt.Errorf("resolve repo: %w", err)
|
||||
}
|
||||
if err := MapStatus(status, body); err != nil {
|
||||
return nil, err
|
||||
branch := r.DefaultBranch
|
||||
if branch == "" {
|
||||
branch = "main"
|
||||
}
|
||||
var env codeSearchEnvelope
|
||||
if err := json.Unmarshal(body, &env); err != nil {
|
||||
return nil, err
|
||||
|
||||
tree, err := c.GetTree(ctx, owner, repo, branch, true)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("tree walk: %w", err)
|
||||
}
|
||||
return env.Data, nil
|
||||
|
||||
qLower := strings.ToLower(q)
|
||||
all := make([]CodeSearchHit, 0)
|
||||
scanned := 0
|
||||
for _, e := range tree.Tree {
|
||||
if ctx.Err() != nil {
|
||||
break
|
||||
}
|
||||
if e.Type != "blob" {
|
||||
continue
|
||||
}
|
||||
if codeSearchBinaryExts[strings.ToLower(path.Ext(e.Path))] {
|
||||
continue
|
||||
}
|
||||
if e.Size > codeSearchMaxFileSize {
|
||||
continue
|
||||
}
|
||||
if scanned >= codeSearchMaxFiles {
|
||||
break
|
||||
}
|
||||
scanned++
|
||||
|
||||
fc, ferr := c.GetFileContents(ctx, owner, repo, e.Path, branch)
|
||||
if ferr != nil {
|
||||
continue // vanished/unreadable between tree walk and read — skip, don't fail the whole search
|
||||
}
|
||||
decoded, derr := base64.StdEncoding.DecodeString(fc.Content)
|
||||
if derr != nil {
|
||||
continue
|
||||
}
|
||||
if bytes.IndexByte(decoded, 0) >= 0 {
|
||||
continue // binary content the extension filter missed
|
||||
}
|
||||
|
||||
content := string(decoded)
|
||||
contentLower := strings.ToLower(content)
|
||||
count := strings.Count(contentLower, qLower)
|
||||
if count == 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
idx := strings.Index(contentLower, qLower)
|
||||
all = append(all, CodeSearchHit{
|
||||
Path: e.Path,
|
||||
Snippet: codeSearchSnippet(content, idx, len(q)),
|
||||
HTMLURL: fmt.Sprintf("%s/%s/%s/src/branch/%s/%s", c.baseURL, owner, repo, branch, e.Path),
|
||||
Score: float64(count),
|
||||
})
|
||||
}
|
||||
|
||||
sort.Slice(all, func(i, j int) bool {
|
||||
if all[i].Score != all[j].Score {
|
||||
return all[i].Score > all[j].Score
|
||||
}
|
||||
return all[i].Path < all[j].Path
|
||||
})
|
||||
|
||||
start := (page - 1) * limit
|
||||
if start >= len(all) {
|
||||
return []CodeSearchHit{}, nil
|
||||
}
|
||||
end := start + limit
|
||||
if end > len(all) {
|
||||
end = len(all)
|
||||
}
|
||||
return all[start:end], nil
|
||||
}
|
||||
|
||||
// codeSearchSnippet returns a short window of text centered on a match,
|
||||
// trimmed to a single line-ish window so results read like a grep hit rather
|
||||
// than a content dump.
|
||||
func codeSearchSnippet(content string, idx, matchLen int) string {
|
||||
const window = 60
|
||||
start := idx - window
|
||||
if start < 0 {
|
||||
start = 0
|
||||
}
|
||||
end := idx + matchLen + window
|
||||
if end > len(content) {
|
||||
end = len(content)
|
||||
}
|
||||
snippet := strings.ReplaceAll(content[start:end], "\n", " ")
|
||||
return strings.TrimSpace(snippet)
|
||||
}
|
||||
|
||||
@@ -2,8 +2,12 @@ package gitea_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/base64"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
|
||||
@@ -11,22 +15,65 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestSearchCode(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, "/api/v1/repos/mathias/infra/search", r.URL.Path)
|
||||
assert.Equal(t, "SearchCode", r.URL.Query().Get("q"))
|
||||
assert.Equal(t, "code", r.URL.Query().Get("type"))
|
||||
func b64(s string) string { return base64.StdEncoding.EncodeToString([]byte(s)) }
|
||||
|
||||
// codeSearchFake serves GetRepo + GetTree + GetFileContents off an in-memory
|
||||
// file map — the REAL endpoints SearchCode uses now that Gitea's REST API has
|
||||
// no code-content-search endpoint (confirmed against a live 1.25.5 instance's
|
||||
// swagger spec: only /repos/search, /repos/issues/search etc exist — the web
|
||||
// UI's own code search falls back to server-side `git grep`, an HTML-only
|
||||
// route, not JSON API). Records which paths were actually fetched, so tests
|
||||
// can assert a file was SKIPPED (binary ext, oversized) without ever being
|
||||
// read, not just absent from results.
|
||||
type codeSearchFake struct {
|
||||
files map[string]string // path -> content
|
||||
sizes map[string]int64 // path -> tree-entry size (defaults to len(content))
|
||||
fetched []string
|
||||
}
|
||||
|
||||
func (f *codeSearchFake) handler(t *testing.T, owner, repo, branch string) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{
|
||||
"data":[{
|
||||
"path":"internal/gitea/code_search.go",
|
||||
"snippet":"func (c *Client) SearchCode",
|
||||
"html_url":"http://gitea.example.com/mathias/infra/src/branch/main/internal/gitea/code_search.go",
|
||||
"score":2.5
|
||||
}],
|
||||
"ok":true
|
||||
}`))
|
||||
}))
|
||||
p := r.URL.Path
|
||||
switch {
|
||||
case r.Method == http.MethodGet && p == "/api/v1/repos/"+owner+"/"+repo:
|
||||
_, _ = fmt.Fprintf(w, `{"name":%q,"full_name":"%s/%s","default_branch":%q}`, repo, owner, repo, branch)
|
||||
|
||||
case r.Method == http.MethodGet && strings.HasPrefix(p, "/api/v1/repos/"+owner+"/"+repo+"/git/trees/"):
|
||||
var entries []string
|
||||
for path, content := range f.files {
|
||||
size := int64(len(content))
|
||||
if s, ok := f.sizes[path]; ok {
|
||||
size = s
|
||||
}
|
||||
entries = append(entries, fmt.Sprintf(`{"path":%q,"type":"blob","sha":"s","size":%d}`, path, size))
|
||||
}
|
||||
_, _ = fmt.Fprintf(w, `{"sha":"root","tree":[%s],"truncated":false}`, strings.Join(entries, ","))
|
||||
|
||||
case r.Method == http.MethodGet && strings.HasPrefix(p, "/api/v1/repos/"+owner+"/"+repo+"/contents/"):
|
||||
path := strings.TrimPrefix(p, "/api/v1/repos/"+owner+"/"+repo+"/contents/")
|
||||
f.fetched = append(f.fetched, path)
|
||||
content, ok := f.files[path]
|
||||
if !ok {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"message":"not found"}`))
|
||||
return
|
||||
}
|
||||
_, _ = fmt.Fprintf(w, `{"path":%q,"sha":"s","size":%d,"content":%q,"encoding":"base64"}`, path, len(content), b64(content))
|
||||
|
||||
default:
|
||||
t.Errorf("unexpected request: %s %s", r.Method, p)
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSearchCode_FindsMatchInTree(t *testing.T) {
|
||||
f := &codeSearchFake{files: map[string]string{
|
||||
"internal/gitea/code_search.go": "func (c *Client) SearchCode(ctx context.Context) {}\n",
|
||||
"internal/gitea/repos.go": "func (c *Client) ListRepos() {}\n",
|
||||
}}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
@@ -34,6 +81,92 @@ func TestSearchCode(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.Len(t, hits, 1)
|
||||
assert.Equal(t, "internal/gitea/code_search.go", hits[0].Path)
|
||||
assert.Equal(t, "func (c *Client) SearchCode", hits[0].Snippet)
|
||||
assert.InDelta(t, 2.5, hits[0].Score, 0.001)
|
||||
assert.Contains(t, hits[0].Snippet, "SearchCode")
|
||||
assert.Contains(t, hits[0].HTMLURL, "internal/gitea/code_search.go")
|
||||
assert.Equal(t, 1.0, hits[0].Score)
|
||||
}
|
||||
|
||||
func TestSearchCode_CaseInsensitive(t *testing.T) {
|
||||
f := &codeSearchFake{files: map[string]string{
|
||||
"README.md": "# MyProject\n\nBuild instructions here.\n",
|
||||
}}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
hits, err := c.SearchCode(context.Background(), "mathias", "infra", "myproject", 1, 30)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, hits, 1)
|
||||
}
|
||||
|
||||
func TestSearchCode_SkipsBinaryExtensionWithoutFetching(t *testing.T) {
|
||||
f := &codeSearchFake{files: map[string]string{
|
||||
"assets/logo.png": "SearchCode", // would match if fetched — must not be
|
||||
"main.go": "package main\n",
|
||||
}}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
hits, err := c.SearchCode(context.Background(), "mathias", "infra", "SearchCode", 1, 30)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, hits)
|
||||
assert.NotContains(t, f.fetched, "assets/logo.png", "binary extension must be skipped before fetch")
|
||||
}
|
||||
|
||||
func TestSearchCode_SkipsOversizedFileWithoutFetching(t *testing.T) {
|
||||
f := &codeSearchFake{
|
||||
files: map[string]string{"vendor/bundle.txt": "SearchCode"},
|
||||
sizes: map[string]int64{"vendor/bundle.txt": 10 * 1024 * 1024}, // 10MB per the tree listing
|
||||
}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
hits, err := c.SearchCode(context.Background(), "mathias", "infra", "SearchCode", 1, 30)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, hits)
|
||||
assert.NotContains(t, f.fetched, "vendor/bundle.txt", "oversized file must be skipped before fetch")
|
||||
}
|
||||
|
||||
func TestSearchCode_SkipsBinaryContentNullByte(t *testing.T) {
|
||||
f := &codeSearchFake{files: map[string]string{
|
||||
"data.bin": "SearchCode\x00binary-marker",
|
||||
}}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
hits, err := c.SearchCode(context.Background(), "mathias", "infra", "SearchCode", 1, 30)
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, hits, "content with a null byte must be treated as binary even past the extension filter")
|
||||
}
|
||||
|
||||
func TestSearchCode_Pagination(t *testing.T) {
|
||||
files := map[string]string{}
|
||||
for i := 0; i < 5; i++ {
|
||||
files[fmt.Sprintf("file%d.go", i)] = strings.Repeat("hit ", i+1) // increasing occurrence count => increasing score
|
||||
}
|
||||
f := &codeSearchFake{files: files}
|
||||
srv := httptest.NewServer(f.handler(t, "mathias", "infra", "main"))
|
||||
defer srv.Close()
|
||||
|
||||
c := gitea.NewClient(srv.URL, "tok")
|
||||
page1, err := c.SearchCode(context.Background(), "mathias", "infra", "hit", 1, 2)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, page1, 2)
|
||||
page2, err := c.SearchCode(context.Background(), "mathias", "infra", "hit", 2, 2)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, page2, 2)
|
||||
|
||||
assert.NotEqual(t, page1[0].Path, page2[0].Path, "pages must not overlap")
|
||||
assert.True(t, page1[0].Score >= page1[1].Score, "page1 sorted desc by score")
|
||||
assert.True(t, page1[1].Score >= page2[0].Score, "page1's worst must rank >= page2's best")
|
||||
}
|
||||
|
||||
func TestSearchCode_EmptyQueryErrors(t *testing.T) {
|
||||
c := gitea.NewClient("http://unused", "tok")
|
||||
_, err := c.SearchCode(context.Background(), "mathias", "infra", "", 1, 30)
|
||||
require.Error(t, err)
|
||||
assert.True(t, errors.Is(err, gitea.ErrValidation))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user