Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ gem 'unicorn', require: false
gem 'puma', require: false
gem 'rbtrace', require: false

# required for feed importing and embedding
gem 'ruby-readability', require: false
gem 'simple-rss', require: false

# perftools only works on 1.9 atm
group :profile do
# travis refuses to install this, instead of fuffing, just avoid it for now
Expand Down
7 changes: 7 additions & 0 deletions Gemfile_rails4.lock
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ GEM
fspath (2.0.5)
given_core (3.1.1)
sorcerer (>= 0.3.7)
guess_html_encoding (0.0.9)
handlebars-source (1.1.2)
hashie (2.0.5)
highline (1.6.20)
Expand Down Expand Up @@ -309,6 +310,9 @@ GEM
rspec-mocks (~> 2.14.0)
ruby-hmac (0.4.0)
ruby-openid (2.3.0)
ruby-readability (0.5.7)
guess_html_encoding (>= 0.0.4)
nokogiri (>= 1.4.2)
sanitize (2.0.6)
nokogiri (>= 1.4.4)
sass (3.2.12)
Expand Down Expand Up @@ -337,6 +341,7 @@ GEM
celluloid (>= 0.14.1)
ice_cube (~> 0.11.0)
sidekiq (~> 2.15.0)
simple-rss (1.3.1)
simplecov (0.7.1)
multi_json (~> 1.0)
simplecov-html (~> 0.7.1)
Expand Down Expand Up @@ -466,6 +471,7 @@ DEPENDENCIES
rinku
rspec-given
rspec-rails
ruby-readability
sanitize
sass
sass-rails
Expand All @@ -474,6 +480,7 @@ DEPENDENCIES
sidekiq (= 2.15.1)
sidekiq-failures
sidetiq (>= 0.3.6)
simple-rss
simplecov
sinatra
slim
Expand Down
27 changes: 27 additions & 0 deletions app/assets/javascripts/embed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* global discourseUrl */
/* global discourseEmbedUrl */
(function() {

var comments = document.getElementById('discourse-comments'),
iframe = document.createElement('iframe');
iframe.src = discourseUrl + "embed/best?embed_url=" + encodeURIComponent(discourseEmbedUrl);
iframe.id = 'discourse-embed-frame';
iframe.width = "100%";
iframe.frameBorder = "0";
iframe.scrolling = "no";
comments.appendChild(iframe);
Comment on lines +5 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add null check for the container element.

If #discourse-comments element doesn't exist on the page, comments.appendChild(iframe) will throw an error.

💡 Proposed fix
   var comments = document.getElementById('discourse-comments'),
       iframe = document.createElement('iframe');
+
+  if (!comments) { return; }
+
   iframe.src = discourseUrl + "embed/best?embed_url=" + encodeURIComponent(discourseEmbedUrl);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var comments = document.getElementById('discourse-comments'),
iframe = document.createElement('iframe');
iframe.src = discourseUrl + "embed/best?embed_url=" + encodeURIComponent(discourseEmbedUrl);
iframe.id = 'discourse-embed-frame';
iframe.width = "100%";
iframe.frameBorder = "0";
iframe.scrolling = "no";
comments.appendChild(iframe);
var comments = document.getElementById('discourse-comments'),
iframe = document.createElement('iframe');
if (!comments) { return; }
iframe.src = discourseUrl + "embed/best?embed_url=" + encodeURIComponent(discourseEmbedUrl);
iframe.id = 'discourse-embed-frame';
iframe.width = "100%";
iframe.frameBorder = "0";
iframe.scrolling = "no";
comments.appendChild(iframe);
🤖 Prompt for AI Agents
In @app/assets/javascripts/embed.js around lines 5 - 12, The code assumes
document.getElementById('discourse-comments') returns an element and will throw
if null; modify the logic around the comments variable (result of
document.getElementById('discourse-comments')) to check for null before
creating/appending the iframe (id 'discourse-embed-frame'), and only build and
append the iframe if comments exists—optionally log or return early when
comments is null.



function postMessageReceived(e) {
if (!e) { return; }
if (discourseUrl.indexOf(e.origin) === -1) { return; }

if (e.data) {
if (e.data.type === 'discourse-resize' && e.data.height) {
iframe.height = e.data.height + "px";
}
}
}
Comment on lines +15 to +24

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Security: Origin check using indexOf is bypassable.

The check discourseUrl.indexOf(e.origin) === -1 can be bypassed. If discourseUrl is https://discourse.example.com, an attacker origin like https://discourse.example.com.evil.com would pass this check since indexOf finds a substring match.

🔒 Proposed fix with strict origin validation
   function postMessageReceived(e) {
     if (!e) { return; }
-    if (discourseUrl.indexOf(e.origin) === -1) { return; }
+    // Extract origin from discourseUrl for strict comparison
+    var defined;
+    try {
+      defined = new URL(discourseUrl).origin;
+    } catch (err) {
+      return;
+    }
+    if (e.origin !== defined) { return; }

     if (e.data) {
       if (e.data.type === 'discourse-resize' && e.data.height) {
         iframe.height = e.data.height + "px";
       }
     }
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function postMessageReceived(e) {
if (!e) { return; }
if (discourseUrl.indexOf(e.origin) === -1) { return; }
if (e.data) {
if (e.data.type === 'discourse-resize' && e.data.height) {
iframe.height = e.data.height + "px";
}
}
}
function postMessageReceived(e) {
if (!e) { return; }
// Extract origin from discourseUrl for strict comparison
var defined;
try {
defined = new URL(discourseUrl).origin;
} catch (err) {
return;
}
if (e.origin !== defined) { return; }
if (e.data) {
if (e.data.type === 'discourse-resize' && e.data.height) {
iframe.height = e.data.height + "px";
}
}
}
🤖 Prompt for AI Agents
In @app/assets/javascripts/embed.js around lines 15 - 24, The origin check in
postMessageReceived uses discourseUrl.indexOf(e.origin) which is vulnerable to
substring matches; replace it with a strict origin validation by comparing
e.origin exactly to the expected origin(s) (e.g. e.origin === discourseUrl) or
by parsing both with the URL constructor and comparing protocol + host (or
maintaining an allowlist array and checking membership), and then only proceed
to set iframe.height when the strict check passes; update references in
postMessageReceived, discourseUrl and the iframe.height assignment to use the
new exact-origin logic.

window.addEventListener('message', postMessageReceived, false);

})();
69 changes: 69 additions & 0 deletions app/assets/stylesheets/embed.css.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//= require ./vendor/normalize
//= require ./common/foundation/base

article.post {
border-bottom: 1px solid #ddd;

.post-date {
float: right;
color: #aaa;
font-size: 12px;
margin: 4px 4px 0 0;
}

.author {
padding: 20px 0;
width: 92px;
float: left;

text-align: center;

h3 {
text-align: center;
color: #4a6b82;
font-size: 13px;
margin: 0;
}
}

.cooked {
padding: 20px 0;
margin-left: 92px;

p {
margin: 0 0 1em 0;
}
}
}

header {
padding: 10px 10px 20px 10px;

font-size: 18px;

border-bottom: 1px solid #ddd;
}

footer {
font-size: 18px;

.logo {
margin-right: 10px;
margin-top: 10px;
}

a[href].button {
margin: 10px 0 0 10px;
}
}

.logo {
float: right;
max-height: 30px;
}

a[href].button {
background-color: #eee;
padding: 5px;
display: inline-block;
}
34 changes: 34 additions & 0 deletions app/controllers/embed_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
class EmbedController < ApplicationController
skip_before_filter :check_xhr
skip_before_filter :preload_json
before_filter :ensure_embeddable
Comment on lines +2 to +4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

before_filter skip_before_filter Rails deprecation removal timeline

💡 Result:

Short answer: Rails deprecated the *_filter methods (including before_filter and skip_before_filter) in Rails 4.2 and removed them in Rails 5.1 (use before_action / skip_before_action instead). (guides.rubyonrails.org)

Citations:


🏁 Script executed:

# Check Rails version from Gemfile or gemspec
fd -e Gemfile -e "*.gemspec" | head -5 | xargs -I {} cat {} | grep -i "^gem.*rails" | head -5

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 67


🏁 Script executed:

# Check if there are other uses of skip_before_filter or before_filter in the codebase
rg "skip_before_filter|before_filter" --type rb | head -20

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 111


🏁 Script executed:

# Search for Gemfile more broadly
find . -name "Gemfile" -o -name "*.gemspec" | xargs cat 2>/dev/null | grep -i "rails"

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 1871


🏁 Script executed:

# Search for deprecated filter methods without type specification
rg "skip_before_filter|before_filter" --max-count 30

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 50398


🏁 Script executed:

# Also check the specific file mentioned in the review
cat -n app/controllers/embed_controller.rb | head -20

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 758


Replace deprecated filter methods with Rails 5+ compatible versions.

skip_before_filter and before_filter were deprecated in Rails 4.2 and removed in Rails 5.1. Update to skip_before_action and before_action:

Suggested change
- skip_before_filter :check_xhr
- skip_before_filter :preload_json
- before_filter :ensure_embeddable
+ skip_before_action :check_xhr
+ skip_before_action :preload_json
+ before_action :ensure_embeddable
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
skip_before_filter :check_xhr
skip_before_filter :preload_json
before_filter :ensure_embeddable
skip_before_action :check_xhr
skip_before_action :preload_json
before_action :ensure_embeddable
🤖 Prompt for AI Agents
In @app/controllers/embed_controller.rb around lines 2 - 4, Replace deprecated
filter methods: change uses of skip_before_filter and before_filter in this
controller to the Rails 5+ equivalents skip_before_action and before_action
(e.g., update the lines referencing skip_before_filter :check_xhr,
skip_before_filter :preload_json, and before_filter :ensure_embeddable to use
skip_before_action and before_action respectively) so the controller uses
ensure_embeddable with the modern callbacks API.


layout 'embed'

def best
embed_url = params.require(:embed_url)
topic_id = TopicEmbed.topic_id_for_embed(embed_url)

if topic_id
@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'
end

discourse_expires_in 1.minute
end

private

def ensure_embeddable
raise Discourse::InvalidAccess.new('embeddable host not set') if SiteSetting.embeddable_host.blank?
raise Discourse::InvalidAccess.new('invalid referer host') if URI(request.referer || '').host != SiteSetting.embeddable_host

response.headers['X-Frame-Options'] = "ALLOWALL"
rescue URI::InvalidURIError
raise Discourse::InvalidAccess.new('invalid referer host')
end


end
24 changes: 24 additions & 0 deletions app/jobs/regular/retrieve_topic.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require_dependency 'email/sender'
require_dependency 'topic_retriever'

module Jobs

# Asynchronously retrieve a topic from an embedded site
class RetrieveTopic < Jobs::Base

def execute(args)
raise Discourse::InvalidParameters.new(:embed_url) unless args[:embed_url].present?

user = nil
if args[:user_id]
user = User.where(id: args[:user_id]).first
end

TopicRetriever.new(args[:embed_url], no_throttle: user.try(:staff?)).retrieve
end

end

end


41 changes: 41 additions & 0 deletions app/jobs/scheduled/poll_feed.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#
# Creates and Updates Topics based on an RSS or ATOM feed.
#
require 'digest/sha1'
require_dependency 'post_creator'
require_dependency 'post_revisor'
require 'open-uri'

module Jobs
class PollFeed < Jobs::Scheduled
recurrence { hourly }
sidekiq_options retry: false

def execute(args)
poll_feed if SiteSetting.feed_polling_enabled? &&
SiteSetting.feed_polling_url.present? &&
SiteSetting.embed_by_username.present?
end

def feed_key
@feed_key ||= "feed-modified:#{Digest::SHA1.hexdigest(SiteSetting.feed_polling_url)}"
end
Comment on lines +20 to +22

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "feed_key" --type ruby

Repository: AI-Code-Review-Evals/coderabbit-discourse

Length of output: 263


Remove the feed_key method or document its intent with a TODO comment.

The method is defined but never used anywhere in the codebase. If this is part of a planned feature, add a TODO comment explaining the intention; otherwise, remove it.

🤖 Prompt for AI Agents
In @app/jobs/scheduled/poll_feed.rb around lines 20 - 22, The method feed_key is
defined but unused; either remove the feed_key method entirely or, if it's
intended for a future feature, add a concise TODO above it explaining its
purpose and expected usage (e.g., why it computes
"feed-modified:#{Digest::SHA1.hexdigest(SiteSetting.feed_polling_url)}" and when
it will be referenced). Ensure the change targets the feed_key method and
mentions the dependency on SiteSetting.feed_polling_url in the TODO so future
readers know the context.


def poll_feed
user = User.where(username_lower: SiteSetting.embed_by_username.downcase).first
return if user.blank?

require 'simple-rss'
rss = SimpleRSS.parse open(SiteSetting.feed_polling_url)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Security: Use URI.open with explicit require for URL fetching.

The Brakeman warning is valid. Using open() with a URL, even from admin settings, should use URI.open explicitly (after require 'open-uri') to make the intent clear and avoid potential issues with Kernel#open.

🔒 Proposed fix
     require 'simple-rss'
-    rss = SimpleRSS.parse open(SiteSetting.feed_polling_url)
+    rss = SimpleRSS.parse URI.open(SiteSetting.feed_polling_url)
🧰 Tools
🪛 Brakeman (7.1.1)

[medium] 29-29: Model attribute used in file name
Type: File Access
Confidence: Medium
More info: https://brakemanscanner.org/docs/warning_types/file_access/

(File Access)

🤖 Prompt for AI Agents
In @app/jobs/scheduled/poll_feed.rb at line 29, The call using Kernel#open in
the feed poll job should explicitly use open-uri: add require 'open-uri' to the
file and change the SimpleRSS.parse invocation to use
URI.open(SiteSetting.feed_polling_url) instead of open(...); update the line
referencing SimpleRSS.parse and SiteSetting.feed_polling_url so it calls
URI.open to satisfy Brakeman and make the URL fetch intent explicit.


rss.items.each do |i|
url = i.link
url = i.id if url.blank? || url !~ /^https?\:\/\//

content = CGI.unescapeHTML(i.content.scrub)
TopicEmbed.import(user, url, i.title, content)
Comment on lines +35 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential NoMethodError if feed item has no content.

If i.content is nil, calling .scrub will raise. Add a nil guard or use safe navigation.

🐛 Proposed fix
-      content = CGI.unescapeHTML(i.content.scrub)
+      content = i.content.present? ? CGI.unescapeHTML(i.content.scrub) : ''
+      next if content.blank?
       TopicEmbed.import(user, url, i.title, content)
🤖 Prompt for AI Agents
In @app/jobs/scheduled/poll_feed.rb around lines 35 - 36, The code calls
i.content.scrub which raises NoMethodError when i.content is nil; guard against
nil by using safe navigation or a fallback before unescaping and scrubbing
(e.g., get a safe_content using i.content&.scrub || "" or similar) and then pass
CGI.unescapeHTML(safe_content) into TopicEmbed.import so TopicEmbed.import(user,
url, i.title, safe_content) always receives a string.

end
end

end
end
9 changes: 9 additions & 0 deletions app/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def self.types
@types ||= Enum.new(:regular, :moderator_action)
end

def self.cook_methods
@cook_methods ||= Enum.new(:regular, :raw_html)
end

def self.find_by_detail(key, value)
includes(:post_details).where(post_details: { key: key, value: value }).first
end
Expand Down Expand Up @@ -124,6 +128,11 @@ def post_analyzer
end

def cook(*args)
# For some posts, for example those imported via RSS, we support raw HTML. In that
# case we can skip the rendering pipeline.
return raw if cook_method == Post.cook_methods[:raw_html]

# Default is to cook posts
Plugin::Filter.apply(:after_post_cook, self, post_analyzer.cook(*args))
end

Expand Down
82 changes: 82 additions & 0 deletions app/models/topic_embed.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
require_dependency 'nokogiri'

class TopicEmbed < ActiveRecord::Base
belongs_to :topic
belongs_to :post
validates_presence_of :embed_url
validates_presence_of :content_sha1

# Import an article from a source (RSS/Atom/Other)
def self.import(user, url, title, contents)
return unless url =~ /^https?\:\/\//

contents << "\n<hr>\n<small>#{I18n.t('embed.imported_from', link: "<a href='#{url}'>#{url}</a>")}</small>\n"

embed = TopicEmbed.where(embed_url: url).first
content_sha1 = Digest::SHA1.hexdigest(contents)
post = nil

# If there is no embed, create a topic, post and the embed.
if embed.blank?
Topic.transaction do
creator = PostCreator.new(user, title: title, raw: absolutize_urls(url, contents), skip_validations: true, cook_method: Post.cook_methods[:raw_html])
post = creator.create
if post.present?
TopicEmbed.create!(topic_id: post.topic_id,
embed_url: url,
content_sha1: content_sha1,
post_id: post.id)
end
end
else
post = embed.post
# Update the topic if it changed
if content_sha1 != embed.content_sha1
revisor = PostRevisor.new(post)
revisor.revise!(user, absolutize_urls(url, contents), skip_validations: true, bypass_rate_limiter: true)
embed.update_column(:content_sha1, content_sha1)
end
end

post
end

def self.import_remote(user, url, opts=nil)
require 'ruby-readability'

opts = opts || {}
doc = Readability::Document.new(open(url).read,
tags: %w[div p code pre h1 h2 h3 b em i strong a img],
attributes: %w[href src])

TopicEmbed.import(user, url, opts[:title] || doc.title, doc.content)
end
Comment on lines +44 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Security: open(url) is vulnerable to SSRF and command injection.

Using Kernel#open with user-controlled URLs is dangerous. In Ruby, open("|command") executes shell commands, and even without that, this allows Server-Side Request Forgery (SSRF) to internal services. Use a safer HTTP client like Net::HTTP or validate/sanitize the URL scheme strictly.

🔒 Proposed fix using URI.open with scheme validation
 def self.import_remote(user, url, opts=nil)
   require 'ruby-readability'
+  require 'open-uri'

   opts = opts || {}
+  
+  # Validate URL scheme to prevent SSRF and command injection
+  uri = URI.parse(url)
+  raise ArgumentError, "Invalid URL scheme" unless %w[http https].include?(uri.scheme)
+  
-  doc = Readability::Document.new(open(url).read,
+  doc = Readability::Document.new(URI.open(url).read,
                                       tags: %w[div p code pre h1 h2 h3 b em i strong a img],
                                       attributes: %w[href src])

   TopicEmbed.import(user, url, opts[:title] || doc.title, doc.content)
 end
🤖 Prompt for AI Agents
In @app/models/topic_embed.rb around lines 44 - 53, In TopicEmbed.import_remote,
avoid using Kernel#open(url) which allows shell execution and SSRF; instead
parse and validate the URL (use URI.parse) to ensure only http/https schemes
(and optionally block local/internal hosts), then fetch the body with a safe
HTTP client (e.g., Net::HTTP or URI.open with strict scheme/host checks) with
timeouts and redirect handling, and rescue/fail gracefully; replace
open(url).read with the validated fetch and keep passing the fetched content
into Readability::Document, and ensure errors are logged/handled before calling
TopicEmbed.import.


# Convert any relative URLs to absolute. RSS is annoying for this.
def self.absolutize_urls(url, contents)
uri = URI(url)
prefix = "#{uri.scheme}://#{uri.host}"
prefix << ":#{uri.port}" if uri.port != 80 && uri.port != 443

fragment = Nokogiri::HTML.fragment(contents)
fragment.css('a').each do |a|
href = a['href']
if href.present? && href.start_with?('/')
a['href'] = "#{prefix}/#{href.sub(/^\/+/, '')}"
end
end
fragment.css('img').each do |a|
src = a['src']
if src.present? && src.start_with?('/')
a['src'] = "#{prefix}/#{src.sub(/^\/+/, '')}"
end
end

fragment.to_html
end

def self.topic_id_for_embed(embed_url)
TopicEmbed.where(embed_url: embed_url).pluck(:topic_id).first
end

end
30 changes: 30 additions & 0 deletions app/views/embed/best.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<header>
<%- if @topic_view.posts.present? %>
<%= link_to(I18n.t('embed.title'), @topic_view.topic.url, class: 'button', target: '_blank') %>
<%- else %>
<%= link_to(I18n.t('embed.start_discussion'), @topic_view.topic.url, class: 'button', target: '_blank') %>
<%- end if %>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Syntax error: end if is invalid Ruby.

<%- end if %> is not valid Ruby syntax. This will cause a template rendering error. Use <%- end %> to close the if block.

🐛 Proposed fix
-  <%- end if %>
+  <%- end %>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<%- end if %>
<%- end %>
🤖 Prompt for AI Agents
In @app/views/embed/best.html.erb at line 6, The ERB template uses invalid Ruby
syntax `<%- end if %>`; update the template to close the if block correctly by
replacing that token with a standard ERB block end (`<%- end %>`) so the `if`
opened earlier is properly terminated and the view will render without a syntax
error.


<%= link_to(image_tag(SiteSetting.logo_url, class: 'logo'), Discourse.base_url) %>
</header>

<%- if @topic_view.posts.present? %>
<%- @topic_view.posts.each do |post| %>
<article class='post'>
<%= link_to post.created_at.strftime("%e %b %Y"), post.url, class: 'post-date', target: "_blank" %>
<div class='author'>
<img src='<%= post.user.small_avatar_url %>'>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential XSS via unescaped avatar URL.

The avatar URL is interpolated directly into the src attribute without escaping. If small_avatar_url contains malicious content (e.g., quotes or JavaScript), it could break the HTML or enable XSS. Use Rails helpers for safer output.

🔒 Proposed fix
-        <img src='<%= post.user.small_avatar_url %>'>
+        <%= image_tag(post.user.small_avatar_url, alt: post.user.username) %>
🤖 Prompt for AI Agents
In @app/views/embed/best.html.erb at line 16, The avatar URL is inserted raw
into an img src which can enable XSS via crafted URLs; replace the raw
interpolation of post.user.small_avatar_url with a Rails helper that safely
escapes/validates the URL (e.g., use image_tag(post.user.small_avatar_url, alt:
...) or tag.img with url_for()) so the value is escaped and properly encoded
before rendering, ensuring you reference post.user.small_avatar_url and the
surrounding img element in your change.

<h3><%= post.user.username %></h3>
</div>
<div class='cooked'><%= raw post.cooked %></div>
<div style='clear: both'></div>
</article>
<%- end %>

<footer>
<%= link_to(I18n.t('embed.continue'), @topic_view.topic.url, class: 'button', target: '_blank') %>
<%= link_to(image_tag(SiteSetting.logo_url, class: 'logo'), Discourse.base_url) %>
</footer>

<% end %>

Loading