Escape page titles once when they are built

app/helpers/application_helper.rb

@@ -2,11 +2,11 @@
 
 module ApplicationHelper
   def page_title
-    [content_for(:title), "GOV.UK Forms"].compact.join(I18n.t("page_titles.separator"))
+    safe_join([content_for(:title), "GOV.UK Forms"].compact, I18n.t("page_titles.separator"))
   end
 
   def set_page_title(title)
-    content_for(:title, title.html_safe)
+    content_for(:title, title)
   end
 
   def title_with_error_prefix(title, error)

spec/helpers/application_helper_spec.rb

@@ -371,7 +371,7 @@ def preview_link(form, mode: :preview_draft, locale: :en)
 
       helper.set_page_title(title)
 
-      expect(helper).to have_received(:content_for).with(:title, title.html_safe)
+      expect(helper).to have_received(:content_for).with(:title, title)
     end
   end
 
@@ -383,8 +383,22 @@ def preview_link(form, mode: :preview_draft, locale: :en)
         helper.set_page_title(title)
       end
 
-      it "renders the special characters correctly" do
-        expect(helper.page_title).to eq "#{title}#{I18n.t('page_titles.separator')}GOV.UK Forms"
+      it "escapes the special characters once" do
+        expect(helper.page_title).to eq "#{ERB::Util.html_escape(title)}#{I18n.t('page_titles.separator')}GOV.UK Forms"
+      end
+
+      it "returns an html safe string" do
+        expect(helper.page_title).to be_html_safe
+      end
+    end
+
+    context "when the title contains a closing title tag" do
+      before do
+        helper.set_page_title("</title><script>alert(1)</script>")
+      end
+
+      it "does not include the unescaped markup" do
+        expect(helper.content_for(:title)).to eq "&lt;/title&gt;&lt;script&gt;alert(1)&lt;/script&gt;"
       end
     end
   end

spec/requests/groups_controller_spec.rb

@@ -197,6 +197,18 @@
         expect(response).to have_http_status :not_found
       end
     end
+
+    context "when the group name contains markup" do
+      before do
+        member_group.update!(name: "</title><script>alert(1)</script>")
+      end
+
+      it "escapes the group name in the page title" do
+        get group_url(member_group)
+        expect(response.body).to include("<title>&lt;/title&gt;&lt;script&gt;alert(1)&lt;/script&gt;#{I18n.t('page_titles.separator')}GOV.UK Forms</title>")
+        expect(response.body).not_to include("<script>alert(1)</script>")
+      end
+    end
   end
 
   describe "GET /new" do