From 3a501cd32c7393ec94852644fe96fce6d0fd99ab Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 18:58:04 +0200 Subject: [PATCH] Render link previews only from web URLs A link preview's link and image come from attributes on the message body, which the composer fills in from the unfurl the server performed. A body written by hand can put anything in those attributes, and the preview partial rendered them as they were. Keep the link and the image only when they parse as absolute http or https URLs, so nothing in a message body can aim either one at another scheme or at a path on this Campfire, and render the title and the description as text. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- .../attachables/_opengraph_embed.html.erb | 4 +- lib/rails_ext/actiontext_opengraph_embeds.rb | 14 +++- test/controllers/rooms_controller_test.rb | 35 ++++++++++ .../actiontext_opengraph_embeds_test.rb | 66 +++++++++++++++++++ 4 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 test/lib/rails_ext/actiontext_opengraph_embeds_test.rb diff --git a/app/views/action_text/attachables/_opengraph_embed.html.erb b/app/views/action_text/attachables/_opengraph_embed.html.erb index f82208a..c5cdd05 100644 --- a/app/views/action_text/attachables/_opengraph_embed.html.erb +++ b/app/views/action_text/attachables/_opengraph_embed.html.erb @@ -3,9 +3,9 @@
- <%= link_to truncate(opengraph_embed.filename, length: 280, omission: "…"), opengraph_embed.href, rel: "noreferrer", target: "_blank" %> + <%= link_to_if opengraph_embed.href.present?, truncate(opengraph_embed.filename, length: 280, omission: "…"), opengraph_embed.href, rel: "noreferrer", target: "_blank" %>
-
<%= truncate(opengraph_embed.caption, length: 560, omission: "…").html_safe %>
+
<%= truncate(opengraph_embed.caption, length: 560, omission: "…") %>
<% if opengraph_embed.url %>
diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 916ab10..102e1d8 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -16,12 +16,22 @@ class ActionText::Attachment::OpengraphEmbed private def attributes_from_node(node) { - href: node["href"], - url: node["url"], + href: web_url(node["href"]), + url: web_url(node["url"]), filename: node["filename"], description: node["caption"] } end + + # A link preview points at what we unfurled, which is always an absolute + # http or https URL. Drop anything else the message body asks for, so a + # body written by hand can't aim the preview's link or its image at another + # scheme or at a path on this Campfire. + def web_url(value) + value if value.present? && URI.parse(value).is_a?(URI::HTTP) + rescue URI::InvalidURIError + nil + end end attr_accessor :href, :url, :filename, :description diff --git a/test/controllers/rooms_controller_test.rb b/test/controllers/rooms_controller_test.rb index 69b2add..2bcae89 100644 --- a/test/controllers/rooms_controller_test.rb +++ b/test/controllers/rooms_controller_test.rb @@ -20,6 +20,35 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest assert response.cookies[:last_room] = users(:david).rooms.last.id end + test "show renders a link preview written by hand without its off-scheme image and link" do + room = rooms(:watercooler) + post room_messages_url(room, format: :turbo_stream), params: { message: { + body: link_preview_body(href: "javascript:alert(1)", url: "data:image/svg+xml;base64,PHN2Zy8+"), + client_message_id: "hand-written-preview" } } + assert_response :success + + get room_url(room) + + assert_response :success + assert_no_match /javascript:alert/, response.body + assert_no_match /data:image\/svg/, response.body + assert_match "Free cookies", response.body + end + + test "show renders an unfurled link preview" do + room = rooms(:watercooler) + post room_messages_url(room, format: :turbo_stream), params: { message: { + body: link_preview_body(href: "https://example.com/page", url: "https://example.com/image.png"), + client_message_id: "unfurled-preview" } } + assert_response :success + + get room_url(room) + + assert_response :success + assert_match %r{ { Room.count }, -1 do @@ -42,4 +71,10 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest delete room_url(rooms(:designers)) end end + + private + def link_preview_body(href:, url:) + %(
) + end end diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb new file mode 100644 index 0000000..15c1481 --- /dev/null +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -0,0 +1,66 @@ +require "test_helper" + +class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase + test "keeps absolute http and https links and images" do + embed = embed_from href: "http://example.com/page", url: "https://example.com/image.png" + + assert_equal "http://example.com/page", embed.href + assert_equal "https://example.com/image.png", embed.url + end + + test "drops a link and an image that aren't web URLs" do + [ "javascript:alert(1)", "data:text/html,pwned", "vbscript:msgbox(1)", "//example.com/image.png", + "/rooms/1", "rooms/1", "", "http://exa mple.com/ " ].each do |value| + embed = embed_from href: value, url: value + + assert_nil embed.href, "expected #{value.inspect} to be dropped as a link" + assert_nil embed.url, "expected #{value.inspect} to be dropped as an image" + end + end + + test "renders the image and the link when both are web URLs" do + html = render_embed href: "https://example.com/page", url: "https://example.com/image.png" + + assert_match %r{Title}, html + assert_match %r{Title", caption: "" + + assert_no_match //, html + assert_no_match /) + node = ActionText::Fragment.wrap(html).find_all(ActionText::Attachment.tag_name).first + + ActionText::Attachment.from_node(node) + end + + def embed_from(**attributes) + attachment_for(**attributes).attachable + end + + def render_embed(**attributes) + attachment = attachment_for(**attributes) + + ApplicationController.render partial: attachment.to_partial_path, locals: { opengraph_embed: attachment } + end +end