feat(search): FTS5 lexical search + security hardening for /api/search - #1058
Open
mmcintosh wants to merge 9 commits into
Open
feat(search): FTS5 lexical search + security hardening for /api/search#1058mmcintosh wants to merge 9 commits into
mmcintosh wants to merge 9 commits into
Conversation
Port infowall FTS5 lexical search into sonicjs-org on the document model. - 0003_documents_fts.sql: documents_fts virtual table (10-col layout, bm25 parity) - DocumentProjection: FTS upsert/delete folded into the document write batch (all write paths incl. updateInPlace), reindexType for backfill/admin reindex - ai-search-plugin: keyword mode now backed by FTS5 engine (legacy content LIKE scan removed), KV settings + search cache, degraded flag - searchable-text harvester, fts5-sanitize (verbatim port), d1-retry - schema: kind:'fulltext' + weight on queryable fields - tests: 6 new sqlite/unit suites + e2e 82-fts5-search Not yet pushed/PR'd to upstream (testing on Infowall infra first).
…r unpublish/delete E2E spec 82 caught a correctness/security leak the unit tests asserted as expected behavior: the KV result cache (cacheTtlSeconds=60 by default) was only invalidated on settings change, never on document writes, so an unpublished/deleted doc stayed in public search results for the TTL. - DEFAULT_FTS_SETTINGS.cacheTtlSeconds = 0 (cache opt-in until write-path invalidation exists) - fts-search-cache.ts doc: proper re-enable = per-tenant search-index version folded into resultCacheKey (needs CACHE_KV threaded into core) - cache unit test updated to opt-in explicitly - plan v2: findings recorded; migrations-bundle timestamp regen
blog_post's seeded document type now declares { name: 'content', kind: 'fulltext' },
so the projection renders the post body (lexical editor stores HTML) into
documents_fts.body — body text becomes keyword-searchable and snippet() returns
highlighted prose instead of coming back empty.
- ai-search manifest 1.1.0: description now reflects the two-engine reality
(always-on FTS5 keyword + optional AI); registry regenerated from manifests
- real-SQLite test: seeded blog_post indexes HTML content, tags stripped
- E2E 82: body-only marker found with <mark> snippet; describe tagged @api
so CI's tag-grep actually selects the spec
Note: bootstrap is skipped when the version-keyed KV marker
(_sonicjs_bootstrap_v<version>) exists, so an already-booted environment must
clear that KV key (or bump the version) for the seed UPDATE to apply.
Existing posts index on next write; reindexType() backfills in bulk.
…ng docs main has since taken 0003 (session_org) and 0004 (forms) for unrelated features — this branch's 0003_documents_fts.sql collided. Renumbered to the next free slot (0005), synced both migrations/ copies, updated the two test harnesses that reference the filename by string, and neutralized one internal-project cross-reference in the migration's header comment. Also dropped the SEARCH-FTS5-PLUGIN-*.md planning/handoff docs — internal working notes from building this, not meant to ship upstream.
… allowlist, XSS-safe highlighting
Three real gaps in the FTS5 keyword-search path, found in review:
- filters.status from the request body could relax the published-only gate
(e.g. filters:{status:['draft']}) on the unauthenticated /api/search
endpoint. publishedOnly is now hardcoded true, server-authoritative, never
derived from client input.
- No restriction on which document types were searchable — internal-only
types (api_key, security_event, analytics_event, user_profile, rbac_*,
site_settings, …) were reachable by an anonymous caller who knew/guessed
the type id. Added publicReadableTypeIds(), intersected against any
requested type scope; empty scope returns no results rather than falling
back to "all types" (the engine's default for an empty/omitted filter).
- FTS5's highlight()/snippet() wrap matches in literal <mark> tags around
RAW, unescaped title/body text — a malicious <img onerror=...> in a
document title would render live in the highlighted result. Fixed with
private-use sentinel characters as the match delimiters: escapeHtml() the
full result first, then swap sentinels for real <mark> tags — so only the
highlight markup we control survives.
Also: a documents_fts self-heal (an existing install that upgrades core past
this migration but hasn't run `wrangler d1 migrations apply` yet would
otherwise brick every document write, not just search — every DocumentsService
write folds an unconditional FTS delete into its batch), a DoS cap on the
public endpoint's result limit, and two deindex fixes for admin-content.ts's
delete paths that bypass DocumentProjection's normal FTS cleanup (a raw
deleted_at UPDATE otherwise leaves a deleted doc searchable indefinitely).
Known, intentional tradeoff: the admin search UI's Draft/Archived status
filter will now silently return nothing (server-authoritative published-only
applies there too) — authenticated non-published search is a separate future
endpoint, not this one. Flagged as follow-up debt.
The previous commit fixed admin-content.ts's two delete paths (raw deleted_at UPDATE, bypassing DocumentProjection's normal FTS cleanup) but missed two more instances of the exact same pattern: DELETE /api/:collection/:id (routes/api.ts) and DELETE /api/content/:id (routes/api-content-crud.ts). Both are real, generic, authenticated content-delete endpoints — after the previous commit landed, deleting through either of these two still left the document fully discoverable (title + body snippet) via the public, unauthenticated /api/search indefinitely. Same fix as the two already-patched sites: an explicit `DELETE FROM documents_fts` follow-up alongside the raw UPDATE. DocumentsService.softDelete() already handles this correctly in one batch, but it isn't a drop-in replacement here — it takes a single document `id`, while both of these routes soft-delete an entire root (WHERE root_id = ?) — so this stays consistent with the pattern the sibling fix already established rather than changing these routes' semantics. Real-SQLite regression tests added to both routes' existing integration suites (assert a documents_fts row exists post-publish, then is gone post-delete). The two matching E2E cases for these routes are in the previous commit's spec 105, alongside the rest of the search-hardening E2E coverage.
Two gaps found in a fresh review of the two prior commits, both real: - MediaDocumentService.softDeleteRoot() has the same raw deleted_at UPDATE pattern the last two commits fixed on the content-delete routes, missed because it lives in a service file, not a route file. media_asset carries baseGrants.public:['read'], so it's in the new publicReadableTypeIds() allowlist — a deleted file's filename stayed permanently, publicly searchable via unauthenticated /api/search. Reachable from three live authenticated routes: DELETE /api/media/:id, bulk media delete, and the admin media library delete action. Same fix as the other four sites: an explicit FTS deindex alongside the raw UPDATE. - Fts5Engine escaped title/snippet before this pass but never slug, despite it going through the exact same JSON response. Not currently exploitable (the only known consumer never renders it via innerHTML, and slug has no charset restriction at the schema layer to rule out attacker content) — but inconsistent with the escaping this same commit series added everywhere else, so closed the gap rather than leave it as a landmine. Regression tests added for both (media-documents.test.ts asserts a documents_fts row exists post-upload and is gone post-delete; ai-search-keyword-fts.sqlite.test.ts asserts a malicious slug is escaped in search results).
…eploy The public search API is core-promoted (mounted in app.ts before the /api/:collection catch-all), not plugin-gated, so a disableAll:true bare-core deploy still serves it. Two guards: - ai-search-disableall.integration.test.ts (runs in CI): with NO plugins mounted, an anonymous POST /api/search returns published results; a self-check reverses the mount order and asserts the historical 401 shadow returns, so the test is provably sensitive to the regression it guards. Uses the mock-middleware route harness (no createSonicJSApp / better-auth) to stay inside the CI-runnable set. - mount-integration.test.ts: route-table assertion that /api/search is registered as a core route under disableAll (quarantined suite; runs when the better-auth CI gap lifts).
lane711
added a commit
that referenced
this pull request
Sep 8, 2026
* feat(auth): two-factor authentication (TOTP + backup codes)
Adds a two-factor-auth core plugin: TOTP enrolment with a scannable QR, single-use
backup codes, a per-account lockout, second-factor enforcement on the passwordless
sign-in paths, and an administrative reset for users who lose their authenticator.
Better Auth's twoFactor() is composed unconditionally, so enrolment state alone
decides whether a user is challenged. The plugin owns the enrolment surface
(/admin/two-factor); core owns the login challenge (/auth/two-factor) so that
turning plugins off stops new enrolments without locking out users who already
have a second factor.
What is enforced where:
- Challenge on password sign-in — Better Auth, via twoFactorRedirect.
- Challenge on magic-link / email-OTP — guardPasswordlessSecondFactor in the
/auth/* catch-all, since BA challenges only the password paths and would
otherwise hand an enrolled account a session with no code.
- Session upgrade after verification — POST /auth/two-factor/complete mints
the same JWT a password login mints, so a 2FA session is never weaker than
a password one. It derives the credential from the session's own user and
never from request input.
- Policy (issuer, lockout, backup-code count) is clamped on read, so no
stored value or failed load can disable the lockout or drop below 5 codes.
The QR is rendered server-side (qrcode-svg, already a core dep, Workers-safe)
because these pages have no client bundler and a CDN script on a page handling
TOTP secrets is not a trade worth making. Its size is COMPUTED from the symbol's
module count rather than fixed: scannability is CSS pixels per module, and the
issuer is operator-configurable to 64 chars and appears in the URI twice, so the
symbol ranges 49-69 modules. A fixed container delivered 4.14 px/module by
default and 3.06 with a long issuer — well-formed, and too small for a phone to
decode. It is now a constant 6.00 px/module at any issuer length, with the size
stamped on the <svg> rather than left to CSS.
Route input is pinned to ^otpauth://totp/ so the endpoint cannot render
arbitrary text as a QR from our origin — a phishing primitive, since a QR is
unreadable to the human deciding whether to trust it.
── Administrative reset (break-glass) ──
Without one, the complete list of ways back into an account with a lost
authenticator was an unused backup code or `wrangler d1 execute --remote`.
Everything an operator would reach for first is closed by design: password reset
deliberately mints no session, magic link and email OTP are refused for enrolled
users, and self-disable needs the session you cannot get. So a sole admin who
enrolled and lost phone plus codes locked the entire organisation out of the
portal until someone with Cloudflare credentials intervened.
POST /admin/two-factor-reset clears a user's second factor, and migration 0004
adds auth_user.two_factor_required so the reset can demand re-enrolment instead
of silently downgrading the account to password-only. The flag lives on auth_user
rather than auth_two_factor because the reset DELETEs that row — a flag stored
there would be destroyed by the action that sets it. required and enabled are
independent: required && !enrolled is what redirects to /admin/two-factor;
required && enrolled means enrolled and not permitted to turn it off.
Controls on the action, and why these:
- Admin role, matching how every other user-management route in admin-users.ts
is gated, so it is governed by role assignments operators already reason about.
- The target's email must be typed back. Not a second factor — it defends
against the realistic failure, which is resetting the wrong row from a list
of similar-looking ones. Password confirmation was considered and rejected:
Better Auth hashes with its own scrypt (salt:key, 161 chars) that
AuthManager.verifyPassword cannot read, and reimplementing it wrong would
break the break-glass itself.
- Every use writes a two_factor_reset security event naming both the actor and
the subject. Best-effort: an unavailable audit sink must not be the reason a
lockout recovery fails.
- A user under the requirement may not turn the factor back off. The redirect
middleware cannot cover that — POST /auth/two-factor/disable is not under
/admin/*, and /admin/two-factor (which hosts the disable form) has to stay
exempt or enrolment would be impossible — so a second guard sits in the
/auth/* catch-all beside guardPasswordlessSecondFactor, ahead of
auth.handler, and the page replaces the form with an explanation. Without it
a user told to enrol could enrol and immediately switch it off, leaving the
account password-only, BA no longer challenging, the passwordless paths
reopened and /api/* ungated, with no audit event to tell the admin.
Deliberately keyed to the REQUIREMENT and not to being enrolled, so ordinary
self-service 2FA management is untouched.
Mounted on its own prefix and NOT behind the plugin's deactivate→404 gate, for
the same reason the login challenge is not: deactivating the plugin stops nothing
about verification, so a recovery path that vanished with the surface would
disappear exactly when the lockout it fixes is still happening. The enforcement
middleware fails OPEN on any DB error and stands aside entirely when the plugin
is off, since /admin/two-factor 404s then and enforcing would loop the user to a
page that cannot exist.
The reset does not revoke the target's sessions — it is a recovery action, not a
containment one; deactivating the account is what containment is for.
Also fixes the plugin sidebar rendering icon NAMES as text ("lock-closed" beside
Two-Factor Auth, "book-open" for API Reference, "variable" for Global
Variables): plugin-menu projected `resolveIcon(icon) || icon`, leaking unmapped
names into markup the layout interpolates.
Verified in a browser: enrolment page and QR render, the displayed QR was proved
in-page to encode the same URI as the manual-entry secret, and the enrolment flow
was confirmed end-to-end with a physical phone scan.
Verified against a running wrangler dev server on real D1 and real Better Auth,
not only under the test harness: 0004 applied to an existing install without
locking out its enrolled user; the reset drove end-to-end (panel render, typed-
email refusal leaving the DB untouched, reset, target then signing in with
password alone, forced re-enrolment redirect for browsers and 403 for JSON, the
break-glass route correctly NOT exempt, audit event naming actor and subject);
the disable guard refused a mandated account while leaving ordinary self-service
disable working; and the 0004 self-heal restored the column after it was dropped
out from under a running app.
Known and deliberately out of scope: CSRF validation is inert across the whole
cookie-authenticated admin surface, because csrfProtection exempts requests with
no auth_token cookie and sign-in mints a Better Auth session cookie instead.
Pre-existing and repo-wide; the fix is the "OD2 Option B" csrf.ts port, which
changes global request handling and needs its own audit. Comments that claimed
CSRF was enforced on these routes have been corrected.
Tests: real Better Auth over real SQLite for the round trip and the lockout, and
41 new cases for the reset — SQL effects, the role and typed-email gates, the
enforcement middleware's exemptions, JSON branch and fail-open paths, the disable
guard, and the four states the enrolment page renders.
Notably the middleware must NOT exempt /admin/two-factor-reset, which a naive
prefix check would hand to the very users who owe an enrolment. E2E in
tests/e2e/101-two-factor-auth.spec.ts and 102-two-factor-admin-reset.spec.ts
(enrolling cases use throwaway accounts so they cannot take out the shared admin).
two-factor-lockout-engages.test.ts gets an explicit 30s timeout: it makes ~10
sequential BA round trips through real scrypt and landed at ~5.1s against the 5s
default, so any added concurrency turned it into a timeout that looked exactly
like a broken lockout.
npm test: 25 failed / 1886 passed — the same 25 pre-existing beta.25 failures as
on the parent commit, verified by diffing failing-test names against a clean
baseline run. Zero regressions.
* chore(auth): renumber 2FA migrations 0003/0004→0006/0007, E2E specs 101/102→106/107
main has since taken 0003 (session_org) and 0004 (forms) for unrelated features
— this branch's 0003_two_factor_lockout.sql/0004_two_factor_required.sql
collided. Renumbered to 0006/0007, deliberately skipping 0005 (reserved for
the FTS5 search PR, also in flight and further along in review). Synced both
migrations/ copies, updated the self-heal comments and test-harness filename
references in migrations.ts/d1-sqlite.ts/two-factor-adapter-create.test.ts,
regenerated the bundle.
E2E specs renumbered 101/102→106/107 to stay clear of both currently-open
PRs (#1057 claims 104, #1058 claims 105) — real main's actual highest spec
is 100, not 103 as an earlier broad search suggested.
* chore(auth): fix remaining stale migration 0003/0004 comment references
The renumbering commit updated services/migrations.ts but missed 5 more
files with the same stale references, found by review — cosmetic only,
the actual filenames/logic were already correctly 0006/0007 everywhere.
* fix(auth): close second-factor gaps found in review of #1059
Eight issues from the review of the two-factor PR, plus two nits.
1. OAuth sign-in bypassed the second factor. The already-linked branch in
oauth-providers mints a session JWT directly, so Better Auth's TOTP
challenge never runs — anyone holding the provider credential skipped a
factor the account owner had deliberately enrolled in. Both the
already-linked and the auto-link branches now refuse an account with a
verified second factor and send it to password sign-in. This cannot
strand anyone: BA requires password verification to reach
/auth/two-factor/enable, so an enrolled account always has a password
login.
2. Forced re-enrolment was only enforced on /admin/*. The same session
cookie drives /api/*, so a user told to enrol kept full read/write access
through the JSON API while the admin who set the requirement believed the
account was blocked. enforceTwoFactorEnrolment is now mounted on /api/*
too, registered ahead of the API route mounts so it actually covers them.
3. API keys are exempt from that gate. Keys are separately revocable and the
only place to mint a replacement (/admin/api-keys) is behind the same
gate, so gating them would take running integrations offline with no way
back. api-key-auth now sets an explicit `authMethod: 'api-key'` marker
and the middleware returns early on it.
4. The gate hit D1 on every request. It now caches the "owes nothing"
verdict for 60s. Only the negative is cached, and never when the read
failed — caching a fail-open default would turn a one-request D1 blip
into a minute of unenforced policy. resetUserTwoFactor (the only writer
of two_factor_required) invalidates the entry.
5. The middleware answered htmx requests with a 403 JSON body. htmx sends
`Accept: */*`, so the HX-Request branch has to come first; it now
redirects via HX-Redirect.
6. The issuer went into the otpauth:// URI on a deny-list. Replaced with a
Unicode-aware allow-list — the first structural character missed forges a
different account entry in the victim's authenticator, and the deny-list
also mangled legitimate names like "Société Générale (EU) Ltd."
7. GET /users/:id/edit selected two_factor_required unconditionally. On an
install that has not run migration 0007 the unknown column rejects the
whole statement, 500ing the page. Falls back through
ensureTwoFactorRequiredColumn self-heal, then a select without the column.
8. The disable guard compared a raw pathname, so POST
/auth/two-factor/disable/ (trailing slash) reached the same BA endpoint
unguarded and deleted an enrolment an admin had mandated. Paths are now
normalized, in both the passwordless guard and the disable guard.
Nits: dropped a dead .catch() in plugin-service (loadTwoFactorPolicy
already catches internally), and corrected two stale comments that
described mount points and MFA assumptions that no longer hold.
The reset button on the user edit page interpolated the user id into an
inline onclick. escapeHtml renders ' as ', which the HTML parser
decodes back to a bare quote before the JS parser sees the attribute — so
the escaping did not protect the handler. Passed via data-user-id instead.
Tests: real-SQLite coverage for the OAuth gate (6), the enrolment cache and
the middleware's htmx / /api/* / API-key branches (8), the issuer allow-list
(3), trailing-slash normalization (1), the edit-page self-heal (1), and the
reset panel's data attribute (4). E2E spec 108 covers API enforcement and
the trailing-slash disable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(e2e): repair 21 pre-existing E2E failures blocking PR #1059
Five distinct root causes, all pre-existing on the branch:
1. Hardcoded Origin (02c-otp-login, 02d-magic-link-auth)
Better Auth answers a POST from an untrusted Origin with 403
INVALID_ORIGIN and trusts the base URL the request arrived on. Both
specs pinned `Origin: http://localhost:9704`, so every request failed
against the CI preview deploy. Use TEST_ORIGIN, which tracks BASE_URL.
2. user-profiles Settings tab was dead code (106-two-factor-auth,
80-user-profile-code-config)
renderPluginSettingsPage gates the Settings tab on the plugin having
user-editable setting keys. user-profiles stores none — its panel is a
static description of the code-defined field model — so the tab, and
the renderUserProfilesSettingsContent behind it, could never render.
Opt such plugins in explicitly via STATIC_SETTINGS_PANEL_PLUGINS.
3. Ambiguous two-factor link (106-two-factor-auth)
The sidebar ships desktop and mobile copies of the /admin/two-factor
link, so an href-based locator matched three elements. Give the profile
security panel's own anchor an id and target that.
4. Role display names live in inputs (85-admin-panel-roles-naming)
They are rendered as <input value="..."> in the bulk-rename form, so
innerText never sees them. Read them out of `value` instead.
5. HTMX form never navigates (68-user-profile-document, 38-user-profile-edit)
The user form submits via hx-put, so waitForURL resolved immediately
and raced the save. Wait on the PUT response instead.
Also rewrites 38-user-profile-edit and 80-user-profile-code-config, whose
premises no longer matched the code: profile fields are declared in
my-sonicjs-app/src/user-profile.model.ts via defineUserProfile(), so the
"unconfigured" assertions and the website-URL-validation test (the field
is declared as plain text) described behavior that does not exist.
Adds unit coverage for the Settings-tab gate so the regression cannot
return silently.
Verified: 53 E2E tests pass locally across the seven touched specs;
packages/core type-check clean; full unit suite green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(graphql,e2e): unblock the two remaining CI failures
GraphQL document mutations were denied for every caller. requireWritePermission
called repo.isAllowed() with an empty typeSettings object, but base grants live
on the document type — with none supplied, isAllowed sees only per-document ACL
rows and denies update/publish/unpublish/delete even for an admin. Load the type
via DocumentTypeRegistry and pass its settings.
Two E2E corrections:
- 100-graphql: the type id is `blog_post`, not `blog_posts`. createDocument on a
type that does not exist surfaced as INTERNAL_SERVER_ERROR.
- 80-user-profile-code-config: a fresh deploy has no `plugins` row for
user-profiles, and #1075 made uninstalled plugins render the Info tab only, so
the guidance panel behind the Settings tab was absent. Install the plugin first
(no-op when already installed).
Verified: 100-graphql 5/5 and 80-user-profile-code-config 3/3 pass locally;
packages/core type-check clean; unit suite 1930 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Lane Campbell <ldc0618@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Wires FTS5 (BM25-ranked lexical search, snippet/highlight, porter-stemmed tokenization) in as the
ai-search-plugin'skeywordmode backend — replacing today'scontent/collections-basedLIKE '%term%'scan, which has no ranking and currently returns nothing on any install matching this repo's actual migrations (those legacy tables were dropped during the document-model migration; the plugin was never updated to match). Indexed at write-time via the existingDocumentProjectionseam.Also closes three real gaps in
POST /api/search(unauthenticated, no CSRF gate) found while building this, plus five instances of the same underlying bug (a rawdeleted_atUPDATE that bypassesDocumentProjection's FTS cleanup, leaving deleted content/media searchable forever) across four route files and one service.Changes
documents_ftsFTS5 virtual table (migration0005) +Fts5Engine(BM25 ranking,snippet()/highlight()), wired askeywordmode's backend./api/searchwas shadowed and plugin-gated: the plugin mountedPOST /api/searchafter the genericapp.route('/api', apiRoutes)catch-all, so requests were silently captured by/api/:collection(POST /api/search→401) and/api/:collection/:id(GET /api/search/analytics→404) — the plugin's own handlers never ran. Confirmed on a livewrangler dev(not just a routing probe):POST /api/searchreturned{"error":"Authentication required"}, byte-identical toPOST /api/nonexistentcollection. This PR promotes the search API out of the plugin and into coreapp.ts, mounted before the catch-all. Consequences: (a) search actually works; (b) the endpoint can no longer be loaded un-hardened, because the hardened handler is the mount; (c) a plugins-off (disableAll: true, "bare core") deploy still serves a working, hardened/api/searchrather than the endpoint disappearing. This supersedes fix(search): close SQLi, draft leak, and DoS in public /api/search #1064, whose standalone plugin-level hardening protected code that had no reachable HTTP path.kind:'fulltext'queryable field.filters.statusbypass: the request body'sfilters.statuscould relax the published-only gate on the unauthenticated search endpoint (e.g.filters:{status:['draft']}).publishedOnlyis now hardcodedtrue, server-authoritative, never derived from client input.api_key,security_event,user_profile,rbac_*, …) were reachable by an anonymous caller who knew/guessed the type id. Added apublicReadableTypeIds()allowlist (settings.baseGrants.publicincludes'read'); an empty resulting scope returns no results rather than falling back to "all types."highlight()/snippet()wrap matches in literal tags around raw, unescaped title/body/slug text. Fixed with private-use sentinel characters as match delimiters — escape the full result first, then swap sentinels for real<mark>tags, so only markup we control survives.admin-content.ts(bulk-action +DELETE /:id),routes/api.ts(DELETE /:collection/:id),routes/api-content-crud.ts(DELETE /:id), andMediaDocumentService.softDeleteRoot()all do a rawdeleted_atUPDATE that bypassesDocumentProjection's normal FTS deindex — deleted content/media stayed searchable indefinitely. All five now explicitly deindex.documents_ftsself-heal inMigrationService— an install that's past this migration in code but hasn't runwrangler d1 migrations applyyet would otherwise brick every document write (everyDocumentsServicewrite folds an FTS delete into its batch), not just search.MAX_LIMIT=100cap on the public endpoint's result limit (DoS guard).cache defaultActivemanifest fix, both independent, pre-existing bugs found while building this.Known, intentional tradeoff: the admin search UI's Draft/Archived status filter will now silently return nothing when routed through keyword mode (server-authoritative published-only applies there too) — authenticated non-published search is a separate future endpoint, not this one.
Out of scope, flagged not fixed: the sibling
mode:'ai'search path (Vectorize/Workers AI) has its own pre-existing published-only gap and an unimplementedremoveContentFromIndex()— both predate this PR (from the original AI-search plugin) and don't fire without Vectorize bindings configured (not in the default install). Worth a follow-up issue.Testing
Reviewed for security/reality/blast-radius across three independent passes before opening.
Unit Tests
ai-search-disableall.integration.test.ts— with every plugin disabled (disableAll) an anonymousPOST /api/searchstill returns published results; a self-check reverses the core-mount order and asserts the historical401shadow returns, proving the test is sensitive to the exact regression it guards. (mount-integration.test.tsalso asserts, via app-factory route-table introspection, that/api/searchis registered as a core route underdisableAll— that suite is in the better-auth CI-quarantine list, so it runs locally / when the quarantine lifts.)tsc --noEmitclean.E2E Tests
105-fts5-search.spec.ts, renumbered from 82 to avoid colliding with an unrelated existing spec onmain): full create→publish→search flow, draft exclusion, unpublish/delete deindex across all three content-delete routes, and the XSS-escaping regression.Screenshots/Videos
No admin UI changes, but here's the live
POST /api/searchbehavior on a local run of this branch:BM25-ranked keyword search,
<mark>-highlighted title:filters.statusbypass attempt from an unauthenticated caller — ignored server-side, no draft content leaked:Checklist