Enhance embed URL handling and validation system#7
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| 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.
Iframe resize messages never reach parent
Medium Severity
parent.postMessage uses request.referer as targetOrigin, but that value includes a full URL path. Browsers match targetOrigin by origin, so the resize message is rejected and embed.js never receives height updates for discourse-embed-frame.
| url = i.link | ||
| url = i.id if url.blank? || url !~ /^https?\:\/\// | ||
|
|
||
| content = CGI.unescapeHTML(i.content.scrub) |
There was a problem hiding this comment.
Feed polling crashes on missing content
Medium Severity
poll_feed assumes every item has i.content and calls i.content.scrub. Many RSS/ATOM feeds only provide fields like description or omit content, so this can raise on nil and abort the polling run.
| url = i.id if url.blank? || url !~ /^https?\:\/\// | ||
|
|
||
| content = CGI.unescapeHTML(i.content.scrub) | ||
| TopicEmbed.import(user, url, i.title, content) |
There was a problem hiding this comment.
Raw feed HTML becomes stored XSS
High Severity
poll_feed imports external HTML directly and TopicEmbed.import stores it with cook_method set to raw_html. Post#cook then returns raw unchanged, so untrusted feed markup is treated as trusted cooked HTML, enabling stored script injection.
Additional Locations (2)
| @topic_view = TopicView.new(topic_id, current_user, {best: 5}) | ||
| else | ||
| Jobs.enqueue(:retrieve_topic, user_id: current_user.try(:id), embed_url: embed_url) | ||
| render 'loading' |
There was a problem hiding this comment.
Embed loading can loop forever
Medium Severity
When no topic exists, embed#best always enqueues :retrieve_topic and renders loading, but it never verifies retrieval can succeed. If TopicRetriever rejects the URL host or cannot resolve embed_by_username, no topic is created and loading.html.erb keeps reloading indefinitely.
Additional Locations (2)
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
Note
High Risk
Adds remote content fetching + raw-HTML rendering and relaxes validations for imported content, which is security- and data-integrity-sensitive. Also introduces new scheduled/async jobs that can affect background load and topic creation/update behavior.
Overview
Adds a new embedding surface:
GET /embed/bestrenders anembedlayout for displaying a topic’s “best” posts inside an iframe, with new JS/CSS assets to embed the iframe and auto-resize it viapostMessage.Introduces
TopicEmbedplus background retrieval (Jobs::RetrieveTopic,TopicRetriever) and an hourlyJobs::PollFeedto create/update topics from remote pages or RSS/Atom feeds (usingruby-readabilityandsimple-rss), gated by newembeddingsite settings.Extends posts with a
cook_methodcolumn andPost.cook_methodsenum so imported posts can bypass the cooking pipeline and store raw HTML;PostCreator/PostRevisorare updated to supportcook_methodand optional validation skipping for imports. Also updates the Disqus import task to bootstrap topics viaTopicEmbed.import_remote, and adds DB migrations + i18n strings for embedding settings/UI.Written by Cursor Bugbot for commit 4f8aed2. Configure here.