Skip to content

fix(store): escape interior double-quotes in sanitizeFTS to prevent FTS5 crash#586

Open
blak0p wants to merge 1 commit into
Gentleman-Programming:mainfrom
blak0p:fix/fts5-interior-quote
Open

fix(store): escape interior double-quotes in sanitizeFTS to prevent FTS5 crash#586
blak0p wants to merge 1 commit into
Gentleman-Programming:mainfrom
blak0p:fix/fts5-interior-quote

Conversation

@blak0p

@blak0p blak0p commented Jul 9, 2026

Copy link
Copy Markdown

🔗 Linked Issue

Closes #574


🏷️ PR Type

  • type:bug — Bug fix

📝 Summary

  • Fixes FTS5 search crash on queries containing an interior double-quote (e.g. hello"worldSQL logic error: unterminated string (1)).
  • Doubles interior " inside sanitizeFTS per the FTS5 string-literal escape rule ("""), so hello"world"hello""world".
  • Adds TestSanitizeFTS with 8 cases (written first, failing per TDD; all pass after the fix).

🔧 Changes

File Change
internal/store/store.go Add strings.ReplaceAll(w, \"\", \"\"\"\") after Trim in sanitizeFTS to escape interior double-quotes for FTS5.
internal/store/store_test.go Add TestSanitizeFTS covering plain words, multiple words, single/multiple interior quotes, already-quoted, just-quotes, mixed, and empty input.

🧪 Test Plan

  • New test TestSanitizeFTS fails on unpatched code (RED) and passes after the fix (GREEN): go test ./internal/store/ -run TestSanitizeFTS -v
  • Full store package passes (no regressions): go test ./internal/store/
  • Built binary no longer crashes: go build -o /tmp/engram-fix ./cmd/engram && /tmp/engram-fix search 'hello"world' → clean "No memories found" (exit 0).
  • Normal search still works: /tmp/engram-fix search 'sanitizeFTS' → returns results.

✅ Contributor Checklist

Summary by CodeRabbit

  • Bug Fixes
    • Improved search query handling so text containing double quotes no longer causes malformed search expressions.
    • Added coverage for quoted, mixed, and empty inputs to ensure search terms are formatted consistently.

…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.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 89c768bb-bb3d-4431-abf6-ccb447523245

📥 Commits

Reviewing files that changed from the base of the PR and between be4b613 and 9190069.

📒 Files selected for processing (2)
  • internal/store/store.go
  • internal/store/store_test.go

📝 Walkthrough

Walkthrough

The sanitizeFTS function in internal/store/store.go now escapes interior double-quote characters within each token by doubling them before wrapping tokens in FTS5-quoted literals, preventing malformed MATCH queries. A new unit test validates this behavior across multiple input cases.

Changes

FTS Sanitization Fix

Layer / File(s) Summary
Escape interior quotes in FTS sanitization
internal/store/store.go, internal/store/store_test.go
sanitizeFTS doubles interior double-quote characters in each token before FTS5-quoting, and a new table-driven test (TestSanitizeFTS) validates plain words, multi-word input, quote escaping, mixed content, and empty input.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main bug fix in sanitizeFTS and its FTS5 crash prevention.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/fts5-interior-quote

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] FTS5 search returns HTTP 500 on queries containing an interior double-quote (e.g. foo"bar)

1 participant