Skip to content

Sanitize search results to prevent XSS from FTS5 output#420

Merged
flavorjones merged 1 commit into
mainfrom
sanitize-search-results
Apr 15, 2026
Merged

Sanitize search results to prevent XSS from FTS5 output#420
flavorjones merged 1 commit into
mainfrom
sanitize-search-results

Conversation

@flavorjones

@flavorjones flavorjones commented Apr 15, 2026

Copy link
Copy Markdown
Member

Summary

Ensure FTS5 search result output is safe to render, allowing only
<mark> tags through. This protects against content that was indexed
before this change.

Also strip tags from content before indexing to keep the FTS table
clean. This is a hygiene measure, not a security boundary.

ref: https://3.basecamp.com/2914079/buckets/1666/card_tables/cards/9713169434

Ensure FTS5 search result output is safe to render, allowing only
`<mark>` tags through. This protects against content that was indexed
before this change.

Also strip tags from content before indexing to keep the FTS table
clean. This is a hygiene measure, not a security boundary.
Copilot AI review requested due to automatic review settings April 15, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the book search UI against XSS by sanitizing FTS5 highlight()/snippet() output at render time (allowing only <mark>), and also strips HTML before indexing to keep the FTS table clean (including protection against previously-indexed “poisoned” content).

Changes:

  • Sanitize rendered search result snippets/titles to only allow <mark> tags.
  • Sanitize/strip tags from titles and searchable content before inserting/updating the FTS5 index.
  • Add model, helper, and controller tests covering both new indexing behavior and display-time sanitization (including pre-existing poisoned index rows).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/helpers/searches_helper.rb Adds sanitize_search_result to safely render FTS5 output while preserving <mark>.
app/views/books/searches/_result.html.erb Replaces html_safe rendering with sanitize_search_result(...) calls.
app/models/leaf/searchable.rb Sanitizes title/content before writing to leaf_search_index.
app/models/page.rb Ensures searchable_content re-escapes plain text so entities remain safe for indexing.
test/helpers/searches_helper_test.rb Adds unit coverage for sanitize_search_result behavior.
test/models/leaf/searchable_test.rb Verifies indexing sanitization for section/page titles and bodies.
test/models/page_test.rb Verifies searchable_content re-encodes entities and remains html_safe?.
test/controllers/books/searches_controller_test.rb End-to-end assertions that rendered results strip dangerous tags and encode entities, including poisoned-index cases.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@flavorjones flavorjones merged commit 182a76d into main Apr 15, 2026
11 checks passed
@flavorjones flavorjones deleted the sanitize-search-results branch April 15, 2026 14:50
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.

2 participants