Skip to content

Commit 32d8193

Browse files
authored
fix(search): cap query length + generic MCP degradation note (#126)
v0.14.0 security-review hardening: enforce MaxSearchQueryLen on the hybrid lexical + embed-query paths (DoS guard regression); make the MCP degradation note generic so the embedding endpoint URL/host isn't leaked to clients (logged server-side instead).
1 parent 54b43a1 commit 32d8193

4 files changed

Lines changed: 52 additions & 2 deletions

File tree

internal/cortex/semantic.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ func (c *Cortex) embedQuery(ctx context.Context, e Embedder, model, query string
127127
if q == "" {
128128
return nil, nil
129129
}
130+
if len(q) > MaxSearchQueryLen {
131+
return nil, fmt.Errorf("query too long (%d chars, max %d)", len(q), MaxSearchQueryLen)
132+
}
130133
vecs, err := e.Embed(ctx, model, []string{q})
131134
if err != nil {
132135
return nil, fmt.Errorf("embedding query: %w", err)
@@ -240,6 +243,9 @@ func rrfFuse(lex []Row, sem []ScoredRow, weight float64, limit int) []ScoredRow
240243
// BM25 relevance order (best first) — the lexical input to hybrid fusion.
241244
// Trashed excluded; archived excluded unless includeArchived.
242245
func (c *Cortex) lexicalRanked(query string, includeArchived bool) ([]Row, error) {
246+
if len(query) > MaxSearchQueryLen {
247+
return nil, fmt.Errorf("query too long (%d chars, max %d)", len(query), MaxSearchQueryLen)
248+
}
243249
ftsQuery := SanitizeFTS5Query(query)
244250
if strings.TrimSpace(ftsQuery) == "" {
245251
return nil, nil

internal/cortex/semantic_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,18 @@ func TestHybridSimilar_ExcludesSource(t *testing.T) {
183183
}
184184
}
185185

186+
func TestSemanticSearch_QueryTooLong(t *testing.T) {
187+
cx, _ := setupSemantic(t)
188+
ctx := context.Background()
189+
long := strings.Repeat("x", 1001) // > MaxSearchQueryLen (1000)
190+
if _, err := cx.SemanticSearch(ctx, topicEmbedder{}, long, cortex.SemanticOpts{Model: "tm"}); err == nil {
191+
t.Error("over-long semantic query should be rejected (DoS guard)")
192+
}
193+
if _, err := cx.HybridSearch(ctx, topicEmbedder{}, long, cortex.SemanticOpts{Model: "tm"}, 0.5); err == nil {
194+
t.Error("over-long hybrid query should be rejected (DoS guard)")
195+
}
196+
}
197+
186198
func TestSemanticSearch_Guards(t *testing.T) {
187199
cx := setup(t)
188200
ctx := context.Background()

internal/mcp/semantic_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,32 @@ func TestSearchTraces_SemanticEndToEnd(t *testing.T) {
158158
}
159159
}
160160

161+
func TestSearchTraces_SemanticEndpointDownGenericNote(t *testing.T) {
162+
cx := newTestCortex(t)
163+
// Configured but unreachable endpoint with a recognizable host:port.
164+
writeSearchConfig(t, cx, "http://127.0.0.1:1/v1", "tm")
165+
tr := trace.New("lexical findable widget", "note", "agent", nil, "body about widgets")
166+
if err := cx.Add(tr); err != nil {
167+
t.Fatalf("Add: %v", err)
168+
}
169+
s := NewServer(cx, "test", "")
170+
text, isErr := callTool(t, s, "search_traces", map[string]any{"query": "findable", "mode": "semantic"})
171+
if isErr {
172+
t.Fatalf("search_traces errored: %s", text)
173+
}
174+
if !strings.Contains(text, "temporarily unavailable") {
175+
t.Errorf("expected generic degradation note, got:\n%s", text)
176+
}
177+
// The endpoint host must NOT leak into client-facing output.
178+
if strings.Contains(text, "127.0.0.1") {
179+
t.Errorf("degradation note leaked the endpoint host:\n%s", text)
180+
}
181+
// Lexical fallback still returns the matching trace.
182+
if !strings.Contains(text, tr.ID) {
183+
t.Errorf("expected lexical fallback result %s, got:\n%s", tr.ID, text)
184+
}
185+
}
186+
161187
func TestSearchTraces_HybridEndToEnd(t *testing.T) {
162188
server := topicEmbedServer(t)
163189
defer server.Close()

internal/mcp/server.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"log"
89
"net/url"
910
"strings"
1011

@@ -265,7 +266,11 @@ func NewServer(cx *cortex.Cortex, noemaVersion string, federationMode string) *s
265266
res, serr = cx.SemanticSearchAs(ctx, emb, query, opts, cortex.ActorAgent, cortex.DefaultSearchHitTopN)
266267
}
267268
if serr != nil {
268-
note = "[" + mode + " search unavailable (" + serr.Error() + "); showing lexical results]\n"
269+
// Log the detailed error server-side; surface only a generic
270+
// note to the client so endpoint URLs / internal hostnames in
271+
// the error don't leak into MCP output.
272+
log.Printf("[mcp] %s search failed, falling back to lexical: %v", mode, serr)
273+
note = "[" + mode + " search temporarily unavailable; showing lexical results]\n"
269274
} else {
270275
return mcp.NewToolResultText(formatRows(scoredToRows(res))), nil
271276
}
@@ -315,7 +320,8 @@ func NewServer(cx *cortex.Cortex, noemaVersion string, federationMode string) *s
315320
res, serr = cx.SemanticSimilarAs(traceID, opts, cortex.ActorAgent, cortex.DefaultSearchHitTopN)
316321
}
317322
if serr != nil {
318-
note = "[" + mode + " similar unavailable (" + serr.Error() + "); showing lexical results]\n"
323+
log.Printf("[mcp] %s similar failed, falling back to lexical: %v", mode, serr)
324+
note = "[" + mode + " similar temporarily unavailable; showing lexical results]\n"
319325
} else {
320326
return mcp.NewToolResultText(formatSimilarMatches(scoredToMatches(res))), nil
321327
}

0 commit comments

Comments
 (0)