From 924352403553dcfcc569876ca76c219493fac9d6 Mon Sep 17 00:00:00 2001
From: Adam Cooke <adam@krystal.io>
Date: Fri, 24 Apr 2026 22:55:46 +0100
Subject: [PATCH] refactor(helpers): escape interpolated values in select
 options

The endpoint and domain option helpers interpolated model attributes
straight into an HTML string before marking the whole buffer html_safe.
Wrap the interpolations in h() so untrusted attributes can't break out
of the surrounding tag.

Also stop the helpers glob in rails_helper from eagerly requiring
_spec.rb files so helper specs can live under spec/helpers/, and add a
small application helper spec covering the escape behaviour.
---
 app/helpers/application_helper.rb       | 12 ++++----
 spec/helpers/application_helper_spec.rb | 37 +++++++++++++++++++++++++
 spec/rails_helper.rb                    |  2 +-
 3 files changed, 44 insertions(+), 7 deletions(-)
 create mode 100644 spec/helpers/application_helper_spec.rb

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 52106ad..17d1242 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -33,7 +33,7 @@ module ApplicationHelper
         s << "<optgroup label='Server Domains'>"
         server_domains.each do |domain|
           selected = domain == selected_domain ? "selected='selected'" : ""
-          s << "<option value='#{domain.id}' #{selected}>#{domain.name}</option>"
+          s << "<option value='#{h(domain.id)}' #{selected}>#{h(domain.name)}</option>"
         end
         s << "</optgroup>"
       end
@@ -43,7 +43,7 @@ module ApplicationHelper
         s << "<optgroup label='Organization Domains'>"
         organization_domains.each do |domain|
           selected = domain == selected_domain ? "selected='selected'" : ""
-          s << "<option value='#{domain.id}' #{selected}>#{domain.name}</option>"
+          s << "<option value='#{h(domain.id)}' #{selected}>#{h(domain.name)}</option>"
         end
         s << "</optgroup>"
       end
@@ -60,7 +60,7 @@ module ApplicationHelper
         http_endpoints.each do |endpoint|
           value = "#{endpoint.class}##{endpoint.uuid}"
           selected = value == selected_value ? "selected='selected'" : ""
-          s << "<option value='#{value}' #{selected}>#{endpoint.description}</option>"
+          s << "<option value='#{h(value)}' #{selected}>#{h(endpoint.description)}</option>"
         end
         s << "</optgroup>"
       end
@@ -71,7 +71,7 @@ module ApplicationHelper
         smtp_endpoints.each do |endpoint|
           value = "#{endpoint.class}##{endpoint.uuid}"
           selected = value == selected_value ? "selected='selected'" : ""
-          s << "<option value='#{value}' #{selected}>#{endpoint.description}</option>"
+          s << "<option value='#{h(value)}' #{selected}>#{h(endpoint.description)}</option>"
         end
         s << "</optgroup>"
       end
@@ -82,7 +82,7 @@ module ApplicationHelper
         address_endpoints.each do |endpoint|
           value = "#{endpoint.class}##{endpoint.uuid}"
           selected = value == selected_value ? "selected='selected'" : ""
-          s << "<option value='#{value}' #{selected}>#{endpoint.address}</option>"
+          s << "<option value='#{h(value)}' #{selected}>#{h(endpoint.address)}</option>"
         end
         s << "</optgroup>"
       end
@@ -94,7 +94,7 @@ module ApplicationHelper
 
           selected = (selected_value == mode ? "selected='selected'" : "")
           text = t("route_modes.#{mode.underscore}")
-          s << "<option value='#{mode}' #{selected}>#{text}</option>"
+          s << "<option value='#{h(mode)}' #{selected}>#{h(text)}</option>"
         end
         s << "</optgroup>"
       end
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
new file mode 100644
index 0000000..4beb3de
--- /dev/null
+++ b/spec/helpers/application_helper_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe ApplicationHelper, type: :helper do
+  describe "#endpoint_options_for_select" do
+    let(:server) { create(:server) }
+
+    context "when an endpoint has HTML characters in its description" do
+      let(:payload) { %q(x'"><script>alert(1)</script>) }
+
+      before do
+        create(:http_endpoint, server: server, name: payload)
+      end
+
+      it "HTML-escapes the endpoint description in the option text" do
+        html = helper.endpoint_options_for_select(server)
+
+        # The raw payload must not appear verbatim — if it does, the browser
+        # will execute the <script> tag.
+        expect(html).not_to include("<script>alert(1)</script>")
+
+        # Escaped form should appear instead.
+        expect(html).to include("&lt;script&gt;alert(1)&lt;/script&gt;")
+      end
+
+      it "does not allow the payload to break out of the option tag" do
+        html = helper.endpoint_options_for_select(server)
+
+        # The ' and > characters in the payload must be escaped so they
+        # cannot close the opening <option value='...'> or terminate the
+        # element early.
+        expect(html).not_to match(/<option[^>]*>[^<]*<script/)
+      end
+    end
+  end
+end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index bf2c9ae..9c22de1 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -16,7 +16,7 @@ require "shoulda-matchers"
 DatabaseCleaner.allow_remote_database_url = true
 ActiveRecord::Base.logger = Logger.new("/dev/null")
 
-Dir[File.expand_path("helpers/**/*.rb", __dir__)].each { |f| require f }
+Dir[File.expand_path("helpers/**/*.rb", __dir__)].reject { |f| f.end_with?("_spec.rb") }.each { |f| require f }
 
 ActionMailer::Base.delivery_method = :test