Skip to content

Commit 96349cc

Browse files
radimsemclaude
andauthored
fix(store): retry FTS5 parse errors with safe-quoted form (#209)
## Summary Fixes #208 — `Store.Search` / `Store.SearchRanked` no longer surface opaque `SQL logic error: ...` driver messages for adversarial input. `rewriteQuery` short-circuits on FTS5 operator characters, passing input verbatim to SQLite when any of `" : * ( OR AND NOT NEAR(` appears. Many inputs that match that gate aren't valid FTS5 (`foo"`, `*`, `"unterminated`, `foo:bar`) — SQLite raises a parse error that bubbles to MCP clients as a 5xx-equivalent. **Fix (issue option 2 — try-then-fall-back):** keep `rewriteQuery` and its operator-passthrough contract unchanged; route `Search` / `SearchRanked` through a new `queryFTS` that retries with safely-quoted terms (`quoteAllTerms`) on FTS5 parse error. - Preserves the advanced-query affordance for `label:foo`, `snap*`, `"exact phrase"` (existing `TestRewriteQuery` rows still green). - Adversarial input degrades to zero results instead of erroring. - Retry trigger matches the `SQL logic error:` class modernc surfaces FTS5 parse failures under (not `fts5: syntax error` — the actual driver wording). The retry is idempotent for non-FTS5 logic errors, so real DB issues still surface. ## Test plan - [x] `TestQuoteAllTerms` — table-driven, covers `"` escape, single-char ops, empty - [x] `TestSearch_AdversarialInput` — 6 inputs from the issue table; asserts `err == nil` for both `Search` and `SearchRanked` - [x] `TestRewriteQuery` operator-passthrough contract preserved (`label:snapshot`, `snap*`, `"exact phrase"`) - [x] `make build`, `make test`, `gofmt -l`, `go vet ./pkg/store/...` all green Reviewed by `go-style-reviewer` (project subagent) + Codex (generic, via `/forge ... codex`). Loop converged in 2 passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e378bf7 commit 96349cc

2 files changed

Lines changed: 96 additions & 2 deletions

File tree

pkg/store/search.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type RankedNode struct {
1212
}
1313

1414
func (s *Store) Search(ctx context.Context, query string, limit int) ([]*Node, error) {
15-
rows, err := s.db.QueryContext(ctx, qSearchFTS, rewriteQuery(query), limit)
15+
rows, err := s.queryFTS(ctx, qSearchFTS, query, limit)
1616
if err != nil {
1717
return nil, err
1818
}
@@ -22,7 +22,7 @@ func (s *Store) Search(ctx context.Context, query string, limit int) ([]*Node, e
2222
}
2323

2424
func (s *Store) SearchRanked(ctx context.Context, query string, limit int) ([]*RankedNode, error) {
25-
rows, err := s.db.QueryContext(ctx, qSearchRanked, rewriteQuery(query), limit)
25+
rows, err := s.queryFTS(ctx, qSearchRanked, query, limit)
2626
if err != nil {
2727
return nil, err
2828
}
@@ -75,3 +75,39 @@ func rewriteQuery(q string) string {
7575

7676
return strings.Join(quoted, " OR ")
7777
}
78+
79+
// Quote each term as an FTS5 phrase, escaping internal " as "" per FTS5 syntax.
80+
func quoteAllTerms(q string) string {
81+
q = strings.TrimSpace(q)
82+
if q == "" {
83+
return q
84+
}
85+
86+
terms := strings.Fields(q)
87+
quoted := make([]string, len(terms))
88+
for i, t := range terms {
89+
quoted[i] = `"` + strings.ReplaceAll(t, `"`, `""`) + `"`
90+
}
91+
92+
return strings.Join(quoted, " OR ")
93+
}
94+
95+
// Match the SQLite logic-error class FTS5 parse failures arrive under in modernc.
96+
func isFTS5SyntaxError(err error) bool {
97+
if err == nil {
98+
return false
99+
}
100+
return strings.Contains(err.Error(), "SQL logic error:")
101+
}
102+
103+
// Retry with safely-quoted terms on FTS5 parse error so adversarial input degrades to zero results.
104+
func (s *Store) queryFTS(ctx context.Context, stmt, q string, limit int) (*sql.Rows, error) {
105+
rows, err := s.db.QueryContext(ctx, stmt, rewriteQuery(q), limit)
106+
if err == nil {
107+
return rows, nil
108+
}
109+
if !isFTS5SyntaxError(err) {
110+
return nil, err
111+
}
112+
return s.db.QueryContext(ctx, stmt, quoteAllTerms(q), limit)
113+
}

pkg/store/store_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,33 @@ func TestRewriteQuery(t *testing.T) {
11831183
}
11841184
}
11851185

1186+
func TestQuoteAllTerms(t *testing.T) {
1187+
tests := []struct {
1188+
in string
1189+
want string
1190+
}{
1191+
{"", ""},
1192+
{"hello", `"hello"`},
1193+
{"hello world", `"hello" OR "world"`},
1194+
// Operator-bearing inputs that rewriteQuery would pass through verbatim
1195+
// are sanitized to well-formed FTS5 phrases here.
1196+
{`foo"`, `"foo"""`},
1197+
{`"unterminated`, `"""unterminated"`},
1198+
{"*", `"*"`},
1199+
{"(", `"("`},
1200+
{"foo:bar", `"foo:bar"`},
1201+
{"snap*", `"snap*"`},
1202+
{"ZEBRA-4471", `"ZEBRA-4471"`},
1203+
}
1204+
1205+
for _, tt := range tests {
1206+
got := quoteAllTerms(tt.in)
1207+
if got != tt.want {
1208+
t.Errorf("quoteAllTerms(%q) = %q, want %q", tt.in, got, tt.want)
1209+
}
1210+
}
1211+
}
1212+
11861213
func TestSearchMultiWord(t *testing.T) {
11871214
st := openTestDB(t)
11881215
ctx := context.Background()
@@ -1246,6 +1273,37 @@ func TestSearchPunctuation(t *testing.T) {
12461273
}
12471274
}
12481275

1276+
func TestSearch_AdversarialInput(t *testing.T) {
1277+
st := openTestDB(t)
1278+
ctx := context.Background()
1279+
1280+
n := testNode("aaaaaaaa", "")
1281+
n.Content = "the quick brown fox jumps over the lazy dog"
1282+
n.Label = "fox sentence"
1283+
must(t, st.UpsertNode(ctx, n))
1284+
1285+
// Inputs from issue #208 — operator-bearing but not valid FTS5. Pre-fix each
1286+
// surfaced an opaque "fts5: syntax error" from the driver; post-fix each
1287+
// must return cleanly (empty or matched), never an error.
1288+
inputs := []string{
1289+
`foo"`,
1290+
`foo(`,
1291+
`*`,
1292+
`"unterminated`,
1293+
`foo:bar`,
1294+
`(`,
1295+
}
1296+
1297+
for _, in := range inputs {
1298+
if _, err := st.Search(ctx, in, 10); err != nil {
1299+
t.Errorf("Search(%q): %v", in, err)
1300+
}
1301+
if _, err := st.SearchRanked(ctx, in, 10); err != nil {
1302+
t.Errorf("SearchRanked(%q): %v", in, err)
1303+
}
1304+
}
1305+
}
1306+
12491307
func TestListFileSummaries(t *testing.T) {
12501308
st := openTestDB(t)
12511309
ctx := context.Background()

0 commit comments

Comments
 (0)