From c1ad057db8db186d5408fa986abf2b750ea864fd Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Fri, 11 Sep 2026 16:00:07 +0200 Subject: [PATCH] Escape the OpenGraph image URL in link previews Pasting a link builds the preview by interpolating the unfurled metadata into an HTML string. The image URL went into src="..." unescaped, so a page whose og:image carries a double quote closes the attribute early and everything after it becomes attributes on the preview's img element. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt --- app/javascript/helpers/string_helpers.js | 6 + .../unfurl/lib/opengraph_embed_operation.js | 4 +- .../unfurl_links_controller_test.rb | 16 ++- test/system/unfurling_links_test.rb | 112 ++++++++++++++++++ 4 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 test/system/unfurling_links_test.rb diff --git a/app/javascript/helpers/string_helpers.js b/app/javascript/helpers/string_helpers.js index c7104d4..c21689b 100644 --- a/app/javascript/helpers/string_helpers.js +++ b/app/javascript/helpers/string_helpers.js @@ -1,3 +1,5 @@ +const HTML_ESCAPES = { "&": "&", "<": "<", ">": ">", "\"": """, "'": "'" } + export function truncateString(string, length, omission = "…") { if (string.length <= length) { return string @@ -5,3 +7,7 @@ export function truncateString(string, length, omission = "…") { return string.slice(0, length - omission.length) + omission } } + +export function escapeHTML(string) { + return String(string).replace(/[&<>"']/g, character => HTML_ESCAPES[character]) +} diff --git a/app/javascript/lib/rich_text/unfurl/lib/opengraph_embed_operation.js b/app/javascript/lib/rich_text/unfurl/lib/opengraph_embed_operation.js index e261fdb..7e976b4 100644 --- a/app/javascript/lib/rich_text/unfurl/lib/opengraph_embed_operation.js +++ b/app/javascript/lib/rich_text/unfurl/lib/opengraph_embed_operation.js @@ -1,5 +1,5 @@ import { post } from "@rails/request.js" -import { truncateString } from "helpers/string_helpers" +import { escapeHTML, truncateString } from "helpers/string_helpers" const UNFURLED_TWITTER_AVATAR_CSS_CLASS = "cf-twitter-avatar" const TWITTER_AVATAR_URL_PREFIX = "https://pbs.twimg.com/profile_images" @@ -68,7 +68,7 @@ export default class OpengraphEmbedOperation {
${truncateString(embed.description, 560)}
- +
` diff --git a/test/controllers/unfurl_links_controller_test.rb b/test/controllers/unfurl_links_controller_test.rb index 0dea633..af04625 100644 --- a/test/controllers/unfurl_links_controller_test.rb +++ b/test/controllers/unfurl_links_controller_test.rb @@ -18,6 +18,18 @@ class UnfurlLinksControllerTest < ActionDispatch::IntegrationTest assert_equal "desc..", json_response["description"] end + test "create strips markup from the title and description" do + entity_encoded_image_tag = "<img src=a onerror=prompt(1)>" + stub_successful_request title: "#{entity_encoded_image_tag}Hey!", description: "#{entity_encoded_image_tag}desc.." + + post unfurl_link_url, params: { url: "https://www.example.com" } + assert_response :success + + json_response = JSON.parse(response.body) + assert_equal "Hey!", json_response["title"] + assert_equal "desc..", json_response["description"] + end + test "create with missing opengraph meta tags" do WebMock.stub_request(:get, "https://www.example.com/").to_return(status: 200, body: "", headers: {}) @@ -49,10 +61,10 @@ class UnfurlLinksControllerTest < ActionDispatch::IntegrationTest end private - def stub_successful_request(url: "https://www.example.com/") + def stub_successful_request(url: "https://www.example.com/", title: "Hey!", description: "desc..") WebMock.stub_request(:get, url).to_return( status: 200, - body: "", + body: "", headers: { content_type: "text/html" } ) diff --git a/test/system/unfurling_links_test.rb b/test/system/unfurling_links_test.rb new file mode 100644 index 0000000..adcd43a --- /dev/null +++ b/test/system/unfurling_links_test.rb @@ -0,0 +1,112 @@ +require "application_system_test_case" +require "socket" + +class UnfurlingLinksTest < ApplicationSystemTestCase + setup do + @website = Website.new + @website.start + RestrictedHTTP::PrivateNetworkGuard.stubs(:resolve).returns("127.0.0.1") + + sign_in "jz@37signals.com" + join_room rooms(:designers) + end + + teardown do + @website.stop + end + + test "a quote in the opengraph image URL cannot add attributes to the preview" do + paste_into_composer @website.page_url + + assert_selector "trix-editor .og-embed__title", text: "A normal looking link" + + assert_equal @website.image_url, preview_image_attributes["src"] + assert_equal %w[ class src ], preview_image_attributes.keys.sort + end + + private + def paste_into_composer(url) + page.execute_script(<<~JS, url) + const editor = document.querySelector("trix-editor") + editor.focus() + + const clipboardData = new DataTransfer() + clipboardData.setData("text/plain", arguments[0]) + editor.dispatchEvent(new ClipboardEvent("paste", { clipboardData, bubbles: true, cancelable: true })) + JS + end + + def preview_image_attributes + page.evaluate_script(<<~JS) + Object.fromEntries(Array.from(document.querySelector("trix-editor .og-embed__image img").attributes, attribute => [ attribute.name, attribute.value ])) + JS + end + + # Serves a page whose og:image URL carries a double quote, so an unescaped + # preview closes the src attribute early and takes the rest as attributes. + class Website + def start + @socket = TCPServer.new("127.0.0.1", 0) + @thread = Thread.new { serve } + end + + def stop + @thread&.kill + @socket&.close + end + + def page_url + "http://127.0.0.1:#{port}/page.html" + end + + def image_url + %(http://127.0.0.1:#{port}/image.png?from=" style="outline:9px solid red) + end + + private + def port + @socket.addr[1] + end + + def serve + loop do + client = @socket.accept + Thread.new(client) { |connection| respond_to(connection) } + end + rescue IOError, Errno::EBADF + nil + end + + def respond_to(client) + request_line = client.gets.to_s + nil while (line = client.gets) && line != "\r\n" + + method, path = request_line.split(" ") + + if path.to_s.start_with?("/image.png") + respond client, "image/png", method == "HEAD" ? "" : "not really a PNG" + else + respond client, "text/html", page + end + rescue IOError, Errno::ECONNRESET + nil + ensure + client.close rescue nil + end + + def respond(client, content_type, body) + client.write "HTTP/1.1 200 OK\r\nContent-Type: #{content_type}\r\nContent-Length: #{body.bytesize}\r\nConnection: close\r\n\r\n#{body}" + end + + def page + <<~HTML + + + + + + Hello + HTML + end + end +end