Enhance embed URL handling and validation system

[akshayutture-augment]

Test 4

---

Replicated from ai-code-review-evaluation/discourse-greptile#4

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[akshayutture-augment]

@greptileai

[greptile-apps]

Greptile Overview

Confidence Score: 0/5

• This PR is UNSAFE to merge due to critical security vulnerabilities
• Score reflects three critical security vulnerabilities: two instances of command injection via Kernel#open that allow arbitrary code execution, one XSS vulnerability from unsanitized URL interpolation, and one insecure postMessage origin validation. These issues create immediate security risks that must be resolved before deployment.
• Critical attention required for app/models/topic_embed.rb, app/jobs/scheduled/poll_feed.rb, and app/assets/javascripts/embed.js due to security vulnerabilities

Important Files Changed

File Analysis

Filename	Score	Overview
app/models/topic_embed.rb	0/5	New model with critical command injection via Kernel#open and XSS vulnerability in URL interpolation
app/jobs/scheduled/poll_feed.rb	0/5	New scheduled job with critical command injection vulnerability in feed URL parsing
app/assets/javascripts/embed.js	1/5	New embed client with insecure postMessage origin validation using indexOf
lib/topic_retriever.rb	3/5	New retrieval logic with host validation and throttling, but depends on vulnerable TopicEmbed.import_remote
app/controllers/embed_controller.rb	3/5	New controller with proper host validation in ensure_embeddable, enqueues retrieval jobs

Sequence Diagram

sequenceDiagram
    participant Client as External Site
    participant JS as embed.js
    participant EC as EmbedController
    participant TE as TopicEmbed
    participant Job as RetrieveTopic Job
    participant TR as TopicRetriever
    participant PF as PollFeed
    participant HTTP as External URL

    Client->>JS: Load page with discourse-comments div
    JS->>EC: GET /embed/best?embed_url=...
    EC->>EC: ensure_embeddable() validates referer host
    EC->>TE: topic_id_for_embed(url)
    
    alt Topic exists
        TE-->>EC: return topic_id
        EC->>EC: Create TopicView with best posts
        EC-->>JS: Render embed view
    else Topic not found
        EC->>Job: enqueue(:retrieve_topic)
        EC-->>JS: Render loading view
        Job->>TR: new(embed_url).retrieve
        TR->>TR: Check invalid_host?
        TR->>TR: Check retrieved_recently? (Redis throttle)
        
        alt Feed polling enabled
            TR->>PF: execute({})
            PF->>HTTP: open(feed_url) [VULNERABLE]
            HTTP-->>PF: RSS/Atom feed
            PF->>TE: import(user, url, title, content)
        end
        
        alt Not in feed or no feed
            TR->>TE: import_remote(user, url)
            TE->>HTTP: open(url).read [VULNERABLE]
            HTTP-->>TE: HTML content
            TE->>TE: Readability extraction
        end
        
        TE->>TE: absolutize_urls(url, contents)
        TE->>TE: Inject footer with URL [XSS VULNERABLE]
        TE->>TE: Create/Update Post with raw_html
    end
    
    JS->>JS: Listen for postMessage events
    EC->>JS: Send discourse-resize message
    JS->>JS: Update iframe height

Loading

[greptile-apps]

_{27 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic: XSS vulnerability: unsanitized URL directly interpolated into HTML. Attacker-controlled url value can inject arbitrary JavaScript.

Suggested change

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

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/models/topic_embed.rb
Line: 13:13

Comment:
**logic:** XSS vulnerability: unsanitized URL directly interpolated into HTML. Attacker-controlled `url` value can inject arbitrary JavaScript.

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

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: CRITICAL: open() with user-controlled URL enables command injection. Kernel#open interprets pipe characters, allowing arbitrary shell command execution via crafted URLs like |ls.

Use URI.open() or Net::HTTP instead:

uri = URI.parse(url)
response = Net::HTTP.get_response(uri)
doc = Readability::Document.new(response.body, ...)

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/models/topic_embed.rb
Line: 48:48

Comment:
**logic:** CRITICAL: `open()` with user-controlled URL enables command injection. Kernel#open interprets pipe characters, allowing arbitrary shell command execution via crafted URLs like `|ls`.

Use `URI.open()` or `Net::HTTP` instead:

```
uri = URI.parse(url)
response = Net::HTTP.get_response(uri)
doc = Readability::Document.new(response.body, ...)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: CRITICAL: same command injection vulnerability via Kernel#open with admin-configured URL. Use URI.open() or safer HTTP client:

uri = URI.parse(SiteSetting.feed_polling_url)
response = Net::HTTP.get(uri)
rss = SimpleRSS.parse(response)

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/jobs/scheduled/poll_feed.rb
Line: 29:29

Comment:
**logic:** CRITICAL: same command injection vulnerability via Kernel#open with admin-configured URL. Use `URI.open()` or safer HTTP client:

```
uri = URI.parse(SiteSetting.feed_polling_url)
response = Net::HTTP.get(uri)
rss = SimpleRSS.parse(response)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: Insecure origin validation. indexOf can be bypassed with URLs like https://evil.com?discourse.example.com. Use strict comparison:

Suggested change

	if (discourseUrl.indexOf(e.origin) === -1) { return; }
	if (e.origin !== discourseUrl.replace(/\/$/, '')) { return; }

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/assets/javascripts/embed.js
Line: 17:17

Comment:
**logic:** Insecure origin validation. `indexOf` can be bypassed with URLs like `https://evil.com?discourse.example.com`. Use strict comparison:

```suggestion
    if (e.origin !== discourseUrl.replace(/\/$/, '')) { return; }
```

How can I resolve this? If you propose a fix, please make it concise.