From b3839c64f47353dc11ebde5c9f17605c6198bd90 Mon Sep 17 00:00:00 2001
From: Sean Rankine <sean.rankine@digital.cabinet-office.gov.uk>
Date: Fri, 16 Oct 2026 11:27:38 +0100
Subject: [PATCH] Escape page titles once when they are built

set_page_title marked every title as html_safe, including form, group
and user names that people type in. The rendered <title> only came out
escaped because Array#join in page_title returns a plain String, which
drops the html_safe flag before ERB outputs it. Anything reading
content_for(:title) directly would get the raw markup.

Let content_for escape the title when it is stored, and use safe_join
in page_title so the result is not escaped a second time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 app/helpers/application_helper.rb       |  4 ++--
 spec/helpers/application_helper_spec.rb | 20 +++++++++++++++++---
 spec/requests/groups_controller_spec.rb | 12 ++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index b3e49261fa..e37d080523 100644
--- a/app/helpers/application_helper.rb
+++ b/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)
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 4c3ddbf59d..87a8e84311 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/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
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index f638093fce..cb2c9a0195 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/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