Skip to content

Enhance embed URL handling and validation system#7

Open
akshayutture-augment wants to merge 1 commit into
embed-url-handling-prefrom
embed-url-handling-post
Open

Enhance embed URL handling and validation system#7
akshayutture-augment wants to merge 1 commit into
embed-url-handling-prefrom
embed-url-handling-post

Conversation

@akshayutture-augment

@akshayutture-augment akshayutture-augment commented Nov 17, 2025

Copy link
Copy Markdown

No description provided.

@augmentcode augmentcode Bot 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.

Review completed. 4 suggestions posted.

Comment augment review to trigger a new review at any time.

<%= 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.

ERB syntax looks invalid here: <%- end if %> should close the earlier if with just end. This will raise a template compilation error and prevent the page from rendering.

🤖 Was this useful? React with 👍 or 👎

window.onload = function() {
if (parent) {
// Send a post message with our loaded height
parent.postMessage({type: 'discourse-resize', height: document['body'].offsetHeight}, '<%= request.referer %>');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The postMessage targetOrigin should be an origin (scheme://host[:port]), not the full referrer URL; passing a full URL may cause the message to be dropped by the browser. Consider restricting this to the referer origin to ensure the iframe resize message is delivered.

🤖 Was this useful? React with 👍 or 👎

Comment thread app/models/topic_embed.rb
# 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])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rendering imported feed content with cook_method: Post.cook_methods[:raw_html] bypasses sanitization and can allow untrusted HTML into posts (XSS risk), especially for RSS/ATOM imports. It would be safer to ensure the content is sanitized/whitelisted before rendering or avoid using the raw HTML cook method for external content.

🤖 Was this useful? React with 👍 or 👎

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

content = CGI.unescapeHTML(i.content.scrub)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Some feeds do not include an item.content; calling scrub on nil here will raise and halt polling. Consider handling missing content (e.g., falling back to description or skipping items without content).

🤖 Was this useful? React with 👍 or 👎

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