FIX: Render excerpt HTML for chat replies and edit (#22559)

It is now safe to render the message excerpt as HTML since
it is no longer using text_entities: true in the server
PrettyText.excerpt call when creating the message excerpt
from the cooked HTML.

This will fix the issue of things like mentions showing
HTML code instead of the actual mention when replying,
and cannot be used to inject improper HTML like style tags
via XSS.
This commit is contained in:
Martin Brennan 2023-07-13 09:44:56 +10:00 committed by GitHub
parent bdecd697b9
commit 4ae26bcaac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 56 additions and 8 deletions

View File

@ -8,7 +8,7 @@
<Chat::UserAvatar @user={{@message.user}} />
<span class="chat-reply__username">{{@message.user.username}}</span>
<span class="chat-reply__excerpt">
{{replace-emoji @message.excerpt}}
{{replace-emoji (html-safe @message.excerpt)}}
</span>
</div>

View File

@ -13,7 +13,7 @@
{{/if}}
<span class="chat-reply__excerpt">
{{replace-emoji @message.inReplyTo.excerpt}}
{{replace-emoji (html-safe @message.inReplyTo.excerpt)}}
</span>
</LinkTo>
{{/if}}

View File

@ -24,7 +24,7 @@ RSpec.describe "Chat | composer | channel", type: :system, js: true do
expect(channel_page.composer.message_details).to have_message(
id: message_1.id,
exact_text: "&lt;mark&gt;not marked&lt;/mark&gt;",
exact_text: "<mark>not marked</mark>",
)
end

View File

@ -241,7 +241,17 @@ RSpec.describe "Chat channel", type: :system do
chat.visit_channel(channel_1)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
"&amp;lt;mark&amp;gt;not marked&amp;lt;/mark&amp;gt;",
"&lt;mark&gt;not marked&lt;/mark&gt;",
)
end
it "renders safe HTML like mentions (which are just links) in the reply-to" do
message_2.update!(message: "@#{other_user.username} <mark>not marked</mark>")
message_2.rebake!
chat.visit_channel(channel_1)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
"<a class=\"mention\" href=\"/u/#{other_user.username}\">@#{other_user.username}</a> &lt;mark&gt;not marked&lt;/mark&gt;",
)
end
end

View File

@ -3,7 +3,13 @@
RSpec.describe "Chat composer draft", type: :system do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) }
fab!(:message_1) do
Fabricate(
:chat_message,
chat_channel: channel_1,
message: "This is a message for draft and replies",
)
end
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }

View File

@ -9,7 +9,12 @@ RSpec.describe "Reply to message - channel - drawer", type: :system do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:original_message) do
Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user))
Fabricate(
:chat_message,
chat_channel: channel_1,
user: Fabricate(:user),
message: "This is a message to reply to!",
)
end
before do

View File

@ -9,7 +9,12 @@ RSpec.describe "Reply to message - channel - full page", type: :system do
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:original_message) do
Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user))
Fabricate(
:chat_message,
chat_channel: channel_1,
user: Fabricate(:user),
message: "This is a message to reply to!",
)
end
before do
@ -100,5 +105,22 @@ RSpec.describe "Reply to message - channel - full page", type: :system do
expect(channel_page).to have_message(text: "reply to message")
end
it "renders safe HTML from the original message excerpt" do
other_user = Fabricate(:user)
original_message.update!(message: "@#{other_user.username} <mark>not marked</mark>")
original_message.rebake!
chat_page.visit_channel(channel_1)
channel_page.reply_to(original_message)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
"<a class=\"mention\" href=\"/u/#{other_user.username}\">@#{other_user.username}</a> &lt;mark&gt;not marked&lt;/mark&gt;",
)
channel_page.fill_composer("reply to message")
channel_page.click_send_message
expect(channel_page).to have_message(text: "reply to message")
end
end
end

View File

@ -9,7 +9,12 @@ RSpec.describe "Reply to message - channel - mobile", type: :system, mobile: tru
fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:category_channel) }
fab!(:original_message) do
Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user))
Fabricate(
:chat_message,
chat_channel: channel_1,
user: Fabricate(:user),
message: "This is a message to reply to!",
)
end
before do