From a2c2f7a7c1b17d74f006d291aeb6b1e04fc5cb02 Mon Sep 17 00:00:00 2001 From: mithral Date: Fri, 9 Oct 2026 21:10:44 -0400 Subject: [PATCH] Phase 2b: FTS5 search - migration 0003: an FTS5 index over media_items as an external-content table, so the index stores no copy of the text. Triggers keep it in sync for every write path, including the scanner's bulk inserts. - migrate: splitStatements now tracks BEGIN...END depth, so CREATE TRIGGER bodies survive as one statement. Without this, the trigger's internal semicolons would tear it apart. - search: every whitespace term is quoted before it reaches FTS5. Unquoted input like a bare quote, a hyphen, or the word OR is valid FTS5 syntax and would either error or silently change the query's meaning. Tested against a list of hostile inputs, including a DROP TABLE attempt. - scoped through the same VisibleLibraryIDs chokepoint; an empty library set returns nothing, never everything. - web: search box in the topbar and a results view. Verified live: finds items across libraries, hostile input returns 200 with no results, index stays in sync on insert/update/delete. --- cmd/vellum/main.go | 2 + internal/core/migrate.go | 74 +++++++- internal/core/migrate_test.go | 28 ++- internal/search/handlers.go | 60 ++++++ internal/search/search.go | 84 ++++++++ internal/search/search_test.go | 200 ++++++++++++++++++++ migrations/0003_search.sql | 32 ++++ web/src/lib/api.ts | 11 ++ web/src/lib/components/SearchResults.svelte | 109 +++++++++++ web/src/lib/components/Shell.svelte | 27 +++ web/src/lib/nav.ts | 1 + 11 files changed, 619 insertions(+), 9 deletions(-) create mode 100644 internal/search/handlers.go create mode 100644 internal/search/search.go create mode 100644 internal/search/search_test.go create mode 100644 migrations/0003_search.sql create mode 100644 web/src/lib/components/SearchResults.svelte diff --git a/cmd/vellum/main.go b/cmd/vellum/main.go index 530067d..6fe992e 100644 --- a/cmd/vellum/main.go +++ b/cmd/vellum/main.go @@ -19,6 +19,7 @@ import ( "github.com/mithral/vellum/internal/media" "github.com/mithral/vellum/internal/modules/comics" "github.com/mithral/vellum/internal/modules/ebooks" + "github.com/mithral/vellum/internal/search" "github.com/mithral/vellum/migrations" ) @@ -77,6 +78,7 @@ func run() error { media.NewHandlers(db, log, authenticator, cfg.ConfigDir).Register(api.Mux()) ebooks.NewHandlers(db, log, authenticator).Register(api.Mux()) comics.NewHandlers(db, log, authenticator).Register(api.Mux()) + search.NewHandlers(db, log, authenticator).Register(api.Mux()) srv := &http.Server{ Addr: cfg.Listen, diff --git a/internal/core/migrate.go b/internal/core/migrate.go index eb4ab5c..5a6f9e5 100644 --- a/internal/core/migrate.go +++ b/internal/core/migrate.go @@ -98,20 +98,80 @@ func migrationNumber(name string) (int, error) { // splitStatements splits a SQL file on semicolons that terminate a statement. // -// This is not a general SQL parser. It is correct for the schema files this -// project writes: no stored procedures, no BEGIN...END blocks, no semicolons -// inside string literals. Comments are stripped first so that a ';' inside a -// comment cannot split a statement. +// Semicolons inside BEGIN...END blocks (trigger bodies) do not split, because +// SQLite's CREATE TRIGGER syntax requires them and the whole trigger must be +// executed as one statement. It is not a general SQL parser: it does not track +// string literals beyond quotes, and it treats a bare END as closing a block, +// so a CASE...END expression inside a trigger would be mis-split. The schema +// files this project writes contain neither. func splitStatements(sqlText string) []string { + clean := stripSQLComments(sqlText) var out []string - for _, stmt := range strings.Split(stripSQLComments(sqlText), ";") { - if s := strings.TrimSpace(stmt); s != "" { - out = append(out, s) + var b strings.Builder + depth := 0 + + for i := 0; i < len(clean); { + c := clean[i] + + // Copy string literals verbatim so a ';' or keyword inside one is inert. + if c == '\'' || c == '"' { + b.WriteByte(c) + quote := c + i++ + for i < len(clean) { + b.WriteByte(clean[i]) + if clean[i] == quote { + i++ + break + } + i++ + } + continue } + + if isWordByte(c) { + start := i + for i < len(clean) && isWordByte(clean[i]) { + i++ + } + switch strings.ToUpper(clean[start:i]) { + case "BEGIN": + depth++ + case "END": + if depth > 0 { + depth-- + } + } + b.WriteString(clean[start:i]) + continue + } + + if c == ';' && depth == 0 { + if s := strings.TrimSpace(b.String()); s != "" { + out = append(out, s) + } + b.Reset() + i++ + continue + } + + b.WriteByte(c) + i++ + } + + if s := strings.TrimSpace(b.String()); s != "" { + out = append(out, s) } return out } +func isWordByte(c byte) bool { + return c == '_' || + (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') +} + // stripSQLComments removes -- line comments and /* */ block comments. func stripSQLComments(s string) string { var b strings.Builder diff --git a/internal/core/migrate_test.go b/internal/core/migrate_test.go index 7045d32..7e6d0f4 100644 --- a/internal/core/migrate_test.go +++ b/internal/core/migrate_test.go @@ -43,8 +43,8 @@ func TestMigrateAppliesAndIsIdempotent(t *testing.T) { if err := db.QueryRowContext(ctx, "PRAGMA user_version").Scan(&version); err != nil { t.Fatalf("read user_version: %v", err) } - if version != 2 { - t.Errorf("user_version = %d, want 2", version) + if version != 3 { + t.Errorf("user_version = %d, want 3", version) } var tables int @@ -122,3 +122,27 @@ CREATE TABLE b (id INTEGER); t.Errorf("statement 1 = %q", got[1]) } } + +func TestSplitStatementsKeepsTriggerBodiesWhole(t *testing.T) { + in := ` +CREATE TABLE t (id INTEGER, name TEXT); +CREATE TRIGGER t_ai AFTER INSERT ON t BEGIN + INSERT INTO t(name) VALUES (new.name); + UPDATE t SET name = 'x' WHERE id = new.id; +END; +CREATE TABLE after_trigger (id INTEGER); +` + got := splitStatements(in) + if len(got) != 3 { + t.Fatalf("got %d statements, want 3:\n%#v", len(got), got) + } + if !strings.Contains(got[1], "CREATE TRIGGER") || !strings.Contains(got[1], "END") { + t.Errorf("trigger was split apart: %q", got[1]) + } + if n := strings.Count(got[1], ";"); n != 2 { + t.Errorf("trigger body has %d semicolons, want 2 (the internal ones)", n) + } + if !strings.Contains(got[2], "after_trigger") { + t.Errorf("statement after the trigger = %q", got[2]) + } +} diff --git a/internal/search/handlers.go b/internal/search/handlers.go new file mode 100644 index 0000000..25bbbb5 --- /dev/null +++ b/internal/search/handlers.go @@ -0,0 +1,60 @@ +package search + +import ( + "database/sql" + "log/slog" + "net/http" + "strconv" + + "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/httpx" +) + +// Handlers serves the search endpoint. +type Handlers struct { + db *sql.DB + log *slog.Logger + auth *auth.Authenticator +} + +// NewHandlers builds the search HTTP layer. +func NewHandlers(db *sql.DB, log *slog.Logger, a *auth.Authenticator) *Handlers { + return &Handlers{db: db, log: log, auth: a} +} + +// Register mounts the route. +func (h *Handlers) Register(mux *http.ServeMux) { + mux.Handle("GET /api/v1/search", h.auth.Require(h.handleSearch)) +} + +func (h *Handlers) handleSearch(w http.ResponseWriter, r *http.Request, u auth.User) { + q := r.URL.Query().Get("q") + if q == "" { + httpx.WriteJSON(w, http.StatusOK, map[string]any{"results": []Result{}, "query": ""}) + return + } + + limit := 50 + if v := r.URL.Query().Get("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 200 { + limit = n + } + } + + // Scope the search to libraries the caller may read. This is the same + // chokepoint every other read path uses. + libs, err := auth.VisibleLibraryIDs(r.Context(), h.db, u) + if err != nil { + h.log.Error("visible libraries", "err", err) + httpx.WriteError(w, http.StatusInternalServerError, "internal server error") + return + } + + results, err := Query(r.Context(), h.db, q, libs, limit) + if err != nil { + h.log.Error("search", "err", err) + httpx.WriteError(w, http.StatusInternalServerError, "internal server error") + return + } + httpx.WriteJSON(w, http.StatusOK, map[string]any{"results": results, "query": q}) +} diff --git a/internal/search/search.go b/internal/search/search.go new file mode 100644 index 0000000..394db30 --- /dev/null +++ b/internal/search/search.go @@ -0,0 +1,84 @@ +// Package search queries the FTS5 index over media item titles and paths. +package search + +import ( + "context" + "database/sql" + "fmt" + "strings" +) + +// Result is one matching item. +type Result struct { + ID int64 `json:"id"` + LibraryID int64 `json:"library_id"` + Kind string `json:"kind"` + Path string `json:"path"` + Title string `json:"title"` +} + +// Query searches visible libraries for a free-text string. +// +// Each whitespace-separated term becomes a quoted FTS5 token joined by +// implicit AND. Quoting is not cosmetic: raw user input containing a quote, a +// hyphen, or the word OR would otherwise be parsed as FTS5 query syntax and +// raise an error, or silently change the meaning of the search. +func Query(ctx context.Context, db *sql.DB, q string, libraryIDs []int64, limit int) ([]Result, error) { + terms := ftsTerms(q) + if len(terms) == 0 { + return []Result{}, nil + } + if len(libraryIDs) == 0 { + return []Result{}, nil + } + if limit <= 0 || limit > 200 { + limit = 50 + } + + placeholders := make([]string, len(libraryIDs)) + args := make([]any, 0, len(libraryIDs)+2) + args = append(args, strings.Join(terms, " ")) + for i, id := range libraryIDs { + placeholders[i] = "?" + args = append(args, id) + } + args = append(args, limit) + + query := ` + SELECT i.id, i.library_id, i.kind, i.path, i.title + FROM media_search s + JOIN media_items i ON i.id = s.rowid + WHERE media_search MATCH ? + AND i.missing = 0 + AND i.library_id IN (` + strings.Join(placeholders, ",") + `) + ORDER BY bm25(media_search) + LIMIT ?` + + rows, err := db.QueryContext(ctx, query, args...) + if err != nil { + return nil, fmt.Errorf("search: query: %w", err) + } + defer rows.Close() + + out := []Result{} + for rows.Next() { + var r Result + if err := rows.Scan(&r.ID, &r.LibraryID, &r.Kind, &r.Path, &r.Title); err != nil { + return nil, err + } + out = append(out, r) + } + return out, rows.Err() +} + +// ftsTerms turns free text into a safe FTS5 expression: every term quoted, so +// the result is always valid FTS5 and always means "all these words". +func ftsTerms(q string) []string { + var terms []string + for _, field := range strings.Fields(q) { + // A double quote inside a quoted FTS5 string is escaped by doubling it. + escaped := strings.ReplaceAll(field, `"`, `""`) + terms = append(terms, `"`+escaped+`"`) + } + return terms +} diff --git a/internal/search/search_test.go b/internal/search/search_test.go new file mode 100644 index 0000000..25c4f6e --- /dev/null +++ b/internal/search/search_test.go @@ -0,0 +1,200 @@ +package search + +import ( + "context" + "database/sql" + "io" + "log/slog" + "path/filepath" + "testing" + + "github.com/mithral/vellum/internal/core" + "github.com/mithral/vellum/migrations" +) + +func testDB(t *testing.T) *sql.DB { + t.Helper() + db, err := core.OpenDB(filepath.Join(t.TempDir(), "search.db")) + if err != nil { + t.Fatalf("OpenDB: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + if err := core.Migrate(context.Background(), db, migrations.FS, log); err != nil { + t.Fatalf("Migrate: %v", err) + } + return db +} + +func addLibrary(t *testing.T, db *sql.DB, name, kind string) int64 { + t.Helper() + var id int64 + err := db.QueryRowContext(context.Background(), + `INSERT INTO libraries (name, kind, root_path) VALUES (?, ?, ?) RETURNING id`, + name, kind, "/media/"+name).Scan(&id) + if err != nil { + t.Fatal(err) + } + return id +} + +func addItem(t *testing.T, db *sql.DB, libraryID int64, kind, path, title string) int64 { + t.Helper() + var id int64 + err := db.QueryRowContext(context.Background(), + `INSERT INTO media_items (library_id, kind, path, title, sort_title) VALUES (?, ?, ?, ?, ?) RETURNING id`, + libraryID, kind, path, title, title).Scan(&id) + if err != nil { + t.Fatal(err) + } + return id +} + +func titles(results []Result) []string { + out := make([]string, len(results)) + for i, r := range results { + out[i] = r.Title + } + return out +} + +func contains(haystack []string, needle string) bool { + for _, h := range haystack { + if h == needle { + return true + } + } + return false +} + +func TestQueryFindsItems(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := addLibrary(t, db, "ebooks", "ebooks") + addItem(t, db, lib, "ebooks", "hobbit.epub", "The Hobbit") + addItem(t, db, lib, "ebooks", "hail.epub", "Project Hail Mary") + + res, err := Query(ctx, db, "hobbit", []int64{lib}, 50) + if err != nil { + t.Fatalf("Query: %v", err) + } + if len(res) != 1 || res[0].Title != "The Hobbit" { + t.Fatalf("results = %v", titles(res)) + } + + // Multi-term is an implicit AND. + res, err = Query(ctx, db, "project mary", []int64{lib}, 50) + if err != nil { + t.Fatal(err) + } + if len(res) != 1 || res[0].Title != "Project Hail Mary" { + t.Errorf("multi-term results = %v", titles(res)) + } + + // A term that matches nothing. + res, err = Query(ctx, db, "nonexistent", []int64{lib}, 50) + if err != nil { + t.Fatal(err) + } + if len(res) != 0 { + t.Errorf("expected no results, got %v", titles(res)) + } +} + +func TestQueryIsScopedToLibraries(t *testing.T) { + db := testDB(t) + ctx := context.Background() + allowed := addLibrary(t, db, "allowed", "ebooks") + hidden := addLibrary(t, db, "hidden", "ebooks") + addItem(t, db, allowed, "ebooks", "a.epub", "Shared Word") + addItem(t, db, hidden, "ebooks", "b.epub", "Shared Word Two") + + res, err := Query(ctx, db, "shared", []int64{allowed}, 50) + if err != nil { + t.Fatal(err) + } + if len(res) != 1 || res[0].LibraryID != allowed { + t.Fatalf("scoping failed: %v", titles(res)) + } + + // No libraries means no results, never everything. + res, err = Query(ctx, db, "shared", nil, 50) + if err != nil { + t.Fatal(err) + } + if len(res) != 0 { + t.Errorf("empty library set returned %v", titles(res)) + } +} + +func TestQueryHandlesHostileInput(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := addLibrary(t, db, "ebooks", "ebooks") + addItem(t, db, lib, "ebooks", "a.epub", "The Hobbit") + + // None of these may raise an FTS5 syntax error. + for _, q := range []string{ + `"`, `"unbalanced`, `OR`, `AND NOT`, `foo*`, `-hobbit`, `title:hobbit`, + `hobbit OR`, `*`, `""`, `NEAR(`, `a"b`, `(); DROP TABLE users;--`, + } { + if _, err := Query(ctx, db, q, []int64{lib}, 50); err != nil { + t.Errorf("Query(%q) errored: %v", q, err) + } + } + + // The injection attempt must not have dropped anything. + var n int + if err := db.QueryRowContext(ctx, `SELECT count(*) FROM media_items`).Scan(&n); err != nil { + t.Fatalf("media_items is gone: %v", err) + } + if n != 1 { + t.Errorf("media_items count = %d, want 1", n) + } +} + +func TestIndexStaysInSync(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := addLibrary(t, db, "ebooks", "ebooks") + + id := addItem(t, db, lib, "ebooks", "x.epub", "Original Title") + if res, _ := Query(ctx, db, "original", []int64{lib}, 50); len(res) != 1 { + t.Fatalf("insert did not reach the index") + } + + if _, err := db.ExecContext(ctx, `UPDATE media_items SET title = 'Changed Title' WHERE id = ?`, id); err != nil { + t.Fatal(err) + } + if res, _ := Query(ctx, db, "changed", []int64{lib}, 50); len(res) != 1 { + t.Error("update did not reach the index") + } + if res, _ := Query(ctx, db, "original", []int64{lib}, 50); len(res) != 0 { + t.Error("stale entry survived the update") + } + + if _, err := db.ExecContext(ctx, `DELETE FROM media_items WHERE id = ?`, id); err != nil { + t.Fatal(err) + } + if res, _ := Query(ctx, db, "changed", []int64{lib}, 50); len(res) != 0 { + t.Error("delete did not reach the index") + } +} + +func TestMissingItemsAreExcluded(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := addLibrary(t, db, "ebooks", "ebooks") + id := addItem(t, db, lib, "ebooks", "gone.epub", "Vanished Book") + + if _, err := db.ExecContext(ctx, `UPDATE media_items SET missing = 1 WHERE id = ?`, id); err != nil { + t.Fatal(err) + } + res, err := Query(ctx, db, "vanished", []int64{lib}, 50) + if err != nil { + t.Fatal(err) + } + if len(res) != 0 { + t.Errorf("missing item appeared in search: %v", titles(res)) + } +} diff --git a/migrations/0003_search.sql b/migrations/0003_search.sql new file mode 100644 index 0000000..5216070 --- /dev/null +++ b/migrations/0003_search.sql @@ -0,0 +1,32 @@ +-- 0003_search.sql — full-text search over items. +-- +-- FTS5 with an external content table: the index stores no copy of the text, +-- it points back at media_items, so the database does not double in size. +-- Triggers keep the index in sync for every write path, including the scanner's +-- bulk inserts and any future module's. + +CREATE VIRTUAL TABLE media_search USING fts5( + title, + path, + content='media_items', + content_rowid='id', + tokenize='unicode61 remove_diacritics 2' +); + +-- Populate the index from any rows that already exist. +INSERT INTO media_search(media_search) VALUES ('rebuild'); + +CREATE TRIGGER media_items_ai AFTER INSERT ON media_items BEGIN + INSERT INTO media_search(rowid, title, path) VALUES (new.id, new.title, new.path); +END; + +CREATE TRIGGER media_items_ad AFTER DELETE ON media_items BEGIN + INSERT INTO media_search(media_search, rowid, title, path) + VALUES ('delete', old.id, old.title, old.path); +END; + +CREATE TRIGGER media_items_au AFTER UPDATE ON media_items BEGIN + INSERT INTO media_search(media_search, rowid, title, path) + VALUES ('delete', old.id, old.title, old.path); + INSERT INTO media_search(rowid, title, path) VALUES (new.id, new.title, new.path); +END; diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index ea62adf..7d796d3 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -146,12 +146,23 @@ export const api = { recent: () => request<{ progress: Progress[] }>('GET', '/api/v1/progress/recent'), + search: (q: string) => + request<{ results: SearchResult[]; query: string }>('GET', `/api/v1/search?q=${encodeURIComponent(q)}`), + users: () => request<{ users: User[] }>('GET', '/api/v1/users'), createUser: (username: string, password: string, display_name: string, is_admin: boolean) => request<{ user: User }>('POST', '/api/v1/users', { username, password, display_name, is_admin }), } +export interface SearchResult { + id: number + library_id: number + kind: string + path: string + title: string +} + export interface ScanResult { library_id: number scanned: number diff --git a/web/src/lib/components/SearchResults.svelte b/web/src/lib/components/SearchResults.svelte new file mode 100644 index 0000000..a7b783a --- /dev/null +++ b/web/src/lib/components/SearchResults.svelte @@ -0,0 +1,109 @@ + + +{#if loading} +
+{:else if error} +
{error}
+{:else if results.length === 0} +
+

No matches

+

Nothing in your libraries matches “{query}”.

+
+{:else} +

{results.length} result{results.length === 1 ? '' : 's'}

+
+ {#each results as r (r.id)} + + {/each} +
+{/if} + + diff --git a/web/src/lib/components/Shell.svelte b/web/src/lib/components/Shell.svelte index 59fd378..eb2a88b 100644 --- a/web/src/lib/components/Shell.svelte +++ b/web/src/lib/components/Shell.svelte @@ -5,8 +5,16 @@ import LibraryView from './LibraryView.svelte' import Reader from './Reader.svelte' import Users from './Users.svelte' + import SearchResults from './SearchResults.svelte' let kindFilter = $state(null) + let searchBox = $state('') + + function submitSearch(event: SubmitEvent) { + event.preventDefault() + const q = searchBox.trim() + if (q) go({ name: 'search', query: q }) + } const kinds = $derived( $mediaTypes.filter((t) => $libraries.some((l) => l.kind === t.Kind)), @@ -24,6 +32,8 @@ return $libraries.find((l) => l.id === v.id)?.name ?? 'Library' case 'reader': return v.title + case 'search': + return 'Search' case 'users': return 'Users' case 'settings': @@ -79,6 +89,15 @@

{title($view)}

+
+ +
@@ -88,6 +107,8 @@ {:else if $view.name === 'reader'} + {:else if $view.name === 'search'} + {:else if $view.name === 'users'} {/if} @@ -189,10 +210,16 @@ .topbar { display: flex; align-items: center; + justify-content: space-between; + gap: var(--space-4); height: 52px; padding: 0 var(--space-5); border-bottom: 1px solid var(--border); } + .search { + flex: 1; + max-width: 320px; + } .content { flex: 1; overflow-y: auto; diff --git a/web/src/lib/nav.ts b/web/src/lib/nav.ts index 8e2b418..b6dce6c 100644 --- a/web/src/lib/nav.ts +++ b/web/src/lib/nav.ts @@ -5,6 +5,7 @@ export type View = | { name: 'libraries' } | { name: 'library'; id: number } | { name: 'reader'; itemId: number; title: string } + | { name: 'search'; query: string } | { name: 'users' } | { name: 'settings' }