fix(store): escape interior double-quotes in sanitizeFTS to prevent FTS5 crash#586
fix(store): escape interior double-quotes in sanitizeFTS to prevent FTS5 crash#586blak0p wants to merge 1 commit into
Conversation
…TS5 crash sanitizeFTS wrapped each whitespace-separated word in double-quotes for FTS5 MATCH queries, but only stripped surrounding quotes via strings.Trim(w, `"`). Interior quotes were left untouched, so an input like `hello"world` produced `"hello"world"` — an unterminated FTS5 string literal that crashed every search path (mem_search, candidate detection) with "SQL logic error: unterminated string (1)". See issue Gentleman-Programming#574. FTS5 escapes a literal double-quote inside a quoted phrase by doubling it (`""`), so we apply strings.ReplaceAll(w, `"`, `""`) after the Trim. Now `hello"world` correctly becomes `"hello""world"`. Why this spot and not the query layer: sanitizeFTS is the single funnel used by both search entry points (store.go:2171, store.go:2644), so the fix covers all affected call sites without touching sanitizeFTSCandidates (relations.go), which builds OR-based queries and has its own escaping. Adds TestSanitizeFTS covering plain words, multiple words, single and multiple interior quotes, already-quoted input, just-quotes, mixed input, and the empty case. Written first (failing) per TDD; all 8 cases pass after the one-line fix.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesFTS Sanitization Fix
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔗 Linked Issue
Closes #574
🏷️ PR Type
type:bug— Bug fix📝 Summary
hello"world→SQL logic error: unterminated string (1)).sanitizeFTSper the FTS5 string-literal escape rule ("""), sohello"world→"hello""world".TestSanitizeFTSwith 8 cases (written first, failing per TDD; all pass after the fix).🔧 Changes
internal/store/store.gostrings.ReplaceAll(w, \"\", \"\"\"\")afterTriminsanitizeFTSto escape interior double-quotes for FTS5.internal/store/store_test.goTestSanitizeFTScovering plain words, multiple words, single/multiple interior quotes, already-quoted, just-quotes, mixed, and empty input.🧪 Test Plan
TestSanitizeFTSfails on unpatched code (RED) and passes after the fix (GREEN):go test ./internal/store/ -run TestSanitizeFTS -vgo test ./internal/store/go build -o /tmp/engram-fix ./cmd/engram && /tmp/engram-fix search 'hello"world'→ clean "No memories found" (exit 0)./tmp/engram-fix search 'sanitizeFTS'→ returns results.✅ Contributor Checklist
type:*label (type:bug)Co-Authored-BytrailersSummary by CodeRabbit