From 00eee298379f1a538d9e0c1d3650d7e9910e0a7d Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Thu, 4 Dec 2025 13:24:55 -0500
Subject: [PATCH] Validate Identity email address

using the "standard" email regexp URI::MailTo::EMAIL_REGEXP. The form
field will validate this in the browser, but if bots are creating
identities, they can put whatever they want in here. So let's add some
protection against that.

The HtmlHelper regex was renamed here to avoid confusing Brakeman,
which does imprecise constant lookup and was confusing the two
constants, one of which uses `\A` and `\z` and the other does
not (intentionally).

ref: https://app.fizzy.do/5986089/cards/3276
---
 app/helpers/html_helper.rb                  |  4 ++--
 app/models/identity.rb                      |  1 +
 test/controllers/signups_controller_test.rb | 12 ++++++++++++
 test/models/identity_test.rb                | 17 +++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/app/helpers/html_helper.rb b/app/helpers/html_helper.rb
index 82a18722d..8998ffcb9 100644
--- a/app/helpers/html_helper.rb
+++ b/app/helpers/html_helper.rb
@@ -9,7 +9,7 @@ module HtmlHelper
 
   private
     EXCLUDED_ELEMENTS = %w[ a figcaption pre code ]
-    EMAIL_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/
+    EMAIL_AUTOLINK_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/
     URL_REGEXP = URI::DEFAULT_PARSER.make_regexp(%w[http https])
 
     def auto_link(fragment)
@@ -48,7 +48,7 @@ module HtmlHelper
     end
 
     def auto_link_emails(text)
-      text.gsub!(EMAIL_REGEXP) do |match|
+      text.gsub!(EMAIL_AUTOLINK_REGEXP) do |match|
         %(<a href="mailto:#{match}">#{match}</a>)
       end
     end
diff --git a/app/models/identity.rb b/app/models/identity.rb
index 135fdaae7..ee4a323b2 100644
--- a/app/models/identity.rb
+++ b/app/models/identity.rb
@@ -10,6 +10,7 @@ class Identity < ApplicationRecord
 
   before_destroy :deactivate_users
 
+  validates :email_address, format: { with: URI::MailTo::EMAIL_REGEXP }
   normalizes :email_address, with: ->(value) { value.strip.downcase.presence }
 
   def send_magic_link(**attributes)
diff --git a/test/controllers/signups_controller_test.rb b/test/controllers/signups_controller_test.rb
index 9d65c7a55..83fea8882 100644
--- a/test/controllers/signups_controller_test.rb
+++ b/test/controllers/signups_controller_test.rb
@@ -34,6 +34,18 @@ class SignupsControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "create with email address containing blanks" do
+    untenanted do
+      assert_no_difference -> { Identity.count } do
+        assert_no_difference -> { MagicLink.count } do
+          post signup_path, params: { signup: { email_address: "sam smith@example.com" } }
+        end
+      end
+
+      assert_response :unprocessable_entity
+    end
+  end
+
   test "create for an authenticated user" do
     identity = identities(:kevin)
     sign_in_as identity
diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb
index 2e7687169..1c6c6b258 100644
--- a/test/models/identity_test.rb
+++ b/test/models/identity_test.rb
@@ -13,6 +13,23 @@ class IdentityTest < ActiveSupport::TestCase
     end
   end
 
+  test "email address format validation" do
+    invalid_emails = [
+      "sam smith@example.com",       # space in local part
+      "@example.com",                # missing local part
+      "test@",                       # missing domain
+      "test",                        # missing @ and domain
+      "<script>@example.com",        # angle brackets
+      "test@example.com\nX-Inject:" # newline (header injection attempt)
+    ]
+
+    invalid_emails.each do |email|
+      identity = Identity.new(email_address: email)
+      assert_not identity.valid?, "expected #{email.inspect} to be invalid"
+      assert identity.errors[:email_address].any?, "expected error on email_address for #{email.inspect}"
+    end
+  end
+
   test "join" do
     identity = identities(:david)
     account = accounts(:initech)