From 0e2c7f7d4d887f5f6fd261298e6494ffc74f2d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanko=20Krtali=C4=87?= Date: Tue, 16 Sep 2025 11:52:15 +0200 Subject: [PATCH] Escape HTML everywhere where html_safe is used (#1114) * Escape HTML everywhere where html_safe is used * Mark search results as HTML safe at the attribute --- app/helpers/events_helper.rb | 32 +++++++++---------- app/models/search/result.rb | 24 ++++++++++++++ app/views/cards/display/common/_meta.html.erb | 2 +- .../cards/display/preview/_meta.html.erb | 2 +- .../display/public_preview/_meta.html.erb | 2 +- app/views/searches/_result.html.erb | 6 ++-- 6 files changed, 46 insertions(+), 22 deletions(-) diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 3fd040da0..0eecdb25f 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -30,9 +30,9 @@ module EventsHelper def event_column_title(base_title, count, day) date_tag = local_datetime_tag day, style: :agoorweekday if count > 0 - "#{base_title} #{date_tag} (#{count})".html_safe + "#{h base_title} #{date_tag} (#{h count})".html_safe else - "#{base_title} #{date_tag}".html_safe + "#{h base_title} #{date_tag}".html_safe end end @@ -72,7 +72,7 @@ module EventsHelper end def comment_event_action_sentence(event) - "#{ event_creator_name(event) } commented on #{ event.eventable.card.title }".html_safe + "#{h event_creator_name(event) } commented on #{h event.eventable.card.title }".html_safe end def event_creator_name(event) @@ -86,32 +86,32 @@ module EventsHelper case event.action when "card_assigned" if event.assignees.include?(Current.user) - "#{ event_creator_name(event) } will handle #{ title }".html_safe + "#{h event_creator_name(event) } will handle #{h title }".html_safe else - "#{ event_creator_name(event) } assigned #{ event.assignees.pluck(:name).to_sentence } to #{ title }".html_safe + "#{h event_creator_name(event) } assigned #{h event.assignees.pluck(:name).to_sentence } to #{h title }".html_safe end when "card_unassigned" - "#{ event_creator_name(event) } unassigned #{ event.assignees.include?(Current.user) ? "yourself" : event.assignees.pluck(:name).to_sentence } from #{ title }".html_safe + "#{h event_creator_name(event) } unassigned #{ h(event.assignees.include?(Current.user) ? "yourself" : event.assignees.pluck(:name).to_sentence) } from #{h title }".html_safe when "card_published" - "#{ event_creator_name(event) } added #{ title }".html_safe + "#{h event_creator_name(event) } added #{h title }".html_safe when "card_closed" - "#{ event_creator_name(event) } closed #{ title }".html_safe + "#{h event_creator_name(event) } closed #{h title }".html_safe when "card_reopened" - "#{ event_creator_name(event) } reopened #{ title }".html_safe + "#{h event_creator_name(event) } reopened #{h title }".html_safe when "card_staged" - "#{event_creator_name(event)} moved #{ title } to the #{event.stage_name} stage".html_safe + "#{h event_creator_name(event)} moved #{h title } to the #{h event.stage_name} stage".html_safe when "card_unstaged" - "#{event_creator_name(event)} moved #{ title } out ofthe #{event.stage_name} stage".html_safe + "#{h event_creator_name(event)} moved #{h title } out ofthe #{h event.stage_name} stage".html_safe when "card_due_date_added" - "#{event_creator_name(event)} set the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ title }".html_safe + "#{h event_creator_name(event)} set the date to #{h event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{h title }".html_safe when "card_due_date_changed" - "#{event_creator_name(event)} changed the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ title }".html_safe + "#{h event_creator_name(event)} changed the date to #{h event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{h title }".html_safe when "card_due_date_removed" - "#{event_creator_name(event)} removed the date on #{ title }" + "#{h event_creator_name(event)} removed the date on #{h title }".html_safe when "card_title_changed" - "#{event_creator_name(event)} renamed #{ title } (was: '#{event.particulars.dig('particulars', 'old_title')})'".html_safe + "#{h event_creator_name(event)} renamed #{h title } (was: '#{h event.particulars.dig('particulars', 'old_title')})'".html_safe when "card_collection_changed" - "#{event_creator_name(event)} moved #{ title } to '#{event.particulars.dig('particulars', 'new_collection')}'".html_safe + "#{h event_creator_name(event)} moved #{h title } to '#{h event.particulars.dig('particulars', 'new_collection')}'".html_safe end end diff --git a/app/models/search/result.rb b/app/models/search/result.rb index 16d7e2bdd..28e1f7172 100644 --- a/app/models/search/result.rb +++ b/app/models/search/result.rb @@ -3,6 +3,18 @@ class Search::Result < ApplicationRecord belongs_to :card, foreign_key: :card_id, optional: true belongs_to :comment, foreign_key: :comment_id, optional: true + def card_title + escape_highlight card_title_in_database + end + + def card_description + escape_highlight card_description_in_database + end + + def comment_body + escape_highlight comment_body_in_database + end + def source comment_id.present? ? comment : card end @@ -10,4 +22,16 @@ class Search::Result < ApplicationRecord def readonly? true end + + private + def escape_highlight(html) + if html + CGI.escapeHTML(html) + .gsub(CGI.escapeHTML(Search::HIGHLIGHT_OPENING_MARK), Search::HIGHLIGHT_OPENING_MARK.html_safe) + .gsub(CGI.escapeHTML(Search::HIGHLIGHT_CLOSING_MARK), Search::HIGHLIGHT_CLOSING_MARK.html_safe) + .html_safe + else + nil + end + end end diff --git a/app/views/cards/display/common/_meta.html.erb b/app/views/cards/display/common/_meta.html.erb index 0ee04a99d..77d306198 100644 --- a/app/views/cards/display/common/_meta.html.erb +++ b/app/views/cards/display/common/_meta.html.erb @@ -27,7 +27,7 @@ <%= icon_tag "arrow-right" if card.assignees.any? %> <%= card.assignees.any? ? "Assigned to" : "Not assigned" %> - <%= card.assignees.map { |assignee| "#{assignee.familiar_name}" }.to_sentence.html_safe %> + <%= card.assignees.map { |assignee| "#{h assignee.familiar_name}" }.to_sentence.html_safe %>
diff --git a/app/views/cards/display/preview/_meta.html.erb b/app/views/cards/display/preview/_meta.html.erb index e38616bab..c385caf72 100644 --- a/app/views/cards/display/preview/_meta.html.erb +++ b/app/views/cards/display/preview/_meta.html.erb @@ -19,7 +19,7 @@ <%= icon_tag "arrow-right" if card.assignees.any? %> - <%= card.assignees.map { |assignee| assignee.familiar_name }.to_sentence(two_words_connector: " / ", last_word_connector: " / ").html_safe %> + <%= card.assignees.map { |assignee| h assignee.familiar_name }.to_sentence(two_words_connector: " / ", last_word_connector: " / ").html_safe %>
diff --git a/app/views/cards/display/public_preview/_meta.html.erb b/app/views/cards/display/public_preview/_meta.html.erb index d47bd1eff..479f11432 100644 --- a/app/views/cards/display/public_preview/_meta.html.erb +++ b/app/views/cards/display/public_preview/_meta.html.erb @@ -21,7 +21,7 @@ <%= "Assigned to" if card.assignees.any? %> - <%= card.assignees.map { |assignee| "#{assignee.familiar_name}" }.to_sentence.html_safe %> + <%= card.assignees.map { |assignee| "#{h assignee.familiar_name}" }.to_sentence.html_safe %>
diff --git a/app/views/searches/_result.html.erb b/app/views/searches/_result.html.erb index 6fa042d34..c4caad8f9 100644 --- a/app/views/searches/_result.html.erb +++ b/app/views/searches/_result.html.erb @@ -2,19 +2,19 @@ <%= link_to result.source, class: "search__result", data: { turbo_frame: "_top" } do %>

- # <%= result.card_id %> <%= result.card_title&.html_safe %> + # <%= result.card_id %> <%= result.card_title %>

<% if result.card_description.present? %>
- <%= result.card_description.html_safe %> + <%= result.card_description %>
<% else if result.comment_body.present? %>
<%= avatar_preview_tag result.creator %>
- <%= result.comment_body.html_safe %> + <%= result.comment_body %>
<% end %>