From e4ef1785e8f4e70109cb9fee5724da27dce7c6e9 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 18:33:17 -0700
Subject: [PATCH 01/25] Update Brakeman to 8.1.0

(cherry picked from commit b2140cf2aa6c7c8ce200bb6d4053a732d34bbe82)
---
 Gemfile.lock | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 842fcb4c..cbb05388 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -146,7 +146,7 @@ GEM
     bcrypt (3.1.22)
     benchmark (0.5.0)
     bigdecimal (4.1.3)
-    brakeman (8.0.6)
+    brakeman (8.1.0)
       racc
     builder (3.3.0)
     bundler-audit (0.9.3)

From a67ecde390d754f504bbdd58391950a811077ceb Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Sat, 17 Oct 2026 00:46:37 -0700
Subject: [PATCH 02/25] Compare a table message's filtered html as markup

The filters can write the same table with or without newlines between
its rows depending on which tests ran first in the process, so the exact
string comparison failed under some seeds. main compares it as markup.
---
 test/helpers/content_filters_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/helpers/content_filters_test.rb b/test/helpers/content_filters_test.rb
index 11df0237..208c87a7 100644
--- a/test/helpers/content_filters_test.rb
+++ b/test/helpers/content_filters_test.rb
@@ -172,7 +172,7 @@ class ContentFiltersTest < ActionView::TestCase
 
     filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body).to_html
 
-    assert_equal body, filtered
+    assert_dom_equal body, filtered
     assert_match %r{<table>.*<th><p>Name</p></th>.*<td><p>Jason</p></td>}m, message_presentation(message)
   end
 

From 6ba5d2ece3445e117681484fa332ed7934381a5d Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Sat, 17 Oct 2026 00:46:37 -0700
Subject: [PATCH 03/25] Link URLs only in a message's text

auto_link finds URLs and email addresses with regular expressions over
the rendered html, which can't tell an attribute value from text, so it
could put a link inside an attribute. Hand it each text node on its own,
escaped the way the serializer writes text, after the final sanitize, so
links only ever land between tags. Write the result with
preserve_newline so a code block keeps its leading blank lines.

Bump the presentation cache version so nothing rendered the old way is
served again.

Backport of 7ea05e1, 4e29c6e and 9b271d0 from #364. 1.5.2 has no
plain-text fragment cache, so only the message partial's version moves.
---
 app/helpers/content_filters.rb       |  2 +-
 app/helpers/messages_helper.rb       | 28 ++++++++++++++++---
 app/views/messages/_message.html.erb |  2 +-
 test/helpers/messages_helper_test.rb | 41 ++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/app/helpers/content_filters.rb b/app/helpers/content_filters.rb
index 0b9ee145..6c23fcd9 100644
--- a/app/helpers/content_filters.rb
+++ b/app/helpers/content_filters.rb
@@ -5,7 +5,7 @@ module ContentFilters
   #
   #   * SanitizeTags strips disallowed markup from the message body
   #   * Action Text sanitizes the rendered content (lib/rails_ext/action_text_allowed_tags.rb)
-  #   * auto_link re-sanitizes the final html (MessagesHelper#message_presentation)
+  #   * the final html is re-sanitized before its text is linked (MessagesHelper#auto_link_text)
   EDITOR_FORMATTING_TAGS = %w[ s u mark table thead tbody tfoot tr th td ]
   EDITOR_FORMATTING_ATTRIBUTES = %w[ data-language ]
 
diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb
index 066e15e9..d14f60c7 100644
--- a/app/helpers/messages_helper.rb
+++ b/app/helpers/messages_helper.rb
@@ -1,9 +1,12 @@
 module MessagesHelper
-  # auto_link re-sanitizes with Rails' default safe list, which lacks the
-  # formatting the rich text editor produces
+  # Rendered text is re-sanitized before linking. Rails' default safe list
+  # lacks the formatting the rich text editor produces
   AUTO_LINK_ALLOWED_TAGS = Rails::HTML5::SafeListSanitizer.allowed_tags + ContentFilters::EDITOR_FORMATTING_TAGS
   AUTO_LINK_ALLOWED_ATTRIBUTES = Rails::HTML5::SafeListSanitizer.allowed_attributes + ContentFilters::EDITOR_FORMATTING_ATTRIBUTES
 
+  # Text auto_link could turn into a link: a URL has a scheme or www., an email address an @
+  LINKABLE_TEXT = /:|@|www\./i
+
   def message_area_tag(room, &)
     tag.div id: "message-area", class: "message-area", contents: true, data: {
       controller: "messages presence drop-target",
@@ -63,8 +66,7 @@ module MessagesHelper
     when "sound"
       message_sound_presentation(message)
     else
-      auto_link h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body)),
-        html: { target: "_blank" }, sanitize_options: { tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES }
+      auto_link_text h(ContentFilters::TextMessagePresentationFilters.apply(message.body.body))
     end
   rescue Exception => e
     Sentry.capture_exception(e, extra: { message: message })
@@ -74,6 +76,24 @@ module MessagesHelper
   end
 
   private
+    # auto_link finds URLs and email addresses with regular expressions, which
+    # can't tell an attribute value from text. Hand it one text node at a time,
+    # escaped as the serializer writes text, so a link can only land between tags.
+    def auto_link_text(html)
+      fragment = Nokogiri::HTML5.fragment(sanitize(html, tags: AUTO_LINK_ALLOWED_TAGS, attributes: AUTO_LINK_ALLOWED_ATTRIBUTES))
+
+      fragment.xpath(".//text()[not(ancestor::a)]").each do |node|
+        if node.text.match?(LINKABLE_TEXT)
+          text = node.to_html
+          linked = auto_link(text, html: { target: "_blank" })
+          node.replace Nokogiri::HTML5.fragment(linked) unless linked == text
+        end
+      end
+
+      # A <pre> starting with a newline needs an extra one written out to survive the next parse
+      fragment.to_html(preserve_newline: true).html_safe
+    end
+
     def messages_actions
       "turbo:before-stream-render@document->messages#beforeStreamRender keydown.up@document->messages#editMyLastMessage"
     end
diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb
index 0e02bc4d..7d02c825 100644
--- a/app/views/messages/_message.html.erb
+++ b/app/views/messages/_message.html.erb
@@ -1,7 +1,7 @@
 <%# Be sure to check/update messages/_template.html.erb when changing this file %>
 
 <%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
-<% cache [ message, "presentation-v6" ] do %>
+<% cache [ message, "presentation-v7" ] do %>
   <%= message_tag message do %>
     <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
 
diff --git a/test/helpers/messages_helper_test.rb b/test/helpers/messages_helper_test.rb
index 78a97c99..20eaf6a7 100644
--- a/test/helpers/messages_helper_test.rb
+++ b/test/helpers/messages_helper_test.rb
@@ -25,6 +25,47 @@ class MessagesHelperTest < ActionView::TestCase
     assert_match /<strong>bold<\/strong>/, presentation
   end
 
+  test "message_presentation links URLs and email addresses in text" do
+    message = Message.create! room: rooms(:pets), body: "<div>See https://example.com/a?b=1&amp;c=2, or write to me@example.com.</div>", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ "https://example.com/a?b=1&c=2", "mailto:me@example.com" ], links.map { it["href"] }
+    assert_equal [ "_blank", "_blank" ], links.map { it["target"] }
+    assert_equal [ "https://example.com/a?b=1&c=2", "me@example.com" ], links.map(&:text)
+  end
+
+  test "message_presentation links an email address with an apostrophe whole" do
+    message = Message.create! room: rooms(:pets), body: "<div>Write to o'brien@example.com</div>", client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "mailto:o%27brien@example.com", "o'brien@example.com" ] ], links.map { [ it["href"], it.text ] }
+  end
+
+  test "message_presentation leaves URLs and email addresses in attribute values as they are" do
+    [ "class", "title" ].product([ "https://example.com", "www.example.com", "me@example.com" ]).each do |attribute, linkable|
+      value = "a> #{linkable} <em>b</em> "
+      message = Message.create! room: rooms(:pets), body: %(<div><span #{attribute}="#{value}">c</span> https://example.org</div>), client_message_id: "0015", creator: users(:jason)
+
+      presentation = Nokogiri::HTML5.fragment(view.message_presentation(message))
+      assert_equal [ value ], presentation.css("span[#{attribute}]").map { it[attribute] }, "#{attribute} with #{linkable}"
+      assert_empty presentation.css("em"), "#{attribute} with #{linkable}"
+      assert_equal [ "https://example.org" ], presentation.css("a").map { it["href"] }, "#{attribute} with #{linkable}"
+    end
+  end
+
+  test "linking text doesn't eat a code block's leading blank line" do
+    linked = view.send(:auto_link_text, "<pre>\n\n\nfirst https://example.com</pre>".html_safe)
+
+    assert_equal "\nfirst https://example.com", Nokogiri::HTML5.fragment(linked).at_css("pre").text
+  end
+
+  test "message_presentation leaves an existing link's text alone" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="https://example.com/">https://example.org/</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    links = Nokogiri::HTML5.fragment(view.message_presentation(message)).css("a")
+    assert_equal [ [ "https://example.com/", "https://example.org/" ] ], links.map { [ it["href"], it.text ] }
+  end
+
   test "message_presentation shows an image's thumbnail made when it was posted" do
     presentation = view.message_presentation(attachment_message("moon.jpg", "image/jpeg", processed: true))
 

From 049fc920d378e047264374965d06809a68a0d8a3 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 16:59:25 -0700
Subject: [PATCH 04/25] Quote a mention as text when replying

Replying swaps each mention in the quoted copy for its text. Assigning
that text to outerHTML parsed it as html; replace the mention with a
text node instead.

(cherry picked from commit 6052b123d5e2e457a798937647f3da5c26667aa9)
---
 app/javascript/controllers/reply_controller.js |  2 +-
 test/system/composer_test.rb                   | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/javascript/controllers/reply_controller.js b/app/javascript/controllers/reply_controller.js
index a90f73c3..696eafff 100644
--- a/app/javascript/controllers/reply_controller.js
+++ b/app/javascript/controllers/reply_controller.js
@@ -28,7 +28,7 @@ export default class extends Controller {
   }
 
   #stripMentionAttachments(node) {
-    node.querySelectorAll(".mention").forEach(mention => mention.outerHTML = mention.textContent.trim())
+    node.querySelectorAll(".mention").forEach(mention => mention.replaceWith(mention.textContent.trim()))
     return node
   }
 
diff --git a/test/system/composer_test.rb b/test/system/composer_test.rb
index 24ccd12f..01ebf125 100644
--- a/test/system/composer_test.rb
+++ b/test/system/composer_test.rb
@@ -174,6 +174,20 @@ class ComposerTest < ApplicationSystemTestCase
     assert_selector last_message_selector("cite"), text: "JZ"
   end
 
+  test "replying quotes a mention's text as text" do
+    message = rooms(:designers).messages.create! creator: users(:kevin), client_message_id: "reply-mention",
+      body: %(<div><span class="mention">&lt;em&gt;Kevin&lt;/em&gt;</span> says hi</div>)
+    join_room rooms(:designers)
+
+    within_message message do
+      reveal_message_actions
+      find("[aria-label='Reply']").click
+    end
+
+    assert_composer_text "<em>Kevin</em> says hi"
+    assert_no_selector "#composer lexxy-editor em"
+  end
+
   test "arrow up edits my last message when the composer is empty" do
     composer_editor.click
     press_in_composer :up

From 3a140f65b2db20b950e59adb9467d051fbed89bb Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 16:36:19 -0700
Subject: [PATCH 05/25] Serve uploads that aren't media as downloads

Uploaded files come from this origin, so one served with a JavaScript
type could run as one of the app's own scripts. Browsers accept sixteen
spellings of that type, and Disk URLs sign the type they were minted
with, so refusing a list of types at upload can't cover links already
out there. Instead, a middleware rewrites the type of anything Active
Storage reads out that isn't plainly media, a PDF or plain text to
application/octet-stream, whatever the URL says.

(cherry picked from commit cad2d7211dff02488ec591dcf2161c1039ad5dd1)
---
 config/initializers/uploaded_script_guard.rb  | 31 ++++++++++++++++
 test/middleware/uploaded_script_guard_test.rb | 37 +++++++++++++++++++
 2 files changed, 68 insertions(+)
 create mode 100644 config/initializers/uploaded_script_guard.rb
 create mode 100644 test/middleware/uploaded_script_guard_test.rb

diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb
new file mode 100644
index 00000000..18cf59e8
--- /dev/null
+++ b/config/initializers/uploaded_script_guard.rb
@@ -0,0 +1,31 @@
+# Uploaded files are served from this origin, so under script-src 'self' an
+# upload served with any JavaScript MIME type would run as one of our scripts.
+# Browsers accept sixteen spellings of that type, and Disk URLs sign the type
+# they were minted with, so a list of types to refuse can't cover old links.
+# Instead, anything Active Storage serves that isn't plainly media goes out as
+# a download, whatever its URL says. Only reads are touched: uploading
+# (direct upload JSON, Disk PUT) answers in its own types.
+#
+# Accepted on upgrade: a browser that cached an upload under a script type
+# before this guard shipped can still run it from that cache until the entry
+# expires, up to an hour after it was fetched (config/initializers/active_storage.rb).
+# Revalidation doesn't help, since a 304 carries no content type.
+class UploadedScriptGuard
+  SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z}
+
+  def initialize(app)
+    @app = app
+  end
+
+  def call(env)
+    status, headers, body = @app.call(env)
+
+    if env["REQUEST_METHOD"].in?(%w[ GET HEAD ]) && env["PATH_INFO"].start_with?("#{ActiveStorage.routes_prefix}/") && (type = headers["content-type"])
+      headers["content-type"] = ActiveStorage.binary_content_type unless type.split(";").first.strip.downcase.match?(SERVABLE)
+    end
+
+    [ status, headers, body ]
+  end
+end
+
+Rails.application.config.middleware.use UploadedScriptGuard
diff --git a/test/middleware/uploaded_script_guard_test.rb b/test/middleware/uploaded_script_guard_test.rb
new file mode 100644
index 00000000..c627c9ff
--- /dev/null
+++ b/test/middleware/uploaded_script_guard_test.rb
@@ -0,0 +1,37 @@
+require "test_helper"
+
+class UploadedScriptGuardTest < ActiveSupport::TestCase
+  # Every JavaScript MIME type essence in the MIME Sniffing Standard, plus
+  # parameters and casing a signed Disk URL might carry.
+  SCRIPT_TYPES = %w[
+    application/ecmascript application/javascript application/x-ecmascript application/x-javascript
+    text/ecmascript text/javascript text/javascript1.0 text/javascript1.1 text/javascript1.2 text/javascript1.3
+    text/javascript1.4 text/javascript1.5 text/jscript text/livescript text/x-ecmascript text/x-javascript
+  ] + [ "Text/JavaScript; charset=utf-8" ]
+
+  test "uploads never leave with a script type, whatever the URL says" do
+    SCRIPT_TYPES.each do |type|
+      assert_equal ActiveStorage.binary_content_type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/probe.js"), type
+    end
+  end
+
+  test "media and downloads keep their type" do
+    %w[ image/png video/mp4 audio/mpeg application/pdf text/plain ].each do |type|
+      assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/disk/key/file"), type
+    end
+  end
+
+  test "direct upload responses keep their JSON type" do
+    assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST")
+  end
+
+  test "application responses outside uploads are untouched" do
+    assert_equal "text/javascript", served_type("text/javascript", "/assets/application-abc123.js")
+  end
+
+  private
+    def served_type(type, path, method: "GET")
+      app = ->(env) { [ 200, { "content-type" => type }, [ "" ] ] }
+      UploadedScriptGuard.new(app).call(Rack::MockRequest.env_for(path, method: method))[1]["content-type"]
+    end
+end

From c3616f81b47bf129c3ffcd90f46591ab296bf6f9 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 16:36:19 -0700
Subject: [PATCH 06/25] Enforce a script policy that allows the importmap by
 hash

script-src 'self', object-src 'none' and base-uri 'self', enforced. The
two inline scripts the importmap helper emits are allowed by their
SHA-256, and the origins its URLs point to (a configured asset host, in
any form Rails accepts) are allowed by name. There is no per-request
nonce, so cached pages stay valid.

DISABLE_CONTENT_SECURITY_POLICY=true leaves the policy off, for an
install whose proxy or customization injects scripts.

(cherry picked from commit 562888251ed80179cfde9ba97ef35aaa4f8a3749)

Backport: 1.5.2 has no response cache, so the cached-page test is left out.
---
 app/controllers/application_controller.rb     |  2 +-
 .../concerns/inline_script_hashes.rb          | 33 ++++++++
 .../initializers/content_security_policy.rb   | 39 ++++------
 .../content_security_policy_test.rb           | 78 +++++++++++++++++++
 4 files changed, 128 insertions(+), 24 deletions(-)
 create mode 100644 app/controllers/concerns/inline_script_hashes.rb
 create mode 100644 test/integration/content_security_policy_test.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5e60f1e8..0cae28e8 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,4 +1,4 @@
 class ApplicationController < ActionController::Base
-  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders
+  include AllowBrowser, Authentication, Authorization, BlockBannedRequests, SetCurrentRequest, SetPlatform, TrackedRoomVisit, VersionHeaders, InlineScriptHashes
   include Turbo::Streams::Broadcasts, Turbo::Streams::StreamName
 end
diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb
new file mode 100644
index 00000000..c0fde41a
--- /dev/null
+++ b/app/controllers/concerns/inline_script_hashes.rb
@@ -0,0 +1,33 @@
+# Allows the scripts the importmap helper emits: the inline importmap and its
+# entry-point module by content hash, and the origins its URLs point to. The
+# hashes change only when the importmap does (a deploy), so cached HTML stays
+# valid, unlike a per-request nonce.
+module InlineScriptHashes
+  extend ActiveSupport::Concern
+
+  ENTRY_POINT_SCRIPT = %(import "application")
+
+  included do
+    content_security_policy do |policy|
+      if sources = policy.directives["script-src"]
+        importmap = Rails.application.importmap.to_json(resolver: helpers)
+        policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
+      end
+    end
+  end
+
+  private
+    # The importmap is resolved by Rails' asset helpers, so its URLs already
+    # carry any configured asset host, however it's written: with or without a
+    # scheme, protocol-relative, a %d set, or a proc.
+    def script_origins(importmap)
+      JSON.parse(importmap)["imports"].values.filter_map do |url|
+        origin = URI.join(request.base_url, url).origin
+        origin unless origin == request.base_url
+      end.uniq
+    end
+
+    def script_hashes(*scripts)
+      scripts.map { |script| "'sha256-#{Digest::SHA256.base64digest(script)}'" }
+    end
+end
diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb
index 54f47cf1..e3435269 100644
--- a/config/initializers/content_security_policy.rb
+++ b/config/initializers/content_security_policy.rb
@@ -1,25 +1,18 @@
-# Be sure to restart your server when you modify this file.
-
-# Define an application-wide content security policy.
-# See the Securing Rails Applications Guide for more information:
-# https://guides.rubyonrails.org/security.html#content-security-policy-header
-
-# Rails.application.configure do
-#   config.content_security_policy do |policy|
-#     policy.default_src :self, :https
-#     policy.font_src    :self, :https, :data
-#     policy.img_src     :self, :https, :data
-#     policy.object_src  :none
-#     policy.script_src  :self, :https
-#     policy.style_src   :self, :https
-#     # Specify URI for violation reports
-#     # policy.report_uri "/csp-violation-report-endpoint"
-#   end
+# Enforcing baseline policy. Scripts run only from this origin; the two inline
+# scripts the importmap helper emits are allowed by hash (InlineScriptHashes),
+# so there is no per-request nonce and rendered HTML stays cacheable.
 #
-#   # Generate session nonces for permitted importmap and inline scripts
-#   config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s }
-#   config.content_security_policy_nonce_directives = %w(script-src)
+# Nothing else is restricted: images, media, fonts, styles (including an
+# account's custom styles) and connections behave as before.
 #
-#   # Report violations without enforcing the policy.
-#   # config.content_security_policy_report_only = true
-# end
+# DISABLE_CONTENT_SECURITY_POLICY=true is the escape hatch for an install whose
+# proxy or customization injects scripts the policy would block.
+unless ENV["DISABLE_CONTENT_SECURITY_POLICY"] == "true"
+  Rails.application.configure do
+    config.content_security_policy do |policy|
+      policy.script_src :self
+      policy.object_src :none
+      policy.base_uri   :self
+    end
+  end
+end
diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb
new file mode 100644
index 00000000..7e78d380
--- /dev/null
+++ b/test/integration/content_security_policy_test.rb
@@ -0,0 +1,78 @@
+require "test_helper"
+
+class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
+  PAGE_PATH = "/"
+
+  setup { sign_in :david }
+
+  test "enforcing policy allows exactly the inline scripts the page carries" do
+    get root_url
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert policy, "expected an enforcing Content-Security-Policy header"
+    assert_nil response.headers["Content-Security-Policy-Report-Only"]
+    assert_match "object-src 'none'", policy
+    assert_match "base-uri 'self'", policy
+    assert_no_match "unsafe-inline", policy
+    assert_no_match "nonce-", policy
+
+    script_src = policy[/script-src ([^;]+)/, 1].split
+    inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" }
+    assert_operator inline.size, :>=, 2
+    inline.each do |script|
+      assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(script.text)}'", "inline #{script["type"]} script not allowed by hash"
+    end
+  end
+
+  test "scripts may load from a configured asset host, however it's written" do
+    {
+      "https://cdn.example.test" => %r{\Ahttps://cdn\.example\.test\z},
+      "cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "//cdn.example.test" => %r{\Ahttp://cdn\.example\.test\z},
+      "https://assets%d.example.test" => %r{\Ahttps://assets[0-3]\.example\.test\z},
+      ->(source) { "https://one.example.test" } => %r{\Ahttps://one\.example\.test\z},
+      ->(source, request) { "https://#{request.host}.example.test" } => %r{\Ahttps://www\.example\.com\.example\.test\z}
+    }.each do |asset_host, expected_origin|
+      with_asset_host(asset_host) do
+        get PAGE_PATH
+        follow_redirect! while response.redirect?
+        assert_response :success
+
+        script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+        page = Nokogiri::HTML5(response.body)
+        urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values +
+          page.css("link[rel=modulepreload]").map { it["href"] } + page.css("script[src]").map { it["src"] }
+        origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ]
+
+        assert_not_empty origins, asset_host.inspect
+        origins.each do |origin|
+          assert_match expected_origin, origin, asset_host.inspect
+          assert_includes script_src, origin, asset_host.inspect
+        end
+        assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect
+      end
+    end
+  end
+
+  test "the escape hatch boots without a policy" do
+    output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" },
+      [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read)
+
+    assert_equal "nil", output
+  end
+
+  test "uploaded files are guarded on their way out" do
+    assert_includes Rails.application.middleware.map(&:klass), UploadedScriptGuard
+  end
+
+  private
+    def with_asset_host(asset_host)
+      previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.asset_host = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+end

From 05ba551cef1e29392293ff7d3ec448a6afddd17f Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 17:07:38 -0700
Subject: [PATCH 07/25] Say how long a pre-upgrade cached upload can last

Disk URLs are cached for an hour, but blob proxy URLs are cached
publicly and as immutable for a hundred years, so a copy taken that way
before the upgrade doesn't expire.

(cherry picked from commit d713f2631db9b41d039ad20bd3744626d4591df6)
---
 config/initializers/uploaded_script_guard.rb | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb
index 18cf59e8..1ce85503 100644
--- a/config/initializers/uploaded_script_guard.rb
+++ b/config/initializers/uploaded_script_guard.rb
@@ -6,10 +6,13 @@
 # a download, whatever its URL says. Only reads are touched: uploading
 # (direct upload JSON, Disk PUT) answers in its own types.
 #
-# Accepted on upgrade: a browser that cached an upload under a script type
-# before this guard shipped can still run it from that cache until the entry
-# expires, up to an hour after it was fetched (config/initializers/active_storage.rb).
-# Revalidation doesn't help, since a 304 carries no content type.
+# Accepted on upgrade: a cache that stored an upload under a script type
+# before this guard shipped can still serve it as one until the entry expires.
+# For Disk URLs that's up to an hour (config/initializers/active_storage.rb).
+# Blob proxy URLs are cached publicly and as immutable for a hundred years
+# (Rails' http_cache_forever), so a copy a browser or a shared cache took that
+# way doesn't expire. Revalidation doesn't help, since a 304 carries no
+# content type.
 class UploadedScriptGuard
   SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z}
 

From 27f7b8a9b01b8ac0745e17bde04481a3e8898912 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 17:45:56 -0700
Subject: [PATCH 08/25] Keep a multi-range upload response's multipart type

A request for several byte ranges gets a 206 multipart/byteranges whose
boundary the client needs to split the body. Rewriting it to
octet-stream lost the boundary. No browser runs that type as a script,
so it passes through.

(cherry picked from commit 9c37f41ff6790260edc91ea662b4cee91ebede68)
---
 config/initializers/uploaded_script_guard.rb  | 6 ++++--
 test/middleware/uploaded_script_guard_test.rb | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb
index 1ce85503..97eb00ce 100644
--- a/config/initializers/uploaded_script_guard.rb
+++ b/config/initializers/uploaded_script_guard.rb
@@ -3,7 +3,9 @@
 # Browsers accept sixteen spellings of that type, and Disk URLs sign the type
 # they were minted with, so a list of types to refuse can't cover old links.
 # Instead, anything Active Storage serves that isn't plainly media goes out as
-# a download, whatever its URL says. Only reads are touched: uploading
+# a download, whatever its URL says. A multi-range response keeps its
+# multipart/byteranges type and boundary, which no browser runs as a script.
+# Only reads are touched: uploading
 # (direct upload JSON, Disk PUT) answers in its own types.
 #
 # Accepted on upgrade: a cache that stored an upload under a script type
@@ -14,7 +16,7 @@
 # way doesn't expire. Revalidation doesn't help, since a 304 carries no
 # content type.
 class UploadedScriptGuard
-  SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain)\z}
+  SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z}
 
   def initialize(app)
     @app = app
diff --git a/test/middleware/uploaded_script_guard_test.rb b/test/middleware/uploaded_script_guard_test.rb
index c627c9ff..7663c3c4 100644
--- a/test/middleware/uploaded_script_guard_test.rb
+++ b/test/middleware/uploaded_script_guard_test.rb
@@ -21,6 +21,11 @@ class UploadedScriptGuardTest < ActiveSupport::TestCase
     end
   end
 
+  test "multi-range responses keep their boundary" do
+    type = "multipart/byteranges; boundary=AaB03x"
+    assert_equal type, served_type(type, "#{ActiveStorage.routes_prefix}/blobs/proxy/key/movie.mp4")
+  end
+
   test "direct upload responses keep their JSON type" do
     assert_equal "application/json", served_type("application/json", "#{ActiveStorage.routes_prefix}/direct_uploads", method: "POST")
   end

From 8f249cbde49c12984352ca13b9e9ae65b2b5627a Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 18:04:05 -0700
Subject: [PATCH 09/25] Leave a request without a policy when the policy is
 turned off

With DISABLE_CONTENT_SECURITY_POLICY=true there's no application policy,
and the hash callback started an empty one, so pages sent an empty
Content-Security-Policy header. Skip the callback then.

(cherry picked from commit 2a449401ec4e129519e66d59771240db967b84c6)
---
 .../concerns/inline_script_hashes.rb           |  4 +++-
 .../content_security_policy_test.rb            | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb
index c0fde41a..bf2761e8 100644
--- a/app/controllers/concerns/inline_script_hashes.rb
+++ b/app/controllers/concerns/inline_script_hashes.rb
@@ -8,7 +8,9 @@ module InlineScriptHashes
   ENTRY_POINT_SCRIPT = %(import "application")
 
   included do
-    content_security_policy do |policy|
+    # With the policy turned off there's nothing to add to, so leave the
+    # request without one rather than starting an empty policy
+    content_security_policy if: -> { request.content_security_policy } do |policy|
       if sources = policy.directives["script-src"]
         importmap = Rails.application.importmap.to_json(resolver: helpers)
         policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb
index 7e78d380..3f0eac7b 100644
--- a/test/integration/content_security_policy_test.rb
+++ b/test/integration/content_security_policy_test.rb
@@ -55,6 +55,16 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "with the policy turned off, pages send none" do
+    without_policy do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      assert_response :success
+      assert_nil response.headers["Content-Security-Policy"]
+    end
+  end
+
   test "the escape hatch boots without a policy" do
     output = IO.popen({ "DISABLE_CONTENT_SECURITY_POLICY" => "true" },
       [ "bin/rails", "runner", "print Rails.application.config.content_security_policy.inspect" ], chdir: Rails.root, err: File::NULL, &:read)
@@ -67,6 +77,14 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
   end
 
   private
+    # What DISABLE_CONTENT_SECURITY_POLICY=true leaves at boot: no application policy
+    def without_policy
+      previous = Rails.application.env_config.delete("action_dispatch.content_security_policy")
+      yield
+    ensure
+      Rails.application.env_config["action_dispatch.content_security_policy"] = previous
+    end
+
     def with_asset_host(asset_host)
       previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host
       Rails.application.importmap.send(:clear_cache)

From 681bd78ea2acf9b3bdc6b5fad6ecab0a92731670 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 18:42:45 -0700
Subject: [PATCH 10/25] Allow scripts only from the asset directory, not the
 whole origin

Uploads are served from this origin too, so script-src 'self' would run one
that a cache still holds under a script type from before the upload guard.
Application pages now allow each importmap URL up to its asset directory
(relative root and asset host included), the inline importmap by hash, and
the service worker by worker-src.

(cherry picked from commit 2be2227f40dbe31cb0a278cb2d9d31ab2c7ca51c)
---
 .../concerns/inline_script_hashes.rb          | 31 +++++----
 .../initializers/content_security_policy.rb   |  9 ++-
 config/initializers/uploaded_script_guard.rb  | 28 ++++----
 .../content_security_policy_test.rb           | 65 +++++++++++++++++--
 4 files changed, 100 insertions(+), 33 deletions(-)

diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb
index bf2761e8..efc7fdea 100644
--- a/app/controllers/concerns/inline_script_hashes.rb
+++ b/app/controllers/concerns/inline_script_hashes.rb
@@ -1,7 +1,8 @@
-# Allows the scripts the importmap helper emits: the inline importmap and its
-# entry-point module by content hash, and the origins its URLs point to. The
-# hashes change only when the importmap does (a deploy), so cached HTML stays
-# valid, unlike a per-request nonce.
+# Allows the scripts the importmap helper emits and nothing else: the inline
+# importmap and its entry-point module by content hash, and the asset
+# directory its URLs point into. Not the whole origin, because uploads are
+# served from this origin too. The hashes change only when the importmap does
+# (a deploy), so cached HTML stays valid, unlike a per-request nonce.
 module InlineScriptHashes
   extend ActiveSupport::Concern
 
@@ -13,19 +14,27 @@ module InlineScriptHashes
     content_security_policy if: -> { request.content_security_policy } do |policy|
       if sources = policy.directives["script-src"]
         importmap = Rails.application.importmap.to_json(resolver: helpers)
-        policy.directives["script-src"] = sources | script_origins(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
+        policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
+        policy.worker_src service_worker_url(format: :js)
       end
     end
   end
 
   private
     # The importmap is resolved by Rails' asset helpers, so its URLs already
-    # carry any configured asset host, however it's written: with or without a
-    # scheme, protocol-relative, a %d set, or a proc.
-    def script_origins(importmap)
-      JSON.parse(importmap)["imports"].values.filter_map do |url|
-        origin = URI.join(request.base_url, url).origin
-        origin unless origin == request.base_url
+    # carry any relative root and asset host, however the host is written:
+    # with or without a scheme, protocol-relative, a %d set, or a proc. Each
+    # is allowed up to its asset directory; one outside it is allowed exactly.
+    #
+    # A path only limits the first request: browsers check just the origin
+    # after a redirect. Nothing under the asset directory redirects.
+    def script_locations(importmap)
+      assets = File.join(config.relative_url_root.to_s, Rails.application.config.assets.prefix, "")
+
+      JSON.parse(importmap)["imports"].values.map do |url|
+        url = URI.join(request.base_url, url)
+        directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets)
+        "#{url.origin}#{directory || url.path}"
       end.uniq
     end
 
diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb
index e3435269..a99e6485 100644
--- a/config/initializers/content_security_policy.rb
+++ b/config/initializers/content_security_policy.rb
@@ -1,6 +1,9 @@
-# Enforcing baseline policy. Scripts run only from this origin; the two inline
-# scripts the importmap helper emits are allowed by hash (InlineScriptHashes),
-# so there is no per-request nonce and rendered HTML stays cacheable.
+# Enforcing baseline policy. Application pages narrow script-src from this
+# origin to its asset directory, since uploads are served from this origin
+# too, and allow the two inline scripts the importmap helper emits by hash
+# (InlineScriptHashes). There is no per-request nonce, so rendered HTML stays
+# cacheable. 'self' remains for responses outside ApplicationController, none
+# of which render user content.
 #
 # Nothing else is restricted: images, media, fonts, styles (including an
 # account's custom styles) and connections behave as before.
diff --git a/config/initializers/uploaded_script_guard.rb b/config/initializers/uploaded_script_guard.rb
index 97eb00ce..d0badc39 100644
--- a/config/initializers/uploaded_script_guard.rb
+++ b/config/initializers/uploaded_script_guard.rb
@@ -1,20 +1,18 @@
-# Uploaded files are served from this origin, so under script-src 'self' an
-# upload served with any JavaScript MIME type would run as one of our scripts.
-# Browsers accept sixteen spellings of that type, and Disk URLs sign the type
-# they were minted with, so a list of types to refuse can't cover old links.
-# Instead, anything Active Storage serves that isn't plainly media goes out as
-# a download, whatever its URL says. A multi-range response keeps its
+# Uploaded files are served from this origin. Application pages only allow
+# scripts from the asset directory (InlineScriptHashes), so an upload can't
+# load as a script whatever type it's served with. This guard is the second
+# layer: browsers accept sixteen spellings of a JavaScript type, and Disk URLs
+# sign the type they were minted with, so instead of refusing a list of types,
+# anything Active Storage serves that isn't plainly media goes out as a
+# download, whatever its URL says. A multi-range response keeps its
 # multipart/byteranges type and boundary, which no browser runs as a script.
-# Only reads are touched: uploading
-# (direct upload JSON, Disk PUT) answers in its own types.
+# Only reads are touched: uploading (direct upload JSON, Disk PUT) answers in
+# its own types.
 #
-# Accepted on upgrade: a cache that stored an upload under a script type
-# before this guard shipped can still serve it as one until the entry expires.
-# For Disk URLs that's up to an hour (config/initializers/active_storage.rb).
-# Blob proxy URLs are cached publicly and as immutable for a hundred years
-# (Rails' http_cache_forever), so a copy a browser or a shared cache took that
-# way doesn't expire. Revalidation doesn't help, since a 304 carries no
-# content type.
+# A cache that stored an upload under a script type before this shipped can
+# still serve it that way until the entry expires (an hour for Disk URLs, and
+# never for blob proxy URLs, which Rails caches as immutable). The policy's
+# path restriction is what keeps such a copy from running.
 class UploadedScriptGuard
   SERVABLE = %r{\A(image|video|audio)/|\A(application/pdf|application/octet-stream|text/plain|multipart/byteranges)\z}
 
diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb
index 3f0eac7b..67cf66cc 100644
--- a/test/integration/content_security_policy_test.rb
+++ b/test/integration/content_security_policy_test.rb
@@ -16,8 +16,11 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
     assert_match "base-uri 'self'", policy
     assert_no_match "unsafe-inline", policy
     assert_no_match "nonce-", policy
+    assert_equal "http://www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1]
 
     script_src = policy[/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "http://www.example.com/assets/"
     inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" }
     assert_operator inline.size, :>=, 2
     inline.each do |script|
@@ -46,15 +49,43 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
         origins = urls.map { URI.join(request.base_url, it).origin }.uniq - [ request.base_url ]
 
         assert_not_empty origins, asset_host.inspect
-        origins.each do |origin|
-          assert_match expected_origin, origin, asset_host.inspect
-          assert_includes script_src, origin, asset_host.inspect
-        end
+        origins.each { assert_match expected_origin, it, asset_host.inspect }
+        assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }, asset_host.inspect
         assert_includes script_src, "'sha256-#{Digest::SHA256.base64digest(page.at_css("script[type=importmap]").text)}'", asset_host.inspect
       end
     end
   end
 
+  test "scripts are limited to the asset directory under a relative root" do
+    with_relative_url_root("/chat") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+      assert_response :success
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      page = Nokogiri::HTML5(response.body)
+      urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] }
+
+      assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect
+      assert_includes script_src, "http://www.example.com/chat/assets/"
+      assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }
+    end
+  end
+
+  test "uploads can't load as scripts" do
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    blob = ActiveStorage::Blob.create_and_upload!(io: StringIO.new("alert(1)"), filename: "upload.js", content_type: "text/javascript")
+
+    disk_url = ActiveStorage::Current.set(url_options: { protocol: "http", host: "www.example.com" }) { blob.url }
+
+    [ rails_blob_url(blob), rails_blob_url(blob, disposition: :inline), rails_storage_proxy_url(blob), disk_url ].each do |url|
+      assert_not allowed_by_path?(script_src, url), "#{url} allowed by #{script_src.inspect}"
+    end
+  end
+
   test "with the policy turned off, pages send none" do
     without_policy do
       get PAGE_PATH
@@ -85,6 +116,32 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
       Rails.application.env_config["action_dispatch.content_security_policy"] = previous
     end
 
+    # Every page script is allowed by a source with a path, never by a bare origin
+    def assert_scripts_allowed_by_path(script_src, urls, message = nil)
+      assert_empty script_src.grep(%r{\A[a-z]+://[^/]+/?\z}), message
+      urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" }
+    end
+
+    # CSP source matching on a first request: 'self' and a bare origin match any
+    # path, a path ending in / matches as a prefix, any other path exactly
+    def allowed_by_path?(script_src, url)
+      url = URI(url)
+      script_src.any? do |source|
+        source = URI(source == "'self'" ? request.base_url : source) if source == "'self'" || source.match?(%r{\A[a-z]+://})
+        source.is_a?(URI) && source.origin == url.origin &&
+          (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path))
+      end
+    end
+
+    def with_relative_url_root(root)
+      previous, ApplicationController.config.relative_url_root = ApplicationController.config.relative_url_root, root
+      Rails.application.importmap.send(:clear_cache)
+      yield
+    ensure
+      ApplicationController.config.relative_url_root = previous
+      Rails.application.importmap.send(:clear_cache)
+    end
+
     def with_asset_host(asset_host)
       previous, ApplicationController.asset_host = ApplicationController.asset_host, asset_host
       Rails.application.importmap.send(:clear_cache)

From 26b3d2abb079cb9ff6264852fc987d348d5dd034 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 18:59:39 -0700
Subject: [PATCH 11/25] Leave the scheme off this origin's script sources

Behind a proxy that terminates TLS without X-Forwarded-Proto, Rails sees
http while the page is https, and an http source on a non-default port
doesn't match. A source without a scheme takes the page's own.

(cherry picked from commit 0a494a10cbcdd065805c06fcc984aaf966375959)
---
 .../concerns/inline_script_hashes.rb          |  8 ++++--
 .../content_security_policy_test.rb           | 25 ++++++++++++++-----
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb
index efc7fdea..357a70d6 100644
--- a/app/controllers/concerns/inline_script_hashes.rb
+++ b/app/controllers/concerns/inline_script_hashes.rb
@@ -15,7 +15,7 @@ module InlineScriptHashes
       if sources = policy.directives["script-src"]
         importmap = Rails.application.importmap.to_json(resolver: helpers)
         policy.directives["script-src"] = (sources - [ "'self'" ]) | script_locations(importmap) | script_hashes(importmap, ENTRY_POINT_SCRIPT)
-        policy.worker_src service_worker_url(format: :js)
+        policy.worker_src "#{request.host_with_port}#{service_worker_path(format: :js)}"
       end
     end
   end
@@ -26,6 +26,9 @@ module InlineScriptHashes
     # with or without a scheme, protocol-relative, a %d set, or a proc. Each
     # is allowed up to its asset directory; one outside it is allowed exactly.
     #
+    # This origin's own sources leave out the scheme, so they match the page's
+    # scheme even behind a proxy that terminates TLS without saying so.
+    #
     # A path only limits the first request: browsers check just the origin
     # after a redirect. Nothing under the asset directory redirects.
     def script_locations(importmap)
@@ -33,8 +36,9 @@ module InlineScriptHashes
 
       JSON.parse(importmap)["imports"].values.map do |url|
         url = URI.join(request.base_url, url)
+        origin = url.origin == request.base_url ? request.host_with_port : url.origin
         directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets)
-        "#{url.origin}#{directory || url.path}"
+        "#{origin}#{directory || url.path}"
       end.uniq
     end
 
diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb
index 67cf66cc..b4672e03 100644
--- a/test/integration/content_security_policy_test.rb
+++ b/test/integration/content_security_policy_test.rb
@@ -16,11 +16,11 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
     assert_match "base-uri 'self'", policy
     assert_no_match "unsafe-inline", policy
     assert_no_match "nonce-", policy
-    assert_equal "http://www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+    assert_equal "www.example.com/service-worker.js", policy[/worker-src ([^;]+)/, 1]
 
     script_src = policy[/script-src ([^;]+)/, 1].split
     assert_not_includes script_src, "'self'"
-    assert_includes script_src, "http://www.example.com/assets/"
+    assert_includes script_src, "www.example.com/assets/"
     inline = Nokogiri::HTML5(response.body).css("script").reject { |s| s["src"] || s["type"] == "text/template" }
     assert_operator inline.size, :>=, 2
     inline.each do |script|
@@ -67,11 +67,21 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
       urls = JSON.parse(page.at_css("script[type=importmap]").text)["imports"].values + page.css("link[rel=modulepreload]").map { it["href"] }
 
       assert urls.all? { it.start_with?("/chat/assets/") }, urls.inspect
-      assert_includes script_src, "http://www.example.com/chat/assets/"
+      assert_includes script_src, "www.example.com/chat/assets/"
       assert_scripts_allowed_by_path script_src, urls.map { URI.join(request.base_url, it).to_s }
     end
   end
 
+  test "this origin's sources keep a non-default port and take the page's scheme" do
+    host! "www.example.com:8443"
+    get PAGE_PATH
+    follow_redirect! while response.redirect?
+
+    policy = response.headers["Content-Security-Policy"]
+    assert_includes policy[/script-src ([^;]+)/, 1].split, "www.example.com:8443/assets/"
+    assert_equal "www.example.com:8443/service-worker.js", policy[/worker-src ([^;]+)/, 1]
+  end
+
   test "uploads can't load as scripts" do
     get PAGE_PATH
     follow_redirect! while response.redirect?
@@ -118,16 +128,19 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
 
     # Every page script is allowed by a source with a path, never by a bare origin
     def assert_scripts_allowed_by_path(script_src, urls, message = nil)
-      assert_empty script_src.grep(%r{\A[a-z]+://[^/]+/?\z}), message
+      assert_empty script_src.grep(%r{\A([a-z]+://)?[^/']+/?\z}), message
       urls.each { assert allowed_by_path?(script_src, it), "#{it} not allowed by #{script_src.inspect} #{message}" }
     end
 
     # CSP source matching on a first request: 'self' and a bare origin match any
-    # path, a path ending in / matches as a prefix, any other path exactly
+    # path, a path ending in / matches as a prefix, any other path exactly. A
+    # source without a scheme takes the page's.
     def allowed_by_path?(script_src, url)
       url = URI(url)
       script_src.any? do |source|
-        source = URI(source == "'self'" ? request.base_url : source) if source == "'self'" || source.match?(%r{\A[a-z]+://})
+        unless source.start_with?("'") && source != "'self'"
+          source = URI(source == "'self'" ? request.base_url : (source.include?("://") ? source : "#{request.scheme}://#{source}"))
+        end
         source.is_a?(URI) && source.origin == url.origin &&
           (source.path.empty? || (source.path.end_with?("/") ? url.path.start_with?(source.path) : url.path == source.path))
       end

From 7a5931fe2a25a7120b04665d615e30cdd1b551fc Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Thu, 15 Oct 2026 19:13:26 -0700
Subject: [PATCH 12/25] Allow an asset host only up to its own asset directory

An asset host's path can contain the asset directory too, as in
https://cdn.example.com/assets, and cutting each URL at the first one would
allow everything the host serves under it. Cut at the last one instead.

(cherry picked from commit 2ba940c683b5c193d0ec0b3bb2ab3b4f73b60330)
---
 .../concerns/inline_script_hashes.rb          |  6 ++++--
 .../content_security_policy_test.rb           | 21 +++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/app/controllers/concerns/inline_script_hashes.rb b/app/controllers/concerns/inline_script_hashes.rb
index 357a70d6..45772922 100644
--- a/app/controllers/concerns/inline_script_hashes.rb
+++ b/app/controllers/concerns/inline_script_hashes.rb
@@ -24,7 +24,9 @@ module InlineScriptHashes
     # The importmap is resolved by Rails' asset helpers, so its URLs already
     # carry any relative root and asset host, however the host is written:
     # with or without a scheme, protocol-relative, a %d set, or a proc. Each
-    # is allowed up to its asset directory; one outside it is allowed exactly.
+    # is allowed up to its asset directory, the last one in its path, since an
+    # asset host's own path may contain one too; a URL outside it is allowed
+    # exactly.
     #
     # This origin's own sources leave out the scheme, so they match the page's
     # scheme even behind a proxy that terminates TLS without saying so.
@@ -37,7 +39,7 @@ module InlineScriptHashes
       JSON.parse(importmap)["imports"].values.map do |url|
         url = URI.join(request.base_url, url)
         origin = url.origin == request.base_url ? request.host_with_port : url.origin
-        directory = url.path[0, url.path.index(assets) + assets.length] if url.path.include?(assets)
+        directory = url.path[0, url.path.rindex(assets) + assets.length] if url.path.include?(assets)
         "#{origin}#{directory || url.path}"
       end.uniq
     end
diff --git a/test/integration/content_security_policy_test.rb b/test/integration/content_security_policy_test.rb
index b4672e03..7a3aaf68 100644
--- a/test/integration/content_security_policy_test.rb
+++ b/test/integration/content_security_policy_test.rb
@@ -56,6 +56,27 @@ class ContentSecurityPolicyTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "an asset host whose path contains the asset directory is limited to the directory it serves" do
+    with_asset_host("https://cdn.example.test/assets") do
+      get PAGE_PATH
+      follow_redirect! while response.redirect?
+
+      script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+      assert_includes script_src, "https://cdn.example.test/assets/assets/"
+      assert_not allowed_by_path?(script_src, "https://cdn.example.test/assets/rails/active_storage/blobs/proxy/x/upload.js")
+    end
+  end
+
+  test "the sign-in page is limited to the asset directory too" do
+    cookies.delete(:session_token)
+    get new_session_url
+    assert_response :success
+
+    script_src = response.headers["Content-Security-Policy"][/script-src ([^;]+)/, 1].split
+    assert_not_includes script_src, "'self'"
+    assert_includes script_src, "www.example.com/assets/"
+  end
+
   test "scripts are limited to the asset directory under a relative root" do
     with_relative_url_root("/chat") do
       get PAGE_PATH

From d3e6602b88dddea7372506f18c58ea8ca64e33f6 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Sat, 17 Oct 2026 00:48:24 -0700
Subject: [PATCH 13/25] Run CI on pull requests to release branches

Patch releases are prepared on a release-* branch cut from their tag,
and CI only ran on pull requests to main. Pushes to a release branch
stay untriggered, as zizmor treats a push trigger on release branches as
a publishing workflow whose setup-ruby cache could be poisoned.
---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e7da9705..b27c2123 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -4,7 +4,7 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches: [ main ]
+    branches: [ main, "release-*" ]
 
 permissions: {}
 

From a1dd6e1d8ad385e8253d504b7ab94cb2218efd02 Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Fri, 9 Oct 2026 00:30:56 +0200
Subject: [PATCH 14/25] Don't let an http_proxy environment variable bypass the
 pinned address in link previews

Opengraph::Fetch connects to the IP it checked, not to a fresh lookup of
the host. With http_proxy set, Net::HTTP would instead send the hostname
to the proxy, which looks it up again, so the connection could end up
somewhere other than the checked address. Pass nil as the proxy address,
as web push delivery already does.

(cherry picked from commit a7e9ea53dd5ba541f95bee1474b12fc9ac14b5d1)
---
 app/models/opengraph/fetch.rb       |  5 ++++-
 test/models/opengraph/fetch_test.rb | 17 +++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/models/opengraph/fetch.rb b/app/models/opengraph/fetch.rb
index bd628127..2c3a40ff 100644
--- a/app/models/opengraph/fetch.rb
+++ b/app/models/opengraph/fetch.rb
@@ -23,11 +23,14 @@ class Opengraph::Fetch
   end
 
   private
+    # The nil proxy address ignores http_proxy: a proxy would look the host up again, and the connection
+    # would no longer go to the checked IP.
+    #
     # The timeouts bound each operation. A host can still send its headers or body a byte at a time,
     # in as many reads as it likes: UnfurlLinksController puts the whole unfurl under one deadline.
     def request(url, request_class, ip:)
       MAX_REDIRECTS.times do
-        Net::HTTP.start(url.host, url.port, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http|
+        Net::HTTP.start(url.host, url.port, nil, ipaddr: ip, use_ssl: url.scheme == "https", **timeouts) do |http|
           http.request request_class.new(url) do |response|
             if response.is_a?(Net::HTTPRedirection)
               url, ip = resolve_redirect(response["location"])
diff --git a/test/models/opengraph/fetch_test.rb b/test/models/opengraph/fetch_test.rb
index dc510cc5..561d2cf0 100644
--- a/test/models/opengraph/fetch_test.rb
+++ b/test/models/opengraph/fetch_test.rb
@@ -74,6 +74,23 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
     end
   end
 
+  test "#fetch_document connects to the resolved IP even when a proxy is configured" do
+    url = URI.parse("http://www.example.com/")
+    saved = ENV.slice("http_proxy", "HTTP_PROXY")
+    %w[ http_proxy HTTP_PROXY ].each { |k| ENV[k] = "http://proxy.internal:3128" }
+
+    WebMock.disable_net_connect! allow: [ url.host ]
+    TCPSocket.expects(:open).with { |*args, **| args.first == "proxy.internal" }.never
+    TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 80 }.throws(:not_proxied)
+
+    assert_throws :not_proxied do
+      @fetch.fetch_document(url, ip: "1.2.3.4")
+    end
+  ensure
+    %w[ http_proxy HTTP_PROXY ].each { |k| ENV.delete(k) }
+    saved.each { |k, v| ENV[k] = v }
+  end
+
   test "#fetch_document is empty following redirects that never finish" do
     WebMock.stub_request(:get, "https://www.example.com/")
       .to_return(status: 302, headers: { location: "https://www.example.com/" })

From 9474e70f5a42381292f0bdb1990e108d83835649 Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Fri, 9 Oct 2026 00:37:49 +0200
Subject: [PATCH 15/25] Clear no_proxy in the link preview proxy test

(cherry picked from commit 2d4b05a74a165ef69fc07f61ea69ccc10f0e47ac)
---
 test/models/opengraph/fetch_test.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/models/opengraph/fetch_test.rb b/test/models/opengraph/fetch_test.rb
index 561d2cf0..3d4f6d6b 100644
--- a/test/models/opengraph/fetch_test.rb
+++ b/test/models/opengraph/fetch_test.rb
@@ -76,7 +76,8 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
 
   test "#fetch_document connects to the resolved IP even when a proxy is configured" do
     url = URI.parse("http://www.example.com/")
-    saved = ENV.slice("http_proxy", "HTTP_PROXY")
+    saved = ENV.slice("http_proxy", "HTTP_PROXY", "no_proxy", "NO_PROXY")
+    %w[ no_proxy NO_PROXY ].each { |k| ENV.delete(k) }
     %w[ http_proxy HTTP_PROXY ].each { |k| ENV[k] = "http://proxy.internal:3128" }
 
     WebMock.disable_net_connect! allow: [ url.host ]
@@ -87,7 +88,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
       @fetch.fetch_document(url, ip: "1.2.3.4")
     end
   ensure
-    %w[ http_proxy HTTP_PROXY ].each { |k| ENV.delete(k) }
+    %w[ http_proxy HTTP_PROXY no_proxy NO_PROXY ].each { |k| ENV.delete(k) }
     saved.each { |k, v| ENV[k] = v }
   end
 

From c78f03fda7caad5bfbf82b60a419a41b5347d637 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 10:28:06 -0700
Subject: [PATCH 16/25] Sign out the account's other sessions when its password
 changes

Changing the password is how someone shuts out whoever else is signed
in to their account, but every session opened with the old password
stayed valid. Now the others end and their live connections close, as
on sign-out. The session the password was changed from stays signed in.

Changing the password outside a request, as from a console, ends every
session.

From #371. 1.5.2 has no record snapshot or response cache, so the test
that a cached session lookup doesn't outlive the password change is
left out; the rest applies as it is.

(cherry picked from commit c78c3fd7efada9d64d2bb6b3b3b31d1b845266d0)
---
 app/controllers/users/profiles_controller.rb  |  2 +
 app/models/user.rb                            |  7 +++
 .../users/profiles_controller_test.rb         | 46 +++++++++++++++++++
 test/models/user_test.rb                      | 22 +++++++++
 4 files changed, 77 insertions(+)

diff --git a/app/controllers/users/profiles_controller.rb b/app/controllers/users/profiles_controller.rb
index 96015d55..9f3726a7 100644
--- a/app/controllers/users/profiles_controller.rb
+++ b/app/controllers/users/profiles_controller.rb
@@ -8,6 +8,8 @@ class Users::ProfilesController < ApplicationController
 
   def update
     @user.update user_params
+    disconnect_remote_connections if @user.saved_change_to_password_digest?
+
     redirect_to user_profile_url, notice: update_notice
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index df2fee19..449ab02b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -20,6 +20,7 @@ class User < ApplicationRecord
   has_secure_password validations: false
 
   after_create_commit :grant_membership_to_open_rooms
+  after_update :sign_out_other_sessions, if: :saved_change_to_password_digest?
 
   scope :ordered, -> { order("LOWER(name)") }
   scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") }
@@ -50,6 +51,12 @@ class User < ApplicationRecord
   end
 
   private
+    # A new password is how someone shuts out whoever else got into their account, so it ends the sessions opened
+    # with the old one. The session it was changed from, if any, stays.
+    def sign_out_other_sessions
+      sessions.excluding(Current.session).delete_all
+    end
+
     # One statement, which SQLite runs under the write lock, so no room is closed between reading the open ones and
     # inserting. A room that Room#destroy_later closes is either closed before, and skipped, or after, and takes this
     # membership away with the others. A room that granted itself to every user when it was created is skipped too,
diff --git a/test/controllers/users/profiles_controller_test.rb b/test/controllers/users/profiles_controller_test.rb
index 0331aafb..c5f95614 100644
--- a/test/controllers/users/profiles_controller_test.rb
+++ b/test/controllers/users/profiles_controller_test.rb
@@ -27,6 +27,52 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_equal "david@37signals.com", users(:david).email_address
   end
 
+  test "changing the password signs out the account's other sessions but not this one" do
+    other_device = open_session
+    other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
+    other_device.get user_profile_url
+    other_device.assert_response :success
+
+    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    assert_redirected_to user_profile_url
+
+    other_device.get user_profile_url
+    other_device.assert_redirected_to new_session_url
+    get user_profile_url
+    assert_response :success
+    assert_equal 1, users(:david).sessions.count
+  end
+
+  test "changing the password closes the account's live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    put user_profile_url, params: { user: { password: "a new secret123456" } }
+
+    assert_redirected_to user_profile_url
+  end
+
+  test "changing the password still signs out other sessions when the realtime service is unreachable" do
+    other_session = users(:david).sessions.first
+    ActionCable.server.stubs(:remote_connections).raises(RuntimeError.new("cable down"))
+
+    put user_profile_url, params: { user: { password: "a new secret123456" } }
+
+    assert_redirected_to user_profile_url
+    assert_not Session.exists?(other_session.id)
+  end
+
+  test "updating the profile without a new password leaves other sessions signed in" do
+    ActionCable.server.expects(:remote_connections).never
+
+    assert_no_difference -> { users(:david).sessions.count } do
+      put user_profile_url, params: { user: { name: "David", password: "" } }
+    end
+
+    assert_redirected_to user_profile_url
+  end
+
   test "updates are limited to the current user" do
     put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
 
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 61bb1c39..0926adaa 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -42,6 +42,28 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  test "changing the password outside a request signs out every session" do
+    users(:david).sessions.start!(user_agent: "Other device", ip_address: "127.0.0.1")
+
+    assert_changes -> { users(:david).sessions.count }, to: 0 do
+      users(:david).update!(password: "a new secret123456")
+    end
+  end
+
+  test "changing the password keeps the session it was changed from" do
+    Current.session = users(:david).sessions.start!(user_agent: "This device", ip_address: "127.0.0.1")
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_equal [ Current.session ], users(:david).sessions.reload
+  end
+
+  test "changing other details keeps every session" do
+    assert_no_changes -> { users(:david).sessions.count } do
+      users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
+    end
+  end
+
   private
     def create_new_user
       User.create!(name: "User", email_address: "user@example.com", password: "secret123456")

From 2fb599e21c0735eb5c9b975754effec098591b5a Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 10:33:46 -0700
Subject: [PATCH 17/25] Close live connections from the model on any password
 change

The sessions end wherever the password changes, but only the profile
page closed the live connections, so a change from a console left a
signed-out device receiving messages until it dropped. Close them after
commit from User, logging rather than failing if the realtime service
can't be reached, since the password is saved by then.

From #371.

(cherry picked from commit c6e01054916e7f7f68304eaee2c4762c12358722)
---
 app/controllers/users/profiles_controller.rb |  2 --
 app/models/user.rb                           |  9 +++++++++
 test/models/user_test.rb                     | 10 ++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/controllers/users/profiles_controller.rb b/app/controllers/users/profiles_controller.rb
index 9f3726a7..96015d55 100644
--- a/app/controllers/users/profiles_controller.rb
+++ b/app/controllers/users/profiles_controller.rb
@@ -8,8 +8,6 @@ class Users::ProfilesController < ApplicationController
 
   def update
     @user.update user_params
-    disconnect_remote_connections if @user.saved_change_to_password_digest?
-
     redirect_to user_profile_url, notice: update_notice
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 449ab02b..2df933ef 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -21,6 +21,7 @@ class User < ApplicationRecord
 
   after_create_commit :grant_membership_to_open_rooms
   after_update :sign_out_other_sessions, if: :saved_change_to_password_digest?
+  after_update_commit :disconnect_signed_out_sessions, if: :saved_change_to_password_digest?
 
   scope :ordered, -> { order("LOWER(name)") }
   scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") }
@@ -57,6 +58,14 @@ class User < ApplicationRecord
       sessions.excluding(Current.session).delete_all
     end
 
+    # After commit, so a connection that drops can't come back on a session that's still there. The password is
+    # saved by then, so a realtime service that can't be reached mustn't fail the change.
+    def disconnect_signed_out_sessions
+      reset_remote_connections
+    rescue => error
+      Rails.logger.warn "Could not disconnect remote connections after a password change: #{error.class}"
+    end
+
     # One statement, which SQLite runs under the write lock, so no room is closed between reading the open ones and
     # inserting. A room that Room#destroy_later closes is either closed before, and skipped, or after, and takes this
     # membership away with the others. A room that granted itself to every user when it was created is skipped too,
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 0926adaa..22b8109e 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -50,6 +50,14 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  test "changing the password outside a request closes the account's live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    users(:david).update!(password: "a new secret123456")
+  end
+
   test "changing the password keeps the session it was changed from" do
     Current.session = users(:david).sessions.start!(user_agent: "This device", ip_address: "127.0.0.1")
 
@@ -59,6 +67,8 @@ class UserTest < ActiveSupport::TestCase
   end
 
   test "changing other details keeps every session" do
+    ActionCable.server.expects(:remote_connections).never
+
     assert_no_changes -> { users(:david).sessions.count } do
       users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
     end

From b1122c7d6da3c6b040f5c256cd0d2494af2951af Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 10:45:34 -0700
Subject: [PATCH 18/25] Give the changing browser a new session rather than
 keeping its own

Keeping the session the password was changed from also kept any copy of
its cookie working, and a copied cookie is one of the ways someone else
gets into an account. Every session ends now, and the profile page
signs the browser that changed the password back in with a new one.

From #371.

(cherry picked from commit b01bb7a0a76d4dd21dba0018c27cdc0aefed3647)
---
 app/controllers/users/profiles_controller.rb       |  2 ++
 app/models/user.rb                                 | 10 +++++-----
 test/controllers/users/profiles_controller_test.rb | 14 +++++++++++++-
 test/models/user_test.rb                           | 14 +++-----------
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/app/controllers/users/profiles_controller.rb b/app/controllers/users/profiles_controller.rb
index 96015d55..bdf13c47 100644
--- a/app/controllers/users/profiles_controller.rb
+++ b/app/controllers/users/profiles_controller.rb
@@ -8,6 +8,8 @@ class Users::ProfilesController < ApplicationController
 
   def update
     @user.update user_params
+    start_new_session_for @user if @user.saved_change_to_password_digest?
+
     redirect_to user_profile_url, notice: update_notice
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 2df933ef..c9ffee57 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -20,7 +20,7 @@ class User < ApplicationRecord
   has_secure_password validations: false
 
   after_create_commit :grant_membership_to_open_rooms
-  after_update :sign_out_other_sessions, if: :saved_change_to_password_digest?
+  after_update :sign_out_all_sessions, if: :saved_change_to_password_digest?
   after_update_commit :disconnect_signed_out_sessions, if: :saved_change_to_password_digest?
 
   scope :ordered, -> { order("LOWER(name)") }
@@ -52,10 +52,10 @@ class User < ApplicationRecord
   end
 
   private
-    # A new password is how someone shuts out whoever else got into their account, so it ends the sessions opened
-    # with the old one. The session it was changed from, if any, stays.
-    def sign_out_other_sessions
-      sessions.excluding(Current.session).delete_all
+    # A new password is how someone shuts out whoever else got into their account, so it ends every session opened
+    # with the old one, copies of the changing browser's cookie included. The profile page signs that browser back in.
+    def sign_out_all_sessions
+      sessions.delete_all
     end
 
     # After commit, so a connection that drops can't come back on a session that's still there. The password is
diff --git a/test/controllers/users/profiles_controller_test.rb b/test/controllers/users/profiles_controller_test.rb
index c5f95614..6d017260 100644
--- a/test/controllers/users/profiles_controller_test.rb
+++ b/test/controllers/users/profiles_controller_test.rb
@@ -27,7 +27,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_equal "david@37signals.com", users(:david).email_address
   end
 
-  test "changing the password signs out the account's other sessions but not this one" do
+  test "changing the password signs out the account's other devices but not this browser" do
     other_device = open_session
     other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
     other_device.get user_profile_url
@@ -43,6 +43,18 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_equal 1, users(:david).sessions.count
   end
 
+  test "changing the password gives this browser a new session, so a copy of its cookie stops working" do
+    copied_cookie = cookies[:session_token]
+
+    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    assert_not_equal copied_cookie, cookies[:session_token]
+
+    copy = open_session
+    copy.cookies[:session_token] = copied_cookie
+    copy.get user_profile_url
+    copy.assert_redirected_to new_session_url
+  end
+
   test "changing the password closes the account's live connections" do
     remote_connections = mock
     remote_connections.expects(:disconnect).with(reconnect: true)
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 22b8109e..2262dcac 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -42,15 +42,15 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
-  test "changing the password outside a request signs out every session" do
-    users(:david).sessions.start!(user_agent: "Other device", ip_address: "127.0.0.1")
+  test "changing the password signs out every session, the current one included" do
+    Current.session = users(:david).sessions.start!(user_agent: "This device", ip_address: "127.0.0.1")
 
     assert_changes -> { users(:david).sessions.count }, to: 0 do
       users(:david).update!(password: "a new secret123456")
     end
   end
 
-  test "changing the password outside a request closes the account's live connections" do
+  test "changing the password closes the account's live connections" do
     remote_connections = mock
     remote_connections.expects(:disconnect).with(reconnect: true)
     ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
@@ -58,14 +58,6 @@ class UserTest < ActiveSupport::TestCase
     users(:david).update!(password: "a new secret123456")
   end
 
-  test "changing the password keeps the session it was changed from" do
-    Current.session = users(:david).sessions.start!(user_agent: "This device", ip_address: "127.0.0.1")
-
-    users(:david).update!(password: "a new secret123456")
-
-    assert_equal [ Current.session ], users(:david).sessions.reload
-  end
-
   test "changing other details keeps every session" do
     ActionCable.server.expects(:remote_connections).never
 

From 00abff408838aa30b83e9624d826e23e004911fc Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 10:57:14 -0700
Subject: [PATCH 19/25] Close live connections on the transaction that changed
 the password

An after_commit condition reads the record's last save, so another save
of the user in the same transaction skipped the disconnect even though
the sessions were gone. The callback that deletes them now registers
the disconnect on the transaction itself, which also leaves a rolled
back change alone. A password changed along with the avatar is covered
too: the attachment's touch doesn't hide the change.

From #371.

(cherry picked from commit e158388f3d006a7b01163f5fee95f62a252fd517)
---
 app/models/user.rb                            | 10 ++++-----
 .../users/profiles_controller_test.rb         | 13 +++++++++++
 test/models/user_test.rb                      | 22 +++++++++++++++++++
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index c9ffee57..de25de1b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -21,7 +21,6 @@ class User < ApplicationRecord
 
   after_create_commit :grant_membership_to_open_rooms
   after_update :sign_out_all_sessions, if: :saved_change_to_password_digest?
-  after_update_commit :disconnect_signed_out_sessions, if: :saved_change_to_password_digest?
 
   scope :ordered, -> { order("LOWER(name)") }
   scope :filtered_by, ->(query) { where("name like ?", "%#{query}%") }
@@ -52,14 +51,15 @@ class User < ApplicationRecord
   end
 
   private
-    # A new password is how someone shuts out whoever else got into their account, so it ends every session opened
-    # with the old one, copies of the changing browser's cookie included. The profile page signs that browser back in.
+    # A new password is how someone shuts out whoever else got into their account, so it ends every session, copies
+    # of the changing browser's cookie included. Connections close once that commits, registered on the transaction
+    # because a later save in it would replace the saved change an after_commit condition reads.
     def sign_out_all_sessions
       sessions.delete_all
+      self.class.current_transaction.after_commit { disconnect_signed_out_sessions }
     end
 
-    # After commit, so a connection that drops can't come back on a session that's still there. The password is
-    # saved by then, so a realtime service that can't be reached mustn't fail the change.
+    # The password is saved by now, so a realtime service that can't be reached mustn't fail the change.
     def disconnect_signed_out_sessions
       reset_remote_connections
     rescue => error
diff --git a/test/controllers/users/profiles_controller_test.rb b/test/controllers/users/profiles_controller_test.rb
index 6d017260..835166e6 100644
--- a/test/controllers/users/profiles_controller_test.rb
+++ b/test/controllers/users/profiles_controller_test.rb
@@ -55,6 +55,19 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     copy.assert_redirected_to new_session_url
   end
 
+  test "changing the password along with the avatar still signs this browser back in and closes live connections" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    put user_profile_url, params: { user: { password: "a new secret123456", avatar: fixture_file_upload("moon.jpg", "image/jpeg") } }
+    assert_redirected_to user_profile_url
+    assert users(:david).reload.avatar.attached?
+
+    get user_profile_url
+    assert_response :success
+  end
+
   test "changing the password closes the account's live connections" do
     remote_connections = mock
     remote_connections.expects(:disconnect).with(reconnect: true)
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 2262dcac..8dbe0ea4 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -58,6 +58,28 @@ class UserTest < ActiveSupport::TestCase
     users(:david).update!(password: "a new secret123456")
   end
 
+  test "changing the password closes live connections even when the user is saved again before commit" do
+    remote_connections = mock
+    remote_connections.expects(:disconnect).with(reconnect: true)
+    ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
+
+    User.transaction do
+      users(:david).update!(password: "a new secret123456")
+      users(:david).update!(name: "Dave")
+    end
+  end
+
+  test "a password change that rolls back keeps every session and connection" do
+    ActionCable.server.expects(:remote_connections).never
+
+    assert_no_changes -> { users(:david).sessions.count } do
+      User.transaction do
+        users(:david).update!(password: "a new secret123456")
+        raise ActiveRecord::Rollback
+      end
+    end
+  end
+
   test "changing other details keeps every session" do
     ActionCable.server.expects(:remote_connections).never
 

From 58a25c628129fc140452ac8153324b9a9831d8d1 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 11:36:16 -0700
Subject: [PATCH 20/25] End sign-in links made before a password change

A transfer link signed only the user id, so one copied before the
password changed still started a session for up to four hours after.
Bind it to the password salt with generates_token_for, so a new
password ends those links too. Links made before this upgrade stop
working; they last four hours at most.

From #371.

(cherry picked from commit 160ba295b5e24fc77a1b8d9c192b7f8f3f8bc66b)
---
 app/models/user/transferable.rb               | 11 ++++-
 .../sessions/transfers_controller_test.rb     | 11 +++++
 test/models/user/transferable_test.rb         | 40 +++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 test/models/user/transferable_test.rb

diff --git a/app/models/user/transferable.rb b/app/models/user/transferable.rb
index 096913aa..3a9ace04 100644
--- a/app/models/user/transferable.rb
+++ b/app/models/user/transferable.rb
@@ -3,13 +3,20 @@ module User::Transferable
 
   TRANSFER_LINK_EXPIRY_DURATION = 4.hours
 
+  included do
+    # Bound to the password, so changing it also ends sign-in links made before.
+    generates_token_for :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION do
+      password_salt&.last(10)
+    end
+  end
+
   class_methods do
     def find_by_transfer_id(id)
-      find_signed(id, purpose: :transfer)
+      find_by_token_for(:transfer, id)
     end
   end
 
   def transfer_id
-    signed_id(purpose: :transfer, expires_in: TRANSFER_LINK_EXPIRY_DURATION)
+    generate_token_for(:transfer)
   end
 end
diff --git a/test/controllers/sessions/transfers_controller_test.rb b/test/controllers/sessions/transfers_controller_test.rb
index 6efb37f4..18322a88 100644
--- a/test/controllers/sessions/transfers_controller_test.rb
+++ b/test/controllers/sessions/transfers_controller_test.rb
@@ -19,4 +19,15 @@ class Sessions::TransfersControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_url
     assert parsed_cookies.signed[:session_token]
   end
+
+  test "update rejects a link made before the password changed" do
+    user = users(:david)
+    transfer_id = user.transfer_id
+    user.update!(password: "a new secret123456")
+
+    put session_transfer_url(transfer_id)
+
+    assert_response :bad_request
+    assert_nil parsed_cookies.signed[:session_token]
+  end
 end
diff --git a/test/models/user/transferable_test.rb b/test/models/user/transferable_test.rb
new file mode 100644
index 00000000..00ce337e
--- /dev/null
+++ b/test/models/user/transferable_test.rb
@@ -0,0 +1,40 @@
+require "test_helper"
+
+class User::TransferableTest < ActiveSupport::TestCase
+  test "a transfer id finds its user" do
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "a transfer id expires" do
+    transfer_id = users(:david).transfer_id
+
+    travel User::TRANSFER_LINK_EXPIRY_DURATION + 1.second
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id made before a password change stops working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_nil User.find_by_transfer_id(transfer_id)
+    assert_equal users(:david), User.find_by_transfer_id(users(:david).transfer_id)
+  end
+
+  test "other changes keep a transfer id working" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
+
+    assert_equal users(:david), User.find_by_transfer_id(transfer_id)
+  end
+
+  test "a transfer id finds only users in scope" do
+    transfer_id = users(:david).transfer_id
+
+    users(:david).deactivate
+
+    assert_nil User.active.find_by_transfer_id(transfer_id)
+  end
+end

From 417f087964622da543c396dd36a2f16dea7ed8aa Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 12:17:14 -0700
Subject: [PATCH 21/25] Ask for the current password to change it

Changing the password now ends every other session, so whoever holds
one could shut the owner out of their own account with a new password.
The profile asks for the current password whenever the password
changes; without it, or with a wrong one, nothing is saved and the page
says why. Other profile edits don't need it, and the admin
reset-password script still works without it.

Attempts are rate limited like signing in, 10 in 3 minutes per address,
so the current password can't be guessed here instead.

From #371. The cached-response test it touches isn't in 1.5.2.

(cherry picked from commit 44a92ff6c6645c71c1106dc0638ecb8b76b8d8d5)
---
 app/controllers/users/profiles_controller.rb  | 36 ++++++++--
 app/helpers/translations_helper.rb            |  1 +
 app/models/user.rb                            |  8 +++
 app/views/users/profiles/show.html.erb        | 10 +++
 .../users/profiles_controller_test.rb         | 66 +++++++++++++++++--
 test/script/admin/reset_password_test.rb      | 21 ++++++
 6 files changed, 133 insertions(+), 9 deletions(-)
 create mode 100644 test/script/admin/reset_password_test.rb

diff --git a/app/controllers/users/profiles_controller.rb b/app/controllers/users/profiles_controller.rb
index bdf13c47..a6acc994 100644
--- a/app/controllers/users/profiles_controller.rb
+++ b/app/controllers/users/profiles_controller.rb
@@ -1,16 +1,23 @@
 class Users::ProfilesController < ApplicationController
   before_action :set_user
 
+  # Limited like signing in, so the current password can't be guessed here instead.
+  rate_limit to: 10, within: 3.minutes, only: :update, if: :changing_password?,
+    with: -> { redirect_to user_profile_url, alert: "Too many attempts. Try again in a few minutes." }
+
   def show
     @direct_memberships, @shared_memberships =
       Current.user.memberships.with_ordered_room.partition { |m| m.room.direct? }
   end
 
   def update
-    @user.update user_params
-    start_new_session_for @user if @user.saved_change_to_password_digest?
+    if @user.update user_params
+      start_new_session_for @user if @user.saved_change_to_password_digest?
 
-    redirect_to user_profile_url, notice: update_notice
+      redirect_to user_profile_url, notice: update_notice
+    else
+      redirect_to user_profile_url, alert: update_alert
+    end
   end
 
   private
@@ -19,10 +26,31 @@ class Users::ProfilesController < ApplicationController
     end
 
     def user_params
-      params.require(:user).permit(:name, :avatar, :email_address, :password, :bio).compact
+      permitted = params.require(:user).permit(:name, :avatar, :email_address, :password, :bio).compact
+
+      if changing_password?
+        permitted.merge(password_challenge: params[:user][:password_challenge].to_s)
+      else
+        permitted
+      end
+    end
+
+    # As has_secure_password reads it: an empty password leaves the current one alone.
+    def changing_password?
+      !params.dig(:user, :password).to_s.empty?
     end
 
     def update_notice
       params[:user][:avatar] ? "It may take up to 30 minutes to change everywhere." : "✓"
     end
+
+    def update_alert
+      if !@user.errors.include?(:password_challenge)
+        @user.errors.full_messages.to_sentence
+      elsif params[:user][:password_challenge].blank?
+        "Enter your current password to change it."
+      else
+        "That isn't your current password."
+      end
+    end
 end
diff --git a/app/helpers/translations_helper.rb b/app/helpers/translations_helper.rb
index 6142ab49..6c269ab3 100644
--- a/app/helpers/translations_helper.rb
+++ b/app/helpers/translations_helper.rb
@@ -2,6 +2,7 @@ module TranslationsHelper
   TRANSLATIONS = {
     email_address:  { "🇺🇸": "Enter your email address", "🇪🇸": "Introduce tu correo electrónico", "🇫🇷": "Entrez votre adresse courriel", "🇮🇳": "अपना ईमेल पता दर्ज करें", "🇩🇪": "Geben Sie Ihre E-Mail-Adresse ein", "🇧🇷": "Insira seu endereço de email", "🇯🇵": "メールアドレスを入力してください" },
     password: { "🇺🇸": "Enter your password", "🇪🇸": "Introduce tu contraseña", "🇫🇷": "Saisissez votre mot de passe", "🇮🇳": "अपना पासवर्ड दर्ज करें", "🇩🇪": "Geben Sie Ihr Passwort ein", "🇧🇷": "Insira sua senha", "🇯🇵": "パスワードを入力してください" },
+    current_password: { "🇺🇸": "Enter your current password", "🇪🇸": "Introduce tu contraseña actual", "🇫🇷": "Saisissez votre mot de passe actuel", "🇮🇳": "अपना वर्तमान पासवर्ड दर्ज करें", "🇩🇪": "Geben Sie Ihr aktuelles Passwort ein", "🇧🇷": "Insira sua senha atual", "🇯🇵": "現在のパスワードを入力してください" },
     update_password: { "🇺🇸": "Change password", "🇪🇸": "Cambiar contraseña", "🇫🇷": "Changer le mot de passe", "🇮🇳": "पासवर्ड बदलें", "🇩🇪": "Passwort ändern", "🇧🇷": "Alterar senha", "🇯🇵": "パスワードを変更" },
     user_name: { "🇺🇸": "Enter your name", "🇪🇸": "Introduce tu nombre", "🇫🇷": "Entrez votre nom", "🇮🇳": "अपना नाम दर्ज करें", "🇩🇪": "Geben Sie Ihren Namen ein", "🇧🇷": "Insira seu nome", "🇯🇵": "お名前を入力してください" },
     account_name: { "🇺🇸": "Name this account", "🇪🇸": "Nombre de esta cuenta", "🇫🇷": "Nommez ce compte", "🇮🇳": "इस खाते का नाम दें", "🇩🇪": "Benennen Sie dieses Konto", "🇧🇷": "Dê um nome a essa conta", "🇯🇵": "アカウントに名前を付ける" },
diff --git a/app/models/user.rb b/app/models/user.rb
index de25de1b..716088e8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -18,6 +18,7 @@ class User < ApplicationRecord
   enum :status, %i[ active deactivated banned ], default: :active
 
   has_secure_password validations: false
+  validate :password_challenge_matches, unless: -> { password_challenge.nil? }
 
   after_create_commit :grant_membership_to_open_rooms
   after_update :sign_out_all_sessions, if: :saved_change_to_password_digest?
@@ -51,6 +52,13 @@ class User < ApplicationRecord
   end
 
   private
+    # The profile sets password_challenge whenever it changes the password; scripts that reset one don't.
+    def password_challenge_matches
+      unless password_digest_was.present? && BCrypt::Password.new(password_digest_was).is_password?(password_challenge)
+        errors.add :password_challenge, :invalid
+      end
+    end
+
     # A new password is how someone shuts out whoever else got into their account, so it ends every session, copies
     # of the changing browser's cookie included. Connections close once that commits, registered on the transaction
     # because a later save in it would replace the saved change an after_commit condition reads.
diff --git a/app/views/users/profiles/show.html.erb b/app/views/users/profiles/show.html.erb
index 1fe919da..368853d8 100644
--- a/app/views/users/profiles/show.html.erb
+++ b/app/views/users/profiles/show.html.erb
@@ -69,6 +69,16 @@
         </label>
       </div>
 
+      <div class="flex align-center gap">
+        <%= translation_button(:current_password) %>
+
+        <label class="flex align-center gap flex-item-grow input input--actor">
+          <%= form.password_field :password_challenge, class: "input txt-large", autocomplete: "current-password",
+                placeholder: "Current password", required: false, maxlength: 72 %>
+          <%= image_tag "password.svg", aria: { hidden: "true" }, size: 24, class: "colorize--black" %>
+        </label>
+      </div>
+
       <div class="flex align-center gap">
         <%= translation_button(:update_password) %>
 
diff --git a/test/controllers/users/profiles_controller_test.rb b/test/controllers/users/profiles_controller_test.rb
index 835166e6..4e93520c 100644
--- a/test/controllers/users/profiles_controller_test.rb
+++ b/test/controllers/users/profiles_controller_test.rb
@@ -33,7 +33,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     other_device.get user_profile_url
     other_device.assert_response :success
 
-    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
     assert_redirected_to user_profile_url
 
     other_device.get user_profile_url
@@ -46,7 +46,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
   test "changing the password gives this browser a new session, so a copy of its cookie stops working" do
     copied_cookie = cookies[:session_token]
 
-    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
     assert_not_equal copied_cookie, cookies[:session_token]
 
     copy = open_session
@@ -60,7 +60,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     remote_connections.expects(:disconnect).with(reconnect: true)
     ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
 
-    put user_profile_url, params: { user: { password: "a new secret123456", avatar: fixture_file_upload("moon.jpg", "image/jpeg") } }
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456", avatar: fixture_file_upload("moon.jpg", "image/jpeg") } }
     assert_redirected_to user_profile_url
     assert users(:david).reload.avatar.attached?
 
@@ -73,7 +73,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     remote_connections.expects(:disconnect).with(reconnect: true)
     ActionCable.server.stubs(:remote_connections).returns(mock.tap { |m| m.expects(:where).with(current_user: users(:david)).returns(remote_connections) })
 
-    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
 
     assert_redirected_to user_profile_url
   end
@@ -82,7 +82,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     other_session = users(:david).sessions.first
     ActionCable.server.stubs(:remote_connections).raises(RuntimeError.new("cable down"))
 
-    put user_profile_url, params: { user: { password: "a new secret123456" } }
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
 
     assert_redirected_to user_profile_url
     assert_not Session.exists?(other_session.id)
@@ -98,9 +98,65 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to user_profile_url
   end
 
+  test "changing the password with the current one" do
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+
+    assert_redirected_to user_profile_url
+    assert_equal "✓", flash[:notice]
+    assert users(:david).reload.authenticate("a new secret123456")
+  end
+
+  test "changing the password without the current one changes nothing" do
+    assert_password_change_refused "Enter your current password to change it.",
+      user: { name: "Someone else", password: "a new secret123456" }
+  end
+
+  test "changing the password with a wrong current one changes nothing" do
+    assert_password_change_refused "That isn't your current password.",
+      user: { name: "Someone else", password: "a new secret123456", password_challenge: "not my password" }
+  end
+
+  test "editing the profile without changing the password doesn't need the current one" do
+    put user_profile_url, params: { user: { name: "Dave", bio: "Acrobat", password: "" } }
+
+    assert_redirected_to user_profile_url
+    assert_equal "Dave", users(:david).reload.name
+    assert users(:david).authenticate("secret123456")
+  end
+
+  test "attempts to change the password are rate limited like signing in" do
+    Users::ProfilesController.cache_store.stubs(:increment).returns(11)
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_redirected_to user_profile_url
+    assert_equal "Too many attempts. Try again in a few minutes.", flash[:alert]
+    assert users(:david).reload.authenticate("secret123456")
+
+    put user_profile_url, params: { user: { name: "Dave" } }
+    assert_equal "Dave", users(:david).reload.name
+  end
+
   test "updates are limited to the current user" do
     put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
 
     assert_equal "Jason", users(:jason).reload.name
   end
+
+  private
+    def assert_password_change_refused(alert, params)
+      other_device = open_session
+      other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
+      ActionCable.server.expects(:remote_connections).never
+
+      assert_no_difference -> { Session.count } do
+        put user_profile_url, params: params
+      end
+
+      assert_redirected_to user_profile_url
+      assert_equal alert, flash[:alert]
+      assert users(:david).reload.authenticate("secret123456")
+      assert_equal "David", users(:david).name
+      other_device.get user_profile_url
+      other_device.assert_response :success
+    end
 end
diff --git a/test/script/admin/reset_password_test.rb b/test/script/admin/reset_password_test.rb
new file mode 100644
index 00000000..637bb950
--- /dev/null
+++ b/test/script/admin/reset_password_test.rb
@@ -0,0 +1,21 @@
+require "test_helper"
+
+class ResetPasswordScriptTest < ActiveSupport::TestCase
+  test "resets a password without the current one and signs out every session" do
+    assert_output "Password has been reset\n" do
+      run_script "david@37signals.com", "a new secret123456"
+    end
+
+    assert users(:david).reload.authenticate("a new secret123456")
+    assert_empty users(:david).sessions
+  end
+
+  private
+    def run_script(*arguments)
+      original_arguments = ARGV.dup
+      ARGV.replace(arguments)
+      load Rails.root.join("script/admin/reset-password").to_s
+    ensure
+      ARGV.replace(original_arguments)
+    end
+end

From 93718f7408d43d4fc9f0572cf9f81837da87c718 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 12:39:03 -0700
Subject: [PATCH 22/25] Remove push subscriptions on a password change, and
 post them again

A push subscription belongs to the user, not a session, so a device
signed out by a password change kept getting notifications. Changing
the password now removes every subscription along with the sessions,
in the same transaction.

A browser still signed in keeps its own subscription in its service
worker, so the notifications controller posts it once per session: the
bell names the session by a digest of its id, never its token, and the
browser remembers the last one it posted for. That post never
unsubscribes the browser when the server refuses it.

A request that authenticated just before the change could put a
subscription back after it. The create action now checks the session
still exists and saves in one immediate transaction, after validating
the endpoint outside it.

From #371.

(cherry picked from commit 6cab9240978ec6d305d5f043bacda1da800d0d12)
---
 .../users/push_subscriptions_controller.rb    | 27 ++++---
 .../users/push_subscriptions_helper.rb        |  7 ++
 .../controllers/notifications_controller.js   | 21 +++++-
 app/models/user.rb                            |  6 +-
 app/views/rooms/involvements/_bell.html.erb   |  3 +-
 .../users/profiles_controller_test.rb         | 14 +++-
 .../push_subscriptions_controller_test.rb     | 48 +++++++++++++
 test/models/user_test.rb                      | 20 +++++-
 test/system/push_subscription_resync_test.rb  | 71 +++++++++++++++++++
 9 files changed, 201 insertions(+), 16 deletions(-)
 create mode 100644 app/helpers/users/push_subscriptions_helper.rb
 create mode 100644 test/system/push_subscription_resync_test.rb

diff --git a/app/controllers/users/push_subscriptions_controller.rb b/app/controllers/users/push_subscriptions_controller.rb
index 8e263b23..e4e46524 100644
--- a/app/controllers/users/push_subscriptions_controller.rb
+++ b/app/controllers/users/push_subscriptions_controller.rb
@@ -5,17 +5,14 @@ class Users::PushSubscriptionsController < ApplicationController
   end
 
   def create
-    if subscription = @push_subscriptions.find_by(push_subscription_params)
-      # Existing endpoints must pass current validations
-      if subscription.valid?
-        subscription.touch
-        head :ok
-      else
-        head :unprocessable_entity
-      end
+    subscription = @push_subscriptions.find_or_initialize_by(push_subscription_params)
+    subscription.user_agent = request.user_agent if subscription.new_record?
+
+    # Existing endpoints must pass current validations too
+    if subscription.valid?
+      head save_while_signed_in(subscription) ? :ok : :unauthorized
     else
-      subscription = @push_subscriptions.create push_subscription_params.merge(user_agent: request.user_agent)
-      head subscription.persisted? ? :ok : :unprocessable_entity
+      head :unprocessable_entity
     end
   end
 
@@ -29,6 +26,16 @@ class Users::PushSubscriptionsController < ApplicationController
       @push_subscriptions = Current.user.push_subscriptions
     end
 
+    # A password change ends every session and subscription. A request that authenticated just before it mustn't
+    # put one back, so the session is checked under the same write lock as the save. Validation, which looks up
+    # the endpoint's address, runs before taking the lock.
+    def save_while_signed_in(subscription)
+      Push::Subscription.transaction do
+        Session.exists?(Current.session.id) &&
+          (subscription.new_record? ? subscription.save!(validate: false) : subscription.touch)
+      end
+    end
+
     def push_subscription_params
       params.require(:push_subscription).permit(:endpoint, :p256dh_key, :auth_key)
     end
diff --git a/app/helpers/users/push_subscriptions_helper.rb b/app/helpers/users/push_subscriptions_helper.rb
new file mode 100644
index 00000000..57c681f9
--- /dev/null
+++ b/app/helpers/users/push_subscriptions_helper.rb
@@ -0,0 +1,7 @@
+module Users::PushSubscriptionsHelper
+  # Names the session without revealing its token, so the browser can tell when it's in a new one. The creation
+  # time keeps an id reused by a new database from looking like the same session.
+  def push_subscription_session_marker
+    Digest::SHA256.hexdigest("push-subscription-session-#{Current.session.id}-#{Current.session.created_at.to_f}") if Current.session
+  end
+end
diff --git a/app/javascript/controllers/notifications_controller.js b/app/javascript/controllers/notifications_controller.js
index 3ce53a5c..b3698a31 100644
--- a/app/javascript/controllers/notifications_controller.js
+++ b/app/javascript/controllers/notifications_controller.js
@@ -4,8 +4,10 @@ import { pageIsTurboPreview } from "helpers/turbo_helpers"
 import { onNextEventLoopTick } from "helpers/timing_helpers"
 import { getCookie, setCookie } from "lib/cookie"
 
+const PUSH_SUBSCRIPTION_SYNCED_KEY = "push-subscription-synced-session"
+
 export default class extends Controller {
-  static values = { subscriptionsUrl: String }
+  static values = { subscriptionsUrl: String, session: String }
   static targets = [ "notAllowedNotice", "bell", "details" ]
   static classes = [ "attention" ]
 
@@ -25,6 +27,8 @@ export default class extends Controller {
           this.#showBellAlert()
         }
       }
+
+      this.#resyncPushSubscriptionOncePerSession()
     }
   }
 
@@ -109,6 +113,21 @@ export default class extends Controller {
     if (!response.ok) subscription.unsubscribe()
   }
 
+  // A password change removes the account's subscriptions, the one this browser holds included. Post it again
+  // once per session, so a browser that's still signed in keeps getting notifications. A failure here leaves the
+  // browser's own subscription alone: the next session tries again.
+  async #resyncPushSubscriptionOncePerSession() {
+    if (this.#allowed && this.sessionValue && localStorage.getItem(PUSH_SUBSCRIPTION_SYNCED_KEY) !== this.sessionValue) {
+      const registration = await this.#serviceWorkerRegistration
+      const subscription = await registration?.pushManager?.getSubscription()
+
+      if (Notification.permission === "granted" && subscription) {
+        await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" })
+        localStorage.setItem(PUSH_SUBSCRIPTION_SYNCED_KEY, this.sessionValue)
+      }
+    }
+  }
+
   async #requestPermissionAndSubscribe(registration) {
     const permission = await Notification.requestPermission()
     if (permission === "granted") this.#subscribe(registration)
diff --git a/app/models/user.rb b/app/models/user.rb
index 716088e8..07f4bd03 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -60,10 +60,12 @@ class User < ApplicationRecord
     end
 
     # A new password is how someone shuts out whoever else got into their account, so it ends every session, copies
-    # of the changing browser's cookie included. Connections close once that commits, registered on the transaction
-    # because a later save in it would replace the saved change an after_commit condition reads.
+    # of the changing browser's cookie included, and every push subscription; browsers still signed in post theirs
+    # again. Connections close once that commits, registered on the transaction because a later save in it would
+    # replace the saved change an after_commit condition reads.
     def sign_out_all_sessions
       sessions.delete_all
+      push_subscriptions.delete_all
       self.class.current_transaction.after_commit { disconnect_signed_out_sessions }
     end
 
diff --git a/app/views/rooms/involvements/_bell.html.erb b/app/views/rooms/involvements/_bell.html.erb
index d9fa32c5..266bdfb0 100644
--- a/app/views/rooms/involvements/_bell.html.erb
+++ b/app/views/rooms/involvements/_bell.html.erb
@@ -1,6 +1,7 @@
 <span>
   <span class="button_to_change_notifying"
-      data-controller="notifications" data-notifications-subscriptions-url-value="<%= user_push_subscriptions_path %>" data-notifications-attention-class="btn--pulsing">
+      data-controller="notifications" data-notifications-subscriptions-url-value="<%= user_push_subscriptions_path %>"
+      data-notifications-session-value="<%= push_subscription_session_marker %>" data-notifications-attention-class="btn--pulsing">
     <%= turbo_frame_tag dom_id(room, :involvement), data: {
           controller: "turbo-frame", action: "notifications:ready@window->turbo-frame#load", turbo_frame_url_param: room_involvement_path(room) } do %>
       <button class="btn" data-action="click->notifications#attemptToSubscribe" data-notifications-target="bell">
diff --git a/test/controllers/users/profiles_controller_test.rb b/test/controllers/users/profiles_controller_test.rb
index 4e93520c..3d82162a 100644
--- a/test/controllers/users/profiles_controller_test.rb
+++ b/test/controllers/users/profiles_controller_test.rb
@@ -136,6 +136,18 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
     assert_equal "Dave", users(:david).reload.name
   end
 
+  test "changing the password removes the other devices' push subscriptions, and this browser can register its own again" do
+    stub_web_push_dns_resolution
+    endpoint = push_subscriptions(:david_chrome).attributes.slice("endpoint", "p256dh_key", "auth_key")
+
+    put user_profile_url, params: { user: { password: "a new secret123456", password_challenge: "secret123456" } }
+    assert_empty users(:david).push_subscriptions
+
+    post user_push_subscriptions_url, params: { push_subscription: endpoint }
+    assert_response :ok
+    assert_equal [ endpoint["endpoint"] ], users(:david).push_subscriptions.pluck(:endpoint)
+  end
+
   test "updates are limited to the current user" do
     put user_profile_url(users(:jason)), params: { user: { name: "John Doe" } }
 
@@ -148,7 +160,7 @@ class Users::ProfilesControllerTest < ActionDispatch::IntegrationTest
       other_device.post session_url, params: { email_address: users(:david).email_address, password: "secret123456" }
       ActionCable.server.expects(:remote_connections).never
 
-      assert_no_difference -> { Session.count } do
+      assert_no_changes -> { [ Session.count, Push::Subscription.count ] } do
         put user_profile_url, params: params
       end
 
diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb
index e69fc4a2..dd0bcd4e 100644
--- a/test/controllers/users/push_subscriptions_controller_test.rb
+++ b/test/controllers/users/push_subscriptions_controller_test.rb
@@ -74,4 +74,52 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
       assert_redirected_to user_push_subscriptions_url
     end
   end
+
+  test "a signed-out session can't register a subscription" do
+    Session.delete_all
+
+    assert_no_difference -> { Push::Subscription.count } do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_redirected_to new_session_url
+  end
+
+  test "a session signed out while its request is in flight doesn't register a subscription" do
+    Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
+
+    assert_no_difference -> { Push::Subscription.count } do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_response :unauthorized
+  end
+
+  test "a session signed out while its request is in flight doesn't keep an existing subscription alive" do
+    subscription = push_subscriptions(:david_chrome)
+    Push::Subscription.any_instance.stubs(:valid?).with { Session.delete_all; true }.returns(true)
+
+    assert_no_changes -> { subscription.reload.updated_at } do
+      post user_push_subscriptions_url, params: { push_subscription: subscription.attributes.slice("endpoint", "p256dh_key", "auth_key") }
+    end
+
+    assert_response :unauthorized
+  end
+
+  test "a room names its session for the browser to post its subscription again in a new one" do
+    get room_url(rooms(:watercooler))
+    first_marker = css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
+
+    assert_match(/\A\h{64}\z/, first_marker)
+    assert_not_includes response.body, users(:david).sessions.order(:id).last.token
+
+    sign_in :david
+    get room_url(rooms(:watercooler))
+    assert_not_equal first_marker, css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
+  end
+
+  private
+    def new_subscription_params
+      { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
+    end
 end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 8dbe0ea4..5cd445a8 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -80,10 +80,28 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  test "changing the password removes every push subscription of the user" do
+    assert users(:david).push_subscriptions.any?
+
+    users(:david).update!(password: "a new secret123456")
+
+    assert_empty users(:david).push_subscriptions.reload
+    assert users(:jason).push_subscriptions.any?
+  end
+
+  test "a password change that rolls back keeps push subscriptions" do
+    assert_no_changes -> { users(:david).push_subscriptions.count } do
+      User.transaction do
+        users(:david).update!(password: "a new secret123456")
+        raise ActiveRecord::Rollback
+      end
+    end
+  end
+
   test "changing other details keeps every session" do
     ActionCable.server.expects(:remote_connections).never
 
-    assert_no_changes -> { users(:david).sessions.count } do
+    assert_no_changes -> { [ users(:david).sessions.count, users(:david).push_subscriptions.count ] } do
       users(:david).update!(name: "Dave", email_address: "dave@37signals.com")
     end
   end
diff --git a/test/system/push_subscription_resync_test.rb b/test/system/push_subscription_resync_test.rb
new file mode 100644
index 00000000..fab36825
--- /dev/null
+++ b/test/system/push_subscription_resync_test.rb
@@ -0,0 +1,71 @@
+require "application_system_test_case"
+
+class PushSubscriptionResyncTest < ApplicationSystemTestCase
+  ENDPOINT = "https://fcm.googleapis.com/fcm/send/system-test"
+
+  setup do
+    stub_web_push_dns_resolution
+    fake_a_browser_push_subscription
+  end
+
+  teardown do
+    page.execute_script("localStorage.clear()") if page.current_url.start_with?("http")
+  end
+
+  test "a browser posts its subscription again once per session, so it survives a password change" do
+    sign_in "david@37signals.com"
+    assert_subscription_registered
+    first_sync = subscription.updated_at
+
+    join_room rooms(:hq)
+    assert_equal first_sync, subscription.reload.updated_at, "posted again in the same session"
+
+    change_password
+    assert_nil Push::Subscription.find_by(endpoint: ENDPOINT)
+
+    join_room rooms(:hq)
+    assert_subscription_registered
+    assert_not page.evaluate_script("localStorage.getItem('unsubscribed-for-test')")
+  end
+
+  test "a subscription the server refuses stays in the browser" do
+    stub_dns_resolution("169.254.169.254")
+
+    sign_in "david@37signals.com"
+    Timeout.timeout(5) { sleep 0.1 until page.evaluate_script("localStorage.getItem('push-subscription-synced-session')") }
+    join_room rooms(:hq)
+
+    assert_not Push::Subscription.exists?(endpoint: ENDPOINT)
+    assert_not page.evaluate_script("localStorage.getItem('unsubscribed-for-test')")
+  end
+
+  private
+    # Headless Chrome has no push service, so stand in for the subscription the service worker would hold.
+    def fake_a_browser_push_subscription
+      page.driver.browser.execute_cdp "Page.addScriptToEvaluateOnNewDocument", source: <<~JS
+        Object.defineProperty(Notification, "permission", { get: () => "granted" })
+        const subscription = {
+          toJSON: () => ({ endpoint: "#{ENDPOINT}", keys: { p256dh: "p256dh-key", auth: "auth-key" } }),
+          unsubscribe: async () => { localStorage.setItem("unsubscribed-for-test", "true") }
+        }
+        navigator.serviceWorker.getRegistration = async () => ({ pushManager: { getSubscription: async () => subscription } })
+      JS
+    end
+
+    def change_password
+      visit user_profile_url
+      find("input[name='user[password_challenge]']").fill_in with: "secret123456"
+      find("input[name='user[password]']").fill_in with: "a new secret123456"
+      find("input[name='user[password]']").send_keys :enter
+      assert_selector ".flash", text: "✓"
+    end
+
+    def assert_subscription_registered
+      assert_selector "[data-controller~=notifications]", visible: :all
+      Timeout.timeout(5) { sleep 0.1 until Push::Subscription.exists?(endpoint: ENDPOINT, user: users(:david)) }
+    end
+
+    def subscription
+      Push::Subscription.find_by!(endpoint: ENDPOINT)
+    end
+end

From 931f6b2c38d57e597fd9a18f50d3a71a5d0bb5e1 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 12:55:14 -0700
Subject: [PATCH 23/25] Remove the fake push subscription after its system test

The browser is shared across system tests, and a script added to every
new document stays until it's removed, so later tests would have seen
the granted permission and the fake subscription too.

From #371.

(cherry picked from commit 1af62a5bff72392bd10a7692e01479c79f8ddee4)
---
 test/system/push_subscription_resync_test.rb | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/test/system/push_subscription_resync_test.rb b/test/system/push_subscription_resync_test.rb
index fab36825..459b0a64 100644
--- a/test/system/push_subscription_resync_test.rb
+++ b/test/system/push_subscription_resync_test.rb
@@ -9,6 +9,7 @@ class PushSubscriptionResyncTest < ApplicationSystemTestCase
   end
 
   teardown do
+    page.driver.browser.execute_cdp "Page.removeScriptToEvaluateOnNewDocument", identifier: @fake_subscription_script
     page.execute_script("localStorage.clear()") if page.current_url.start_with?("http")
   end
 
@@ -40,15 +41,18 @@ class PushSubscriptionResyncTest < ApplicationSystemTestCase
   end
 
   private
-    # Headless Chrome has no push service, so stand in for the subscription the service worker would hold.
+    # Headless Chrome has no push service, so stand in for the subscription the service worker would hold. The
+    # browser is shared with other system tests, so teardown removes this again.
     def fake_a_browser_push_subscription
-      page.driver.browser.execute_cdp "Page.addScriptToEvaluateOnNewDocument", source: <<~JS
-        Object.defineProperty(Notification, "permission", { get: () => "granted" })
-        const subscription = {
-          toJSON: () => ({ endpoint: "#{ENDPOINT}", keys: { p256dh: "p256dh-key", auth: "auth-key" } }),
-          unsubscribe: async () => { localStorage.setItem("unsubscribed-for-test", "true") }
-        }
-        navigator.serviceWorker.getRegistration = async () => ({ pushManager: { getSubscription: async () => subscription } })
+      @fake_subscription_script = page.driver.browser.execute_cdp("Page.addScriptToEvaluateOnNewDocument", source: <<~JS)["identifier"]
+        (() => {
+          Object.defineProperty(Notification, "permission", { get: () => "granted" })
+          const subscription = {
+            toJSON: () => ({ endpoint: "#{ENDPOINT}", keys: { p256dh: "p256dh-key", auth: "auth-key" } }),
+            unsubscribe: async () => { localStorage.setItem("unsubscribed-for-test", "true") }
+          }
+          navigator.serviceWorker.getRegistration = async () => ({ pushManager: { getSubscription: async () => subscription } })
+        })()
       JS
     end
 

From fadf6f1859ecf8b3e1dc4d74a62c4c68995157bb Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 13:15:10 -0700
Subject: [PATCH 24/25] Post the browser's push subscription from every page,
 and register it once

Only room pages posted the subscription again, so a browser that
changed its password and closed the profile page went without
notifications until it next opened a room. A small controller in the
layout does it now, still once per session, and the bell is back to how
it was.

Two tabs posting at once after a password change could both miss the
row and insert two, so the endpoint is looked up again under the same
write lock as the session check.

From #371.

(cherry picked from commit 3786ee52444935fbf6a4da9f1b45866b34884b31)
---
 .../users/push_subscriptions_controller.rb    | 18 +++++++----
 .../controllers/notifications_controller.js   | 21 +-----------
 .../push_subscription_sync_controller.js      | 32 +++++++++++++++++++
 app/views/layouts/application.html.erb        |  5 +++
 app/views/rooms/involvements/_bell.html.erb   |  3 +-
 .../push_subscriptions_controller_test.rb     | 22 +++++++++++--
 test/system/push_subscription_resync_test.rb  |  7 ++--
 7 files changed, 73 insertions(+), 35 deletions(-)
 create mode 100644 app/javascript/controllers/push_subscription_sync_controller.js

diff --git a/app/controllers/users/push_subscriptions_controller.rb b/app/controllers/users/push_subscriptions_controller.rb
index e4e46524..a3517a83 100644
--- a/app/controllers/users/push_subscriptions_controller.rb
+++ b/app/controllers/users/push_subscriptions_controller.rb
@@ -10,7 +10,7 @@ class Users::PushSubscriptionsController < ApplicationController
 
     # Existing endpoints must pass current validations too
     if subscription.valid?
-      head save_while_signed_in(subscription) ? :ok : :unauthorized
+      head register_while_signed_in(subscription) ? :ok : :unauthorized
     else
       head :unprocessable_entity
     end
@@ -27,12 +27,18 @@ class Users::PushSubscriptionsController < ApplicationController
     end
 
     # A password change ends every session and subscription. A request that authenticated just before it mustn't
-    # put one back, so the session is checked under the same write lock as the save. Validation, which looks up
-    # the endpoint's address, runs before taking the lock.
-    def save_while_signed_in(subscription)
+    # put one back, so the session is checked under the same write lock as the save, and the endpoint is looked up
+    # again there so two tabs posting at once register it once. Validation, which resolves the endpoint's address,
+    # runs before taking the lock.
+    def register_while_signed_in(subscription)
       Push::Subscription.transaction do
-        Session.exists?(Current.session.id) &&
-          (subscription.new_record? ? subscription.save!(validate: false) : subscription.touch)
+        if Session.exists?(Current.session.id)
+          if existing = @push_subscriptions.find_by(push_subscription_params)
+            existing.touch
+          else
+            subscription.save!(validate: false)
+          end
+        end
       end
     end
 
diff --git a/app/javascript/controllers/notifications_controller.js b/app/javascript/controllers/notifications_controller.js
index b3698a31..3ce53a5c 100644
--- a/app/javascript/controllers/notifications_controller.js
+++ b/app/javascript/controllers/notifications_controller.js
@@ -4,10 +4,8 @@ import { pageIsTurboPreview } from "helpers/turbo_helpers"
 import { onNextEventLoopTick } from "helpers/timing_helpers"
 import { getCookie, setCookie } from "lib/cookie"
 
-const PUSH_SUBSCRIPTION_SYNCED_KEY = "push-subscription-synced-session"
-
 export default class extends Controller {
-  static values = { subscriptionsUrl: String, session: String }
+  static values = { subscriptionsUrl: String }
   static targets = [ "notAllowedNotice", "bell", "details" ]
   static classes = [ "attention" ]
 
@@ -27,8 +25,6 @@ export default class extends Controller {
           this.#showBellAlert()
         }
       }
-
-      this.#resyncPushSubscriptionOncePerSession()
     }
   }
 
@@ -113,21 +109,6 @@ export default class extends Controller {
     if (!response.ok) subscription.unsubscribe()
   }
 
-  // A password change removes the account's subscriptions, the one this browser holds included. Post it again
-  // once per session, so a browser that's still signed in keeps getting notifications. A failure here leaves the
-  // browser's own subscription alone: the next session tries again.
-  async #resyncPushSubscriptionOncePerSession() {
-    if (this.#allowed && this.sessionValue && localStorage.getItem(PUSH_SUBSCRIPTION_SYNCED_KEY) !== this.sessionValue) {
-      const registration = await this.#serviceWorkerRegistration
-      const subscription = await registration?.pushManager?.getSubscription()
-
-      if (Notification.permission === "granted" && subscription) {
-        await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" })
-        localStorage.setItem(PUSH_SUBSCRIPTION_SYNCED_KEY, this.sessionValue)
-      }
-    }
-  }
-
   async #requestPermissionAndSubscribe(registration) {
     const permission = await Notification.requestPermission()
     if (permission === "granted") this.#subscribe(registration)
diff --git a/app/javascript/controllers/push_subscription_sync_controller.js b/app/javascript/controllers/push_subscription_sync_controller.js
new file mode 100644
index 00000000..d2636523
--- /dev/null
+++ b/app/javascript/controllers/push_subscription_sync_controller.js
@@ -0,0 +1,32 @@
+import { Controller } from "@hotwired/stimulus"
+import { post } from "@rails/request.js"
+
+const SYNCED_SESSION_KEY = "push-subscription-synced-session"
+
+// A password change removes the account's push subscriptions, the one this browser holds included. Post it again
+// once per session, so a browser that's still signed in keeps getting notifications. A failure here leaves the
+// browser's own subscription alone: the next session tries again.
+export default class extends Controller {
+  static values = { url: String, session: String }
+
+  async connect() {
+    if (this.#allowed && this.sessionValue && localStorage.getItem(SYNCED_SESSION_KEY) !== this.sessionValue) {
+      const registration = await navigator.serviceWorker.getRegistration(window.location.origin)
+      const subscription = await registration?.pushManager?.getSubscription()
+
+      if (Notification.permission === "granted" && subscription) {
+        await post(this.urlValue, { body: this.#payload(subscription), responseKind: "turbo-stream" })
+        localStorage.setItem(SYNCED_SESSION_KEY, this.sessionValue)
+      }
+    }
+  }
+
+  get #allowed() {
+    return navigator.serviceWorker && window.Notification
+  }
+
+  #payload(subscription) {
+    const { endpoint, keys: { p256dh, auth } } = subscription.toJSON()
+    return JSON.stringify({ push_subscription: { endpoint, p256dh_key: p256dh, auth_key: auth } })
+  }
+}
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 23a66c5d..a3c90ec4 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -63,6 +63,11 @@
 
     <%= render "layouts/lightbox" %>
 
+    <% if Current.session %>
+      <%= tag.div hidden: true, data: { controller: "push-subscription-sync",
+            push_subscription_sync_url_value: user_push_subscriptions_path, push_subscription_sync_session_value: push_subscription_session_marker } %>
+    <% end %>
+
     <a href="https://once.com" id="app-logo" target="_blank" aria-label="Once software from 37signals home page">
       <%= image_tag "campfire-icon.png", alt: "Campfire logo", width: 256, height: 216 %>
     </a>
diff --git a/app/views/rooms/involvements/_bell.html.erb b/app/views/rooms/involvements/_bell.html.erb
index 266bdfb0..d9fa32c5 100644
--- a/app/views/rooms/involvements/_bell.html.erb
+++ b/app/views/rooms/involvements/_bell.html.erb
@@ -1,7 +1,6 @@
 <span>
   <span class="button_to_change_notifying"
-      data-controller="notifications" data-notifications-subscriptions-url-value="<%= user_push_subscriptions_path %>"
-      data-notifications-session-value="<%= push_subscription_session_marker %>" data-notifications-attention-class="btn--pulsing">
+      data-controller="notifications" data-notifications-subscriptions-url-value="<%= user_push_subscriptions_path %>" data-notifications-attention-class="btn--pulsing">
     <%= turbo_frame_tag dom_id(room, :involvement), data: {
           controller: "turbo-frame", action: "notifications:ready@window->turbo-frame#load", turbo_frame_url_param: room_involvement_path(room) } do %>
       <button class="btn" data-action="click->notifications#attemptToSubscribe" data-notifications-target="bell">
diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb
index dd0bcd4e..7815626b 100644
--- a/test/controllers/users/push_subscriptions_controller_test.rb
+++ b/test/controllers/users/push_subscriptions_controller_test.rb
@@ -106,16 +106,32 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
     assert_response :unauthorized
   end
 
-  test "a room names its session for the browser to post its subscription again in a new one" do
+  test "pages name their session for the browser to post its subscription again in a new one" do
+    get user_profile_url
+    assert_equal user_push_subscriptions_path, css_select("[data-push-subscription-sync-url-value]").first["data-push-subscription-sync-url-value"]
+
     get room_url(rooms(:watercooler))
-    first_marker = css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
+    first_marker = css_select("[data-push-subscription-sync-session-value]").first["data-push-subscription-sync-session-value"]
 
     assert_match(/\A\h{64}\z/, first_marker)
     assert_not_includes response.body, users(:david).sessions.order(:id).last.token
 
     sign_in :david
     get room_url(rooms(:watercooler))
-    assert_not_equal first_marker, css_select("[data-notifications-session-value]").first["data-notifications-session-value"]
+    assert_not_equal first_marker, css_select("[data-push-subscription-sync-session-value]").first["data-push-subscription-sync-session-value"]
+  end
+
+  test "two posts of the same new subscription at once register it once" do
+    Push::Subscription.any_instance.stubs(:valid?).with do
+      users(:david).push_subscriptions.build(new_subscription_params).save!(validate: false) unless Push::Subscription.exists?(endpoint: new_subscription_params["endpoint"])
+      true
+    end.returns(true)
+
+    assert_difference -> { Push::Subscription.where(endpoint: new_subscription_params["endpoint"]).count }, +1 do
+      post user_push_subscriptions_url, params: { push_subscription: new_subscription_params }
+    end
+
+    assert_response :ok
   end
 
   private
diff --git a/test/system/push_subscription_resync_test.rb b/test/system/push_subscription_resync_test.rb
index 459b0a64..9d66f8f7 100644
--- a/test/system/push_subscription_resync_test.rb
+++ b/test/system/push_subscription_resync_test.rb
@@ -21,11 +21,10 @@ class PushSubscriptionResyncTest < ApplicationSystemTestCase
     join_room rooms(:hq)
     assert_equal first_sync, subscription.reload.updated_at, "posted again in the same session"
 
+    first_id = subscription.id
     change_password
-    assert_nil Push::Subscription.find_by(endpoint: ENDPOINT)
-
-    join_room rooms(:hq)
     assert_subscription_registered
+    assert_not_equal first_id, subscription.id, "not removed and posted again by the profile page"
     assert_not page.evaluate_script("localStorage.getItem('unsubscribed-for-test')")
   end
 
@@ -65,7 +64,7 @@ class PushSubscriptionResyncTest < ApplicationSystemTestCase
     end
 
     def assert_subscription_registered
-      assert_selector "[data-controller~=notifications]", visible: :all
+      assert_selector "[data-controller~=push-subscription-sync]", visible: :all
       Timeout.timeout(5) { sleep 0.1 until Push::Subscription.exists?(endpoint: ENDPOINT, user: users(:david)) }
     end
 

From bf2b9cc61b4e7e04dc581b791edecca9e88be3cb Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 19 Oct 2026 13:34:34 -0700
Subject: [PATCH 25/25] Look the endpoint up past the query cache, and keep a
 subscription a race refused

The lookup under the write lock repeated the one made before
validation, so the query cache could answer it with that earlier miss
even after another connection had registered the endpoint. It runs
uncached now.

Enabling notifications unsubscribed the browser on any refusal, so a
session signed out between authenticating and saving lost the browser's
subscription too, and the next session had nothing to post. Only a
refused endpoint unsubscribes it now.

From #371.

(cherry picked from commit aa93269ceb3e2173be9f57fdb3b6e9baef510586)
---
 .../users/push_subscriptions_controller.rb    | 18 ++++++-----
 .../controllers/notifications_controller.js   |  3 +-
 .../push_subscriptions_controller_test.rb     | 30 +++++++++++++++++++
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/app/controllers/users/push_subscriptions_controller.rb b/app/controllers/users/push_subscriptions_controller.rb
index a3517a83..6b33452f 100644
--- a/app/controllers/users/push_subscriptions_controller.rb
+++ b/app/controllers/users/push_subscriptions_controller.rb
@@ -28,15 +28,17 @@ class Users::PushSubscriptionsController < ApplicationController
 
     # A password change ends every session and subscription. A request that authenticated just before it mustn't
     # put one back, so the session is checked under the same write lock as the save, and the endpoint is looked up
-    # again there so two tabs posting at once register it once. Validation, which resolves the endpoint's address,
-    # runs before taking the lock.
+    # again there, past the query cache, so two tabs posting at once register it once. Validation, which resolves
+    # the endpoint's address, runs before taking the lock.
     def register_while_signed_in(subscription)
-      Push::Subscription.transaction do
-        if Session.exists?(Current.session.id)
-          if existing = @push_subscriptions.find_by(push_subscription_params)
-            existing.touch
-          else
-            subscription.save!(validate: false)
+      Push::Subscription.uncached do
+        Push::Subscription.transaction do
+          if Session.exists?(Current.session.id)
+            if existing = @push_subscriptions.find_by(push_subscription_params)
+              existing.touch
+            else
+              subscription.save!(validate: false)
+            end
           end
         end
       end
diff --git a/app/javascript/controllers/notifications_controller.js b/app/javascript/controllers/notifications_controller.js
index 3ce53a5c..f7dfe7ec 100644
--- a/app/javascript/controllers/notifications_controller.js
+++ b/app/javascript/controllers/notifications_controller.js
@@ -106,7 +106,8 @@ export default class extends Controller {
 
   async #syncPushSubscription(subscription) {
     const response = await post(this.subscriptionsUrlValue, { body: this.#extractJsonPayloadAsString(subscription), responseKind: "turbo-stream" })
-    if (!response.ok) subscription.unsubscribe()
+    // Only a refused endpoint is for good. A session signed out mid-request posts it again after signing in.
+    if (response.unprocessableEntity) subscription.unsubscribe()
   }
 
   async #requestPermissionAndSubscribe(registration) {
diff --git a/test/controllers/users/push_subscriptions_controller_test.rb b/test/controllers/users/push_subscriptions_controller_test.rb
index 7815626b..cbfacb40 100644
--- a/test/controllers/users/push_subscriptions_controller_test.rb
+++ b/test/controllers/users/push_subscriptions_controller_test.rb
@@ -139,3 +139,33 @@ class Users::PushSubscriptionsControllerTest < ActionDispatch::IntegrationTest
       { "endpoint" => "https://fcm.googleapis.com/fcm/send/abc123", "p256dh_key" => "123", "auth_key" => "456" }
     end
 end
+
+class Users::PushSubscriptionsControllerForeignWriteTest < ActionDispatch::IntegrationTest
+  self.use_transactional_tests = false
+
+  setup do
+    sign_in :david
+    stub_web_push_dns_resolution
+  end
+
+  test "a subscription another connection registers while this one validates isn't registered twice" do
+    params = { "endpoint" => "https://fcm.googleapis.com/fcm/send/foreign", "p256dh_key" => "123", "auth_key" => "456" }
+    Push::Subscription.any_instance.stubs(:valid?).with { foreign_insert(params); true }.returns(true)
+
+    post user_push_subscriptions_url, params: { push_subscription: params }
+
+    assert_response :ok
+    assert_equal 1, Push::Subscription.where(endpoint: params["endpoint"]).count
+  end
+
+  private
+    def foreign_insert(params)
+      unless @inserted
+        SQLite3::Database.new(ActiveRecord::Base.connection_db_config.database) do |database|
+          database.execute "INSERT INTO push_subscriptions (user_id, endpoint, p256dh_key, auth_key, created_at, updated_at) " \
+            "VALUES (?, ?, ?, ?, datetime('now'), datetime('now'))", [ users(:david).id, *params.values ]
+        end
+        @inserted = true
+      end
+    end
+end