API: Add DoS protection to FTS search

[MrDirkelz]

No description provided.

[MrDirkelz]

Review: 1722-api-add-dos-protection-to-the-fts-endpoint
The branch does not compile. npx tsc --noEmit in api/ fails with 4 errors. The PR renamed search → searchWithStats (new return type FtsSearchWithStats = {results, stats}) but only updated 5 of the 8 return statements, and deleted a constant that's still referenced. CI's api typecheck will fail, and every un-fixed path crashes the controller at runtime.

Findings (most severe first)

1. Aux path (User/Redirect search) returns bare array → controller crash — ftsSearch.service.ts:200
   searchWithStats does return this.searchAux(...), but searchAux returns FtsSearchResultDto[], not FtsSearchWithStats. Runtime: any types: [User] / types: [Redirect] search returns an array; the controller's const { results, stats } = await …searchWithStats(…) yields stats = undefined, then isExpensiveFtsSearch(undefined) throws TypeError: Cannot read properties of undefined. Compile error TS2739.

2. Strict path (matchAllWords / sort) returns bare array → controller crash — ftsSearch.service.ts:389 and :395
   Both early returns in the strict block still return [] / the mapped array instead of {results, stats}. Same crash as #1 for every strict/sorted FTS query. Compile errors TS2739.

3. FTS_MIN_TRIGRAMS referenced after deletion — ftsSearch.service.ts:533
   The diff deleted const FTS_MIN_TRIGRAMS = 3 but searchAux's budget loop still uses it. Compile error TS2304 (Cannot find name 'FTS_MIN_TRIGRAMS') — the aux corpus never got the strict-budget change, so the removal broke it. Decide: keep the floor in aux (restore a local const) or apply the same strict cap there.

4. The DoS strike mechanism is effectively dead — ftsSearch.controller.ts:96-100 / ftsSearch.service.ts:285
   isExpensiveFtsSearch fires when estimatedCandidateRows >= budget OR candidateRows >= budget. But the new strict budget loop breaks before a trigram pushes the sum over budget, so estimatedCandidateRows is always ≤ budget (only == in the exact-fit case) and candidateRows (actual rows for kept trigrams) tracks it — so neither disjunct realistically reaches >= budget. Net: expensive-query logging and recordStrike almost never trigger. The old FTS_MIN_TRIGRAMS floor was the thing that let candidate rows exceed budget (the case worth striking). Worth confirming this is intended — right now the rate limiter ships but has nothing to bite on. (PLAUSIBLE — design/efficacy, not a crash.)

Test gap
The new specs pass only because the controller spec mocks searchWithStats, and the service spec exercises only the content BM25 path — neither covers the aux path, the strict path, or the real return shape. That's exactly why three compile-breaking return statements slipped through green tests.

[ivanslabbert]

Review summary — 🟡 Approve with nits

Effective, correctly mirrors the /query DoS pattern, but ships behind a dark-by-default limiter and carries one legit-search behavior regression worth calling out.

• Fulfills the ticket. Adds the three layers /query has: input caps (ftsSearch.controller.ts validateFtsSearchCaps — queryString ≤256, limit ≤50, offset+limit ≤500, maxTrigramDocPercent ≤50), a hard cost bound (candidate-row budget), and the reused strike limiter with 429 + Retry-After. Wiring correct (app.module.ts:61,70).
• Real cost bound is the strict budget, not the limiter. ftsSearch.service.ts removes the FTS_MIN_TRIGRAMS=3 floor so kept trigrams' summed df can never exceed FTS_CANDIDATE_ROW_BUDGET=3000 → candidate fetch is hard-capped regardless of config. This is the actual DoS guard and holds even with the limiter disabled.
• Legit-search regression (please confirm intended). Dropping the min-trigram floor means a query whose rarest usable trigram alone has df > 3000 (yet ≤50% of corpus, so it passes the over-common filter) now returns zero results instead of matches (see new test "does not fetch candidates when the rarest trigram exceeds the row budget"). Bounds cost but silently drops some valid queries — worth acknowledging.
• Limiter ships dark and is shared with /query. QueryRateLimiterService is a singleton reading query.rateLimit (default enabled:false, configuration.ts:127), so the 429 gate is a no-op until an operator opts in. Note strikes cross-pollinate: an expensive /fts strike counts against the same per-identity bucket as /query, and there's no FTS-specific config. Acceptable but undocumented.
• isExpensiveFtsSearch fires only at budget saturation. Because the budget is now a strict cap, a strike is recorded exactly for budget-saturating queries — a reasonable signal; logging correctly omits queryString (asserted in spec).
• Minor: no size cap on types/languages/tags arrays (bounded Set-membership work, low risk); bm25k1/bm25b overrides remain unvalidated (scoring-only, no cost impact). Tests cover 429, each oversized-input rejection, the expensive-strike path, and the stats contract.

_{🤖 AI-assisted review summary (Claude Code). Read-only analysis of the PR diff — not a substitute for human review.}