From 3a501cd32c7393ec94852644fe96fce6d0fd99ab Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 18:58:04 +0200 Subject: [PATCH 1/7] 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 From c3ae67a2b699fd7697661774c063bcc8dd6cc19a Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 19:02:25 +0200 Subject: [PATCH 2/7] Require a host on a link preview's link and image Ruby parses "https:/rooms/1" as an HTTPS URL with no host, and a browser resolves it against whatever origin Campfire is served from, so the scheme check alone still let a message body aim the preview at a path here. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- lib/rails_ext/actiontext_opengraph_embeds.rb | 13 +++++++++---- .../rails_ext/actiontext_opengraph_embeds_test.rb | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 102e1d8..8f283cc 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -24,11 +24,16 @@ class ActionText::Attachment::OpengraphEmbed 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. + # http or https URL naming a host. 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. A URL like "https:/rooms/1" + # needs the host check as well as the scheme one: Ruby parses it as HTTPS, + # and a browser resolves it against whatever origin Campfire is served from. def web_url(value) - value if value.present? && URI.parse(value).is_a?(URI::HTTP) + return if value.blank? + + parsed = URI.parse(value) + value if parsed.is_a?(URI::HTTP) && parsed.host.present? rescue URI::InvalidURIError nil end diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb index 15c1481..2cb5f5d 100644 --- a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -10,7 +10,8 @@ class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase 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| + "/rooms/1", "rooms/1", "", "http://exa mple.com/ ", + "https:/rooms/1", "https:rooms/1", "http:/rooms/1", "https://", "http://:80/rooms/1" ].each do |value| embed = embed_from href: value, url: value assert_nil embed.href, "expected #{value.inspect} to be dropped as a link" From eceec2898b9d62c2b1e3eeceea05a1b40a42a214 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 19:08:54 +0200 Subject: [PATCH 3/7] Keep a link preview's link and image off this Campfire's own host A preview belongs to the page it previews, so both URLs point somewhere else. An absolute URL on our own host passed the scheme and host checks, and every reader's browser fetched it with their session attached, which turns a message into a GET request made on the reader's behalf. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- lib/rails_ext/actiontext_opengraph_embeds.rb | 19 ++++++++++++------- test/controllers/rooms_controller_test.rb | 16 ++++++++++++++++ .../actiontext_opengraph_embeds_test.rb | 16 ++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 8f283cc..a45b6f5 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -23,20 +23,25 @@ class ActionText::Attachment::OpengraphEmbed } end - # A link preview points at what we unfurled, which is always an absolute - # http or https URL naming a host. 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. A URL like "https:/rooms/1" - # needs the host check as well as the scheme one: Ruby parses it as HTTPS, - # and a browser resolves it against whatever origin Campfire is served from. + # A link preview points at what we unfurled: an absolute http or https URL + # on some other host. Drop anything else a message body asks for, so it + # can't aim the preview's link or its image at this Campfire and have every + # reader's browser fetch it with their session attached. def web_url(value) return if value.blank? parsed = URI.parse(value) - value if parsed.is_a?(URI::HTTP) && parsed.host.present? + value if parsed.is_a?(URI::HTTP) && elsewhere?(parsed.host) rescue URI::InvalidURIError nil end + + # "https:/rooms/1" parses as HTTPS with no host at all, and a browser + # resolves both that and our own hostname against the origin Campfire is + # served from. + def elsewhere?(host) + host.present? && !host.casecmp?(Current.request_host.to_s) + 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 2bcae89..62edf72 100644 --- a/test/controllers/rooms_controller_test.rb +++ b/test/controllers/rooms_controller_test.rb @@ -35,6 +35,22 @@ class RoomsControllerTest < ActionDispatch::IntegrationTest assert_match "Free cookies", response.body end + test "show renders a link preview written by hand without its image pointed at this Campfire" do + room = rooms(:watercooler) + own_url = room_url(room, host: "www.example.com") + post room_messages_url(room, format: :turbo_stream), params: { message: { + body: link_preview_body(href: own_url, url: own_url), + client_message_id: "same-host-preview" } } + assert_response :success + + get room_url(room) + + assert_response :success + assert_no_match %r{ "once.campfire.test") do + [ "https://once.campfire.test/rooms/1", "http://once.campfire.test/rooms/1", + "https://ONCE.Campfire.Test/rooms/1" ].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 + + embed = embed_from href: "https://example.com/page", url: "https://example.com/image.png" + assert_equal "https://example.com/page", embed.href + assert_equal "https://example.com/image.png", embed.url + 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" From ef4b88d74867d31fad8ad8fe12f9588cc40e53d3 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 19:14:59 +0200 Subject: [PATCH 4/7] Compare a preview's host to ours with the escapes resolved Ruby leaves a percent-escape in URI#host, so "https://%77ww.example.com" read as a different host than the one Campfire answers on while a browser unescaped it straight back to us. A host that carries an escape, or a trailing dot, is now measured the way the browser will read it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- lib/rails_ext/actiontext_opengraph_embeds.rb | 12 ++++++++++-- .../rails_ext/actiontext_opengraph_embeds_test.rb | 5 +++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index a45b6f5..888b405 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -38,9 +38,17 @@ class ActionText::Attachment::OpengraphEmbed # "https:/rooms/1" parses as HTTPS with no host at all, and a browser # resolves both that and our own hostname against the origin Campfire is - # served from. + # served from. A percent-escape hides our hostname from this comparison + # while a browser still unescapes it back to us, so an escaped host is out + # too, and neither case is anything an unfurl could have produced. def elsewhere?(host) - host.present? && !host.casecmp?(Current.request_host.to_s) + return false if host.blank? || host.include?("%") + + canonical_host(host) != canonical_host(Current.request_host.to_s) + end + + def canonical_host(host) + host.downcase.delete_suffix(".") end end diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb index 8185e13..523232f 100644 --- a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -19,10 +19,11 @@ class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase end end - test "drops a link and an image on this Campfire's own host" do + test "drops a link and an image on this Campfire's own host, however it is spelled" do Current.set request: ActionDispatch::TestRequest.create("HTTP_HOST" => "once.campfire.test") do [ "https://once.campfire.test/rooms/1", "http://once.campfire.test/rooms/1", - "https://ONCE.Campfire.Test/rooms/1" ].each do |value| + "https://ONCE.Campfire.Test/rooms/1", "https://once.campfire.test./rooms/1", + "https://%6fnce.campfire.test/rooms/1", "https://%77ww.example.com/x.png" ].each do |value| embed = embed_from href: value, url: value assert_nil embed.href, "expected #{value.inspect} to be dropped as a link" From 32e3e5aea8db140bd6cf97a53a1a31ba38b994a8 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 19:20:46 +0200 Subject: [PATCH 5/7] Bust the cached message presentation The room caches each message's rendered presentation, and its key can't see the link preview partial, which ActionText renders by name rather than through a render call the digestor can follow. Without a new version, a message already in the cache would keep its old preview. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- app/views/messages/_message.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb index b3b1213..d0c491f 100644 --- a/app/views/messages/_message.html.erb +++ b/app/views/messages/_message.html.erb @@ -1,7 +1,7 @@ <%# Be sure to check/update messages/_template.html.erb when changing this file %> <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %> -<% cache [ message, "presentation-v2" ] do %> +<% cache [ message, "presentation-v3" ] do %> <%= message_tag message do %>

<%= local_datetime_tag message.created_at, style: :date %>

From 9e19658ee0e4d7902fb81e87dc6fcc35ffbd88f9 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 20:21:01 +0200 Subject: [PATCH 6/7] Take a link preview's host as a domain name, not an address A browser rewrites the many spellings of an address into one before it fetches, so "http://2130706433/rooms/1" arrives at 127.0.0.1 while a comparison here still reads the digits. A preview names a page on the public internet, so require its host to look like a domain name and leave the rewriting race alone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- lib/rails_ext/actiontext_opengraph_embeds.rb | 10 +++++++++- test/lib/rails_ext/actiontext_opengraph_embeds_test.rb | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 888b405..712844d 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -42,11 +42,19 @@ class ActionText::Attachment::OpengraphEmbed # while a browser still unescapes it back to us, so an escaped host is out # too, and neither case is anything an unfurl could have produced. def elsewhere?(host) - return false if host.blank? || host.include?("%") + return false unless named_host?(host) canonical_host(host) != canonical_host(Current.request_host.to_s) end + # A preview names a page on the public internet, so its host is a domain + # name: it has a dot and a letter in it, and no escapes. A bare address is + # not one, and a browser rewrites the many spellings of an address into a + # single one before it fetches, which is a race a comparison here loses. + def named_host?(host) + host.present? && host.exclude?("%") && host.include?(".") && host.match?(/[a-z]/i) + end + def canonical_host(host) host.downcase.delete_suffix(".") end diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb index 523232f..a93360b 100644 --- a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -36,6 +36,16 @@ class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase end end + test "drops a link and an image on a bare address rather than a domain name" do + [ "http://127.0.0.1/rooms/1", "http://2130706433/rooms/1", "http://0177.0.0.1/rooms/1", + "http://[::1]/rooms/1", "http://localhost/rooms/1", "https://203.0.113.10/image.png" ].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" From 436ea064578f0bbca154faf5e416bdfa01f4cf8c Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 20:25:35 +0200 Subject: [PATCH 7/7] Read a preview's host as a name by its last label A domain name ends in a word, which is what keeps it from reading as an address. "0x7f.0.0.1" carries a dot and a letter, so the previous shape check let it through while a browser fetched 127.0.0.1. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- lib/rails_ext/actiontext_opengraph_embeds.rb | 12 +++++++++--- .../rails_ext/actiontext_opengraph_embeds_test.rb | 10 +++++++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/rails_ext/actiontext_opengraph_embeds.rb b/lib/rails_ext/actiontext_opengraph_embeds.rb index 712844d..d0748cd 100644 --- a/lib/rails_ext/actiontext_opengraph_embeds.rb +++ b/lib/rails_ext/actiontext_opengraph_embeds.rb @@ -48,11 +48,17 @@ class ActionText::Attachment::OpengraphEmbed end # A preview names a page on the public internet, so its host is a domain - # name: it has a dot and a letter in it, and no escapes. A bare address is - # not one, and a browser rewrites the many spellings of an address into a + # name, written plainly. A bare address is not one, and a browser rewrites + # the many spellings of an address ("2130706433", "0x7f.0.0.1") into a # single one before it fetches, which is a race a comparison here loses. def named_host?(host) - host.present? && host.exclude?("%") && host.include?(".") && host.match?(/[a-z]/i) + host.present? && host.exclude?("%") && host.include?(".") && domain_ending?(host.split(".").last) + end + + # What keeps a name from reading as an address is its last label, which is + # a word: never a number, and never the hexadecimal spelling of one. + def domain_ending?(label) + label.match?(/[a-z]/i) && !label.match?(/\A0x/i) end def canonical_host(host) diff --git a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb index a93360b..6949f79 100644 --- a/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb +++ b/test/lib/rails_ext/actiontext_opengraph_embeds_test.rb @@ -38,7 +38,8 @@ class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase test "drops a link and an image on a bare address rather than a domain name" do [ "http://127.0.0.1/rooms/1", "http://2130706433/rooms/1", "http://0177.0.0.1/rooms/1", - "http://[::1]/rooms/1", "http://localhost/rooms/1", "https://203.0.113.10/image.png" ].each do |value| + "http://0x7f.0.0.1/rooms/1", "http://1.2.3.0xff/rooms/1", "http://[::1]/rooms/1", + "http://localhost/rooms/1", "https://203.0.113.10/image.png" ].each do |value| embed = embed_from href: value, url: value assert_nil embed.href, "expected #{value.inspect} to be dropped as a link" @@ -46,6 +47,13 @@ class ActionText::Attachment::OpengraphEmbedTest < ActiveSupport::TestCase end end + test "keeps an internationalized domain written in punycode" do + embed = embed_from href: "https://xn--80aswg.xn--p1ai/page", url: "https://xn--80aswg.xn--p1ai/image.png" + + assert_equal "https://xn--80aswg.xn--p1ai/page", embed.href + assert_equal "https://xn--80aswg.xn--p1ai/image.png", embed.url + 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"