Enhance embed URL handling and validation system - #7
Conversation
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
This PR implements a comprehensive embedding and feed import system for Discourse. It enables external websites to embed Discourse discussions via iframes with dynamic height adjustment through postMessage API. The feature includes RSS/ATOM feed polling to automatically import content as forum topics, with support for raw HTML rendering and content change detection via SHA1 hashing. New models, controllers, background jobs, and views support the embedding workflow, including topic retrieval with rate limiting and host validation. Configuration is managed through four new site settings for embeddable hosts, feed polling, and user attribution. The implementation includes full test coverage and internationalization support.
Changes
| File(s) | Summary |
|---|---|
GemfileGemfile_rails4.lock |
Added ruby-readability (v0.5.7) and simple-rss (v1.3.1) gem dependencies with their transitive dependencies for feed importing and content extraction functionality. |
app/controllers/embed_controller.rb |
Introduced new controller to handle embedded content with best action that retrieves/enqueues topic fetching, validates embeddable host against referer, sets X-Frame-Options to ALLOWALL, and caches responses for 1 minute. |
app/models/topic_embed.rb |
Added ActiveRecord model managing embedded content with associations to Topic/Post, SHA1-based change detection, and methods for importing/updating topics from external URLs with content parsing and URL absolutization. |
app/models/post.rb |
Added cook_methods enum (:regular, :raw_html) and modified cook method to conditionally skip rendering pipeline for raw HTML posts imported from external sources. |
app/jobs/regular/retrieve_topic.rb |
Created background job for asynchronous topic retrieval from embedded sites with parameter validation and staff user throttling bypass. |
app/jobs/scheduled/poll_feed.rb |
Implemented hourly scheduled job to fetch RSS/ATOM feeds and import items as topics using SimpleRSS parsing with SHA1-based feed key generation and content sanitization. |
lib/topic_retriever.rb |
Introduced class encapsulating topic retrieval logic with host validation, Redis-based rate limiting (60-second throttle), and multi-stage retrieval strategy (existing embeds, RSS polling, HTTP fetch). |
lib/post_creator.rblib/post_revisor.rb |
Enhanced to support custom cook_method assignment during post creation and added skip_validations option to bypass validation during post save operations. |
app/assets/javascripts/embed.jsapp/views/layouts/embed.html.erb |
Implemented bidirectional postMessage communication for dynamic iframe height adjustment, with embed.js creating iframe and layout sending height updates to parent frame. |
app/assets/stylesheets/embed.css.scss |
Added stylesheet for embedded content with author section layout, post date styling, header/footer borders, and floating logo component. |
app/views/embed/best.html.erbapp/views/embed/loading.html.erb |
Created view templates for displaying best posts with author avatars and content, and loading state with 30-second auto-refresh mechanism. |
db/migrate/20131217174004_create_topic_embeds.rb |
Created topic_embeds table with foreign keys to topics/posts, unique indexed embed_url, and content_sha1 field for change tracking. |
db/migrate/20131219203905_add_cook_method_to_posts.rb |
Added cook_method integer column to posts table with default value of 1 and NOT NULL constraint. |
config/site_settings.ymlconfig/locales/server.en.ymlconfig/locales/client.en.yml |
Added 'embedding' configuration section with four settings (embeddable_host, feed_polling_enabled, feed_polling_url, embed_by_username) and corresponding i18n strings. |
config/routes.rb |
Added GET route /embed/best mapping to EmbedController#best action. |
lib/tasks/disqus.thor |
Refactored to use TopicEmbed.import_remote instead of direct PostCreator calls, removing category assignment logic. |
db/migrate/20131210181901_migrate_word_counts.rbdb/migrate/20131223171005_create_top_topics.rb |
Whitespace cleanup and added force: true option to ensure table recreation on migration re-runs. |
spec/components/topic_retriever_spec.rbspec/controllers/embed_controller_spec.rbspec/jobs/poll_feed_spec.rbspec/models/topic_embed_spec.rb |
Added comprehensive test coverage for TopicRetriever validation/rate-limiting, EmbedController behavior, PollFeed preconditions, and TopicEmbed import functionality. |
Sequence Diagram
This diagram shows the interactions between components:
sequenceDiagram
actor User
participant Controller
participant FeedImporter
participant SimpleRSS as SimpleRSS Gem
participant Readability as Ruby-Readability Gem
participant Database
User->>Controller: Request to import feed URL
Controller->>FeedImporter: import_feed(url)
activate FeedImporter
FeedImporter->>SimpleRSS: parse(feed_url)
activate SimpleRSS
SimpleRSS-->>FeedImporter: feed items (title, link, content)
deactivate SimpleRSS
loop For each feed item
FeedImporter->>Readability: parse(item.link)
activate Readability
Readability-->>FeedImporter: cleaned content & metadata
deactivate Readability
FeedImporter->>Database: save_post(content, metadata)
Database-->>FeedImporter: confirmation
end
FeedImporter-->>Controller: import complete
deactivate FeedImporter
Controller-->>User: Display imported posts
🔗 Cross-Repository Impact Analysis
Enable automatic detection of breaking changes across your dependent repositories. → Set up now
Learn more about Cross-Repository Analysis
What It Does
- Automatically identifies repositories that depend on this code
- Analyzes potential breaking changes across your entire codebase
- Provides risk assessment before merging to prevent cross-repo issues
How to Enable
- Visit Settings → Code Management
- Configure repository dependencies
- Future PRs will automatically include cross-repo impact analysis!
Benefits
- 🛡️ Prevent breaking changes across repositories
- 🔍 Catch integration issues before they reach production
- 📊 Better visibility into your multi-repo architecture
Install the extension
Note for Windsurf
Please change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery
Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items
Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts below
Emoji Descriptions:
⚠️ Potential Issue - May require further investigation.- 🔒 Security Vulnerability - Fix to ensure system safety.
- 💻 Code Improvement - Suggestions to enhance code quality.
- 🔨 Refactor Suggestion - Recommendations for restructuring code.
- ℹ️ Others - General comments and information.
Interact with the Bot:
- Send a message or request using the format:
@entelligenceai + *your message*
Example: @entelligenceai Can you suggest improvements for this code?
- Help the Bot learn by providing feedback on its responses.
@entelligenceai + *feedback*
Example: @entelligenceai Do not comment on `save_auth` function !
Also you can trigger various commands with the bot by doing
@entelligenceai command
The current supported commands are
config- shows the current configretrigger_review- retriggers the review
More commands to be added soon.
| url = i.link | ||
| url = i.id if url.blank? || url !~ /^https?\:\/\// | ||
|
|
||
| content = CGI.unescapeHTML(i.content.scrub) |
There was a problem hiding this comment.
Correctness: Calling i.content.scrub will raise a NoMethodError and abort the job if an RSS item lacks a content tag (returning nil). Use i.content.to_s.scrub or provide a fallback like i.description to ensure the job handles items with missing content.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `app/jobs/scheduled/poll_feed.rb` at the line where `content` is built from `i.content`, make the call nil-safe so RSS items without content don't raise. Change `i.content.scrub` to `i.content.to_s.scrub` (or fallback to `description` if preferred) while preserving existing behavior.
| def self.import_remote(user, url, opts=nil) | ||
| require 'ruby-readability' | ||
|
|
||
| opts = opts || {} | ||
| doc = Readability::Document.new(open(url).read, |
There was a problem hiding this comment.
Correctness: import_remote calls open(url) before any scheme validation, so a non-HTTP URL (e.g., file:// or a local path) can be read from the server. This bypasses the safety check in import and enables SSRF/local file reads. Add the same http/https guard here before open is called.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In app/models/topic_embed.rb lines 44-48, add a guard in import_remote to ensure the URL is http/https before calling open(url). This prevents SSRF/local file reads. Insert `return unless url =~ /^https?:\/\//` immediately after the require statement.
| window.onload = function() { | ||
| if (parent) { | ||
| // Send a post message with our loaded height | ||
| parent.postMessage({type: 'discourse-resize', height: document['body'].offsetHeight}, '<%= request.referer %>'); |
There was a problem hiding this comment.
Correctness: parent.postMessage will throw a SyntaxError DOMException if request.referer is blank (e.g., due to Referrer-Policy), as an empty string is an invalid targetOrigin. Provide a fallback origin or use '*' to prevent the script from crashing when the referrer header is missing.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: app/views/layouts/embed.html.erb (line 11). Problem: request.referer can be blank, producing an empty targetOrigin that throws a DOMException in postMessage. Fix: introduce a targetOrigin variable and only call parent.postMessage when it is non-empty (or use a safe fallback if required).
| if options[:dry_run].blank? | ||
| creator = PostCreator.new(user, title: t[:title], raw: "\[[Permalink](#{t[:link]})\]", created_at: Date.parse(t[:created_at]), category: category_id) | ||
| post = creator.create | ||
|
|
There was a problem hiding this comment.
Correctness: The call to TopicEmbed.import_remote on this line omits the created_at timestamp previously passed to PostCreator. This causes the topic to be created with the current timestamp while subsequent replies use historical dates, breaking the thread timeline. Ensure the topic and its first post use Date.parse(t[:created_at]).
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
File: lib/tasks/disqus.thor. At the `TopicEmbed.import_remote` call (around line 147), ensure the imported topic/post preserves the Disqus thread’s `created_at`. Either pass `created_at: Date.parse(t[:created_at])` if supported, or update the created post/topic timestamps immediately after import. Keep replies’ timestamps intact.
2 participants
Test 4
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/discourse-coderabbit#4
EntelligenceAI PR Summary
This PR implements a complete embedding and RSS/ATOM feed import system for Discourse forums.